Gabriel39 commented on code in PR #66247:
URL: https://github.com/apache/doris/pull/66247#discussion_r3680118680
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonSysExternalTable.java:
##########
@@ -117,15 +120,24 @@ private static long generateSysTableId(long
sourceTableId, String sysTableType)
* Note: system tables currently ignore snapshot semantics.
*/
public Table getSysPaimonTable() {
+ validateEffectiveDataTable(null);
Review Comment:
Fixed in 0f07b707b69. `buildFullSchema()` now reads the cached raw system
wrapper only for its row type, so descriptor serialization no longer re-enters
scan validation against the physical handle after relation binding accepted a
safe override. The FE test now carries the unsafe-physical/safe-relation case
through `toThrift()`, and the P0 query is a named result assertion through
descriptor serialization.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonExternalTable.java:
##########
@@ -178,6 +178,19 @@ private PaimonSnapshotCacheValue
getPaimonSnapshotCacheValue(Optional<TableSnaps
"Failed to get Paimon snapshot: " + (e.getMessage() ==
null ? "unknown cause" : e.getMessage()),
e);
}
+ } else if (scanParams.isPresent() && scanParams.get().isOptions()) {
+ Table baseTable = getBasePaimonTable();
+ Map<String, String> resolvedOptions =
scanParams.get().getOrResolveMapParams(
+ options -> PaimonScanParams.resolveOptions(baseTable,
options));
+ Table effectiveTable = PaimonScanParams.applyOptions(baseTable,
resolvedOptions);
+ if (PaimonScanParams.hasOnlyReaderOptions(resolvedOptions)) {
+ // Reader tuning cannot change snapshot metadata. Reuse the
memoized projection so
+ // a per-query batch-size change does not enumerate every
partition again.
+ return PaimonUtils.getLatestSnapshotCacheValue(this);
Review Comment:
Fixed in 0f07b707b69. Partition projection now validates only
manifest-planning settings; reader batch and async settings remain validated on
the final effective scan handle. This lets a memoized partition projection use
the raw metadata handle without rejecting a reader-only physical value that the
relation copy overrides. Added both an FE metadata-loader test and a
partitioned P0 case with physical `read.batch-size=0` plus relation
`read.batch-size=4096`.
##########
be/src/format_v2/jni/jni_table_reader.cpp:
##########
@@ -511,13 +513,21 @@ void JniTableReader::_prepare_jni_scanner_schema() {
required_fields.push_back(column.java_name);
column_types.push_back(
JniDataBridge::get_jni_type_with_different_string(column.transfer_type));
+ encoded_column_types.push_back(
Review Comment:
Fixed in 0f07b707b69. Encoded schema publication is now a V2 JNI reader
capability that defaults to disabled and is enabled only by `PaimonJniReader`.
Unrelated connectors keep the legacy schema fields and skip recursive
encoded-type generation, Base64 encoding, and the two extra map entries. Added
boundary tests proving generic JNI readers omit the fields while Paimon still
publishes them.
##########
regression-test/suites/external_table_p0/paimon/test_paimon_jni_reader_guardrails.groovy:
##########
@@ -0,0 +1,224 @@
+// 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_jni_reader_guardrails", "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_jni_reader_guardrails"
+ String physicalCatalogName = "test_paimon_jni_reader_physical_guardrails"
+ String dbName = "paimon_jni_guardrails_db"
+ def scannerV2Rows = sql "show variables like 'enable_file_scanner_v2'"
+ String originalScannerV2 = scannerV2Rows[0][1]
+
+ def catalogDdl = { String name, String extraProperty ->
+ return """
+ create catalog ${name} 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'
+ ${extraProperty}
+ )
+ """
+ }
+
+ for (def invalid : [
+ ["invalid_batch", ", 'paimon.table-option.read.batch-size'='0'",
"read.batch-size"],
+ ["unsafe_branch", ", 'paimon.table-option.branch'='archive'",
"branch"]
+ ]) {
+ String invalidCatalog = "${catalogName}_${invalid[0]}"
+ sql "drop catalog if exists ${invalidCatalog}"
+ try {
+ test {
+ sql(catalogDdl(invalidCatalog, invalid[1]))
+ exception invalid[2]
+ }
+ } finally {
+ sql "drop catalog if exists ${invalidCatalog}"
+ }
+ }
+
+ sql "drop catalog if exists ${catalogName}"
+ sql(catalogDdl(catalogName, """
+ , 'paimon.table-option.read.batch-size'='1024'
+ , 'paimon.table-option.file-reader-async-threshold'='16 MB'
+ """))
+
+ try {
+ spark_paimon_multi """
+ create database if not exists paimon.${dbName};
+ drop table if exists paimon.${dbName}.quoted_reader_options;
+ create table paimon.${dbName}.quoted_reader_options (
+ id int,
+ `region,code` string,
+ `hash#name` string,
+ `display name` string,
+ `地区 名` string,
+ `nested#value`
struct<`hash#name`:string,`region,code`:string,`colon:name`:string>
+ ) using paimon
+ tblproperties ('file.format'='parquet', 'read.batch-size'='512');
+ insert into paimon.${dbName}.quoted_reader_options values
+ (1, 'east,01', 'hash-one', 'first row', '华东',
+ named_struct('hash#name', 'nested-one', 'region,code',
'east,01', 'colon:name', 'a:1')),
+ (2, 'west,02', 'hash-two', 'second row', '华西',
+ named_struct('hash#name', 'nested-two', 'region,code',
'west,02', 'colon:name', 'b:2'));
+ drop table if exists paimon.${dbName}.unsafe_physical_batch;
+ create table paimon.${dbName}.unsafe_physical_batch (id int) using
paimon
+ tblproperties ('read.batch-size'='0');
+ insert into paimon.${dbName}.unsafe_physical_batch values (1);
+ drop table if exists paimon.${dbName}.unsafe_physical_manifest;
+ create table paimon.${dbName}.unsafe_physical_manifest (id int,
part int) using paimon
+ partitioned by (part)
+ tblproperties ('scan.manifest.parallelism'='0');
+ insert into paimon.${dbName}.unsafe_physical_manifest values (1,
10);
+ drop table if exists paimon.${dbName}.empty_identifier;
+ create table paimon.${dbName}.empty_identifier (`` string) using
paimon;
+ insert into paimon.${dbName}.empty_identifier values
('empty-name');
+ """
+
+ sql "switch ${catalogName}"
+ sql "use ${dbName}"
+ sql "set force_jni_scanner=true"
+
+ test {
Review Comment:
Fixed in 0f07b707b69. All positive guardrail paths now use named
`qt_`/`order_qt_` assertions with an expected-result file, including quoted and
nested identifiers in both scanner modes, the empty identifier, catalog and
relation overrides, relation isolation, the failed-ALTER follow-up, partitioned
reader-option composition, manifest override, and system-table descriptor
serialization. Negative paths remain `test { ... exception ... }` cases.
--
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]
