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

accept queries that make array-to-scalar comparisons #107

Conversation

hallettj
Copy link
Collaborator

@hallettj hallettj commented Sep 24, 2024

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,

db.movies.aggregate([{ $match: { cast: { $eq: "Albert Austin" } } }])

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:

  • find a scalar comparison operator definition for equality corresponding to the scalar type of array elements in the given column
  • infer the appropriate scalar type for the comparison value instead of assuming that the value type in an equality comparison is the same as the column type

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.

  1. inspect inline values or variable values given for the right operand, and assume an
    array-to-scalar comparison only if all of those are non-array values
  2. or get the GraphQL Engine to include a type with ComparisonValue in which case we can
    use 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.

@hallettj hallettj self-assigned this Sep 24, 2024
@hallettj hallettj marked this pull request as ready for review September 25, 2024 00:23
@hallettj
Copy link
Collaborator Author

hallettj commented Sep 25, 2024

I included a test that uses query request variables, which is failing. It comes down to an inconsistency in MongoDB's behavior:

sample_mflix> db.movies.aggregate([{ $match: { cast: { $eq: "Albert Austin" } } }, { $count: "rows" }])
[ { rows: 1 } ]

sample_mflix> db.movies.aggregate([{ $match: { $expr: { $eq: ["$cast", "Albert Austin"] } } }, { $count: "rows" }])
< no rows >

We need to use the second syntax when comparing to variables because variables cannot be referenced in a query document (like the argument to $match in the first query). But because these two forms behave differently existential comparison over arrays won't work in remote joins.

Edit: I fixed the variable problem by building an explicit array operation in that case.

return Ok(());
}
}

Copy link
Collaborator

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.

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 think it would make sense to disable v5 testing in another PR

@hallettj hallettj merged commit 1062532 into main Sep 27, 2024
1 check passed
@hallettj hallettj deleted the jessehallett/eng-1118-mongodb-support-comparing-array-columns-to-scalar-values branch September 27, 2024 00:03
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.

2 participants