From e09b8a26ae7a90ecf8985c92d8f1ae507f48b420 Mon Sep 17 00:00:00 2001 From: sreemanamala Date: Mon, 26 Feb 2024 06:47:05 +0530 Subject: [PATCH 1/6] array overlap to take numeric columns & dynamic params arrays null element --- .../ArrayOverlapOperatorConversion.java | 6 +- .../planner/SqlParameterizerShuttle.java | 9 ++- .../sql/calcite/CalciteArraysQueryTest.java | 58 +++++++++++++++++++ 3 files changed, 66 insertions(+), 7 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ArrayOverlapOperatorConversion.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ArrayOverlapOperatorConversion.java index 23cfcfaa4a45..4764b6a90c9b 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ArrayOverlapOperatorConversion.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ArrayOverlapOperatorConversion.java @@ -59,11 +59,13 @@ public class ArrayOverlapOperatorConversion extends BaseExpressionDimFilterOpera "'ARRAY_OVERLAP(array, array)'", OperandTypes.or( OperandTypes.family(SqlTypeFamily.ARRAY), - OperandTypes.family(SqlTypeFamily.STRING) + OperandTypes.family(SqlTypeFamily.STRING), + OperandTypes.family(SqlTypeFamily.NUMERIC) ), OperandTypes.or( OperandTypes.family(SqlTypeFamily.ARRAY), - OperandTypes.family(SqlTypeFamily.STRING) + OperandTypes.family(SqlTypeFamily.STRING), + OperandTypes.family(SqlTypeFamily.NUMERIC) ) ) ) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/SqlParameterizerShuttle.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/SqlParameterizerShuttle.java index 29da45b085fd..d71f874d42af 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/SqlParameterizerShuttle.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/SqlParameterizerShuttle.java @@ -129,13 +129,12 @@ private SqlNode createArrayLiteral(Object value, int posn) List args = new ArrayList<>(list.size()); for (int i = 0, listSize = list.size(); i < listSize; i++) { Object element = list.get(i); - if (element == null) { - throw InvalidSqlInput.exception("parameter [%d] is an array, with an illegal null at index [%d]", posn + 1, i); - } SqlNode node; - if (element instanceof String) { + if (element == null) { + node = SqlLiteral.createNull(SqlParserPos.ZERO); + } else if (element instanceof String) { node = SqlLiteral.createCharString((String) element, SqlParserPos.ZERO); - } else if (element instanceof Integer || element instanceof Long) { + } else if (element instanceof Integer || element instanceof Long || element instanceof Double) { // No direct way to create a literal from an Integer or Long, have // to parse a string, sadly. node = SqlLiteral.createExactNumeric(element.toString(), SqlParserPos.ZERO); diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java index 61b484b6a8b2..789c6a157c67 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java @@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.inject.Injector; +import org.apache.calcite.avatica.SqlType; import org.apache.druid.common.config.NullHandling; import org.apache.druid.data.input.ResourceInputSource; import org.apache.druid.guice.DruidInjectorBuilder; @@ -88,6 +89,7 @@ import org.apache.druid.sql.calcite.filtration.Filtration; import org.apache.druid.sql.calcite.util.CalciteTests; import org.apache.druid.sql.calcite.util.TestDataBuilder; +import org.apache.druid.sql.http.SqlParameter; import org.apache.druid.timeline.DataSegment; import org.apache.druid.timeline.partition.LinearShardSpec; import org.junit.Assert; @@ -1072,6 +1074,62 @@ public void testArrayOverlapFilterArrayDoubleColumns() ); } + @Test + public void testArrayOverlapFilterNumeric() + { + testQuery( + "SELECT dim3 FROM druid.numfoo WHERE ARRAY_OVERLAP(l1, ARRAY[1, 7]) LIMIT 5", + ImmutableList.of( + newScanQueryBuilder() + .dataSource(CalciteTests.DATASOURCE3) + .intervals(querySegmentSpec(Filtration.eternity())) + .filters(new InDimFilter("l1", ImmutableList.of("1", "7"), null)) + .columns("dim3") + .resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST) + .limit(5) + .context(QUERY_CONTEXT_DEFAULT) + .build() + ), + ImmutableList.of( + new Object[]{"[\"a\",\"b\"]"} + ) + ); + } + + @Test + public void testArrayOverlapFilterWithDynamicParameter() + { + Druids.ScanQueryBuilder builder = newScanQueryBuilder() + .dataSource(CalciteTests.DATASOURCE3) + .intervals(querySegmentSpec(Filtration.eternity())) + .filters(new InDimFilter("d1", Arrays.asList("1.0", "1.7", null), null)) + .columns("dim3") + .resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST) + .limit(5) + .context(QUERY_CONTEXT_DEFAULT); + + testQuery( + PLANNER_CONFIG_DEFAULT, + QUERY_CONTEXT_DEFAULT, + ImmutableList.of( + new SqlParameter(SqlType.ARRAY, Arrays.asList("1.0", "1.7", null)) + ), + "SELECT dim3 FROM druid.numfoo WHERE ARRAY_OVERLAP(?, d1) LIMIT 5", + CalciteTests.REGULAR_USER_AUTH_RESULT, + ImmutableList.of(builder.build()), + NullHandling.sqlCompatible() ? ImmutableList.of( + new Object[]{"[\"a\",\"b\"]"}, + new Object[]{"[\"b\",\"c\"]"}, + new Object[]{""}, + new Object[]{null}, + new Object[]{null} + ) : ImmutableList.of( + new Object[]{"[\"a\",\"b\"]"}, + new Object[]{"[\"b\",\"c\"]"} + ) + ); + } + @Test public void testArrayContainsFilter() { From 04198439604705543a0f6bf3e96ef030cbbf78bd Mon Sep 17 00:00:00 2001 From: sreemanamala Date: Mon, 26 Feb 2024 09:53:28 +0530 Subject: [PATCH 2/6] uts for branch coverage --- .../sql/calcite/CalciteArraysQueryTest.java | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java index 789c6a157c67..d12a7fe913d5 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java @@ -1130,6 +1130,38 @@ public void testArrayOverlapFilterWithDynamicParameter() ); } + @Test + public void testArrayOverlapFilterWithDynamicParameterLong() + { + Druids.ScanQueryBuilder builder = newScanQueryBuilder() + .dataSource(CalciteTests.DATASOURCE3) + .intervals(querySegmentSpec(Filtration.eternity())) + .filters(new InDimFilter("l1", Arrays.asList("1", "7", null), null)) + .columns("dim3") + .resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST) + .limit(5) + .context(QUERY_CONTEXT_DEFAULT); + + testQuery( + PLANNER_CONFIG_DEFAULT, + QUERY_CONTEXT_DEFAULT, + ImmutableList.of( + new SqlParameter(SqlType.ARRAY, Arrays.asList("1", "7", null)) + ), + "SELECT dim3 FROM druid.numfoo WHERE ARRAY_OVERLAP(?, l1) LIMIT 5", + CalciteTests.REGULAR_USER_AUTH_RESULT, + ImmutableList.of(builder.build()), + NullHandling.sqlCompatible() ? ImmutableList.of( + new Object[]{"[\"a\",\"b\"]"}, + new Object[]{""}, + new Object[]{null}, + new Object[]{null} + ) : ImmutableList.of( + new Object[]{"[\"a\",\"b\"]"} + ) + ); + } + @Test public void testArrayContainsFilter() { @@ -1398,6 +1430,48 @@ public void testArrayContainsFilterArrayDoubleColumns() ); } + @Test + public void testArrayContainsFilterWithDynamicParameter() + { + Druids.ScanQueryBuilder builder = newScanQueryBuilder() + .dataSource(CalciteTests.DATASOURCE3) + .intervals(querySegmentSpec(Filtration.eternity())) + .columns("dim3") + .resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST) + .limit(5) + .context(QUERY_CONTEXT_DEFAULT); + + if (NullHandling.sqlCompatible()) { + builder = builder.filters( + and( + equality("dim3", "a", ColumnType.STRING), + equality("dim3", "b", ColumnType.STRING) + ) + ); + } else { + builder = builder.filters( + and( + selector("dim3", "a"), + selector("dim3", "b") + ) + ); + } + + testQuery( + PLANNER_CONFIG_DEFAULT, + QUERY_CONTEXT_DEFAULT, + ImmutableList.of( + new SqlParameter(SqlType.ARRAY, Arrays.asList("a", "b")) + ), + "SELECT dim3 FROM druid.numfoo WHERE ARRAY_CONTAINS(dim3, ?) LIMIT 5", + CalciteTests.REGULAR_USER_AUTH_RESULT, + ImmutableList.of(builder.build()), + ImmutableList.of( + new Object[]{"[\"a\",\"b\"]"} + ) + ); + } + @Test public void testArraySlice() { From ca7590e82c27bc7a17759e4eaa199e8d9514b8f2 Mon Sep 17 00:00:00 2001 From: sreemanamala Date: Mon, 26 Feb 2024 14:41:44 +0530 Subject: [PATCH 3/6] branch coverage --- .../org/apache/druid/sql/calcite/CalciteArraysQueryTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java index d12a7fe913d5..fbcff463585f 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java @@ -1112,7 +1112,7 @@ public void testArrayOverlapFilterWithDynamicParameter() PLANNER_CONFIG_DEFAULT, QUERY_CONTEXT_DEFAULT, ImmutableList.of( - new SqlParameter(SqlType.ARRAY, Arrays.asList("1.0", "1.7", null)) + new SqlParameter(SqlType.ARRAY, Arrays.asList(1.0, 1.7, null)) ), "SELECT dim3 FROM druid.numfoo WHERE ARRAY_OVERLAP(?, d1) LIMIT 5", CalciteTests.REGULAR_USER_AUTH_RESULT, @@ -1146,7 +1146,7 @@ public void testArrayOverlapFilterWithDynamicParameterLong() PLANNER_CONFIG_DEFAULT, QUERY_CONTEXT_DEFAULT, ImmutableList.of( - new SqlParameter(SqlType.ARRAY, Arrays.asList("1", "7", null)) + new SqlParameter(SqlType.ARRAY, Arrays.asList(1, 7, null)) ), "SELECT dim3 FROM druid.numfoo WHERE ARRAY_OVERLAP(?, l1) LIMIT 5", CalciteTests.REGULAR_USER_AUTH_RESULT, From 3421aeaf7bd53879853984c55d6abe04ee31e81a Mon Sep 17 00:00:00 2001 From: sreemanamala Date: Thu, 7 Mar 2024 08:30:31 +0530 Subject: [PATCH 4/6] use approx numeric for double --- .../druid/sql/calcite/planner/SqlParameterizerShuttle.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/SqlParameterizerShuttle.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/SqlParameterizerShuttle.java index d71f874d42af..326a535b8edd 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/SqlParameterizerShuttle.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/SqlParameterizerShuttle.java @@ -134,10 +134,12 @@ private SqlNode createArrayLiteral(Object value, int posn) node = SqlLiteral.createNull(SqlParserPos.ZERO); } else if (element instanceof String) { node = SqlLiteral.createCharString((String) element, SqlParserPos.ZERO); - } else if (element instanceof Integer || element instanceof Long || element instanceof Double) { + } else if (element instanceof Integer || element instanceof Long) { // No direct way to create a literal from an Integer or Long, have // to parse a string, sadly. node = SqlLiteral.createExactNumeric(element.toString(), SqlParserPos.ZERO); + } else if (element instanceof Double || element instanceof Float) { + node = SqlLiteral.createApproxNumeric(element.toString(), SqlParserPos.ZERO); } else if (element instanceof Boolean) { node = SqlLiteral.createBoolean((Boolean) value, SqlParserPos.ZERO); } else { From 8f123265822c9de25581ac05dbc45fbfc8d28170 Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Wed, 13 Mar 2024 15:37:27 -0700 Subject: [PATCH 5/6] MSQ: Validate that strings and string arrays are not mixed. (#15920) * MSQ: Validate that strings and string arrays are not mixed. When multi-value strings and string arrays coexist in the same column, it causes problems with "classic MVD" style queries such as: select * from wikipedia -- fails at runtime select count(*) from wikipedia where flags = 'B' -- fails at planning time select flags, count(*) from wikipedia group by 1 -- fails at runtime To avoid these problems, this patch adds type verification for INSERT and REPLACE. It is targeted: the only type changes that are blocked are string-to-array and array-to-string. There is also a way to exclude certain columns from the type checks, if the user really knows what they're doing. * Fixes. * Tests and docs and error messages. * More docs. * Adjustments. * Adjust message. * Fix tests. * Fix test in DV mode. --- .../sql/calcite/CalciteArraysQueryTest.java | 43 +++++++++++++++++-- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java index ee1d3b76f84d..ee4923d1e508 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java @@ -22,7 +22,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.inject.Injector; import org.apache.calcite.avatica.SqlType; import org.apache.druid.common.config.NullHandling; import org.apache.druid.guice.DruidInjectorBuilder; @@ -72,10 +71,7 @@ import org.apache.druid.segment.virtual.ExpressionVirtualColumn; import org.apache.druid.sql.calcite.filtration.Filtration; import org.apache.druid.sql.calcite.util.CalciteTests; -import org.apache.druid.sql.calcite.util.TestDataBuilder; import org.apache.druid.sql.http.SqlParameter; -import org.apache.druid.timeline.DataSegment; -import org.apache.druid.timeline.partition.LinearShardSpec; import org.junit.Assert; import org.junit.Test; @@ -1337,6 +1333,45 @@ public void testArrayContainsFilterWithDynamicParameter() ); } + @Test + public void testArrayContainsConstantNull() + { + testQuery( + "SELECT ARRAY_CONTAINS(null, ARRAY['a','b'])", + ImmutableList.of( + NullHandling.sqlCompatible() + ? newScanQueryBuilder() + .dataSource( + InlineDataSource.fromIterable( + ImmutableList.of(new Object[]{NullHandling.defaultLongValue()}), + RowSignature.builder().add("EXPR$0", ColumnType.LONG).build() + ) + ) + .intervals(querySegmentSpec(Filtration.eternity())) + .columns("EXPR$0") + .resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST) + .context(QUERY_CONTEXT_DEFAULT) + .build() + : newScanQueryBuilder() + .dataSource( + InlineDataSource.fromIterable( + ImmutableList.of(new Object[]{0L}), + RowSignature.builder().add("ZERO", ColumnType.LONG).build() + ) + ) + .virtualColumns(expressionVirtualColumn("v0", "0", ColumnType.LONG)) + .intervals(querySegmentSpec(Filtration.eternity())) + .columns("v0") + .resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST) + .context(QUERY_CONTEXT_DEFAULT) + .build() + ), + ImmutableList.of( + new Object[]{NullHandling.sqlCompatible() ? null : false} + ) + ); + } + @Test public void testArraySlice() { From 04840dabfef102ed6cad87cde84a818af336e3f6 Mon Sep 17 00:00:00 2001 From: sreemanamala Date: Thu, 14 Mar 2024 07:20:50 +0530 Subject: [PATCH 6/6] fix conflict --- .../org/apache/druid/sql/calcite/CalciteArraysQueryTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java index 0316bcdc09d8..cb92008d31fc 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java @@ -1250,7 +1250,7 @@ public void testArrayContainsArrayStringColumns() "SELECT ARRAY_CONTAINS(arrayStringNulls, ARRAY['a', 'b']), ARRAY_CONTAINS(arrayStringNulls, arrayString) FROM druid.arrays LIMIT 5", ImmutableList.of( newScanQueryBuilder() - .dataSource(DATA_SOURCE_ARRAYS) + .dataSource(CalciteTests.ARRAYS_DATASOURCE) .intervals(querySegmentSpec(Filtration.eternity())) .columns("v0", "v1") .virtualColumns(