kgyrtkirk commented on code in PR #15241:
URL: https://github.com/apache/druid/pull/15241#discussion_r1375189099
##########
sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidOuterQueryRel.java:
##########
@@ -47,7 +47,7 @@
*/
public class DruidOuterQueryRel extends DruidRel<DruidOuterQueryRel>
{
- private static final TableDataSource DUMMY_DATA_SOURCE = new
TableDataSource("__subquery__");
+ public static final TableDataSource DUMMY_DATA_SOURCE = new
TableDataSource("__subquery__");
Review Comment:
yes - totally agree that is not the best like this...
I think its rather odd to pass this constant
[here](https://github.com/apache/druid/blob/737947754dfed23e649d980e2fc71b33ecb6e479/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidOuterQueryRel.java#L123)
as its being used to show the internal parts that this outer query has a
"TableDataSource"...
I think we should probably
* remove that `?:`:
* change the `DUMMY_QUERY_DATA_SOURCE` to not look like a `ScanQuery` -
instead some opaque `query`; so that it can't interfere with things
* possibly reconsider the `instanceof` at
[DruidQuery#computeQuery](https://github.com/apache/druid/blob/737947754dfed23e649d980e2fc71b33ecb6e479/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidQuery.java#L981)
as when there is no `window` because of that `?:` it essentially never
becomes true (during early stages of planning) because it passes a non-query
datasource ....
...but I can try the `isConcreate()` impl approach for now - but if that
breaks stuff; I would probably go back to the current reference checking
approach - and submit the above changes in a followup PR
--
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]
