This is an automated email from the ASF dual-hosted git repository.
clintropolis 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 7d1eed5655f feat: better validation for catalog types, more permissive
parsing for native type strings (#19769)
7d1eed5655f is described below
commit 7d1eed5655f9fc4c773b8b065f9c9cc417f09685
Author: Clint Wylie <[email protected]>
AuthorDate: Wed Jul 29 23:17:03 2026 -0700
feat: better validation for catalog types, more permissive parsing for
native type strings (#19769)
---
.../org/apache/druid/catalog/CatalogException.java | 15 +++++
.../apache/druid/catalog/http/CatalogResource.java | 13 ++--
.../org/apache/druid/catalog/http/TableEditor.java | 5 +-
.../server/http/catalog/CatalogResourceTest.java | 8 +++
.../org/apache/druid/segment/column/Types.java | 18 +++---
.../apache/druid/math/expr/ExpressionTypeTest.java | 16 +++--
.../druid/segment/column/ColumnTypeTest.java | 16 +++--
.../ClusteredValueGroupsBaseTableMetadata.java | 14 +++++
.../org/apache/druid/catalog/model/TableDefn.java | 20 ++++++
.../druid/catalog/model/table/DatasourceDefn.java | 10 ---
.../catalog/model/table/ExternalTableDefn.java | 7 ---
.../ClusteredValueGroupsBaseTableMetadataTest.java | 34 +++++++++-
.../catalog/model/table/DatasourceTableTest.java | 36 ++++++++++-
.../catalog/model/table/ExternalTableTest.java | 24 +++++++
.../druid/sql/calcite/external/Externals.java | 10 ++-
.../druid/sql/calcite/IngestTableFunctionTest.java | 73 ++++++++++++++++++++++
16 files changed, 273 insertions(+), 46 deletions(-)
diff --git
a/extensions-core/druid-catalog/src/main/java/org/apache/druid/catalog/CatalogException.java
b/extensions-core/druid-catalog/src/main/java/org/apache/druid/catalog/CatalogException.java
index 3b71af7af0c..ddf2142dfe3 100644
---
a/extensions-core/druid-catalog/src/main/java/org/apache/druid/catalog/CatalogException.java
+++
b/extensions-core/druid-catalog/src/main/java/org/apache/druid/catalog/CatalogException.java
@@ -20,6 +20,8 @@
package org.apache.druid.catalog;
import com.google.common.collect.ImmutableMap;
+import org.apache.druid.error.DruidException;
+import org.apache.druid.java.util.common.IAE;
import org.apache.druid.java.util.common.StringUtils;
import javax.ws.rs.core.Response;
@@ -86,6 +88,19 @@ public class CatalogException extends Exception
);
}
+ /**
+ * Converts a table validation failure into a bad request: {@link IAE} (the
traditional catalog validation
+ * exception) and invalid-input {@link DruidException}s become bad requests;
a {@link DruidException} of any other
+ * category is a genuine server error, and is rethrown rather than being
misreported as a problem with the request.
+ */
+ public static CatalogException validationError(RuntimeException e)
+ {
+ if (e instanceof DruidException && ((DruidException) e).getCategory() !=
DruidException.Category.INVALID_INPUT) {
+ throw e;
+ }
+ return badRequest(e.getMessage());
+ }
+
public Response toResponse()
{
return Response
diff --git
a/extensions-core/druid-catalog/src/main/java/org/apache/druid/catalog/http/CatalogResource.java
b/extensions-core/druid-catalog/src/main/java/org/apache/druid/catalog/http/CatalogResource.java
index a766b86caec..f96f042e722 100644
---
a/extensions-core/druid-catalog/src/main/java/org/apache/druid/catalog/http/CatalogResource.java
+++
b/extensions-core/druid-catalog/src/main/java/org/apache/druid/catalog/http/CatalogResource.java
@@ -31,6 +31,7 @@ import org.apache.druid.catalog.model.TableMetadata;
import org.apache.druid.catalog.model.TableSpec;
import org.apache.druid.catalog.storage.CatalogStorage;
import org.apache.druid.common.utils.IdUtils;
+import org.apache.druid.error.DruidException;
import org.apache.druid.java.util.common.IAE;
import org.apache.druid.java.util.common.Pair;
import org.apache.druid.java.util.common.StringUtils;
@@ -140,8 +141,8 @@ public class CatalogResource
try {
catalog.validate(table);
}
- catch (IAE e) {
- throw CatalogException.badRequest(e.getMessage());
+ catch (IAE | DruidException e) {
+ throw CatalogException.validationError(e);
}
long newVersion;
@@ -271,6 +272,10 @@ public class CatalogResource
catch (CatalogException e) {
return e.toResponse();
}
+ catch (IAE | DruidException e) {
+ // Edits merge and re-validate the table spec, so validation failures
surface here.
+ return CatalogException.validationError(e).toResponse();
+ }
}
// ---------------------------------------------------------------------
@@ -524,8 +529,8 @@ public class CatalogResource
try {
spec.validate();
}
- catch (IAE e) {
- throw CatalogException.badRequest(e.getMessage());
+ catch (IAE | DruidException e) {
+ throw CatalogException.validationError(e);
}
if (!schema.accepts(spec.type())) {
diff --git
a/extensions-core/druid-catalog/src/main/java/org/apache/druid/catalog/http/TableEditor.java
b/extensions-core/druid-catalog/src/main/java/org/apache/druid/catalog/http/TableEditor.java
index f7a92d61f40..b3a762b0568 100644
---
a/extensions-core/druid-catalog/src/main/java/org/apache/druid/catalog/http/TableEditor.java
+++
b/extensions-core/druid-catalog/src/main/java/org/apache/druid/catalog/http/TableEditor.java
@@ -36,6 +36,7 @@ import org.apache.druid.catalog.model.TableMetadata;
import org.apache.druid.catalog.model.TableSpec;
import org.apache.druid.catalog.model.table.DatasourceDefn;
import org.apache.druid.catalog.storage.CatalogStorage;
+import org.apache.druid.error.DruidException;
import org.apache.druid.java.util.common.IAE;
import org.apache.druid.utils.CollectionUtils;
@@ -312,8 +313,8 @@ public class TableEditor
try {
defn.validateColumns(revised);
}
- catch (IAE e) {
- throw CatalogException.badRequest(e.getMessage());
+ catch (IAE | DruidException e) {
+ throw CatalogException.validationError(e);
}
return existingSpec.withColumns(revised);
}
diff --git
a/extensions-core/druid-catalog/src/test/java/org/apache/druid/server/http/catalog/CatalogResourceTest.java
b/extensions-core/druid-catalog/src/test/java/org/apache/druid/server/http/catalog/CatalogResourceTest.java
index 066d5a44cad..8f99eec43f5 100644
---
a/extensions-core/druid-catalog/src/test/java/org/apache/druid/server/http/catalog/CatalogResourceTest.java
+++
b/extensions-core/druid-catalog/src/test/java/org/apache/druid/server/http/catalog/CatalogResourceTest.java
@@ -146,6 +146,14 @@ public class CatalogResourceTest
resp = resource.postTable(TableId.DRUID_SCHEMA, tableName, dsSpec, 0,
false, postBy(CatalogTests.WRITER_USER));
assertEquals(Response.Status.BAD_REQUEST.getStatusCode(),
resp.getStatus());
+ // Invalid column type: table-level validation failures (which raise
DruidException rather than IAE) must also
+ // surface as a bad request, not an internal error.
+ TableSpec badTypeSpec = TableBuilder.datasource("badType", "P1D")
+ .column("foo", "FOO")
+ .buildSpec();
+ resp = resource.postTable(TableId.DRUID_SCHEMA, "badType", badTypeSpec, 0,
false, postBy(CatalogTests.SUPER_USER));
+ assertEquals(Response.Status.BAD_REQUEST.getStatusCode(),
resp.getStatus());
+
// Inline input source
TableSpec inputSpec = TableBuilder.external("inline")
.inputSource(toMap(new InlineInputSource("a,b,1\nc,d,2\n")))
diff --git
a/processing/src/main/java/org/apache/druid/segment/column/Types.java
b/processing/src/main/java/org/apache/druid/segment/column/Types.java
index 831f5c76ce9..469a7e39d0a 100644
--- a/processing/src/main/java/org/apache/druid/segment/column/Types.java
+++ b/processing/src/main/java/org/apache/druid/segment/column/Types.java
@@ -19,7 +19,6 @@
package org.apache.druid.segment.column;
-import com.google.common.base.Preconditions;
import org.apache.druid.java.util.common.IAE;
import org.apache.druid.java.util.common.StringUtils;
@@ -39,7 +38,8 @@ public class Types
if (typeString == null) {
return null;
}
- switch (StringUtils.toUpperCase(typeString)) {
+ final String upperTypeString = StringUtils.toUpperCase(typeString);
+ switch (upperTypeString) {
case "STRING":
return typeFactory.ofString();
case "LONG":
@@ -57,14 +57,16 @@ public class Types
case "COMPLEX":
return typeFactory.ofComplex(null);
default:
- // we do not convert to uppercase here, because complex type name must
be preserved in original casing
- // array could be converted, but are not for no particular reason
other than less spooky magic
- if (typeString.startsWith(ARRAY_PREFIX)) {
+ // Prefix matching is case-insensitive, consistent with the scalar
handling above, but the type parameter is
+ // taken from the original string: complex type names are
case-sensitive registry keys which must be preserved
+ // in their original casing.
+ if (upperTypeString.startsWith(ARRAY_PREFIX) &&
upperTypeString.endsWith(">")) {
T elementType = fromString(typeFactory,
typeString.substring(ARRAY_PREFIX.length(), typeString.length() - 1));
- Preconditions.checkNotNull(elementType, "Array element type must not
be null");
- return typeFactory.ofArray(elementType);
+ if (elementType != null) {
+ return typeFactory.ofArray(elementType);
+ }
}
- if (typeString.startsWith(COMPLEX_PREFIX)) {
+ if (upperTypeString.startsWith(COMPLEX_PREFIX) &&
upperTypeString.endsWith(">")) {
return
typeFactory.ofComplex(typeString.substring(COMPLEX_PREFIX.length(),
typeString.length() - 1));
}
}
diff --git
a/processing/src/test/java/org/apache/druid/math/expr/ExpressionTypeTest.java
b/processing/src/test/java/org/apache/druid/math/expr/ExpressionTypeTest.java
index 52c8994c779..c73e6973432 100644
---
a/processing/src/test/java/org/apache/druid/math/expr/ExpressionTypeTest.java
+++
b/processing/src/test/java/org/apache/druid/math/expr/ExpressionTypeTest.java
@@ -67,12 +67,18 @@ public class ExpressionTypeTest
Assertions.assertEquals(ExpressionType.STRING_ARRAY,
MAPPER.readValue("\"string_array\"", ExpressionType.class));
Assertions.assertEquals(ExpressionType.LONG_ARRAY,
MAPPER.readValue("\"long_array\"", ExpressionType.class));
Assertions.assertEquals(ExpressionType.DOUBLE_ARRAY,
MAPPER.readValue("\"double_array\"", ExpressionType.class));
- // ARRAY<*> and COMPLEX<*> patterns must match exactly ...
- Assertions.assertNotEquals(ExpressionType.STRING_ARRAY,
MAPPER.readValue("\"array<string>\"", ExpressionType.class));
- Assertions.assertNotEquals(ExpressionType.LONG_ARRAY,
MAPPER.readValue("\"array<LONG>\"", ExpressionType.class));
- Assertions.assertNotEquals(SOME_COMPLEX,
MAPPER.readValue("\"COMPLEX<FOO>\"", ExpressionType.class));
- // this works though because array recursively calls on element type...
+ // the ARRAY<*> and COMPLEX<*> prefixes match case-insensitively, like the
scalar type names ...
+ Assertions.assertEquals(ExpressionType.STRING_ARRAY,
MAPPER.readValue("\"array<string>\"", ExpressionType.class));
+ Assertions.assertEquals(ExpressionType.LONG_ARRAY,
MAPPER.readValue("\"array<LONG>\"", ExpressionType.class));
Assertions.assertEquals(ExpressionType.DOUBLE_ARRAY,
MAPPER.readValue("\"ARRAY<double>\"", ExpressionType.class));
+ Assertions.assertEquals(SOME_COMPLEX, MAPPER.readValue("\"complex<foo>\"",
ExpressionType.class));
+ // ... but the complex type name inside the brackets is a case-sensitive
registry key, preserved as written
+ Assertions.assertNotEquals(SOME_COMPLEX,
MAPPER.readValue("\"COMPLEX<FOO>\"", ExpressionType.class));
+ // a parameterized type missing its closing bracket is malformed, not a
truncated type parameter
+ Assertions.assertNull(MAPPER.readValue("\"COMPLEX<foo\"",
ExpressionType.class));
+ // an unrecognized array element type makes the whole array type
unrecognized, rather than an error
+ Assertions.assertNull(MAPPER.readValue("\"ARRAY<FOO>\"",
ExpressionType.class));
+ Assertions.assertNull(MAPPER.readValue("\"ARRAY<ARRAY<FOO>>\"",
ExpressionType.class));
}
@Test
diff --git
a/processing/src/test/java/org/apache/druid/segment/column/ColumnTypeTest.java
b/processing/src/test/java/org/apache/druid/segment/column/ColumnTypeTest.java
index 88e167614fc..6302ea96c74 100644
---
a/processing/src/test/java/org/apache/druid/segment/column/ColumnTypeTest.java
+++
b/processing/src/test/java/org/apache/druid/segment/column/ColumnTypeTest.java
@@ -69,12 +69,18 @@ public class ColumnTypeTest
Assertions.assertEquals(ColumnType.STRING_ARRAY,
MAPPER.readValue("\"string_array\"", ColumnType.class));
Assertions.assertEquals(ColumnType.LONG_ARRAY,
MAPPER.readValue("\"long_array\"", ColumnType.class));
Assertions.assertEquals(ColumnType.DOUBLE_ARRAY,
MAPPER.readValue("\"double_array\"", ColumnType.class));
- // ARRAY<*> and COMPLEX<*> patterns must match exactly ...
- Assertions.assertNotEquals(ColumnType.STRING_ARRAY,
MAPPER.readValue("\"array<string>\"", ColumnType.class));
- Assertions.assertNotEquals(ColumnType.LONG_ARRAY,
MAPPER.readValue("\"array<LONG>\"", ColumnType.class));
- Assertions.assertNotEquals(SOME_COMPLEX,
MAPPER.readValue("\"COMPLEX<FOO>\"", ColumnType.class));
- // this works though because array recursively calls on element type...
+ // the ARRAY<*> and COMPLEX<*> prefixes match case-insensitively, like the
scalar type names ...
+ Assertions.assertEquals(ColumnType.STRING_ARRAY,
MAPPER.readValue("\"array<string>\"", ColumnType.class));
+ Assertions.assertEquals(ColumnType.LONG_ARRAY,
MAPPER.readValue("\"array<LONG>\"", ColumnType.class));
Assertions.assertEquals(ColumnType.DOUBLE_ARRAY,
MAPPER.readValue("\"ARRAY<double>\"", ColumnType.class));
+ Assertions.assertEquals(SOME_COMPLEX, MAPPER.readValue("\"complex<foo>\"",
ColumnType.class));
+ // ... but the complex type name inside the brackets is a case-sensitive
registry key, preserved as written
+ Assertions.assertNotEquals(SOME_COMPLEX,
MAPPER.readValue("\"COMPLEX<FOO>\"", ColumnType.class));
+ // a parameterized type missing its closing bracket is malformed, not a
truncated type parameter
+ Assertions.assertNull(MAPPER.readValue("\"COMPLEX<foo\"",
ColumnType.class));
+ // an unrecognized array element type makes the whole array type
unrecognized, rather than an error
+ Assertions.assertNull(MAPPER.readValue("\"ARRAY<FOO>\"",
ColumnType.class));
+ Assertions.assertNull(MAPPER.readValue("\"ARRAY<ARRAY<FOO>>\"",
ColumnType.class));
}
@Test
diff --git
a/server/src/main/java/org/apache/druid/catalog/model/ClusteredValueGroupsBaseTableMetadata.java
b/server/src/main/java/org/apache/druid/catalog/model/ClusteredValueGroupsBaseTableMetadata.java
index f0a3321285d..feaf647e402 100644
---
a/server/src/main/java/org/apache/druid/catalog/model/ClusteredValueGroupsBaseTableMetadata.java
+++
b/server/src/main/java/org/apache/druid/catalog/model/ClusteredValueGroupsBaseTableMetadata.java
@@ -183,6 +183,20 @@ public class ClusteredValueGroupsBaseTableMetadata
implements DatasourceBaseTabl
{
ColumnType druidType = Columns.druidType(column);
if (druidType == null) {
+ // A column declared without a type defaults to STRING (mirroring
Columns.convertSignature), but a declared
+ // type that does not parse must be rejected rather than silently
defaulted: the declared type is the physical
+ // segment schema here.
+ if (column.dataType() != null) {
+ throw InvalidInput.exception(
+ "column [%s] has an unrecognized type [%s]; declare a SQL type
(such as [%s]) or a Druid type string"
+ + " (such as [%s] or [%s])",
+ column.name(),
+ column.dataType(),
+ Columns.SQL_BIGINT,
+ ColumnType.LONG_ARRAY.asTypeString(),
+ ColumnType.NESTED_DATA.asTypeString()
+ );
+ }
druidType = ColumnType.STRING;
}
if (customSchema != null) {
diff --git a/server/src/main/java/org/apache/druid/catalog/model/TableDefn.java
b/server/src/main/java/org/apache/druid/catalog/model/TableDefn.java
index 0e7a26a89ec..f986210aa82 100644
--- a/server/src/main/java/org/apache/druid/catalog/model/TableDefn.java
+++ b/server/src/main/java/org/apache/druid/catalog/model/TableDefn.java
@@ -22,7 +22,9 @@ package org.apache.druid.catalog.model;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.base.Strings;
import org.apache.druid.catalog.model.ModelProperties.PropertyDefn;
+import org.apache.druid.error.InvalidInput;
import org.apache.druid.java.util.common.IAE;
+import org.apache.druid.segment.column.ColumnType;
import java.util.ArrayList;
import java.util.Collections;
@@ -106,9 +108,27 @@ public class TableDefn extends ObjectDefn
/**
* Table-specific validation of a column spec. Override for table definitions
* that need table-specific validation rules.
+ * <p>
+ * A column declared without a type is legal (the type is resolved from the
physical schema, the ingestion query, or
+ * the input format), but a declared type must parse to a Druid type,
otherwise we would silently substitute STRING
+ * for the unrecognized declaration. This runs at catalog write time only:
reads never validate, so tables stored
+ * before this rule keep resolving (though editing them surfaces the invalid
type). {@link Columns#druidType} maps
+ * {@code __time} to LONG regardless of the declared type, which {@link
ColumnSpec#validate} already restricts to
+ * LONG or untyped.
*/
protected void validateColumn(ColumnSpec colSpec)
{
+ if (colSpec.dataType() != null && Columns.druidType(colSpec) == null) {
+ throw InvalidInput.exception(
+ "Column [%s] has an unrecognized type [%s]; declare a SQL type (such
as [%s]) or a Druid type string"
+ + " (such as [%s] or [%s])",
+ colSpec.name(),
+ colSpec.dataType(),
+ Columns.SQL_BIGINT,
+ ColumnType.LONG_ARRAY.asTypeString(),
+ ColumnType.NESTED_DATA.asTypeString()
+ );
+ }
}
/**
diff --git
a/server/src/main/java/org/apache/druid/catalog/model/table/DatasourceDefn.java
b/server/src/main/java/org/apache/druid/catalog/model/table/DatasourceDefn.java
index ef3a41dba46..89982a94883 100644
---
a/server/src/main/java/org/apache/druid/catalog/model/table/DatasourceDefn.java
+++
b/server/src/main/java/org/apache/druid/catalog/model/table/DatasourceDefn.java
@@ -21,7 +21,6 @@ package org.apache.druid.catalog.model.table;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
-import org.apache.druid.catalog.model.ColumnSpec;
import org.apache.druid.catalog.model.Columns;
import org.apache.druid.catalog.model.DatasourceBaseTableMetadata;
import org.apache.druid.catalog.model.DatasourceProjectionMetadata;
@@ -126,15 +125,6 @@ public class DatasourceDefn extends TableDefn
}
}
- @Override
- protected void validateColumn(ColumnSpec spec)
- {
- super.validateColumn(spec);
- if (Columns.isTimeColumn(spec.name()) && spec.dataType() != null) {
- // Validate type in next PR
- }
- }
-
/**
* Check if {@link TableSpec#type()} is {@link DatasourceDefn#TABLE_TYPE}
*/
diff --git
a/server/src/main/java/org/apache/druid/catalog/model/table/ExternalTableDefn.java
b/server/src/main/java/org/apache/druid/catalog/model/table/ExternalTableDefn.java
index 015f7195c2a..c967f3e1a9c 100644
---
a/server/src/main/java/org/apache/druid/catalog/model/table/ExternalTableDefn.java
+++
b/server/src/main/java/org/apache/druid/catalog/model/table/ExternalTableDefn.java
@@ -21,7 +21,6 @@ package org.apache.druid.catalog.model.table;
import com.fasterxml.jackson.core.type.TypeReference;
import com.google.common.annotations.VisibleForTesting;
-import org.apache.druid.catalog.model.ColumnSpec;
import org.apache.druid.catalog.model.ModelProperties.ObjectPropertyDefn;
import org.apache.druid.catalog.model.ModelProperties.PropertyDefn;
import org.apache.druid.catalog.model.ResolvedTable;
@@ -284,12 +283,6 @@ public class ExternalTableDefn extends TableDefn
return new ResolvedExternalTable(table).resolve(registry).tableFn();
}
- @Override
- protected void validateColumn(ColumnSpec colSpec)
- {
- // Validate type in next PR
- }
-
/**
* Return the {@link ExternalTableSpec} for a catalog entry for a
* fully-defined table. This form exists for completeness, since ingestion
never
diff --git
a/server/src/test/java/org/apache/druid/catalog/model/ClusteredValueGroupsBaseTableMetadataTest.java
b/server/src/test/java/org/apache/druid/catalog/model/ClusteredValueGroupsBaseTableMetadataTest.java
index e7891dd9c2a..a31a8e4dad8 100644
---
a/server/src/test/java/org/apache/druid/catalog/model/ClusteredValueGroupsBaseTableMetadataTest.java
+++
b/server/src/test/java/org/apache/druid/catalog/model/ClusteredValueGroupsBaseTableMetadataTest.java
@@ -421,7 +421,10 @@ public class ClusteredValueGroupsBaseTableMetadataTest
extends InitializedNullHa
new ColumnSpec("tags", Columns.SQL_VARCHAR_ARRAY, null),
new ColumnSpec("vals", Columns.SQL_BIGINT_ARRAY, null),
new ColumnSpec("ratios", Columns.SQL_FLOAT_ARRAY, null),
- new ColumnSpec("attrs", ColumnType.NESTED_DATA.asTypeString(), null)
+ new ColumnSpec("attrs", ColumnType.NESTED_DATA.asTypeString(), null),
+ // type prefixes match case-insensitively; these must not silently
fall back to STRING
+ new ColumnSpec("attrs2", "complex<json>", null),
+ new ColumnSpec("vals2", "array<long>", null)
);
// Declared types are retained in the ingestion schema rather than left to
inference: arrays cast an auto column
// to the declared type (an all-null batch has no values to infer from;
FLOAT ARRAY is stored as DOUBLE ARRAY by
@@ -434,7 +437,9 @@ public class ClusteredValueGroupsBaseTableMetadataTest
extends InitializedNullHa
new
AutoTypeColumnSchema("tags", ColumnType.STRING_ARRAY, null),
new
AutoTypeColumnSchema("vals", ColumnType.LONG_ARRAY, null),
new
AutoTypeColumnSchema("ratios", ColumnType.DOUBLE_ARRAY, null),
- new
NestedDataColumnSchema("attrs", NestedDataColumnSchema.DEFAULT_FORMAT_VERSION)
+ new
NestedDataColumnSchema("attrs", NestedDataColumnSchema.DEFAULT_FORMAT_VERSION),
+ new
NestedDataColumnSchema("attrs2", NestedDataColumnSchema.DEFAULT_FORMAT_VERSION),
+ new
AutoTypeColumnSchema("vals2", ColumnType.LONG_ARRAY, null)
)
.clusteringColumns("tenant")
.build(),
@@ -459,6 +464,31 @@ public class ClusteredValueGroupsBaseTableMetadataTest
extends InitializedNullHa
Assert.assertTrue(e.getMessage().contains("column [unique_things] has
unsupported type [COMPLEX<hyperUnique>]"));
}
+ @Test
+ public void testCreateSpecUnparseableTypeFails()
+ {
+ // A column declared WITHOUT a type defaults to STRING, but a declared
type that does not parse must be rejected
+ // rather than silently defaulted: the declared type is the physical
segment schema here. (A malformed
+ // parameterized type such as a missing closing bracket parses to null.)
+ final DatasourceBaseTableMetadata metadata = new
ClusteredValueGroupsBaseTableMetadata(
+ Collections.singletonList("tenant"),
+ null,
+ null
+ );
+ for (String badType : new String[]{"COMPLEX<json", "ARRAY<LONG",
"ARRAY<FOO>", "FOO"}) {
+ final List<ColumnSpec> columns = Arrays.asList(
+ new ColumnSpec("tenant", Columns.SQL_VARCHAR, null),
+ new ColumnSpec(Columns.TIME_COLUMN, null, null),
+ new ColumnSpec("busted", badType, null)
+ );
+ final DruidException e = Assert.assertThrows(DruidException.class, () ->
metadata.createSpec(columns));
+ Assert.assertTrue(
+ "expected unrecognized-type error for [" + badType + "] but got: " +
e.getMessage(),
+ e.getMessage().contains("column [busted] has an unrecognized type ["
+ badType + "]")
+ );
+ }
+ }
+
@Test
public void testCreateSpecClusteringColumnsNotLeadingPrefixFails()
{
diff --git
a/server/src/test/java/org/apache/druid/catalog/model/table/DatasourceTableTest.java
b/server/src/test/java/org/apache/druid/catalog/model/table/DatasourceTableTest.java
index 287e9477c89..ba32f61de7f 100644
---
a/server/src/test/java/org/apache/druid/catalog/model/table/DatasourceTableTest.java
+++
b/server/src/test/java/org/apache/druid/catalog/model/table/DatasourceTableTest.java
@@ -43,6 +43,7 @@ import org.apache.druid.math.expr.ExprMacroTable;
import org.apache.druid.segment.VirtualColumns;
import org.apache.druid.segment.column.ColumnType;
import org.apache.druid.segment.virtual.ExpressionVirtualColumn;
+import org.apache.druid.testing.InitializedNullHandlingTest;
import org.junit.Ignore;
import org.junit.Test;
import org.junit.experimental.categories.Category;
@@ -66,7 +67,7 @@ import static org.junit.Assert.assertTrue;
* Test of validation and serialization of the catalog table definitions.
*/
@Category(CatalogTest.class)
-public class DatasourceTableTest
+public class DatasourceTableTest extends InitializedNullHandlingTest
{
private static final Logger LOG = new Logger(DatasourceTableTest.class);
@@ -447,6 +448,39 @@ public class DatasourceTableTest
.buildSpec();
expectValidationFails(spec);
}
+
+ // A declared type must parse to a Druid type; no type at all remains
legal (covered above).
+ {
+ TableSpec spec = builder.copy()
+ .column("foo", "FOO")
+ .buildSpec();
+ DruidException e = assertThrows(DruidException.class, () ->
registry.resolve(spec).validate());
+ assertTrue(e.getMessage().contains("Column [foo] has an unrecognized
type [FOO]"));
+ }
+ {
+ // A parameterized type missing its closing bracket is malformed, and
must not silently pass as undeclared.
+ TableSpec spec = builder.copy()
+ .column("foo", "COMPLEX<json")
+ .buildSpec();
+ DruidException e = assertThrows(DruidException.class, () ->
registry.resolve(spec).validate());
+ assertTrue(e.getMessage().contains("Column [foo] has an unrecognized
type [COMPLEX<json]"));
+ }
+ {
+ // An unrecognized array element type is an invalid input, not an
internal error.
+ TableSpec spec = builder.copy()
+ .column("foo", "ARRAY<FOO>")
+ .buildSpec();
+ DruidException e = assertThrows(DruidException.class, () ->
registry.resolve(spec).validate());
+ assertTrue(e.getMessage().contains("Column [foo] has an unrecognized
type [ARRAY<FOO>]"));
+ }
+ {
+ // Parse-level validation only: any well-formed complex type is accepted
at the logical layer, whether or not
+ // its serde is registered on this server.
+ TableSpec spec = builder.copy()
+ .column("foo", "COMPLEX<thetaSketch>")
+ .buildSpec();
+ expectValidationSucceeds(spec);
+ }
}
@Test
diff --git
a/server/src/test/java/org/apache/druid/catalog/model/table/ExternalTableTest.java
b/server/src/test/java/org/apache/druid/catalog/model/table/ExternalTableTest.java
index acbea354703..20048d02cef 100644
---
a/server/src/test/java/org/apache/druid/catalog/model/table/ExternalTableTest.java
+++
b/server/src/test/java/org/apache/druid/catalog/model/table/ExternalTableTest.java
@@ -30,6 +30,7 @@ import org.apache.druid.data.input.impl.HttpInputSourceConfig;
import org.apache.druid.data.input.impl.InlineInputSource;
import org.apache.druid.data.input.impl.JsonInputFormat;
import org.apache.druid.data.input.impl.LocalInputSource;
+import org.apache.druid.error.DruidException;
import org.apache.druid.java.util.common.IAE;
import org.apache.druid.java.util.common.logger.Logger;
import org.apache.druid.metadata.DefaultPasswordProvider;
@@ -43,6 +44,7 @@ import java.util.Collections;
import java.util.Map;
import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
public class ExternalTableTest extends BaseExternTableTest
{
@@ -132,6 +134,28 @@ public class ExternalTableTest extends BaseExternTableTest
resolved.validate();
}
+ @Test
+ public void testValidateUnrecognizedColumnType()
+ {
+ // A declared column type must parse to a Druid type: an unrecognized type
would otherwise silently become
+ // STRING when the columns are converted to the input row signature.
+ CsvInputFormat format = new CsvInputFormat(
+ Collections.singletonList("a"), ";", false, false, 0, null);
+ for (String badType : new String[]{"FOO", "COMPLEX<json"}) {
+ TableMetadata table = TableBuilder.external("foo")
+ .inputSource(toMap(new InlineInputSource("a\n")))
+ .inputFormat(formatToMap(format))
+ .column("a", badType)
+ .build();
+ ResolvedTable resolved = registry.resolve(table.spec());
+ DruidException e = assertThrows(DruidException.class, () ->
resolved.validate());
+ assertTrue(
+ "expected unrecognized-type error for [" + badType + "] but got: " +
e.getMessage(),
+ e.getMessage().contains("Column [a] has an unrecognized type [" +
badType + "]")
+ );
+ }
+ }
+
/**
* Test case for multiple of the {@code ext.md} examples. To use this,
enable the
* test, run it, then copy the JSON from the console. The examples pull out
bits
diff --git
a/sql/src/main/java/org/apache/druid/sql/calcite/external/Externals.java
b/sql/src/main/java/org/apache/druid/sql/calcite/external/Externals.java
index 598bdf97375..53908dbc470 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/external/Externals.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/external/Externals.java
@@ -273,8 +273,14 @@ public class Externals
throw unsupportedType(name, dataType);
}
String simpleName = typeNameIdentifier.getSimple();
- if (StringUtils.toLowerCase(simpleName).startsWith(("complex<"))) {
- return simpleName;
+ if (StringUtils.toLowerCase(simpleName).startsWith("complex<")) {
+ // Parse and validate rather than passing the raw string downstream,
where a malformed type string would
+ // silently resolve to a different type; return the canonical form.
+ final ColumnType complexType = ColumnType.fromString(simpleName);
+ if (complexType == null) {
+ throw unsupportedType(name, dataType);
+ }
+ return complexType.asTypeString();
}
SqlTypeName type = SqlTypeName.get(simpleName);
if (type == null) {
diff --git
a/sql/src/test/java/org/apache/druid/sql/calcite/IngestTableFunctionTest.java
b/sql/src/test/java/org/apache/druid/sql/calcite/IngestTableFunctionTest.java
index 1828984d78c..47b220f7177 100644
---
a/sql/src/test/java/org/apache/druid/sql/calcite/IngestTableFunctionTest.java
+++
b/sql/src/test/java/org/apache/druid/sql/calcite/IngestTableFunctionTest.java
@@ -34,6 +34,7 @@ import org.apache.druid.data.input.impl.HttpInputSourceConfig;
import org.apache.druid.data.input.impl.JsonInputFormat;
import org.apache.druid.data.input.impl.LocalInputSource;
import org.apache.druid.data.input.impl.systemfield.SystemFields;
+import org.apache.druid.error.DruidException;
import org.apache.druid.initialization.DruidModule;
import org.apache.druid.java.util.common.ISE;
import org.apache.druid.java.util.common.StringUtils;
@@ -458,6 +459,78 @@ public class IngestTableFunctionTest extends
CalciteIngestionDmlTest
.verify();
}
+ /**
+ * The COMPLEX type prefix in an EXTEND clause matches case-insensitively
(like all other type names) and is
+ * normalized to the canonical form; the complex type name inside the
brackets keeps its casing.
+ */
+ @Test
+ public void testHttpJsonLowercaseComplexTypePrefix()
+ {
+ final ExternalDataSource httpDataSource = new ExternalDataSource(
+ new HttpInputSource(
+ Collections.singletonList(toURI("http://foo.com/bar.json";)),
+ "bob",
+ new DefaultPasswordProvider("secret"),
+ SystemFields.none(),
+ null,
+ new HttpInputSourceConfig(null, null)
+ ),
+ new JsonInputFormat(null, null, null, null, null),
+ RowSignature.builder()
+ .add("x", ColumnType.STRING)
+ .add("z", ColumnType.NESTED_DATA)
+ .build()
+ );
+ testIngestionQuery()
+ .sql("INSERT INTO dst SELECT *\n" +
+ "FROM TABLE(http(userName => 'bob',\n" +
+ " password => 'secret',\n" +
+ " uris => ARRAY['http://foo.com/bar.json'],\n" +
+ " format => 'json'))\n" +
+ " EXTEND (x VARCHAR, z TYPE('complex<json>'))\n" +
+ "PARTITIONED BY ALL TIME")
+ .authentication(CalciteTests.SUPER_USER_AUTH_RESULT)
+ .expectTarget("dst", httpDataSource.getSignature())
+ .expectResources(dataSourceWrite("dst"),
Externals.EXTERNAL_RESOURCE_ACTION)
+ .expectQuery(
+ newScanQueryBuilder()
+ .dataSource(httpDataSource)
+ .intervals(querySegmentSpec(Filtration.eternity()))
+ .columns("x", "z")
+ .columnTypes(ColumnType.STRING, ColumnType.ofComplex("json"))
+
.context(CalciteIngestionDmlTest.PARTITIONED_BY_ALL_TIME_QUERY_CONTEXT)
+ .build()
+ )
+ .verify();
+ }
+
+ /**
+ * A malformed complex type in an EXTEND clause (missing the closing
bracket) is rejected rather than silently
+ * resolving to a different type.
+ */
+ @Test
+ public void testHttpJsonMalformedComplexTypeRejected()
+ {
+ testIngestionQuery()
+ .sql("INSERT INTO dst SELECT *\n" +
+ "FROM TABLE(http(userName => 'bob',\n" +
+ " password => 'secret',\n" +
+ " uris => ARRAY['http://foo.com/bar.json'],\n" +
+ " format => 'json'))\n" +
+ " EXTEND (x VARCHAR, z TYPE('complex<json'))\n" +
+ "PARTITIONED BY ALL TIME")
+ .authentication(CalciteTests.SUPER_USER_AUTH_RESULT)
+ .expectValidationError(
+ CoreMatchers.allOf(
+ CoreMatchers.instanceOf(DruidException.class),
+ ThrowableMessageMatcher.hasMessage(
+ CoreMatchers.containsString("Column [z] has an unsupported
type")
+ )
+ )
+ )
+ .verify();
+ }
+
/**
* Basic use of an inline input source via EXTERN
*/
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
