From c1c21b289b08382af040802b09a7bd53ac3aef50 Mon Sep 17 00:00:00 2001 From: sreemanamala Date: Wed, 4 Sep 2024 21:59:55 +0530 Subject: [PATCH 1/5] reject mvds --- .../rowsandcols/ArrayListRowsAndColumns.java | 21 +++++++++++++++- .../sql/calcite/CalciteWindowQueryTest.java | 25 +++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java index 883371fec656..5c1e2fbef982 100644 --- a/processing/src/main/java/org/apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java @@ -25,6 +25,8 @@ import it.unimi.dsi.fastutil.ints.IntList; import org.apache.druid.common.semantic.SemanticCreator; import org.apache.druid.common.semantic.SemanticUtils; +import org.apache.druid.error.DruidException; +import org.apache.druid.error.InvalidInput; import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.guava.Sequences; import org.apache.druid.query.operator.ColumnWithDirection; @@ -153,11 +155,28 @@ public Column findColumn(String name) return new LimitedColumn(retVal, startOffset, endOffset); } - final Function adapterForValue = rowAdapter.columnFunction(name); final Optional maybeColumnType = rowSignature.getColumnType(name); final ColumnType columnType = maybeColumnType.orElse(ColumnType.UNKNOWN_COMPLEX); final Comparator comparator = Comparator.nullsFirst(columnType.getStrategy()); + final Function adapterForValue; + if (columnType.equals(ColumnType.STRING)) { + // special handling to reject MVDs + adapterForValue = f -> { + Object value = rowAdapter.columnFunction(name).apply(f); + if (value instanceof String) { + return value; + } + throw InvalidInput.exception( + "Encountered a multi value column [%s]. Window processing does not support MVDs. " + + "Try considering grouping on the column or use MV_TO_ARRAY()", + name + ); + }; + } else { + adapterForValue = rowAdapter.columnFunction(name); + } + return new Column() { @Nonnull diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteWindowQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteWindowQueryTest.java index 165b4aa3f631..6ad7131f35e6 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteWindowQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteWindowQueryTest.java @@ -24,6 +24,7 @@ import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import org.apache.druid.error.DruidException; import org.apache.druid.jackson.DefaultObjectMapper; import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.RE; @@ -51,6 +52,7 @@ import java.util.Map; import java.util.Objects; +import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -249,6 +251,29 @@ public void testWithArrayConcat() .run(); } + @Test + public void testFailure_partitionByMVD() + { + final DruidException e = Assert.assertThrows( + DruidException.class, + () -> testBuilder() + .sql("select cityName, countryName, array_to_mv(array[1,length(cityName)]),\n" + + "row_number() over (partition by array_to_mv(array[1,length(cityName)]) order by countryName, cityName)\n" + + "from wikipedia\n" + + "where countryName in ('Austria', 'Republic of Korea') and cityName is not null\n" + + "order by 1, 2, 3") + .queryContext(DEFAULT_QUERY_CONTEXT) + .expectedResults(ImmutableList.of()) + .run() + ); + + assertEquals( + "Encountered a multi value column [v0]. Window processing does not support MVDs. " + + "Try considering grouping on the column or use MV_TO_ARRAY()", + e.getMessage() + ); + } + private WindowOperatorQuery getWindowOperatorQuery(List> queries) { assertEquals(1, queries.size()); From 361c4b6da4be2c57720c36db2582f0cc8551f135 Mon Sep 17 00:00:00 2001 From: sreemanamala Date: Wed, 4 Sep 2024 22:06:11 +0530 Subject: [PATCH 2/5] chckstyle --- .../apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java | 1 - .../org/apache/druid/sql/calcite/CalciteWindowQueryTest.java | 1 - 2 files changed, 2 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java index 5c1e2fbef982..198f6607df51 100644 --- a/processing/src/main/java/org/apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java @@ -25,7 +25,6 @@ import it.unimi.dsi.fastutil.ints.IntList; import org.apache.druid.common.semantic.SemanticCreator; import org.apache.druid.common.semantic.SemanticUtils; -import org.apache.druid.error.DruidException; import org.apache.druid.error.InvalidInput; import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.guava.Sequences; diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteWindowQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteWindowQueryTest.java index 6ad7131f35e6..d34f0e3ced76 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteWindowQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteWindowQueryTest.java @@ -52,7 +52,6 @@ import java.util.Map; import java.util.Objects; -import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; From 712664b3fecbabcb70c8c1ac6fbff64e5063c0b0 Mon Sep 17 00:00:00 2001 From: sreemanamala Date: Thu, 5 Sep 2024 07:04:27 +0530 Subject: [PATCH 3/5] refactor --- .../query/rowsandcols/ArrayListRowsAndColumns.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java index 198f6607df51..815cc0cd150e 100644 --- a/processing/src/main/java/org/apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java @@ -163,14 +163,14 @@ public Column findColumn(String name) // special handling to reject MVDs adapterForValue = f -> { Object value = rowAdapter.columnFunction(name).apply(f); - if (value instanceof String) { - return value; + if (value instanceof Object[] || value instanceof List) { + throw InvalidInput.exception( + "Encountered a multi value column [%s]. Window processing does not support MVDs. " + + "Try considering grouping on the column or use MV_TO_ARRAY()", + name + ); } - throw InvalidInput.exception( - "Encountered a multi value column [%s]. Window processing does not support MVDs. " - + "Try considering grouping on the column or use MV_TO_ARRAY()", - name - ); + return value; }; } else { adapterForValue = rowAdapter.columnFunction(name); From e32db3bb8b8bc2b0c8bbbdaa5bcb76457eac8fd3 Mon Sep 17 00:00:00 2001 From: sreemanamala Date: Thu, 5 Sep 2024 10:27:13 +0530 Subject: [PATCH 4/5] comments --- .../druid/query/rowsandcols/ArrayListRowsAndColumns.java | 2 +- .../org/apache/druid/sql/calcite/CalciteWindowQueryTest.java | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java index 815cc0cd150e..33ab0fc1ee26 100644 --- a/processing/src/main/java/org/apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java @@ -166,7 +166,7 @@ public Column findColumn(String name) if (value instanceof Object[] || value instanceof List) { throw InvalidInput.exception( "Encountered a multi value column [%s]. Window processing does not support MVDs. " - + "Try considering grouping on the column or use MV_TO_ARRAY()", + + "Consider using UNNEST or MV_TO_ARRAY.", name ); } diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteWindowQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteWindowQueryTest.java index d34f0e3ced76..ccf459e743e7 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteWindowQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteWindowQueryTest.java @@ -262,13 +262,12 @@ public void testFailure_partitionByMVD() + "where countryName in ('Austria', 'Republic of Korea') and cityName is not null\n" + "order by 1, 2, 3") .queryContext(DEFAULT_QUERY_CONTEXT) - .expectedResults(ImmutableList.of()) .run() ); assertEquals( "Encountered a multi value column [v0]. Window processing does not support MVDs. " - + "Try considering grouping on the column or use MV_TO_ARRAY()", + + "Consider using UNNEST or MV_TO_ARRAY.", e.getMessage() ); } From f300b89735bdd2265b2819a4a3112a43385bb4fa Mon Sep 17 00:00:00 2001 From: sreemanamala Date: Fri, 6 Sep 2024 08:48:54 +0530 Subject: [PATCH 5/5] comments --- .../apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java index 33ab0fc1ee26..7ce3df8e0668 100644 --- a/processing/src/main/java/org/apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java @@ -163,7 +163,7 @@ public Column findColumn(String name) // special handling to reject MVDs adapterForValue = f -> { Object value = rowAdapter.columnFunction(name).apply(f); - if (value instanceof Object[] || value instanceof List) { + if (value instanceof List) { throw InvalidInput.exception( "Encountered a multi value column [%s]. Window processing does not support MVDs. " + "Consider using UNNEST or MV_TO_ARRAY.",