dotnet / dotnet/efcore

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

Open
#33,784 1 comment 0 reactions 0 assignees View on GitHub
area-json area-query customer-reported needs-design
Dominant language
C#
Stars
14.8k
Forks
3.4k
PR merge metrics
PR metrics pending

Description

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:

```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.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.