aokolnychyi commented on a change in pull request #2248:
URL: https://github.com/apache/iceberg/pull/2248#discussion_r580752432
##########
File path: spark3/src/main/java/org/apache/iceberg/spark/Spark3Util.java
##########
@@ -474,15 +476,33 @@ public static boolean isLocalityEnabled(FileIO io, String
location, CaseInsensit
return false;
}
- public static boolean isVectorizationEnabled(Map<String, String> properties,
CaseInsensitiveStringMap readOptions) {
- String batchReadsSessionConf = SparkSession.active().conf()
- .get("spark.sql.iceberg.vectorization.enabled", null);
- if (batchReadsSessionConf != null) {
- return Boolean.valueOf(batchReadsSessionConf);
+ public static boolean isVectorizationEnabled(FileFormat fileFormat,
+ Map<String, String> properties,
+ RuntimeConfig sessionConf,
+ CaseInsensitiveStringMap
readOptions) {
+
+ String readOptionValue =
readOptions.get(SparkReadOptions.VECTORIZATION_ENABLED);
+ if (readOptionValue != null) {
+ return Boolean.parseBoolean(readOptionValue);
Review comment:
I see it a bit differently, actually. I think we break the config
hierarchy right now. If someone made an effort and set a read option (which is
the most low-level config), don't we have to respect that? I'd interpret such
configs like no matter what my SQL conf is, I know I want to use this value in
this particular job.
Like in my
[example](https://github.com/apache/iceberg/pull/2248#discussion_r580612852)
above, the current order of precedence prohibits using SQL conf for common
defaults. For example, Iceberg is used by multiple orgs and each org frequently
has its own SQL conf template. Let's say somebody accesses multiple tables in
one Spark app and wants to use the default config for all tables except one.
Instead of adding a specific read/write option in a single job, he/she would
have to modify every single job in the Spark app.
I think users that want to turn on and off certain feature using SQL, should
not set read/write options. Instead, rely solely on the SQL conf.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
