Gabriel39 commented on code in PR #67904:
URL: https://github.com/apache/doris/pull/67904#discussion_r3998973776
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnectorMetadata.java:
##########
@@ -733,9 +734,9 @@ public Optional<ConnectorMvccSnapshot> resolveTimeTravel(
// latestSnapshot() only).
Table branchTable =
resolveTable(paimonHandle.withBranch(branchName));
long snapshotId =
catalogOps.latestSnapshotId(branchTable).orElse(-1L);
- long schemaId = snapshotId < 0
- ? -1L
- : catalogOps.snapshotSchemaId(branchTable,
snapshotId).orElse(-1L);
+ // A schema-only ALTER advances the branch schema without
creating a data snapshot.
+ // Keep the data fence but bind the branch's current schema
instead of that snapshot's old schema.
+ long schemaId = -1L;
Review Comment:
Fixed in 4ffdc1fc1d. The branch name now travels with the resolved positive
or empty data fence and the exact current branch schema. Real Paimon mutation
tests change both schema and data after resolution, including the first commit
to an empty branch, and verify planning plus serialized reads.
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonScanPlanProvider.java:
##########
@@ -346,7 +346,13 @@ Table resolveScanTable(PaimonTableHandle paimonHandle) {
Map<String, String> scanOptions = paimonHandle.getScanOptions();
Table finalTable = table;
if (scanOptions != null && !scanOptions.isEmpty()) {
- if (PaimonScanParams.isOptionsPin(scanOptions)) {
+ if (table instanceof FileStoreTable
+ && PaimonScanParams.preservesBoundSchema(scanOptions)) {
Review Comment:
Fixed in 4ffdc1fc1d. Statement-fenced system aliases apply the bound schema
to their source before rebuilding the wrapper for binding, native schema
dictionaries and JNI serialization. A warm-table $ro OPTIONS test renames the
source again after binding and verifies all three paths retain the bound field.
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonScanPlanProvider.java:
##########
@@ -346,7 +346,13 @@ Table resolveScanTable(PaimonTableHandle paimonHandle) {
Map<String, String> scanOptions = paimonHandle.getScanOptions();
Table finalTable = table;
if (scanOptions != null && !scanOptions.isEmpty()) {
- if (PaimonScanParams.isOptionsPin(scanOptions)) {
+ if (table instanceof FileStoreTable
+ && PaimonScanParams.preservesBoundSchema(scanOptions)) {
+ // A statement fence owns data visibility, not schema time
travel. Reusing Table.copy
+ // here would roll schema-only ALTERs back to the data
snapshot's older schema.
+ finalTable = PaimonScanParams.applyOptionsWithoutTimeTravel(
+ (FileStoreTable) table, scanOptions);
Review Comment:
Fixed in 4ffdc1fc1d. Statement snapshots now carry the exact current schema
ID independently of the data snapshot. The generic MVCC layer retains that
schema for analysis, and scan copies restore it from stale table handles. The
catalog-loader removal path also preserves the schema rather than letting the
factory time-travel again. Warm-table tests cover plain and reader-only OPTIONS
reads, native dictionaries and serialized reads; the FE MVCC suite also passes.
##########
regression-test/suites/external_table_p0/paimon/test_paimon_schema_only_snapshot_precision.groovy:
##########
@@ -0,0 +1,124 @@
+// 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_schema_only_snapshot_precision", "p0,external,paimon") {
+ String enabled = context.config.otherConfigs.get("enablePaimonTest")
+ if (enabled == null || !enabled.equalsIgnoreCase("true")) {
+ logger.info("disable paimon test")
+ return
+ }
+
+ String minioPort = context.config.otherConfigs.get("iceberg_minio_port")
+ String externalEnvIp = context.config.otherConfigs.get("externalEnvIp")
+ String catalogName = "test_paimon_schema_only_snapshot_precision"
+ String dbName = "paimon_schema_only_snapshot_precision_db"
+ String tableName = "schema_only_timeline"
+ String branchName = "schema_only_branch"
+
+ def latestSnapshotId = {
+ List<List<Object>> rows = spark_paimon """
+ select snapshot_id
+ from paimon.${dbName}.`${tableName}\$snapshots`
+ order by snapshot_id desc
+ limit 1
+ """
+ assertEquals(1, rows.size())
+ return rows[0][0].toString()
+ }
+
+ sql """drop catalog if exists ${catalogName}"""
+ sql """
+ create catalog ${catalogName} properties (
+ 'type'='paimon',
+ 'warehouse'='s3://warehouse/wh',
+ 's3.endpoint'='http://${externalEnvIp}:${minioPort}',
+ 's3.access_key'='admin',
+ 's3.secret_key'='password',
+ 's3.path.style.access'='true',
+ 'meta.cache.paimon.table.ttl-second'='0'
+ )
+ """
+
+ try {
+ spark_paimon_multi """
+ create database if not exists paimon.${dbName};
+ drop table if exists paimon.${dbName}.${tableName};
+ create table paimon.${dbName}.${tableName} (
+ id int,
+ old_name string,
+ event_time timestamp
Review Comment:
Fixed in 4ffdc1fc1d. The Spark column and inserted literal now explicitly
use TIMESTAMP_NTZ, so the precision regression targets the NTZ pushdown path.
The external Spark/Doris suite was not run locally; the Paimon module tests and
FE MVCC tests pass.
--
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]