search

apache / datafusion-comet

Apache DataFusion Comet Spark Accelerator
https://datafusion.apache.org/comet
Apache License 2.0
447 stars 100 forks source link

chore: add a utils method to getColumnReader with SQLConf #360

Open huaxingao opened 2 weeks ago

huaxingao commented 2 weeks ago

Which issue does this PR close?

Closes #.

Rationale for this change

Add a Utils method getColumnReader which can passed in SQLConf, so Iceberg can pass in CometConf for ColumnReader

What changes are included in this PR?

How are these changes tested?

huaxingao commented 2 weeks ago

This will make it harder to make the column reader Spark agnostic as you're adding one more point of dependency.

We need a way for users to config useLazyMaterialization etc, and the iceberg folks don't want to parse any of the CometConf in iceberg code. That's why I need to pass SQLConf instead of useLazyMaterialization etc. Also I think it's more flexible to pass SQLConf. If we need to add more ColumnReader related CometConf in the future, we don't need to change this API any more.

viirya commented 2 weeks ago

Hmm, I think @parthchandra meant that by adding SQLConf, common will depend on Spark. Does it conflict with the direction to make common not Spark dependent?