You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "gianm (via GitHub)" <gi...@apache.org> on 2024/08/06 21:18:03 UTC
[PR] Segments sorted by non-time columns. (druid)
gianm opened a new pull request, #16849:
URL: https://github.com/apache/druid/pull/16849
Currently, segments are always sorted by __time, followed by the sort order provided by the user via dimensionsSpec or CLUSTERED BY. Sorting by __time enables efficient execution of queries involving time-ordering or granularity. Time-ordering is a simple matter of reading the rows in stored order, and granular cursors can be generated in streaming fashion.
However, for various workloads, it's better for storage footprint and query performance to sort by arbitrary orders that do not start with __time. With this patch, users can sort segments by such orders.
For spec-based ingestion, users add "useExplicitSegmentSortOrder: true" to dimensionsSpec. The "dimensions" list determines the sort order. To define a sort order that includes "__time", users explicitly include a dimension named "__time".
For SQL-based ingestion, users set the context parameter "useExplicitSegmentSortOrder: true". The CLUSTERED BY clause is then used as the explicit segment sort order.
In both cases, when the new "useExplicitSegmentSortOrder" parameter is false (the default), __time is implicitly prepended to the sort order, as it always was prior to this patch.
The new parameter is experimental for two main reasons. First, such segments can cause errors when loaded by older servers, due to violating their expectations that timestamps are always monotonically increasing. Second, even on newer servers, not all queries can run on non-time-sorted segments. Scan queries involving time-ordering and any query involving granularity will not run. (To partially mitigate this, a currently-undocumented SQL feature "sqlUseGranularity" is provided. When set to false the SQL planner avoids using "granularity".)
Changes on the write path:
1) DimensionsSpec can now optionally contain a __time dimension, which
controls the placement of __time in the sort order. If not present,
__time is considered to be first in the sort order, as it has always
been.
2) IncrementalIndex and IndexMerger are updated to sort facts more
flexibly; not always by time first.
3) Metadata (stored in metadata.drd) gains a "sortOrder" field.
4) MSQ can generate range-based shard specs even when not all columns are
singly-valued strings. It merely stops accepting new clustering key
fields when it encounters the first one that isn't a singly-valued
string. This is useful because it enables range shard specs on
"someDim" to be created for clauses like "CLUSTERED BY someDim, __time".
Changes on the read path:
1) Add StorageAdapter#getSortOrder so query engines can tell how a
segment is sorted.
2) Update QueryableIndexStorageAdapter, IncrementalIndexStorageAdapter,
and VectorCursorGranularizer to throw errors when using granularities
on non-time-ordered segments.
3) Update ScanQueryEngine to throw an error when using the time-ordering
"order" parameter on non-time-ordered segments.
4) Update TimeBoundaryQueryRunnerFactory to perform a segment scan when
running on a non-time-ordered segment.
5) Add "sqlUseGranularity" context parameter that causes the SQL planner
to avoid using granularities other than ALL.
Other changes:
1) Rename DimensionsSpec "hasCustomDimensions" to "hasFixedDimensions"
and change the meaning subtly: it now returns true if the DimensionsSpec
represents an unchanging list of dimensions, or false if there is
some discovery happening. This is what call sites had expected anyway.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #16849:
URL: https://github.com/apache/druid/pull/16849#discussion_r1706160179
##########
processing/src/main/java/org/apache/druid/segment/TimeAndDimsPointer.java:
##########
@@ -148,19 +177,23 @@
@Override
public int compareTo(@Nonnull TimeAndDimsPointer rhs)
{
- long timestamp = getTimestamp();
- long rhsTimestamp = rhs.getTimestamp();
- int timestampDiff = Long.compare(timestamp, rhsTimestamp);
- if (timestampDiff != 0) {
- return timestampDiff;
- }
- for (int dimIndex = 0; dimIndex < dimensionSelectors.length; dimIndex++) {
- int dimDiff = dimensionSelectorComparators[dimIndex].compare(
- dimensionSelectors[dimIndex],
- rhs.dimensionSelectors[dimIndex]
- );
- if (dimDiff != 0) {
- return dimDiff;
+ for (int index = 0; index < (dimensionSelectors.length + 1); index++) {
+ if (index == timePosition) {
+ final long timestamp = getTimestamp();
+ final long rhsTimestamp = rhs.getTimestamp();
+ final int timestampDiff = Long.compare(timestamp, rhsTimestamp);
+ if (timestampDiff != 0) {
+ return timestampDiff;
+ }
+ } else {
+ final int dimIndex = index < timePosition ? index : index - 1;
+ final int dimDiff = dimensionSelectorComparators[dimIndex].compare(
+ dimensionSelectors[dimIndex],
Review Comment:
It can't be out of bounds, because if it's easy to the array length, it would get decremented for being `>= timePosition`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on PR #16849:
URL: https://github.com/apache/druid/pull/16849#issuecomment-2274560960
From the discussion in https://github.com/apache/druid/pull/16849#discussion_r1708084545, after #16533 is merged we should revisit this one to (a) resolve conflict (b) replace `List<String>` with `List<OrderBy>` in the sort order field.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "clintropolis (via GitHub)" <gi...@apache.org>.
clintropolis commented on PR #16849:
URL: https://github.com/apache/druid/pull/16849#issuecomment-2623266513
>I think this PR broke dataSourceMetadata query.
Seems like MaxIngestedEventTimeInspector in DataSourceMetadataQueryRunner will always be null.
The intention of the new `MaxIngestedEventTimeInspector` is that it is only for use with realtime data, because every other implementation on the old `StorageAdapter` of `getMaxIngestedEventTime()` was just calling `getMaxTime()` which gives the end interval of the segment. Could use `timeBoundary` instead if there is no realtime data.
It does seem odd we return min datetime, i would have expected null. I don't think it makes much sense to have an implementation of `MaxIngestedEventTimeInspector` for published segments given that `timeBoundary` exists, but the docs could probably more clearly spell this out. Apologies for the behavior change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #16849:
URL: https://github.com/apache/druid/pull/16849#discussion_r1727561448
##########
docs/querying/sql-query-context.md:
##########
@@ -50,11 +50,11 @@ Configure Druid SQL query planning using the parameters in the table below.
|`sqlReverseLookup`|Whether to consider the [reverse-lookup rewrite](lookups.md#reverse-lookup) of the `LOOKUP` function during SQL planning.<br /><br />Calls to `LOOKUP` are only reversed when the number of matching keys is lower than both `inSubQueryThreshold` and `sqlReverseLookupThreshold`.|true|
|`sqlReverseLookupThreshold`|Maximum size of `IN` filter to create when applying a [reverse-lookup rewrite](lookups.md#reverse-lookup). If a `LOOKUP` call matches more keys than this threshold, it is left as-is.<br /><br />If `inSubQueryThreshold` is lower than `sqlReverseLookupThreshold`, the `inSubQueryThreshold` is used as the threshold instead.|10000|
|`sqlPullUpLookup`|Whether to consider the [pull-up rewrite](lookups.md#pull-up) of the `LOOKUP` function during SQL planning.|true|
-|`enableJoinLeftTableScanDirect`|`false`|This flag applies to queries which have joins. For joins, where left child is a simple scan with a filter, by default, druid will run the scan as a query and the join the results to the right child on broker. Setting this flag to true overrides that behavior and druid will attempt to push the join to data servers instead. Please note that the flag could be applicable to queries even if there is no explicit join. since queries can internally translated into a join by the SQL planner.|
-|`maxNumericInFilters`|`-1`|Max limit for the amount of numeric values that can be compared for a string type dimension when the entire SQL WHERE clause of a query translates only to an [OR](../querying/filters.md#or) of [Bound filter](../querying/filters.md#bound-filter). By default, Druid does not restrict the amount of of numeric Bound Filters on String columns, although this situation may block other queries from running. Set this parameter to a smaller value to prevent Druid from running queries that have prohibitively long segment processing times. The optimal limit requires some trial and error; we recommend starting with 100. Users who submit a query that exceeds the limit of `maxNumericInFilters` should instead rewrite their queries to use strings in the `WHERE` clause instead of numbers. For example, `WHERE someString IN (‘123’, ‘456’)`. This value cannot exceed the set system configuration `druid.sql.planner.maxNumericInFilters`. This value is ignored if `druid.s
ql.planner.maxNumericInFilters` is not set explicitly.|
-|`inFunctionThreshold`|`100`| At or beyond this threshold number of values, SQL `IN` is converted to [`SCALAR_IN_ARRAY`](sql-functions.md#scalar_in_array). A threshold of 0 forces this conversion in all cases. A threshold of [Integer.MAX_VALUE] disables this conversion. The converted function is eligible for fewer planning-time optimizations, which speeds up planning, but may prevent certain planning-time optimizations.|
-|`inFunctionExprThreshold`|`2`| At or beyond this threshold number of values, SQL `IN` is eligible for execution using the native function `scalar_in_array` rather than an <code>||</code> of `==`, even if the number of values is below `inFunctionThreshold`. This property only affects translation of SQL `IN` to a [native expression](math-expr.md). It does not affect translation of SQL `IN` to a [native filter](filters.md). This property is provided for backwards compatibility purposes, and may be removed in a future release.|
-|`inSubQueryThreshold`|`2147483647`| At or beyond this threshold number of values, SQL `IN` is converted to `JOIN` on an inline table. `inFunctionThreshold` takes priority over this setting. A threshold of 0 forces usage of an inline table in all cases where the size of a SQL `IN` is larger than `inFunctionThreshold`. A threshold of `2147483647` disables the rewrite of SQL `IN` to `JOIN`. |
+|`enableJoinLeftTableScanDirect`|This flag applies to queries which have joins. For joins, where left child is a simple scan with a filter, by default, druid will run the scan as a query and the join the results to the right child on broker. Setting this flag to true overrides that behavior and druid will attempt to push the join to data servers instead. Please note that the flag could be applicable to queries even if there is no explicit join. since queries can internally translated into a join by the SQL planner.|`false`|
+|`maxNumericInFilters`|Max limit for the amount of numeric values that can be compared for a string type dimension when the entire SQL WHERE clause of a query translates only to an [OR](../querying/filters.md#or) of [Bound filter](../querying/filters.md#bound-filter). By default, Druid does not restrict the amount of of numeric Bound Filters on String columns, although this situation may block other queries from running. Set this parameter to a smaller value to prevent Druid from running queries that have prohibitively long segment processing times. The optimal limit requires some trial and error; we recommend starting with 100. Users who submit a query that exceeds the limit of `maxNumericInFilters` should instead rewrite their queries to use strings in the `WHERE` clause instead of numbers. For example, `WHERE someString IN (‘123’, ‘456’)`. This value cannot exceed the set system configuration `druid.sql.planner.maxNumericInFilters`. This value is ignored if `druid.sql.pl
anner.maxNumericInFilters` is not set explicitly.|`-1`|
+|`inFunctionThreshold`| At or beyond this threshold number of values, SQL `IN` is converted to [`SCALAR_IN_ARRAY`](sql-functions.md#scalar_in_array). A threshold of 0 forces this conversion in all cases. A threshold of [Integer.MAX_VALUE] disables this conversion. The converted function is eligible for fewer planning-time optimizations, which speeds up planning, but may prevent certain planning-time optimizations.| `100`|
+|`inFunctionExprThreshold`|At or beyond this threshold number of values, SQL `IN` is eligible for execution using the native function `scalar_in_array` rather than an <code>||</code> of `==`, even if the number of values is below `inFunctionThreshold`. This property only affects translation of SQL `IN` to a [native expression](math-expr.md). It does not affect translation of SQL `IN` to a [native filter](filters.md). This property is provided for backwards compatibility purposes, and may be removed in a future release.|`2`|
+|`inSubQueryThreshold`|At or beyond this threshold number of values, SQL `IN` is converted to `JOIN` on an inline table. `inFunctionThreshold` takes priority over this setting. A threshold of 0 forces usage of an inline table in all cases where the size of a SQL `IN` is larger than `inFunctionThreshold`. A threshold of `2147483647` disables the rewrite of SQL `IN` to `JOIN`. |`2147483647`|
Review Comment:
I meant to leave it undocumented, for the reason mentioned in the PR description,
> This is undocumented, because it's hopefully a short-term hack. The more ideal thing would be to have all the native queries work properly on these segments.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on code in PR #16849:
URL: https://github.com/apache/druid/pull/16849#discussion_r1706147840
##########
extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/exec/MSQReplaceTest.java:
##########
@@ -194,6 +196,393 @@
.verifyResults();
}
+ @MethodSource("data")
+ @ParameterizedTest(name = "{index}:with context {0}")
+ public void testReplaceOnFooWithAllClusteredByDim(String contextName, Map<String, Object> context)
+ {
+ // Tests [CLUSTERED BY dim1] with the default useExplicitSegmentSortOrder (false). In this case,
+ // partitioning uses [dim1] but segment sort uses [__time, dim1].
+ RowSignature rowSignature = RowSignature.builder()
+ .add("__time", ColumnType.LONG)
+ .add("dim1", ColumnType.STRING)
+ .add("m1", ColumnType.FLOAT)
+ .build();
+
+ DataSegment existingDataSegment0 = DataSegment.builder()
+ .interval(Intervals.of("2000-01-01T/2000-01-04T"))
+ .size(50)
+ .version(MSQTestTaskActionClient.VERSION)
+ .dataSource("foo")
+ .build();
+
+ DataSegment existingDataSegment1 = DataSegment.builder()
+ .interval(Intervals.of("2001-01-01T/2001-01-04T"))
+ .size(50)
+ .version(MSQTestTaskActionClient.VERSION)
+ .dataSource("foo")
+ .build();
+
+ Mockito.doCallRealMethod()
+ .doReturn(ImmutableSet.of(existingDataSegment0, existingDataSegment1))
+ .when(testTaskActionClient)
+ .submit(new RetrieveUsedSegmentsAction(
+ EasyMock.eq("foo"),
+ EasyMock.eq(ImmutableList.of(Intervals.ETERNITY))
+ ));
+
+ testIngestQuery().setSql(" REPLACE INTO foo OVERWRITE ALL "
+ + "SELECT __time, dim1, m1 "
+ + "FROM foo "
+ + "PARTITIONED BY ALL "
+ + "CLUSTERED BY dim1")
+ .setExpectedDataSource("foo")
+ .setExpectedRowSignature(rowSignature)
+ .setQueryContext(context)
+ .setExpectedDestinationIntervals(Intervals.ONLY_ETERNITY)
+ .setExpectedSegments(
+ ImmutableSet.of(
+ SegmentId.of("foo", Intervals.ETERNITY, "test", 0)
+ )
+ )
+ .setExpectedShardSpec(DimensionRangeShardSpec.class)
+ .setExpectedResultRows(
+ ImmutableList.of(
+ new Object[]{946684800000L, "", 1.0f},
+ new Object[]{946771200000L, "10.1", 2.0f},
+ new Object[]{946857600000L, "2", 3.0f},
+ new Object[]{978307200000L, "1", 4.0f},
+ new Object[]{978393600000L, "def", 5.0f},
+ new Object[]{978480000000L, "abc", 6.0f}
+ )
+ )
+ .setExpectedSegmentGenerationProgressCountersForStageWorker(
+ CounterSnapshotMatcher
+ .with().segmentRowsProcessed(6),
+ 1, 0
+ )
+ .setExpectedLastCompactionState(
+ expectedCompactionState(
+ context,
+ Collections.singletonList("dim1"),
+ DimensionsSpec.builder()
+ .setDimensions(
+ ImmutableList.of(
+ new StringDimensionSchema("dim1"),
+ new FloatDimensionSchema("m1")
+ )
+ )
+ .setDimensionExclusions(Collections.singletonList("__time"))
+ .build(),
+ GranularityType.ALL,
+ Intervals.ETERNITY
+ )
+ )
+ .verifyResults();
+ }
+
+ @MethodSource("data")
+ @ParameterizedTest(name = "{index}:with context {0}")
+ public void testReplaceOnFooWithAllClusteredByDimExplicitSort(String contextName, Map<String, Object> context)
Review Comment:
## Useless parameter
The parameter 'contextName' is never used.
[Show more details](https://github.com/apache/druid/security/code-scanning/7679)
##########
processing/src/main/java/org/apache/druid/segment/incremental/IncrementalIndex.java:
##########
@@ -887,19 +942,23 @@
* Currently called to initialize IncrementalIndex dimension order during index creation
* Index dimension ordering could be changed to initialize from DimensionsSpec after resolution of
* https://github.com/apache/druid/issues/2011
+ *
+ * @param oldDimensionOrder dimension order to initialize
+ * @param oldColumnFormats formats for the dimensions
*/
public void loadDimensionIterable(
Iterable<String> oldDimensionOrder,
- Map<String, ColumnFormat> oldColumnCapabilities
+ Map<String, ColumnFormat> oldColumnFormats
)
{
synchronized (dimensionDescs) {
- if (!dimensionDescs.isEmpty()) {
- throw new ISE("Cannot load dimension order when existing order[%s] is not empty.", dimensionDescs.keySet());
+ if (size() != 0) {
+ throw new ISE("Cannot load dimension order when existing index is not empty.", dimensionDescs.keySet());
Review Comment:
## Unused format argument
This format call refers to 0 argument(s) but supplies 1 argument(s).
[Show more details](https://github.com/apache/druid/security/code-scanning/7683)
##########
server/src/test/java/org/apache/druid/segment/indexing/DataSchemaTest.java:
##########
@@ -251,29 +254,124 @@
}
@Test
- public void testOverlapTimeAndDim()
+ public void testOverlapTimeAndDimPositionZero()
{
- expectedException.expect(IllegalArgumentException.class);
+ DataSchema schema = new DataSchema(
+ IdUtilsTest.VALID_ID_CHARS,
+ new TimestampSpec("time", "auto", null),
+ DimensionsSpec.builder()
+ .setDimensions(
+ ImmutableList.of(
+ new LongDimensionSchema("__time"),
+ new StringDimensionSchema("dimA"),
+ new StringDimensionSchema("dimB")
+ )
+ )
+ .setDimensionExclusions(ImmutableList.of("dimC"))
+ .build(),
+ null,
+ new ArbitraryGranularitySpec(Granularities.DAY, ImmutableList.of(Intervals.of("2014/2015"))),
+ null,
+ null,
+ jsonMapper
+ );
+
+ Assert.assertEquals(
+ ImmutableList.of("__time", "dimA", "dimB"),
+ schema.getDimensionsSpec().getDimensionNames()
+ );
+
+ Assert.assertFalse(schema.getDimensionsSpec().isUseExplicitSegmentSortOrder());
+ }
+
+ @Test
+ public void testOverlapTimeAndDimPositionZeroWrongType()
+ {
+ expectedException.expect(DruidException.class);
+ expectedException.expectMessage("Encountered dimension[__time] with incorrect type[STRING]. Type must be 'long'.");
+
+ DataSchema schema = new DataSchema(
+ IdUtilsTest.VALID_ID_CHARS,
+ new TimestampSpec("time", "auto", null),
+ DimensionsSpec.builder()
+ .setDimensions(
+ ImmutableList.of(
+ new StringDimensionSchema("__time"),
+ new StringDimensionSchema("dimA"),
+ new StringDimensionSchema("dimB")
+ )
+ )
+ .setDimensionExclusions(ImmutableList.of("dimC"))
+ .build(),
+ null,
+ new ArbitraryGranularitySpec(Granularities.DAY, ImmutableList.of(Intervals.of("2014/2015"))),
+ null,
+ null,
+ jsonMapper
+ );
Review Comment:
## Unread local variable
Variable 'DataSchema schema' is never read.
[Show more details](https://github.com/apache/druid/security/code-scanning/7684)
##########
extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/exec/MSQReplaceTest.java:
##########
@@ -194,6 +196,393 @@
.verifyResults();
}
+ @MethodSource("data")
+ @ParameterizedTest(name = "{index}:with context {0}")
+ public void testReplaceOnFooWithAllClusteredByDim(String contextName, Map<String, Object> context)
+ {
+ // Tests [CLUSTERED BY dim1] with the default useExplicitSegmentSortOrder (false). In this case,
+ // partitioning uses [dim1] but segment sort uses [__time, dim1].
+ RowSignature rowSignature = RowSignature.builder()
+ .add("__time", ColumnType.LONG)
+ .add("dim1", ColumnType.STRING)
+ .add("m1", ColumnType.FLOAT)
+ .build();
+
+ DataSegment existingDataSegment0 = DataSegment.builder()
+ .interval(Intervals.of("2000-01-01T/2000-01-04T"))
+ .size(50)
+ .version(MSQTestTaskActionClient.VERSION)
+ .dataSource("foo")
+ .build();
+
+ DataSegment existingDataSegment1 = DataSegment.builder()
+ .interval(Intervals.of("2001-01-01T/2001-01-04T"))
+ .size(50)
+ .version(MSQTestTaskActionClient.VERSION)
+ .dataSource("foo")
+ .build();
+
+ Mockito.doCallRealMethod()
+ .doReturn(ImmutableSet.of(existingDataSegment0, existingDataSegment1))
+ .when(testTaskActionClient)
+ .submit(new RetrieveUsedSegmentsAction(
+ EasyMock.eq("foo"),
+ EasyMock.eq(ImmutableList.of(Intervals.ETERNITY))
+ ));
+
+ testIngestQuery().setSql(" REPLACE INTO foo OVERWRITE ALL "
+ + "SELECT __time, dim1, m1 "
+ + "FROM foo "
+ + "PARTITIONED BY ALL "
+ + "CLUSTERED BY dim1")
+ .setExpectedDataSource("foo")
+ .setExpectedRowSignature(rowSignature)
+ .setQueryContext(context)
+ .setExpectedDestinationIntervals(Intervals.ONLY_ETERNITY)
+ .setExpectedSegments(
+ ImmutableSet.of(
+ SegmentId.of("foo", Intervals.ETERNITY, "test", 0)
+ )
+ )
+ .setExpectedShardSpec(DimensionRangeShardSpec.class)
+ .setExpectedResultRows(
+ ImmutableList.of(
+ new Object[]{946684800000L, "", 1.0f},
+ new Object[]{946771200000L, "10.1", 2.0f},
+ new Object[]{946857600000L, "2", 3.0f},
+ new Object[]{978307200000L, "1", 4.0f},
+ new Object[]{978393600000L, "def", 5.0f},
+ new Object[]{978480000000L, "abc", 6.0f}
+ )
+ )
+ .setExpectedSegmentGenerationProgressCountersForStageWorker(
+ CounterSnapshotMatcher
+ .with().segmentRowsProcessed(6),
+ 1, 0
+ )
+ .setExpectedLastCompactionState(
+ expectedCompactionState(
+ context,
+ Collections.singletonList("dim1"),
+ DimensionsSpec.builder()
+ .setDimensions(
+ ImmutableList.of(
+ new StringDimensionSchema("dim1"),
+ new FloatDimensionSchema("m1")
+ )
+ )
+ .setDimensionExclusions(Collections.singletonList("__time"))
+ .build(),
+ GranularityType.ALL,
+ Intervals.ETERNITY
+ )
+ )
+ .verifyResults();
+ }
+
+ @MethodSource("data")
+ @ParameterizedTest(name = "{index}:with context {0}")
+ public void testReplaceOnFooWithAllClusteredByDimExplicitSort(String contextName, Map<String, Object> context)
+ {
+ RowSignature rowSignature = RowSignature.builder()
+ .add("__time", ColumnType.LONG)
+ .add("dim1", ColumnType.STRING)
+ .add("m1", ColumnType.FLOAT)
+ .build();
+
+ DataSegment existingDataSegment0 = DataSegment.builder()
+ .interval(Intervals.of("2000-01-01T/2000-01-04T"))
+ .size(50)
+ .version(MSQTestTaskActionClient.VERSION)
+ .dataSource("foo")
+ .build();
+
+ DataSegment existingDataSegment1 = DataSegment.builder()
+ .interval(Intervals.of("2001-01-01T/2001-01-04T"))
+ .size(50)
+ .version(MSQTestTaskActionClient.VERSION)
+ .dataSource("foo")
+ .build();
+
+ Map<String, Object> queryContext = new HashMap<>(context);
+ queryContext.put(DimensionsSpec.PARAMETER_EXPLICIT_SORT_ORDER, true);
+
+ Mockito.doCallRealMethod()
+ .doReturn(ImmutableSet.of(existingDataSegment0, existingDataSegment1))
+ .when(testTaskActionClient)
+ .submit(new RetrieveUsedSegmentsAction(
+ EasyMock.eq("foo"),
+ EasyMock.eq(ImmutableList.of(Intervals.ETERNITY))
+ ));
+
+ testIngestQuery().setSql(" REPLACE INTO foo OVERWRITE ALL "
+ + "SELECT __time, dim1, m1 "
+ + "FROM foo "
+ + "PARTITIONED BY ALL "
+ + "CLUSTERED BY dim1")
+ .setExpectedDataSource("foo")
+ .setExpectedRowSignature(rowSignature)
+ .setQueryContext(queryContext)
+ .setExpectedDestinationIntervals(Intervals.ONLY_ETERNITY)
+ .setExpectedSegments(
+ ImmutableSet.of(
+ SegmentId.of("foo", Intervals.ETERNITY, "test", 0)
+ )
+ )
+ .setExpectedShardSpec(DimensionRangeShardSpec.class)
+ .setExpectedResultRows(
+ ImmutableList.of(
+ new Object[]{946684800000L, "", 1.0f},
+ new Object[]{978307200000L, "1", 4.0f},
+ new Object[]{946771200000L, "10.1", 2.0f},
+ new Object[]{946857600000L, "2", 3.0f},
+ new Object[]{978480000000L, "abc", 6.0f},
+ new Object[]{978393600000L, "def", 5.0f}
+ )
+ )
+ .setExpectedSegmentGenerationProgressCountersForStageWorker(
+ CounterSnapshotMatcher
+ .with().segmentRowsProcessed(6),
+ 1, 0
+ )
+ .setExpectedLastCompactionState(
+ expectedCompactionState(
+ context,
+ Collections.singletonList("dim1"),
+ DimensionsSpec.builder()
+ .setDimensions(
+ ImmutableList.of(
+ new StringDimensionSchema("dim1"),
+ new LongDimensionSchema("__time"),
+ new FloatDimensionSchema("m1")
+ )
+ )
+ .setUseExplicitSegmentSortOrder(true)
+ .build(),
+ GranularityType.ALL,
+ Intervals.ETERNITY
+ )
+ )
+ .verifyResults();
+ }
+
+ @MethodSource("data")
+ @ParameterizedTest(name = "{index}:with context {0}")
+ public void testReplaceOnFooWithAllClusteredByDimThenTimeExplicitSort(String contextName, Map<String, Object> context)
Review Comment:
## Useless parameter
The parameter 'contextName' is never used.
[Show more details](https://github.com/apache/druid/security/code-scanning/7678)
##########
processing/src/main/java/org/apache/druid/segment/TimeAndDimsPointer.java:
##########
@@ -148,19 +177,23 @@
@Override
public int compareTo(@Nonnull TimeAndDimsPointer rhs)
{
- long timestamp = getTimestamp();
- long rhsTimestamp = rhs.getTimestamp();
- int timestampDiff = Long.compare(timestamp, rhsTimestamp);
- if (timestampDiff != 0) {
- return timestampDiff;
- }
- for (int dimIndex = 0; dimIndex < dimensionSelectors.length; dimIndex++) {
- int dimDiff = dimensionSelectorComparators[dimIndex].compare(
- dimensionSelectors[dimIndex],
- rhs.dimensionSelectors[dimIndex]
- );
- if (dimDiff != 0) {
- return dimDiff;
+ for (int index = 0; index < (dimensionSelectors.length + 1); index++) {
+ if (index == timePosition) {
+ final long timestamp = getTimestamp();
+ final long rhsTimestamp = rhs.getTimestamp();
+ final int timestampDiff = Long.compare(timestamp, rhsTimestamp);
+ if (timestampDiff != 0) {
+ return timestampDiff;
+ }
+ } else {
+ final int dimIndex = index < timePosition ? index : index - 1;
+ final int dimDiff = dimensionSelectorComparators[dimIndex].compare(
+ dimensionSelectors[dimIndex],
Review Comment:
## Array index out of bounds
This array access might be out of bounds, as the index might be equal to the array length.
[Show more details](https://github.com/apache/druid/security/code-scanning/7682)
##########
extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/exec/MSQReplaceTest.java:
##########
@@ -194,6 +196,393 @@
.verifyResults();
}
+ @MethodSource("data")
+ @ParameterizedTest(name = "{index}:with context {0}")
+ public void testReplaceOnFooWithAllClusteredByDim(String contextName, Map<String, Object> context)
Review Comment:
## Useless parameter
The parameter 'contextName' is never used.
[Show more details](https://github.com/apache/druid/security/code-scanning/7680)
##########
extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/exec/MSQReplaceTest.java:
##########
@@ -194,6 +196,393 @@
.verifyResults();
}
+ @MethodSource("data")
+ @ParameterizedTest(name = "{index}:with context {0}")
+ public void testReplaceOnFooWithAllClusteredByDim(String contextName, Map<String, Object> context)
+ {
+ // Tests [CLUSTERED BY dim1] with the default useExplicitSegmentSortOrder (false). In this case,
+ // partitioning uses [dim1] but segment sort uses [__time, dim1].
+ RowSignature rowSignature = RowSignature.builder()
+ .add("__time", ColumnType.LONG)
+ .add("dim1", ColumnType.STRING)
+ .add("m1", ColumnType.FLOAT)
+ .build();
+
+ DataSegment existingDataSegment0 = DataSegment.builder()
+ .interval(Intervals.of("2000-01-01T/2000-01-04T"))
+ .size(50)
+ .version(MSQTestTaskActionClient.VERSION)
+ .dataSource("foo")
+ .build();
+
+ DataSegment existingDataSegment1 = DataSegment.builder()
+ .interval(Intervals.of("2001-01-01T/2001-01-04T"))
+ .size(50)
+ .version(MSQTestTaskActionClient.VERSION)
+ .dataSource("foo")
+ .build();
+
+ Mockito.doCallRealMethod()
+ .doReturn(ImmutableSet.of(existingDataSegment0, existingDataSegment1))
+ .when(testTaskActionClient)
+ .submit(new RetrieveUsedSegmentsAction(
+ EasyMock.eq("foo"),
+ EasyMock.eq(ImmutableList.of(Intervals.ETERNITY))
+ ));
+
+ testIngestQuery().setSql(" REPLACE INTO foo OVERWRITE ALL "
+ + "SELECT __time, dim1, m1 "
+ + "FROM foo "
+ + "PARTITIONED BY ALL "
+ + "CLUSTERED BY dim1")
+ .setExpectedDataSource("foo")
+ .setExpectedRowSignature(rowSignature)
+ .setQueryContext(context)
+ .setExpectedDestinationIntervals(Intervals.ONLY_ETERNITY)
+ .setExpectedSegments(
+ ImmutableSet.of(
+ SegmentId.of("foo", Intervals.ETERNITY, "test", 0)
+ )
+ )
+ .setExpectedShardSpec(DimensionRangeShardSpec.class)
+ .setExpectedResultRows(
+ ImmutableList.of(
+ new Object[]{946684800000L, "", 1.0f},
+ new Object[]{946771200000L, "10.1", 2.0f},
+ new Object[]{946857600000L, "2", 3.0f},
+ new Object[]{978307200000L, "1", 4.0f},
+ new Object[]{978393600000L, "def", 5.0f},
+ new Object[]{978480000000L, "abc", 6.0f}
+ )
+ )
+ .setExpectedSegmentGenerationProgressCountersForStageWorker(
+ CounterSnapshotMatcher
+ .with().segmentRowsProcessed(6),
+ 1, 0
+ )
+ .setExpectedLastCompactionState(
+ expectedCompactionState(
+ context,
+ Collections.singletonList("dim1"),
+ DimensionsSpec.builder()
+ .setDimensions(
+ ImmutableList.of(
+ new StringDimensionSchema("dim1"),
+ new FloatDimensionSchema("m1")
+ )
+ )
+ .setDimensionExclusions(Collections.singletonList("__time"))
+ .build(),
+ GranularityType.ALL,
+ Intervals.ETERNITY
+ )
+ )
+ .verifyResults();
+ }
+
+ @MethodSource("data")
+ @ParameterizedTest(name = "{index}:with context {0}")
+ public void testReplaceOnFooWithAllClusteredByDimExplicitSort(String contextName, Map<String, Object> context)
+ {
+ RowSignature rowSignature = RowSignature.builder()
+ .add("__time", ColumnType.LONG)
+ .add("dim1", ColumnType.STRING)
+ .add("m1", ColumnType.FLOAT)
+ .build();
+
+ DataSegment existingDataSegment0 = DataSegment.builder()
+ .interval(Intervals.of("2000-01-01T/2000-01-04T"))
+ .size(50)
+ .version(MSQTestTaskActionClient.VERSION)
+ .dataSource("foo")
+ .build();
+
+ DataSegment existingDataSegment1 = DataSegment.builder()
+ .interval(Intervals.of("2001-01-01T/2001-01-04T"))
+ .size(50)
+ .version(MSQTestTaskActionClient.VERSION)
+ .dataSource("foo")
+ .build();
+
+ Map<String, Object> queryContext = new HashMap<>(context);
+ queryContext.put(DimensionsSpec.PARAMETER_EXPLICIT_SORT_ORDER, true);
+
+ Mockito.doCallRealMethod()
+ .doReturn(ImmutableSet.of(existingDataSegment0, existingDataSegment1))
+ .when(testTaskActionClient)
+ .submit(new RetrieveUsedSegmentsAction(
+ EasyMock.eq("foo"),
+ EasyMock.eq(ImmutableList.of(Intervals.ETERNITY))
+ ));
+
+ testIngestQuery().setSql(" REPLACE INTO foo OVERWRITE ALL "
+ + "SELECT __time, dim1, m1 "
+ + "FROM foo "
+ + "PARTITIONED BY ALL "
+ + "CLUSTERED BY dim1")
+ .setExpectedDataSource("foo")
+ .setExpectedRowSignature(rowSignature)
+ .setQueryContext(queryContext)
+ .setExpectedDestinationIntervals(Intervals.ONLY_ETERNITY)
+ .setExpectedSegments(
+ ImmutableSet.of(
+ SegmentId.of("foo", Intervals.ETERNITY, "test", 0)
+ )
+ )
+ .setExpectedShardSpec(DimensionRangeShardSpec.class)
+ .setExpectedResultRows(
+ ImmutableList.of(
+ new Object[]{946684800000L, "", 1.0f},
+ new Object[]{978307200000L, "1", 4.0f},
+ new Object[]{946771200000L, "10.1", 2.0f},
+ new Object[]{946857600000L, "2", 3.0f},
+ new Object[]{978480000000L, "abc", 6.0f},
+ new Object[]{978393600000L, "def", 5.0f}
+ )
+ )
+ .setExpectedSegmentGenerationProgressCountersForStageWorker(
+ CounterSnapshotMatcher
+ .with().segmentRowsProcessed(6),
+ 1, 0
+ )
+ .setExpectedLastCompactionState(
+ expectedCompactionState(
+ context,
+ Collections.singletonList("dim1"),
+ DimensionsSpec.builder()
+ .setDimensions(
+ ImmutableList.of(
+ new StringDimensionSchema("dim1"),
+ new LongDimensionSchema("__time"),
+ new FloatDimensionSchema("m1")
+ )
+ )
+ .setUseExplicitSegmentSortOrder(true)
+ .build(),
+ GranularityType.ALL,
+ Intervals.ETERNITY
+ )
+ )
+ .verifyResults();
+ }
+
+ @MethodSource("data")
+ @ParameterizedTest(name = "{index}:with context {0}")
+ public void testReplaceOnFooWithAllClusteredByDimThenTimeExplicitSort(String contextName, Map<String, Object> context)
+ {
+ // Tests that [CLUSTERED BY dim1, __time] and [CLUSTERED BY dim1] are same when useExplicitSegmentSortOrder = true.
+ // (Same expectations as the prior test, testReplaceOnFooWithAllClusteredByDimExplicitSort.)
+ RowSignature rowSignature = RowSignature.builder()
+ .add("__time", ColumnType.LONG)
+ .add("dim1", ColumnType.STRING)
+ .add("m1", ColumnType.FLOAT)
+ .build();
+
+ DataSegment existingDataSegment0 = DataSegment.builder()
+ .interval(Intervals.of("2000-01-01T/2000-01-04T"))
+ .size(50)
+ .version(MSQTestTaskActionClient.VERSION)
+ .dataSource("foo")
+ .build();
+
+ DataSegment existingDataSegment1 = DataSegment.builder()
+ .interval(Intervals.of("2001-01-01T/2001-01-04T"))
+ .size(50)
+ .version(MSQTestTaskActionClient.VERSION)
+ .dataSource("foo")
+ .build();
+
+ Map<String, Object> queryContext = new HashMap<>(context);
+ queryContext.put(DimensionsSpec.PARAMETER_EXPLICIT_SORT_ORDER, true);
+
+ Mockito.doCallRealMethod()
+ .doReturn(ImmutableSet.of(existingDataSegment0, existingDataSegment1))
+ .when(testTaskActionClient)
+ .submit(new RetrieveUsedSegmentsAction(
+ EasyMock.eq("foo"),
+ EasyMock.eq(ImmutableList.of(Intervals.ETERNITY))
+ ));
+
+ testIngestQuery().setSql(" REPLACE INTO foo OVERWRITE ALL "
+ + "SELECT __time, dim1, m1 "
+ + "FROM foo "
+ + "PARTITIONED BY ALL "
+ + "CLUSTERED BY dim1, __time")
+ .setExpectedDataSource("foo")
+ .setExpectedRowSignature(rowSignature)
+ .setQueryContext(queryContext)
+ .setExpectedDestinationIntervals(Intervals.ONLY_ETERNITY)
+ .setExpectedSegments(
+ ImmutableSet.of(
+ SegmentId.of("foo", Intervals.ETERNITY, "test", 0)
+ )
+ )
+ .setExpectedShardSpec(DimensionRangeShardSpec.class)
+ .setExpectedResultRows(
+ ImmutableList.of(
+ new Object[]{946684800000L, "", 1.0f},
+ new Object[]{978307200000L, "1", 4.0f},
+ new Object[]{946771200000L, "10.1", 2.0f},
+ new Object[]{946857600000L, "2", 3.0f},
+ new Object[]{978480000000L, "abc", 6.0f},
+ new Object[]{978393600000L, "def", 5.0f}
+ )
+ )
+ .setExpectedSegmentGenerationProgressCountersForStageWorker(
+ CounterSnapshotMatcher
+ .with().segmentRowsProcessed(6),
+ 1, 0
+ )
+ .setExpectedLastCompactionState(
+ expectedCompactionState(
+ context,
+ Collections.singletonList("dim1"),
+ DimensionsSpec.builder()
+ .setDimensions(
+ ImmutableList.of(
+ new StringDimensionSchema("dim1"),
+ new LongDimensionSchema("__time"),
+ new FloatDimensionSchema("m1")
+ )
+ )
+ .setUseExplicitSegmentSortOrder(true)
+ .build(),
+ GranularityType.ALL,
+ Intervals.ETERNITY
+ )
+ )
+ .verifyResults();
+ }
+
+ @MethodSource("data")
+ @ParameterizedTest(name = "{index}:with context {0}")
+ public void testReplaceOnFooWithAllClusteredByDimThenTimeError(String contextName, Map<String, Object> context)
+ {
+ // Tests that [CLUSTERED BY dim1, __time] is an error when useExplicitSegmentSortOrder = false (the default).
+ testIngestQuery().setSql(" REPLACE INTO foo OVERWRITE ALL "
+ + "SELECT __time, dim1, m1 "
+ + "FROM foo "
+ + "PARTITIONED BY ALL "
+ + "CLUSTERED BY dim1, __time")
+ .setExpectedDataSource("foo")
+ .setQueryContext(context)
+ .setExpectedValidationErrorMatcher(invalidSqlContains(
+ "Sort order (CLUSTERED BY) cannot include[__time] in position[1] unless context "
+ + "parameter[useExplicitSegmentSortOrder] is set to[true]."
+ ))
+ .verifyPlanningErrors();
+ }
+
+ @MethodSource("data")
+ @ParameterizedTest(name = "{index}:with context {0}")
+ public void testReplaceOnFooWithAllClusteredByDimThenTimeError2(String contextName, Map<String, Object> context)
Review Comment:
## Useless parameter
The parameter 'contextName' is never used.
[Show more details](https://github.com/apache/druid/security/code-scanning/7676)
##########
extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/exec/MSQReplaceTest.java:
##########
@@ -194,6 +196,393 @@
.verifyResults();
}
+ @MethodSource("data")
+ @ParameterizedTest(name = "{index}:with context {0}")
+ public void testReplaceOnFooWithAllClusteredByDim(String contextName, Map<String, Object> context)
+ {
+ // Tests [CLUSTERED BY dim1] with the default useExplicitSegmentSortOrder (false). In this case,
+ // partitioning uses [dim1] but segment sort uses [__time, dim1].
+ RowSignature rowSignature = RowSignature.builder()
+ .add("__time", ColumnType.LONG)
+ .add("dim1", ColumnType.STRING)
+ .add("m1", ColumnType.FLOAT)
+ .build();
+
+ DataSegment existingDataSegment0 = DataSegment.builder()
+ .interval(Intervals.of("2000-01-01T/2000-01-04T"))
+ .size(50)
+ .version(MSQTestTaskActionClient.VERSION)
+ .dataSource("foo")
+ .build();
+
+ DataSegment existingDataSegment1 = DataSegment.builder()
+ .interval(Intervals.of("2001-01-01T/2001-01-04T"))
+ .size(50)
+ .version(MSQTestTaskActionClient.VERSION)
+ .dataSource("foo")
+ .build();
+
+ Mockito.doCallRealMethod()
+ .doReturn(ImmutableSet.of(existingDataSegment0, existingDataSegment1))
+ .when(testTaskActionClient)
+ .submit(new RetrieveUsedSegmentsAction(
+ EasyMock.eq("foo"),
+ EasyMock.eq(ImmutableList.of(Intervals.ETERNITY))
+ ));
+
+ testIngestQuery().setSql(" REPLACE INTO foo OVERWRITE ALL "
+ + "SELECT __time, dim1, m1 "
+ + "FROM foo "
+ + "PARTITIONED BY ALL "
+ + "CLUSTERED BY dim1")
+ .setExpectedDataSource("foo")
+ .setExpectedRowSignature(rowSignature)
+ .setQueryContext(context)
+ .setExpectedDestinationIntervals(Intervals.ONLY_ETERNITY)
+ .setExpectedSegments(
+ ImmutableSet.of(
+ SegmentId.of("foo", Intervals.ETERNITY, "test", 0)
+ )
+ )
+ .setExpectedShardSpec(DimensionRangeShardSpec.class)
+ .setExpectedResultRows(
+ ImmutableList.of(
+ new Object[]{946684800000L, "", 1.0f},
+ new Object[]{946771200000L, "10.1", 2.0f},
+ new Object[]{946857600000L, "2", 3.0f},
+ new Object[]{978307200000L, "1", 4.0f},
+ new Object[]{978393600000L, "def", 5.0f},
+ new Object[]{978480000000L, "abc", 6.0f}
+ )
+ )
+ .setExpectedSegmentGenerationProgressCountersForStageWorker(
+ CounterSnapshotMatcher
+ .with().segmentRowsProcessed(6),
+ 1, 0
+ )
+ .setExpectedLastCompactionState(
+ expectedCompactionState(
+ context,
+ Collections.singletonList("dim1"),
+ DimensionsSpec.builder()
+ .setDimensions(
+ ImmutableList.of(
+ new StringDimensionSchema("dim1"),
+ new FloatDimensionSchema("m1")
+ )
+ )
+ .setDimensionExclusions(Collections.singletonList("__time"))
+ .build(),
+ GranularityType.ALL,
+ Intervals.ETERNITY
+ )
+ )
+ .verifyResults();
+ }
+
+ @MethodSource("data")
+ @ParameterizedTest(name = "{index}:with context {0}")
+ public void testReplaceOnFooWithAllClusteredByDimExplicitSort(String contextName, Map<String, Object> context)
+ {
+ RowSignature rowSignature = RowSignature.builder()
+ .add("__time", ColumnType.LONG)
+ .add("dim1", ColumnType.STRING)
+ .add("m1", ColumnType.FLOAT)
+ .build();
+
+ DataSegment existingDataSegment0 = DataSegment.builder()
+ .interval(Intervals.of("2000-01-01T/2000-01-04T"))
+ .size(50)
+ .version(MSQTestTaskActionClient.VERSION)
+ .dataSource("foo")
+ .build();
+
+ DataSegment existingDataSegment1 = DataSegment.builder()
+ .interval(Intervals.of("2001-01-01T/2001-01-04T"))
+ .size(50)
+ .version(MSQTestTaskActionClient.VERSION)
+ .dataSource("foo")
+ .build();
+
+ Map<String, Object> queryContext = new HashMap<>(context);
+ queryContext.put(DimensionsSpec.PARAMETER_EXPLICIT_SORT_ORDER, true);
+
+ Mockito.doCallRealMethod()
+ .doReturn(ImmutableSet.of(existingDataSegment0, existingDataSegment1))
+ .when(testTaskActionClient)
+ .submit(new RetrieveUsedSegmentsAction(
+ EasyMock.eq("foo"),
+ EasyMock.eq(ImmutableList.of(Intervals.ETERNITY))
+ ));
+
+ testIngestQuery().setSql(" REPLACE INTO foo OVERWRITE ALL "
+ + "SELECT __time, dim1, m1 "
+ + "FROM foo "
+ + "PARTITIONED BY ALL "
+ + "CLUSTERED BY dim1")
+ .setExpectedDataSource("foo")
+ .setExpectedRowSignature(rowSignature)
+ .setQueryContext(queryContext)
+ .setExpectedDestinationIntervals(Intervals.ONLY_ETERNITY)
+ .setExpectedSegments(
+ ImmutableSet.of(
+ SegmentId.of("foo", Intervals.ETERNITY, "test", 0)
+ )
+ )
+ .setExpectedShardSpec(DimensionRangeShardSpec.class)
+ .setExpectedResultRows(
+ ImmutableList.of(
+ new Object[]{946684800000L, "", 1.0f},
+ new Object[]{978307200000L, "1", 4.0f},
+ new Object[]{946771200000L, "10.1", 2.0f},
+ new Object[]{946857600000L, "2", 3.0f},
+ new Object[]{978480000000L, "abc", 6.0f},
+ new Object[]{978393600000L, "def", 5.0f}
+ )
+ )
+ .setExpectedSegmentGenerationProgressCountersForStageWorker(
+ CounterSnapshotMatcher
+ .with().segmentRowsProcessed(6),
+ 1, 0
+ )
+ .setExpectedLastCompactionState(
+ expectedCompactionState(
+ context,
+ Collections.singletonList("dim1"),
+ DimensionsSpec.builder()
+ .setDimensions(
+ ImmutableList.of(
+ new StringDimensionSchema("dim1"),
+ new LongDimensionSchema("__time"),
+ new FloatDimensionSchema("m1")
+ )
+ )
+ .setUseExplicitSegmentSortOrder(true)
+ .build(),
+ GranularityType.ALL,
+ Intervals.ETERNITY
+ )
+ )
+ .verifyResults();
+ }
+
+ @MethodSource("data")
+ @ParameterizedTest(name = "{index}:with context {0}")
+ public void testReplaceOnFooWithAllClusteredByDimThenTimeExplicitSort(String contextName, Map<String, Object> context)
+ {
+ // Tests that [CLUSTERED BY dim1, __time] and [CLUSTERED BY dim1] are same when useExplicitSegmentSortOrder = true.
+ // (Same expectations as the prior test, testReplaceOnFooWithAllClusteredByDimExplicitSort.)
+ RowSignature rowSignature = RowSignature.builder()
+ .add("__time", ColumnType.LONG)
+ .add("dim1", ColumnType.STRING)
+ .add("m1", ColumnType.FLOAT)
+ .build();
+
+ DataSegment existingDataSegment0 = DataSegment.builder()
+ .interval(Intervals.of("2000-01-01T/2000-01-04T"))
+ .size(50)
+ .version(MSQTestTaskActionClient.VERSION)
+ .dataSource("foo")
+ .build();
+
+ DataSegment existingDataSegment1 = DataSegment.builder()
+ .interval(Intervals.of("2001-01-01T/2001-01-04T"))
+ .size(50)
+ .version(MSQTestTaskActionClient.VERSION)
+ .dataSource("foo")
+ .build();
+
+ Map<String, Object> queryContext = new HashMap<>(context);
+ queryContext.put(DimensionsSpec.PARAMETER_EXPLICIT_SORT_ORDER, true);
+
+ Mockito.doCallRealMethod()
+ .doReturn(ImmutableSet.of(existingDataSegment0, existingDataSegment1))
+ .when(testTaskActionClient)
+ .submit(new RetrieveUsedSegmentsAction(
+ EasyMock.eq("foo"),
+ EasyMock.eq(ImmutableList.of(Intervals.ETERNITY))
+ ));
+
+ testIngestQuery().setSql(" REPLACE INTO foo OVERWRITE ALL "
+ + "SELECT __time, dim1, m1 "
+ + "FROM foo "
+ + "PARTITIONED BY ALL "
+ + "CLUSTERED BY dim1, __time")
+ .setExpectedDataSource("foo")
+ .setExpectedRowSignature(rowSignature)
+ .setQueryContext(queryContext)
+ .setExpectedDestinationIntervals(Intervals.ONLY_ETERNITY)
+ .setExpectedSegments(
+ ImmutableSet.of(
+ SegmentId.of("foo", Intervals.ETERNITY, "test", 0)
+ )
+ )
+ .setExpectedShardSpec(DimensionRangeShardSpec.class)
+ .setExpectedResultRows(
+ ImmutableList.of(
+ new Object[]{946684800000L, "", 1.0f},
+ new Object[]{978307200000L, "1", 4.0f},
+ new Object[]{946771200000L, "10.1", 2.0f},
+ new Object[]{946857600000L, "2", 3.0f},
+ new Object[]{978480000000L, "abc", 6.0f},
+ new Object[]{978393600000L, "def", 5.0f}
+ )
+ )
+ .setExpectedSegmentGenerationProgressCountersForStageWorker(
+ CounterSnapshotMatcher
+ .with().segmentRowsProcessed(6),
+ 1, 0
+ )
+ .setExpectedLastCompactionState(
+ expectedCompactionState(
+ context,
+ Collections.singletonList("dim1"),
+ DimensionsSpec.builder()
+ .setDimensions(
+ ImmutableList.of(
+ new StringDimensionSchema("dim1"),
+ new LongDimensionSchema("__time"),
+ new FloatDimensionSchema("m1")
+ )
+ )
+ .setUseExplicitSegmentSortOrder(true)
+ .build(),
+ GranularityType.ALL,
+ Intervals.ETERNITY
+ )
+ )
+ .verifyResults();
+ }
+
+ @MethodSource("data")
+ @ParameterizedTest(name = "{index}:with context {0}")
+ public void testReplaceOnFooWithAllClusteredByDimThenTimeError(String contextName, Map<String, Object> context)
Review Comment:
## Useless parameter
The parameter 'contextName' is never used.
[Show more details](https://github.com/apache/druid/security/code-scanning/7677)
##########
extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/exec/MSQReplaceTest.java:
##########
@@ -194,6 +196,393 @@
.verifyResults();
}
+ @MethodSource("data")
+ @ParameterizedTest(name = "{index}:with context {0}")
+ public void testReplaceOnFooWithAllClusteredByDim(String contextName, Map<String, Object> context)
+ {
+ // Tests [CLUSTERED BY dim1] with the default useExplicitSegmentSortOrder (false). In this case,
+ // partitioning uses [dim1] but segment sort uses [__time, dim1].
+ RowSignature rowSignature = RowSignature.builder()
+ .add("__time", ColumnType.LONG)
+ .add("dim1", ColumnType.STRING)
+ .add("m1", ColumnType.FLOAT)
+ .build();
+
+ DataSegment existingDataSegment0 = DataSegment.builder()
+ .interval(Intervals.of("2000-01-01T/2000-01-04T"))
+ .size(50)
+ .version(MSQTestTaskActionClient.VERSION)
+ .dataSource("foo")
+ .build();
+
+ DataSegment existingDataSegment1 = DataSegment.builder()
+ .interval(Intervals.of("2001-01-01T/2001-01-04T"))
+ .size(50)
+ .version(MSQTestTaskActionClient.VERSION)
+ .dataSource("foo")
+ .build();
+
+ Mockito.doCallRealMethod()
+ .doReturn(ImmutableSet.of(existingDataSegment0, existingDataSegment1))
+ .when(testTaskActionClient)
+ .submit(new RetrieveUsedSegmentsAction(
+ EasyMock.eq("foo"),
+ EasyMock.eq(ImmutableList.of(Intervals.ETERNITY))
+ ));
+
+ testIngestQuery().setSql(" REPLACE INTO foo OVERWRITE ALL "
+ + "SELECT __time, dim1, m1 "
+ + "FROM foo "
+ + "PARTITIONED BY ALL "
+ + "CLUSTERED BY dim1")
+ .setExpectedDataSource("foo")
+ .setExpectedRowSignature(rowSignature)
+ .setQueryContext(context)
+ .setExpectedDestinationIntervals(Intervals.ONLY_ETERNITY)
+ .setExpectedSegments(
+ ImmutableSet.of(
+ SegmentId.of("foo", Intervals.ETERNITY, "test", 0)
+ )
+ )
+ .setExpectedShardSpec(DimensionRangeShardSpec.class)
+ .setExpectedResultRows(
+ ImmutableList.of(
+ new Object[]{946684800000L, "", 1.0f},
+ new Object[]{946771200000L, "10.1", 2.0f},
+ new Object[]{946857600000L, "2", 3.0f},
+ new Object[]{978307200000L, "1", 4.0f},
+ new Object[]{978393600000L, "def", 5.0f},
+ new Object[]{978480000000L, "abc", 6.0f}
+ )
+ )
+ .setExpectedSegmentGenerationProgressCountersForStageWorker(
+ CounterSnapshotMatcher
+ .with().segmentRowsProcessed(6),
+ 1, 0
+ )
+ .setExpectedLastCompactionState(
+ expectedCompactionState(
+ context,
+ Collections.singletonList("dim1"),
+ DimensionsSpec.builder()
+ .setDimensions(
+ ImmutableList.of(
+ new StringDimensionSchema("dim1"),
+ new FloatDimensionSchema("m1")
+ )
+ )
+ .setDimensionExclusions(Collections.singletonList("__time"))
+ .build(),
+ GranularityType.ALL,
+ Intervals.ETERNITY
+ )
+ )
+ .verifyResults();
+ }
+
+ @MethodSource("data")
+ @ParameterizedTest(name = "{index}:with context {0}")
+ public void testReplaceOnFooWithAllClusteredByDimExplicitSort(String contextName, Map<String, Object> context)
+ {
+ RowSignature rowSignature = RowSignature.builder()
+ .add("__time", ColumnType.LONG)
+ .add("dim1", ColumnType.STRING)
+ .add("m1", ColumnType.FLOAT)
+ .build();
+
+ DataSegment existingDataSegment0 = DataSegment.builder()
+ .interval(Intervals.of("2000-01-01T/2000-01-04T"))
+ .size(50)
+ .version(MSQTestTaskActionClient.VERSION)
+ .dataSource("foo")
+ .build();
+
+ DataSegment existingDataSegment1 = DataSegment.builder()
+ .interval(Intervals.of("2001-01-01T/2001-01-04T"))
+ .size(50)
+ .version(MSQTestTaskActionClient.VERSION)
+ .dataSource("foo")
+ .build();
+
+ Map<String, Object> queryContext = new HashMap<>(context);
+ queryContext.put(DimensionsSpec.PARAMETER_EXPLICIT_SORT_ORDER, true);
+
+ Mockito.doCallRealMethod()
+ .doReturn(ImmutableSet.of(existingDataSegment0, existingDataSegment1))
+ .when(testTaskActionClient)
+ .submit(new RetrieveUsedSegmentsAction(
+ EasyMock.eq("foo"),
+ EasyMock.eq(ImmutableList.of(Intervals.ETERNITY))
+ ));
+
+ testIngestQuery().setSql(" REPLACE INTO foo OVERWRITE ALL "
+ + "SELECT __time, dim1, m1 "
+ + "FROM foo "
+ + "PARTITIONED BY ALL "
+ + "CLUSTERED BY dim1")
+ .setExpectedDataSource("foo")
+ .setExpectedRowSignature(rowSignature)
+ .setQueryContext(queryContext)
+ .setExpectedDestinationIntervals(Intervals.ONLY_ETERNITY)
+ .setExpectedSegments(
+ ImmutableSet.of(
+ SegmentId.of("foo", Intervals.ETERNITY, "test", 0)
+ )
+ )
+ .setExpectedShardSpec(DimensionRangeShardSpec.class)
+ .setExpectedResultRows(
+ ImmutableList.of(
+ new Object[]{946684800000L, "", 1.0f},
+ new Object[]{978307200000L, "1", 4.0f},
+ new Object[]{946771200000L, "10.1", 2.0f},
+ new Object[]{946857600000L, "2", 3.0f},
+ new Object[]{978480000000L, "abc", 6.0f},
+ new Object[]{978393600000L, "def", 5.0f}
+ )
+ )
+ .setExpectedSegmentGenerationProgressCountersForStageWorker(
+ CounterSnapshotMatcher
+ .with().segmentRowsProcessed(6),
+ 1, 0
+ )
+ .setExpectedLastCompactionState(
+ expectedCompactionState(
+ context,
+ Collections.singletonList("dim1"),
+ DimensionsSpec.builder()
+ .setDimensions(
+ ImmutableList.of(
+ new StringDimensionSchema("dim1"),
+ new LongDimensionSchema("__time"),
+ new FloatDimensionSchema("m1")
+ )
+ )
+ .setUseExplicitSegmentSortOrder(true)
+ .build(),
+ GranularityType.ALL,
+ Intervals.ETERNITY
+ )
+ )
+ .verifyResults();
+ }
+
+ @MethodSource("data")
+ @ParameterizedTest(name = "{index}:with context {0}")
+ public void testReplaceOnFooWithAllClusteredByDimThenTimeExplicitSort(String contextName, Map<String, Object> context)
+ {
+ // Tests that [CLUSTERED BY dim1, __time] and [CLUSTERED BY dim1] are same when useExplicitSegmentSortOrder = true.
+ // (Same expectations as the prior test, testReplaceOnFooWithAllClusteredByDimExplicitSort.)
+ RowSignature rowSignature = RowSignature.builder()
+ .add("__time", ColumnType.LONG)
+ .add("dim1", ColumnType.STRING)
+ .add("m1", ColumnType.FLOAT)
+ .build();
+
+ DataSegment existingDataSegment0 = DataSegment.builder()
+ .interval(Intervals.of("2000-01-01T/2000-01-04T"))
+ .size(50)
+ .version(MSQTestTaskActionClient.VERSION)
+ .dataSource("foo")
+ .build();
+
+ DataSegment existingDataSegment1 = DataSegment.builder()
+ .interval(Intervals.of("2001-01-01T/2001-01-04T"))
+ .size(50)
+ .version(MSQTestTaskActionClient.VERSION)
+ .dataSource("foo")
+ .build();
+
+ Map<String, Object> queryContext = new HashMap<>(context);
+ queryContext.put(DimensionsSpec.PARAMETER_EXPLICIT_SORT_ORDER, true);
+
+ Mockito.doCallRealMethod()
+ .doReturn(ImmutableSet.of(existingDataSegment0, existingDataSegment1))
+ .when(testTaskActionClient)
+ .submit(new RetrieveUsedSegmentsAction(
+ EasyMock.eq("foo"),
+ EasyMock.eq(ImmutableList.of(Intervals.ETERNITY))
+ ));
+
+ testIngestQuery().setSql(" REPLACE INTO foo OVERWRITE ALL "
+ + "SELECT __time, dim1, m1 "
+ + "FROM foo "
+ + "PARTITIONED BY ALL "
+ + "CLUSTERED BY dim1, __time")
+ .setExpectedDataSource("foo")
+ .setExpectedRowSignature(rowSignature)
+ .setQueryContext(queryContext)
+ .setExpectedDestinationIntervals(Intervals.ONLY_ETERNITY)
+ .setExpectedSegments(
+ ImmutableSet.of(
+ SegmentId.of("foo", Intervals.ETERNITY, "test", 0)
+ )
+ )
+ .setExpectedShardSpec(DimensionRangeShardSpec.class)
+ .setExpectedResultRows(
+ ImmutableList.of(
+ new Object[]{946684800000L, "", 1.0f},
+ new Object[]{978307200000L, "1", 4.0f},
+ new Object[]{946771200000L, "10.1", 2.0f},
+ new Object[]{946857600000L, "2", 3.0f},
+ new Object[]{978480000000L, "abc", 6.0f},
+ new Object[]{978393600000L, "def", 5.0f}
+ )
+ )
+ .setExpectedSegmentGenerationProgressCountersForStageWorker(
+ CounterSnapshotMatcher
+ .with().segmentRowsProcessed(6),
+ 1, 0
+ )
+ .setExpectedLastCompactionState(
+ expectedCompactionState(
+ context,
+ Collections.singletonList("dim1"),
+ DimensionsSpec.builder()
+ .setDimensions(
+ ImmutableList.of(
+ new StringDimensionSchema("dim1"),
+ new LongDimensionSchema("__time"),
+ new FloatDimensionSchema("m1")
+ )
+ )
+ .setUseExplicitSegmentSortOrder(true)
+ .build(),
+ GranularityType.ALL,
+ Intervals.ETERNITY
+ )
+ )
+ .verifyResults();
+ }
+
+ @MethodSource("data")
+ @ParameterizedTest(name = "{index}:with context {0}")
+ public void testReplaceOnFooWithAllClusteredByDimThenTimeError(String contextName, Map<String, Object> context)
+ {
+ // Tests that [CLUSTERED BY dim1, __time] is an error when useExplicitSegmentSortOrder = false (the default).
+ testIngestQuery().setSql(" REPLACE INTO foo OVERWRITE ALL "
+ + "SELECT __time, dim1, m1 "
+ + "FROM foo "
+ + "PARTITIONED BY ALL "
+ + "CLUSTERED BY dim1, __time")
+ .setExpectedDataSource("foo")
+ .setQueryContext(context)
+ .setExpectedValidationErrorMatcher(invalidSqlContains(
+ "Sort order (CLUSTERED BY) cannot include[__time] in position[1] unless context "
+ + "parameter[useExplicitSegmentSortOrder] is set to[true]."
+ ))
+ .verifyPlanningErrors();
+ }
+
+ @MethodSource("data")
+ @ParameterizedTest(name = "{index}:with context {0}")
+ public void testReplaceOnFooWithAllClusteredByDimThenTimeError2(String contextName, Map<String, Object> context)
+ {
+ // Tests that setting segmentSortOrder = [dim1, __time] is an error when
+ // useExplicitSegmentSortOrder = false (the default).
+ Map<String, Object> queryContext = new HashMap<>(context);
+ queryContext.put(MultiStageQueryContext.CTX_SORT_ORDER, "dim1, __time");
+
+ testIngestQuery().setSql(" REPLACE INTO foo OVERWRITE ALL "
+ + "SELECT __time, dim1, m1 "
+ + "FROM foo "
+ + "PARTITIONED BY ALL "
+ + "CLUSTERED BY dim1")
+ .setExpectedDataSource("foo")
+ .setQueryContext(queryContext)
+ .setExpectedValidationErrorMatcher(invalidSqlContains(
+ "Context parameter[segmentSortOrder] must start with[__time] unless context "
+ + "parameter[useExplicitSegmentSortOrder] is set to[true]."
+ ))
+ .verifyPlanningErrors();
+ }
+
+ @MethodSource("data")
+ @ParameterizedTest(name = "{index}:with context {0}")
+ public void testReplaceOnFooWithAllClusteredByTimeThenDimExplicitSort(String contextName, Map<String, Object> context)
Review Comment:
## Useless parameter
The parameter 'contextName' is never used.
[Show more details](https://github.com/apache/druid/security/code-scanning/7675)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on PR #16849:
URL: https://github.com/apache/druid/pull/16849#issuecomment-2305641753
Pushed a change to make the parameter `forceSegmentSortByTime` (default `true`) rather than `useExplicitSegmentSortOrder` (default `false`).
I think this makes more sense, because it refers to "time" explicitly (this is really about time) and it also would work better for a migration to changing the default. If the default changes to not force sort by time, then users wanting backwards compatibility could set `forceSegmentSortByTime: true`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #16849:
URL: https://github.com/apache/druid/pull/16849#discussion_r1708055566
##########
processing/src/main/java/org/apache/druid/segment/UnnestStorageAdapter.java:
##########
@@ -256,6 +256,23 @@ public Metadata getMetadata()
return baseAdapter.getMetadata();
}
+ @Override
+ public List<String> getSortOrder()
+ {
+ final List<String> baseSortOrder = baseAdapter.getSortOrder();
+
+ // Sorted the same way as the base segment, unless the unnested column shadows one of the base columns.
Review Comment:
I figure the common case is that the unnested column will not be first in the sort order. So we'll return whatever prefix of the order is there up to the unnested column.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "vogievetsky (via GitHub)" <gi...@apache.org>.
vogievetsky commented on code in PR #16849:
URL: https://github.com/apache/druid/pull/16849#discussion_r1727956216
##########
docs/ingestion/ingestion-spec.md:
##########
@@ -232,7 +232,7 @@ A `dimensionsSpec` can have the following components:
| `spatialDimensions` | An array of [spatial dimensions](../querying/geo.md). | `[]` |
| `includeAllDimensions` | Note that this field only applies to string-based schema discovery where Druid ingests dimensions it discovers as strings. This is different from schema auto-discovery where Druid infers the type for data. You can set `includeAllDimensions` to true to ingest both explicit dimensions in the `dimensions` field and other dimensions that the ingestion task discovers from input data. In this case, the explicit dimensions will appear first in the order that you specify them, and the dimensions dynamically discovered will come after. This flag can be useful especially with auto schema discovery using [`flattenSpec`](./data-formats.md#flattenspec). If this is not set and the `dimensions` field is not empty, Druid will ingest only explicit dimensions. If this is not set and the `dimensions` field is empty, all discovered dimensions will be ingested. | false |
| `useSchemaDiscovery` | Configure Druid to use schema auto-discovery to discover some or all of the dimensions and types for your data. For any dimensions that aren't a uniform type, Druid ingests them as JSON. You can use this for native batch or streaming ingestion. | false |
-
+| `forceSegmentSortByTime` | When set to true (the default), segments created by the ingestion job are sorted by `{__time, dimensions[0], dimensions[1], ...}`. When set to false, segments created by the ingestion job are sorted by `{dimensions[0], dimensions[1], ...}`. To include `__time` in the sort order when this parameter is set to `false`, you must include a dimension named `__time` with type `long` explicitly in the `dimensions` list.<br /><br />Setting this to `false` is an experimental feature; see [Sorting](partitioning.md#sorting) for details. | `false` |
Review Comment:
You need to update the default (3rd column) to `true`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #16849:
URL: https://github.com/apache/druid/pull/16849#discussion_r1708113077
##########
processing/src/main/java/org/apache/druid/segment/RowBasedStorageAdapter.java:
##########
@@ -167,6 +168,15 @@ public Metadata getMetadata()
throw new UnsupportedOperationException("Cannot retrieve metadata");
}
+ @Override
+ public List<String> getSortOrder()
+ {
+ // It's possibly incorrect in some cases for sort order to be SORTED_BY_TIME_ONLY here, but for historical reasons,
+ // we're keeping this in place for now. The handling of "interval" in "makeCursors", which has been in place for
+ // some time, suggests we think the data is always sorted by time.
Review Comment:
Yes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #16849:
URL: https://github.com/apache/druid/pull/16849#discussion_r1708112437
##########
processing/src/main/java/org/apache/druid/segment/Metadata.java:
##########
@@ -233,6 +260,39 @@ public String toString()
", timestampSpec=" + timestampSpec +
", queryGranularity=" + queryGranularity +
", rollup=" + rollup +
+ ", sortOrder=" + sortOrder +
'}';
}
+
+ @Nullable
+ private static List<String> mergeSortOrders(List<List<String>> sortOrdersToMerge)
+ {
+ if (sortOrdersToMerge.isEmpty()) {
+ return null;
+ }
+
+ if (sortOrdersToMerge.stream().anyMatch(Objects::isNull)) {
+ return null;
+ }
+
+ final List<String> mergedSortOrder = new ArrayList<>();
+
+ while (true) {
+ String column = null;
+
+ for (final List<String> sortOrder : sortOrdersToMerge) {
+ if (mergedSortOrder.size() >= sortOrder.size()) {
Review Comment:
I added some comments and also fixed a problem: `null` is now treated as `[__time]` (which makes sense, since that was the only possible sort order prior to the `sortOrder` field being added). Due to this change the method is no longer `@Nullable`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #16849:
URL: https://github.com/apache/druid/pull/16849#discussion_r1708084545
##########
processing/src/main/java/org/apache/druid/segment/StorageAdapter.java:
##########
@@ -129,6 +141,18 @@ default RowSignature getRowSignature()
@Nullable
Metadata getMetadata();
+ /**
+ * Returns column names that this adapter's data is sorted by. Cursors returned by this adapter return rows in
+ * this ordering, using the natural comparator for the type of the column as returned by
+ * {@link #getColumnCapabilities(String)}. Includes {@link ColumnHolder#TIME_COLUMN_NAME} if appropriate.
+ */
+ List<String> getSortOrder();
+
+ default boolean isTimeOrdered()
+ {
+ return ColumnHolder.TIME_COLUMN_NAME.equals(Iterables.getFirst(getSortOrder(), null));
+ }
Review Comment:
This sounds like a good idea, although in the interests of minimizing conflicts, it'd probably be good to do this after #16533 is merged (since it moves `OrderBy` around).
##########
processing/src/main/java/org/apache/druid/segment/Metadata.java:
##########
@@ -100,6 +112,14 @@ public Boolean isRollup()
return rollup;
}
+ @Nullable
+ @JsonProperty
+ @JsonInclude(JsonInclude.Include.NON_NULL)
+ public List<String> getSortOrder()
+ {
+ return sortOrder;
+ }
+
Review Comment:
see https://github.com/apache/druid/pull/16849#discussion_r1708084545
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #16849:
URL: https://github.com/apache/druid/pull/16849#discussion_r1708115849
##########
processing/src/main/java/org/apache/druid/segment/incremental/IncrementalIndex.java:
##########
@@ -334,6 +349,17 @@ protected IncrementalIndex(
if (!spatialDimensions.isEmpty()) {
this.rowTransformers.add(new SpatialDimensionRowTransformer(spatialDimensions));
}
+
+ // Set metadata last, so dimensionOrder is populated
+ final List<String> dimensionOrder = getDimensionOrder();
+ this.metadata = new Metadata(
+ null,
+ getCombiningAggregators(metrics),
+ incrementalIndexSchema.getTimestampSpec(),
+ this.gran,
+ this.rollup,
+ ColumnHolder.TIME_COLUMN_NAME.equals(Iterables.getFirst(dimensionOrder, null)) ? null : dimensionOrder
Review Comment:
Hmm I don't totally remember why this is conditional. It doesn't really make sense to be conditional IMO. I updated it to always write out.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "maytasm (via GitHub)" <gi...@apache.org>.
maytasm commented on PR #16849:
URL: https://github.com/apache/druid/pull/16849#issuecomment-2623279128
@clintropolis
Thanks for the response.
- Im a little confused on when you said that getMaxTime() gives the end interval of the segment. I believe that this query (dataSourceMetadata) was previously returning something like `2025-01-21T08:23:32.096Z` for our non-realtime datasource. The code for getMaxTime() in QueryableIndexStorageAdapter looks something like:
```
private void populateMinMaxTime()
{
// Compute and cache minTime, maxTime.
final ColumnHolder columnHolder = index.getColumnHolder(ColumnHolder.TIME_COLUMN_NAME);
try (NumericColumn column = (NumericColumn) columnHolder.getColumn()) {
this.minTime = DateTimes.utc(column.getLongSingleValueRow(0));
this.maxTime = DateTimes.utc(column.getLongSingleValueRow(column.length() - 1));
}
}
```
which does seems to return the actual time (min and max) of the rows/data (not the end interval of the segment)... unless I am missing something.
- Given that the intention of the new MaxIngestedEventTimeInspector is for only realtime data, is it still fair to say that the dataSourceMetadata query should still work for non-realtime datasource (as it was previously working for non-realtime datasources)?
- It is returning min datetime as currMaxIngestedEventTime is null at: https://github.com/apache/druid/blob/master/processing/src/main/java/org/apache/druid/query/datasourcemetadata/DataSourceMetadataQuery.java#L109
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on code in PR #16849:
URL: https://github.com/apache/druid/pull/16849#discussion_r1722769440
##########
processing/src/main/java/org/apache/druid/segment/join/table/IndexedTableJoinMatcher.java:
##########
@@ -133,7 +127,7 @@
);
}
- ColumnSelectorFactory selectorFactory = table.makeColumnSelectorFactory(joinableOffset, descending, closer);
+ ColumnSelectorFactory selectorFactory = table.makeColumnSelectorFactory(joinableOffset, closer);
Review Comment:
## Possible confusion of local and field
Potentially confusing name: [IndexedTableJoinMatcher](1) also refers to field [selectorFactory](2) (as this.selectorFactory).
[Show more details](https://github.com/apache/druid/security/code-scanning/7734)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #16849:
URL: https://github.com/apache/druid/pull/16849#discussion_r1708051126
##########
processing/src/main/java/org/apache/druid/segment/incremental/OnheapIncrementalIndex.java:
##########
@@ -777,11 +790,18 @@ public Iterator<IncrementalIndexRow> iterator(boolean descending)
@Override
public Iterable<IncrementalIndexRow> timeRangeIterable(boolean descending, long timeStart, long timeEnd)
{
- IncrementalIndexRow start = new IncrementalIndexRow(timeStart, new Object[]{}, dimensionDescsList);
- IncrementalIndexRow end = new IncrementalIndexRow(timeEnd, new Object[]{}, dimensionDescsList);
- ConcurrentNavigableMap<IncrementalIndexRow, IncrementalIndexRow> subMap = facts.subMap(start, end);
- ConcurrentMap<IncrementalIndexRow, IncrementalIndexRow> rangeMap = descending ? subMap.descendingMap() : subMap;
- return rangeMap.keySet();
+ if (timeOrdered) {
+ IncrementalIndexRow start = new IncrementalIndexRow(timeStart, new Object[]{}, dimensionDescsList);
+ IncrementalIndexRow end = new IncrementalIndexRow(timeEnd, new Object[]{}, dimensionDescsList);
+ ConcurrentNavigableMap<IncrementalIndexRow, IncrementalIndexRow> subMap = facts.subMap(start, end);
+ ConcurrentMap<IncrementalIndexRow, IncrementalIndexRow> rangeMap = descending ? subMap.descendingMap() : subMap;
+ return rangeMap.keySet();
+ } else {
+ return Iterables.filter(
Review Comment:
I think it's also used for `intervals` filtering even for non-granular cursors. It seems like it'd be useful for that at least.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #16849:
URL: https://github.com/apache/druid/pull/16849#discussion_r1708115849
##########
processing/src/main/java/org/apache/druid/segment/incremental/IncrementalIndex.java:
##########
@@ -334,6 +349,17 @@ protected IncrementalIndex(
if (!spatialDimensions.isEmpty()) {
this.rowTransformers.add(new SpatialDimensionRowTransformer(spatialDimensions));
}
+
+ // Set metadata last, so dimensionOrder is populated
+ final List<String> dimensionOrder = getDimensionOrder();
+ this.metadata = new Metadata(
+ null,
+ getCombiningAggregators(metrics),
+ incrementalIndexSchema.getTimestampSpec(),
+ this.gran,
+ this.rollup,
+ ColumnHolder.TIME_COLUMN_NAME.equals(Iterables.getFirst(dimensionOrder, null)) ? null : dimensionOrder
Review Comment:
Hmm I don't totally remember why this is conditional. I updated it to always write out.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "clintropolis (via GitHub)" <gi...@apache.org>.
clintropolis commented on PR #16849:
URL: https://github.com/apache/druid/pull/16849#issuecomment-2623283243
>Im a little confused on when you said that getMaxTime() gives the end interval of the segment.
Oh sorry, i meant the max timestamp within the segment interval, that code is the same, its just on `TimeBoundaryInspector` now, https://github.com/apache/druid/blob/master/processing/src/main/java/org/apache/druid/segment/QueryableIndexTimeBoundaryInspector.java#L85
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #16849:
URL: https://github.com/apache/druid/pull/16849#discussion_r1708052963
##########
processing/src/main/java/org/apache/druid/segment/incremental/OnheapIncrementalIndex.java:
##########
@@ -918,4 +940,84 @@ public void clear()
facts.clear();
}
}
+
+ static final class PlainNonTimeOrderedFactsHolder implements FactsHolder
+ {
+ private final Deque<IncrementalIndexRow> facts;
+ private final Comparator<IncrementalIndexRow> incrementalIndexRowComparator;
+ private volatile long minTime = DateTimes.MAX.getMillis();
+ private volatile long maxTime = DateTimes.MIN.getMillis();
+
+ public PlainNonTimeOrderedFactsHolder(Comparator<IncrementalIndexRow> incrementalIndexRowComparator)
+ {
+ this.facts = new ArrayDeque<>();
+ this.incrementalIndexRowComparator = incrementalIndexRowComparator;
+ }
+
+ @Override
+ public int getPriorIndex(IncrementalIndexRow key)
+ {
+ // always return EMPTY_ROW_INDEX to indicate that no prior key cause we always add new row
+ return IncrementalIndexRow.EMPTY_ROW_INDEX;
+ }
+
+ @Override
+ public long getMinTimeMillis()
+ {
+ return minTime;
+ }
+
+ @Override
+ public long getMaxTimeMillis()
+ {
+ return maxTime;
+ }
+
+ @Override
+ public Iterator<IncrementalIndexRow> iterator(boolean descending)
Review Comment:
It seemed to make more sense to implement it here rather than throw an error. I suppose in theory there could be a use case, in the future, for situations like `ORDER BY userId DESC` when the segment is sorted by `userId`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #16849:
URL: https://github.com/apache/druid/pull/16849#discussion_r1708087327
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/AbstractBatchIndexTask.java:
##########
@@ -620,10 +620,18 @@ public static Function<Set<DataSegment>, Set<DataSegment>> addCompactionStateToS
if (storeCompactionState) {
TuningConfig tuningConfig = ingestionSpec.getTuningConfig();
GranularitySpec granularitySpec = ingestionSpec.getDataSchema().getGranularitySpec();
- // We do not need to store dimensionExclusions and spatialDimensions since auto compaction does not support them
- DimensionsSpec dimensionsSpec = ingestionSpec.getDataSchema().getDimensionsSpec() == null
- ? null
- : new DimensionsSpec(ingestionSpec.getDataSchema().getDimensionsSpec().getDimensions());
+ DimensionsSpec dimensionsSpec;
+ if (ingestionSpec.getDataSchema().getDimensionsSpec() == null) {
+ dimensionsSpec = null;
+ } else {
+ // We do not need to store dimensionExclusions and spatialDimensions since auto compaction does not support them
Review Comment:
will update.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #16849:
URL: https://github.com/apache/druid/pull/16849#discussion_r1706160627
##########
processing/src/main/java/org/apache/druid/segment/incremental/IncrementalIndex.java:
##########
@@ -887,19 +942,23 @@
* Currently called to initialize IncrementalIndex dimension order during index creation
* Index dimension ordering could be changed to initialize from DimensionsSpec after resolution of
* https://github.com/apache/druid/issues/2011
+ *
+ * @param oldDimensionOrder dimension order to initialize
+ * @param oldColumnFormats formats for the dimensions
*/
public void loadDimensionIterable(
Iterable<String> oldDimensionOrder,
- Map<String, ColumnFormat> oldColumnCapabilities
+ Map<String, ColumnFormat> oldColumnFormats
)
{
synchronized (dimensionDescs) {
- if (!dimensionDescs.isEmpty()) {
- throw new ISE("Cannot load dimension order when existing order[%s] is not empty.", dimensionDescs.keySet());
+ if (size() != 0) {
+ throw new ISE("Cannot load dimension order when existing index is not empty.", dimensionDescs.keySet());
Review Comment:
fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "clintropolis (via GitHub)" <gi...@apache.org>.
clintropolis commented on code in PR #16849:
URL: https://github.com/apache/druid/pull/16849#discussion_r1727617989
##########
docs/querying/sql-query-context.md:
##########
@@ -50,11 +50,11 @@ Configure Druid SQL query planning using the parameters in the table below.
|`sqlReverseLookup`|Whether to consider the [reverse-lookup rewrite](lookups.md#reverse-lookup) of the `LOOKUP` function during SQL planning.<br /><br />Calls to `LOOKUP` are only reversed when the number of matching keys is lower than both `inSubQueryThreshold` and `sqlReverseLookupThreshold`.|true|
|`sqlReverseLookupThreshold`|Maximum size of `IN` filter to create when applying a [reverse-lookup rewrite](lookups.md#reverse-lookup). If a `LOOKUP` call matches more keys than this threshold, it is left as-is.<br /><br />If `inSubQueryThreshold` is lower than `sqlReverseLookupThreshold`, the `inSubQueryThreshold` is used as the threshold instead.|10000|
|`sqlPullUpLookup`|Whether to consider the [pull-up rewrite](lookups.md#pull-up) of the `LOOKUP` function during SQL planning.|true|
-|`enableJoinLeftTableScanDirect`|`false`|This flag applies to queries which have joins. For joins, where left child is a simple scan with a filter, by default, druid will run the scan as a query and the join the results to the right child on broker. Setting this flag to true overrides that behavior and druid will attempt to push the join to data servers instead. Please note that the flag could be applicable to queries even if there is no explicit join. since queries can internally translated into a join by the SQL planner.|
-|`maxNumericInFilters`|`-1`|Max limit for the amount of numeric values that can be compared for a string type dimension when the entire SQL WHERE clause of a query translates only to an [OR](../querying/filters.md#or) of [Bound filter](../querying/filters.md#bound-filter). By default, Druid does not restrict the amount of of numeric Bound Filters on String columns, although this situation may block other queries from running. Set this parameter to a smaller value to prevent Druid from running queries that have prohibitively long segment processing times. The optimal limit requires some trial and error; we recommend starting with 100. Users who submit a query that exceeds the limit of `maxNumericInFilters` should instead rewrite their queries to use strings in the `WHERE` clause instead of numbers. For example, `WHERE someString IN (‘123’, ‘456’)`. This value cannot exceed the set system configuration `druid.sql.planner.maxNumericInFilters`. This value is ignored if `druid.s
ql.planner.maxNumericInFilters` is not set explicitly.|
-|`inFunctionThreshold`|`100`| At or beyond this threshold number of values, SQL `IN` is converted to [`SCALAR_IN_ARRAY`](sql-functions.md#scalar_in_array). A threshold of 0 forces this conversion in all cases. A threshold of [Integer.MAX_VALUE] disables this conversion. The converted function is eligible for fewer planning-time optimizations, which speeds up planning, but may prevent certain planning-time optimizations.|
-|`inFunctionExprThreshold`|`2`| At or beyond this threshold number of values, SQL `IN` is eligible for execution using the native function `scalar_in_array` rather than an <code>||</code> of `==`, even if the number of values is below `inFunctionThreshold`. This property only affects translation of SQL `IN` to a [native expression](math-expr.md). It does not affect translation of SQL `IN` to a [native filter](filters.md). This property is provided for backwards compatibility purposes, and may be removed in a future release.|
-|`inSubQueryThreshold`|`2147483647`| At or beyond this threshold number of values, SQL `IN` is converted to `JOIN` on an inline table. `inFunctionThreshold` takes priority over this setting. A threshold of 0 forces usage of an inline table in all cases where the size of a SQL `IN` is larger than `inFunctionThreshold`. A threshold of `2147483647` disables the rewrite of SQL `IN` to `JOIN`. |
+|`enableJoinLeftTableScanDirect`|This flag applies to queries which have joins. For joins, where left child is a simple scan with a filter, by default, druid will run the scan as a query and the join the results to the right child on broker. Setting this flag to true overrides that behavior and druid will attempt to push the join to data servers instead. Please note that the flag could be applicable to queries even if there is no explicit join. since queries can internally translated into a join by the SQL planner.|`false`|
+|`maxNumericInFilters`|Max limit for the amount of numeric values that can be compared for a string type dimension when the entire SQL WHERE clause of a query translates only to an [OR](../querying/filters.md#or) of [Bound filter](../querying/filters.md#bound-filter). By default, Druid does not restrict the amount of of numeric Bound Filters on String columns, although this situation may block other queries from running. Set this parameter to a smaller value to prevent Druid from running queries that have prohibitively long segment processing times. The optimal limit requires some trial and error; we recommend starting with 100. Users who submit a query that exceeds the limit of `maxNumericInFilters` should instead rewrite their queries to use strings in the `WHERE` clause instead of numbers. For example, `WHERE someString IN (‘123’, ‘456’)`. This value cannot exceed the set system configuration `druid.sql.planner.maxNumericInFilters`. This value is ignored if `druid.sql.pl
anner.maxNumericInFilters` is not set explicitly.|`-1`|
+|`inFunctionThreshold`| At or beyond this threshold number of values, SQL `IN` is converted to [`SCALAR_IN_ARRAY`](sql-functions.md#scalar_in_array). A threshold of 0 forces this conversion in all cases. A threshold of [Integer.MAX_VALUE] disables this conversion. The converted function is eligible for fewer planning-time optimizations, which speeds up planning, but may prevent certain planning-time optimizations.| `100`|
+|`inFunctionExprThreshold`|At or beyond this threshold number of values, SQL `IN` is eligible for execution using the native function `scalar_in_array` rather than an <code>||</code> of `==`, even if the number of values is below `inFunctionThreshold`. This property only affects translation of SQL `IN` to a [native expression](math-expr.md). It does not affect translation of SQL `IN` to a [native filter](filters.md). This property is provided for backwards compatibility purposes, and may be removed in a future release.|`2`|
+|`inSubQueryThreshold`|At or beyond this threshold number of values, SQL `IN` is converted to `JOIN` on an inline table. `inFunctionThreshold` takes priority over this setting. A threshold of 0 forces usage of an inline table in all cases where the size of a SQL `IN` is larger than `inFunctionThreshold`. A threshold of `2147483647` disables the rewrite of SQL `IN` to `JOIN`. |`2147483647`|
Review Comment:
oops, forgot about that :+1:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm merged PR #16849:
URL: https://github.com/apache/druid/pull/16849
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "maytasm (via GitHub)" <gi...@apache.org>.
maytasm commented on PR #16849:
URL: https://github.com/apache/druid/pull/16849#issuecomment-2623163755
@clintropolis @gianm
I think this PR broke dataSourceMetadata query.
Seems like MaxIngestedEventTimeInspector in DataSourceMetadataQueryRunner will always be null.
This can be reproduced with the wikipedia dataset.
Just run
```
{
"queryType" : "dataSourceMetadata",
"dataSource" : "wikipedia"
}
```
The "maxIngestedEventTime" returned is -146136543-09-08T08:23:32.096Z is wrong.
Doesn't look like this code:
```
@SuppressWarnings("unchecked")
@Nullable
@Override
public <T> T as(@Nonnull Class<T> clazz)
{
final Function<QueryableIndexSegment, ?> fn = AS_MAP.get(clazz);
if (fn != null) {
final T fnApply = (T) fn.apply(this);
if (fnApply != null) {
return fnApply;
}
}
if (TimeBoundaryInspector.class.equals(clazz)) {
return (T) timeBoundaryInspector;
} else if (Metadata.class.equals(clazz)) {
return (T) index.getMetadata();
} else if (PhysicalSegmentInspector.class.equals(clazz)) {
return (T) new QueryableIndexPhysicalSegmentInspector(index);
} else if (TopNOptimizationInspector.class.equals(clazz)) {
return (T) new SimpleTopNOptimizationInspector(true);
}
return Segment.super.as(clazz);
}
```
Is handling MaxIngestedEventTimeInspector?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #16849:
URL: https://github.com/apache/druid/pull/16849#discussion_r1708118633
##########
processing/src/main/java/org/apache/druid/segment/incremental/IncrementalIndexStorageAdapter.java:
##########
@@ -295,6 +300,16 @@ public Metadata getMetadata()
return index.getMetadata();
}
+ @Override
+ public List<String> getSortOrder()
+ {
+ if (index.timePosition == 0) {
+ return Metadata.SORTED_BY_TIME_ONLY;
+ } else {
+ return Collections.emptyList();
Review Comment:
I think that makes sense. I added a comment about this being a possible change in the future.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "clintropolis (via GitHub)" <gi...@apache.org>.
clintropolis commented on code in PR #16849:
URL: https://github.com/apache/druid/pull/16849#discussion_r1706220824
##########
processing/src/main/java/org/apache/druid/segment/incremental/OnheapIncrementalIndex.java:
##########
@@ -918,4 +940,84 @@ public void clear()
facts.clear();
}
}
+
+ static final class PlainNonTimeOrderedFactsHolder implements FactsHolder
+ {
+ private final Deque<IncrementalIndexRow> facts;
+ private final Comparator<IncrementalIndexRow> incrementalIndexRowComparator;
+ private volatile long minTime = DateTimes.MAX.getMillis();
+ private volatile long maxTime = DateTimes.MIN.getMillis();
+
+ public PlainNonTimeOrderedFactsHolder(Comparator<IncrementalIndexRow> incrementalIndexRowComparator)
+ {
+ this.facts = new ArrayDeque<>();
+ this.incrementalIndexRowComparator = incrementalIndexRowComparator;
+ }
+
+ @Override
+ public int getPriorIndex(IncrementalIndexRow key)
+ {
+ // always return EMPTY_ROW_INDEX to indicate that no prior key cause we always add new row
+ return IncrementalIndexRow.EMPTY_ROW_INDEX;
+ }
+
+ @Override
+ public long getMinTimeMillis()
+ {
+ return minTime;
+ }
+
+ @Override
+ public long getMaxTimeMillis()
+ {
+ return maxTime;
+ }
+
+ @Override
+ public Iterator<IncrementalIndexRow> iterator(boolean descending)
Review Comment:
descending is pretty tightly coupled with time ordering, but i guess is harmless to implement
##########
processing/src/main/java/org/apache/druid/segment/incremental/OnheapIncrementalIndex.java:
##########
@@ -777,11 +790,18 @@ public Iterator<IncrementalIndexRow> iterator(boolean descending)
@Override
public Iterable<IncrementalIndexRow> timeRangeIterable(boolean descending, long timeStart, long timeEnd)
{
- IncrementalIndexRow start = new IncrementalIndexRow(timeStart, new Object[]{}, dimensionDescsList);
- IncrementalIndexRow end = new IncrementalIndexRow(timeEnd, new Object[]{}, dimensionDescsList);
- ConcurrentNavigableMap<IncrementalIndexRow, IncrementalIndexRow> subMap = facts.subMap(start, end);
- ConcurrentMap<IncrementalIndexRow, IncrementalIndexRow> rangeMap = descending ? subMap.descendingMap() : subMap;
- return rangeMap.keySet();
+ if (timeOrdered) {
+ IncrementalIndexRow start = new IncrementalIndexRow(timeStart, new Object[]{}, dimensionDescsList);
+ IncrementalIndexRow end = new IncrementalIndexRow(timeEnd, new Object[]{}, dimensionDescsList);
+ ConcurrentNavigableMap<IncrementalIndexRow, IncrementalIndexRow> subMap = facts.subMap(start, end);
+ ConcurrentMap<IncrementalIndexRow, IncrementalIndexRow> rangeMap = descending ? subMap.descendingMap() : subMap;
+ return rangeMap.keySet();
+ } else {
+ return Iterables.filter(
Review Comment:
I know we need this right now, but i'm not sure we will need this after #16533. timeRangeIterable is primarily used to support query granularity buckets in topN (in my branch to support mark/resetToMark to move the cursor in the facts table to the correct granularity bucket without having to advance the cursor directly). i think we could just use `iterator` or expose an alternative iteralble for incremental index cursor if we aren't requesting time ordering
##########
processing/src/main/java/org/apache/druid/segment/Metadata.java:
##########
@@ -100,6 +112,14 @@ public Boolean isRollup()
return rollup;
}
+ @Nullable
+ @JsonProperty
+ @JsonInclude(JsonInclude.Include.NON_NULL)
+ public List<String> getSortOrder()
+ {
+ return sortOrder;
+ }
+
Review Comment:
i know it cannot be specified during ingest with the current mechanisms, but it seems like we should include the direction of the ordering as well, maybe re-use the scan query ordering?
##########
processing/src/main/java/org/apache/druid/segment/incremental/IncrementalIndexStorageAdapter.java:
##########
@@ -295,6 +300,16 @@ public Metadata getMetadata()
return index.getMetadata();
}
+ @Override
+ public List<String> getSortOrder()
+ {
+ if (index.timePosition == 0) {
+ return Metadata.SORTED_BY_TIME_ONLY;
+ } else {
+ return Collections.emptyList();
Review Comment:
i suppose depending on the type of facts holder we could actually report something here (rollup should be ordered i think?)
##########
processing/src/main/java/org/apache/druid/segment/incremental/IncrementalIndex.java:
##########
@@ -334,6 +349,17 @@ protected IncrementalIndex(
if (!spatialDimensions.isEmpty()) {
this.rowTransformers.add(new SpatialDimensionRowTransformer(spatialDimensions));
}
+
+ // Set metadata last, so dimensionOrder is populated
+ final List<String> dimensionOrder = getDimensionOrder();
+ this.metadata = new Metadata(
+ null,
+ getCombiningAggregators(metrics),
+ incrementalIndexSchema.getTimestampSpec(),
+ this.gran,
+ this.rollup,
+ ColumnHolder.TIME_COLUMN_NAME.equals(Iterables.getFirst(dimensionOrder, null)) ? null : dimensionOrder
Review Comment:
maybe we should always write this out instead of leaving it null? i guess it is done like this to make it easier to fill in older segments and new segments ordered by time first?
##########
processing/src/main/java/org/apache/druid/segment/StorageAdapter.java:
##########
@@ -129,6 +141,18 @@ default RowSignature getRowSignature()
@Nullable
Metadata getMetadata();
+ /**
+ * Returns column names that this adapter's data is sorted by. Cursors returned by this adapter return rows in
+ * this ordering, using the natural comparator for the type of the column as returned by
+ * {@link #getColumnCapabilities(String)}. Includes {@link ColumnHolder#TIME_COLUMN_NAME} if appropriate.
+ */
+ List<String> getSortOrder();
+
+ default boolean isTimeOrdered()
+ {
+ return ColumnHolder.TIME_COLUMN_NAME.equals(Iterables.getFirst(getSortOrder(), null));
+ }
Review Comment:
in the context of #16533, this should probably live on `CursorHolder` instead of `StorageAdapter`, though it will require a bunch of tests to make and dispose of a cursor holder to check if their query against the sort order, but otherwise shouldn't be very disruptive. I wonder if we should include a direction similar to in that PR though, maybe re-using scan query ordering so that these two changes are compatible?
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/AbstractBatchIndexTask.java:
##########
@@ -620,10 +620,18 @@ public static Function<Set<DataSegment>, Set<DataSegment>> addCompactionStateToS
if (storeCompactionState) {
TuningConfig tuningConfig = ingestionSpec.getTuningConfig();
GranularitySpec granularitySpec = ingestionSpec.getDataSchema().getGranularitySpec();
- // We do not need to store dimensionExclusions and spatialDimensions since auto compaction does not support them
- DimensionsSpec dimensionsSpec = ingestionSpec.getDataSchema().getDimensionsSpec() == null
- ? null
- : new DimensionsSpec(ingestionSpec.getDataSchema().getDimensionsSpec().getDimensions());
+ DimensionsSpec dimensionsSpec;
+ if (ingestionSpec.getDataSchema().getDimensionsSpec() == null) {
+ dimensionsSpec = null;
+ } else {
+ // We do not need to store dimensionExclusions and spatialDimensions since auto compaction does not support them
Review Comment:
nit: this comment is slightly misleading after #15321, as compaction does retain spatial dimensions, they just get translated into normal dimensions list via the handler instead of the separate deprecated spatialDimensions list
##########
processing/src/main/java/org/apache/druid/segment/UnnestStorageAdapter.java:
##########
@@ -256,6 +256,23 @@ public Metadata getMetadata()
return baseAdapter.getMetadata();
}
+ @Override
+ public List<String> getSortOrder()
+ {
+ final List<String> baseSortOrder = baseAdapter.getSortOrder();
+
+ // Sorted the same way as the base segment, unless the unnested column shadows one of the base columns.
Review Comment:
given that we will basically always be reading the unnested column, i guess the main expected utility of this will be if ordering by one of the columns that is not being unnested?
##########
processing/src/main/java/org/apache/druid/segment/RowBasedStorageAdapter.java:
##########
@@ -167,6 +168,15 @@ public Metadata getMetadata()
throw new UnsupportedOperationException("Cannot retrieve metadata");
}
+ @Override
+ public List<String> getSortOrder()
+ {
+ // It's possibly incorrect in some cases for sort order to be SORTED_BY_TIME_ONLY here, but for historical reasons,
+ // we're keeping this in place for now. The handling of "interval" in "makeCursors", which has been in place for
+ // some time, suggests we think the data is always sorted by time.
Review Comment:
is this referring to the `RowWalker` with its `skipToDateTime`?
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerImpl.java:
##########
@@ -2266,7 +2300,12 @@ private static void populateDimensionsAndAggregators(
QueryContext context
)
{
- if (outputColumnAggregatorFactories.containsKey(outputColumn)) {
+ if (ColumnHolder.TIME_COLUMN_NAME.equals(outputColumn)) {
+ if (!type.is(ValueType.LONG)) {
+ throw DruidException.defensive("Incorrect type[%s] for column[%s]", type, outputColumn);
+ }
+ dimensions.add(new LongDimensionSchema(outputColumn));
Review Comment:
i wonder if this should use `AutoTypeColumnSchema` with `castToType` set to `LONG` until we make a version of `LongDimensionSchema` with the features of the auto columns, since indexes on time column might be useful.
I guess I should get around to bringing some of the features of the auto columns to the classic dimension schemas, or at least re-branding the `AutoTypeColumnSchema` when `castToType` is set to something like `BuiltInTypeColumnSchema` so it looks less weird...
##########
processing/src/main/java/org/apache/druid/segment/Metadata.java:
##########
@@ -233,6 +260,39 @@ public String toString()
", timestampSpec=" + timestampSpec +
", queryGranularity=" + queryGranularity +
", rollup=" + rollup +
+ ", sortOrder=" + sortOrder +
'}';
}
+
+ @Nullable
+ private static List<String> mergeSortOrders(List<List<String>> sortOrdersToMerge)
+ {
+ if (sortOrdersToMerge.isEmpty()) {
+ return null;
+ }
+
+ if (sortOrdersToMerge.stream().anyMatch(Objects::isNull)) {
+ return null;
+ }
+
+ final List<String> mergedSortOrder = new ArrayList<>();
+
+ while (true) {
+ String column = null;
+
+ for (final List<String> sortOrder : sortOrdersToMerge) {
+ if (mergedSortOrder.size() >= sortOrder.size()) {
Review Comment:
does this happen from columns with all null values or something? maybe this method could use some comments to make the rationale behind the decisions clearer
##########
processing/src/main/java/org/apache/druid/segment/Metadata.java:
##########
@@ -233,6 +260,39 @@ public String toString()
", timestampSpec=" + timestampSpec +
", queryGranularity=" + queryGranularity +
", rollup=" + rollup +
+ ", sortOrder=" + sortOrder +
'}';
}
+
+ @Nullable
+ private static List<String> mergeSortOrders(List<List<String>> sortOrdersToMerge)
+ {
+ if (sortOrdersToMerge.isEmpty()) {
+ return null;
+ }
+
+ if (sortOrdersToMerge.stream().anyMatch(Objects::isNull)) {
+ return null;
Review Comment:
should this be an error if they aren't all null? seems like it should be pretty consistent across indexable adapters...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #16849:
URL: https://github.com/apache/druid/pull/16849#discussion_r1708092790
##########
processing/src/main/java/org/apache/druid/segment/Metadata.java:
##########
@@ -233,6 +260,39 @@ public String toString()
", timestampSpec=" + timestampSpec +
", queryGranularity=" + queryGranularity +
", rollup=" + rollup +
+ ", sortOrder=" + sortOrder +
'}';
}
+
+ @Nullable
+ private static List<String> mergeSortOrders(List<List<String>> sortOrdersToMerge)
+ {
+ if (sortOrdersToMerge.isEmpty()) {
+ return null;
+ }
+
+ if (sortOrdersToMerge.stream().anyMatch(Objects::isNull)) {
+ return null;
Review Comment:
I made it lenient for two reasons:
- Merging of the other parts of the Metadata is also lenient
- It isn't documented that this method requires that all Metadatas are sourced from segments created with the same ingestion spec + the same version of the software
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on PR #16849:
URL: https://github.com/apache/druid/pull/16849#issuecomment-2298211708
Pushed up a commit resolving conflicts with #16533. Main changes:
- Changed frame cursors to report "no ordering" rather than time ordering.
- Removed `descending` from `Joinable#makeJoinMatcher`. Now that `descending` has more or less been phased out in favor of `ordering: [__time DESC]`, the joinable order no longer needs to be reversed. (All joined rows arising from the same left-hand row have the same value.)
- Moved getMinTime, getMaxTime, and getMaxIngestedEventTime from StorageAdapter to TimeBoundaryInspector and MaxIngestedEventTimeInspector. This is mainly necessary for timeBoundary queries to be able to tell whether they can use getMinTime and getMaxTime, vs. needing to use a cursor. Previously, timeBoundary assumed that an adapter backing a TableDataSource was guaranteed to have an exact getMinTime and getMaxTime. But after this patch, that isn't necessarily going to be the case (in particular: a non-time-sorted segment won't be able to know its exact min/max time without a full scan.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "clintropolis (via GitHub)" <gi...@apache.org>.
clintropolis commented on code in PR #16849:
URL: https://github.com/apache/druid/pull/16849#discussion_r1725939927
##########
docs/querying/sql-query-context.md:
##########
@@ -50,11 +50,11 @@ Configure Druid SQL query planning using the parameters in the table below.
|`sqlReverseLookup`|Whether to consider the [reverse-lookup rewrite](lookups.md#reverse-lookup) of the `LOOKUP` function during SQL planning.<br /><br />Calls to `LOOKUP` are only reversed when the number of matching keys is lower than both `inSubQueryThreshold` and `sqlReverseLookupThreshold`.|true|
|`sqlReverseLookupThreshold`|Maximum size of `IN` filter to create when applying a [reverse-lookup rewrite](lookups.md#reverse-lookup). If a `LOOKUP` call matches more keys than this threshold, it is left as-is.<br /><br />If `inSubQueryThreshold` is lower than `sqlReverseLookupThreshold`, the `inSubQueryThreshold` is used as the threshold instead.|10000|
|`sqlPullUpLookup`|Whether to consider the [pull-up rewrite](lookups.md#pull-up) of the `LOOKUP` function during SQL planning.|true|
-|`enableJoinLeftTableScanDirect`|`false`|This flag applies to queries which have joins. For joins, where left child is a simple scan with a filter, by default, druid will run the scan as a query and the join the results to the right child on broker. Setting this flag to true overrides that behavior and druid will attempt to push the join to data servers instead. Please note that the flag could be applicable to queries even if there is no explicit join. since queries can internally translated into a join by the SQL planner.|
-|`maxNumericInFilters`|`-1`|Max limit for the amount of numeric values that can be compared for a string type dimension when the entire SQL WHERE clause of a query translates only to an [OR](../querying/filters.md#or) of [Bound filter](../querying/filters.md#bound-filter). By default, Druid does not restrict the amount of of numeric Bound Filters on String columns, although this situation may block other queries from running. Set this parameter to a smaller value to prevent Druid from running queries that have prohibitively long segment processing times. The optimal limit requires some trial and error; we recommend starting with 100. Users who submit a query that exceeds the limit of `maxNumericInFilters` should instead rewrite their queries to use strings in the `WHERE` clause instead of numbers. For example, `WHERE someString IN (‘123’, ‘456’)`. This value cannot exceed the set system configuration `druid.sql.planner.maxNumericInFilters`. This value is ignored if `druid.s
ql.planner.maxNumericInFilters` is not set explicitly.|
-|`inFunctionThreshold`|`100`| At or beyond this threshold number of values, SQL `IN` is converted to [`SCALAR_IN_ARRAY`](sql-functions.md#scalar_in_array). A threshold of 0 forces this conversion in all cases. A threshold of [Integer.MAX_VALUE] disables this conversion. The converted function is eligible for fewer planning-time optimizations, which speeds up planning, but may prevent certain planning-time optimizations.|
-|`inFunctionExprThreshold`|`2`| At or beyond this threshold number of values, SQL `IN` is eligible for execution using the native function `scalar_in_array` rather than an <code>||</code> of `==`, even if the number of values is below `inFunctionThreshold`. This property only affects translation of SQL `IN` to a [native expression](math-expr.md). It does not affect translation of SQL `IN` to a [native filter](filters.md). This property is provided for backwards compatibility purposes, and may be removed in a future release.|
-|`inSubQueryThreshold`|`2147483647`| At or beyond this threshold number of values, SQL `IN` is converted to `JOIN` on an inline table. `inFunctionThreshold` takes priority over this setting. A threshold of 0 forces usage of an inline table in all cases where the size of a SQL `IN` is larger than `inFunctionThreshold`. A threshold of `2147483647` disables the rewrite of SQL `IN` to `JOIN`. |
+|`enableJoinLeftTableScanDirect`|This flag applies to queries which have joins. For joins, where left child is a simple scan with a filter, by default, druid will run the scan as a query and the join the results to the right child on broker. Setting this flag to true overrides that behavior and druid will attempt to push the join to data servers instead. Please note that the flag could be applicable to queries even if there is no explicit join. since queries can internally translated into a join by the SQL planner.|`false`|
+|`maxNumericInFilters`|Max limit for the amount of numeric values that can be compared for a string type dimension when the entire SQL WHERE clause of a query translates only to an [OR](../querying/filters.md#or) of [Bound filter](../querying/filters.md#bound-filter). By default, Druid does not restrict the amount of of numeric Bound Filters on String columns, although this situation may block other queries from running. Set this parameter to a smaller value to prevent Druid from running queries that have prohibitively long segment processing times. The optimal limit requires some trial and error; we recommend starting with 100. Users who submit a query that exceeds the limit of `maxNumericInFilters` should instead rewrite their queries to use strings in the `WHERE` clause instead of numbers. For example, `WHERE someString IN (‘123’, ‘456’)`. This value cannot exceed the set system configuration `druid.sql.planner.maxNumericInFilters`. This value is ignored if `druid.sql.pl
anner.maxNumericInFilters` is not set explicitly.|`-1`|
+|`inFunctionThreshold`| At or beyond this threshold number of values, SQL `IN` is converted to [`SCALAR_IN_ARRAY`](sql-functions.md#scalar_in_array). A threshold of 0 forces this conversion in all cases. A threshold of [Integer.MAX_VALUE] disables this conversion. The converted function is eligible for fewer planning-time optimizations, which speeds up planning, but may prevent certain planning-time optimizations.| `100`|
+|`inFunctionExprThreshold`|At or beyond this threshold number of values, SQL `IN` is eligible for execution using the native function `scalar_in_array` rather than an <code>||</code> of `==`, even if the number of values is below `inFunctionThreshold`. This property only affects translation of SQL `IN` to a [native expression](math-expr.md). It does not affect translation of SQL `IN` to a [native filter](filters.md). This property is provided for backwards compatibility purposes, and may be removed in a future release.|`2`|
+|`inSubQueryThreshold`|At or beyond this threshold number of values, SQL `IN` is converted to `JOIN` on an inline table. `inFunctionThreshold` takes priority over this setting. A threshold of 0 forces usage of an inline table in all cases where the size of a SQL `IN` is larger than `inFunctionThreshold`. A threshold of `2147483647` disables the rewrite of SQL `IN` to `JOIN`. |`2147483647`|
Review Comment:
did you mean to document `sqlUseGranularity` here perhaps for when querying non time ordered segments?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on PR #16849:
URL: https://github.com/apache/druid/pull/16849#issuecomment-2625145086
Oops, the behavior change was unintentional (at least I didn't intend it, and this was my PR). This should restore the old behavior: #17686
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Segments sorted by non-time columns. (druid)
Posted by "maytasm (via GitHub)" <gi...@apache.org>.
maytasm commented on PR #16849:
URL: https://github.com/apache/druid/pull/16849#issuecomment-2623282686
- Ah, the timeBoundary does return the actual time (min and max) of the rows/data which matches with the previous dataSourceMetadata. We can switch to the timeBoundary query as a workaround. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org