[CALCITE-6563] RelToSqlConverter should not merge two window functions #3949
Conversation
| if (rel instanceof Project | ||
| && ((Project) rel).containsOver() | ||
| && maxClause == Clause.SELECT) { | ||
| // Cannot merge a Project that contains windowed functions onto an |
There was a problem hiding this comment.
you lost this comment. wasn't it useful?
There was a problem hiding this comment.
Previously, only the merging of Window functions in Project was handled here, but now window functions are handled whether they are in Project or not. So this comment doesn't quite fit anymore.
| @@ -2006,6 +2010,31 @@ private boolean hasSortByOrdinal(@UnknownInitialization Result this, | |||
| return false; | |||
| } | |||
|
|
|||
| private boolean containsOver(@UnknownInitialization Result this, | |||
There was a problem hiding this comment.
this looks very much like an unrolled visitor.
Couldn't the job be done by a shuttle?
There was a problem hiding this comment.
In this scenario, only the SqlCall in the SqlSelectList needs to be handled, so i don't think using SqlShuttle makes the code easier, WDYT
There was a problem hiding this comment.
@suibianwanwank
I added a function namedSqlUtil#hasOverto my employer.
public static boolean hasOver(SqlNode node) {
if (node == null) {
return false;
}
try {
node.accept(new SqlShuttle() {
@Override public SqlNode visit(SqlCall call) {
if (call.getKind() == SqlKind.OVER) {
throw new Util.FoundOne(call);
}
return super.visit(call);
}
});
} catch (Util.FoundOne e) {
return true;
}
return false;
}
Can you use it to solve the problem?
if (node instanceof SqlSelect) {
final SqlSelect sqlSelect = (SqlSelect) node;
for (SqlNode sqlNode: sqlSelect.getSelectList()) {
final boolean hasOver = SqlUtil.hasOver(sqlNode);
if (hasOver) {
return true;
}
}
}
There was a problem hiding this comment.
In calciteSqlUtil,we can do it like this
/**
* Returns whether a given node contains an over function.
*
* @param node AST tree
*/
public static boolean containsOver(SqlNode node) {
final Predicate<SqlCall> callPredicate = call ->
call.getKind() == SqlKind.OVER;
return containsCall(node, callPredicate);
}
But I can't be sure if the node is necessarily a SqlSelect, it might be a SqlInsert? If so, I still have to visit all the node's inputs before finding a SqlSelect.
|
I don't know why this workflow fails, is it temporary? |
Yes. Don't worry about it. |
mihaibudiu
added
the
LGTM-will-merge-soon
No description provided.