Skip to content

Conversation

@gatorsmile
Copy link
Member

What changes were proposed in this pull request?

DESC FUNCTION and DESC EXTENDED FUNCTION should be added to SQLQueryTestSuite for us to review the outputs of each function.

How was this patch tested?

N/A

@SparkQA
Copy link

SparkQA commented Mar 25, 2017

Test build #75194 has finished for PR 17424 at commit 3ef9a06.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@gatorsmile
Copy link
Member Author

retest this please

@SparkQA
Copy link

SparkQA commented Mar 25, 2017

Test build #75201 has finished for PR 17424 at commit 3ef9a06.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@rxin
Copy link
Contributor

rxin commented Mar 25, 2017

Hm - so this would require us to update the test suite every time there is an update to the docs?

@gatorsmile
Copy link
Member Author

gatorsmile commented Mar 25, 2017

@rxin Yes! After we add it to SQLQueryTestSuite, we can copy and paste the contents to the document in http://spark.apache.org/docs/latest/sql-programming-guide.html So far, our Spark SQL users have no idea which functions we provide unless they read the source codes.

In the future, if anybody update the docs in the source codes, they have to update the test suite. Then, we can add a reminder in the test case to let them also update the public SQL document.

Do you like the idea?

@gatorsmile
Copy link
Member Author

retest this please

@SparkQA
Copy link

SparkQA commented Mar 25, 2017

Test build #75222 has finished for PR 17424 at commit 3ef9a06.

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

@viirya
Copy link
Member

viirya commented Mar 29, 2017

To document the functions available sounds good. But I am not sure if we want to put this into test. Sounds like we use the test as document generator.

@gatorsmile
Copy link
Member Author

@viirya That is not only for document generator. It can help us review/correct the output of DESC FUNCTIONS EXTENDED. You can find many inconsistent parts we should correct when reading describe-functions-extended.sql.out.

Actually, I also think we need to output the physical plans for many important queries in SQLQueryTestSuite . That can help us capture the accidental plan changes when we fix the bugs.

@viirya
Copy link
Member

viirya commented Mar 30, 2017

I am not sure if it helps review by dumping the output of DESC EXTENDED FUNCTION to the test. We may not frequently change the output as I see. IMHO, It is hard to tell which is "correct" output for a function, except for obvious incorrectness like wrong parameters, results, but this kind of errors should be detected in reviewing. It is also hard to check the consistency of output in a 3000-lines text file.

@gatorsmile gatorsmile closed this Jun 13, 2017
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.

4 participants