github-actions[bot] commented on code in PR #66227:
URL: https://github.com/apache/doris/pull/66227#discussion_r4058981946
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/source/PaimonScanNode.java:
##########
@@ -411,10 +412,102 @@ private void setPaimonParams(TFileRangeDesc rangeDesc,
PaimonSplit paimonSplit)
String fileFormat = getFileFormat(paimonSplit.getPathString());
if (split != null) {
+ // use jni reader / paimon-cpp reader / paimon-rust reader
rangeDesc.setFormatType(TFileFormatType.FORMAT_JNI);
- // A logical DataSplit may span multiple files, so keep it intact
for the JNI reader.
- fileDesc.setReaderType(TPaimonReaderType.PAIMON_JNI);
- fileDesc.setPaimonSplit(PaimonUtil.encodeObjectToString(split));
+ // paimon-cpp and paimon-rust both consume Paimon native binary
serialization,
+ // which only supports DataSplit. Any other split type falls back
to JNI.
+ boolean nativeSplit = split instanceof DataSplit;
Review Comment:
[P1] Keep fallback-read splits on JNI. Paimon's `FallbackDataSplit` extends
`DataSplit`, so this test passes, but its serializer appends an `isFallback`
byte after the ordinary split. The pinned Rust decoder requires full-buffer
consumption and rejects that trailing byte; even a permissive decoder would
still lack the second table identity needed to honor the fallback-side
discriminator. Please gate fallback wrappers/splits to JNI until the Rust ABI
represents both sides, and add a routing test with a split from the fallback
branch.
##########
thirdparty/build-thirdparty.sh:
##########
@@ -2169,6 +2169,222 @@ build_lance_c() {
fi
}
+# paimon-rust
+build_paimon_rust() {
+ check_if_source_exist "${PAIMON_RUST_SOURCE}"
+ cd "${TP_SOURCE_DIR}/${PAIMON_RUST_SOURCE}"
+
+ rm -rf "${BUILD_DIR}"
+ mkdir -p "${BUILD_DIR}"
+
+ local cargo_bin="${PAIMON_RUST_CARGO:-${CARGO:-cargo}}"
+ if ! command -v "${cargo_bin}" >/dev/null 2>&1; then
+ echo "cargo is required to build paimon-rust. Install Rust 1.91.0 or
set PAIMON_RUST_CARGO."
+ exit 1
+ fi
+
+ local required_rust_version="1.91.0"
+ local cargo_env=(
+ "CARGO_BUILD_JOBS=${PARALLEL}"
+ "CARGO_TARGET_DIR=${PWD}/${BUILD_DIR}"
+ )
+ if command -v rustup >/dev/null 2>&1 && [[ -z "${RUSTUP_TOOLCHAIN}" ]];
then
+ if ! rustup toolchain list | grep -Eq '^1\.91\.0([[:space:]-]|$)'; then
+ rustup toolchain install "${required_rust_version}" --profile
minimal
+ fi
+ cargo_env+=("RUSTUP_TOOLCHAIN=${required_rust_version}")
+ fi
+
+ local cargo_version
+ if ! cargo_version="$(env "${cargo_env[@]}" "${cargo_bin}" --version | awk
'{print $2}')"; then
+ echo "failed to get cargo version for paimon-rust. Install Rust
${required_rust_version} or set PAIMON_RUST_CARGO/RUSTUP_TOOLCHAIN."
+ exit 1
+ fi
+ # Rust 1.91.0 is the minimum supported version. Allow newer toolchains when
+ # callers explicitly select one or rustup is unavailable on the system.
+ # NOTE: paimon_c and lance_c are both Rust staticlibs linked into the same
+ # BE binary; they must be built with the SAME rustc toolchain so the linker
+ # resolves both crates' std references against a single std copy. Mixing
+ # toolchains makes the precompiled std hashes differ and the linker pulls
+ # both std copies in, colliding on the unmangled `rust_eh_personality`
+ # (duplicate symbol). Build lance_c and paimon_rust with one toolchain.
+ if ! awk -v required="${required_rust_version}" -v
actual="${cargo_version}" 'BEGIN {
+ split(required, r, ".");
+ split(actual, a, ".");
+ for (i = 1; i <= 3; i++) {
+ if ((a[i] + 0) > (r[i] + 0)) {
+ exit 0;
+ }
+ if ((a[i] + 0) < (r[i] + 0)) {
+ exit 1;
+ }
+ }
+ exit 0;
+ }'; then
+ echo "paimon-rust requires Rust/Cargo ${required_rust_version} or
newer, but found ${cargo_version}."
+ echo "Install Rust ${required_rust_version} or set
PAIMON_RUST_CARGO/RUSTUP_TOOLCHAIN."
+ exit 1
+ fi
+
+ if [[ "${KERNEL}" != 'Darwin' ]]; then
+ cargo_env+=("CFLAGS=${CFLAGS:-} -std=gnu17")
+ fi
+
+ # paimon-vindex-core 0.4.0 uses the unstable `stdarch_neon_f16` intrinsics
+ # (vcvt_f32_f16 / vreinterpret_f16_u16) in its aarch64 NEON fast path,
which
+ # do not compile on stable Rust. On aarch64/arm64, replace the registry
+ # crate with a patched path source so the build succeeds. The patch swaps
+ # the unstable f16->f32 NEON convert for a stable scalar conversion; the
+ # rest of the NEON accumulation is left untouched. x86_64 and other arches
+ # are unaffected and keep using the pristine registry crate.
+ if [[ "$(uname -m)" == "aarch64" || "$(uname -m)" == "arm64" ]]; then
+ local vindex_override="${PWD}/.doris-vindex-override"
+ local vindex_crate
+ vindex_crate="$(find "${CARGO_HOME:-$HOME/.cargo}/registry/cache" \
+ -type f -name 'paimon-vindex-core-0.4.0.crate' 2>/dev/null | head
-n1)"
+ if [[ -z "${vindex_crate}" ]]; then
+ # Ensure the .crate is downloaded into the registry cache first.
+ local fetch_args=(fetch)
+ if [[ "$(echo "${PAIMON_RUST_CARGO_OFFLINE}" | tr '[:lower:]'
'[:upper:]')" == "ON" ]]; then
+ fetch_args+=(--offline)
+ fi
+ env "${cargo_env[@]}" "${cargo_bin}" "${fetch_args[@]}"
+ vindex_crate="$(find "${CARGO_HOME:-$HOME/.cargo}/registry/cache" \
+ -type f -name 'paimon-vindex-core-0.4.0.crate' 2>/dev/null |
head -n1)"
+ fi
+ if [[ -z "${vindex_crate}" ]]; then
+ echo "failed to locate paimon-vindex-core-0.4.0.crate in the cargo
registry cache"
+ exit 1
+ fi
+ # Verify the cached crate against the workspace Cargo.lock checksum
+ # before extraction. The cache lookup picks an ambient file by name,
+ # and once the [patch.crates-io] path override is applied, cargo
+ # build --locked no longer authenticates those bytes — without this
+ # check a stale or poisoned same-named cache (e.g. from another
+ # registry mirror) would enter libpaimon_c.a, and identical Doris
+ # sources could produce different artifacts. Iterate the candidates
+ # (multiple registries may cache the crate) and use the one whose
+ # sha256 matches the lock; fail when none does.
+ local vindex_checksum
+ vindex_checksum="$(awk '
+ $0 == "[[package]]" { in_pkg = 1; name = ""; version = "";
checksum = ""; next }
+ in_pkg && $1 == "name" { gsub(/[",]/, "", $3); name = $3 }
+ in_pkg && $1 == "version" { gsub(/[",]/, "", $3); version = $3 }
+ in_pkg && $1 == "checksum" { gsub(/[",]/, "", $3); checksum = $3 }
+ in_pkg && $0 == "" {
+ if (name == "paimon-vindex-core" && version == "0.4.0" &&
checksum != "") { print checksum; found = 1; exit }
+ in_pkg = 0
+ }
+ END {
+ if (!found && in_pkg && name == "paimon-vindex-core" &&
version == "0.4.0" && checksum != "") { print checksum }
+ }
+ ' Cargo.lock)"
+ if [[ -z "${vindex_checksum}" ]]; then
+ echo "failed to read the paimon-vindex-core 0.4.0 checksum from
Cargo.lock"
+ exit 1
+ fi
+ local vindex_verified=""
+ local candidate
+ while IFS= read -r candidate; do
+ local candidate_sum
+ if command -v sha256sum >/dev/null 2>&1; then
+ candidate_sum="$(sha256sum "${candidate}" | awk '{print $1}')"
+ else
+ candidate_sum="$(shasum -a 256 "${candidate}" | awk '{print
$1}')"
+ fi
+ if [[ "${candidate_sum}" == "${vindex_checksum}" ]]; then
+ vindex_verified="${candidate}"
+ break
+ fi
+ done < <(find "${CARGO_HOME:-$HOME/.cargo}/registry/cache" \
+ -type f -name 'paimon-vindex-core-0.4.0.crate' 2>/dev/null)
+ if [[ -z "${vindex_verified}" ]]; then
+ echo "no paimon-vindex-core-0.4.0.crate in the cargo registry
cache matches"
+ echo "the Cargo.lock checksum ${vindex_checksum}; refusing to
build from"
+ echo "unverified bytes (the aarch64 patch overrides the crate with
a path"
+ echo "dependency, which cargo build --locked cannot authenticate)"
+ exit 1
+ fi
+ vindex_crate="${vindex_verified}"
+ rm -rf "${vindex_override}"
+ mkdir -p "${vindex_override}"
+ tar xzf "${vindex_crate}" -C "${vindex_override}" --strip-components=1
+ (cd "${vindex_override}" && \
+ patch -p1 -s
<"${TP_PATCH_DIR}/paimon-vindex-core-0.4.0-aarch64-stable.patch")
+ # Inject the [patch.crates-io] override into the workspace manifest.
+ # Idempotent: skip if a previous run already injected it.
+ if ! grep -q "DORIS_PATCHED_VINDEX" Cargo.toml; then
+ cat >>Cargo.toml <<'EOF'
+
+# DORIS_PATCHED_VINDEX: override the registry crate with a build that compiles
+# on stable Rust for aarch64 (avoids the unstable stdarch_neon_f16 intrinsics).
+[patch.crates-io]
+paimon-vindex-core = { path = ".doris-vindex-override" }
+EOF
+ fi
+ # Record the path source in Cargo.lock so --locked stays satisfied.
+ env "${cargo_env[@]}" "${cargo_bin}" update \
Review Comment:
[P2] Make this update retry-safe. It changes `paimon-vindex-core` to a
path-source lock entry without the registry checksum. If any later cargo,
cbindgen, copy, or strip step fails, `set -e` exits before
`cleanup_package_source`; the next invocation reuses this mutated tree and the
earlier checksum extraction fails because that checksum is now gone. Please
patch/build in a disposable copy or preserve and validate the original checksum
independently, and cover a retry after a failure past this line.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/source/PaimonScanNode.java:
##########
@@ -411,10 +412,102 @@ private void setPaimonParams(TFileRangeDesc rangeDesc,
PaimonSplit paimonSplit)
String fileFormat = getFileFormat(paimonSplit.getPathString());
if (split != null) {
+ // use jni reader / paimon-cpp reader / paimon-rust reader
rangeDesc.setFormatType(TFileFormatType.FORMAT_JNI);
- // A logical DataSplit may span multiple files, so keep it intact
for the JNI reader.
- fileDesc.setReaderType(TPaimonReaderType.PAIMON_JNI);
- fileDesc.setPaimonSplit(PaimonUtil.encodeObjectToString(split));
+ // paimon-cpp and paimon-rust both consume Paimon native binary
serialization,
+ // which only supports DataSplit. Any other split type falls back
to JNI.
+ boolean nativeSplit = split instanceof DataSplit;
+ // paimon-rust additionally requires (a) FileScannerV2: the V1
FileScanner
+ // explicitly rejects PAIMON_RUST, so with enable_file_scanner_v2
disabled
+ // the split falls back to JNI instead of encoding a rust request
that the
+ // selected scanner cannot consume, and (b) a FileStoreTable: BE
opens the
+ // table via paimon_table_from_schema_json, which needs the
resolved
+ // TableSchema that only FileStoreTable exposes via schema(). If
the table
+ // is not a FileStoreTable (e.g. a sys table backed by DataSplit),
we cannot
+ // ship a schema JSON, so fall back to CPP / JNI rather than
sending an
+ // incomplete PAIMON_RUST request that BE would reject.
+ //
+ // Serialize the same effective table that planning and the JNI
reader use.
+ // Relation options such as t@options('read.batch-size'='1') are
applied by
+ // getProcessedTable() (doInitialize caches it in processedTable),
and the
+ // rust reader derives its read batch size from the schema options
— the raw
+ // cached table would silently drop the override. Copies,
delegates and
+ // fallback wrappers of getProcessedTable() are still
FileStoreTable, so the
+ // instanceof gate keeps its semantics.
+ Table paimonTable = processedTable;
+ // The paimon-rust S3 bridge maps static credentials, anonymous
+ // access (AWS_CREDENTIALS_PROVIDER_TYPE=ANONYMOUS -> s3.anonymous)
+ // and assume-role (AWS_ROLE_ARN / AWS_EXTERNAL_ID ->
+ // s3.assumed.role.*), but the remaining credential-provider modes
+ // are ambient JVM provider chains (ENV, SYSTEM_PROPERTIES,
+ // WEB_IDENTITY, CONTAINER, INSTANCE_PROFILE) with no paimon-rust
+ // equivalent — rust would silently sign with whatever the ambient
+ // chain resolves to. Gate those modes away from the rust reader
+ // here so the configured provider is honored via the JNI path.
+ boolean providerModeTranslatable = true;
+ String providerType = backendStorageProperties == null
+ ? null :
backendStorageProperties.get("AWS_CREDENTIALS_PROVIDER_TYPE");
+ if (providerType != null) {
+ String mode = providerType.trim().toUpperCase(Locale.ROOT);
+ providerModeTranslatable = mode.equals("DEFAULT")
+ || mode.equals("ANONYMOUS");
+ // The rust OSS FileIO parser (oss:// warehouses) has no
+ // skip-signature switch, so an anonymous OSS catalog cannot be
+ // served by the rust reader either — fall back to JNI.
+ if (mode.equals("ANONYMOUS")) {
+ String location = source.getTableLocation();
+ if (location != null && location.startsWith("oss://")) {
+ providerModeTranslatable = false;
+ }
+ }
+ }
+ // Incremental scans (binlog / changelog / delta / diff) must stay
+ // on the JNI path: this wire format carries only an ordinary
+ // DataSplit and the rust reader invokes TableRead::to_arrow, but
+ // paimon 1.4 marks incremental splits as streaming (which the
+ // pinned rust deserializer rejects), diff requires a separate
+ // IncrementalPlan instead of an ordinary plan, and ordinary
+ // primary-key reads can merge versions rather than return the
+ // changes — until the C ABI transports the mode and plan, the
+ // rust reader cannot express any of these.
+ TableScanParams incrementalParams = getScanParams();
+ boolean isIncremental = incrementalParams != null &&
incrementalParams.incrementalRead();
+ boolean canUseRust = sessionVariable.isEnablePaimonRustReader()
+ && sessionVariable.enableFileScannerV2 && nativeSplit &&
!isIncremental
+ && providerModeTranslatable
+ && paimonTable instanceof FileStoreTable;
Review Comment:
[P1] Gate `query-auth.enabled` tables off the Rust reader. When catalog
authorization succeeds with no row filter or column mask, Paimon leaves an
ordinary `DataSplit`, so it passes this compound gate (restricted results use
`QueryAuthSplit` and already fall back). The shipped schema still carries
`query-auth.enabled=true`, and the pinned Rust `ReadBuilder::new_read`
deliberately rejects every such table, turning a valid authorized scan into a
BE-open failure. Keep this option on JNI until the authorization result can be
transported and enforced.
##########
regression-test/suites/external_table_p0/paimon/test_paimon_rust_reader_v2.groovy:
##########
@@ -0,0 +1,97 @@
+// 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.
+
+// Paimon rust reader under FileScannerV2: enable_paimon_rust_reader=true must
work with
+// the default enable_file_scanner_v2=true (before the v2 port, a rust split
was rejected by
+// FileScannerV2::is_supported and the reader was only reachable with v2
disabled).
+suite("test_paimon_rust_reader_v2", "p0,external") {
+ String enabled = context.config.otherConfigs.get("enablePaimonTest")
+ if (enabled == null || !enabled.equalsIgnoreCase("true")) {
+ logger.info("disabled paimon test")
+ return
+ }
+
+ String catalogName = "test_paimon_rust_reader_v2"
+ String hdfsPort = context.config.otherConfigs.get("hive2HdfsPort")
+ String externalEnvIp = context.config.otherConfigs.get("externalEnvIp")
+
+ try {
+ sql """drop catalog if exists ${catalogName}"""
+ sql """create catalog if not exists ${catalogName} properties (
+ "type" = "paimon",
+ "paimon.catalog.type" = "filesystem",
+ "warehouse" =
"hdfs://${externalEnvIp}:${hdfsPort}/user/doris/paimon1"
+ );"""
+ sql """switch ${catalogName}"""
+ sql """use db1"""
+ sql """set force_jni_scanner=false"""
+ // FileScannerV2 is the default; make the intent of this suite
explicit.
+ sql """set enable_file_scanner_v2=true"""
+
+ def testQueries = [
+ """select c1 from complex_all order by c1""",
+ // Filter on a data column exercises the v2 predicate pushdown
into the
+ // paimon-rust filter (PaimonRustPredicateConverter
global-index mode).
+ """select c1 from complex_all where c1 >= 2 order by c1""",
+ """select * from all_table order by c1""",
+ """select * from all_table_with_parquet where c13 like '13%'
order by c1""",
+ """select * from complex_tab order by c1""",
+ """select c3['a_test'], c3['b_test'], c3['bbb'], c3['ccc']
from complex_tab order by c3['a_test'], c3['b_test']""",
+ """select array_max(c2) c from complex_tab order by c""",
+ """select c20[0] c from complex_all order by c""",
+ """select * from deletion_vector_orc""",
Review Comment:
[P2] Make these differential results deterministic before comparing list
strings. Both deletion-vector scans here omit `ORDER BY`, so JNI and Rust may
return the same three rows in different valid file/batch order and fail the
suite. The new EQ_FOR_NULL suite has the same issue for `(1,1)` and `(1,2)`: it
orders only by their tied `a` value before asserting a fixed list. Add a total
order (including a tie-breaker) or compare canonicalized multisets.
##########
thirdparty/build-thirdparty.sh:
##########
@@ -2169,6 +2169,222 @@ build_lance_c() {
fi
}
+# paimon-rust
+build_paimon_rust() {
+ check_if_source_exist "${PAIMON_RUST_SOURCE}"
+ cd "${TP_SOURCE_DIR}/${PAIMON_RUST_SOURCE}"
+
+ rm -rf "${BUILD_DIR}"
+ mkdir -p "${BUILD_DIR}"
+
+ local cargo_bin="${PAIMON_RUST_CARGO:-${CARGO:-cargo}}"
+ if ! command -v "${cargo_bin}" >/dev/null 2>&1; then
+ echo "cargo is required to build paimon-rust. Install Rust 1.91.0 or
set PAIMON_RUST_CARGO."
+ exit 1
+ fi
+
+ local required_rust_version="1.91.0"
+ local cargo_env=(
+ "CARGO_BUILD_JOBS=${PARALLEL}"
+ "CARGO_TARGET_DIR=${PWD}/${BUILD_DIR}"
+ )
+ if command -v rustup >/dev/null 2>&1 && [[ -z "${RUSTUP_TOOLCHAIN}" ]];
then
+ if ! rustup toolchain list | grep -Eq '^1\.91\.0([[:space:]-]|$)'; then
+ rustup toolchain install "${required_rust_version}" --profile
minimal
+ fi
+ cargo_env+=("RUSTUP_TOOLCHAIN=${required_rust_version}")
+ fi
+
+ local cargo_version
+ if ! cargo_version="$(env "${cargo_env[@]}" "${cargo_bin}" --version | awk
'{print $2}')"; then
+ echo "failed to get cargo version for paimon-rust. Install Rust
${required_rust_version} or set PAIMON_RUST_CARGO/RUSTUP_TOOLCHAIN."
+ exit 1
+ fi
+ # Rust 1.91.0 is the minimum supported version. Allow newer toolchains when
Review Comment:
[P2] Enforce the same-rustc invariant documented here. `LANCE_C_CARGO` and
`PAIMON_RUST_CARGO` are selected independently, and both checks accept any
version at least 1.91, so supported overrides can build the two linked static
libraries with different std hashes and defer the documented
`rust_eh_personality` collision to final BE link. Resolve one shared toolchain
or compare the exact `rustc -vV` identity and fail early.
--
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]
