westonpace commented on issue #28866:
URL: https://github.com/apache/arrow/issues/28866#issuecomment-2107721576
> No, now it just uses the binary serialized substrait as jni input.
If it is using serialized substrait as input then I don't understand how the
C++ consumer would be able to apply the CSV options onto the plan.
Would this be a second pass that transforms the plan once it is
deserialized? E.g. something like:
```
def run_plan(substrait_plan, extra_options):
plan = deserialize_plan(substrait_plan)
plan = transform_plan(plan, extra_options)
run_deserialized_plan(plan)
```
> No, now it just uses the binary serialized substrait as jni input
I still think approach 1 is preferred. The extra options can be supplied as
part of `ReadRel::advanced_extension`.
> And ConvertOptions
has complex config value like std::unordered_map<std::string,
std::shared_ptr<DataType>> column_types; it should also be serialized, but we
can support the basic option by HashMap, I'm not sure if their is requirement
for the complex options in the future.
I think `HashMap` should be sufficient. Passing `shared_ptr<DataType>`
across the JNI boundary is going to be error-prone. If we need advanced
configuration like that then it should be done with a more complicated
Substrait mesasge (e.g. use `CsvOptions` substrait message instead of a simple
map. There are Substrait messages for types already).
--
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]
