advancedxy commented on PR #7733: URL: https://github.com/apache/iceberg/pull/7733#issuecomment-1567808941
> > If #7732 is merged, is this new setting `spark.sql.iceberg.locality.enabled` still needed? I think you can disable locality by setting `spark.datasource.iceberg.locality=false`? > > This is needed. `SessionConfigSupport` is a mixed-in trait of `TableProvider`, it only takes effect for iceberg tables loaded through table provider, but not for those tables loaded through catalog. Ah, yeah. The options are not effective for tables loaded through catalog. But it may bring some confusions to users, such as: ``` set spark.datasource.iceberg.locality=false; -- works only for DataFrame set spark.sql.iceberg.locality.enabled=false; -- works both DataFrame and Spark SQL ``` I was thinking maybe we could inject a new `ResolutionRule` in IcebergSparkSessionExtensions that collects session configurations(including `spark.datasource.iceberg.xxx` and `spark.sql.iceberg.xxx` values) and set options to Iceberg tables. In that way, configurations are unified for DataFrame and Catalog tables. WDYT? -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
