pzhdfy commented on code in PR #68988:
URL: https://github.com/apache/doris/pull/68988#discussion_r4321185063


##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/source/PaimonScanNode.java:
##########
@@ -982,72 +1008,156 @@ && splitHasOrcFile((DataSplit) split)
             // rust. Gate until the rust leaf has a Variant Arrow decoder.
             boolean projectedVariant = desc.getSlots().stream()
                     .anyMatch(slot -> 
PaimonUtil.containsVariant(slot.getType()));
-            // Scheme capability gate: the pinned paimon-rust storage
-            // dispatcher (io/storage.rs) selects the FileIO parser from the
-            // table location's URI scheme, and libpaimon_c.a compiles in
-            // separate COS, OBS, GCS and Azdls parsers besides the OSS and S3
-            // ones. Doris normalizes every object store's credentials into
-            // the AWS_* / use_path_style aliases (see the *Properties storage
-            // classes), which the BE rust bridge translates only into the
-            // fs.oss.* and s3.* key families — a cosn:// / obs:// / gs:// /
-            // abfs:// warehouse would reach its scheme's parser without the
-            // key family it reads (fs.cosn.userinfo.*, fs.obs.*, gcs.*,
-            // azure.*) and fail the open instead of using JNI. Only the
-            // schemes whose property translation is implemented and
-            // open-tested (s3 / s3a / oss, via RUST_VERIFIED_LOCATION_SCHEMES)
-            // plus the credential-free hdfs and local-filesystem parsers stay
-            // rust-eligible; every other scheme falls back to JNI. A null
-            // location also routes to JNI: the rust path needs the
-            // paimon_table that only a real location can provide (BE rejects
-            // a split without it).
-            boolean schemeCapabilityVerified = 
isRustVerifiedLocationScheme(source.getTableLocation());
-            // An hdfs:// location is scheme-verified only together with the 
credential-free
-            // backend shape: the backend storage properties that ship to BE 
also carry an
-            // HDFS catalog's authentication (kerberos principal / keytab, 
proxy user, HA
-            // nameservice config), none of which the pinned rust HDFS parser 
reads — the
-            // scan would open as the BE process's ambient identity instead of 
the
-            // catalog's configured one and fail the access JNI honors. See
-            // isRustVerifiedHdfsBackend.
-            boolean hdfsBackendVerified = 
!isHdfsLocationScheme(source.getTableLocation())
-                    || isRustVerifiedHdfsBackend(backendStorageProperties);
-            // With merge-on-read=true the whole table already routes to JNI 
(dvMergeOnRead);
-            // for the remaining partial-update/aggregation DV tables, a split 
that is
+            // With merge-on-read=true the whole table already routes to JNI
+            // (rustEligibility.dvMergeOnRead); for the remaining
+            // partial-update/aggregation DV tables, a split that is
             // not fully materialized (uncompacted level-0 data, or 
retractions not
             // known to be applied — even a split with no deletion file 
attached yet)
             // fails the rust is_fully_materialized_pk_dv guard, so it falls 
back per
             // split instead of turning into a BE-open failure. nativeSplit and
             // !fallbackRead only guard the cast — those splits already route 
to JNI.
-            boolean splitDvNotMaterialized = puAggDeletionVectors && 
!dvMergeOnRead
+            boolean splitDvNotMaterialized = 
rustEligibility.puAggDeletionVectors
+                    && !rustEligibility.dvMergeOnRead
                     && nativeSplit && !fallbackRead
                     && !isFullyMaterializedPkDvSplit((DataSplit) split);
+            boolean vectorSearchSplit = paimonSplit.getVectorPayloadBytes() != 
null;
+            // Session, shared table / storage / backend, and scan-shape 
gates. The
+            // shared PaimonRustEligibility computation is also the
+            // PushDownVectorTopNIntoPaimonScan rewrite's pre-flight
+            // (PaimonVectorSearch.vectorScanEligible): the rewrite is 
one-way, so both
+            // sites must agree on the same gates, and the scan node stays 
authoritative
+            // — it evaluates the relation-option-processed table, so a 
t@options
+            // override the raw-table pre-flight cannot see still rejects here 
(loudly,
+            // as the vector path has no JNI fallback). !isIncremental and the
+            // split-half of the fallback-read gate are the same backstops: an
+            // incremental scan has no PK-vector semantics (the VectorScan 
builder has
+            // no incremental mode), and a FallbackSplit shape cannot ride the 
vector
+            // payload either — both fail loudly here rather than silently 
returning
+            // snapshot rows for a query that asked for changes. Each gate's 
value is
+            // also what the fallback logging below reports.
             boolean canUseRust = sessionVariable.isEnablePaimonRustReader()
-                    && sessionVariable.enableFileScannerV2 && nativeSplit && 
!fallbackRead
-                    && !isIncremental && providerModeTranslatable && 
!queryAuthTable
-                    && !restTokenTable
-                    && !dvMergeOnRead && !splitDvNotMaterialized
-                    && !orcLtzSchema && !projectedVariant && !externalFileSplit
-                    && !deduplicateIgnoreDelete && !rustUnsupportedMergeOption
-                    && schemeCapabilityVerified && hdfsBackendVerified
-                    && paimonFileStoreTable != null;
-            if (canUseRust) {
-                // Branch tables can retain persisted modes without a 
selector. Validate the
-                // transported options too: the pinned Rust builder only 
accepts default here.
-                String scanMode = 
PaimonScanParams.withoutTimeTravelSelectors(paimonFileStoreTable.schema())
-                        .options().get(CoreOptions.SCAN_MODE.key());
-                canUseRust = scanMode == null || 
"default".equalsIgnoreCase(scanMode);
-            }
-            if (canUseRust) {
-                // Every candidate can receive this range; older BEs cannot 
decode reader type 3.
-                canUseRust = !backendPolicy.getBackends().isEmpty()
-                        && 
backendPolicy.getBackends().stream().allMatch(Backend::isPaimonRustReaderSupported);
-            }
-            if (canUseRust) {
-                if (rustReaderCapabilities == null) {
-                    rustReaderCapabilities = new 
PaimonRustReaderCapabilities(paimonFileStoreTable, desc);
+                    && sessionVariable.enableFileScannerV2
+                    && rustEligibility.eligible()
+                    && !fallbackRead && !isIncremental
+                    && !orcLtzSchema && !projectedVariant;
+            // The remaining gates judge the DataSplit the BATCH reader opens: 
the
+            // native DataSplit shape, unmaterialized partial-update / 
aggregation
+            // deletion vectors, external-path files, and the
+            // PaimonRustReaderCapabilities whitelist. A PK-vector search 
split is
+            // never read as a DataSplit (it rides serialized in 
vector_payload and
+            // executes paimon-rust's eager Top-K chain, which applies deletion
+            // vectors and the merge inside the search itself), so it does not
+            // consult them.
+            if (!vectorSearchSplit && canUseRust) {

Review Comment:
    Confirmed and fixed. The vector split skipped canRead entirely, and 
hasFullNestedProjection was the one gate inside it that is NOT 
DataSplit-specific — the ABI projects root names and the Arrow struct SerDe 
binds children by ordinal, so a pruned complex slot is wrong on the vector path 
exactly as on the batch path.
   ▎
   ▎ Two layers now: the rewrite pre-flight declines when any project 
expression consumes a complex-typed column other than as a bare slot or inside 
the distance call itself (those slots never reach the scan — the rewrite 
replaces every occurrence of the function with the synthetic slot; my first cut 
missed the distance-function exemption and the existing nested-occurrence UT 
caught it); the scan node additionally keeps hasFullNestedProjection for vector 
splits as a loud backstop, with a pruned_nested_projection fallback reason. 
Covered by UT: pruned MAP consumption declines, whole-column pass-through and 
dist*2 aliases stay eligible.



-- 
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]

Reply via email to