Gabriel39 commented on code in PR #66413:
URL: https://github.com/apache/doris/pull/66413#discussion_r3790907158
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/scan/PluginDrivenScanNode.java:
##########
@@ -194,6 +205,87 @@ public PluginDrivenScanNode(PlanNodeId id, TupleDescriptor
desc,
this.currentHandle = tableHandle;
}
+ @Override
+ protected void doInitialize() throws UserException {
+ super.doInitialize();
+ // Compatibility must inspect the snapshot-specific handle: latest
metadata may answer
+ // COUNT(*) while an older time-travel snapshot still requires a
Variant data scan.
+ pinMvccSnapshot();
Review Comment:
Fixed in 1ed27dc518f. The pinned handle is still initialized in
doInitialize(), while the backend compatibility decision now runs in
doFinalize() after scan-slot pruning. PluginDrivenScanNodeCompatibilityTest
covers the init -> prune -> finalize lifecycle.
##########
be/src/format_v2/table/iceberg_reader.cpp:
##########
@@ -64,6 +64,94 @@ namespace doris::format::iceberg {
static constexpr const char* ROW_LINEAGE_ROW_ID = "_row_id";
static constexpr int32_t ROW_LINEAGE_ROW_ID_FIELD_ID = 2147483540;
+namespace {
+
+bool contains_variant_type(const DataTypePtr& input) {
+ if (input == nullptr) {
+ return false;
+ }
+ const auto type = remove_nullable(input);
+ switch (type->get_primitive_type()) {
+ case TYPE_VARIANT:
+ return true;
+ case TYPE_ARRAY:
+ return contains_variant_type(assert_cast<const
DataTypeArray&>(*type).get_nested_type());
+ case TYPE_MAP: {
+ const auto& map = assert_cast<const DataTypeMap&>(*type);
+ return contains_variant_type(map.get_key_type()) ||
+ contains_variant_type(map.get_value_type());
+ }
+ case TYPE_STRUCT:
+ return std::ranges::any_of(assert_cast<const
DataTypeStruct&>(*type).get_elements(),
+ contains_variant_type);
+ default:
+ return false;
+ }
+}
+
+bool mapping_reads_variant(const format::ColumnMapping& mapping) {
+ if (!mapping.file_local_id.has_value()) {
+ return false;
+ }
+ if (contains_variant_type(mapping.original_file_type)) {
+ return true;
+ }
+ if (mapping.table_type != nullptr &&
+ remove_nullable(mapping.table_type)->get_primitive_type() ==
TYPE_VARIANT) {
+ return true;
+ }
+ return std::ranges::any_of(mapping.child_mappings, mapping_reads_variant);
+}
+
+const char* file_format_name(FileFormat format) {
+ switch (format) {
+ case FileFormat::PARQUET:
+ return "PARQUET";
+ case FileFormat::ORC:
+ return "ORC";
+ case FileFormat::CSV:
+ return "CSV";
+ case FileFormat::JSON:
+ return "JSON";
+ case FileFormat::TEXT:
+ return "TEXT";
+ case FileFormat::JNI:
+ return "JNI";
+ case FileFormat::NATIVE:
+ return "NATIVE";
+ case FileFormat::ARROW:
+ return "ARROW";
+ case FileFormat::WAL:
+ return "WAL";
+ }
+ return "UNKNOWN";
+}
+
+} // namespace
+
+Status IcebergTableReader::validate_variant_file_mappings(
+ FileFormat format, const std::vector<format::ColumnMapping>& mappings)
{
+ if (format == FileFormat::PARQUET || !std::ranges::any_of(mappings,
mapping_reads_variant)) {
+ return Status::OK();
+ }
+ // Gate on a physical mapping, not the table schema: an older ORC/Avro
file may legitimately
+ // omit a Variant field added by schema evolution, in which case the
mapper synthesizes NULL.
+ return Status::NotSupported(
+ "Iceberg Variant is supported only for Parquet files in
FileScannerV2; file format {} "
+ "(including ORC/Avro readers) is not supported",
+ file_format_name(format));
+}
+
+Status IcebergTableReader::validate_file_mapping(const
format::TableColumnMapper& mapper) const {
+ if (_push_down_agg_type == TPushAggOp::type::COUNT &&
_push_down_count_columns.has_value() &&
Review Comment:
Fixed in 1ed27dc518f. The Iceberg mapping exemption now requires
_supports_aggregate_pushdown(COUNT), so position deletes and deletion vectors
keep physical Variant validation active. IcebergV2ReaderTest covers ORC Variant
with a position delete.
##########
regression-test/suites/external_table_p0/remote_doris/test_remote_doris_variant_select.groovy:
##########
@@ -111,8 +111,9 @@ suite("test_remote_doris_variant_select", "p0,external") {
sql """
select * from
`${catalog_arrow_name}`.`${db_name}`.`test_remote_doris_variant_select_t` order
by id
"""
- // check exception message contains
- exception "[NOT_IMPLEMENTED_ERROR]read_column_from_arrow with type
variant"
+ // Keep the concatenation inside one DSL argument; a leading '+'
starts a unary expression.
+ exception("External Variant is supported only for Parquet files in
FileScannerV2; "
Review Comment:
Fixed in 1ed27dc518f. FileScannerV2 now rejects unsupported Arrow Variant
projections before reader materialization with the deterministic
format-specific error. FileScannerV2Test covers this production path.
--
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]
