advancedxy commented on code in PR #632:
URL: https://github.com/apache/datafusion-comet/pull/632#discussion_r1670631896
##########
core/src/execution/datafusion/shuffle_writer.rs:
##########
@@ -80,6 +79,8 @@ pub struct ShuffleWriterExec {
/// Metrics
metrics: ExecutionPlanMetricsSet,
cache: PlanProperties,
+ /// zstd compression level
+ compression_level: i32,
Review Comment:
If the compression_level is stored in SessionConf, we may retrieve that when
executing instead of defining a new field?
##########
core/src/execution/datafusion/planner.rs:
##########
@@ -128,27 +128,32 @@ pub struct PhysicalPlanner {
exec_context_id: i64,
execution_props: ExecutionProps,
session_ctx: Arc<SessionContext>,
+ compression_level: i32,
Review Comment:
It seems like that we are passing around the `compression_level` parameters.
It's somehow a bit ugly if we are going to add more configurable entries later.
Could we leverage the `SessionConf` in `session_ctx.state` to avoid that?
##########
core/src/execution/jni_api.rs:
##########
@@ -455,6 +466,7 @@ pub unsafe extern "system" fn
Java_org_apache_comet_Native_writeSortedFileNative
checksum_enabled: jboolean,
checksum_algo: jint,
current_checksum: jlong,
+ compression_level: jlong,
Review Comment:
I believe you need to update jni code in the JVM side too:
org.apache.comet.Native#writeSortedFileNative
--
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]
