-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-21936][SQL][2.2] backward compatibility test framework for HiveExternalCatalog #19163
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…rnalCatalog `HiveExternalCatalog` is a semi-public interface. When creating tables, `HiveExternalCatalog` converts the table metadata to hive table format and save into hive metastore. It's very import to guarantee backward compatibility here, i.e., tables created by previous Spark versions should still be readable in newer Spark versions. Previously we find backward compatibility issues manually, which is really easy to miss bugs. This PR introduces a test framework to automatically test `HiveExternalCatalog` backward compatibility, by downloading Spark binaries with different versions, and create tables with these Spark versions, and read these tables with current Spark version. test-only change Author: Wenchen Fan <[email protected]> Closes apache#19148 from cloud-fan/test.
|
cc @gatorsmile |
dongjoon-hyun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, LGTM.
|
Test build #81547 has finished for PR 19163 at commit
|
|
|
||
| object PROCESS_TABLES extends QueryTest with SQLTestUtils { | ||
| // Tests the latest version of every release line. | ||
| val testingVersions = Seq("2.0.2", "2.1.1", "2.2.0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is a backport to 2.2, do we still need to test backward compatibility on 2.2?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought it's needed against 2.2.0, here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is needed
|
LGTM |
…eExternalCatalog backport #19148 to 2.2 Author: Wenchen Fan <[email protected]> Closes #19163 from cloud-fan/test.
|
Thanks! Merged to 2.2. Could you close this PR? |
…eExternalCatalog backport apache#19148 to 2.2 Author: Wenchen Fan <[email protected]> Closes apache#19163 from cloud-fan/test.
backport #19148 to 2.2