Skip to content
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

[SPARK-28624][SQL][TESTS][3.0] Run date.sql via Thrift Server #28723

Closed

Conversation

MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Jun 4, 2020

What changes were proposed in this pull request?

Enable date.sql and run it via Thrift Server in ThriftServerQueryTestSuite.

Why are the changes needed?

To improve test coverage.

Does this PR introduce any user-facing change?

No

How was this patch tested?

By running the enabled tests via:

$ build/sbt -Phive-thriftserver "hive-thriftserver/test-only *ThriftServerQueryTestSuite -- -z date.sql"

@MaxGekk
Copy link
Member Author

MaxGekk commented Jun 4, 2020

@HyukjinKwon Please, review this PR.

@HyukjinKwon
Copy link
Member

Looks good if the tests pass. WDYT can you point out which change fixed this if you already know?

@MaxGekk
Copy link
Member Author

MaxGekk commented Jun 4, 2020

@HyukjinKwon I guess this PR fixes the issue mainly:
#28582

@SparkQA
Copy link

SparkQA commented Jun 4, 2020

Test build #123519 has finished for PR 28723 at commit 81f63e0.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HyukjinKwon
Copy link
Member

Let's cut out RC3 first and then merge.

@@ -68,8 +68,6 @@ class ThriftServerQueryTestSuite extends SQLQueryTestSuite with SharedThriftServ
// Missing UDF
"postgreSQL/boolean.sql",
"postgreSQL/case.sql",
// SPARK-28624
"date.sql",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thriftserver doesn't support negative year, I think we still need to ignore this test.

@HyukjinKwon
Copy link
Member

I discussed with @MaxGekk @cloud-fan. The tests will have to be disabled back at SPARK-30808. Basically, resulting to 0045-03-15 itself is actually controversial.

We strictly can just merge and enable it for Spark 3.0 specifically because SPARK-30808 won't land to branch-3.0 but let me just don't merge this for simplicity.

@HyukjinKwon HyukjinKwon closed this Jun 5, 2020
@MaxGekk MaxGekk deleted the enable-date.sql-for-thrift-3.0 branch June 5, 2020 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants