github-actions[bot] commented on code in PR #69488:
URL: https://github.com/apache/doris/pull/69488#discussion_r4355508080
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/InsertUtils.java:
##########
@@ -476,15 +489,62 @@ private static Plan normalizePlanWithoutLock(LogicalPlan
plan, TableIf table,
}
static List<Column> connectorWriteSchema(TableIf table, boolean full) {
+ return pinnedConnectorWriteSchema(table).orElseGet(() ->
table.getBaseSchema(full));
+ }
+
+ private static Optional<List<Column>> pinnedConnectorWriteSchema(TableIf
table) {
ConnectContext context = ConnectContext.get();
- if (context != null && context.getStatementContext() != null) {
- Optional<List<Column>> pinned =
-
context.getStatementContext().getConnectorWriteSchema(table.getId());
- if (pinned.isPresent()) {
- return pinned.get();
- }
+ if (context == null || context.getStatementContext() == null) {
+ return Optional.empty();
+ }
+ return
context.getStatementContext().getConnectorWriteSchema(table.getId());
+ }
+
+ /**
+ * The pinned writer schema of a target whose column defaults apply to
writes only. The read schema of such a
+ * table carries no defaults, so DEFAULT(column) in an INSERT into it must
resolve against this schema.
+ */
+ private static Optional<List<Column>> writeOnlyDefaultColumns(TableIf
table) {
+ if (!(table instanceof PluginDrivenExternalTable)
+ || !((PluginDrivenExternalTable)
table).hasWriteOnlyDefaults()) {
+ return Optional.empty();
}
- return table.getBaseSchema(full);
+ return pinnedConnectorWriteSchema(table);
+ }
+
+ /** Resolve DEFAULT(column) in one inserted value by name against the
target's pinned writer schema. */
+ private static NamedExpression resolveDefaultReferences(NamedExpression
value,
+ Optional<List<Column>> writeOnlyDefaultColumns, TableIf table,
List<String> targetNameParts) {
+ if (!writeOnlyDefaultColumns.isPresent()) {
+ return value;
+ }
+ return (NamedExpression)
ConnectorWriteSchemaUtils.resolveDefaultReferences(value,
+ writeOnlyDefaultColumns.get(), (ExternalTable) table,
ConnectContext.get(), targetNameParts, null);
+ }
+
+ /**
+ * Resolve 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, so it names a
target column whatever the source is.
+ */
+ private static Plan resolveSelectDefaultReferences(Plan query,
Optional<List<Column>> writeOnlyDefaultColumns,
+ TableIf table, List<String> targetNameParts) {
+ return query.rewriteUp(plan -> {
Review 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.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/RewriteDefaultExpression.java:
##########
@@ -82,6 +87,15 @@ 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)
Review 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.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/InsertUtils.java:
##########
@@ -476,15 +489,62 @@ private static Plan normalizePlanWithoutLock(LogicalPlan
plan, TableIf table,
}
static List<Column> connectorWriteSchema(TableIf table, boolean full) {
+ return pinnedConnectorWriteSchema(table).orElseGet(() ->
table.getBaseSchema(full));
+ }
+
+ private static Optional<List<Column>> pinnedConnectorWriteSchema(TableIf
table) {
ConnectContext context = ConnectContext.get();
- if (context != null && context.getStatementContext() != null) {
- Optional<List<Column>> pinned =
-
context.getStatementContext().getConnectorWriteSchema(table.getId());
- if (pinned.isPresent()) {
- return pinned.get();
- }
+ if (context == null || context.getStatementContext() == null) {
+ return Optional.empty();
+ }
+ return
context.getStatementContext().getConnectorWriteSchema(table.getId());
+ }
+
+ /**
+ * The pinned writer schema of a target whose column defaults apply to
writes only. The read schema of such a
+ * table carries no defaults, so DEFAULT(column) in an INSERT into it must
resolve against this schema.
+ */
+ private static Optional<List<Column>> writeOnlyDefaultColumns(TableIf
table) {
+ if (!(table instanceof PluginDrivenExternalTable)
+ || !((PluginDrivenExternalTable)
table).hasWriteOnlyDefaults()) {
+ return Optional.empty();
}
- return table.getBaseSchema(full);
+ return pinnedConnectorWriteSchema(table);
+ }
+
+ /** Resolve DEFAULT(column) in one inserted value by name against the
target's pinned writer schema. */
+ private static NamedExpression resolveDefaultReferences(NamedExpression
value,
+ Optional<List<Column>> writeOnlyDefaultColumns, TableIf table,
List<String> targetNameParts) {
+ if (!writeOnlyDefaultColumns.isPresent()) {
+ return value;
+ }
+ return (NamedExpression)
ConnectorWriteSchemaUtils.resolveDefaultReferences(value,
Review 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.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/InsertUtils.java:
##########
@@ -476,15 +489,62 @@ private static Plan normalizePlanWithoutLock(LogicalPlan
plan, TableIf table,
}
static List<Column> connectorWriteSchema(TableIf table, boolean full) {
+ return pinnedConnectorWriteSchema(table).orElseGet(() ->
table.getBaseSchema(full));
+ }
+
+ private static Optional<List<Column>> pinnedConnectorWriteSchema(TableIf
table) {
ConnectContext context = ConnectContext.get();
- if (context != null && context.getStatementContext() != null) {
- Optional<List<Column>> pinned =
-
context.getStatementContext().getConnectorWriteSchema(table.getId());
- if (pinned.isPresent()) {
- return pinned.get();
- }
+ if (context == null || context.getStatementContext() == null) {
+ return Optional.empty();
+ }
+ return
context.getStatementContext().getConnectorWriteSchema(table.getId());
+ }
+
+ /**
+ * The pinned writer schema of a target whose column defaults apply to
writes only. The read schema of such a
+ * table carries no defaults, so DEFAULT(column) in an INSERT into it must
resolve against this schema.
+ */
+ private static Optional<List<Column>> writeOnlyDefaultColumns(TableIf
table) {
+ if (!(table instanceof PluginDrivenExternalTable)
+ || !((PluginDrivenExternalTable)
table).hasWriteOnlyDefaults()) {
+ return Optional.empty();
}
- return table.getBaseSchema(full);
+ return pinnedConnectorWriteSchema(table);
+ }
+
+ /** Resolve DEFAULT(column) in one inserted value by name against the
target's pinned writer schema. */
+ private static NamedExpression resolveDefaultReferences(NamedExpression
value,
+ Optional<List<Column>> writeOnlyDefaultColumns, TableIf table,
List<String> targetNameParts) {
+ if (!writeOnlyDefaultColumns.isPresent()) {
+ return value;
+ }
+ return (NamedExpression)
ConnectorWriteSchemaUtils.resolveDefaultReferences(value,
+ writeOnlyDefaultColumns.get(), (ExternalTable) table,
ConnectContext.get(), targetNameParts, null);
Review 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.
##########
fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnectorMetadata.java:
##########
@@ -625,7 +625,8 @@ public ConnectorTableSchema getTableSchema(
ConnectorCapability.SUPPORTS_NESTED_COLUMN_PRUNE,
ConnectorCapability.SUPPORTS_FIELD_ID_ACCESS_PATH,
ConnectorCapability.SUPPORTS_STORAGE_PREDICATE_PRUNING,
- ConnectorCapability.SUPPORTS_NESTED_COLUMN_SCHEMA_CHANGE));
+ ConnectorCapability.SUPPORTS_NESTED_COLUMN_SCHEMA_CHANGE,
+ ConnectorCapability.SUPPORTS_WRITE_ONLY_DEFAULTS));
Review 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.
--
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]
