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

Make MySQL queries wrap set operations with parens #782

Merged
merged 2 commits into from
Apr 26, 2024

Conversation

pandaxtc
Copy link
Contributor

Add parens around set operations for MySQL. Fixes #773.

Note that I removed the tests for checking no parens for MySQL, since adding back the parens brings the behavior in line with the default. I assume the tests are for checking that the behavior is not default. Let me know if the tests should be added back.

@wd60622
Copy link
Contributor

wd60622 commented Dec 28, 2023

I'm not fully versed on the test suite, but it might be best to test that the union now has parentheses instead of deleting the previous test. No harm in double checking

@pandaxtc
Copy link
Contributor Author

pandaxtc commented Jan 4, 2024

Gotcha, just added the tests back.

Copy link
Contributor

@wd60622 wd60622 left a comment

Choose a reason for hiding this comment

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

Awesome. With passing tests, looks good to me!

@wd60622
Copy link
Contributor

wd60622 commented Jan 17, 2024

@AzisK Could you run the test suite?

@coveralls
Copy link

Coverage Status

coverage: 98.39%. remained the same
when pulling 1e797c7 on pandaxtc:pandaxtc/fix-mysql-set-wrap
into 8841520 on kayak:master.

@danielenricocahall
Copy link
Contributor

@AzisK I hope all is going well! Would we be able to get this in as well? Looks like a small, useful change to include in a release if one is anticipated soon. 😄

@AzisK
Copy link

AzisK commented Apr 26, 2024

Looks good to me

@AzisK AzisK merged commit 53a77eb into kayak:master Apr 26, 2024
5 checks passed
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.

MySQL queries should wrap set operations
5 participants