clintropolis commented on code in PR #15075:
URL: https://github.com/apache/druid/pull/15075#discussion_r1348208042
##########
processing/src/main/java/org/apache/druid/query/JoinDataSource.java:
##########
@@ -476,10 +476,18 @@ private Function<SegmentReference, SegmentReference>
createSegmentMapFunctionInt
.orElse(null)
)
);
-
+ final Function<SegmentReference, SegmentReference> baseMapFn;
+ if (left instanceof JoinDataSource) {
Review Comment:
this seems worth a comment on what is going on. Is it still ok to do if left
is not concrete?
##########
processing/src/main/java/org/apache/druid/query/JoinDataSource.java:
##########
@@ -501,20 +509,37 @@ private static Triple<DataSource, DimFilter,
List<PreJoinableClause>> flattenJoi
DimFilter currentDimFilter = null;
final List<PreJoinableClause> preJoinableClauses = new ArrayList<>();
- while (current instanceof JoinDataSource) {
- final JoinDataSource joinDataSource = (JoinDataSource) current;
- current = joinDataSource.getLeft();
- currentDimFilter = validateLeftFilter(current,
joinDataSource.getLeftFilter());
- preJoinableClauses.add(
- new PreJoinableClause(
- joinDataSource.getRightPrefix(),
- joinDataSource.getRight(),
- joinDataSource.getJoinType(),
- joinDataSource.getConditionAnalysis()
- )
- );
+ // There can be queries like
+ // Join of Unnest of Join of Unnest of Filter
+ // so these checks are needed to be ORed
+ // to get the base
+ // This also means that an addition of a new datasource
+ // Will need an instanceof check here
+ // A future work should look into if the flattenJoin
+ // can be refactored to omit these instanceof checks
+ while (current instanceof JoinDataSource || current instanceof
UnnestDataSource || current instanceof FilteredDataSource) {
+ if (current instanceof JoinDataSource) {
+ final JoinDataSource joinDataSource = (JoinDataSource) current;
+ current = joinDataSource.getLeft();
+ currentDimFilter = validateLeftFilter(current,
joinDataSource.getLeftFilter());
+ preJoinableClauses.add(
+ new PreJoinableClause(
+ joinDataSource.getRightPrefix(),
+ joinDataSource.getRight(),
+ joinDataSource.getJoinType(),
+ joinDataSource.getConditionAnalysis()
+ )
+ );
+ } else if (current instanceof UnnestDataSource) {
Review Comment:
it doesn't seem intuitive to me that we can flatten away unnest and filtered
datasources, could we add comments explaining why its ok? is it still ok if the
unnest datasource is wrapping a join datasource? like does it flatten through
it? where does the unnest and filters go in that case?
--
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]
