You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "guixiaowen (via GitHub)" <gi...@apache.org> on 2025/08/18 17:38:50 UTC
[PR] Use SqlApiConf.CASE_SENSITIVE_KEY() instead of hard-coded spark.sql.caseSensitive [iceberg]
guixiaowen opened a new pull request, #13858:
URL: https://github.com/apache/iceberg/pull/13858
Use SqlApiConf.CASE_SENSITIVE_KEY() instead of hard-coded spark.sql.caseSensitive #13857
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Use SqlApiConf.CASE_SENSITIVE_KEY() instead of hard-coded spark.sql.caseSensitive [iceberg]
Posted by "guixiaowen (via GitHub)" <gi...@apache.org>.
guixiaowen commented on code in PR #13858:
URL: https://github.com/apache/iceberg/pull/13858#discussion_r2286691717
##########
spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/SparkUtil.java:
##########
@@ -254,7 +255,7 @@ public static String toColumnName(NamedReference ref) {
}
public static boolean caseSensitive(SparkSession spark) {
- return Boolean.parseBoolean(spark.conf().get("spark.sql.caseSensitive"));
+ return Boolean.parseBoolean(spark.conf().get(SqlApiConf.CASE_SENSITIVE_KEY()));
Review Comment:
> also it appears that there are many more places that use the hardcoded string, so we should probably update those places as well
@nastra I also found this in many UTs. Does it need to be modified as well?
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Fixes #13857 Use SqlApiConf.CASE_SENSITIVE_KEY() instead of hard-coded spark.sql.caseSensitive [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #13858:
URL: https://github.com/apache/iceberg/pull/13858#discussion_r2290379265
##########
spark/v3.4/spark/src/main/java/org/apache/iceberg/spark/SparkUtil.java:
##########
@@ -290,7 +291,7 @@ public static String toColumnName(NamedReference ref) {
}
public static boolean caseSensitive(SparkSession spark) {
- return Boolean.parseBoolean(spark.conf().get("spark.sql.caseSensitive"));
+ return Boolean.parseBoolean(spark.conf().get(SQLConf.CASE_SENSITIVE().key()));
Review Comment:
@guixiaowen I'm counting 30 places across the entire Iceberg codebase that use the hardcoded string, so we should update all of those places if we're going to update it here
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Use SqlApiConf.CASE_SENSITIVE_KEY() instead of hard-coded spark.sql.caseSensitive [iceberg]
Posted by "ebyhr (via GitHub)" <gi...@apache.org>.
ebyhr commented on PR #13858:
URL: https://github.com/apache/iceberg/pull/13858#issuecomment-3204202818
You can update the PR description like `Fixes #13857` so we can close the issue automatically when we merge this PR.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Fixes #13857 Use SqlApiConf.CASE_SENSITIVE_KEY() instead of hard-coded spark.sql.caseSensitive [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13858:
URL: https://github.com/apache/iceberg/pull/13858#issuecomment-3316377285
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Use SqlApiConf.CASE_SENSITIVE_KEY() instead of hard-coded spark.sql.caseSensitive [iceberg]
Posted by "guixiaowen (via GitHub)" <gi...@apache.org>.
guixiaowen commented on code in PR #13858:
URL: https://github.com/apache/iceberg/pull/13858#discussion_r2285315064
##########
spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/SparkUtil.java:
##########
@@ -254,7 +255,7 @@ public static String toColumnName(NamedReference ref) {
}
public static boolean caseSensitive(SparkSession spark) {
- return Boolean.parseBoolean(spark.conf().get("spark.sql.caseSensitive"));
+ return Boolean.parseBoolean(spark.conf().get(SqlApiConf.CASE_SENSITIVE_KEY()));
Review Comment:
> why not consistently use `SQLConf.CASE_SENSITIVE().key()` across all Spark versions?
@nastra Thank you for your review. I have made the revisions as per your request.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Use SqlApiConf.CASE_SENSITIVE_KEY() instead of hard-coded spark.sql.caseSensitive [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #13858:
URL: https://github.com/apache/iceberg/pull/13858#discussion_r2284436468
##########
spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/SparkUtil.java:
##########
@@ -254,7 +255,7 @@ public static String toColumnName(NamedReference ref) {
}
public static boolean caseSensitive(SparkSession spark) {
- return Boolean.parseBoolean(spark.conf().get("spark.sql.caseSensitive"));
+ return Boolean.parseBoolean(spark.conf().get(SqlApiConf.CASE_SENSITIVE_KEY()));
Review Comment:
also it appears that there are many more places that use the hardcoded string, so we should probably update those places as well
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Fixes #13857 Use SqlApiConf.CASE_SENSITIVE_KEY() instead of hard-coded spark.sql.caseSensitive [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13858:
URL: https://github.com/apache/iceberg/pull/13858#issuecomment-3349471054
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Fixes #13857 Use SqlApiConf.CASE_SENSITIVE_KEY() instead of hard-coded spark.sql.caseSensitive [iceberg]
Posted by "guixiaowen (via GitHub)" <gi...@apache.org>.
guixiaowen commented on PR #13858:
URL: https://github.com/apache/iceberg/pull/13858#issuecomment-3206966007
> You can update the PR description like `Fixes #13857` so we can close the issue automatically when we merge this PR.
@ebyhr okok
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Use SqlApiConf.CASE_SENSITIVE_KEY() instead of hard-coded spark.sql.caseSensitive [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #13858:
URL: https://github.com/apache/iceberg/pull/13858#discussion_r2284435249
##########
spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/SparkUtil.java:
##########
@@ -254,7 +255,7 @@ public static String toColumnName(NamedReference ref) {
}
public static boolean caseSensitive(SparkSession spark) {
- return Boolean.parseBoolean(spark.conf().get("spark.sql.caseSensitive"));
+ return Boolean.parseBoolean(spark.conf().get(SqlApiConf.CASE_SENSITIVE_KEY()));
Review Comment:
why not consistently use `SQLConf.CASE_SENSITIVE().key()` across all Spark versions?
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Use SqlApiConf.CASE_SENSITIVE_KEY() instead of hard-coded spark.sql.caseSensitive [iceberg]
Posted by "guixiaowen (via GitHub)" <gi...@apache.org>.
guixiaowen commented on code in PR #13858:
URL: https://github.com/apache/iceberg/pull/13858#discussion_r2285399380
##########
spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/SparkUtil.java:
##########
@@ -254,7 +255,7 @@ public static String toColumnName(NamedReference ref) {
}
public static boolean caseSensitive(SparkSession spark) {
- return Boolean.parseBoolean(spark.conf().get("spark.sql.caseSensitive"));
+ return Boolean.parseBoolean(spark.conf().get(SqlApiConf.CASE_SENSITIVE_KEY()));
Review Comment:
> also it appears that there are many more places that use the hardcoded string, so we should probably update those places as well
@nastra I found another one and have submitted it.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Use SqlApiConf.CASE_SENSITIVE_KEY() instead of hard-coded spark.sql.caseSensitive [iceberg]
Posted by "guixiaowen (via GitHub)" <gi...@apache.org>.
guixiaowen commented on PR #13858:
URL: https://github.com/apache/iceberg/pull/13858#issuecomment-3199021702
@nastra Could you review this PR when you have time?
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Fixes #13857 Use SqlApiConf.CASE_SENSITIVE_KEY() instead of hard-coded spark.sql.caseSensitive [iceberg]
Posted by "guixiaowen (via GitHub)" <gi...@apache.org>.
guixiaowen commented on PR #13858:
URL: https://github.com/apache/iceberg/pull/13858#issuecomment-3206964388
> You can update the PR description like `Fixes #13857` so we can close the issue automatically when we merge this PR.
okok
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Fixes #13857 Use SqlApiConf.CASE_SENSITIVE_KEY() instead of hard-coded spark.sql.caseSensitive [iceberg]
Posted by "guixiaowen (via GitHub)" <gi...@apache.org>.
guixiaowen commented on code in PR #13858:
URL: https://github.com/apache/iceberg/pull/13858#discussion_r2293367687
##########
spark/v3.4/spark/src/main/java/org/apache/iceberg/spark/SparkUtil.java:
##########
@@ -290,7 +291,7 @@ public static String toColumnName(NamedReference ref) {
}
public static boolean caseSensitive(SparkSession spark) {
- return Boolean.parseBoolean(spark.conf().get("spark.sql.caseSensitive"));
+ return Boolean.parseBoolean(spark.conf().get(SQLConf.CASE_SENSITIVE().key()));
Review Comment:
> @guixiaowen I'm counting 30 places across the entire Iceberg codebase that use the hardcoded string, so we should update all of those places if we're going to update it here
@nastra I'm a little confused. Are we supposed to change all the Spark-related configurations? Even in the unit tests.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Fixes #13857 Use SqlApiConf.CASE_SENSITIVE_KEY() instead of hard-coded spark.sql.caseSensitive [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #13858:
URL: https://github.com/apache/iceberg/pull/13858#discussion_r2290375489
##########
spark/v3.4/spark/src/main/java/org/apache/iceberg/spark/Spark3Util.java:
##########
@@ -538,7 +539,7 @@ public static String describe(org.apache.iceberg.SortOrder order) {
}
public static boolean extensionsEnabled(SparkSession spark) {
- String extensions = spark.conf().get("spark.sql.extensions", "");
+ String extensions = spark.conf().get(StaticSQLConf.SPARK_SESSION_EXTENSIONS().key(), "");
Review Comment:
let's leave this change out, because there are more places across the codebase that use the hardcoded string which haven't been updated
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Fixes #13857 Use SqlApiConf.CASE_SENSITIVE_KEY() instead of hard-coded spark.sql.caseSensitive [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #13858: Fixes #13857 Use SqlApiConf.CASE_SENSITIVE_KEY() instead of hard-coded spark.sql.caseSensitive
URL: https://github.com/apache/iceberg/pull/13858
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org