Skip to content

Commit

Permalink
[CALCITE-6691] QUALIFY on project references wrong columns
Browse files Browse the repository at this point in the history
The bug was that we were translating the QUALIFY expression after
the SELECT clause, and the mapping was wrong if columns had moved
around of it the expressions were non-trivial. The fix is to convert
the QUALIFY expression at the same time as the HAVING expression.

Also add support for a QUALIFY clause in a query with a GROUP BY. In
this case, the QUALIFY expression may reference aggregate functions.
Previously such queries would give a ClassCastException (trying to
convert a LogicalAggregate to a LogicalProject).

If a QUALIFY expression references or duplicates an expression in the
SELECT clause, we no longer detect and deduplicate that. This has
made one or two plans more verbose. Potentially we would add back
deduplication. Or just let Calc do common subexpression elimination
later in the planning process, as it always has done.

Also refactor SqlToRelConverter. Previously we were creating some
`SqlNodeList` wrappers only to use their `accept(SqlVisitor)` method;
now we leave them as `List<SqlNode>` and call new method
`SqlVisitor.visitAll(List<SqlNode>)`.

Close #4061
  • Loading branch information
julianhyde committed Dec 20, 2024
1 parent 0badcce commit fae49bc
Show file tree
Hide file tree
Showing 5 changed files with 212 additions and 185 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
import org.apache.calcite.sql.SqlNodeList;
import org.apache.calcite.sql.SqlOperator;

import java.util.List;

/**
* Visitor class, follows the
* {@link org.apache.calcite.util.Glossary#VISITOR_PATTERN visitor pattern}.
Expand Down Expand Up @@ -103,4 +105,9 @@ public interface SqlVisitor<R> {
default R visitNode(SqlNode n) {
return n.accept(this);
}

/** Visits all nodes in a list. */
default void visitAll(List<SqlNode> selectList) {
selectList.forEach(e -> e.accept(this));
}
}
Loading

0 comments on commit fae49bc

Please sign in to comment.