From 55fc0c9aa4b33c3ab0b60fc05cd159a4cdfb402e Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Wed, 1 Dec 2021 22:27:36 -0800 Subject: [PATCH 1/2] allow `DruidSchema` to fallback to segment metadata type if typeSignature is null, to avoid producing incorrect SQL schema if broker is upgraded to 0.23 before historicals --- .../druid/sql/calcite/schema/DruidSchema.java | 16 ++- .../sql/calcite/schema/DruidSchemaTest.java | 101 ++++++++++++++++++ 2 files changed, 114 insertions(+), 3 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/schema/DruidSchema.java b/sql/src/main/java/org/apache/druid/sql/calcite/schema/DruidSchema.java index 5db9bfa46895..42fc8526d847 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/schema/DruidSchema.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/schema/DruidSchema.java @@ -875,7 +875,8 @@ Sequence runSegmentMetadataQuery( .runSimple(segmentMetadataQuery, escalator.createEscalatedAuthenticationResult(), Access.OK); } - private static RowSignature analysisToRowSignature(final SegmentAnalysis analysis) + @VisibleForTesting + static RowSignature analysisToRowSignature(final SegmentAnalysis analysis) { final RowSignature.Builder rowSignatureBuilder = RowSignature.builder(); for (Map.Entry entry : analysis.getColumns().entrySet()) { @@ -886,9 +887,18 @@ private static RowSignature analysisToRowSignature(final SegmentAnalysis analysi ColumnType valueType = entry.getValue().getTypeSignature(); - // this shouldn't happen, but if it does assume types are some flavor of COMPLEX. + // this shouldn't happen, but if it does, first try to fall back to legacy type information field in case + // standard upgrade order was not followed for 0.22 to 0.23+, and if that also fails, then assume types are some + // flavor of COMPLEX. if (valueType == null) { - valueType = ColumnType.UNKNOWN_COMPLEX; + // at some point in the future this can be simplified to the contents of the catch clause here, once the + // likelyhood of upgrading from some version lower than 0.23 is low + try { + valueType = ColumnType.fromString(entry.getValue().getType()); + } + catch (IllegalArgumentException ignored) { + valueType = ColumnType.UNKNOWN_COMPLEX; + } } rowSignatureBuilder.add(entry.getKey(), valueType); diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/schema/DruidSchemaTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/schema/DruidSchemaTest.java index 2bfd3ca0038b..2f229bc7f657 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/schema/DruidSchemaTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/schema/DruidSchemaTest.java @@ -40,11 +40,15 @@ import org.apache.druid.query.aggregation.LongSumAggregatorFactory; import org.apache.druid.query.aggregation.hyperloglog.HyperUniquesAggregatorFactory; import org.apache.druid.query.metadata.metadata.AllColumnIncluderator; +import org.apache.druid.query.metadata.metadata.ColumnAnalysis; +import org.apache.druid.query.metadata.metadata.SegmentAnalysis; import org.apache.druid.query.metadata.metadata.SegmentMetadataQuery; import org.apache.druid.query.spec.MultipleSpecificSegmentSpec; import org.apache.druid.segment.IndexBuilder; import org.apache.druid.segment.QueryableIndex; import org.apache.druid.segment.TestHelper; +import org.apache.druid.segment.column.ColumnType; +import org.apache.druid.segment.column.RowSignature; import org.apache.druid.segment.incremental.IncrementalIndexSchema; import org.apache.druid.segment.join.MapJoinableFactory; import org.apache.druid.segment.writeout.OffHeapMemorySegmentWriteOutMediumFactory; @@ -65,6 +69,7 @@ import org.apache.druid.timeline.partition.LinearShardSpec; import org.apache.druid.timeline.partition.NumberedShardSpec; import org.easymock.EasyMock; +import org.joda.time.Interval; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -1087,6 +1092,102 @@ public void testRunSegmentMetadataQueryWithContext() throws Exception } + @Test + public void testSegmentMetadataColumnType() + { + RowSignature signature = DruidSchema.analysisToRowSignature( + new SegmentAnalysis( + "id", + ImmutableList.of(new Interval(1L, 2L)), + ImmutableMap.of( + "a", + new ColumnAnalysis( + ColumnType.STRING, + ColumnType.STRING.asTypeString(), + false, + true, + 1234, + 26, + "a", + "z", + null + ), + "count", + new ColumnAnalysis( + ColumnType.LONG, + ColumnType.LONG.asTypeString(), + false, + true, + 1234, + 26, + "a", + "z", + null + ) + ), + 1234, + 100, + null, + null, + null, + null + ) + ); + + Assert.assertEquals( + RowSignature.builder().add("a", ColumnType.STRING).add("count", ColumnType.LONG).build(), + signature + ); + } + + + @Test + public void testSegmentMetadataFallbackType() + { + RowSignature signature = DruidSchema.analysisToRowSignature( + new SegmentAnalysis( + "id", + ImmutableList.of(new Interval(1L, 2L)), + ImmutableMap.of( + "a", + new ColumnAnalysis( + null, + ColumnType.STRING.asTypeString(), + false, + true, + 1234, + 26, + "a", + "z", + null + ), + "count", + new ColumnAnalysis( + null, + ColumnType.LONG.asTypeString(), + false, + true, + 1234, + 26, + "a", + "z", + null + ) + ), + 1234, + 100, + null, + null, + null, + null + ) + ); + Assert.assertEquals( + RowSignature.builder().add("a", ColumnType.STRING).add("count", ColumnType.LONG).build(), + signature + ); + } + private static DataSegment newSegment(String datasource, int partitionId) { return new DataSegment( From 477ec17bd40b378649740cd0edea2a1a85f03ec9 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Thu, 2 Dec 2021 11:26:29 -0800 Subject: [PATCH 2/2] mmm, forbidden tests --- .../org/apache/druid/sql/calcite/schema/DruidSchemaTest.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/schema/DruidSchemaTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/schema/DruidSchemaTest.java index 2f229bc7f657..0d0ba2f6f95d 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/schema/DruidSchemaTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/schema/DruidSchemaTest.java @@ -69,7 +69,6 @@ import org.apache.druid.timeline.partition.LinearShardSpec; import org.apache.druid.timeline.partition.NumberedShardSpec; import org.easymock.EasyMock; -import org.joda.time.Interval; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -1098,7 +1097,7 @@ public void testSegmentMetadataColumnType() RowSignature signature = DruidSchema.analysisToRowSignature( new SegmentAnalysis( "id", - ImmutableList.of(new Interval(1L, 2L)), + ImmutableList.of(Intervals.utc(1L, 2L)), ImmutableMap.of( "a", new ColumnAnalysis( @@ -1147,7 +1146,7 @@ public void testSegmentMetadataFallbackType() RowSignature signature = DruidSchema.analysisToRowSignature( new SegmentAnalysis( "id", - ImmutableList.of(new Interval(1L, 2L)), + ImmutableList.of(Intervals.utc(1L, 2L)), ImmutableMap.of( "a", new ColumnAnalysis(