danepitkin commented on code in PR #35570:
URL: https://github.com/apache/arrow/pull/35570#discussion_r1312200301
##########
java/dataset/src/main/cpp/jni_wrapper.cc:
##########
@@ -458,8 +467,8 @@ JNIEXPORT void JNICALL
Java_org_apache_arrow_dataset_jni_JniWrapper_closeDataset
* Signature: (J[Ljava/lang/String;JJ)J
*/
JNIEXPORT jlong JNICALL
Java_org_apache_arrow_dataset_jni_JniWrapper_createScanner(
- JNIEnv* env, jobject, jlong dataset_id, jobjectArray columns, jlong
batch_size,
- jlong memory_pool_id) {
+ JNIEnv* env, jobject, jlong dataset_id, jobjectArray columns,
+ jobject substrait_extended_expression, jlong batch_size, jlong
memory_pool_id) {
Review Comment:
I think that should be fine. At this point we have already crossed the JNI
boundary so I don't anticipate much performance impact in making the call
twice. Ideally, we should keep the implementation flexible enough such that
somebody can easily add support for Acero compute expressions as well. e.g.
from the user perspective a `filter` can be either a compute expression or a
substrait binary blob that gets parsed into a compute expression (wrapped in
named/bounded expression objects). We don't need to add that additional Acero
functionality in this PR, though.
--
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]
