github-actions[bot] commented on code in PR #65503:
URL: https://github.com/apache/doris/pull/65503#discussion_r3567610226
##########
be/src/exec/operator/file_scan_operator.cpp:
##########
@@ -116,10 +116,17 @@ bool
FileScanLocalState::_should_use_file_scanner_v2(const TQueryOptions& query_
const bool is_transactional_hive =
scan_params.__isset.table_format_params &&
scan_params.table_format_params.table_format_type ==
"transactional_hive";
+ // FE stores Paimon's table-format descriptor per split, but
paimon_predicate is a scan-level
+ // Paimon marker. PAIMON_CPP is also selected per split, and FileScannerV2
cannot dispatch that
+ // reader type, so retain the V1 path until the V2 hybrid reader supports
it.
+ const bool uses_paimon_cpp_reader = scan_params.__isset.paimon_predicate &&
Review Comment:
The exclusion still misses the rolling-upgrade shape where Paimon metadata
is only on each split. This selector only sees `TFileScanRangeParams`, so an
old FE that has `enable_paimon_cpp_reader=true` but still sends Paimon
predicate/reader metadata only in
`TFileRangeDesc.table_format_params.paimon_params` will leave
`scan_params.__isset.paimon_predicate` false and V2 remains selected. The first
`FORMAT_JNI` range with `TPaimonReaderType::PAIMON_CPP` is not supported by V2:
`FileScannerV2::is_supported()` accepts Paimon JNI only for `PAIMON_JNI`, and
`PaimonHybridReader` dispatches only `PAIMON_JNI` to the JNI child. Please
either use a marker that is guaranteed for both current and rolling-upgrade
Paimon CPP scans, or conservatively keep JNI Paimon CPP on V1 until the
split-level reader type can be inspected, and add a test for scan params
without scan-level `paimon_predicate` plus a split-level `PAIMON_CPP` range.
##########
be/src/format_v2/jni/jni_table_reader.cpp:
##########
@@ -287,9 +298,15 @@ Status JniTableReader::close() {
if (_closed) {
return Status::OK();
}
- _closed = true;
- RETURN_IF_ERROR(_close_jni_scanner());
Review Comment:
This makes the JNI reader itself retryable, but the normal FileScannerV2
lifecycle still cannot reach the retry. `FileScannerV2::close()` calls
`_try_close()` before `_table_reader->close()`, and `_try_close()` marks the
scanner closed immediately. If Java `releaseTable()` or `close()` fails here,
`JniTableReader` deliberately keeps `_closed=false` and preserves the Java
object, but the next `FileScannerV2::close()` returns OK at the outer close
guard without calling back into this reader. At that point destroying the
reader only drops the JNI reference; Java cleanup is never retried. Please make
the scanner-level close state commit only after table-reader cleanup succeeds,
or track table-reader cleanup with a separate retryable flag, and cover the
failing-then-succeeding close path through `FileScannerV2`.
--
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]
