dongjoon-hyun commented on PR #59965: URL: https://github.com/apache/spark/pull/59965#issuecomment-6562044908
Thank you for the second review, @peter-toth ! I addressed findings 5 to 10 and 13 in 958490e358a, and kept 11 and 12 out of this PR. 5. **The check reading the V2 form:** The check now reads the text first, so it agrees with the auto-fill. 6. **The fallback keeping the ANSI parts:** A column that the fixed settings do not resolve is now read entirely under the session, so both of your cases match the base. 7. **`spark.sql.timestampType`:** This setting now stays with the session. 8. **`spark.sql.decimalOperations.allowPrecisionLoss`:** This setting also stays with the session. I did not apply the suggestion as is, because it lets a decimal value change silently. 9. **The session parser:** The check now uses `CatalystSqlParser`, as the auto-fill does. 10. **Migration guide:** The entry now lists what still follows the session, and covers a table created under a non-default parser setting. I did not add a legacy config. 11. **`SET PATH`:** The behavior is unchanged from the base. The rewrite will come in a separate PR. 12. **Stored configs:** I kept this out of this PR for the reasons you list. 13. **Scaladoc:** Applied your suggestion. While rechecking these, I also found that a text the fixed settings cannot parse failed every write, such as `s."f"` in a table created with `spark.sql.ansi.doubleQuotedIdentifiers=true`. In a2ac9a984dd, such a text is parsed under the session. I replied in each thread with the details and updated the PR description. -- 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]
