-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-28885][SQL] Follow ANSI store assignment rules in table insertion by default #26107
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
Changes from all commits
ae40a71
6f9cfa1
ad8f578
758ac1f
28c49e7
3d80200
4bd989e
944a426
24056b9
b602083
cb70ddc
e24e35d
e7ccbac
4b77736
b9abb67
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -143,7 +143,19 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession { | |
|
|
||
| /** List of test cases to ignore, in lower cases. */ | ||
| protected def blackList: Set[String] = Set( | ||
| "blacklist.sql" // Do NOT remove this one. It is here to test the blacklist functionality. | ||
| "blacklist.sql", // Do NOT remove this one. It is here to test the blacklist functionality. | ||
| // SPARK-28885 String value is not allowed to be stored as numeric type with | ||
| // ANSI store assignment policy. | ||
| "postgreSQL/numeric.sql", | ||
| "postgreSQL/int2.sql", | ||
| "postgreSQL/int4.sql", | ||
| "postgreSQL/int8.sql", | ||
| "postgreSQL/float4.sql", | ||
| "postgreSQL/float8.sql", | ||
| // SPARK-28885 String value is not allowed to be stored as date/timestamp type with | ||
| // ANSI store assignment policy. | ||
| "postgreSQL/date.sql", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @gengliangwang Sorry, I just realized recently that my changed are not tested by
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about just setting
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @maropu I am not sure about that.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But, before this pr's been merged, we tested the PgSQL tests in the LEGACY mode? Is my understanding wrong? Personally, I think we need to explicitly file issues in jira (Or, comment out them?) if we have inaccurate tests in
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about changing the timestamp/date values from string literal to timestamp/date literal in those sql files, just as https://github.com/apache/spark/pull/26107/files#diff-431a4d1f056a06e853da8a60c46e9ca0R68
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yea, I think that's ok. cc: @wangyum @dongjoon-hyun @HyukjinKwon
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm +1 for @gengliangwang 's suggestion |
||
| "postgreSQL/timestamp.sql" | ||
| ) | ||
|
|
||
| // Create all the test cases. | ||
|
|
||
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 think we can write null to any nullable column even if it's strict policy. We can have a followup PR to discuss it further.