imply-cheddar commented on code in PR #13773:
URL: https://github.com/apache/druid/pull/13773#discussion_r1255016876
##########
sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidQuery.java:
##########
@@ -1380,11 +1383,21 @@ private WindowOperatorQuery toWindowQuery()
return null;
}
+ final DataSource myDataSource;
+ if (dataSource instanceof TableDataSource) {
Review Comment:
This code is insufficient for those cases. I expect that the Drill tests
will end up covering those cases. I.e. that's still WIP. In the "simplest"
solution, however, this should perhaps be inverted to just check if it's
already a `QueryDataSource` and if it's not, to make it one. That would cover
all of the cases and handle things as a scan.
That said, the whole planning a scan query thing is a short-term
hack/work-around anyway, I expect that in the end this will just plan the
"operator query" all the way down to the segment (which is what happens with
this workaround as well, it just happens in a really... odd... way)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
