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

Alias validation code throws false positive for subquery referenced multiple times in the tree #32455

Closed
roji opened this issue Nov 29, 2023 · 1 comment

Comments

@roji
Copy link
Member

roji commented Nov 29, 2023

The following Northwind query:

AssertQuery(
            async,
            ss => ss.Set<OrderDetail>()
                .Where(e => e.OrderID == 8)
                .GroupBy(e => e.Quantity)
                .Select(g => new { g.Key, MaxTimestamp = g.Select(e => e.Order.OrderDate).Max() })
                .OrderBy(x => x.MaxTimestamp));

... triggers a failure in DEBUG only (also in 7.0), from TableAliasVerifyingExpressionVisitor. The SQL that would get generated (without the check) is the following:

SELECT [o].[Quantity] AS [Key], (
    SELECT MAX([o3].[OrderDate])
    FROM [Order Details] AS [o2]
    INNER JOIN [Orders] AS [o3] ON [o2].[OrderID] = [o3].[OrderID]
    WHERE [o2].[OrderID] = 8 AND [o].[Quantity] = [o2].[Quantity]) AS [MaxTimestamp]
FROM [Order Details] AS [o]
WHERE [o].[OrderID] = 8
GROUP BY [o].[Quantity]
ORDER BY (
    SELECT MAX([o3].[OrderDate])
    FROM [Order Details] AS [o2]
    INNER JOIN [Orders] AS [o3] ON [o2].[OrderID] = [o3].[OrderID]
    WHERE [o2].[OrderID] = 8 AND [o].[Quantity] = [o2].[Quantity])

Note that [o1] is missing, which is what triggers the error. The check is generally to detect referential integrity issues, but this seems to be a false positive.

What's happening is that the same SelectExpression instance is referenced from both the ordering and the projection; when we apply the projection after translation, we uniquify the aliases within; this mutates the TableReferenceExpression, but since that's shared, the change occurs both in the ordering and in the projection.

AFAICT this has no actual effect except for the aliases not being properly ordered in the final SQL (but it blocks proper testing on #32234). Ideally, if we get to a place where ColumnExpressions reference TableExpressionBases directly (no more TableReferenceExpression) and everything is properly immutable, this would go away on its own.

Discovered as part of the work on #32234 - we generally have problems with SelectExpression which are referenced multiple times.

@roji
Copy link
Member Author

roji commented Nov 29, 2023

Duplicate of #26104

@roji roji marked this as a duplicate of #26104 Nov 29, 2023
@roji roji closed this as not planned Won't fix, can't repro, duplicate, stale Nov 29, 2023
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

1 participant