go-gorm / gorm

The fantastic ORM library for Golang, aims to be developer friendly
https://gorm.io
MIT License
36.03k stars 3.86k forks source link

postgresql : UPDATE query formed incorrectly, update ... set ... FROM ... where #7055

Closed alkuma closed 5 days ago

alkuma commented 3 weeks ago

GORM Playground Link

https://github.com/go-gorm/playground/pull/746 provided by @ajhodges

Description

I have an update executed within a database transaction and that is leading to the following faulty query

UPDATE users 
SET col1 = value,
col2  = value,
...
col 20  = value
FROM users -- WRONG! FROM must not be present in the update query 
WHERE user_id = value 
AND user_role.is_del = 0

The error obtained from postgres is

ERROR: table name "uaa_role" specified more than once (SQLSTATE 42712)

I have another location where this query works fine, the only difference I see is that at SELECT has been performed on the same table before this update.

Is this a case of the stale FROM clause from the SELECT lying in the parse tree when the UPDATE is query is being formed? If so what is a workaround for this?

this is using

    gorm.io/driver/postgres v1.5.7
    gorm.io/gorm v1.25.10

Here is a sequence of queries that ran before getting to this faulty update query. The sequence is edited by hand for brevity


[rows:1] SELECT count(*) FROM users WHERE user_id = value AND users.is_del = 0

[rows:26] SELECT * FROM users WHERE ("tenant_id","user_id") IN (('Value','Value'))

[rows:1] SELECT * FROM users WHERE user_id = 'value' AND users.is_del = 0 ORDER BY user_id 

[rows:1] SELECT count(*) FROM users WHERE users.is_del = 0

followed by the faulty query

Are there any changes that happened which could lead to the previous SELECT query's parse tree is still lying around and polluting the parser input for the update?

If so is there a workaround for this?

github-actions[bot] commented 3 weeks ago

The issue has been automatically marked as stale as it missing playground pull request link, which is important to help others understand your issue effectively and make sure the issue hasn't been fixed on latest master, checkout https://github.com/go-gorm/playground for details. it will be closed in 30 days if no further activity occurs. if you are asking question, please use the Question template, most likely your question already answered https://github.com/go-gorm/gorm/issues or described in the document https://gorm.ioSearch Before Asking

marek-veber commented 3 weeks ago

I have the same issue (with gorm.io/driver/postgres v1.5.7& gorm.io/gorm v1.25.10). It's based on: db.Model(...).Find(....).Update(...)

github-actions[bot] commented 3 weeks ago

The issue has been automatically marked as stale as it missing playground pull request link, which is important to help others understand your issue effectively and make sure the issue hasn't been fixed on latest master, checkout https://github.com/go-gorm/playground for details. it will be closed in 30 days if no further activity occurs. if you are asking question, please use the Question template, most likely your question already answered https://github.com/go-gorm/gorm/issues or described in the document https://gorm.ioSearch Before Asking

ajhodges commented 1 week ago

@alkuma I added a playground test case for this here, I'm facing the same issue: https://github.com/go-gorm/playground/pull/746

ajhodges commented 1 week ago

FWIW it looks like this is a regression that started in gorm.io/driver/postgres v1.5.3 (v1.5.2 does not have this issue)

github-actions[bot] commented 1 week ago

The issue has been automatically marked as stale as it missing playground pull request link, which is important to help others understand your issue effectively and make sure the issue hasn't been fixed on latest master, checkout https://github.com/go-gorm/playground for details. it will be closed in 30 days if no further activity occurs. if you are asking question, please use the Question template, most likely your question already answered https://github.com/go-gorm/gorm/issues or described in the document https://gorm.ioSearch Before Asking

github-actions[bot] commented 1 week ago

The issue has been automatically marked as stale as it missing playground pull request link, which is important to help others understand your issue effectively and make sure the issue hasn't been fixed on latest master, checkout https://github.com/go-gorm/playground for details. it will be closed in 30 days if no further activity occurs. if you are asking question, please use the Question template, most likely your question already answered https://github.com/go-gorm/gorm/issues or described in the document https://gorm.ioSearch Before Asking

alkuma commented 1 week ago

@ajhodges https://github.com/go-gorm/playground/pull/746 is a much more concise test case than mine. Let me add this to the original issue report thank you

gabizou commented 1 week ago

Just got bit by this issue as well, it's even more apparent when using soft deletion "updates".

a631807682 commented 5 days ago

use Session api https://gorm.io/docs/method_chaining.html#Reusability-and-Safety

ajhodges commented 3 days ago

@a631807682 I updated the example here with a session: https://github.com/go-gorm/playground/pull/746

It has no effect. I still get this error:

2024/06/29 16:28:21 /Users/adamhodges/src/gorm-playground/main_test.go:20 ERROR: table name "users" specified more than once (SQLSTATE 42712)
[0.635ms] [rows:1] UPDATE "users" SET "name"='jinzhu 2',"updated_at"='2024-06-29 16:28:21.723' FROM "users" WHERE "users"."id" = 1 AND "users"."deleted_at" IS NULL AND "id" = 1
--- FAIL: TestGORM (0.00s)
    main_test.go:21: Failed, got error: ERROR: table name "users" specified more than once (SQLSTATE 42712)
FAIL
exit status 1
FAIL    gorm.io/playground  0.580s

Again, this seems to be a regression in gorm.io/driver/postgres v1.5.3+

a631807682 commented 2 days ago

A critical aspect of GORM is understanding when a gorm.DB instance is safe to reuse. Following a Chain Method or Finisher Method, GORM returns an initialized gorm.DB instance. This instance is not safe for reuse as it may carry over conditions from previous operations, potentially leading to contaminated SQL queries.

The documentation explains why sessions are needed, but it does not mean that you can use sessions everywhere as you expect. https://github.com/go-gorm/playground/pull/746/files#diff-79ce3229c13921b79b1175dcb211336aaf84dfd8edcf19c07698934d4fe70e5eR20