MaxGekk commented on code in PR #45057:
URL: https://github.com/apache/spark/pull/45057#discussion_r1481646652


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -2339,7 +2339,8 @@ object SQLConf {
       .doc("When false, the `strfmt` in `format_string(strfmt, obj, ...)` and 
" +
         "`printf(strfmt, obj, ...)` will no longer support to use \"0$\" to 
specify the first " +
         "argument, the first argument should always reference by \"1$\" when 
use argument index " +
-        "to indicating the position of the argument in the argument list.")
+        "to indicating the position of the argument in the argument list. " +
+        "This config will be removed in the future releases.")

Review Comment:
   I think we should not do that because put the config to `removedSQLConfigs` 
means we shall remove it from `SQLConf` which leads to breaking existing user's 
code in two cases:
   - User's code set the config to the default value `false` explicitly. In 
that case, user's app won't compile.
   - User's code set it to `true` but don't refer to the zero index at all for 
some reasons - so user's might not face to 
`java.util.IllegalFormatArgumentIndexException`. But the removing the config, 
we break the app.
   
   I believe we should deprecate the config first of all, and only after that 
we can remove it safely.



-- 
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]

Reply via email to