github-actions[bot] commented on code in PR #66247:
URL: https://github.com/apache/doris/pull/66247#discussion_r3679236492
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonExternalCatalog.java:
##########
@@ -136,9 +137,12 @@ public Table getPaimonTable(NameMapping nameMapping,
String branch, String query
}
return executionAuthenticator.execute(() -> {
Table table = catalog.getTable(identifier);
- Map<String, String> tableOptions =
-
paimonProperties.getTableOptionsForCopy(table.options());
- return tableOptions.isEmpty() ? table :
table.copy(tableOptions);
+ Map<String, String> tableOptions =
paimonProperties.getTableOptionsForCopy();
+ Table effectiveTable = tableOptions.isEmpty() ? table :
table.copy(tableOptions);
+ // Physical table options bypass Doris property validation, so
validate the final
+ // merged view before Paimon can allocate batches or replace
its manifest executor.
+
PaimonReaderOptions.validateEffectiveTableOptions(effectiveTable.options());
Review Comment:
[P1] Validate after relation-scoped options take precedence
This validates only the physical-plus-catalog handle.
`PaimonExternalTable.getPaimonTable(TableScanParams)` cannot apply
`PaimonScanParams.applyOptions` until the cached/base handle returns, so a
physical `read.batch-size=0` with `@options('read.batch-size'='4096')` is
rejected here even though the documented relation > catalog > physical
precedence makes the final value safe. The same ordering affects relation-level
manifest parallelism and system-table copies. Please form the complete option
copy chain before final validation (while retaining the no-override rejection)
and add coverage for safe catalog and relation overrides of unsafe physical
values.
##########
fe/be-java-extensions/paimon-scanner/src/main/java/org/apache/doris/paimon/PaimonJniScanner.java:
##########
@@ -483,7 +498,51 @@ private static long nonNegative(long value) {
}
private static int currentAsyncReaderThreadCount() {
- return countThreadsByNamePrefix(ASYNC_READER_THREAD_NAME_PREFIX);
+ return ASYNC_READER_THREAD_COUNTER.get();
+ }
+
+ static String buildDebugSummary(int batchSize, int requiredFieldCount) {
+ return "Paimon JNI scanner configuration: batchSize=" + batchSize
+ + ", requiredFieldCount=" + requiredFieldCount;
+ }
+
+ static String[] requiredFields(Map<String, String> params) {
+ String encodedFields = params.get("required_fields_base64");
+ if (encodedFields == null) {
+ return splitParam(params.get("required_fields"), ",");
+ }
+ if (encodedFields.isEmpty()) {
Review Comment:
[P1] Distinguish one empty identifier from zero projected fields
A single empty field name still serializes to the same payload as an empty
projection: Base64 of `""` is `""`, so C++ `join({""}, ",")` and `join({},
",")` both publish an empty `required_fields_base64`. This branch turns that
into zero fields, while the paired encoded type list contains one type, and the
constructor rejects the scan on cardinality. Paimon 1.3.1 permits a
programmatically authored empty `DataField` name, and Doris imports that name
without `FeNameFormat` validation, so `SELECT *` on a one-column external table
reaches both V1 and V2 producers. Please make the element count unambiguous
(for example, prefix every token or carry an explicit count) and test `{}`
separately from `{""}` on both routes.
--
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]
