github-actions[bot] commented on code in PR #66345:
URL: https://github.com/apache/doris/pull/66345#discussion_r3694759423
##########
be/src/format_v2/table_reader.h:
##########
@@ -1216,17 +1216,40 @@ class TableReader {
if (const auto* array_type = typeid_cast<const
DataTypeArray*>(table_type.get())) {
const auto& array_column = assert_cast<const
ColumnArray&>(**column);
ColumnPtr nested_column = array_column.get_data_ptr();
- RETURN_IF_ERROR(
- _align_column_nullability(&nested_column,
array_type->get_nested_type()));
+ NullMap descendant_parent_null_map;
+ // Collection entries use offset coordinates, so inherited row
masks must be projected
+ // only when a required descendant can consume them. This avoids
scratch proportional
+ // to all array entries for the common all-required schema.
+ const NullMap* descendant_parent_null_map_ptr = nullptr;
+ if (_requires_parent_null_map_for_alignment(nested_column,
Review Comment:
`_requires_parent_null_map_for_alignment()` runs before this code
establishes that the inherited parent mask exists and hides at least one row.
For a large ARRAY/MAP with nullable-to-required descendants and a null or
all-clear parent mask, the probe scans descendant null maps via `has_null()`,
projection returns no entry mask, and normal alignment scans those descendants
again. Please gate the descendant probe on a parent mask that can actually hide
rows so this schema-evolution path does not add an avoidable O(number of
entries) pass per block.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Cast.java:
##########
@@ -90,6 +90,11 @@ public boolean nullable() {
return castNullable(child().nullable(), child().getDataType(),
targetType);
}
+ /** Nullability of this cast when invalid input fails instead of producing
NULL. */
+ public boolean strictModeNullable() {
+ return child().nullable();
Review Comment:
Strict failure semantics do not make every successful cast physically
non-nullable. BE's strict BIGINT-to-INT specialization still has type
`Nullable(Int32)` (with an all-clear map for valid rows), while this reports a
required field and FE serializes `Struct<Int32>`. The generic nested-type
compatibility check admits that mismatch, then `FunctionStruct` tries to insert
the `ColumnNullable` argument into a non-null `ColumnInt32` field and hits the
column-type assertion even for an in-range value. Please keep the field
nullable until strict casts actually return non-null types, or normalize the
strict cast's physical result as part of this change.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergWritePlanProvider.java:
##########
@@ -230,6 +231,37 @@ public ConnectorSinkPlan planWrite(ConnectorSession
session, ConnectorWriteHandl
}
}
+ private void validateBoundWriteColumns(Table table, ConnectorWriteHandle
handle,
+ WriteOperation writeOperation) {
+ if (writeOperation == WriteOperation.DELETE ||
handle.getColumns().isEmpty()) {
+ return;
+ }
+ List<NestedField> currentColumns = table.schema().columns();
+ List<ConnectorColumn> boundColumns = handle.getColumns();
+ boolean hasSyntheticRowId = boundColumns.size() ==
currentColumns.size() + 1
+ &&
DORIS_ICEBERG_ROWID_COL.equals(boundColumns.get(boundColumns.size() -
1).getName());
+ if (boundColumns.size() != currentColumns.size() &&
!hasSyntheticRowId) {
Review Comment:
`handle.getColumns()` is the query's bound write list, not the full schema
snapshot. `BindSink` intentionally omits unspecified columns and static
partition columns, and `PhysicalPlanTranslator` copies that subset into the
handle, so a valid `INSERT INTO t (id) ...` for a two-column table (or an
INSERT with a static partition) reaches this branch with different sizes and is
rejected as schema drift even when the table never changed. Please carry the
complete bound target schema separately for this comparison, while retaining
the subset for value-to-column mapping.
--
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]