Skip to content

Commit 27f69ff

Browse files
committed
fix: fix bind errors and add parameter validation
1 parent ee5c8b9 commit 27f69ff

File tree

4 files changed

+37
-15
lines changed

4 files changed

+37
-15
lines changed

src/textplot_bar.cpp

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ struct TextplotBarBindData : public FunctionData {
6868
} else if (shape == "heart") {
6969
lookup_map = &emoji_hearts;
7070
} else {
71-
throw BinderException("tp_bar: 'shape' argument must be one of 'square', 'circle', or 'heart'");
71+
throw BinderException("tp_bar: 'shape' argument must be one of 'square', 'circle', 'heart'");
7272
}
7373

7474
if (!color.empty()) {
@@ -100,7 +100,10 @@ unique_ptr<FunctionData> TextplotBarBindData::Copy() const {
100100
}
101101

102102
bool TextplotBarBindData::Equals(const FunctionData &other_p) const {
103-
return true;
103+
const auto &other = other_p.Cast<TextplotBarBindData>();
104+
return min == other.min && max == other.max && width == other.width && on == other.on && off == other.off &&
105+
filled == other.filled && thresholds == other.thresholds && char_shape == other.char_shape &&
106+
on_color == other.on_color && off_color == other.off_color;
104107
}
105108

106109
unique_ptr<FunctionData> TextplotBarBind(ClientContext &context, ScalarFunction &bound_function,
@@ -221,10 +224,18 @@ unique_ptr<FunctionData> TextplotBarBind(ClientContext &context, ScalarFunction
221224
shape = "square";
222225
} else {
223226
if (shape != "square" && shape != "circle" && shape != "heart") {
224-
throw BinderException("tp_bar: 'shape' argument must be one of 'square', 'circle', or 'heart'");
227+
throw BinderException("tp_bar: 'shape' argument must be one of 'square', 'circle', 'heart'");
225228
}
226229
}
227230

231+
if (width < 1) {
232+
throw BinderException("tp_bar: 'width' argument must be at least 1");
233+
}
234+
235+
if (min >= max) {
236+
throw BinderException("tp_bar: 'min' must be less than 'max'");
237+
}
238+
228239
return make_uniq<TextplotBarBindData>(min, max, width, on, off, filled, thresholds, shape, on_color, off_color);
229240
}
230241

src/textplot_density.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,6 @@ const std::unordered_map<std::string, std::vector<std::string>> density_sets = {
2525
{"white", {" ", "", "🔘", ""}},
2626
};
2727

28-
// Density plot character sets
29-
extern const std::unordered_map<std::string, std::vector<std::string>> density_sets;
30-
3128
// Density plot bind data structure
3229
struct TextplotDensityBindData : public FunctionData {
3330
int64_t width = 20;
@@ -42,7 +39,8 @@ struct TextplotDensityBindData : public FunctionData {
4239
return make_uniq<TextplotDensityBindData>(width, density_chars, marker_char);
4340
}
4441
bool Equals(const FunctionData &other_p) const override {
45-
return true;
42+
const auto &other = other_p.Cast<TextplotDensityBindData>();
43+
return width == other.width && density_chars == other.density_chars && marker_char == other.marker_char;
4644
}
4745
};
4846

@@ -128,6 +126,11 @@ unique_ptr<FunctionData> TextplotDensityBind(ClientContext &context, ScalarFunct
128126
throw BinderException(StringUtil::Format("tp_density: Unknown style '%s'", style));
129127
}
130128
}
129+
130+
if (width < 1) {
131+
throw BinderException("tp_density: 'width' argument must be at least 1");
132+
}
133+
131134
return make_uniq<TextplotDensityBindData>(width, graph_characters, marker_char);
132135
}
133136

src/textplot_qr.cpp

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ unique_ptr<FunctionData> TextplotQRBindData::Copy() const {
2727
}
2828

2929
bool TextplotQRBindData::Equals(const FunctionData &other_p) const {
30-
return true;
30+
const auto &other = other_p.Cast<TextplotQRBindData>();
31+
return ecc == other.ecc && on == other.on && off == other.off;
3132
}
3233

3334
unique_ptr<FunctionData> TextplotQRBind(ClientContext &context, ScalarFunction &bound_function,
@@ -75,6 +76,11 @@ unique_ptr<FunctionData> TextplotQRBind(ClientContext &context, ScalarFunction &
7576
}
7677
}
7778

79+
// Validate ECC at bind time
80+
if (ecc != "low" && ecc != "medium" && ecc != "quartile" && ecc != "high") {
81+
throw BinderException("tp_qr: 'ecc' argument must be one of 'low', 'medium', 'quartile', 'high'");
82+
}
83+
7884
if (off.empty()) {
7985
off = "";
8086
}
@@ -97,17 +103,14 @@ void TextplotQR(DataChunk &args, ExpressionState &state, Vector &result) {
97103
// auto qr = qrcodegen::QrCode::encodeSegments(segs, qrcodegen::QrCode::Ecc::LOW, bind_data.min_version,
98104
// bind_data.max_version, -1, bind_data.boost_ecc);
99105

106+
// ECC is already validated at bind time
100107
qrcodegen::QrCode::Ecc ecc_level = qrcodegen::QrCode::Ecc::LOW;
101-
if (bind_data.ecc == "low") {
102-
ecc_level = qrcodegen::QrCode::Ecc::LOW;
103-
} else if (bind_data.ecc == "medium") {
108+
if (bind_data.ecc == "medium") {
104109
ecc_level = qrcodegen::QrCode::Ecc::MEDIUM;
105110
} else if (bind_data.ecc == "quartile") {
106111
ecc_level = qrcodegen::QrCode::Ecc::QUARTILE;
107112
} else if (bind_data.ecc == "high") {
108113
ecc_level = qrcodegen::QrCode::Ecc::HIGH;
109-
} else {
110-
throw InvalidTypeException("tp_qr: 'ecc' argument must be one of 'low', 'medium', 'quartile', 'high'");
111114
}
112115

113116
auto qr = qrcodegen::QrCode::encodeText(value.GetString().c_str(), ecc_level);

src/textplot_sparkline.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,8 @@ unique_ptr<FunctionData> TextplotSparklineBindData::Copy() const {
283283
}
284284

285285
bool TextplotSparklineBindData::Equals(const FunctionData &other_p) const {
286-
return true;
286+
const auto &other = other_p.Cast<TextplotSparklineBindData>();
287+
return mode == other.mode && theme == other.theme && width == other.width;
287288
}
288289

289290
unique_ptr<FunctionData> TextplotSparklineBind(ClientContext &context, ScalarFunction &bound_function,
@@ -359,10 +360,14 @@ unique_ptr<FunctionData> TextplotSparklineBind(ClientContext &context, ScalarFun
359360
}
360361
}
361362
if (std::find(available_themes.begin(), available_themes.end(), theme) == available_themes.end()) {
362-
throw BinderException(StringUtil::Format("tp_sparkline: Unknown theme '%s' for type '%s' available are <%s>",
363+
throw BinderException(StringUtil::Format("tp_sparkline: Unknown theme '%s' for mode '%s', available are <%s>",
363364
theme, specified_mode, StringUtil::Join(available_themes, ", ")));
364365
}
365366

367+
if (width < 1) {
368+
throw BinderException("tp_sparkline: 'width' argument must be at least 1");
369+
}
370+
366371
return make_uniq<TextplotSparklineBindData>(mode, theme, width);
367372
}
368373

0 commit comments

Comments
 (0)