From 0944f2c07001809bbf1b4bb9dae4fd817094c253 Mon Sep 17 00:00:00 2001 From: Darek Date: Fri, 16 Oct 2026 19:33:25 +0200 Subject: [PATCH] fix(binder): keep a variable's name when an arithmetic identity returns it A projection names each item by setting the alias on its expression, and a variable reference binds to the scope's own object. When x + 0, x * 1, x - 0 or x / 1 simplified to that object, the item's alias renamed the variable: WITH x + 0 AS y, x RETURN y, x and RETURN x, x + 0 each returned two columns named x. An arithmetic identity over a scope expression now stays as written, so the alias lands on it. A scope expression is also never simplified again, and NOT NOT no longer unwraps a NOT from the scope. Otherwise a nested reference to a kept expression, e.g. CASE WHEN true THEN m END after WITH n + 0 AS m, was simplified into an operand that is no longer in scope, which ran an aggregate again or failed to evaluate. Boolean and CASE simplifications still return the operand and still rename it, as WITH x AS y, x does; that is unchanged. Property references are copies, so n.id + 0 = 1 still becomes an index lookup. Co-Authored-By: Claude Opus 5.5 --- src/binder/expression_binder.cpp | 21 +++--- src/include/binder/binder_scope.h | 6 ++ .../projection/simplified_variable_alias.test | 68 +++++++++++++++++++ 3 files changed, 87 insertions(+), 8 deletions(-) create mode 100644 test/test_files/projection/simplified_variable_alias.test diff --git a/src/binder/expression_binder.cpp b/src/binder/expression_binder.cpp index 367fd282e4..4d56fcdee7 100644 --- a/src/binder/expression_binder.cpp +++ b/src/binder/expression_binder.cpp @@ -183,13 +183,7 @@ std::shared_ptr ExpressionBinder::bindExpression( throw NotImplementedException( "bindExpression(" + ExpressionTypeUtil::toString(expressionType) + ")."); } - // A direct reference to a variable returns an expression of an earlier clause, simplified when - // it was bound; it is not simplified again, as a new object would escape the binder's identity - // check for expressions an earlier clause computed. References nested in a larger expression - // rely on each simplifier returning an already simplified input unchanged. - if (expressionType != ExpressionType::VARIABLE) { - expression = simplifyExpression(expression); - } + expression = simplifyExpression(expression); if (ConstantExpressionVisitor::needFold(*expression)) { return tryFoldExpression(expression); } @@ -198,6 +192,11 @@ std::shared_ptr ExpressionBinder::bindExpression( std::shared_ptr ExpressionBinder::simplifyExpression( const std::shared_ptr& expression) { + // Earlier clauses' expressions are recognised by identity, so a scope expression is never + // simplified again, and no simplifier returns a part of one in place of its parent. + if (binder->scope.contains(*expression)) { + return expression; + } switch (expression->expressionType) { case ExpressionType::AND: case ExpressionType::OR: @@ -263,6 +262,12 @@ std::shared_ptr ExpressionBinder::simplifyArithmeticExpression( if (expression->dataType != survivor->dataType) { return expression; } + // A projection names its item by setting the alias on the expression, so an identity over a + // scope expression stays as written: returning the operand would let the alias rename the + // variable, e.g. x in WITH x + 0 AS y, x. + if (binder->scope.contains(*survivor)) { + return expression; + } // Preserve the original expression name, mirroring foldExpression: without this, // e.g. RETURN t.age, t.age + 0 would produce two identically-named columns. if (expression->hasAlias()) { @@ -286,7 +291,7 @@ std::shared_ptr ExpressionBinder::simplifyBooleanExpression( if (isBoolLiteral(*child, false)) { return createLiteralExpression(Value(true)); } - if (child->expressionType == ExpressionType::NOT) { + if (child->expressionType == ExpressionType::NOT && !binder->scope.contains(*child)) { return child->getChild(0); } if (child != expression->getChild(0)) { diff --git a/src/include/binder/binder_scope.h b/src/include/binder/binder_scope.h index 508c11108d..909f04d391 100644 --- a/src/include/binder/binder_scope.h +++ b/src/include/binder/binder_scope.h @@ -1,5 +1,7 @@ #pragma once +#include + #include "binder/expression/expression.h" #include "binder/expression/node_expression.h" #include "common/case_insensitive_map.h" @@ -14,6 +16,10 @@ class BinderScope { bool empty() const { return expressions.empty(); } bool contains(const std::string& varName) const { return nameToExprIdx.contains(varName); } + bool contains(const Expression& expression) const { + return std::ranges::any_of(expressions, + [&](const auto& scopeExpr) { return scopeExpr.get() == &expression; }); + } std::shared_ptr getExpression(const std::string& varName) const { DASSERT(nameToExprIdx.contains(varName)); return expressions[nameToExprIdx.at(varName)]; diff --git a/test/test_files/projection/simplified_variable_alias.test b/test/test_files/projection/simplified_variable_alias.test new file mode 100644 index 0000000000..ad71c640b7 --- /dev/null +++ b/test/test_files/projection/simplified_variable_alias.test @@ -0,0 +1,68 @@ +-DATASET CSV empty + +-- + +-CASE ArithmeticIdentityOverVariableKeepsAlias +-STATEMENT UNWIND [1, 2] AS x WITH x + 0 AS y, x RETURN y, x +-CHECK_COLUMN_NAMES +---- 3 +y|x +1|1 +2|2 +-STATEMENT UNWIND [1, 2] AS x WITH x * 1 AS y, x RETURN y, x +-CHECK_COLUMN_NAMES +---- 3 +y|x +1|1 +2|2 +-STATEMENT UNWIND [3] AS x WITH count(*) AS n WITH n + 0 AS m RETURN m, m * 1 +-CHECK_COLUMN_NAMES +---- 2 +m|*(m,1) +1|1 +-STATEMENT UNWIND [1] AS x RETURN x, x + 0 +-CHECK_COLUMN_NAMES +---- 2 +x|+(x,0) +1|1 +-STATEMENT UNWIND [1, 2, 3] AS x WITH count(*) AS n WITH n + 0 AS m RETURN m +---- 1 +3 + +-CASE SimplifiedNodeStaysPattern +-STATEMENT CREATE NODE TABLE P(id INT64 PRIMARY KEY) +---- ok +-STATEMENT CREATE REL TABLE R(FROM P TO P) +---- ok +-STATEMENT CREATE (:P {id: 1})-[:R]->(:P {id: 2}) +---- ok +-STATEMENT MATCH (n:P) WITH CASE WHEN true THEN n END AS m MATCH (m)-[:R]->(o) RETURN m.id, o.id +---- 1 +1|2 + +-CASE SimplifiedScopeExpressionReferencedLater +-STATEMENT UNWIND [1, 2, 3] AS x WITH count(*) AS n WITH n + 0 AS m RETURN m, CASE WHEN true THEN m END AS k +---- 1 +3|3 +-STATEMENT UNWIND [1, 2] AS x WITH x + 0 AS y RETURN CASE WHEN true THEN y END AS z +---- 2 +1 +2 +-STATEMENT UNWIND [true, false] AS b WITH b AND true AS y RETURN y AND true AS r +---- 2 +True +False +-STATEMENT UNWIND [true, false] AS b WITH NOT b AS z RETURN NOT z AS r +---- 2 +True +False +-STATEMENT UNWIND [1, 2] AS x RETURN x + 0 AS y, count(*) AS c ORDER BY y * 1 +-CHECK_ORDER +---- 2 +1|1 +2|1 +-STATEMENT UNWIND [1, 2] AS x RETURN x + 0 AS y, count(*) AS c ORDER BY x + 0 +-CHECK_ORDER +---- 2 +1|1 +2|1