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]