From bf226cfc1730f59961abbc64f8a8649d352e5901 Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Fri, 30 Jun 2023 09:28:33 -0700 Subject: [PATCH 1/2] MSQ: Improve InsertTimeOutOfBounds error message. 1) Refer to the user-provided parameter "OVERWRITE WHERE" rather than the internal parameter "replaceExistingTimeChunks". 2) Include the value of "OVERWRITE WHERE", for reference. --- .../apache/druid/msq/exec/ControllerImpl.java | 2 +- .../error/InsertTimeOutOfBoundsFault.java | 37 +++++++++++++++++-- .../apache/druid/msq/exec/MSQFaultsTest.java | 7 +++- .../msq/indexing/error/MSQFaultSerdeTest.java | 6 ++- 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerImpl.java b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerImpl.java index 72657ed0e280..eebc95077b35 100644 --- a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerImpl.java +++ b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerImpl.java @@ -1007,7 +1007,7 @@ private List generateSegmentIdsWithShardSpecsForReplace( // Validate interval against the replaceTimeChunks set of intervals. if (destination.getReplaceTimeChunks().stream().noneMatch(chunk -> chunk.contains(interval))) { - throw new MSQException(new InsertTimeOutOfBoundsFault(interval)); + throw new MSQException(new InsertTimeOutOfBoundsFault(interval, destination.getReplaceTimeChunks())); } final List> ranges = bucketEntry.getValue(); diff --git a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/error/InsertTimeOutOfBoundsFault.java b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/error/InsertTimeOutOfBoundsFault.java index c15a3fb6d17a..7c0d9e035e72 100644 --- a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/error/InsertTimeOutOfBoundsFault.java +++ b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/error/InsertTimeOutOfBoundsFault.java @@ -22,6 +22,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; import org.joda.time.Interval; +import java.util.List; import java.util.Objects; public class InsertTimeOutOfBoundsFault extends BaseMSQFault @@ -29,11 +30,21 @@ public class InsertTimeOutOfBoundsFault extends BaseMSQFault static final String CODE = "InsertTimeOutOfBounds"; private final Interval interval; + private final List intervalBounds; - public InsertTimeOutOfBoundsFault(@JsonProperty("interval") Interval interval) + public InsertTimeOutOfBoundsFault( + @JsonProperty("interval") Interval interval, + @JsonProperty("intervalBounds") List intervalBounds + ) { - super(CODE, "Query generated time chunk [%s] out of bounds specified by replaceExistingTimeChunks", interval); + super( + CODE, + "Query generated time chunk[%s] out of bounds specified by OVERWRITE WHERE[%s]", + interval, + intervalBounds + ); this.interval = interval; + this.intervalBounds = intervalBounds; } @JsonProperty @@ -42,6 +53,12 @@ public Interval getInterval() return interval; } + @JsonProperty + public List getIntervalBounds() + { + return intervalBounds; + } + @Override public boolean equals(Object o) { @@ -55,12 +72,24 @@ public boolean equals(Object o) return false; } InsertTimeOutOfBoundsFault that = (InsertTimeOutOfBoundsFault) o; - return Objects.equals(interval, that.interval); + return Objects.equals(interval, that.interval) && Objects.equals( + intervalBounds, + that.intervalBounds + ); } @Override public int hashCode() { - return Objects.hash(super.hashCode(), interval); + return Objects.hash(super.hashCode(), interval, intervalBounds); + } + + @Override + public String toString() + { + return "InsertTimeOutOfBoundsFault{" + + "interval=" + interval + + ", intervalBounds=" + intervalBounds + + '}'; } } diff --git a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/exec/MSQFaultsTest.java b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/exec/MSQFaultsTest.java index cfcc1073331b..b3bf3c408a29 100644 --- a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/exec/MSQFaultsTest.java +++ b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/exec/MSQFaultsTest.java @@ -153,7 +153,12 @@ public void testInsertTimeOutOfBoundsFault() "replace into foo1 overwrite where __time >= TIMESTAMP '2002-01-02 00:00:00' and __time < TIMESTAMP '2002-01-03 00:00:00' select __time, dim1 , count(*) as cnt from foo where dim1 is not null group by 1, 2 PARTITIONED by day clustered by dim1") .setExpectedDataSource("foo1") .setExpectedRowSignature(rowSignature) - .setExpectedMSQFault(new InsertTimeOutOfBoundsFault(Intervals.of("2000-01-02T00:00:00.000Z/2000-01-03T00:00:00.000Z"))) + .setExpectedMSQFault( + new InsertTimeOutOfBoundsFault( + Intervals.of("2000-01-02T00:00:00.000Z/2000-01-03T00:00:00.000Z"), + Collections.singletonList(Intervals.of("2002-01-02/2002-01-03")) + ) + ) .verifyResults(); } diff --git a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/indexing/error/MSQFaultSerdeTest.java b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/indexing/error/MSQFaultSerdeTest.java index bcd0de1ef689..9e0dbd8accf3 100644 --- a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/indexing/error/MSQFaultSerdeTest.java +++ b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/indexing/error/MSQFaultSerdeTest.java @@ -32,6 +32,7 @@ import java.io.IOException; import java.util.Arrays; +import java.util.Collections; public class MSQFaultSerdeTest { @@ -63,7 +64,10 @@ public void testFaultSerde() throws IOException assertFaultSerde(new InsertCannotBeEmptyFault("the datasource")); assertFaultSerde(InsertLockPreemptedFault.INSTANCE); assertFaultSerde(InsertTimeNullFault.INSTANCE); - assertFaultSerde(new InsertTimeOutOfBoundsFault(Intervals.ETERNITY)); + assertFaultSerde(new InsertTimeOutOfBoundsFault( + Intervals.of("2001/2002"), + Collections.singletonList(Intervals.of("2000/2001")) + )); assertFaultSerde(new InvalidNullByteFault("the source", 1, "the column", "the value", 2)); assertFaultSerde(new NotEnoughMemoryFault(1000, 1000, 900, 1, 2)); assertFaultSerde(QueryNotSupportedFault.INSTANCE); From 080d69c1aa40947e4208e53356f69b1ca9bc8777 Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Fri, 30 Jun 2023 09:34:42 -0700 Subject: [PATCH 2/2] Even nicer error message. --- .../druid/msq/indexing/error/InsertTimeOutOfBoundsFault.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/error/InsertTimeOutOfBoundsFault.java b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/error/InsertTimeOutOfBoundsFault.java index 7c0d9e035e72..3a204cb79cfc 100644 --- a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/error/InsertTimeOutOfBoundsFault.java +++ b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/error/InsertTimeOutOfBoundsFault.java @@ -39,7 +39,9 @@ public InsertTimeOutOfBoundsFault( { super( CODE, - "Query generated time chunk[%s] out of bounds specified by OVERWRITE WHERE[%s]", + "Inserted data contains time chunk[%s] outside of bounds specified by OVERWRITE WHERE[%s]. " + + "If you want to include this data, expand your OVERWRITE WHERE. " + + "If you do not want to include this data, use SELECT ... WHERE to filter it from your inserted data.", interval, intervalBounds );