github-actions[bot] commented on code in PR #67207:
URL: https://github.com/apache/doris/pull/67207#discussion_r3879023055
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnectorMetadata.java:
##########
@@ -257,15 +259,17 @@ public ConnectorTableSchema getTableSchema(
table,
schema.fields(),
schema.partitionKeys(),
- schema.primaryKeys());
+ schema.primaryKeys(),
+ true);
}
}
return buildTableSchema(
paimonHandle.getTableName(),
table,
table.rowType().getFields(),
paimonHandle.getPartitionKeys(),
- table.primaryKeys());
+ table.primaryKeys(),
+ !paimonHandle.isSystemTable());
Review Comment:
[P1] Please do not exempt inherited-schema Paimon system tables from this
collision check. `$ro` (and other inherited views) can carry a legitimate base
field named `__paimon_file_path` or `__paimon_row_index`; resolving `t$ro` does
not load the guarded base schema first. Its handle remains physical, but the
name-only classifier marks it SYNTHESIZED and the BE mapper gives the virtual
name precedence, so `$ro` returns file metadata instead of the stored value.
Reject collisions for inherited system schemas/handles too, or make
classification/materialization honor the physical handle, and cover `$ro`.
##########
regression-test/suites/external_table_p0/paimon/test_paimon_file_metadata_columns.groovy:
##########
@@ -0,0 +1,108 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+suite("test_paimon_file_metadata_columns", "p0,external") {
+ String enabled = context.config.otherConfigs.get("enablePaimonTest")
+ if (enabled == null || !enabled.equalsIgnoreCase("true")) {
+ logger.info("Paimon test is disabled")
+ return
+ }
+
+ String catalogName = "test_paimon_file_metadata_columns"
+ String hdfsPort = context.config.otherConfigs.get("hive2HdfsPort")
+ String externalEnvIp = context.config.otherConfigs.get("externalEnvIp")
+
+ def verifyMetadata = { String tableName ->
+ def rows = sql """
+ select id, `__paimon_file_path`, `__paimon_row_index`
+ from ${tableName}
+ where id > 2
+ order by id, `__paimon_file_path`, `__paimon_row_index`
+ """
+ assertTrue(rows.size() > 0)
Review Comment:
[P2] Please make this regression prove the advertised path and row-index
semantics. Every assertion below still passes if BE returns the normalized
Doris read path instead of `RawFile.path()`, or if it densely renumbers
surviving rows after predicate/deletion filtering; uniqueness only catches
duplicate pairs within this scan. Compare the path against an authoritative
raw-path oracle (or a fixture whose raw and normalized paths differ), and
compare a stable key-to-metadata mapping across unfiltered, filtered, and
differently split reads. A deletion-vector case also needs a known
physical-position gap to prove deleted rows were not closed up.
--
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]
