FrankChen021 commented on code in PR #20377:
URL: https://github.com/apache/druid/pull/20377#discussion_r4056933657
##########
processing/src/main/java/org/apache/druid/data/input/impl/BaseTableProjectionSpec.java:
##########
@@ -104,4 +109,40 @@ default boolean
hasEqualCompactionState(BaseTableProjectionSpec other)
{
return equals(other);
}
+
+ /**
+ * Validates that {@code queryGranularity} can be used by a base-table spec:
only period granularities in the UTC
+ * time zone are currently allowed.
+ */
+ static void validateQueryGranularity(Granularity queryGranularity, String
typeName)
+ {
+ if (!(queryGranularity instanceof PeriodGranularity periodGranularity)
+ || !DateTimeZone.UTC.equals(periodGranularity.getTimeZone())) {
+ throw InvalidInput.exception(
+ "Query granularity[%s] is not supported for [%s] base tables; only
period granularities in the UTC time"
+ + " zone are supported",
+ queryGranularity,
+ typeName
+ );
+ }
+ }
+
+ /**
+ * Validates a {@link Granularities#GRANULARITY_VIRTUAL_COLUMN_NAME}
supplied directly to a spec (catalog JSON, or
+ * a translated DDL body): it must decode to a granularity {@link
#validateQueryGranularity} accepts, so a
+ * spec cannot be constructed claiming a granularity that reads back as
something else.
+ */
+ static void validateGranularity(VirtualColumn granularityCarrier, String
typeName)
+ {
+ final Granularity granularity =
Granularities.fromVirtualColumn(granularityCarrier);
Review Comment:
P2 Require the reserved granularity carrier to read __time
**Finding:** validateGranularity decodes any timestamp_floor expression and
validates only the resulting PeriodGranularity; it never checks
granularityCarrier.requiredColumns(). A directly supplied base-table JSON or
catalog spec can therefore name an expression such as timestamp_floor(foo,
'P1D') as the reserved carrier, pass construction, and later make
DataSchema.queryGranularityFromSpec floor the ingested __time values even
though the declared granularity is based on another column. This silently
changes stored time bucketing and bypasses the equivalent required-column check
in ProjectionSpecTranslator.
**Suggestion:** Reject carriers whose requiredColumns() is not exactly the
singleton __time column before calling Granularities.fromVirtualColumn,
matching ProjectionSpecTranslator's validation.
--
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]
