Skip to content

Conversation

ajcvickers
Copy link
Contributor

Fixes #32325
Port of #32510

Description

When using OPENJSON for the new translation of Contains, if the items in the list are concatenated from items that have type facets, then the resultant type mapping might not be big enough to contain the entire value. This then may return incorrect results.

Customer impact

Queries which previously worked may now contain incorrect results.

How found

Customers reported on 8.0

Regression

Yes

Testing

Added

Risk

Low and quirked.

ajcvickers and others added 2 commits December 4, 2023 19:47
* Work on type mapping inference for string concatenation

Fixes #32325, see also #32333

* Tweaks, update baselines add more tests and cleanup

* Update baselines, add more tests, some cleanup.

---------

Co-authored-by: Shay Rojansky <[email protected]>
@ajcvickers ajcvickers requested a review from a team December 5, 2023 10:11
@ajcvickers ajcvickers added this to the 8.0.x milestone Dec 5, 2023
@ajcvickers ajcvickers requested a review from roji December 5, 2023 11:00
@leecow leecow modified the milestones: 8.0.x, 8.0.2 Dec 5, 2023
@AndriySvyryd
Copy link
Member

Note that the branch is currently closed.

@indrajitjadeja
Copy link

Fix #32325 has resulted in regressions for Oracle EF Provider customers.

Using the replication code given in bug,
SQL EF Provider 8.0.0 generates the following query:

SELECT COUNT(*) FROM [Persons] AS [p] WHERE COALESCE([p].[ThreeCharacterProperty], '') + ';' + COALESCE([p].[FiveCharacterProperty], '') IN ( SELECT [q].[value] FROM OPENJSON(@__queryData_0) WITH ([value] char(3) '$') AS [q] )

which requires correction in OPENJSON subquery generation.

After the fix is delivered in 8.0.2, the same test case generates the following query:

SELECT COUNT(*) FROM [Persons] AS [p] WHERE COALESCE([p].[ThreeCharacterProperty], N'') + N';' + COALESCE([p].[FiveCharacterProperty], N'') IN (N'AAA;BBBBB', N'AAA;CCCCC')

which got updated with the standard IN condition and skipped the creation of the OPENJSON subquery.

This fix was not anticipated in the relational layer because the OPENJSON function is SQL Server provider-specific. As a result, the Oracle provider, which uses a standard IN condition in such cases and is dependent on inferring the right type and size by base class, experienced a regression.

@ajcvickers @roji
Why was the fix (inferring the size and type of the SQL binary expression) decided to be included in the relational layer rather than in the SQL Server provider by overriding the SqlExpressionFactory.ApplyTypeMapping() method?

@roji
Copy link
Member

roji commented Feb 29, 2024

@indrajitjadeja the fix in #32510 only corrected the length on the type mapping; this is a general type mapping inference fix that isn't SQL Server-specific (even if the original bug manifested specifically in SQL Server). I'm still not clear on how it exactly it affects the Oracle SQL translation - can you please open a new issue with a clear minimal, runnable repro and the SQL generated in 8.0.1 and 8.0.2?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants