This is an automated email from the ASF dual-hosted git repository.
gianm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new b6d638b067e fix: Properly compute virtual column chains post-join.
(#20538)
b6d638b067e is described below
commit b6d638b067ed68fc624826e6f12aec08e89d8aa0
Author: Gian Merlino <[email protected]>
AuthorDate: Thu Oct 8 22:33:09 2026 -0700
fix: Properly compute virtual column chains post-join. (#20538)
Previously, when separating virtual columns into pre- and post-join
sets, we would only look one layer deep. This patch changes the
logic to look through multiple layers, so chains of virtual columns
that depend on post-join inputs are properly moved post-join in
their entirety.
---
.../segment/join/HashJoinSegmentCursorFactory.java | 2 +-
.../druid/segment/join/filter/JoinableClauses.java | 39 ++++++++-
.../join/HashJoinSegmentCursorFactoryTest.java | 96 ++++++++++++++++++++++
.../druid/sql/calcite/CalciteJoinQueryTest.java | 49 +++++++++++
4 files changed, 183 insertions(+), 3 deletions(-)
diff --git
a/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegmentCursorFactory.java
b/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegmentCursorFactory.java
index a7030fae546..e952520b608 100644
---
a/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegmentCursorFactory.java
+++
b/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegmentCursorFactory.java
@@ -231,7 +231,7 @@ public class HashJoinSegmentCursorFactory implements
CursorFactory
Iterables.concat(
Sets.difference(
ImmutableSet.copyOf(spec.getVirtualColumns().getVirtualColumns()),
- joinFilterPreAnalysis.getPostJoinVirtualColumns()
+ actualPreAnalysis.getPostJoinVirtualColumns()
),
joinFilterSplit.getPushDownVirtualColumns()
)
diff --git
a/processing/src/main/java/org/apache/druid/segment/join/filter/JoinableClauses.java
b/processing/src/main/java/org/apache/druid/segment/join/filter/JoinableClauses.java
index 5774bab5708..c8afbdeccad 100644
---
a/processing/src/main/java/org/apache/druid/segment/join/filter/JoinableClauses.java
+++
b/processing/src/main/java/org/apache/druid/segment/join/filter/JoinableClauses.java
@@ -32,8 +32,10 @@ import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
+import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
@@ -91,15 +93,17 @@ public class JoinableClauses
}
/**
- * Retrieve subset of virtual columns which require inputs which are only
present on the join table
+ * Retrieve subset of virtual columns which require inputs which are only
present on the join table, either
+ * directly or through other virtual columns.
*/
public Set<VirtualColumn> getPostJoinVirtualColumns(
final VirtualColumns virtualColumns
)
{
final Set<VirtualColumn> postJoinVirtualColumns = new HashSet<>();
+ final Map<String, Boolean> postJoinByName = new HashMap<>();
for (VirtualColumn virtualColumn : virtualColumns.getVirtualColumns()) {
- if (areSomeColumnsFromJoin(virtualColumn.requiredColumns())) {
+ if (isPostJoinVirtualColumn(virtualColumn, virtualColumns,
postJoinByName)) {
postJoinVirtualColumns.add(virtualColumn);
}
}
@@ -132,6 +136,37 @@ public class JoinableClauses
return null;
}
+ /**
+ * Whether a virtual column requires a join table column, directly or
through other virtual columns in
+ * {@code virtualColumns}. Memoizes results in {@code postJoinByName}.
Recursion terminates because
+ * {@link VirtualColumns} rejects dependency cycles.
+ */
+ private boolean isPostJoinVirtualColumn(
+ final VirtualColumn virtualColumn,
+ final VirtualColumns virtualColumns,
+ final Map<String, Boolean> postJoinByName
+ )
+ {
+ final Boolean memoized = postJoinByName.get(virtualColumn.getOutputName());
+ if (memoized != null) {
+ return memoized;
+ }
+
+ boolean postJoin = areSomeColumnsFromJoin(virtualColumn.requiredColumns());
+ if (!postJoin) {
+ for (String column : virtualColumn.requiredColumns()) {
+ final VirtualColumn dependency =
virtualColumns.getVirtualColumn(column);
+ if (dependency != null && isPostJoinVirtualColumn(dependency,
virtualColumns, postJoinByName)) {
+ postJoin = true;
+ break;
+ }
+ }
+ }
+
+ postJoinByName.put(virtualColumn.getOutputName(), postJoin);
+ return postJoin;
+ }
+
private static void checkPreJoinableClausesForDuplicatesAndShadowing(
final List<PreJoinableClause> preJoinableClauses
)
diff --git
a/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentCursorFactoryTest.java
b/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentCursorFactoryTest.java
index aa9fb0eab3c..60e89bff6b0 100644
---
a/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentCursorFactoryTest.java
+++
b/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentCursorFactoryTest.java
@@ -39,6 +39,7 @@ import org.apache.druid.segment.CursorHolder;
import org.apache.druid.segment.DeferredCursorFactory;
import org.apache.druid.segment.ReferenceCountedSegmentProvider;
import org.apache.druid.segment.TopNOptimizationInspector;
+import org.apache.druid.segment.VirtualColumn;
import org.apache.druid.segment.VirtualColumns;
import org.apache.druid.segment.column.ColumnCapabilities;
import org.apache.druid.segment.column.ValueType;
@@ -1453,6 +1454,101 @@ public class HashJoinSegmentCursorFactoryTest extends
BaseHashJoinSegmentCursorF
);
}
+ @Test
+ public void
test_makeCursor_factToCountryInnerUsingChainedPostJoinVirtualColumns()
+ {
+ // "v1" refers to the join table only through "v0", so it must be computed
post-join along with "v0".
+ List<JoinableClause> joinableClauses =
ImmutableList.of(factToCountryOnIsoCode(JoinType.INNER));
+
+ VirtualColumns virtualColumns = VirtualColumns.create(
+ makeExpressionVirtualColumn(
+ StringUtils.format("upper(\"%scountryName\")",
FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX),
+ "v0"
+ ),
+ makeExpressionVirtualColumn("concat(\"v0\", '!')", "v1")
+ );
+
+ Filter filter = new SelectorDimFilter("v1", "CANADA!", null).toFilter();
+
+ JoinFilterPreAnalysis joinFilterPreAnalysis = makeDefaultConfigPreAnalysis(
+ filter,
+ joinableClauses,
+ virtualColumns
+ );
+ JoinTestHelper.verifyCursor(
+ new HashJoinSegmentCursorFactory(
+ factSegment.as(CursorFactory.class),
+ null,
+ joinableClauses,
+ joinFilterPreAnalysis
+ ).makeCursorHolder(
+
CursorBuildSpec.builder().setFilter(filter).setVirtualColumns(virtualColumns).build()
+ ),
+ ImmutableList.of(
+ "page",
+ "countryIsoCode",
+ "v0",
+ "v1"
+ ),
+ ImmutableList.of(
+ new Object[]{"Didier Leclair", "CA", "CANADA", "CANADA!"},
+ new Object[]{"Les Argonautes", "CA", "CANADA", "CANADA!"},
+ new Object[]{"Sarah Michelle Gellar", "CA", "CANADA", "CANADA!"}
+ )
+ );
+ }
+
+ @Test
+ public void
test_makeCursor_factToCountryInnerUsingVirtualColumnsThatDifferFromPreAnalysis()
+ {
+ // The pre-analysis sees "v1" as post-join, through "v0". The cursor build
spec redefines "v0" on the base table,
+ // making "v1" pre-join, so the cursor must classify virtual columns using
its own spec, not the pre-analysis.
+ List<JoinableClause> joinableClauses =
ImmutableList.of(factToCountryOnIsoCode(JoinType.INNER));
+
+ Filter filter = new SelectorDimFilter("countryIsoCode", "CA",
null).toFilter();
+
+ VirtualColumn v1 = makeExpressionVirtualColumn("concat(\"v0\", '!')",
"v1");
+
+ JoinFilterPreAnalysis joinFilterPreAnalysis = makeDefaultConfigPreAnalysis(
+ filter,
+ joinableClauses,
+ VirtualColumns.create(
+ makeExpressionVirtualColumn(
+ StringUtils.format("upper(\"%scountryName\")",
FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX),
+ "v0"
+ ),
+ v1
+ )
+ );
+
+ VirtualColumns virtualColumns = VirtualColumns.create(
+ makeExpressionVirtualColumn("upper(\"countryIsoCode\")", "v0"),
+ v1
+ );
+
+ JoinTestHelper.verifyCursor(
+ new HashJoinSegmentCursorFactory(
+ factSegment.as(CursorFactory.class),
+ null,
+ joinableClauses,
+ joinFilterPreAnalysis
+ ).makeCursorHolder(
+
CursorBuildSpec.builder().setFilter(filter).setVirtualColumns(virtualColumns).build()
+ ),
+ ImmutableList.of(
+ "page",
+ "countryIsoCode",
+ "v0",
+ "v1"
+ ),
+ ImmutableList.of(
+ new Object[]{"Didier Leclair", "CA", "CA", "CA!"},
+ new Object[]{"Les Argonautes", "CA", "CA", "CA!"},
+ new Object[]{"Sarah Michelle Gellar", "CA", "CA", "CA!"}
+ )
+ );
+ }
+
@Test
public void test_makeCursor_factToCountryUsingExpression()
{
diff --git
a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteJoinQueryTest.java
b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteJoinQueryTest.java
index f7819cf9d81..8d9c8c579bc 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteJoinQueryTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteJoinQueryTest.java
@@ -81,6 +81,8 @@ import org.apache.druid.segment.column.ColumnType;
import org.apache.druid.segment.column.RowSignature;
import org.apache.druid.segment.join.JoinType;
import org.apache.druid.segment.virtual.ListFilteredVirtualColumn;
+import org.apache.druid.segment.virtual.NestedFieldVirtualColumn;
+import org.apache.druid.segment.virtual.NestedObjectVirtualColumn;
import org.apache.druid.server.QueryLifecycle;
import org.apache.druid.server.security.AuthorizationResult;
import org.apache.druid.sql.calcite.DecoupledTestConfig.IgnoreQueriesReason;
@@ -1628,6 +1630,53 @@ public class CalciteJoinQueryTest extends
BaseCalciteQueryTest
);
}
+ @MethodSource("provideQueryContexts")
+ @ParameterizedTest(name = "{0}")
+ public void testInnerJoinLookupWithChainedPostJoinVirtualColumns(Map<String,
Object> queryContext)
+ {
+ // JSON_VALUE of JSON_OBJECT plans into two virtual columns: "v1" reads
the joined column "j0.v", and "v0" reads
+ // only "v1". Both must be computed after the join.
+ testQuery(
+ "SELECT dim1, dim2, JSON_VALUE(JSON_OBJECT(KEY 'x' VALUE l.v),
'$.x')\n"
+ + "FROM foo\n"
+ + "INNER JOIN lookup.lookyloo l ON foo.dim2 = l.k",
+ queryContext,
+ ImmutableList.of(
+ newScanQueryBuilder()
+ .dataSource(
+ join(
+ new TableDataSource(CalciteTests.DATASOURCE1),
+ new LookupDataSource("lookyloo"),
+ "j0.",
+ equalsCondition(makeColumnExpression("dim2"),
makeColumnExpression("j0.k")),
+ JoinType.INNER
+ )
+ )
+ .intervals(querySegmentSpec(Filtration.eternity()))
+ .virtualColumns(
+ new NestedFieldVirtualColumn("v1", "$.x", "v0",
ColumnType.STRING),
+ new NestedObjectVirtualColumn(
+ "v1",
+ ImmutableMap.of(
+ "x",
+ new
NestedObjectVirtualColumn.TypedExpression("\"j0.v\"", ColumnType.STRING)
+ ),
+ queryFramework().macroTable()
+ )
+ )
+ .columns("dim1", "dim2", "v0")
+ .columnTypes(ColumnType.STRING, ColumnType.STRING,
ColumnType.STRING)
+ .context(queryContext)
+ .build()
+ ),
+ ImmutableList.of(
+ new Object[]{"", "a", "xa"},
+ new Object[]{"1", "a", "xa"},
+ new Object[]{"def", "abc", "xabc"}
+ )
+ );
+ }
+
@MethodSource("provideQueryContexts")
@ParameterizedTest(name = "{0}")
public void testInnerJoinTwoLookupsToTableUsingNumericColumn(Map<String,
Object> queryContext)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
