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

Bug Report: Update with empty map/struct model causes invalid SQL generation #858

Open
jamesjulich opened this issue Jun 28, 2023 · 2 comments

Comments

@jamesjulich
Copy link

If you ask bun to create an update statement using a model with all nil fields, then call OmitZero, you will end up with an invalid query as seen here.

db.NewUpdate().
	Where("? = ?", bun.Ident("id"), c.Params("id")).
	OmitZero().
	Model(&myEmptyStruct{}). // also confirmed to happen with empty map, ie. new(map[string]interface{})
	String();

will produce SQL invalid SQL similar to this: UPDATE "test" AS "test_struct" SET WHERE ("id" = '1')

I think #684 is reporting the same issue, but it's been dormant for a while and I think this warrants bumping.

Since 'SET' is a required field, I think it makes sense to raise an error if nothing is appended to the SET statement in mustAppendSet.

@jamesjulich jamesjulich changed the title Bug Report: Update with empty map/struct causes invalid SQL generation Bug Report: Update with empty map/struct model causes invalid SQL generation Jun 28, 2023
@jamesjulich
Copy link
Author

jamesjulich commented Jun 30, 2023

I believe one of your test cases is also generating invalid sql. Unless syntax errors are intended to be caught at execution time (and not at generation), then this is probably also a bug.

// internal\dbtest\query_test.go
// Line 191
func(db *bun.DB) schema.QueryAppender {
			return db.NewUpdate().Model(new(Model)).WherePK()
		},

This test query generates invalid SQL, and your test cases are checking that the library generates queries that match it.

@hohobilly
Copy link
Contributor

Also looking into the same issue.
Please correct me if I'm wrong, I wonder if // Line 191 db.NewUpdate().Model(new(Model)).WherePK() is valid because without OmitZero() it should imply for setting zero value for the empty fields.
Instead, there is another test case in Line 571 which seems to fit this invalid generation issue.

  func(db *bun.DB) schema.QueryAppender {
    return db.NewUpdate().Model(&Model{ID: 42}).OmitZero().WherePK()
  },

> UPDATE `models` AS `model` SET WHERE (`model`.`id` = 42)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants