FrankChen021 commented on code in PR #20379:
URL: https://github.com/apache/druid/pull/20379#discussion_r4325147038
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java:
##########
@@ -797,16 +832,72 @@ private static DataSchema createDataSchema(
@Nonnull ClientCompactionTaskGranularitySpec granularitySpec,
@Nullable List<AggregateProjectionSpec> projections,
@Nullable BaseTableProjectionSpec baseTable,
+ boolean sealed,
+ boolean rangePartitioned,
boolean needMultiValuedColumns
)
{
if (baseTable != null) {
// BaseTable mode: the spec owns dimensions/metrics and (via its
query-granularity virtual column) query
// granularity; segment granularity + intervals are the compaction
config's. Attach the config's query
// granularity up front (a no-op if absent/NONE); the generator's
incremental index applies it at build time.
- // Existing-segment analysis (dims/metrics/rollup inference) is bypassed
— the spec is authoritative.
- final BaseTableProjectionSpec baseTableSpec =
-
baseTable.withQueryGranularity(granularitySpec.getQueryGranularity());
+ // A sealed spec is the complete description of the table, so the
columns and projections it leaves undeclared
+ // are taken to not exist rather than inferred. Range partitioning is
the exception that reads the segments even
+ // when sealed: only they can say whether a partition column is
multi-valued, which MSQ cannot partition by.
+ //
+ // Which string columns actually hold multiple values is likewise only
knowable from the segments, and a caller
+ // that does not get told assumes they all might: MSQ groups every
string column as an array and converts it
+ // back on write, which can rewrite a single-valued column as something
else. So the answer is produced whenever
+ // it was asked for and the segments are being read anyway. A sealed
table without range partitioning is not
+ // read, and keeps that conservative assumption rather than paying for a
download to refine it.
+ final boolean analyzeSchema = !sealed;
+ final boolean analyzeProjections = !sealed && projections == null;
+ final boolean analyzeMultiValued = rangePartitioned ||
(needMultiValuedColumns && analyzeSchema);
Review Comment:
[P2] Resolved in the reviewed head. Sealed base-table compaction now passes
an explicit empty multi-valued-dimension set, so MSQ does not add array
conversion virtual columns for scalar strings. Actual multi-valued data fails
instead of being silently rewritten. No additional actionable issues were found.
<!-- mergelens:review -->
--
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]
