Repository navigation
[fix](fe) Resolve DEFAULT(column) on Iceberg tables from the write schema - #69488
morningman merged 7 commits into
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
/review |
|
run buildall |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed exact head bda3e28cdab081693dc2dd8d739d784005b7ce9d against the supplied authoritative aggregate patch. Five new verified findings: one P1 and four P2, all inline. No existing review threads or blocking comments were present on refreshed intake. No additional user focus was supplied.
Critical checkpoint conclusions:
- Goal and proof: ordinary INSERT VALUES/SELECT and UPDATE/MERGE predicates now consult pinned write defaults; the inline findings show incorrect typed-expression results and uncovered statement/schema paths. Added tests exercise the ordinary cases but do not prove the broader behavior.
- Scope, clarity, reuse: the capability-based change is focused and connector-agnostic. Reusing the default resolver loses referenced-column typing and reuses a narrower assignment-name validator.
- Concurrency and locks: no new threads or locks. Existing table read-lock/finally-unlock structure remains. Independently cached read schemas versus live writer schemas expose the reported type-evolution issue; no separate lock-order/deadlock defect found.
- Lifecycle and initialization: write schemas/identities remain statement-scoped; INSERT and row-level command pinning, retry/prepared-execution reset, plugin metadata scope and classloader boundaries were traced. No new lifetime/static-initialization defect found. Overwrite repinning behavior also exists for preexisting bare DEFAULT and is not a new finding here.
- Configuration: no configuration items added.
- Compatibility: the enum changes the plugin surface; the deliberate major API bump to 20.0, version pin and plugin-surface baseline agree. Metadata method signatures are unchanged. No new wire or storage format is introduced.
- Thrift/protobuf IDs: no gensrc definitions changed; branch field-ID checks are not applicable.
- Parallel paths: INSERT/OVERWRITE/EXPLAIN, UPDATE/MERGE/DELETE and CHANGELOG/POSITION_DELETE paths were traced. Row-level commands pin before planning. Aggregate/CTE/subquery normalization and snapshot Hive capability inheritance are incomplete as reported.
- Conditional checks: capability gates preserve ordinary Hive/Paimon/internal behavior; table-ID plus field-ID matching is consistent. Fully qualified target names are wrongly rejected as reported. A differing source alias is not raised because the PR deliberately specifies target-name semantics.
- Tests and modularity: added JUnit 5 cases cover direct VALUES/projection/no-FROM, unknown names, bound pin/absent pin, capability-disabled behavior and ordinary sibling delegation. Missing coverage includes typed nested expressions, grouped/nested queries, full qualification, evolved read/write types and snapshot sibling delegation.
- Changed expected results: the ordered Parquet/ORC rows 20-22 and expected SELECT rejection match the added statements and existing default value 35. Result-file generation provenance was not independently verified.
- Observability: existing analysis errors and statement tracing suffice for this planning change; no new distributed operation or metrics requirement identified.
- Persistence and failover: no EditLog/replay or durable metadata mutation protocol changes.
- Writes, atomicity and crashes: planning precedes existing writer transaction/finalization/commit fences. No new commit/rollback or crash-cleanup defect found; the P1 expression issue can persist the wrong computed value in an otherwise valid write.
- FE/BE state: no new transmitted variables or serialization/send paths. Integer-width function semantics were cross-checked in FE and BE for the P1 finding.
- Performance: traversal and default lookup add planning work proportional to query size and target width. No substantiated material CPU/memory regression or repeated external-I/O defect found.
- Other invariants: no BE allocation ownership, partition visible-version, or MoW delete-bitmap changes. Errors propagate through existing analysis exceptions. Stable connector field IDs and writer metadata fences remain in place, with the reported default-type caveat.
Validation was static only, as required: all 15 changed files, related callers/control flow, tests and expected outputs were inspected; no build, runtime test, source change, or format rewrite was performed. No unresolved candidate or assigned static coverage gap remains. Runtime execution and output-generation provenance remain unverified. Convergence and review severity are reported separately by the delivery helper.
Review converged after 2 round(s).
| if (!writeOnlyDefaultColumns.isPresent()) { | ||
| return value; | ||
| } | ||
| return (NamedExpression) ConnectorWriteSchemaUtils.resolveDefaultReferences(value, |
There was a problem hiding this comment.
[P1] Preserve the referenced column type before binding its consumers
For an Iceberg t(c INT) with write-default -1, INSERT INTO t(c) SELECT bit_count(DEFAULT(c)) should evaluate bit_count on an INT and write 32. This new route instead produces OneRow(bit_count(-1:TINYINT)): ConnectorWriteSchemaUtils.resolveDefaultReferences parses the default SQL without casting it to the referenced column type, and the parser chooses TINYINT for -1. BitCount has width-specific signatures, and both its FE evaluator and BE implementation return 8 for TINYINT -1, versus 32 for INT -1. Casting the completed expression to the sink's INT cannot restore the lost value; VALUES expressions have the same problem. Cast each substituted default to its referenced writer column's type before binding surrounding operators/functions, as the bound DEFAULT rewrite does.
There was a problem hiding this comment.
Fixed in cf75568. ConnectorWriteSchemaUtils.resolveDefaultReferences now casts each resolved default to the type of the writer column it references (TypeCoercionUtils.castUnbound) before the expression around it is bound, so bit_count(DEFAULT(c)) on an INT column with write-default -1 gets an INT -1 in VALUES and in SELECT. The external UPDATE / MERGE / changelog paths use the same helper and get the same typing. branch-4.2 is not affected: it builds write defaults as literals of the column type. Test: InsertUtilsTest#writeOnlyDefaultIsTypedAsItsColumnBeforeBinding (VALUES and SELECT); with the previous commit the argument of bit_count is TinyIntLiteral(-1).
| */ | ||
| private static Plan resolveSelectDefaultReferences(Plan query, Optional<List<Column>> writeOnlyDefaultColumns, | ||
| TableIf table, List<String> targetNameParts) { | ||
| return query.rewriteUp(plan -> { |
There was a problem hiding this comment.
[P2] Normalize defaults in every SELECT output shape and nested query
With target t(c INT) having a write-default, INSERT INTO t(c) SELECT DEFAULT(c) FROM (SELECT 1 AS k) s GROUP BY k parses as Sink(t.c) -> Aggregate(groupBy=k, output=DEFAULT(c)) -> Alias(s) -> OneRow(k=1). The lambda handles only Project/OneRow outputs, so it leaves this DEFAULT untouched and binding fails with unknown column c against source s.k; removing GROUP BY resolves the same target reference successfully. CTE definitions and scalar-subquery plans also escape this traversal: command-level WITH is attached after normalization, CTE definitions are extraPlans(), and subquery plans are not expression children. Resolve target defaults in aggregate/repeat outputs and explicitly traverse these separately owned query plans, with grouped/CTE/subquery coverage.
There was a problem hiding this comment.
Not changing this in this PR. The INSERT ... SELECT resolution covers the select lists of the main query (projects and the one-row relation), the same shapes as branch-4.2, which this PR ports (InsertUtils#resolveIcebergSelectDefaultReferences there). In an aggregate output, a WITH clause or a subquery, DEFAULT(c) keeps its meaning from before this PR and names a column of its own FROM clause, as in MySQL: with no such column the statement fails with an unknown-column error; a column of the target table itself gets the pinned writer default through the bound rewrite; a column of another Iceberg table now fails with "must reference a column of the table that this statement writes" instead of answering NULL; a column of a Hive or OLAP source gets that column's default, as before. Resolving target names in those shapes needs other analysis stages (the command-level WITH is attached after normalizePlan; CTE and subquery plans are not children of the project), so it would be a separate change.
There was a problem hiding this comment.
Correction to my reply above: for a derived table in FROM, "keeps its meaning from before this PR" did not hold. The walk descended into it and resolved its DEFAULT(c) against the target, as #69488 (comment) found. 724071d stops the walk there, so a derived table now behaves as described above; aggregate outputs, WITH and scalar subqueries are unchanged by that commit.
| return value; | ||
| } | ||
| return (NamedExpression) ConnectorWriteSchemaUtils.resolveDefaultReferences(value, | ||
| writeOnlyDefaultColumns.get(), (ExternalTable) table, ConnectContext.get(), targetNameParts, null); |
There was a problem hiding this comment.
[P2] Accept fully qualified target column references
INSERT INTO ctl.db.t(c) SELECT DEFAULT(ctl.db.t.c) now reaches ConnectorWriteSchemaUtils.resolveDefaultReferences, which passes the four-part name to UpdateCommand.checkAssignmentColumn. That assignment-LHS validator accepts only one, two, or three name parts and throws column in assignment list is invalid for this unambiguous target reference. Ordinary Nereids column binding explicitly supports catalog.db.table.column, and this form should also resolve against the pinned target schema. Use qualification validation that includes the catalog component rather than applying the narrower UPDATE-assignment grammar; cover the fully qualified form in SELECT and VALUES.
There was a problem hiding this comment.
Not changing this here. DEFAULT(...) in VALUES / SELECT accepts the qualifications that the existing Iceberg UPDATE / MERGE DEFAULT handling accepts, through the same UpdateCommand.checkAssignmentColumn (branch-4.2 does the same): c, t.c, db.t.c. ctl.db.t.c fails with "column in assignment list is invalid" instead of resolving; it does not write a wrong value. Accepting catalog-qualified names would change the assignment-column rule of all DML paths, so it belongs in its own change.
| if (originalTable.filter(PluginDrivenExternalTable.class::isInstance) | ||
| .filter(table -> ((PluginDrivenExternalTable) table).hasWriteOnlyDefaults()) | ||
| .isPresent()) { | ||
| column = pinnedWriteColumn(context.cascadesContext.getStatementContext(), originalTable.get(), column) |
There was a problem hiding this comment.
[P2] Derive the result type after selecting the writer column
targetType is computed from the read column before this substitution and remains unchanged afterward. The read schema can still be cached as INT after an external Iceberg promotion to LONG, while pinAndGet independently loads current LONG writer columns and a new write-default such as 4900000001. Then UPDATE t SET other = 'x' WHERE DEFAULT(score) = 4900000001 produces Filter(CAST(4900000001 AS INT) = 4900000001) above the cached INT scan, instead of using the BIGINT writer default, causing overflow/error or an incorrect predicate. The writer fences compare the current writer columns/identity and do not validate this stale predicate type. Compute the type from the selected writer column and add a same-field-ID read-INT/write-BIGINT test.
There was a problem hiding this comment.
Fixed in cf75568. RewriteDefaultExpression now takes the result type from the pinned writer column, after the substitution. Test: RewriteDefaultExpressionTest#writeOnlyDefaultTakesTheTypeOfThePinnedWriterColumn pins a BIGINT writer column with default 4900000001 over an INT read column with the same field id and expects a BIGINT literal; with the previous commit the result is cast(4900000001 as INT).
There was a problem hiding this comment.
Follow-up: superseded in 688e8d5 (see the newer thread on this file). Typing the substituted default as the writer column fixed the DEFAULT node itself but not the expressions bound around it against the read type, so a writer column whose type differs from the read column now fails with a refresh hint instead.
| ConnectorCapability.SUPPORTS_STORAGE_PREDICATE_PRUNING, | ||
| ConnectorCapability.SUPPORTS_NESTED_COLUMN_SCHEMA_CHANGE)); | ||
| ConnectorCapability.SUPPORTS_NESTED_COLUMN_SCHEMA_CHANGE, | ||
| ConnectorCapability.SUPPORTS_WRITE_ONLY_DEFAULTS)); |
There was a problem hiding this comment.
[P2] Reflect this capability for snapshot schemas as well
The two-argument getTableSchema reflects this set, but the snapshot overload still returns the sibling schema directly. For SELECT DEFAULT(c) FROM hms_catalog.db.iceberg_t FOR VERSION AS OF <snapshot_id>, PluginDrivenMvccExternalTable.loadSnapshot publishes that unreflected historical schema. tableCapabilities() reads the ambient pinned schema, while the Hive catalog itself does not declare write-only defaults, so hasWriteOnlyDefaults() is false and the bound rewrite returns NULL for a nullable column instead of rejecting this read-only use. Direct Iceberg catalogs retain the guard through their connector-wide capability. Apply the same reflection to getTableSchema(session, handle, snapshot) and test that overload.
There was a problem hiding this comment.
Not in this PR. HiveConnectorMetadata#getTableSchema(session, handle, snapshot) returns the sibling's schema without reflectSiblingCapabilities for every inherited capability, not only this one (auto-analyze, Top-N lazy materialization, nested column prune, field-id access path, storage predicate pruning, nested column DDL); that predates this PR. So for a time-travel read of an iceberg-on-HMS table, SELECT DEFAULT(c) ... FOR VERSION AS OF keeps master's current answer (NULL) instead of the new error; nothing is written. Reflecting capabilities on the snapshot path changes scan planning for all of them and should come with its own tests, so it is left to a separate change.
There was a problem hiding this comment.
Correction to my reply above. Since #69299 (merged on 10-26, after that reply), the latest Iceberg pin retains its schema, so fe-core publishes the schema from getTableSchema(session, handle, snapshot) for every statement that pins the table, not only for time travel. On an iceberg-on-HMS table that overload returns the sibling schema without reflectSiblingCapabilities, so hasWriteOnlyDefaults() is false in those statements: DEFAULT(c) in UPDATE WHERE / MERGE ON still evaluates to NULL and changes no row, and SELECT DEFAULT(c) still returns NULL, the same results as master today. Direct Iceberg catalogs are not affected, because the Iceberg connector declares the capability connector-wide.
From the code, the overload leaves out every capability the Hive connector reflects from the sibling, not only this one (auto and sampled analyze, Top-N lazy materialization, nested column pruning, field-id access paths, storage predicate pruning, nested column DDL). Reflecting them in the snapshot overload changes planning for all of them, so it will come in its own PR with its own tests, after this one. The PR description now states the iceberg-on-HMS limitation.
There was a problem hiding this comment.
Follow-up: fixed in #69999. The snapshot overload of getTableSchema now reflects the same inherited capabilities as the two-argument one (both go through one helper), so an iceberg-on-HMS table keeps them in every statement that pins a snapshot: explicit time travel, and since #69299 the latest-snapshot pin as well. Checked on a local cluster against a Hive 3.1.2 metastore: through the HMS catalog, SELECT DEFAULT(d) FROM t FOR VERSION AS OF <snapshot> now fails with DEFAULT(d) must reference a column of the table that this statement writes, as it does through an Iceberg catalog, and UPDATE ... WHERE d = DEFAULT(d) and MERGE ... ON ... AND t.d = DEFAULT(t.d) now update the rows.
TPC-H: Total hot run time: 29120 ms |
TPC-DS: Total hot run time: 151076 ms |
ClickBench: Total hot run time: 24.29 s |
bda3e28 to
cf75568
Compare
|
run buildall |
|
/review |
There was a problem hiding this comment.
Static review of all 16 changed paths at cf75568, including related binding, writer-schema, capability, and commit paths. One new P2 finding: the bound DEFAULT rewrite can widen its result after an enclosing IF has retained an INT signature, causing a valid BIGINT default to become NULL or overflow during folding.
The existing P1 4355508072 (parsed default width before binding) and the direct DEFAULT result-type instance in P2 4355508096 are fixed at this head. No existing P0/P1 remains applicable. Qualification (4355508088), grouped/CTE/subquery traversal (4355508080), and HMS historical-schema capability reflection (4355508100) remain covered by their existing threads and replies; they are not reposted. The new finding concerns a different narrowing cast synthesized later by the enclosing IF consumer.
Critical checkpoint conclusions:
- Goal and proof: writer-only defaults now use the target's pinned schema in the covered INSERT and row-level paths, and read-only use is rejected. Added tests cover these cases, but the enclosing-consumer promotion case in the inline finding remains incorrect.
- Scope and clarity: changes are localized and reuse the capability accessor, statement schema pin, and shared default resolver.
- Concurrency and locks: no new threads, shared mutators, locks, or lock order; existing statement ownership and connector generation fences remain in use. No new heavy work inside a newly introduced lock was found.
- Lifecycle and initialization: writer columns/identity are pinned before normalization or synthesis, selected by target table and field ID, and cleared across retries/executions. Branch selection remains in the existing writer scope. No static initialization dependency is introduced.
- Configuration: no configuration item is added.
- Compatibility: connector API major and test pin move from 20.0 to 21.0 with the new enum surface entry. Metadata method surface is unchanged. No storage-format or FE/BE protocol change.
- Thrift/protobuf: no gensrc definitions are changed.
- Parallel paths: inspected VALUES, SELECT projects/one-row plans, INSERT OVERWRITE, UPDATE/MERGE assignments and predicates, changelog helper consumers, and direct/HMS Iceberg delegation. Existing traversal/qualification/snapshot limits are identified above.
- Conditions and errors: capability gates preserve other connectors' behavior; absent matching target/field and invalid references fail explicitly. Optional absent Iceberg defaults serialize as SQL NULL; required absent defaults fail.
- Test coverage: positive/negative FE tests, capability/SPI tests, and Parquet/ORC regression additions cover the intended paths. Consumer binding plus rewrite under a stale read type needs coverage as requested inline. Tests were inspected, not executed.
- Expected results: both new ordered regression blocks match rows 20-22, default35, UPDATE labels and the MERGE change to row22. Result-generation provenance was not independently verified.
- Observability: existing analysis errors identify the DEFAULT reference/column; no new operational metric or log is needed for these local substitutions.
- Transactions and persistence: no EditLog, replay, failover or persistence algorithm changes; existing writer schema/identity checks and commit fences remain responsible for remote metadata races.
- Writes and atomicity: existing row-level transaction/commit machinery is reused. The inline finding concerns an incorrect pre-write predicate or analysis failure; no new crash/resource/partial-commit mechanism was identified.
- FE/BE variables: no new transported variable or sending path.
- Performance and memory: planning work is linear in visited plan/expressions with a writer-column lookup per DEFAULT; no new per-row runtime work or substantiated performance defect. No BE memory ownership, partition visible-version or MoW delete-bitmap code changes.
- Other issues and user focus: no additional user focus was supplied. All initial risks and candidates are accepted, duplicate-fenced or dismissed with recorded evidence.
Coverage limits: static inspection only, as required; no builds, unit/regression execution, live external-cluster behavior, or output-generation provenance were validated. No unresolved static candidate or unreviewed changed path remains. Convergence is reported separately by the delivery metadata.
Review converged after 2 round(s).
| } | ||
| // The pinned writer column can be wider than the read column the slot was bound with, e.g. after a type | ||
| // promotion committed outside Doris, and the default is a value of the writer column. | ||
| DataType targetType = DataType.fromCatalogType(column.getType()); |
There was a problem hiding this comment.
[P2] Bind enclosing consumers against the writer column type
With a cached read score INT and a same-field-ID writer score BIGINT whose write-default is 4900000001, consider:
UPDATE t SET score = DEFAULT(score)
WHERE IF(TRUE, DEFAULT(score), 0) = 4900000001;Binding produces Filter(CAST(IF(TRUE, DEFAULT(score:INT), 0):INT AS BIGINT) = 4900000001) -> Scan(score:INT). This rewrite correctly replaces DEFAULT with a BIGINT literal, but Rewriter preserves function signatures, so If.withChildren/FunctionParams retain IF's INT result. FoldConstantRuleOnFE.visitIf then calls ensureSameResultType on the true branch and synthesizes CAST(4900000001 AS INT): loose casting yields NULL and updates no rows, while strict casting throws overflow. The predicate should be true for every row.
The earlier direct DEFAULT result-type issue is fixed here; this narrowing cast is created separately by the already-bound IF consumer. Resolve the pinned writer type before binding its consumers, or rebind their signatures/coercions after substitution, and cover an enclosing IF through both analysis and rewrite.
There was a problem hiding this comment.
Fixed in 688e8d5. Agreed: once IF, CASE or COALESCE have been bound against the read type, a wider writer-typed default cannot be substituted from the DEFAULT node, because the rewriter keeps their signatures and constant folding narrows the value back. The read and writer types of one field only differ while Doris's cached read schema predates a type change committed outside Doris (reads go through the table cache, the writer schema is loaded for the statement), so the bound rewrite now fails such a DEFAULT and asks to refresh the table instead of retyping its consumers. When the two types agree, which is every case without an outside type change, nothing changes. DEFAULT in VALUES, in INSERT ... SELECT projections and in UPDATE SET is resolved against the writer schema before binding, so its consumers already bind against the writer type. RewriteDefaultExpressionTest covers a direct DEFAULT and one under an IF bound to INT; both are rejected.
TPC-H: Total hot run time: 28331 ms |
TPC-DS: Total hot run time: 150589 ms |
ClickBench: Total hot run time: 24.43 s |
FE Regression Coverage ReportIncrement line coverage |
cf75568 to
f1287fd
Compare
|
run buildall |
TPC-H: Total hot run time: 28559 ms |
TPC-DS: Total hot run time: 151566 ms |
ClickBench: Total hot run time: 24.6 s |
FE UT Coverage ReportIncrement line coverage |
|
/review |
There was a problem hiding this comment.
Reviewed exact head 688e8d59412ab4269516ff421a22015d5df4e281 across all 17 changed files and related planning/write paths. No new inline findings. Existing P1 #4355508072 is fixed: each substituted default is typed as its referenced writer column before consumer binding. Existing type-drift findings #4355508096 and #4358048801 are fixed by rejecting mismatched read/writer types with a refresh hint.
Known P2 limits remain covered by existing threads, without duplicate comments: #4355508080 (Aggregate/Repeat outputs and separately owned CTE/scalar-subquery plans), #4355508088 (four-part target qualification), and #4355508100 (historical Iceberg-on-HMS capability reflection). These are verified dispositions, with no unresolved new candidate. No existing P0/P1 finding still applies. No additional user focus was supplied.
Critical checkpoint conclusions:
- Purpose and proof: ordinary VALUES/SELECT projections and bound-target UPDATE WHERE/MERGE ON now use the pinned writer default. Inspected unit/regression additions cover these paths; the known scope limits above remain.
- Focus and reuse: the change is localized and reuses the capability union, pinned writer schema, shared DEFAULT resolver and coercion helpers; generic planning stays connector-agnostic.
- Concurrency and locks: no new threads, lock acquisitions or shared mutable state; rewriting consumes the existing statement pin in the established planning lock scope. No new lock-order or concurrency defect found.
- Lifecycle: INSERT, overwrite and row-level writes pin before normalization; branch selection survives overwrite's inner INSERT. Prepared execution/retry reset clears writer schemas and identities. No new static-initialization or resource lifecycle.
- Configuration: no configuration change.
- Compatibility: the added capability is recorded in the plugin surface and the API major is consistently 21.0. ConnectorMetadata methods and storage/wire formats are unchanged.
- Thrift/protobuf: no definitions changed; cross-branch field-ID comparison is inapplicable.
- Parallel paths: explicit/implicit VALUES columns, FROM/no-FROM projections, overwrite/EXPLAIN, UPDATE, MERGE and shared changelog callers were traced. Plain Hive/Paimon/internal paths retain their existing non-opt-in behavior.
- Conditions and failures: missing target fields and mismatched writer types fail with context. Stable field-ID lookup avoids selecting a replacement field solely by reused name; target-name SELECT semantics are explicit and tested.
- Test coverage: inspected positive/negative tests cover capability declaration/reflection, writer-versus-read defaults, unknown columns, non-target rejection, non-opt-in behavior, width-sensitive numeric typing, and direct/enclosing type drift. End-to-end additions cover Parquet and ORC.
- Expected results: the new ordered rows 20/21/22 with default_int=35 and UPDATE/MERGE strings agree with the fixture and statements by inspection. Output-generation provenance and runtime passing status were not independently verified.
- Observability: analysis errors identify the DEFAULT reference or conflicting types and refresh action. No new distributed operation needs additional metrics/logging.
- Persistence and failover: no EditLog, replay or transaction-state change; existing writer metadata-identity/schema fences and commit paths remain in use.
- Writes and atomicity: changed expressions flow into existing analyzed connector sinks; no new commit, crash cleanup or partial-write mechanism. No new atomicity issue found.
- FE/BE propagation: no new transmitted variable or operator.
- Performance: one plan traversal and bounded linear column lookup per DEFAULT; no new per-row evaluation or concrete significant regression found.
- Core invariants and remaining issues: no new visibility-version, delete-bitmap or native memory-ownership path; errors fail analysis before execution. Final recheck found no additional substantiated issue beyond the existing threads.
Validation was static-only: no builds, tests, cluster execution or source modifications. Runtime behavior, full plugin packaging and test-generation provenance remain outside this invocation's validation.
Review converged after 2 round(s).
ClickBench: Total hot run time: 24.58 s |
…hema ### What problem does this PR solve? Issue Number: None Related PR: apache#66538, apache#57531, apache#65851 Problem Summary: An Iceberg write-default exists only in the writer schema that a write statement pins (IcebergWriteSchemaContext); the columns of the read schema carry no default. Master resolves DEFAULT(column) against that writer schema only in UPDATE / MERGE SET and in MERGE INSERT VALUES, and everywhere else falls back to the read column. On a v3 table whose column d has write-default 42, INSERT ... VALUES (DEFAULT(d)) and INSERT ... SELECT DEFAULT(d) without FROM fail with "Unknown column", INSERT ... SELECT DEFAULT(d) FROM t writes NULL, UPDATE ... WHERE d = DEFAULT(d) and MERGE ... ON ... d = DEFAULT(d) compare d with NULL and silently match no row, and SELECT DEFAULT(d) FROM t returns NULL. branch-4.1 and branch-4.2 (apache#66538) resolve all of these against the pinned write target, by name against the target in INSERT ... SELECT, and reject DEFAULT(column) on an Iceberg column in a statement that does not write the table. This change adds the connector capability SUPPORTS_WRITE_ONLY_DEFAULTS, declared by the Iceberg connector and inherited by iceberg-on-HMS tables. For a table that declares it, InsertUtils resolves DEFAULT(column) in INSERT VALUES and in the select lists of INSERT ... SELECT by name against the pinned writer schema, and RewriteDefaultExpression takes the default of a DEFAULT(column) bound to the table from the pinned writer column with the same field id, which covers the UPDATE and MERGE conditions, and rejects it when the statement pinned no writer schema for the table. Hive, Paimon and OLAP tables keep the current behavior. The new enum constant changes the connector plugin API surface, so the plugin API version goes from 26.0 to 27.0. ### Release note Fix: On Iceberg tables, DEFAULT(column) in INSERT VALUES, INSERT ... SELECT and the WHERE / ON conditions of UPDATE and MERGE now resolves to the column's write-default. Before, it failed, wrote NULL, or matched no row. SELECT DEFAULT(column) on an Iceberg table now fails instead of returning NULL. ### Check List (For Author) - Test: Regression test / Unit Test - test_iceberg_initial_defaults: writes rows 20-22 with DEFAULT(default_int) in VALUES and in INSERT ... SELECT with and without FROM, then updates them with UPDATE ... WHERE default_int = DEFAULT(default_int) and MERGE ... ON ... = DEFAULT(...), for Parquet and ORC; SELECT DEFAULT(default_int) must fail. Passes locally; with master's FE it fails at the first INSERT with "Unknown column 'default_int'" - InsertUtilsTest: VALUES and INSERT ... SELECT (from a staging relation and without FROM) resolve against the pinned writer schema; unknown column fails; a table without the capability is unchanged - RewriteDefaultExpressionTest: pinned writer column wins, no pinned schema fails, a table without the capability keeps the read default - IcebergConnectorTest, HiveConnectorMetadataSiblingDelegationTest, ConnectorPluginSurfaceTest - Behavior changed: Yes. See the release note. - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
### What problem does this PR solve? Issue Number: None Related PR: apache#66538 Problem Summary: Two review findings on the previous commit. ConnectorWriteSchemaUtils.resolveDefaultReferences parsed the default SQL of a writer column and substituted it without a cast, so the default kept the type the parser gives the literal: -1 is a TINYINT. For an INT column with write-default -1, INSERT INTO t(c) VALUES (bit_count(DEFAULT(c))) and INSERT INTO t(c) SELECT bit_count(DEFAULT(c)) evaluated bit_count on a TINYINT and wrote 8 instead of 32; casting the finished value to INT cannot restore that. The external UPDATE, MERGE and changelog paths use the same helper and typed DEFAULT(column) the same way. The resolved default is now cast to its column's type before the expression around it is bound, as RewriteDefaultExpression does for a bound DEFAULT(column). branch-4.2 builds write defaults as literals of the column type and is not affected. RewriteDefaultExpression took the result type from the read column before substituting the pinned writer column. After a type promotion committed outside Doris, the read schema can still hold an INT column while the writer column is a BIGINT whose default an INT cannot hold, such as 4900000001, and the default was cast to INT. The type now comes from the writer column the default is taken from. ### Release note None ### Check List (For Author) - Test: Unit Test - InsertUtilsTest#writeOnlyDefaultIsTypedAsItsColumnBeforeBinding: VALUES and SELECT pass DEFAULT(bits) to bit_count as an INT; with the previous commit it is a TINYINT - RewriteDefaultExpressionTest#writeOnlyDefaultTakesTheTypeOfThePinned WriterColumn: an INT read column with a BIGINT pinned writer column gives the BIGINT default; with the previous commit it is cast(4900000001 as INT) - InsertUtilsTest, RewriteDefaultExpressionTest, ConnectorWriteSchemaUtilsTest, ConnectorChangelogPlanBuilderTest, ChangelogRowLevelDmlTransformTest, ExternalRowLevelUpdatePlanBuilderTest, ExternalRowLevelMergePlanBuilderTest, PluginDrivenExternalTableTest: 93 tests pass - Behavior changed: No. DEFAULT(column) inside an expression of an external UPDATE / MERGE / changelog assignment now has the column's type, which is the type a bound DEFAULT(column) already had. - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
### What problem does this PR solve? Issue Number: None Related PR: apache#66538 Problem Summary: apache#69394 added two ConnectorWriteSchemaUtilsTest cases that expect resolveDefaultReferences to return the default with the type the parser gives its literal (9 is a TINYINT). The previous commit casts the resolved default to its writer column's type, so for the INT test column the result is now an INT literal 9, and both cases failed in FE UT once this branch met master. The two expectations now use the column's type. ### Release note None ### Check List (For Author) - Test: Unit Test - ConnectorWriteSchemaUtilsTest, RewriteDefaultExpressionTest, InsertUtilsTest - Behavior changed: No - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
… read type ### What problem does this PR solve? Issue Number: None Related PR: apache#66538 Problem Summary: Review follow-up. The previous commits typed the default that replaces a bound DEFAULT(column) as its pinned writer column. That type differs from the read column's only when the table changed outside Doris after Doris cached its read schema, e.g. an INT column promoted to BIGINT whose new write-default is 4900000001. The expressions around DEFAULT(column) were already bound against the read type and the rewriter keeps their signatures, so in UPDATE t SET ... WHERE IF(TRUE, DEFAULT(score), 0) = 4900000001 the IF keeps its INT result type and constant folding casts 4900000001 back to INT: NULL in non-strict mode, so no row matches, and an overflow error in strict mode. A substituted value cannot repair the types its consumers were bound with, so the rewrite now fails such a DEFAULT and asks to refresh the table. When the two types agree, which is every case without a type change made outside Doris, nothing changes. ### Release note None ### Check List (For Author) - Test: Unit Test - RewriteDefaultExpressionTest: a DEFAULT whose writer type differs is rejected, directly and under an IF bound to the read type - ConnectorWriteSchemaUtilsTest, InsertUtilsTest, ExternalRowLevelUpdatePlanBuilderTest, ExternalRowLevelMergePlanBuilderTest, ChangelogRowLevelDmlTransformTest, PluginDrivenExternalTableTest - Behavior changed: No - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
…T on its own FROM clause ### What problem does this PR solve? Issue Number: None Related PR: apache#69488 Problem Summary: For a target with write-only defaults (Iceberg), `InsertUtils.normalizePlan` resolves `DEFAULT(column)` in the select lists of an `INSERT ... SELECT` by name against the target's pinned writer schema, before the source relations are bound. It walked the whole query plan with `rewriteUp`, so it also rewrote the projections of a derived table in `FROM`, which is a query of its own. With a source `src(c INT DEFAULT 7)` and a target `t(c INT)` whose write-default is 9, INSERT INTO t(c) SELECT v FROM (SELECT DEFAULT(c) AS v FROM src) q wrote 9 instead of 7, and `DEFAULT(src.c)` inside the derived table failed with "column in assignment list is invalid", because its qualifier was checked against the target. branch-4.2's `resolveIcebergSelectDefaultReferences` walks the plan the same way. The walk now stops at `LogicalSubQueryAlias`. `DEFAULT(column)` inside a derived table is left to binding and names a column of that table's own `FROM` clause, as it does on master. The select lists of the INSERT's own query, set operation branches included, are resolved as before, still bottom-up. ### Release note None ### Check List (For Author) - Test: Unit Test - New `InsertUtilsTest#writeOnlyDefaultInDerivedTableNamesItsOwnSource` covers an unqualified and a qualified `DEFAULT(score)` inside a derived table (left to binding while the outer `DEFAULT(name)` resolves to the pinned default), and a UNION whose first branch resolves and whose second branch reads a derived table. It fails with the previous `rewriteUp` walk. - `InsertUtilsTest`, `ConnectorWriteSchemaUtilsTest`, `RewriteDefaultExpressionTest`, `IvmDeltaRewriterTest` and `IvmLinearDeltaHandlerTest` pass, as does the fe-core checkstyle. - Behavior changed: Yes. Within this PR, `DEFAULT(column)` in a derived table of an `INSERT ... SELECT` into an Iceberg table no longer resolves against the target; it keeps master's meaning. - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
…ELECT against the target ### What problem does this PR solve? Issue Number: None Related PR: apache#69488 Problem Summary: For a target with write-only defaults (Iceberg), `InsertUtils.normalizePlan` resolves `DEFAULT(column)` in the select lists of the INSERT's own query by name against the target's pinned writer schema, before the source relations are bound, by rewriting the expression tree of each projection. `UnboundStar` keeps the REPLACE list of `SELECT * REPLACE(expr AS c)` in a field instead of as expression children, so the rewrite never reached a `DEFAULT(c)` there and binding later resolved it against the source column. With a source `src(c INT DEFAULT 7)` and a target `t(c INT)` whose write-default is 9, INSERT INTO t(c) SELECT * REPLACE(DEFAULT(c) AS c) FROM src wrote 7, while `INSERT INTO t(c) SELECT DEFAULT(c) FROM src` wrote 9. branch-4.2's `resolveIcebergDefaultReferences` walks each projection the same way and has the same gap. `resolveDefaultReferences` now also resolves each replacement of a star and rebuilds the star with the same qualifier, EXCEPT list and SQL position. A replacement follows the rules of the rest of the select list, so a qualifier other than the target's (`DEFAULT(src.c)`) fails there too. A star inside a derived table is still left to binding, and a star without REPLACE keeps its expression. ### Release note None ### Check List (For Author) - Test: Unit Test - New `InsertUtilsTest#writeOnlyDefaultInStarReplacementResolvesByTargetColumnName` covers `src.* EXCEPT(score) REPLACE(DEFAULT(name) AS name)`: the replacement becomes the pinned default, and the qualifier and the EXCEPT list are kept. With the previous `InsertUtils` the replacement is still `DEFAULT(name)`. - Behavior changed: Yes. Within this PR, `DEFAULT(column)` in a `SELECT * REPLACE` of an `INSERT ... SELECT` into an Iceberg table names the target column, as in the rest of the INSERT's select lists. - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 <[email protected]>
…ly defaults ### What problem does this PR solve? Issue Number: None Related PR: apache#69488 Problem Summary: `RowLevelDmlCommand` pins the target's writer schema for every row-level DML, a DELETE included: its delete sink takes the writer columns from it. `RewriteDefaultExpression` takes the default of a `DEFAULT(column)` on a table with write-only defaults (Iceberg) from that pinned schema, so on a table whose `c` has write-default 35, DELETE FROM t WHERE c = DEFAULT(c) deleted every row with `c = 35`. A DELETE writes no row values, so no write default applies to it. branch-4.2 rejects `DEFAULT(column)` there, because its DELETE installs no write schema context, and master without this PR read the default from the read schema, which carries none, and deleted nothing. `RowLevelDmlCommand` now also marks the target of a DELETE in the `StatementContext` when it pins the writer schema, on the run and the EXPLAIN paths alike, and `RewriteDefaultExpression` rejects `DEFAULT(column)` on a marked table that has write-only defaults. UPDATE and MERGE are not marked and resolve as before. The mark is cleared with the pinned schemas when a prepared statement starts its next execution. ### Release note None ### Check List (For Author) - Test: Unit Test / Regression test - New `RewriteDefaultExpressionTest#writeOnlyDefaultInADeleteIsRejected` and `RowLevelDmlCommandTest#onlyADeleteMarksItsTargetAsWritingNoRowValues`; 9 classes, 85 cases pass locally (`RewriteDefaultExpressionTest`, `RowLevelDmlCommandTest`, `PositionDeleteRowLevelDmlTransformTest`, `ChangelogRowLevelDmlTransformTest`, `PhysicalPlanTranslatorIcebergRowLevelDmlTest`, `InsertUtilsTest`, `ConnectorWriteSchemaUtilsTest`, `DeleteFromCommandTest`, `UpdateCommandTest`) - `test_iceberg_initial_defaults` expects the error for a DELETE with `DEFAULT(default_int)` on rows that hold the write-default; not run locally (needs the Iceberg docker environment) - Behavior changed: Yes. `DEFAULT(column)` in a DELETE on a table with write-only defaults is rejected (master evaluated it to NULL). - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 <[email protected]>
7ddd7c9 to
985b588
Compare
|
Rebased onto master 339e087. The only conflict was in Local, on the rebased head: |
|
run buildall |
|
/review |
There was a problem hiding this comment.
Static review of apache/doris PR #69488 at the contracted head 985b588 found one verified P1 issue. Delete-only MERGE statements are dispatched as RowLevelDmlOp.MERGE, so the new DELETE marker is absent even though the sink writes only position deletes; DEFAULT(t.c) in the MERGE ON predicate can therefore use the Iceberg write-default and delete matching rows. The capability declaration, API 27.0 surface baseline, writer-schema typing, INSERT plan traversal, statement reset lifecycle, and regression/unit-test structure were inspected and found consistent outside this issue. Existing P1 threads for plain DELETE, derived-query traversal, and default typing were independently revalidated as fixed on this head and are not duplicated. No additional user review focus was provided. No builds or tests were run because this invocation is static-only. The critical review checkpoints are covered: the task goal is incomplete only for delete-only MERGE semantics; no new configuration, Thrift/Protobuf, persistence, or FE-BE protocol changes were found; statement lifecycle/reset behavior and mixed MERGE branches were traced; and no separate concurrency, locking, observability, or performance defect was substantiated. Review converged after 2 bounded rounds.
Review converged after 2 round(s).
| @@ -154,7 +154,7 @@ public void run(ConnectContext ctx, StmtExecutor stmtExecutor) throws Exception | |||
| public Plan getExplainPlan(ConnectContext ctx) { | |||
There was a problem hiding this comment.
[P1] Guard write-only defaults in delete-only MERGE
pinWriteSchema marks connectorDeleteTargets only for RowLevelDmlOp.DELETE. A valid MERGE with no insert/update branch (for example, WHEN MATCHED THEN DELETE) is dispatched as RowLevelDmlOp.MERGE, even though its sink writes no row values. Its ON predicate therefore lets RewriteDefaultExpression replace DEFAULT(t.c) with the pinned Iceberg write-default; with c defaulting to 35, ... ON t.id = s.id AND t.c = DEFAULT(t.c) WHEN MATCHED THEN DELETE deletes rows whose c is 35. Treat an all-delete/no-insert MERGE as a delete target (including EXPLAIN), or reject this DEFAULT use, and add coverage.
There was a problem hiding this comment.
Keeping this as is. A MERGE resolves DEFAULT(column) in its ON condition against the writer schema it pins, whatever its WHEN branches, and so does branch-4.2, which this PR ports. There, IcebergMergeCommand installs the write schema context for every MERGE, delete-only ones included, both when it runs and for EXPLAIN (ExplainCommand installs it again from the merge sink), and RewriteDefaultExpression resolves a target column from it. So on branch-4.2 too, MERGE ... ON t.id = s.id AND t.c = DEFAULT(t.c) WHEN MATCHED THEN DELETE deletes the matched rows whose c is 35. Master evaluates DEFAULT(t.c) to NULL there and deletes nothing.
To be precise about the DELETE thread (#69488 (comment)): the line follows branch-4.2 and depends on the kind of statement. branch-4.2 gives a DELETE no write schema context, so DEFAULT(c) fails there, and gives every MERGE one. "Writes no row values" explains why the DELETE has none. It is not a test applied to each MERGE. Rejecting a delete-only MERGE would break, after an upgrade, a statement that runs on branch-4.1 and branch-4.2. It would also make the same ON condition fail or resolve depending on whether the MERGE has an UPDATE or INSERT branch, although the condition selects the same rows to delete either way. The description now states that a MERGE resolves it whatever its WHEN branches, also when they all delete.
TPC-H: Total hot run time: 28371 ms |
TPC-DS: Total hot run time: 150931 ms |
ClickBench: Total hot run time: 24.32 s |
FE UT Coverage ReportIncrement line coverage |
|
run external |
924060929
left a comment
924060929
left a comment
There was a problem hiding this comment.
Iceberg write defaults belong to the statement-pinned writer schema. This change resolves INSERT defaults before consumer binding and bound UPDATE/MERGE references by table and field identity, preserves column types, and rejects unsupported SELECT/DELETE uses.
Static review of all 20 changed files and relevant FE call chains at 985b588 found no blocking P0/P1/P2 issue. Previously reported typing, derived-query scope, star-replacement and DELETE issues are fixed. The delete-only MERGE comment was rechecked against current code and local branch-4.2 code: resolving its ON condition from the pinned writer schema is intentional regardless of WHEN branches. Remaining documented traversal, qualification and HMS snapshot limitations do not establish a new regression requiring this PR to expand scope.
APPROVE for the reviewed FE scope. No builds or tests were run. Supplied current-head status reports P0, FE UT and external regression success, but FE coverage failure; required-check completeness and merge eligibility are separate from this code review.
Automated FE-focused static review (GPT-6.1 Sol, medium). No build or tests were run.
github-actions
Bot
added
the
approved
|
PR approved by at least one committer and no changes requested. |
…t a snapshot (#69999) ### What problem does this PR solve? Issue Number: None Related PR: #69299, #69488 Problem Summary: #### In short An Iceberg table stored in HMS and read through an HMS catalog (`'type'='hms'`) loses every capability it inherits from the embedded Iceberg connector in any statement that pins the table at a snapshot. Since #69299 that is every statement that reads it: it gets no Top-N lazy materialization and no nested column pruning, and `DEFAULT(column)` in the conditions of `UPDATE` and `MERGE` silently matches nothing. This PR makes the snapshot overload of `HiveConnectorMetadata#getTableSchema` pass the inherited capabilities on, as the latest-schema overload already does. It is a master-only fix (branch-4.x has no connector SPI), left over from #69488 (review comment 4355508100). #### Background fe-core resolves a table-scoped capability (`PluginDrivenExternalTable.hasCapability`) from the catalog's connector or from the per-table set in the table's schema. It never asks the embedded connector that built the schema of an iceberg-on-HMS table. So `HiveConnectorMetadata` copies the capabilities that fe-core resolves per table (`SIBLING_INHERITABLE_CAPABILITIES`: auto and sampled analyze, Top-N lazy materialization, nested column pruning, field-id access paths, storage predicate pruning, nested column DDL and write-only defaults) from the owning connector into the per-table set of each schema it delegates. Iceberg declares seven of the eight; it does not declare sampled analyze. It did this only in `getTableSchema(session, handle)`. `getTableSchema(session, handle, snapshot)` returned the sibling's schema unchanged. fe-core uses that overload for explicit time travel. Since #69299 it also uses it for the latest-snapshot pin of every statement that reads an Iceberg table: the pin retains its schema (`retainSchema(true)`), `PluginDrivenMvccExternalTable` builds the schema at that pin, and `getSchemaCacheValue()`, where `hasCapability` reads the per-table set, returns it for the rest of the statement. #### The problem, and what it cost Reproduced on a local cluster with a standalone Hive 3.1.2 metastore. Two identical format-version 3 tables `(id INT, c INT, s STRUCT<a:INT, b:STRING>)` get two rows, then `ADD COLUMN d INT DEFAULT 42`, `RENAME COLUMN s.a TO a2` and a third row with `d = 7`. The same statements then run against one table through the Iceberg catalog and against the other through the HMS catalog. | Statement | Iceberg catalog | HMS catalog, master | HMS catalog, this PR | |---|---|---|---| | `EXPLAIN SELECT * FROM t ORDER BY id LIMIT 1`, also with `FOR VERSION AS OF` | `VMaterializeNode` | no lazy materialization | `VMaterializeNode` | | `EXPLAIN SELECT struct_element(s, 'a2') FROM t` | `all access paths: [s.a2]` | no pruning | `all access paths: [s.a2]` | | `UPDATE t SET c = 100 WHERE d = DEFAULT(d)` | updates rows 0 and 1 | updates nothing | updates rows 0 and 1 | | `MERGE INTO t USING (SELECT 0 AS id) s ON t.id = s.id AND t.d = DEFAULT(t.d) WHEN MATCHED THEN UPDATE SET c = 300` | updates row 0 | updates nothing | updates row 0 | | `DELETE FROM t WHERE d = DEFAULT(d)` | fails: `cannot be used in a DELETE` | deletes nothing | fails the same way | | `SELECT DEFAULT(d) FROM t` | fails: `must reference a column of the table that this statement writes` | returns NULL | fails the same way | The `UPDATE`, `MERGE` and `DELETE` rows fail silently: the statement succeeds and changes nothing. Reads of the renamed field return the right values in all three columns: on master the HMS catalog does not prune the struct at all, and with this PR it prunes it with field-id access paths, as the Iceberg catalog does. `INSERT ... VALUES (DEFAULT(d))` and `INSERT ... SELECT DEFAULT(d)` already wrote 42 through both catalogs. #### How this PR fixes it Both overloads now go through one private helper, `delegatedTableSchema`. It resolves the owning sibling once, asks it for the schema (at the pin, for the snapshot overload), and passes the result through the existing `reflectSiblingCapabilities`. The snapshot reaches the sibling unchanged. A hudi-on-HMS table is unaffected: the Hudi connector declares no capabilities, so its schema comes back as it was. A plain Hive table never reaches this branch. #### Results Through the HMS catalog, every statement in the table above now plans and answers as it does through the Iceberg catalog.
…load ### What problem does this PR solve? Issue Number: N/A Related PR: apache#65851, apache#69488, apache#69499 Problem Summary: Required and nested ADD COLUMN defaults must retain their SQL values when converted to Iceberg metadata. Normalize decimal scale, valid Boolean whitespace, compact UUIDs and escaped strings; preserve nanosecond gap semantics and temporal precision while keeping unqualified zoned values compatible with existing connector inputs. Resolve Iceberg zoned defaults through the session timezone and reject non-finite floating defaults before metadata publication. Verify real Hadoop metadata commits/reloads, extend read/write regression coverage and update both existing v2 rejection assertions. ### Release note Typed literal defaults for required and nested Iceberg column additions preserve their values through metadata serialization and reads/writes. Non-finite floating defaults fail before commit. ### Check List (For Author) - Test: 89 focused FE tests passed; full build.sh --fe, Checkstyle and both connector architecture gates passed. The preset regression runner generated the new expected output and then successfully compared all four affected external suites against isolated REST/MinIO services. Local read/write validation used the branch-built FE and an ASAN BE cache; BE code is unchanged. - Behavior changed: Yes; correct typed default conversion and prevent publication of unreadable floating defaults. Existing SHOW CREATE default presentation remains unchanged. - Does this need documentation: Yes; the connector contract and fixture requirements are updated in this commit.
### What problem does this PR solve? Issue Number: N/A Related PR: apache#65851, apache#69488 Problem Summary: Equivalent explicit and session-zone TIMESTAMPTZ defaults can differ by one hour when fractional rounding crosses a DST overlap. A valid civil timestamp can also resolve outside the UTC range supported by Doris and become an unreadable default after schema commit. Preserve civil precision until the existing Iceberg session boundary, round the resolved instant at the original declared precision, and reject unsupported final UTC years before staging. Share the conversion across single, batch and nested additions and retain unparameterized SPI epoch microseconds. ### Release note Iceberg zoned column defaults consistently interpret explicit and implicit time zones across DST rounding and reject unsupported UTC values before committing schema changes. ### Check List (For Author) - Test: 91 focused FE tests passed; full build.sh --fe, Checkstyle and both connector architecture gates passed. The preset regression runner generated expected results and passed the four affected external suites, including old/new files, explicit/session CET equality, UTC upper/lower rejection and batch atomicity. Local native validation uses an existing ASAN BE cache; C++ code is unchanged. - Behavior changed: Yes; zoned ADD COLUMN defaults resolve time zones before target rounding and validate the final UTC range. Existing default presentation is preserved. - Does this need documentation: Yes; the connector contract describes the precision and UTC-range policy.
What problem does this PR solve?
Issue Number: None
Related PR: #66538, #57531, #65851
Problem Summary:
In short. On an Iceberg table,
DEFAULT(column)anywhere exceptUPDATE ... SETandMERGE ... VALUESreads the default from the read schema, which carries no Iceberg write-default. This PR resolves it against the write schema that the statement pinned for its target, as branch-4.1 and branch-4.2 do.Background. An Iceberg
write-defaultapplies to rows written after it is set. So it lives in the writer schema that a write statement pins (IcebergWriteSchemaContext), not in the columns of the read schema. #65851 resolvesDEFAULT(column)against that pinned schema inUPDATE ... SET,MERGE ... UPDATE SETandMERGE ... INSERT VALUES(ConnectorWriteSchemaUtils). Every otherDEFAULT(column)either fails at binding or reachesRewriteDefaultExpression(#57531), which reads the default from the bound column. branch-4.1 #66538, also in branch-4.2, resolves these against the pinned write target too. It rejectsDEFAULT(column)on an Iceberg column in a statement that does not write the table.The problem, and what it cost. Reproduced on a local cluster with a
file://format-version 3 table:CREATE TABLE t (id INT, c INT),ALTER TABLE t ADD COLUMN d INT DEFAULT 42, one row(0, 0), sod = 42.INSERT INTO t (id, c, d) VALUES (1, 1, DEFAULT(d))Unknown column 'd' in 'table list'INSERT INTO t (id, c, d) SELECT 2, 2, DEFAULT(d) FROM t WHERE id = 0INSERT INTO t (id, c, d) SELECT 3, 3, DEFAULT(d)Unknown column 'd'UPDATE t SET c = 100 WHERE d = DEFAULT(d)d = NULLmakes the plan an empty setd = 42is pushed down to the scanMERGE INTO t USING (SELECT 0 AS id) s ON t.id = s.id AND t.d = DEFAULT(t.d) WHEN MATCHED THEN UPDATE SET c = 300SELECT DEFAULT(d) FROM tDEFAULT(d) must reference a column of the table that this statement writesThe
UPDATEandMERGEcases fail silently: the statement succeeds and changes nothing.How this PR fixes it.
SUPPORTS_WRITE_ONLY_DEFAULTSsays which kind a table is. The Iceberg connector declares it. The Hive connector passes it on to iceberg-on-HMS tables, like the other per-table capabilities, but only through the two-argumentgetTableSchema. Since [improvement](datasource) Version Iceberg partition views and SQL cache metadata #69299, a statement that pins the table at its latest snapshot reads the schema from the snapshot overload, which reflects none of them, so on an iceberg-on-HMS table such a statement keeps master's results (for example,UPDATE t SET c = 100 WHERE d = DEFAULT(d)still changes no row). Reflecting the sibling's capabilities there changes planning for all of them and is left to a follow-up PR.InsertUtils.normalizePlanresolvesDEFAULT(column)inINSERT ... VALUESand in the select lists ofINSERT ... SELECT, including the replacements ofSELECT * REPLACE(...), against the pinned write schema. It does this by name, before the source relations are bound, so the name always refers to a target column, as in branch-4.2. In MySQL,DEFAULT(column)in aSELECTrefers to a source column instead. A derived table inFROMis a query of its own and is left alone:DEFAULT(column)in it still names a column of its ownFROMclause.RewriteDefaultExpressiontakes the default for aDEFAULT(column)bound to such a table from the pinned write column with the same field id. That covers theWHEREofUPDATEand theONofMERGE. A MERGE resolves it whatever its WHEN branches, also when they all delete, as branch-4.2 does. If the statement pinned no write schema for that table, it fails. ADELETEpins the writer schema too, for its delete sink, but it writes no row values, soRowLevelDmlCommandmarks its target andDEFAULT(column)on it fails withDEFAULT(d) cannot be used in a DELETE: the defaults of this table apply only to the rows that a statement writes, as in branch-4.2, whose DELETE installs no write schema context. Master reads NULL there, soDELETE FROM t WHERE d = DEFAULT(d)deletes nothing; resolved to the write-default it would delete every row withd = 42.RewriteDefaultExpressionalready does for a boundDEFAULT(column). The parsed literal-1is a TINYINT, so without the castbit_count(DEFAULT(c))on an INT column would count 8 bits instead of 32. The externalUPDATE,MERGEand changelog paths use the same helper.RewriteDefaultExpressionrejects a boundDEFAULT(column)whose writer column has another type than the read column the statement was bound with, which happens only when the table changed outside Doris after Doris cached it, and asks to refresh the table: the expressions around it were bound against the read type.connector.plugin.api.versiongoes from 26.0 to 27.0, as for the other recent changes to that surface ([improvement](datasource) Version Iceberg partition views and SQL cache metadata #69299, [feature](fe) Gate catalog SSRF endpoints and driver_url via a connector security surface #69588, [improvement](datasource) Bound and fairly schedule external scan planning #69298, [improvement](fe) Default external scans to bounded split batches #69683 and [fix](fe) Stop split enumeration when queries finish or terminate #69835 took 22.0 to 26.0).Where
DEFAULT(column)resolves on an Iceberg target (direct Iceberg catalogs).DEFAULT(column)INSERT/INSERT OVERWRITEVALUESrowsFROM, set operation branches,SELECT * REPLACE)* REPLACE, which branch-4.2 binds to the sourceGROUP BY/ROLLUPoutputs, a derived table inFROM,WITHdefinitions, subqueries,WHERE/ONDEFAULT(column)(last row)VALUESbranch of a set operationDEFAULT(src.c),DEFAULT(ctl.db.t.c)) in the positions abovecolumn in assignment list is invalidUPDATESET/WHEREMERGEONandWHEN ... ANDconditions, whatever the WHEN branches, also when they all deleteUPDATE SET/INSERT VALUESDELETE(alsoDELETE ... USING)cannot be used in a DELETEDEFAULT(x.c)xis the written table: its write-default by field id; an OLAP or Hive table: that column's default; another Iceberg table: fails withmust reference a column of the table that this statement writesA bound
DEFAULT(column)whose writer column has another type than the read column fails and asks to refresh the table. WhereDEFAULT(column)is bound rather than resolved by name (the third row), it keeps its meaning from before this PR and names a column of its ownFROMclause, as on master and branch-4.2: for example inINSERT INTO t SELECT id, DEFAULT(d) FROM src GROUP BY id, dwith an OLAP or Hivesrc, it takes the default ofsrc.d.Results.
EXPLAIN UPDATE t SET c = 5 WHERE d = DEFAULT(d)goes from an empty set to a scan withd = 42pushed down to Iceberg.InsertUtilsTest, 6 in the newRewriteDefaultExpressionTestand 1 inRowLevelDmlCommandTest. They cover VALUES, and INSERT ... SELECT both from a staging relation whose own column is alsonameand without FROM: each resolves to the pinned default, and an unknown column fails.DEFAULT(column)inside a derived table, qualified or not and also under a UNION branch, is left to binding. ASELECT * REPLACE(DEFAULT(name) AS name)replacement resolves like the rest of the select list and keeps the star's qualifier andEXCEPTlist. The pinned write column wins over the read column. A statement that writes nothing fails. A table without the capability is unchanged. With master'sInsertUtilsandRewriteDefaultExpression, the 5 tests of the fix fail and the 2 tests of the unchanged path pass. TheSELECT * REPLACEtest fails with this PR's previousInsertUtils, where the replacement staysDEFAULT(name).IcebergConnectorTest,HiveConnectorMetadataSiblingDelegationTestandConnectorPluginSurfaceTestpass. So do the fe-core tests around the write schema:ConnectorWriteSchemaUtilsTest,ExternalRowLevelUpdatePlanBuilderTest,ExternalRowLevelMergePlanBuilderTest,ChangelogRowLevelDmlTransformTestandPluginDrivenExternalTableTest. Two of the tests pin the typing:bit_count(DEFAULT(bits))gets the default as an INT in VALUES and in SELECT, and fails without the cast; a BIGINT writer column over an INT read column is rejected, directly and under an IF bound to INT.DEFAULT(column)on the target of a DELETE fails even with a pinned write-default, and only a DELETE marks its target (an UPDATE does not); skipping the check, or marking UPDATE instead of DELETE, makes exactly these two tests fail.PositionDeleteRowLevelDmlTransformTest,PhysicalPlanTranslatorIcebergRowLevelDmlTest,DeleteFromCommandTestandUpdateCommandTestpass.test_iceberg_initial_defaultsnow writes rows 20 to 22 withDEFAULT(default_int)in VALUES and in INSERT ... SELECT with and without FROM. It then updates them withWHERE default_int = DEFAULT(default_int)andMERGE ... ON ... = DEFAULT(...), for Parquet and ORC, and checks the rows in the.out. A plainSELECT DEFAULT(default_int)must fail, and so must aDELETE ... WHERE default_int = DEFAULT(default_int)on rows 20 to 22, which hold the write-default. The suite passed locally before the DELETE block was added, and the whole suite, DELETE block included, has since passed in two External regression runs on CI. With master's FE it fails at the first of these statements:Unknown column 'default_int' in 'table list'.Release note
Fix: On Iceberg tables,
DEFAULT(column)inINSERT ... VALUES,INSERT ... SELECTand theWHERE/ONconditions ofUPDATEandMERGEnow resolves to the column's write-default. Before, it failed, wrote NULL, or matched no row.SELECT DEFAULT(column)on an Iceberg table now fails instead of returning NULL, and so doesDEFAULT(column)in aDELETEon an Iceberg table, which used to be NULL and delete nothing.Check List (For Author)
Test
On a local cluster with a
file://Iceberg catalog:CREATE TABLE t (id INT, c INT) PROPERTIES ('format-version'='3'),ALTER TABLE t ADD COLUMN d INT DEFAULT 42, then the statements in the table above.Behavior changed:
See the release note.
INSERT ... SELECT DEFAULT(column)into an Iceberg table, also in the replacements ofSELECT * REPLACE, names a target column, even when the source has a column of that name; inside a derived table inFROMit still names a column of that table's ownFROMclause. In aDELETE,DEFAULT(column)on an Iceberg table is rejected.Does this need documentation?
Check List (For Reviewer who merge this PR)