Copilot commented on code in PR #67207:
URL: https://github.com/apache/doris/pull/67207#discussion_r3870579463
##########
be/src/exec/scan/file_scanner_v2.cpp:
##########
@@ -196,7 +196,11 @@ bool is_wal_format(TFileFormatType::type format_type) {
bool is_partition_slot(const TFileScanSlotInfo& slot_info, const std::string&
column_name) {
if (column_name.starts_with(BeConsts::GLOBAL_ROWID_COL) ||
- column_name == BeConsts::ICEBERG_ROWID_COL) {
+ column_name == BeConsts::ICEBERG_ROWID_COL ||
+ column_name == BeConsts::ICEBERG_FILE_PATH_COL ||
+ column_name == BeConsts::ICEBERG_ROW_POSITION_COL ||
+ column_name == BeConsts::PAIMON_FILE_PATH_COL ||
+ column_name == BeConsts::PAIMON_ROW_POSITION_COL) {
return false;
}
Review Comment:
These metadata-column checks are case-sensitive (`column_name == ...`), but
the rest of the stack treats metadata columns case-insensitively (e.g., FE
`classifyColumn` uses `equalsIgnoreCase`). If the query requests `_POS` /
`_FILE` (or any case variant), BE may misclassify the slot as
partition/data-file and attempt to resolve it against the file schema. Use a
case-insensitive comparison (e.g., `iequal`) or normalize `column_name` before
comparing for `_file`, `_pos`, and Paimon metadata columns (and apply the same
fix in `is_data_file_slot`).
##########
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;
+}
Review Comment:
`_data_file_path()` returns a `std::string` by value, which copies
`_original_file_path` each time metadata is materialized. Since this is used
during block materialization, consider returning `const std::string&` (or
`std::string_view`) to avoid repeated allocations/copies.
--
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]
