cecemei commented on code in PR #18486:
URL: https://github.com/apache/druid/pull/18486#discussion_r2331278973
##########
processing/src/test/java/org/apache/druid/segment/projections/ProjectionsTest.java:
##########
@@ -220,6 +231,219 @@ void testSchemaMatchFilterIncludedInProjection()
Assertions.assertEquals(expected, projectionMatch);
}
+ @Test
+ public void testSchemaMatchIntervalEternity()
+ {
+ final DateTime time = Granularities.DAY.bucketStart(DateTimes.nowUtc());
+ RowSignature baseTable = RowSignature.builder()
+ .addTimeColumn()
+ .add("a", ColumnType.STRING)
+ .build();
+
+ AggregateProjectionMetadata spec = new AggregateProjectionMetadata(
+ AggregateProjectionSpec.builder("some_projection")
+ .groupingColumns(new StringDimensionSchema("a"))
+ .build()
+ .toMetadataSchema(),
+ 12345
+ );
+
+ CursorBuildSpec cursorBuildSpec = CursorBuildSpec.builder()
+
.setPhysicalColumns(Set.of("a"))
+
.setGroupingColumns(List.of("a"))
+ .build();
+
+ ProjectionMatch projectionMatch = Projections.matchAggregateProjection(
+ spec.getSchema(),
+ cursorBuildSpec,
+ Intervals.ETERNITY,
+ new RowSignatureChecker(baseTable)
+ );
+ ProjectionMatch expected = new ProjectionMatch(
+ CursorBuildSpec.builder()
+ .setPhysicalColumns(Set.of("a"))
+ .setGroupingColumns(List.of("a"))
+ .setAggregators(List.of())
+ .build(),
+ Map.of()
+ );
+ Assertions.assertEquals(expected, projectionMatch);
+
+ // projection with no time column can still match cursor build spec with
eternity interval
+ projectionMatch = Projections.matchAggregateProjection(
+ spec.getSchema(),
+ cursorBuildSpec,
+ new Interval(time, time.plusHours(1)),
+ new RowSignatureChecker(baseTable)
+ );
+
+ Assertions.assertEquals(expected, projectionMatch);
+ }
+
+ @Test
+ public void testSchemaMatchIntervalProjectionGranularityEternity()
+ {
+ final DateTime time = Granularities.DAY.bucketStart(DateTimes.nowUtc());
+
+ RowSignature baseTable = RowSignature.builder()
+ .addTimeColumn()
+ .add("a", ColumnType.STRING)
+ .build();
+
+ // hour granularity projection
+ AggregateProjectionMetadata spec = new AggregateProjectionMetadata(
+ AggregateProjectionSpec.builder("some_projection")
+ .groupingColumns(
+ new
LongDimensionSchema(Granularities.GRANULARITY_VIRTUAL_COLUMN_NAME),
+ new StringDimensionSchema("a")
+ )
+ .virtualColumns(
+ Granularities.toVirtualColumn(
+ Granularities.HOUR,
+
Granularities.GRANULARITY_VIRTUAL_COLUMN_NAME
+ )
+ )
+ .build()
+ .toMetadataSchema(),
+ 12345
+ );
+
+ // eternity interval cursor build spec with granularity set
+ CursorBuildSpec cursorBuildSpec = CursorBuildSpec.builder()
+
.setPhysicalColumns(Set.of("__time", "a"))
+
.setGroupingColumns(List.of("v0", "a"))
+ .setVirtualColumns(
+ VirtualColumns.create(
+
Granularities.toVirtualColumn(Granularities.HOUR, "v0")
+ )
+ )
+ .build();
+
+
+ ProjectionMatch expectedWithGranularity = new ProjectionMatch(
+ CursorBuildSpec.builder()
+ .setPhysicalColumns(Set.of("__time", "a"))
+ .setGroupingColumns(List.of("v0", "a"))
+ .setAggregators(List.of())
+ .build(),
+ Map.of("v0", "__time")
+ );
+
+ ProjectionMatch projectionMatch = Projections.matchAggregateProjection(
+ spec.getSchema(),
+ cursorBuildSpec,
+ Intervals.ETERNITY,
+ new RowSignatureChecker(baseTable)
+ );
+ Assertions.assertEquals(expectedWithGranularity, projectionMatch);
+
+ projectionMatch = Projections.matchAggregateProjection(
+ spec.getSchema(),
+ cursorBuildSpec,
+ new Interval(time, time.plusHours(1)),
+ new RowSignatureChecker(baseTable)
+ );
+
+ Assertions.assertEquals(expectedWithGranularity, projectionMatch);
+
+ }
+
+ @Test
+ public void testSchemaMatchIntervalProjectionGranularity()
+ {
+ final DateTime time = Granularities.DAY.bucketStart(DateTimes.nowUtc());
+
+ RowSignature baseTable = RowSignature.builder()
+ .addTimeColumn()
+ .add("a", ColumnType.STRING)
+ .build();
+
+ // hour granularity projection
+ AggregateProjectionMetadata spec = new AggregateProjectionMetadata(
+ AggregateProjectionSpec.builder("some_projection")
+ .groupingColumns(
+ new
LongDimensionSchema(Granularities.GRANULARITY_VIRTUAL_COLUMN_NAME),
+ new StringDimensionSchema("a")
+ )
+ .virtualColumns(
+ Granularities.toVirtualColumn(
+ Granularities.HOUR,
+
Granularities.GRANULARITY_VIRTUAL_COLUMN_NAME
+ )
+ )
+ .build()
+ .toMetadataSchema(),
+ 12345
+ );
+
+ Interval day = new Interval(time, time.plusDays(1));
+ Interval hour = new Interval(time, time.plusHours(1));
+ Interval partial = new Interval(time, time.plusMinutes(42));
+ // aligned interval cursor build spec
+ CursorBuildSpec cursorBuildSpecHourInterval = CursorBuildSpec.builder()
+
.setInterval(hour)
+
.setPhysicalColumns(Set.of("a"))
+
.setGroupingColumns(List.of("a"))
+ .build();
+
+ ProjectionMatch expected = new ProjectionMatch(
+ CursorBuildSpec.builder()
+ .setInterval(hour)
+ .setPhysicalColumns(Set.of("a"))
+ .setGroupingColumns(List.of("a"))
+ .setAggregators(List.of())
+ .build(),
+ Map.of()
+ );
+ ProjectionMatch projectionMatch = Projections.matchAggregateProjection(
+ spec.getSchema(),
+ cursorBuildSpecHourInterval,
+ day,
+ new RowSignatureChecker(baseTable)
+ );
+ Assertions.assertEquals(expected, projectionMatch);
+
+
+ // partial interval does not align with projection granularity (and does
not contain data interval)
+ CursorBuildSpec cursorBuildSpecPartialInterval = CursorBuildSpec.builder()
+
.setInterval(partial)
+
.setPhysicalColumns(Set.of("a"))
+
.setGroupingColumns(List.of("a"))
+ .build();
+
+ projectionMatch = Projections.matchAggregateProjection(
+ spec.getSchema(),
+ cursorBuildSpecPartialInterval,
+ day,
+ new RowSignatureChecker(baseTable)
+ );
+ Assertions.assertNull(projectionMatch);
+
+ // interval larger than data interval can match, though this shouldn't
really happen in practice
Review Comment:
not sure i follow, why would it not happen in practice? i thought the idea
is that if we have daily segments, and query on partial+day1+day2+partial, we
can still use projection on day1 and day2.
##########
processing/src/main/java/org/apache/druid/segment/projections/Projections.java:
##########
@@ -117,6 +121,11 @@ public static ProjectionMatch matchAggregateProjection(
if
(CollectionUtils.isNullOrEmpty(queryCursorBuildSpec.getPhysicalColumns())) {
return null;
}
+
+
+ if (isUnalignedInterval(projection, queryCursorBuildSpec, dataInterval)) {
Review Comment:
nit: additiona line break.
also in line 382, it only checks for virtual column that only have __time as
required input, but there can be multiple required inputs and __time being one
of them, e.x. concat(column1, __time), in this case, granularity is not
checked. maybe we should update the condition to be
`requiredInputs.contains("__time")`
##########
processing/src/main/java/org/apache/druid/segment/projections/Projections.java:
##########
@@ -117,6 +121,11 @@ public static ProjectionMatch matchAggregateProjection(
if
(CollectionUtils.isNullOrEmpty(queryCursorBuildSpec.getPhysicalColumns())) {
return null;
}
+
+
+ if (isUnalignedInterval(projection, queryCursorBuildSpec, dataInterval)) {
Review Comment:
nit: additional line break.
also in line 382, it only checks for virtual column that only have __time as
required input, but there can be multiple required inputs and __time being one
of them, e.x. concat(column1, __time), in this case, granularity is not
checked. maybe we should update the condition to be
`requiredInputs.contains("__time")`
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
