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-30644][SQL][TEST] Remove query index from the golden files of SQLQueryTestSuite #27361

Closed
wants to merge 3 commits into from

Conversation

gatorsmile
Copy link
Member

What changes were proposed in this pull request?

This PR is to remove query index from the golden files of SQLQueryTestSuite

Why are the changes needed?

Because the SQLQueryTestSuite's golden files have the query index for each query, removal of any query statement [except the last one] will generate many unneeded difference. This will make code review harder. The number of changed lines is misleading.

Does this PR introduce any user-facing change?

No

How was this patch tested?

N/A

@gatorsmile
Copy link
Member Author

cc @dongjoon-hyun @maropu

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Jan 25, 2020

Ya. I agree that the index causes too many meaningless diff although the original intention was good.

@maropu
Copy link
Member

maropu commented Jan 26, 2020

Yeah, it looks reasonable to me. We still need -- !query and -- !query output as separators? It seems PgSQL has such a separator in the golden files, e.g., : https://github.com/postgres/postgres/blob/REL_12_STABLE/src/test/regress/expected/groupingsets.out

@SparkQA
Copy link

SparkQA commented Jan 26, 2020

Test build #117403 has finished for PR 27361 at commit 0cbbe9c.

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

@gatorsmile
Copy link
Member Author

I think the separators are still needed.

select grouping(a), a, array_agg(b),
       rank(a) within group (order by b nulls first),
       rank(a) within group (order by b nulls last)
  from (values (1,1),(1,4),(1,5),(3,1),(3,2)) v(a,b)
 group by rollup (a) order by a;
 grouping | a |  array_agg  | rank | rank 
----------+---+-------------+------+------
        0 | 1 | {1,4,5}     |    1 |    1
        0 | 3 | {1,2}       |    3 |    3
        1 |   | {1,4,5,1,2} |    1 |    6
(3 rows)

The PgSQL golden files look cleaner, but it only shows the output column names, but does not output the column data types.

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-30644] [TEST] Remove query index from the golden files of SQLQueryTestSuite [SPARK-30644][SQL][TEST] Remove query index from the golden files of SQLQueryTestSuite Jan 26, 2020
@SparkQA
Copy link

SparkQA commented Jan 26, 2020

Test build #117408 has finished for PR 27361 at commit 02b15f8.

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

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM. Merged to master.
Thank you, @gatorsmile and @maropu .

Copy link
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

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

LGTM too

@maropu
Copy link
Member

maropu commented Jan 26, 2020

yea, it looks fine. late LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants