glebarez / sqlite

The pure-Go SQLite driver for GORM
MIT License
615 stars 40 forks source link

AutoMigrate causes an "invalid DDL, unbalanced brackets" error #51

Closed morremeyer closed 2 years ago

morremeyer commented 2 years ago

Bug

Using db.AutoMigrate with a specific Model causes the sqlite module to return an error invalid DDL, unbalanced brackets.

This is happening in https://github.com/envelope-zero/backend/blob/2adc00f348d0be5ad0101d72fbe37f2ecbc95bbf/main.go#L47. Using git bisect, I tracked it back to the upgrade of gorm to 1.23.6, (this commit), however, I am not sure why it only started occuring there.

The bug does not occur with the upstream sqlite driver.

Details

The error specifically seems to occur on the Transactions model. Tracing it back by debugging the parseDDL function, I noticed that one call to the function is broken.

parseDDL is called with one string:

"CREATE TABLE `transactions__temp` (`id` text,`created_at` datetime,`updated_at` datetime,`deleted_at` datetime,`date` datetime,`amount` ?,8),`note` text,`budget_id` text,`source_account_id` text,`destination_account_id` text,`envelope_id` text,`reconciled` numeric,PRIMARY KEY (`id`),CONSTRAINT `fk_transactions_budget` FOREIGN KEY (`budget_id`) REFERENCES `budgets`(`id`),CONSTRAINT `fk_transactions_source_account` FOREIGN KEY (`source_account_id`) REFERENCES `accounts`(`id`),CONSTRAINT `fk_transactions_destination_account` FOREIGN KEY (`destination_account_id`) REFERENCES `accounts`(`id`),CONSTRAINT `fk_transactions_envelope` FOREIGN KEY (`envelope_id`) REFERENCES `envelopes`(`id`))"

Notice the

`amount` ?,8)

part, which is clearly broken. The model specifies this as

Amount               decimal.Decimal `json:"amount" gorm:"type:DECIMAL(20,8)"`

The error is returned on ddlmod.go, line 77, with a bracketLevel of -1.

Reproduction

  1. git clone https://github.com/envelope-zero/backend.git
  2. go run main.go and attach a debugger with set breakpoints (In VSCode, you can use the Run API (SQLite)

Additional info

I’d like to debug this further, but so far I failed to find a good approach to find out where the string gets mangled and wasn't able to find this out. I’ll happily debug this further once I find an approach and I’m happy about any input anyone might be able to provide.

glebarez commented 2 years ago

I will merge recent commits from upstream soon and let you know, so you can retest this. I am 99% sure the reason is falling back behind upstream, specifically they had recent commits for supporting DDL with type sizes brb

glebarez commented 2 years ago

Merged the upstream. Can you get latest (non-tagged) version from master and re-test?

morremeyer commented 2 years ago

Re-tested, can't reproduce the problem. Thanks for the fast response and taking care of it!