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

SQL-2626: Add spec tests for subquery comparison expressions #38

Merged
merged 4 commits into from
Feb 28, 2025

Conversation

mattChiaravalloti
Copy link
Collaborator

This PR adds type constraint spec tests for subquery comparison expressions. In the design doc, we initially said we would query test the new document and array comparison features for subquery expressions. However, as I audited the existing spec query and spec type constraint tests, I realized (1) we do not have any other type-related query tests for subquery comparison expressions and (2) we did not have any existing spec type constraint tests for subquery comparison expressions. Given that, I decided to make these new type constraint tests and they demonstrate exactly what we wanted to check.

@@ -4,7 +4,7 @@ use crate::{
catalog::Catalog,
map, parser,
schema::{Atomic, Document, Schema},
set, SchemaCheckingMode,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was an unused import 😮

Comment on lines +238 to +239
keys,
required,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I mainly did this because I wanted the subquery tests to look reasonable as opposed to being something like SELECT * FROM foo WHERE arg1 = ANY(SELECT arg2 FROM foo).

@@ -59,10 +59,10 @@ Here is an example of such a test, followed by a brief explanation:
```
In this example, the query contains the expression `arg1 + arg2`, an addition operation.
There is one map in the `valid_types` sequence. It indicates that `arg1` can be any
numeric type, `NULL`, or missing; it also indicates that `arg2` can be any numeric type,
`NULL`, or missing. Since these lists are included in the same map, that means any
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Patrick recently realized we can't actually support MISSING in these tests using the test runner as implemented because it is invalid for a field's sole schema to be MISSING. So we removed missing from the test suite.

This change is just some lingering clean up to remove references to missing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since I added the same keys/required for foo to bar, we need to disambiguate in the queries now. That's not a bad tradeoff by any means.

Copy link
Member

@bucaojit bucaojit left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@terakilobyte terakilobyte left a comment

Choose a reason for hiding this comment

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

LGTM :shipit:

@mattChiaravalloti mattChiaravalloti added this pull request to the merge queue Feb 28, 2025
Merged via the queue into mongodb:main with commit d906c7c Feb 28, 2025
10 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.

3 participants