You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by ab...@apache.org on 2022/01/27 12:36:40 UTC
[druid] branch master updated: Fix SQL queries for inline datasource with null values (#12092)
This is an automated email from the ASF dual-hosted git repository.
abhishek 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 1b8808c Fix SQL queries for inline datasource with null values (#12092)
1b8808c is described below
commit 1b8808cce8b4880f92d9e8e7dda5be7b65a24b19
Author: Abhishek Agarwal <14...@users.noreply.github.com>
AuthorDate: Thu Jan 27 18:04:12 2022 +0530
Fix SQL queries for inline datasource with null values (#12092)
Fixes a bug because of which some SQL queries cannot be parsed using druid convention. Specifically, these queries translate to an inline datasource and have some null values. Calcite internally uses NULL as SQL type for these literals and that is not supported by the druid.
I am now allowing null column types to be returned while building RowSignature in org.apache.druid.sql.calcite.table.RowSignatures#fromRelDataType. RowSignature already allows null column type for any column. Doing so should also fix bindable queries such as select (1,2). When such queries are run with headers set to true, we get an exception in org.apache.druid.sql.http.ArrayWriter#writeHeader. This is again a similar exception to the one addressed in this PR. Because SQL type for th [...]
---
.../druid/sql/calcite/expression/Expressions.java | 1 +
.../sql/calcite/rule/DruidLogicalValuesRule.java | 5 +
.../druid/sql/calcite/table/RowSignatures.java | 8 --
.../org/apache/druid/sql/http/ArrayWriter.java | 3 +-
.../java/org/apache/druid/sql/http/CsvWriter.java | 3 +-
.../org/apache/druid/sql/http/ObjectWriter.java | 3 +-
.../druid/sql/calcite/CalciteSelectQueryTest.java | 84 +++++++++++++++++
.../org/apache/druid/sql/http/SqlResourceTest.java | 103 +++++++++++++++++++++
8 files changed, 199 insertions(+), 11 deletions(-)
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/Expressions.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/Expressions.java
index 1b34c38..98ae4a5 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/Expressions.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/Expressions.java
@@ -242,6 +242,7 @@ public class Expressions
.lookupOperatorConversion(operator);
if (conversion == null) {
+ plannerContext.setPlanningError("SQL query requires '%s' operator that is not supported.", operator.getName());
return null;
} else {
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidLogicalValuesRule.java b/sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidLogicalValuesRule.java
index 845c460..aae2148 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidLogicalValuesRule.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidLogicalValuesRule.java
@@ -128,6 +128,11 @@ public class DruidLogicalValuesRule extends RelOptRule
case TIMESTAMP:
case DATE:
return Calcites.calciteDateTimeLiteralToJoda(literal, plannerContext.getTimeZone()).getMillis();
+ case NULL:
+ if (!literal.isNull()) {
+ throw new UnsupportedSQLQueryException("Query has a non-null constant but is of NULL type.");
+ }
+ return null;
case TIMESTAMP_WITH_LOCAL_TIME_ZONE:
case TIME:
case TIME_WITH_LOCAL_TIME_ZONE:
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/table/RowSignatures.java b/sql/src/main/java/org/apache/druid/sql/calcite/table/RowSignatures.java
index 35fa3e4..73c2527 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/table/RowSignatures.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/table/RowSignatures.java
@@ -61,14 +61,6 @@ public class RowSignatures
for (int i = 0; i < rowOrder.size(); i++) {
final RelDataType dataType = rowType.getFieldList().get(i).getType();
final ColumnType valueType = Calcites.getColumnTypeForRelDataType(dataType);
- if (valueType == null) {
- throw new ISE(
- "Cannot translate sqlTypeName[%s] to Druid type for field[%s]",
- dataType.getSqlTypeName(),
- rowOrder.get(i)
- );
- }
-
rowSignatureBuilder.add(rowOrder.get(i), valueType);
}
diff --git a/sql/src/main/java/org/apache/druid/sql/http/ArrayWriter.java b/sql/src/main/java/org/apache/druid/sql/http/ArrayWriter.java
index 67db8a2..b8893de 100644
--- a/sql/src/main/java/org/apache/druid/sql/http/ArrayWriter.java
+++ b/sql/src/main/java/org/apache/druid/sql/http/ArrayWriter.java
@@ -23,6 +23,7 @@ import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.apache.calcite.rel.type.RelDataType;
import org.apache.druid.segment.column.RowSignature;
+import org.apache.druid.segment.column.TypeSignature;
import org.apache.druid.sql.calcite.table.RowSignatures;
import javax.annotation.Nullable;
@@ -111,7 +112,7 @@ public class ArrayWriter implements ResultFormat.Writer
if (includeTypes) {
jsonGenerator.writeStartArray();
for (int i = 0; i < signature.size(); i++) {
- jsonGenerator.writeString(signature.getColumnType(i).get().asTypeString());
+ jsonGenerator.writeString(signature.getColumnType(i).map(TypeSignature::asTypeString).orElse(null));
}
jsonGenerator.writeEndArray();
}
diff --git a/sql/src/main/java/org/apache/druid/sql/http/CsvWriter.java b/sql/src/main/java/org/apache/druid/sql/http/CsvWriter.java
index 253c957..e5e9973 100644
--- a/sql/src/main/java/org/apache/druid/sql/http/CsvWriter.java
+++ b/sql/src/main/java/org/apache/druid/sql/http/CsvWriter.java
@@ -22,6 +22,7 @@ package org.apache.druid.sql.http;
import com.opencsv.CSVWriter;
import org.apache.calcite.rel.type.RelDataType;
import org.apache.druid.segment.column.RowSignature;
+import org.apache.druid.segment.column.TypeSignature;
import org.apache.druid.sql.calcite.table.RowSignatures;
import javax.annotation.Nullable;
@@ -76,7 +77,7 @@ public class CsvWriter implements ResultFormat.Writer
final String[] types = new String[rowType.getFieldCount()];
for (int i = 0; i < signature.size(); i++) {
- types[i] = signature.getColumnType(i).get().asTypeString();
+ types[i] = signature.getColumnType(i).map(TypeSignature::asTypeString).orElse(null);
}
writer.writeNext(types, false);
diff --git a/sql/src/main/java/org/apache/druid/sql/http/ObjectWriter.java b/sql/src/main/java/org/apache/druid/sql/http/ObjectWriter.java
index 0f7d9f2..464cd4b 100644
--- a/sql/src/main/java/org/apache/druid/sql/http/ObjectWriter.java
+++ b/sql/src/main/java/org/apache/druid/sql/http/ObjectWriter.java
@@ -23,6 +23,7 @@ import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.apache.calcite.rel.type.RelDataType;
import org.apache.druid.segment.column.RowSignature;
+import org.apache.druid.segment.column.TypeSignature;
import org.apache.druid.sql.calcite.table.RowSignatures;
import javax.annotation.Nullable;
@@ -119,7 +120,7 @@ public class ObjectWriter implements ResultFormat.Writer
if (includeTypes) {
jsonGenerator.writeStringField(
ObjectWriter.TYPE_HEADER_NAME,
- signature.getColumnType(i).get().asTypeString()
+ signature.getColumnType(i).map(TypeSignature::asTypeString).orElse(null)
);
}
diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteSelectQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteSelectQueryTest.java
index 049a935..3f10e57 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteSelectQueryTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteSelectQueryTest.java
@@ -123,6 +123,90 @@ public class CalciteSelectQueryTest extends BaseCalciteQueryTest
}
@Test
+ public void testValuesContainingNull() throws Exception
+ {
+ testQuery(
+ "SELECT * FROM (VALUES (NULL, 'United States'))",
+ ImmutableList.of(
+ Druids.newScanQueryBuilder()
+ .dataSource(
+ InlineDataSource.fromIterable(
+ ImmutableList.of(new Object[]{null, "United States"}),
+ RowSignature
+ .builder()
+ .add("EXPR$0", null)
+ .add("EXPR$1", ColumnType.STRING)
+ .build()
+ )
+ )
+ .intervals(querySegmentSpec(Filtration.eternity()))
+ .columns("EXPR$0", "EXPR$1")
+ .resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST)
+ .legacy(false)
+ .context(QUERY_CONTEXT_DEFAULT)
+ .build()
+ ),
+ ImmutableList.of(new Object[]{null, "United States"})
+ );
+ }
+
+ @Test
+ public void testMultipleValuesContainingNull() throws Exception
+ {
+ testQuery(
+ "SELECT * FROM (VALUES (NULL, 'United States'), ('Delhi', 'India'))",
+ ImmutableList.of(
+ Druids.newScanQueryBuilder()
+ .dataSource(
+ InlineDataSource.fromIterable(
+ ImmutableList.of(new Object[]{null, "United States"}, new Object[]{"Delhi", "India"}),
+ RowSignature
+ .builder()
+ .add("EXPR$0", ColumnType.STRING)
+ .add("EXPR$1", ColumnType.STRING)
+ .build()
+ )
+ )
+ .intervals(querySegmentSpec(Filtration.eternity()))
+ .columns("EXPR$0", "EXPR$1")
+ .resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST)
+ .legacy(false)
+ .context(QUERY_CONTEXT_DEFAULT)
+ .build()
+ ),
+ ImmutableList.of(new Object[]{NULL_STRING, "United States"}, new Object[]{"Delhi", "India"})
+ );
+ }
+
+ @Test
+ public void testMultipleValuesContainingNullAndIntegerValues() throws Exception
+ {
+ testQuery(
+ "SELECT * FROM (VALUES (NULL, 'United States'), (50, 'India'))",
+ ImmutableList.of(
+ Druids.newScanQueryBuilder()
+ .dataSource(
+ InlineDataSource.fromIterable(
+ ImmutableList.of(new Object[]{null, "United States"}, new Object[]{50L, "India"}),
+ RowSignature
+ .builder()
+ .add("EXPR$0", ColumnType.LONG)
+ .add("EXPR$1", ColumnType.STRING)
+ .build()
+ )
+ )
+ .intervals(querySegmentSpec(Filtration.eternity()))
+ .columns("EXPR$0", "EXPR$1")
+ .resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST)
+ .legacy(false)
+ .context(QUERY_CONTEXT_DEFAULT)
+ .build()
+ ),
+ ImmutableList.of(new Object[]{null, "United States"}, new Object[]{50, "India"})
+ );
+ }
+
+ @Test
public void testSelectNonNumericNumberLiterals() throws Exception
{
// Tests to convert NaN, positive infinity and negative infinity as literals.
diff --git a/sql/src/test/java/org/apache/druid/sql/http/SqlResourceTest.java b/sql/src/test/java/org/apache/druid/sql/http/SqlResourceTest.java
index 1a4aab2..c94c79e 100644
--- a/sql/src/test/java/org/apache/druid/sql/http/SqlResourceTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/http/SqlResourceTest.java
@@ -100,6 +100,7 @@ import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -672,6 +673,30 @@ public class SqlResourceTest extends CalciteTestBase
}
@Test
+ public void testArrayResultFormatWithHeader_nullColumnType() throws Exception
+ {
+ // Test a query that returns null header for some of the columns
+ final String query = "SELECT (1, 2)";
+ Assert.assertEquals(
+ ImmutableList.of(
+ Collections.singletonList("EXPR$0"),
+ Collections.singletonList(null),
+ Collections.singletonList("ROW"),
+ Collections.singletonList(
+ Arrays.asList(
+ 1,
+ 2
+ )
+ )
+ ),
+ doPost(
+ new SqlQuery(query, ResultFormat.ARRAY, true, true, true, null, null),
+ new TypeReference<List<List<Object>>>() {}
+ ).rhs
+ );
+ }
+
+ @Test
public void testArrayLinesResultFormat() throws Exception
{
final String query = "SELECT *, CASE dim2 WHEN '' THEN dim2 END FROM foo LIMIT 2";
@@ -765,6 +790,34 @@ public class SqlResourceTest extends CalciteTestBase
}
@Test
+ public void testArrayLinesResultFormatWithHeader_nullColumnType() throws Exception
+ {
+ final String query = "SELECT (1, 2)";
+ final Pair<QueryException, String> pair = doPostRaw(
+ new SqlQuery(query, ResultFormat.ARRAYLINES, true, true, true, null, null)
+ );
+ Assert.assertNull(pair.lhs);
+ final String response = pair.rhs;
+ final List<String> lines = Splitter.on('\n').splitToList(response);
+
+ Assert.assertEquals(6, lines.size());
+ Assert.assertEquals(Collections.singletonList("EXPR$0"), JSON_MAPPER.readValue(lines.get(0), List.class));
+ Assert.assertEquals(Collections.singletonList(null), JSON_MAPPER.readValue(lines.get(1), List.class));
+ Assert.assertEquals(Collections.singletonList("ROW"), JSON_MAPPER.readValue(lines.get(2), List.class));
+ Assert.assertEquals(
+ Collections.singletonList(
+ Arrays.asList(
+ 1,
+ 2
+ )
+ ),
+ JSON_MAPPER.readValue(lines.get(3), List.class)
+ );
+ Assert.assertEquals("", lines.get(4));
+ Assert.assertEquals("", lines.get(5));
+ }
+
+ @Test
public void testObjectResultFormat() throws Exception
{
final String query = "SELECT *, CASE dim2 WHEN '' THEN dim2 END FROM foo LIMIT 2";
@@ -994,6 +1047,35 @@ public class SqlResourceTest extends CalciteTestBase
}
@Test
+ public void testObjectLinesResultFormatWithFullHeader_nullColumnType() throws Exception
+ {
+ final String query = "SELECT (1, 2)";
+ final Pair<QueryException, String> pair =
+ doPostRaw(new SqlQuery(query, ResultFormat.OBJECTLINES, true, true, true, null, null));
+ Assert.assertNull(pair.lhs);
+ final String response = pair.rhs;
+ final List<String> lines = Splitter.on('\n').splitToList(response);
+
+ final Map<String, String> typeMap = new HashMap<>();
+ typeMap.put(ObjectWriter.TYPE_HEADER_NAME, null);
+ typeMap.put(ObjectWriter.SQL_TYPE_HEADER_NAME, "ROW");
+ final Map<String, Object> expectedHeader = ImmutableMap.of("EXPR$0", typeMap);
+
+ Assert.assertEquals(4, lines.size());
+ Assert.assertEquals(expectedHeader, JSON_MAPPER.readValue(lines.get(0), Object.class));
+ Assert.assertEquals(
+ ImmutableMap
+ .<String, Object>builder()
+ .put("EXPR$0", Arrays.asList(1, 2))
+ .build(),
+ JSON_MAPPER.readValue(lines.get(1), Object.class)
+ );
+
+ Assert.assertEquals("", lines.get(2));
+ Assert.assertEquals("", lines.get(3));
+ }
+
+ @Test
public void testCsvResultFormat() throws Exception
{
final String query = "SELECT *, CASE dim2 WHEN '' THEN dim2 END FROM foo LIMIT 2";
@@ -1041,6 +1123,27 @@ public class SqlResourceTest extends CalciteTestBase
}
@Test
+ public void testCsvResultFormatWithHeaders_nullColumnType() throws Exception
+ {
+ final String query = "SELECT (1, 2)";
+ final Pair<QueryException, String> pair = doPostRaw(
+ new SqlQuery(query, ResultFormat.CSV, true, true, true, null, null)
+ );
+ Assert.assertNull(pair.lhs);
+ final String response = pair.rhs;
+ final List<String> lines = Splitter.on('\n').splitToList(response);
+
+ Assert.assertEquals(
+ ImmutableList.of(
+ "EXPR$0",
+ "",
+ "ROW"
+ ),
+ lines.subList(0, 3)
+ );
+ }
+
+ @Test
public void testExplainCountStar() throws Exception
{
Map<String, Object> queryContext = ImmutableMap.of(PlannerContext.CTX_SQL_QUERY_ID, DUMMY_SQL_QUERY_ID);
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org