feat: add plain table base table spec for catalog wiring

[clintropolis]

Description

Follow-up to #19830, this adds a plain table BaseTableProjectionSpec, the API side companion of the internal v10 segment metadata TableProjectionSchema that we store internally for non-rollup tables. This is mainly going to be useful for catalog and compaction uses since it will allow catalog compaction to always be able to compute a baseTable spec (in a follow-up PR), but it also works anywhere a baseTable is accepted (such as DDL statement base table projection definitions, various ingest specs, etc).

This PR also relaxes a restriction on the DDL projection statements to allow any type of VirtualColumn to be defined (instead of just ExpressionVirtualColumn) by adding a new supportsOutputNameRewrite/withOutputName functions to let the projection spec match declared names.

A follow-up PR will add a spec for rollup tables.

[FrankChen021]

🟡 Changes recommended

The new plain-table base spec is not ready to merge because valid duration granularities are accepted but cannot round-trip through the carrier used by ingestion, and arbitrary virtual columns can masquerade as the reserved carrier name and alter bucketing silently.

Reviewed 25 of 25 changed files; findings are in 1 file.

Validation run: git diff --check passed. No builds, tests, dependency installs, or formatters were run, per the static-review scope.

Severity	Findings
P0	0
P1	1
P2	1
P3	0
Total	2

---

This is an automated review by Codex GPT-5.6-Luna(max)

After addressing the findings or replying to the comments, you can request another review from me to trigger a new automated review.

[FrankChen021]

[P1] [P1] Preserve duration query granularities

Finding: When the caller supplies a valid DurationGranularity, this stores the result of Granularities.toVirtualColumn(), which represents duration granularities as an identifier-only __time virtual column. The corresponding getQueryGranularity()/DataSchema decode path uses Granularities.fromVirtualColumn(), which returns NONE for that form, so plain-table ingestion and compaction silently lose the requested duration/origin bucketing and persist a spec that no longer represents the configured granularity.

Suggestion: Encode duration granularities in a reversible carrier representation, or reject unsupported granularity implementations before constructing the spec instead of accepting and silently degrading them.

[FrankChen021]

[P2] [P2] Validate the carrier expression, not only its name

Finding: A JSON/API caller can supply any VirtualColumn named __virtualGranularity and this validation accepts it. The shared decoder interprets an identifier such as otherColumn as Granularities.ALL (and other expressions as NONE), so a malformed or forged carrier can silently change timestamp bucketing or disable it even though the spec claims to carry query granularity.

Suggestion: Require the carrier to have the expected __time dependency and a supported granularity expression, rejecting unrecognized or ALL results during spec construction.

[FrankChen021]

🟡 Changes recommended

The prior review's duration-granularity finding is resolved at the current head: unsupported duration and non-UTC period granularities are rejected, and I rechecked the current constructor and ingestion paths. One current-head correctness issue remains: the reserved carrier validator decodes a timestamp-floor expression without enforcing that it reads __time, so a directly supplied base-table JSON or catalog spec can claim a granularity based on another column and still cause ingestion to floor __time using that granularity. This should be fixed before merging.

Reviewed 27 of 27 changed files; the finding is in 1 file.

Validation: git diff --check 9f5363cc5f373120493a3552b503a973ca9e5b3b 89168c581bcdc30001c002ba9372540f475fb716 passed. No builds, tests, dependency installs, or formatters were run; this was a static review.

Severity	Findings
P0	0
P1	0
P2	1
P3	0
Total	1

---

This is an automated review by Codex GPT-5.6-Luna(max)

After addressing the findings or replying to the comments, you can request another review from me to trigger a new automated review.

[FrankChen021]

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.

[FrankChen021]

🟡 Changes recommended

Reviewed 31 of 31 changed files.

This updated-head review started with the incremental diff, then rechecked the full current diff and surrounding planner, virtual-column, catalog, and ingestion code. The two prior findings about unsupported duration/non-UTC query granularities and carrier validation against a non-time input are resolved at the current head.

Validation: git diff --check 9f5363cc5f373120493a3552b503a973ca9e5b3b 8890ea854c4e416702f8276d8638fa3d85684918 passed. No broad builds or test suites were run; this was a static review.

Severity	Findings
P0	0
P1	0
P2	1
P3	0
Total	1

---

This is an automated review by Codex GPT-5.6-Luna(max)

After addressing the findings or replying to the comments, you can request another review from me to trigger a new automated review.

[FrankChen021]

P2 Specialized computed columns lose dependencies

Finding: When a clustered __base DDL expression is composed from planner-specialized virtual columns, liftComputedColumns records only the selected output virtual column. The ScanQuery can contain intermediary virtual columns recursively (for example, a nested JSON expression can produce an outer NestedFieldVirtualColumn that reads a NestedObjectVirtualColumn which reads an inner NestedFieldVirtualColumn), but those dependencies are not copied into the materialized base-table spec. ClusteredValueGroupsBaseTableProjectionSpec.validateVirtualColumns then sees the renamed output still reading synthetic vN names that are neither stored columns nor virtual columns, so valid composed expressions are rejected during DDL translation.

Suggestion: Retain the full dependency closure from the planned ScanQuery when lifting a computed output, keeping intermediary virtual columns under their synthetic names while renaming only the materialized root (or rewrite the root into a self-contained expression). Add a clustered base-table DDL test for a nested specialized expression composition.

[FrankChen021]

🟢 Approval recommended

This updated-head review began with the incremental diff from the previously reviewed commit 8890ea854c4e416702f8276d8638fa3d85684918 to the current head e9a87f321927668ee6c2b25a634fe15b6812618d, then rechecked the full merge-base diff and the surrounding planner, virtual-column, catalog metadata, serialization, and ingestion paths. The current head carries the transitive virtual-column dependency closure for composed clustered computed columns, and I found no remaining actionable correctness, compatibility, security, lifecycle, or data-loss issue requiring a pre-merge fix.

Reviewed 31 of 31 changed files.

Validation: git diff --check 9f5363cc5f373120493a3552b503a973ca9e5b3 e9a87f321927668ee6c2b25a634fe15b6812618d and the corresponding incremental range check passed. No builds, tests, dependency installs, or formatters were run; this was a static review.

---

This is an automated review by Codex GPT-5.6-Luna(max)

[capistrant]

one non-blocking nit on a test for you to consider. thanks for working on the continued progress in this area!

[capistrant]

nit: other impls added equiv key validation post rename as part of testing rename

[FrankChen021]

🟢 Approval recommended

The update strengthens the output-name rewrite test by checking that the renamed expression virtual column retains the original equivalence key. The assertion matches the interface contract and implementation, and retains the existing check of the renamed column's expression and output type. No actionable issues were found in this incremental review.

Review mode: updated_since_review. Reviewed 1 of 1 files changed since the previous review, plus 3 supporting source files (4 files total):

• processing/src/test/java/org/apache/druid/segment/virtual/ExpressionVirtualColumnTest.java
• processing/src/main/java/org/apache/druid/segment/virtual/ExpressionVirtualColumn.java
• processing/src/main/java/org/apache/druid/segment/VirtualColumn.java
• sql/src/main/java/org/apache/druid/sql/calcite/planner/ProjectionSpecTranslator.java

Coverage includes the relevant merge-base diff hunks, rename implementation, equivalence-key comparison, and planner caller. The remaining files in the full PR were not re-reviewed this turn; the incremental diff contains no production-code changes. This recommendation covers the update since the prior review.

Validation: git diff --check e9a87f321927668ee6c2b25a634fe15b6812618d 055e3e2b3c5da0bd239bf8d729140556c7fafec8 passed, and the prepared worktree HEAD matches the packet. Static review only; no builds or tests were run.

---

This is an automated review by Codex GPT-5.6-Luna(max)