You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "kgyrtkirk (via GitHub)" <gi...@apache.org> on 2025/01/15 14:28:46 UTC
Re: [PR] Updating plans when using joins with unnest on the left (druid)
kgyrtkirk commented on code in PR #15075:
URL: https://github.com/apache/druid/pull/15075#discussion_r1916747042
##########
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 means that processing will stop at the 1st nested join!
it could only be `Join -> Unnest -> Join`
and never `Join -> Join -> Unnest` ?
##########
processing/src/main/java/org/apache/druid/query/JoinDataSource.java:
##########
@@ -501,18 +516,39 @@ 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 method is called to get the analysis for the join data source
+ // Since the analysis of an UnnestDS or FilteredDS always delegates to its base
+ // To obtain the base data source underneath a Join
+ // we also iterate through the base of the FilterDS and UnnestDS in its path
+ // the base of which can be a concrete data source
+ // 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) {
+ final UnnestDataSource unnestDataSource = (UnnestDataSource) current;
+ current = unnestDataSource.getBase();
+ } else {
+ final FilteredDataSource filteredDataSource = (FilteredDataSource) current;
+ current = filteredDataSource.getBase();
+ }
Review Comment:
this logic is wrong ; the meaning of these datasources are discarded as `preJoinableClauses` are taken into account and nothing else
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org