imply-cheddar commented on code in PR #13501:
URL: https://github.com/apache/druid/pull/13501#discussion_r1042118533
##########
processing/src/main/java/org/apache/druid/query/QueryDataSource.java:
##########
@@ -112,6 +113,25 @@ public byte[] getCacheKey()
return null;
}
+ @Override
+ public DataSourceAnalysis getAnalysisForDataSource()
+ {
+ final Query<?> subQuery = this.getQuery();
+ if (!(subQuery instanceof BaseQuery)) {
+ // We must verify that the subQuery is a BaseQuery, because it is
required to make
+ // "DataSourceAnalysis.getBaseQuerySegmentSpec" work properly.
+ // All built-in query types are BaseQuery, so we only expect this with
funky extension queries.
+ throw new IAE("Cannot analyze subquery of class[%s]",
subQuery.getClass().getName());
+ }
+ final DataSource current = subQuery.getDataSource();
+ DataSourceAnalysis currentAnalysis = current.getAnalysisForDataSource();
+ // check if the baseQuery is already present while moving up
+ // if not add the current query as the baseQuery to the analysis
+ if (!currentAnalysis.getBaseQuery().isPresent()) {
Review Comment:
It would be safer for `DataSourceAnalysis` to encapsulate this check. Maybe
name the method `maybeWithBaseQuery(subQuery)` and move the if statement inside
of that.
##########
processing/src/main/java/org/apache/druid/query/JoinDataSource.java:
##########
@@ -479,4 +481,47 @@ public byte[] getCacheKey()
}
return keyBuilder.build();
}
+
+ @Override
+ public DataSourceAnalysis getAnalysisForDataSource()
Review Comment:
The result of this is cached in the constructor, is there a reason not to
return that and move the logic into a private method?
--
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]