diff --git a/src/binder/expression_binder.cpp b/src/binder/expression_binder.cpp index 367fd282e..4d56fcdee 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 508c11108..909f04d39 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 000000000..ad71c640b --- /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