From ed08dc19d502d30177cb31d25d0db549968a3c19 Mon Sep 17 00:00:00 2001 From: Abhishek Agarwal <1477457+abhishekagarwal87@users.noreply.github.com> Date: Tue, 29 Sep 2020 21:57:27 +0530 Subject: [PATCH 1/2] Virtual column on __time should be in pre-join --- .../druid/segment/join/HashJoinSegmentStorageAdapter.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapter.java b/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapter.java index 03f3f946d461..d6517c1bfbcc 100644 --- a/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapter.java +++ b/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapter.java @@ -34,6 +34,7 @@ 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.ColumnHolder; import org.apache.druid.segment.data.Indexed; import org.apache.druid.segment.data.ListIndexed; import org.apache.druid.segment.join.filter.JoinFilterAnalyzer; @@ -305,6 +306,7 @@ public Set determineBaseColumnsWithPreAndPostJoinVirtualColumns( ) { final Set baseColumns = new HashSet<>(); + baseColumns.add(ColumnHolder.TIME_COLUMN_NAME); Iterables.addAll(baseColumns, baseAdapter.getAvailableDimensions()); Iterables.addAll(baseColumns, baseAdapter.getAvailableMetrics()); From d1dc429eb6d33a3334af3bb5a9e3a5292eacdbdb Mon Sep 17 00:00:00 2001 From: Abhishek Agarwal <1477457+abhishekagarwal87@users.noreply.github.com> Date: Mon, 5 Oct 2020 12:58:35 +0530 Subject: [PATCH 2/2] Add unit test --- ...BaseHashJoinSegmentStorageAdapterTest.java | 17 ++++++ .../HashJoinSegmentStorageAdapterTest.java | 53 ++++++++++++++----- 2 files changed, 57 insertions(+), 13 deletions(-) diff --git a/processing/src/test/java/org/apache/druid/segment/join/BaseHashJoinSegmentStorageAdapterTest.java b/processing/src/test/java/org/apache/druid/segment/join/BaseHashJoinSegmentStorageAdapterTest.java index 6a6af72dc39d..d5dc9a2f1f8f 100644 --- a/processing/src/test/java/org/apache/druid/segment/join/BaseHashJoinSegmentStorageAdapterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/join/BaseHashJoinSegmentStorageAdapterTest.java @@ -27,7 +27,9 @@ import org.apache.druid.query.filter.Filter; import org.apache.druid.query.lookup.LookupExtractor; import org.apache.druid.segment.QueryableIndexSegment; +import org.apache.druid.segment.VirtualColumn; import org.apache.druid.segment.VirtualColumns; +import org.apache.druid.segment.column.ValueType; import org.apache.druid.segment.join.filter.JoinFilterAnalyzer; import org.apache.druid.segment.join.filter.JoinFilterPreAnalysis; import org.apache.druid.segment.join.filter.JoinFilterPreAnalysisKey; @@ -242,4 +244,19 @@ protected static JoinFilterPreAnalysis makeDefaultConfigPreAnalysis( ) ); } + + protected VirtualColumn makeExpressionVirtualColumn(String expression) + { + return makeExpressionVirtualColumn(expression, "virtual"); + } + + protected VirtualColumn makeExpressionVirtualColumn(String expression, String columnName) + { + return new ExpressionVirtualColumn( + columnName, + expression, + ValueType.STRING, + ExprMacroTable.nil() + ); + } } diff --git a/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java b/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java index 6406d7afe098..55469624ac82 100644 --- a/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java @@ -31,6 +31,7 @@ import org.apache.druid.query.filter.Filter; import org.apache.druid.query.filter.OrDimFilter; import org.apache.druid.query.filter.SelectorDimFilter; +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; @@ -38,10 +39,10 @@ import org.apache.druid.segment.join.filter.JoinFilterPreAnalysis; import org.apache.druid.segment.join.lookup.LookupJoinable; import org.apache.druid.segment.join.table.IndexedTableJoinable; -import org.apache.druid.segment.virtual.ExpressionVirtualColumn; import org.junit.Assert; import org.junit.Test; +import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -1502,12 +1503,7 @@ public void test_makeCursors_factToCountryUsingVirtualColumn() VirtualColumns virtualColumns = VirtualColumns.create( Collections.singletonList( - new ExpressionVirtualColumn( - "virtual", - "concat(substring(countryIsoCode, 0, 1),'L')", - ValueType.STRING, - ExprMacroTable.nil() - ) + makeExpressionVirtualColumn("concat(substring(countryIsoCode, 0, 1),'L')") ) ); @@ -1563,12 +1559,7 @@ public void test_makeCursors_factToCountryUsingVirtualColumnUsingLookup() VirtualColumns virtualColumns = VirtualColumns.create( Collections.singletonList( - new ExpressionVirtualColumn( - "virtual", - "concat(substring(countryIsoCode, 0, 1),'L')", - ValueType.STRING, - ExprMacroTable.nil() - ) + makeExpressionVirtualColumn("concat(substring(countryIsoCode, 0, 1),'L')") ) ); @@ -2028,4 +2019,40 @@ public void test_makeCursors_originalFilterDoesNotMatchPreAnalysis_shouldThrowIS ); } + @Test + public void test_determineBaseColumnsWithPreAndPostJoinVirtualColumns() + { + List joinableClauses = ImmutableList.of(factToCountryOnIsoCode(JoinType.LEFT)); + JoinFilterPreAnalysis analysis = makeDefaultConfigPreAnalysis(null, joinableClauses, VirtualColumns.EMPTY); + HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( + factSegment.asStorageAdapter(), + joinableClauses, + analysis + ); + List expectedPreJoin = ImmutableList.of( + makeExpressionVirtualColumn("concat(countryIsoCode,'L')", "v0"), + makeExpressionVirtualColumn("concat(countryIsoCode, countryNumber)", "v1"), + makeExpressionVirtualColumn("channel_uniques - 1", "v2"), + makeExpressionVirtualColumn("channel_uniques - __time", "v3") + ); + + List expectedPostJoin = ImmutableList.of( + makeExpressionVirtualColumn("concat(countryIsoCode, dummyColumn)", "v4"), + makeExpressionVirtualColumn("dummyMetric - __time", "v5") + ); + List actualPreJoin = new ArrayList<>(); + List actualPostJoin = new ArrayList<>(); + List allVirtualColumns = new ArrayList<>(); + allVirtualColumns.addAll(expectedPreJoin); + allVirtualColumns.addAll(expectedPostJoin); + adapter.determineBaseColumnsWithPreAndPostJoinVirtualColumns( + VirtualColumns.create(allVirtualColumns), + actualPreJoin, + actualPostJoin + ); + + Assert.assertEquals(expectedPreJoin, actualPreJoin); + Assert.assertEquals(expectedPostJoin, actualPostJoin); + } + }