Copilot commented on code in PR #67207:
URL: https://github.com/apache/doris/pull/67207#discussion_r3870579524
##########
be/src/format_v2/table/paimon_reader.cpp:
##########
@@ -280,10 +292,105 @@ Status
PaimonReader::annotate_file_schema(std::vector<format::ColumnDefinition>*
Status PaimonReader::customize_file_scan_request(format::FileScanRequest*
file_request) {
DORIS_CHECK(file_request != nullptr);
RETURN_IF_ERROR(format::TableReader::customize_file_scan_request(file_request));
+ if (_need_metadata_columns()) {
+ if (_original_file_path.empty()) {
+ return Status::InvalidArgument(
+ "Paimon metadata columns require a FileScannerV2 native
Parquet/ORC split with "
+ "the original RawFile path");
+ }
+ RETURN_IF_ERROR(_append_row_position_output_column(file_request));
+ }
file_request->variant_schema_overrides = _variant_schema_overrides;
return Status::OK();
}
+Status PaimonReader::materialize_virtual_columns(Block* table_block) {
+ DORIS_CHECK(table_block != nullptr);
+ for (size_t column_idx = 0; column_idx <
_data_reader.column_mapper->mappings().size();
+ ++column_idx) {
+ const auto& mapping =
_data_reader.column_mapper->mappings()[column_idx];
+ switch (mapping.virtual_column_type) {
+ case format::TableVirtualColumnType::PAIMON_FILE_PATH:
+ RETURN_IF_ERROR(_materialize_file_path(table_block, column_idx));
+ break;
+ case format::TableVirtualColumnType::PAIMON_ROW_POSITION:
+ RETURN_IF_ERROR(_materialize_row_position(table_block,
column_idx));
+ break;
+ default:
+ break;
+ }
+ }
+ return Status::OK();
+}
+
+std::string PaimonReader::_data_file_path() const {
+ DORIS_CHECK(!_original_file_path.empty());
+ return _original_file_path;
+}
+
+Status
PaimonReader::_append_row_position_output_column(format::FileScanRequest*
request) {
+ const auto row_position_column_id =
format::LocalColumnId(format::ROW_POSITION_COLUMN_ID);
+ _append_file_scan_column(request, row_position_column_id,
&request->non_predicate_columns);
+ _row_position_block_position =
request->local_positions.at(row_position_column_id).value();
+ return Status::OK();
+}
+
+Status PaimonReader::_materialize_file_path(Block* table_block, size_t
column_idx) {
+ DORIS_CHECK(_row_position_block_position <
_data_reader.block_template.columns());
+ const auto& row_position_column = assert_cast<const ColumnInt64&>(
+
*_data_reader.block_template.get_by_position(_row_position_block_position).column);
+ auto column =
table_block->get_by_position(column_idx).type->create_column();
+ auto* nullable_column = check_and_get_column<ColumnNullable>(*column);
+ auto* string_column = nullable_column != nullptr
+ ? check_and_get_column<ColumnString>(
+
nullable_column->get_nested_column_ptr().get())
+ :
check_and_get_column<ColumnString>(column.get());
+ DORIS_CHECK(string_column != nullptr);
+ const auto file_path = _data_file_path();
+ string_column->insert_data(file_path.data(), file_path.size());
+ if (nullable_column != nullptr) {
+ nullable_column->get_null_map_data().resize_fill(1, 0);
+ }
+ table_block->replace_by_position(
+ column_idx, ColumnConst::create(std::move(column),
row_position_column.size()));
+ return Status::OK();
+}
+
+Status PaimonReader::_materialize_row_position(Block* table_block, size_t
column_idx) {
+ DORIS_CHECK(_row_position_block_position <
_data_reader.block_template.columns());
+ const auto& row_position_column = assert_cast<const ColumnInt64&>(
+
*_data_reader.block_template.get_by_position(_row_position_block_position).column);
+ auto column =
table_block->get_by_position(column_idx).type->create_column();
Review Comment:
Unlike the Iceberg implementation, the Paimon materialization path does not
assert that `row_position_column.size()` matches `table_block->rows()`. If
filtering/predicate pushdown causes the output block to have a different row
count than the block template column, using `row_position_column.size()` to
size the const column (and copying row positions without validating sizes) can
produce incorrect results or trigger hard checks later. Add an explicit size
check (like Iceberg does) and prefer `table_block->rows()` as the authoritative
output size.
--
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]
