[CALCITE-7580] Remove Gandiva dependency from Arrow adapter#4992
[CALCITE-7580] Remove Gandiva dependency from Arrow adapter#4992caicancai wants to merge 3 commits into
Conversation
caicancai
force-pushed
the
calcite-7580-remove-gandiva
branch
2 times, most recently
from
8aefea7 to
3cc76fe
Compare
There was a problem hiding this comment.
Pull request overview
Removes the Arrow adapter’s dependency on the native Gandiva/LLVM runtime by switching projection to direct ValueVector reads and implementing evaluation of the existing Arrow filter token subset in pure Java, enabling Arrow adapter tests to run without platform-specific native libraries.
Changes:
- Drop
arrow-gandivafrom version constraints, the BOM, and the Arrow module dependencies. - Remove Gandiva-based projector/filter enumerators and route filtering through a new Java-based filter enumerator.
- Stop conditionally skipping Arrow tests based on Gandiva native library availability; keep filter token representation and translation aligned with the Java evaluator.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| gradle.properties | Removes the arrow-gandiva version property from global version constraints. |
| bom/build.gradle.kts | Removes org.apache.arrow.gandiva:arrow-gandiva from the published BOM. |
| arrow/build.gradle.kts | Drops the Arrow module’s direct dependency on arrow-gandiva. |
| arrow/src/test/java/org/apache/calcite/adapter/arrow/ArrowExtension.java | Removes Gandiva JNI load checks so Arrow tests are not skipped based on native availability. |
| arrow/src/main/java/org/apache/calcite/adapter/arrow/ConditionToken.java | Renames away from Gandiva terminology and introduces a typed Operator enum for filter tokens. |
| arrow/src/main/java/org/apache/calcite/adapter/arrow/ArrowTranslator.java | Updates translation to emit typed operators and maintains the current supported operator subset. |
| arrow/src/main/java/org/apache/calcite/adapter/arrow/ArrowTable.java | Removes Gandiva projector/filter construction and passes conditions through to Arrow enumerable for Java evaluation. |
| arrow/src/main/java/org/apache/calcite/adapter/arrow/ArrowRules.java | Updates commentary to reflect Arrow adapter limitations rather than Gandiva-specific ones. |
| arrow/src/main/java/org/apache/calcite/adapter/arrow/ArrowProjectEnumerator.java | Removes the Gandiva-based projection enumerator. |
| arrow/src/main/java/org/apache/calcite/adapter/arrow/ArrowJavaFilterEnumerator.java | Adds Java evaluation of Arrow filter tokens using ValueVector reads. |
| arrow/src/main/java/org/apache/calcite/adapter/arrow/ArrowFilterEnumerator.java | Removes the Gandiva-based filter enumerator. |
| arrow/src/main/java/org/apache/calcite/adapter/arrow/ArrowEnumerable.java | Switches enumeration to use the Java filter enumerator when conditions are present. |
| arrow/src/main/java/org/apache/calcite/adapter/arrow/AbstractArrowEnumerator.java | Makes getValue reusable by the new Java filter enumerator. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
caicancai
force-pushed
the
calcite-7580-remove-gandiva
branch
3 times, most recently
from
7cdb4eb to
b3c5c16
Compare
caicancai
force-pushed
the
calcite-7580-remove-gandiva
branch
from
b3c5c16 to
43ed745
Compare
|
xiedeyantu
left a comment
xiedeyantu
left a comment
There was a problem hiding this comment.
I think we should add a note about this breaking change to history.md. I'm not very familiar with this area, but I've gone through the PR and the tests overall and it looks good to me. It would be great if someone with more expertise could help review it though.
| .explainContains(plan); | ||
| } | ||
|
|
||
| @Test void testArrowProjectSkipsEmptyBatch() { |
There was a problem hiding this comment.
We'd better add the JIRA title here and other test cases.
| /** | ||
| * Enumerator that evaluates Arrow filter tokens in Java. | ||
| */ | ||
| class ArrowJavaFilterEnumerator extends AbstractArrowEnumerator { |
There was a problem hiding this comment.
Should we still call it ArrowFilterEnumerator? My understanding is that this PR is a breaking change upgrade. The same as ArrowProjectEnumerator?
https://issues.apache.org/jira/browse/CALCITE-7580
This removes the Arrow adapter's dependency on Gandiva.
The adapter now uses Arrow
ValueVectorreads for projection, and evaluates the currentArrowTranslatorfilter token subset in Java. This keeps the Arrow adapter correctness path in Java and avoids requiring the native Gandiva/LLVM runtime for Arrow adapter tests.Changes:
arrow-gandivafrom the Arrow module and BOM/version constraints;