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]

Reply via email to