bun icon indicating copy to clipboard operation
bun copied to clipboard

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

Open jamesjulich opened this issue 2 years ago • 2 comments

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 avatar Jun 28 '23 04:06 jamesjulich

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.

jamesjulich avatar Jun 30 '23 03:06 jamesjulich

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)

hohobilly avatar Jul 23 '23 14:07 hohobilly