This is an automated email from the ASF dual-hosted git repository.
JingsongLi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/paimon.git
The following commit(s) were added to refs/heads/master by this push:
new 1bebd7c6e7 [vortex] Free the intermediate native expressions built for
a scan (#9841)
1bebd7c6e7 is described below
commit 1bebd7c6e7dd6501fdceca060aa425f4112518ef
Author: jackylee <[email protected]>
AuthorDate: Tue Sep 22 11:40:18 2026 +0800
[vortex] Free the intermediate native expressions built for a scan (#9841)
---
.../paimon/format/vortex/VortexReaderFactory.java | 20 +++++---
.../paimon/format/vortex/VortexRecordsReader.java | 25 +++++----
.../src/main/java/dev/vortex/api/Expression.java | 10 +++-
.../dev/vortex/api/ExpressionLifetimeTest.java | 59 ++++++++++++++++++++++
4 files changed, 95 insertions(+), 19 deletions(-)
diff --git
a/paimon-vortex/paimon-vortex-format/src/main/java/org/apache/paimon/format/vortex/VortexReaderFactory.java
b/paimon-vortex/paimon-vortex-format/src/main/java/org/apache/paimon/format/vortex/VortexReaderFactory.java
index 6974ac489d..6ffc3bef81 100644
---
a/paimon-vortex/paimon-vortex-format/src/main/java/org/apache/paimon/format/vortex/VortexReaderFactory.java
+++
b/paimon-vortex/paimon-vortex-format/src/main/java/org/apache/paimon/format/vortex/VortexReaderFactory.java
@@ -56,16 +56,20 @@ public class VortexReaderFactory implements
FormatReaderFactory {
@Override
public FileRecordReader<InternalRow> createReader(Context context) {
long[] rowIndices = toRowIndices(context.selection());
- Expression predicate =
VortexPredicateConverter.toVortexExpression(predicates);
Pair<Path, Map<String, String>> vortexSpecified =
toVortexSpecifiedForReader(context.fileIO(),
context.filePath());
- return new VortexRecordsReader(
- vortexSpecified.getLeft(),
- dataSchemaRowType,
- projectedRowType,
- rowIndices,
- predicate,
- vortexSpecified.getRight());
+ // The scan clones the filter it is given, the same way it clones the
projection, so this
+ // tree is ours to release once the reader's constructor has built the
scan. One tree is
+ // built per data file, and nothing downstream reaches it again.
+ try (Expression predicate =
VortexPredicateConverter.toVortexExpression(predicates)) {
+ return new VortexRecordsReader(
+ vortexSpecified.getLeft(),
+ dataSchemaRowType,
+ projectedRowType,
+ rowIndices,
+ predicate,
+ vortexSpecified.getRight());
+ }
}
@Nullable
diff --git
a/paimon-vortex/paimon-vortex-format/src/main/java/org/apache/paimon/format/vortex/VortexRecordsReader.java
b/paimon-vortex/paimon-vortex-format/src/main/java/org/apache/paimon/format/vortex/VortexRecordsReader.java
index c41ac27e42..8a194f5e18 100644
---
a/paimon-vortex/paimon-vortex-format/src/main/java/org/apache/paimon/format/vortex/VortexRecordsReader.java
+++
b/paimon-vortex/paimon-vortex-format/src/main/java/org/apache/paimon/format/vortex/VortexRecordsReader.java
@@ -91,17 +91,22 @@ public class VortexRecordsReader implements
FileRecordReader<InternalRow> {
ImmutableScanOptions.builder().ordered(true);
java.util.List<String> columns =
physicalReadRowType.getFieldNames();
- scanBuilder.projection(
- Expression.select(columns.toArray(new String[0]),
Expression.root()));
-
- if (rowIndices != null) {
- scanBuilder.selectionIndices(rowIndices);
-
scanBuilder.selectionMode(ScanOptions.SelectionMode.INCLUDE);
- }
- if (predicate != null) {
- scanBuilder.filter(predicate);
+ // The scan clones the projection it is given, so both the
root and the select
+ // built here are ours to release once dataSource.scan has
returned.
+ try (Expression root = Expression.root();
+ Expression projection =
+ Expression.select(columns.toArray(new
String[0]), root)) {
+ scanBuilder.projection(projection);
+
+ if (rowIndices != null) {
+ scanBuilder.selectionIndices(rowIndices);
+
scanBuilder.selectionMode(ScanOptions.SelectionMode.INCLUDE);
+ }
+ if (predicate != null) {
+ scanBuilder.filter(predicate);
+ }
+ this.scan = dataSource.scan(scanBuilder.build());
}
- this.scan = dataSource.scan(scanBuilder.build());
} catch (Exception e) {
dataSource.close();
throw e;
diff --git
a/paimon-vortex/paimon-vortex-jni/src/main/java/dev/vortex/api/Expression.java
b/paimon-vortex/paimon-vortex-jni/src/main/java/dev/vortex/api/Expression.java
index ccff4e4aa2..ceb2a2a41f 100644
---
a/paimon-vortex/paimon-vortex-jni/src/main/java/dev/vortex/api/Expression.java
+++
b/paimon-vortex/paimon-vortex-jni/src/main/java/dev/vortex/api/Expression.java
@@ -51,7 +51,15 @@ public final class Expression implements AutoCloseable {
public static Expression column(String name) {
long rootPtr = NativeExpression.root();
- return new Expression(NativeExpression.getItem(name, rootPtr));
+ try {
+ return new Expression(NativeExpression.getItem(name, rootPtr));
+ } finally {
+ // Vortex builders do not take ownership of their inputs -- see
the module doc of
+ // vortex-jni/src/expression.rs at 0.73.0 -- so releasing this
intermediate is our
+ // responsibility, and getItem has already cloned it into the
expression we return.
+ // It is never wrapped in an Expression, so without this no caller
can reach it.
+ NativeExpression.free(rootPtr);
+ }
}
public static Expression select(String[] columns, Expression parent) {
diff --git
a/paimon-vortex/paimon-vortex-jni/src/test/java/dev/vortex/api/ExpressionLifetimeTest.java
b/paimon-vortex/paimon-vortex-jni/src/test/java/dev/vortex/api/ExpressionLifetimeTest.java
new file mode 100644
index 0000000000..af63ceffda
--- /dev/null
+++
b/paimon-vortex/paimon-vortex-jni/src/test/java/dev/vortex/api/ExpressionLifetimeTest.java
@@ -0,0 +1,59 @@
+/*
+ * 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.
+ */
+
+package dev.vortex.api;
+
+import dev.vortex.jni.NativeLoader;
+
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * {@link Expression#column} releases the intermediate root it builds, so that
pointer is freed
+ * outside any {@link Expression} wrapper. Closing the returned expression
must therefore free its
+ * own pointer once and never the root's again: a second free of either would
abort the JVM inside
+ * this test rather than fail it.
+ *
+ * <p>That the child stays valid after the root is freed is covered end to end
by the round-trip
+ * tests in {@code VortexPredicateConverterTest}, which read rows through a
scan built from
+ * expressions this reader has already released.
+ */
+class ExpressionLifetimeTest {
+
+ @BeforeAll
+ static void loadNativeLibrary() {
+ // Report a missing library as a test failure rather than an
ExceptionInInitializerError
+ // from the static initializer of NativeExpression.
+ NativeLoader.loadJni();
+ }
+
+ @Test
+ void closingAColumnFreesItsPointerExactlyOnce() {
+ Expression column = Expression.column("col_a");
+ assertThat(column.nativePointer()).isNotZero();
+
+ column.close();
+ assertThat(column.nativePointer()).isZero();
+
+ // Idempotent: the second close must not reach the native side.
+ column.close();
+ assertThat(column.nativePointer()).isZero();
+ }
+}
