[SPARK-28152][SQL][FOLLOWUP] Add a legacy conf for old MsSqlServerDialect numeric mapping #27184
Conversation
…erDialect numeric mapping
dongjoon-hyun
changed the title
|
cc @shivsood , @HyukjinKwon , @maropu , @srowen , @gatorsmile , @zsxwing |
|
Thank you for review and approval, @HyukjinKwon ! |
|
Test build #116601 has finished for PR 27184 at commit
|
|
All Scala/Java test passed and the PySpark failure is due to a known |
…lect numeric mapping This is a follow-up for #25248 . The new behavior cannot access the existing table which is created by old behavior. This PR provides a way to avoid new behavior for the existing users. Yes. This will fix the broken behavior on the existing tables. Pass the Jenkins and manually run JDBC integration test. ``` build/mvn install -DskipTests build/mvn -Pdocker-integration-tests -pl :spark-docker-integration-tests_2.12 test ``` Closes #27184 from dongjoon-hyun/SPARK-28152-CONF. Authored-by: Dongjoon Hyun <dhyun@apple.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com> (cherry picked from commit 28fc043) Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
|
Late LGTM. |
| @@ -2161,6 +2161,13 @@ object SQLConf { | |||
| .booleanConf | |||
| .createWithDefault(true) | |||
|
|
|||
| val LEGACY_MSSQLSERVER_NUMERIC_MAPPING_ENABLED = | |||
| buildConf("spark.sql.legacy.mssqlserver.numericMapping.enabled") | |||
There was a problem hiding this comment.
For such a change, could we have an item in the migration guide?
There was a problem hiding this comment.
Sure. Can we handle that after releasing 2.4.5 since It's a documentation change?
| val LEGACY_MSSQLSERVER_NUMERIC_MAPPING_ENABLED = | ||
| buildConf("spark.sql.legacy.mssqlserver.numericMapping.enabled") | ||
| .internal() | ||
| .doc("When true, use legacy MySqlServer SMALLINT and REAL type mapping.") |
There was a problem hiding this comment.
It is not very clear what is the legacy MySqlServer SMALLINT and REAL type mapping if they do not read the PR. Could we make it clear? The behavior when the value is set to true and the behavior when the value is false
There was a problem hiding this comment.
Of course, I'd like to mention it at the migration guide fully.
|
BTW, @gatorsmile . If you have any concern on 2.4.5 RC1, could you vote on the mailing list like @cloud-fan ? I want to collect all feedback. |
What changes were proposed in this pull request?
This is a follow-up for #25248 .
Why are the changes needed?
The new behavior cannot access the existing table which is created by old behavior.
This PR provides a way to avoid new behavior for the existing users.
Does this PR introduce any user-facing change?
Yes. This will fix the broken behavior on the existing tables.
How was this patch tested?
Pass the Jenkins and manually run JDBC integration test.