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

Sqlite: Using column from table being updated in left join predicate generates invalid SQL #28886

Open
smitpatel opened this issue Aug 25, 2022 · 6 comments

Comments

@smitpatel
Copy link
Contributor

Test: Update_with_cross_join_left_join_set_constant

        AssertExecuteUpdateSql(
            @"UPDATE ""Customers"" AS ""c""
SET ""ContactName"" = 'Updated'
FROM (
    SELECT ""c0"".""CustomerID"", ""c0"".""Address"", ""c0"".""City"", ""c0"".""CompanyName"", ""c0"".""ContactName"", ""c0"".""ContactTitle"", ""c0"".""Country"", ""c0"".""Fax"", ""c0"".""Phone"", ""c0"".""PostalCode"", ""c0"".""Region""
    FROM ""Customers"" AS ""c0""
    WHERE ""c0"".""City"" IS NOT NULL AND (""c0"".""City"" LIKE 'S%')
) AS ""t""
LEFT JOIN (
    SELECT ""o"".""OrderID"", ""o"".""CustomerID"", ""o"".""EmployeeID"", ""o"".""OrderDate""
    FROM ""Orders"" AS ""o""
    WHERE ""o"".""OrderID"" < 10300
) AS ""t0"" ON ""c"".""CustomerID"" = ""t0"".""CustomerID""
WHERE ""c"".""CustomerID"" LIKE 'F%'");

Now throws invalid SQL error earlier used to work. Change in PR #28884
Filed https://www.sqlite.org/forum/forumpost/09862e12c7

@roji
Copy link
Member

roji commented Aug 25, 2022

😠

The person on that issue is asking for our schema. We're just like users when we file issues on other repos 🤣

@smitpatel
Copy link
Contributor Author

The person on that issue is asking for our schema. We're just like users when we file issues on other repos 🤣

Not really. I don't think there is anything specific to schema there. It is like if some user file issue on us that string.IndexOf is not working on SQLServer, we don't ask for schema if they post the query and generated SQL or exception message.

@ajcvickers ajcvickers added this to the Backlog milestone Aug 31, 2022
@roji
Copy link
Member

roji commented Nov 9, 2022

This seems like it's the same as npgsql/efcore.pg#2478, which I worked around in npgsql/efcore.pg#2478. Note that this limitation blocks some fairly basic scenarios, like two inner joins.

I've posted another comment on the SQLite issue, to clarify whether they intend to fix this or not. We can simply port my PostgreSQL workaround to SQLite to fix this.

@roji roji removed this from the Backlog milestone Nov 9, 2022
@ajcvickers ajcvickers added this to the Backlog milestone Nov 16, 2022
@roji
Copy link
Member

roji commented Nov 19, 2022

This doesn't seem to be a problem with MySQL/MariaDB (the test isn't skipped), @lauxjpn can you confirm?

So this seems to be a problem with PostgreSQL and SQLite, and I worked around it in PostgreSQL (npgsql/efcore.pg#2554). We can wait to see if the SQLite folks react on their issue, and if users complain, we can consider bringing the same workaround from PG to SQLite.

@roji roji removed the blocked label Nov 19, 2022
@roji roji self-assigned this Nov 19, 2022
@lauxjpn
Copy link
Contributor

lauxjpn commented Nov 21, 2022

This doesn't seem to be a problem with MySQL/MariaDB (the test isn't skipped), @lauxjpn can you confirm?

Yes, there are no issues with the Update_with_cross_join_left_join_set_constant test and any version of MySQL and MariaDB that we support.

@roji
Copy link
Member

roji commented Nov 21, 2022

Thanks @lauxjpn

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

4 participants