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

ExecuteUpdate with cross apply produces invalid SQL #28823

Closed
roji opened this issue Aug 22, 2022 · 1 comment · Fixed by #28834
Closed

ExecuteUpdate with cross apply produces invalid SQL #28823

roji opened this issue Aug 22, 2022 · 1 comment · Fixed by #28834
Assignees
Labels
area-bulkcud closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Milestone

Comments

@roji
Copy link
Member

roji commented Aug 22, 2022

On PG, test Update_with_cross_apply_set_constant produces the following SQL:

UPDATE "Customers" AS c
SET "ContactName" = 'Updated'
FROM (
    SELECT o."OrderID", o."CustomerID", o."EmployeeID", o."OrderDate"
    FROM "Orders" AS o
    WHERE o."OrderID" < 10300 AND date_part('year', o."OrderDate")::int < length(c."ContactName")::int
) AS t
WHERE c."CustomerID" LIKE 'F%'

This fails with the following, since c.ContactName references an outer column:

ERROR:  invalid reference to FROM-clause entry for table "c" at character 240
HINT:  There is an entry for table "c", but it cannot be referenced from this part of the query.

Originally discussed in npgsql/efcore.pg#2475 (comment)

@smitpatel
Copy link
Contributor

Currently we generate 5 kind of "join" like table sources. Each of them have different kind of effect on results. (also there could be more if someone extends TableExpressionBase)

  • Inner Join
  • Left Join
  • Cross Join
  • Cross Apply
  • Outer Apply

For the Update statement, if we end up omitting very first table then we need to lift the next table into the "FROM" clause. FROM clause doesn't allow everything all joins allow.

  • It doesn't allow any references from outside (even from the table being updated, that also has to be on outer level only). So both APPLY kind of joins are eliminated.
  • A Left joined table may have lesser rows (and fill in nulls when joining), so raising it to FROM will clause the result set to change so that is also eliminated.

Only Inner/Cross join are safe. (We don't know about custom ones which providers can handle).
This restriction only applies if we are removing the first table, if it is the table being updated. Once "FROM" table is set, all kind of joins are fine.

@smitpatel smitpatel added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Aug 22, 2022
@smitpatel smitpatel added this to the 7.0.0 milestone Aug 23, 2022
@smitpatel smitpatel modified the milestones: 7.0.0, 7.0.0-rc2 Aug 29, 2022
@ajcvickers ajcvickers modified the milestones: 7.0.0-rc2, 7.0.0 Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-bulkcud closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants