-
Notifications
You must be signed in to change notification settings - Fork 1
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
accept queries that make array-to-scalar comparisons #107
accept queries that make array-to-scalar comparisons #107
Conversation
I included a test that uses query request variables, which is failing. It comes down to an inconsistency in MongoDB's behavior:
We need to use the second syntax when comparing to variables because variables cannot be referenced in a query document (like the argument to Edit: I fixed the variable problem by building an explicit array operation in that case. |
return Ok(()); | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still need mongodb 5 tests? We stopped supporting it. In the readme we state 6 and above. Also 5 is EOL Oct 2024.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would make sense to disable v5 testing in another PR
…ing-array-columns-to-scalar-values
Important: this change allows filtering documents by scalar values inside arrays. But it does so using behavior of the GraphQL Engine that is currently undefined. We are in the process of defining that behavior - in a future Engine update to keep array-of-scalar comparisons working it will be necessary to configure additional metadata, and it will be necessary to upgrade the MongoDB connector at the same time. We expect that GraphQL APIs will remain the same before and after that Engine update.
MongoDB allows comparing array values to scalars. For example,
The condition passes if any array element passes the comparison.
Currently the GraphQL Engine does not have a way to filter by scalar values in array fields. But it does allow declaring that array fields can be used in scalar comparisons. So with some connector updates this allows us to unblock users who need to be able to filter by array values.
This PR updates the connector to:
Some notes on the implementation and implications:
The connector needs to know operand types for comparisons. The left operand is always a database
document field so we can get that type from the schema. But if the right operand is an inline value
or a variable we have to infer its type from context.
Normally we assume that the right operand of Equal is the same type as the left operand. But since
MongoDB allows comparing arrays to scalar values then in case that is desired then instead of
inferring an array type for the right operand to match the left, we want the type of the right
operand to be the array element type of the left operand. That brings up the question: how do we
know if the user's intention is to make an array-to-scalar comparison, or an array-to-array
comparison? Since we don't support array-to-array comparisons yet for simplicity this PR assumes
that if the field has an array type, the right-operand type is a scalar type.
When we do support array-to-array comparisons we have two options.
array-to-scalar comparison only if all of those are non-array values
ComparisonValue
in which case we canuse that as the right-operand type
It is important that queries behave the same when given an inline value or variables. So we can't
just check inline values, and punt on variables. It will require a little more work to thread
variables through the code to the point they are needed for this check so I haven't done that just
yet.
Edit: We have plans to update the NDC spec to get the necessary type information from the Engine in a future version.