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

Access beyond end of array returns null, which can lead to us returning incorrect results #33784

Open
roji opened this issue May 22, 2024 · 0 comments

Comments

@roji
Copy link
Member

roji commented May 22, 2024

See test PrimitiveCollectionsQuerySqlServerTest.Nullable_reference_column_collection_index_equals_nullable_column.

The LINQ query in question is: `ss.Set().Where(c => c.NullableStrings[2] == c.NullableString)``. Since both sides of the equality are nullable, we generate the following SQL:

WHERE JSON_VALUE([p].[NullableStrings], '$[2]') = [p].[NullableString] OR (JSON_VALUE([p].[NullableStrings], '$[2]') IS NULL AND [p].[NullableString] IS NULL)

The null compensation here is meant for the case where NullableStrings actually contains NULL in the appropriate index, and also NullableString. However, when the index (2) is beyond the end of the array, that also returns NULL, which incorrectly matches the actual NULL on the other side.

We can include an array length check, but that seems like a very heavy hammer for each and every query where an array access is performed. Another option is to switch to strict mode; that would cause queries to fail whenever they access beyond the end of the array - but that's also the .NET behavior (note the breaking change).

Interestingly, on Cosmos, accessing beyond the end of the array yields undefined, not NULL, and so we actually get correct results. Note that in the test (Nullable_reference_column_collection_index_equals_nullable_column), the "expected" (LINQ to Objects) assertion in the test is written incorrectly, also defaulting to null when the array is too small; so the test incorrectly passes for SQL Server but fails for Cosmos.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants