Postgres - add x-migrations-table-quoted url query option (#95)

[stephane-klein]

Following this comment, this is a new Pull Request.

By default, gomigrate quote migrations table name, if x-migrations-table-quoted is enabled, then you must to quote migrations table name manually, for instance "gomigrate"."schema_migrations".

I have used this playground to test this branch.

[stephane-klein]

@dhui what do you think about this implementation?

[FAQinghere]

I’m probably just confused, but in ensureVersionTable(), line 454, why does it not respect the config setting for the schema when it’s verifying whether or not the table exists already?

	query := `SELECT COUNT(1) FROM information_schema.tables WHERE table_name = $1 AND table_schema = (SELECT current_schema()) LIMIT 1`

[stephane-klein]

I’m probably just confused, but in ensureVersionTable(), line 454, why does it not respect the config setting for the schema when it’s verifying whether or not the table exists already?

Ok, it is a new code: spacefill@511ae9f

I'm going to fix this part 👍

[stephane-klein]

I’m probably just confused, but in ensureVersionTable(), line 454, why does it not respect the config setting for the schema when it’s verifying whether or not the table exists already?

@FAQinghere @dhui implemented in my last push.

I think that the next task is to implement the same feature for database/pgx/ version, isn't it?

[stephane-klein]

I don't understand this golangci-lint error:

$ golangci-lint run
database/postgres/postgres.go:44:13: struct of size 80 bytes could be of size 72 bytes (maligned)
type Config struct {
            ^

Exited with code exit status 1

The code is here

type Config struct {
	MigrationsTable       string
	MigrationsTableQuoted bool
	DatabaseName          string
	SchemaName            string
	StatementTimeout      time.Duration
	MultiStatementEnabled bool
	MultiStatementMaxSize int
}

I don't understand where is my error🤔

[FAQinghere]

Words are 4 or 8 bytes long (32-bit vs 64-bit systems), and fields in structs are probably word-aligned. If a bool only needs one byte, and is followed by something else that doesn’t fit within the same word, then the rest of the word will be padded out (skipped), bloating the struct. Maybe move the bools to the end.

[coveralls]

Pull Request Test Coverage Report for Build 292

• 77 of 81 (95.06%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.4%) to 57.241%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
database/pgx/pgx.go	38	40	95.0%
database/postgres/postgres.go	39	41	95.12%

Totals
Change from base Build 274:	0.4%
Covered Lines:	3502
Relevant Lines:	6118

---

💛 - Coveralls

[stephane-klein]

I think that the next task is to implement the same feature for database/pgx/ version, isn't it?

database/pgx/ implementation done.

@dhui I think that you can review.

[dhui]

Thanks for the PR!

Thanks for updating both the postgres and pgx drivers! Don't forget to apply any changes to both the pgx and postgres drivers.

[stephane-klein]

@dhui all you feedbacks had been applied.

New need review 😉

[dhui]

Thanks for addressing the PR feedback!

[stephane-klein]

New need review 😉 (see this comment)

[dhui]

Thanks for addressing the PR feedback!

For future changes, can you push your changes as a new commit on top of the latest PR commit? If you need to force push to re-run tests, just amend the latest commit. This makes it easier for me to review so I don't need to review the whole PR again. If there are a lot of commits, I'll squash-merge the PR.

[stephane-klein]

For future changes, can you push your changes as a new commit on top of the latest PR commit?

Ok 👍

@dhui New commit with fix is here: 37af6dd

[dhui]

Thanks for going through the multiple rounds of feedback!

[stephane-klein]

@dhui thanks 🙂

nitpick: last commit message Work in progress has not been removed throughout the squash-merge :

* Postgres and pgx - Add x-migrations-table-quoted url query option to postgres and pgx drivers (#95)

By default, gomigrate quote migrations table name, if `x-migrations-table-quoted` is enabled, then you must to quote migrations table name manually, for instance `"gomigrate"."schema_migrations"`

* Work In Progress

[dhui]

With Github's squash-merge, the commit messages are combined into the squashed commit's message.

[stephane-klein]

With Github's squash-merge, the commit messages are combined into the squashed commit's message.

Yes I know, but it was a "work in progress" commit message.

That why I like to squash + push --force to avoid to push accidentally this "bad" commit message to master branch.