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

Add some zero column tests covering LIMIT, GROUP BY, WHERE, JOIN, and WINDOW #11624

Merged
merged 4 commits into from
Jul 25, 2024

Conversation

Kev1n8
Copy link
Contributor

@Kev1n8 Kev1n8 commented Jul 23, 2024

Which issue does this PR close?

Closes: #5713.

Rationale for this change

To make sure there are not any other execs having issues when it comes to zero-column batches.

What changes are included in this PR?

I added some tests to sqllogictest to see if statements work with no column when using LIMIT, GROUP BY, WHERE JOIN and WINDOW. Please let me know if there's anything to improve or wrong.

Are these changes tested?

PASSed the sqllogictests.

Are there any user-facing changes?

@github-actions github-actions bot added the sqllogictest SQL Logic Tests (.slt) label Jul 23, 2024
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @Kev1n8 🙏

FYI @kazuyukitanimura as the original filer of #5713

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you for this contribution @Kev1n8 -- I double checked the tests and I think the coverage is good.

@@ -1225,6 +1225,63 @@ statement ok
SELECT * EXCEPT(a, b, c, d)
FROM table1

# try zero column with LIMIT, 1 row but empty
statement ok
Copy link
Contributor

Choose a reason for hiding this comment

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

I had these tests checked out locally to double check they actually did produce no columns but a single row and by using query rather than statement

Thing looks great to me, so I pushed a commit to this branch to update the tests

Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 that seems to cause errors on mac, so I reverted the change

@alamb
Copy link
Contributor

alamb commented Jul 24, 2024

I also updated the description to say this PR closes #5713, unless there is additional work we know about

@Kev1n8
Copy link
Contributor Author

Kev1n8 commented Jul 25, 2024

Thanks for reviewing :)

Copy link
Contributor

@kazuyukitanimura kazuyukitanimura left a comment

Choose a reason for hiding this comment

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

Thank you @Kev1n8 @alamb

@alamb alamb merged commit 49d9d45 into apache:main Jul 25, 2024
24 checks passed
@alamb
Copy link
Contributor

alamb commented Jul 25, 2024

🚀

@Kev1n8 Kev1n8 deleted the add-zero-column-test branch July 25, 2024 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sqllogictest SQL Logic Tests (.slt)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make an end to end reproducer for zero column batch issues
3 participants