github-actions[bot] commented on code in PR #65851:
URL: https://github.com/apache/doris/pull/65851#discussion_r3716373400


##########
be/src/format_v2/column_mapper.cpp:
##########
@@ -2491,16 +2468,23 @@ Status TableColumnMapper::_create_direct_mapping(const 
ColumnDefinition& table_c
                 }
             }
             if (file_child == nullptr) {
+                if (table_child.default_expr == nullptr &&

Review Comment:
   [P1] Prepare nested defaults for FileScannerV2 `$position_deletes.row`
   
   This system-table reader inherits generic `TableReader` annotation: 
access-path expansion copies a nested `initial_default_value`, but neither it 
nor the inner `PositionDeleteFileTableReader` runs Iceberg's typed default 
preparation. An old delete file whose physical `row` lacks even a scalar child 
defaulted to `7` therefore reaches this new guard with `default_expr == 
nullptr` and the scan fails; the removed branch used to materialize that nested 
default directly. Please prepare typed defaults for the projected `row` tree 
(or preserve equivalent mapper materialization) and add Parquet/ORC 
FileScannerV2 coverage.



##########
be/src/format_v2/column_mapper.cpp:
##########
@@ -2491,16 +2468,23 @@ Status TableColumnMapper::_create_direct_mapping(const 
ColumnDefinition& table_c
                 }
             }
             if (file_child == nullptr) {
+                if (table_child.default_expr == nullptr &&
+                    table_child.initial_default_value.has_value()) {
+                    return Status::InvalidArgument(
+                            "Missing typed initial-default expression for 
table field '{}'",
+                            table_child.name);
+                }
+                if (_options.reject_missing_required_field && 
table_child.is_optional.has_value() &&

Review Comment:
   [P1] Enforce required children in FileScannerV2 `$position_deletes.row`
   
   This check is gated by `reject_missing_required_field`, but the specialized 
`PositionDeleteFileTableReader` never enables that option; only ordinary 
`IcebergTableReader::init()` does. A historical `row` missing a newly required 
child with no initial default therefore skips the preceding default guard, 
bypasses this check, and is filled with the generic type default/NULL, while V1 
rejects the same payload. Please enable the strict policy for the inner reader 
under semantics v2 and add Parquet/ORC FileScannerV2 coverage asserting V1 
parity.



##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergScanPlanProvider.java:
##########
@@ -1548,6 +1554,20 @@ public Map<String, String> getScanNodeProperties(
         Table table = resolveTable(session, iceHandle);
         Map<String, String> props = new LinkedHashMap<>();
         boolean systemTable = iceHandle.isSystemTable();
+        Schema scanSchema = null;
+        TableScan exactScan = null;
+        boolean hasApplicableEqualityDeletes = false;
+        if (!systemTable) {

Review Comment:
   [P1] Fence native `$position_deletes.row` scans during smooth upgrade
   
   This gate skips every system table, but `$position_deletes` is the exception 
that uses the native reader. This change also sends its `row` schema/default 
carrier and advertises semantics v2, so a historical row missing a newly 
defaulted complex child (or a newly required child) can return different 
results on source and current BEs. Please apply the current-semantics backend 
requirement when `$position_deletes.row` needs these rules (or conservatively 
for native `row` reads), and cover the mixed-version case.



-- 
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