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

Support select and explain for UNION queries #1972

Merged
merged 3 commits into from
Jul 26, 2024

Conversation

friedelwolff
Copy link
Contributor

Some UNION queries can start with "(", causing it to not be classified as a SELECT query, and consequently the select and explain buttons are missing on the SQL panel.

Some UNION queries can start with "(", causing it to not be
classified as a SELECT query, and consequently the select and
explain buttons are missing on the SQL panel.
@@ -729,6 +729,16 @@ def test_similar_and_duplicate_grouping(self):
self.assertNotEqual(queries[0]["similar_color"], queries[3]["similar_color"])
self.assertNotEqual(queries[0]["duplicate_color"], queries[3]["similar_color"])

@unittest.skipUnless(
connection.vendor == "postgresql", "Test valid only on PostgreSQL"
Copy link
Member

Choose a reason for hiding this comment

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

How come this is only valid for postgres? I feel like the other db drivers should also pass this test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From a quick look at the Django code, this can only happen if the DB backend has supports_slicing_ordering_in_compound = True. But I see now it is more widely supported than I thought. Shall I rather use

@unittest.skipUnless(connection.vendor != "sqlite", "...")

?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure why the skipUnless is even necessary? The test should pass on all database engines anyway, or not?

@tim-schilling tim-schilling merged commit 969f0a7 into django-commons:main Jul 26, 2024
25 checks passed
@tim-schilling
Copy link
Member

Thank you @friedelwolff!

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.

3 participants