Skip to content

Commit 6b87a2e

Browse files
evertlammertsevertlammerts
committed
fix None on expressions
1 parent c551c77 commit 6b87a2e

2 files changed

Lines changed: 142 additions & 47 deletions

File tree

‎src/duckdb_py/pyexpression/initialize.cpp‎

Lines changed: 73 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,44 @@
66

77
namespace duckdb {
88

9+
namespace {
10+
11+
// Binary operators take their operand as py::object (not Expression) so that None can bind: nanobind rejects None for a
12+
// bound-type parameter before the registered implicit conversion runs, so `expr == None` / `expr + None` would never
13+
// reach the None -> SQL NULL conversion otherwise. We convert explicitly via TryToExpression (an existing Expression is
14+
// copied, a str becomes a column reference, any other value -- including None -- becomes a constant). On a genuinely
15+
// unconvertible operand we return Py_NotImplemented so Python falls back to the reflected operator / identity
16+
// comparison, exactly as the is_operator() overload did under pybind11 (keeps e.g. `expr == object()` returning False
17+
// instead of raising).
18+
template <typename Build>
19+
py::object ExpressionBinaryOp(const py::object &other, Build &&build) {
20+
std::unique_ptr<DuckDBPyExpression> converted;
21+
if (!DuckDBPyExpression::TryToExpression(other, converted)) {
22+
return py::borrow(py::handle(Py_NotImplemented));
23+
}
24+
return py::cast(build(*converted));
25+
}
26+
27+
} // namespace
28+
29+
// Forward binary operator __op__: self <op> other (other converted via ExpressionBinaryOp, so None -> SQL NULL).
30+
#define DUCKDB_EXPR_BINARY_OP(PYNAME, METHOD) \
31+
m.def( \
32+
PYNAME, \
33+
[](DuckDBPyExpression &self, const py::object &other) { \
34+
return ExpressionBinaryOp(other, [&](const DuckDBPyExpression &rhs) { return self.METHOD(rhs); }); \
35+
}, \
36+
py::arg("expr").none(), docs, py::is_operator())
37+
38+
// Reflected binary operator __rop__: other <op> self (other is the left operand, also accepts None).
39+
#define DUCKDB_EXPR_REFLECTED_OP(PYNAME, METHOD) \
40+
m.def( \
41+
PYNAME, \
42+
[](DuckDBPyExpression &self, const py::object &other) { \
43+
return ExpressionBinaryOp(other, [&](const DuckDBPyExpression &lhs) { return lhs.METHOD(self); }); \
44+
}, \
45+
py::arg("expr").none(), docs, py::is_operator())
46+
947
void InitializeStaticMethods(py::module_ &m) {
1048
const char *docs;
1149

@@ -62,10 +100,8 @@ static void InitializeDunderMethods(py::class_<DuckDBPyExpression> &m) {
62100
FunctionExpression: self '+' expr
63101
)";
64102

65-
m.def("__add__", &DuckDBPyExpression::Add, py::arg("expr"), docs, py::is_operator());
66-
m.def(
67-
"__radd__", [](const DuckDBPyExpression &a, const DuckDBPyExpression &b) { return b.Add(a); }, docs,
68-
py::is_operator());
103+
DUCKDB_EXPR_BINARY_OP("__add__", Add);
104+
DUCKDB_EXPR_REFLECTED_OP("__radd__", Add);
69105

70106
docs = R"(
71107
Negate the expression.
@@ -84,10 +120,8 @@ static void InitializeDunderMethods(py::class_<DuckDBPyExpression> &m) {
84120
Returns:
85121
FunctionExpression: self '-' expr
86122
)";
87-
m.def("__sub__", &DuckDBPyExpression::Subtract, docs, py::is_operator());
88-
m.def(
89-
"__rsub__", [](const DuckDBPyExpression &a, const DuckDBPyExpression &b) { return b.Subtract(a); }, docs,
90-
py::is_operator());
123+
DUCKDB_EXPR_BINARY_OP("__sub__", Subtract);
124+
DUCKDB_EXPR_REFLECTED_OP("__rsub__", Subtract);
91125

92126
docs = R"(
93127
Multiply self by expr
@@ -98,10 +132,8 @@ static void InitializeDunderMethods(py::class_<DuckDBPyExpression> &m) {
98132
Returns:
99133
FunctionExpression: self '*' expr
100134
)";
101-
m.def("__mul__", &DuckDBPyExpression::Multiply, docs, py::is_operator());
102-
m.def(
103-
"__rmul__", [](const DuckDBPyExpression &a, const DuckDBPyExpression &b) { return b.Multiply(a); }, docs,
104-
py::is_operator());
135+
DUCKDB_EXPR_BINARY_OP("__mul__", Multiply);
136+
DUCKDB_EXPR_REFLECTED_OP("__rmul__", Multiply);
105137

106138
docs = R"(
107139
Divide self by expr
@@ -112,15 +144,11 @@ static void InitializeDunderMethods(py::class_<DuckDBPyExpression> &m) {
112144
Returns:
113145
FunctionExpression: self '/' expr
114146
)";
115-
m.def("__div__", &DuckDBPyExpression::Division, docs, py::is_operator());
116-
m.def(
117-
"__rdiv__", [](const DuckDBPyExpression &a, const DuckDBPyExpression &b) { return b.Division(a); }, docs,
118-
py::is_operator());
147+
DUCKDB_EXPR_BINARY_OP("__div__", Division);
148+
DUCKDB_EXPR_REFLECTED_OP("__rdiv__", Division);
119149

120-
m.def("__truediv__", &DuckDBPyExpression::Division, docs, py::is_operator());
121-
m.def(
122-
"__rtruediv__", [](const DuckDBPyExpression &a, const DuckDBPyExpression &b) { return b.Division(a); }, docs,
123-
py::is_operator());
150+
DUCKDB_EXPR_BINARY_OP("__truediv__", Division);
151+
DUCKDB_EXPR_REFLECTED_OP("__rtruediv__", Division);
124152

125153
docs = R"(
126154
(Floor) Divide self by expr
@@ -131,10 +159,8 @@ static void InitializeDunderMethods(py::class_<DuckDBPyExpression> &m) {
131159
Returns:
132160
FunctionExpression: self '//' expr
133161
)";
134-
m.def("__floordiv__", &DuckDBPyExpression::FloorDivision, docs, py::is_operator());
135-
m.def(
136-
"__rfloordiv__", [](const DuckDBPyExpression &a, const DuckDBPyExpression &b) { return b.FloorDivision(a); },
137-
docs, py::is_operator());
162+
DUCKDB_EXPR_BINARY_OP("__floordiv__", FloorDivision);
163+
DUCKDB_EXPR_REFLECTED_OP("__rfloordiv__", FloorDivision);
138164

139165
docs = R"(
140166
Modulo self by expr
@@ -145,10 +171,8 @@ static void InitializeDunderMethods(py::class_<DuckDBPyExpression> &m) {
145171
Returns:
146172
FunctionExpression: self '%' expr
147173
)";
148-
m.def("__mod__", &DuckDBPyExpression::Modulo, docs, py::is_operator());
149-
m.def(
150-
"__rmod__", [](const DuckDBPyExpression &a, const DuckDBPyExpression &b) { return b.Modulo(a); }, docs,
151-
py::is_operator());
174+
DUCKDB_EXPR_BINARY_OP("__mod__", Modulo);
175+
DUCKDB_EXPR_REFLECTED_OP("__rmod__", Modulo);
152176

153177
docs = R"(
154178
Power self by expr
@@ -159,10 +183,8 @@ static void InitializeDunderMethods(py::class_<DuckDBPyExpression> &m) {
159183
Returns:
160184
FunctionExpression: self '**' expr
161185
)";
162-
m.def("__pow__", &DuckDBPyExpression::Power, docs, py::is_operator());
163-
m.def(
164-
"__rpow__", [](const DuckDBPyExpression &a, const DuckDBPyExpression &b) { return b.Power(a); }, docs,
165-
py::is_operator());
186+
DUCKDB_EXPR_BINARY_OP("__pow__", Power);
187+
DUCKDB_EXPR_REFLECTED_OP("__rpow__", Power);
166188

167189
docs = R"(
168190
Create an equality expression between two expressions
@@ -173,7 +195,7 @@ static void InitializeDunderMethods(py::class_<DuckDBPyExpression> &m) {
173195
Returns:
174196
FunctionExpression: self '=' expr
175197
)";
176-
m.def("__eq__", &DuckDBPyExpression::Equality, docs, py::is_operator());
198+
DUCKDB_EXPR_BINARY_OP("__eq__", Equality);
177199

178200
docs = R"(
179201
Create an inequality expression between two expressions
@@ -184,7 +206,7 @@ static void InitializeDunderMethods(py::class_<DuckDBPyExpression> &m) {
184206
Returns:
185207
FunctionExpression: self '!=' expr
186208
)";
187-
m.def("__ne__", &DuckDBPyExpression::Inequality, docs, py::is_operator());
209+
DUCKDB_EXPR_BINARY_OP("__ne__", Inequality);
188210

189211
docs = R"(
190212
Create a greater than expression between two expressions
@@ -195,7 +217,7 @@ static void InitializeDunderMethods(py::class_<DuckDBPyExpression> &m) {
195217
Returns:
196218
FunctionExpression: self '>' expr
197219
)";
198-
m.def("__gt__", &DuckDBPyExpression::GreaterThan, docs, py::is_operator());
220+
DUCKDB_EXPR_BINARY_OP("__gt__", GreaterThan);
199221

200222
docs = R"(
201223
Create a greater than or equal expression between two expressions
@@ -206,7 +228,7 @@ static void InitializeDunderMethods(py::class_<DuckDBPyExpression> &m) {
206228
Returns:
207229
FunctionExpression: self '>=' expr
208230
)";
209-
m.def("__ge__", &DuckDBPyExpression::GreaterThanOrEqual, docs, py::is_operator());
231+
DUCKDB_EXPR_BINARY_OP("__ge__", GreaterThanOrEqual);
210232

211233
docs = R"(
212234
Create a less than expression between two expressions
@@ -217,7 +239,7 @@ static void InitializeDunderMethods(py::class_<DuckDBPyExpression> &m) {
217239
Returns:
218240
FunctionExpression: self '<' expr
219241
)";
220-
m.def("__lt__", &DuckDBPyExpression::LessThan, docs, py::is_operator());
242+
DUCKDB_EXPR_BINARY_OP("__lt__", LessThan);
221243

222244
docs = R"(
223245
Create a less than or equal expression between two expressions
@@ -228,7 +250,7 @@ static void InitializeDunderMethods(py::class_<DuckDBPyExpression> &m) {
228250
Returns:
229251
FunctionExpression: self '<=' expr
230252
)";
231-
m.def("__le__", &DuckDBPyExpression::LessThanOrEqual, docs, py::is_operator());
253+
DUCKDB_EXPR_BINARY_OP("__le__", LessThanOrEqual);
232254

233255
// AND, NOT and OR
234256

@@ -241,7 +263,7 @@ static void InitializeDunderMethods(py::class_<DuckDBPyExpression> &m) {
241263
Returns:
242264
FunctionExpression: self '&' expr
243265
)";
244-
m.def("__and__", &DuckDBPyExpression::And, docs, py::is_operator());
266+
DUCKDB_EXPR_BINARY_OP("__and__", And);
245267

246268
docs = R"(
247269
Binary-or self together with expr
@@ -252,7 +274,7 @@ static void InitializeDunderMethods(py::class_<DuckDBPyExpression> &m) {
252274
Returns:
253275
FunctionExpression: self '|' expr
254276
)";
255-
m.def("__or__", &DuckDBPyExpression::Or, docs, py::is_operator());
277+
DUCKDB_EXPR_BINARY_OP("__or__", Or);
256278

257279
docs = R"(
258280
Create a binary-not expression from self
@@ -271,9 +293,7 @@ static void InitializeDunderMethods(py::class_<DuckDBPyExpression> &m) {
271293
Returns:
272294
FunctionExpression: expr '&' self
273295
)";
274-
m.def(
275-
"__rand__", [](const DuckDBPyExpression &a, const DuckDBPyExpression &b) { return b.And(a); }, docs,
276-
py::is_operator());
296+
DUCKDB_EXPR_REFLECTED_OP("__rand__", And);
277297

278298
docs = R"(
279299
Binary-or self together with expr
@@ -284,11 +304,12 @@ static void InitializeDunderMethods(py::class_<DuckDBPyExpression> &m) {
284304
Returns:
285305
FunctionExpression: expr '|' self
286306
)";
287-
m.def(
288-
"__ror__", [](const DuckDBPyExpression &a, const DuckDBPyExpression &b) { return b.Or(a); }, docs,
289-
py::is_operator());
307+
DUCKDB_EXPR_REFLECTED_OP("__ror__", Or);
290308
}
291309

310+
#undef DUCKDB_EXPR_BINARY_OP
311+
#undef DUCKDB_EXPR_REFLECTED_OP
312+
292313
static void InitializeImplicitConversion(py::class_<DuckDBPyExpression> &m) {
293314
m.def(py::new_([](const string &name) {
294315
auto names = py::cast<py::args>(py::make_tuple(py::str(name.c_str(), name.size())));
@@ -425,7 +446,12 @@ void DuckDBPyExpression::Initialize(py::module_ &m) {
425446
expression.def("cast", &DuckDBPyExpression::Cast, py::arg("type"), docs);
426447

427448
docs = "";
428-
expression.def("between", &DuckDBPyExpression::Between, py::arg("lower"), py::arg("upper"), docs);
449+
expression.def(
450+
"between",
451+
[](DuckDBPyExpression &self, const py::object &lower, const py::object &upper) {
452+
return self.Between(*DuckDBPyExpression::ToExpression(lower), *DuckDBPyExpression::ToExpression(upper));
453+
},
454+
py::arg("lower").none(), py::arg("upper").none(), docs);
429455

430456
docs = "";
431457
expression.def("collate", &DuckDBPyExpression::Collate, py::arg("collation"), docs);

‎tests/fast/test_expression_implicit_conversion.py‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,79 @@ def rel():
9191
def test_binary_operator_constant_rhs(rel, value, column):
9292
"""Expression == <constant> should work for every constant type."""
9393
expr = ColumnExpression(column) == value
94+
# `==` must build a SQL Expression, never fall back to a Python bool: a bool RHS would still let
95+
# select() yield one row, masking a None/operator regression -- so assert the type explicitly.
96+
assert isinstance(expr, duckdb.Expression)
9497
result = rel.select(expr).fetchall()
9598
assert len(result) == 1
9699

97100

101+
# ---------------------------------------------------------------------------
102+
# 1b. None operand: None is a meaningful value (SQL NULL), not "argument absent".
103+
# nanobind gates None for bound-type params before implicit conversion, so the
104+
# operators/between take py::object + route None through ToExpression -> NULL constant.
105+
# These guard the P0 (`== None` -> Python bool) and P1 (operators/between raise on None).
106+
# ---------------------------------------------------------------------------
107+
108+
109+
@pytest.mark.parametrize(
110+
"build",
111+
[
112+
lambda c: c == None, # noqa: E711
113+
lambda c: c != None, # noqa: E711
114+
lambda c: c + None,
115+
lambda c: c - None,
116+
lambda c: c * None,
117+
lambda c: c < None,
118+
lambda c: c > None,
119+
lambda c: c & None,
120+
lambda c: c | None,
121+
lambda c: c.between(None, 5),
122+
lambda c: c.between(1, None),
123+
lambda c: None + c, # reflected (__radd__)
124+
lambda c: None & c, # reflected (__rand__)
125+
],
126+
ids=[
127+
"eq",
128+
"ne",
129+
"add",
130+
"sub",
131+
"mul",
132+
"lt",
133+
"gt",
134+
"and",
135+
"or",
136+
"between_lower",
137+
"between_upper",
138+
"reflected_add",
139+
"reflected_and",
140+
],
141+
)
142+
def test_none_operand_builds_sql_null_expression(build):
143+
"""A None operand becomes a SQL NULL constant on every operator/between, yielding a real Expression."""
144+
expr = build(ColumnExpression("a"))
145+
assert isinstance(expr, duckdb.Expression)
146+
assert "NULL" in str(expr)
147+
148+
149+
def test_none_filter_keeps_no_rows():
150+
"""`col != None` builds `(col != NULL)`: SQL NULL semantics keep no rows (a Python-bool True kept all)."""
151+
rel = duckdb.connect().sql("SELECT * FROM (VALUES (1), (NULL), (3)) t(a)")
152+
assert rel.filter(ColumnExpression("a") != None).fetchall() == [] # noqa: E711
153+
154+
155+
def test_unconvertible_operand_preserves_notimplemented():
156+
"""An unconvertible operand must still yield NotImplemented so Python falls back.
157+
158+
`expr == object()` stays a bool, `expr + object()` raises TypeError -- not a thrown duckdb error.
159+
"""
160+
a = ColumnExpression("a")
161+
assert (a == object()) is False
162+
assert (a != object()) is True
163+
with pytest.raises(TypeError):
164+
a + object()
165+
166+
98167
# ---------------------------------------------------------------------------
99168
# 2. Binary operator with str: str becomes a ColumnExpression (column ref)
100169
# ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)