Repository navigation
[fix](fe) Resolve DEFAULT(column) on Iceberg tables from the write schema #69488
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
43b09bf
8b046c6
95191a9
fc049a2
724071d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,10 @@ | |
| package org.apache.doris.nereids.rules.expression.rules; | ||
|
|
||
| import org.apache.doris.catalog.Column; | ||
| import org.apache.doris.catalog.TableIf; | ||
| import org.apache.doris.datasource.plugin.PluginDrivenExternalTable; | ||
| import org.apache.doris.nereids.CascadesContext; | ||
| import org.apache.doris.nereids.StatementContext; | ||
| import org.apache.doris.nereids.analyzer.UnboundFunction; | ||
| import org.apache.doris.nereids.exceptions.AnalysisException; | ||
| import org.apache.doris.nereids.parser.NereidsParser; | ||
|
|
@@ -46,6 +49,8 @@ | |
| * 1. DEFAULT(column) with default value -> the actual default value expression | ||
| * 2. DEFAULT(column) with no default value but nullable -> NULL | ||
| * 3. DEFAULT(column) with no default value and not nullable -> error | ||
| * A table whose defaults apply to writes only takes them from the writer schema that the statement | ||
| * pinned for its write target; DEFAULT(column) on any other such table is an error. | ||
| */ | ||
| public class RewriteDefaultExpression implements ExpressionPatternRuleFactory { | ||
|
|
||
|
|
@@ -82,6 +87,24 @@ private static Expression rewrite(ExpressionMatchingContext<Default> context) { | |
| + column.getName() + "'"); | ||
| } | ||
|
|
||
| Optional<TableIf> originalTable = slotRef.getOriginalTable(); | ||
| if (originalTable.filter(PluginDrivenExternalTable.class::isInstance) | ||
| .filter(table -> ((PluginDrivenExternalTable) table).hasWriteOnlyDefaults()) | ||
| .isPresent()) { | ||
| column = pinnedWriteColumn(context.cascadesContext.getStatementContext(), originalTable.get(), column) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] Derive the result type after selecting the writer column
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in cf75568.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| .orElseThrow(() -> new AnalysisException("DEFAULT(" + slotRef.toSql() | ||
| + ") must reference a column of the table that this statement writes")); | ||
| // The expressions around DEFAULT(column) were bound against the read column, so a default of another type | ||
| // cannot stand in for it. The types differ when the table changed outside Doris after its read schema was | ||
| // cached, e.g. by a type promotion. | ||
| DataType writeType = DataType.fromCatalogType(column.getType()); | ||
| if (!writeType.equals(targetType)) { | ||
| throw new AnalysisException("DEFAULT(" + slotRef.toSql() + ") reads the column as " | ||
| + targetType.toSql() + " but the table now writes it as " + writeType.toSql() | ||
| + "; the table changed outside Doris, refresh it and retry"); | ||
| } | ||
| } | ||
|
|
||
| String defaultValueSql = column.getDefaultValueSql(); | ||
| if (defaultValueSql == null) { | ||
| if (column.isAllowNull()) { | ||
|
|
@@ -101,4 +124,12 @@ private static Expression rewrite(ExpressionMatchingContext<Default> context) { | |
|
|
||
| return TypeCoercionUtils.castIfNotSameType(defaultValueExpr, targetType); | ||
| } | ||
|
|
||
| private static Optional<Column> pinnedWriteColumn(StatementContext statementContext, TableIf table, | ||
| Column readColumn) { | ||
| return statementContext.getConnectorWriteSchema(table.getId()) | ||
| .flatMap(columns -> columns.stream() | ||
| .filter(column -> column.getUniqueId() == readColumn.getUniqueId()) | ||
| .findFirst()); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] Reflect this capability for snapshot schemas as well
The two-argument
getTableSchemareflects this set, but the snapshot overload still returns the sibling schema directly. ForSELECT DEFAULT(c) FROM hms_catalog.db.iceberg_t FOR VERSION AS OF <snapshot_id>,PluginDrivenMvccExternalTable.loadSnapshotpublishes that unreflected historical schema.tableCapabilities()reads the ambient pinned schema, while the Hive catalog itself does not declare write-only defaults, sohasWriteOnlyDefaults()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 togetTableSchema(session, handle, snapshot)and test that overload.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not in this PR.
HiveConnectorMetadata#getTableSchema(session, handle, snapshot)returns the sibling's schema withoutreflectSiblingCapabilitiesfor 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 OFkeeps 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.