Skip to content

postgres support - #360

Closed
erma07 wants to merge 5 commits into
cschleiden:mainfrom
erma07:main
Closed

postgres support#360
erma07 wants to merge 5 commits into
cschleiden:mainfrom
erma07:main

Conversation

@erma07

@erma07 erma07 commented Jul 18, 2024

Copy link
Copy Markdown

No description provided.

Comment on lines +35 to +40
func SQLReplacer(src string) string {
for nParam := 1; strings.Contains(src, "?"); nParam++ {
src = strings.Replace(src, "?", fmt.Sprintf("$%d", nParam), 1)
}
return src
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that the only change in the code compared to the mysql backend or was there more you had to change?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I can remember it correct, the migration sql files and the SQLReplacer was the only thing i changed in order to get postgres to work. Wanted the query's to remain as same as possible so there would be only copy-paste without much thinking to do.

@erma07 erma07 Aug 2, 2024

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are minor changes in backend/postgres/postgres.go file:

"github.com/golang-migrate/migrate/v4/database/postgres"

dsn := fmt.Sprintf("host=%s port=%d user=%s password=%s dbname=%s sslmode=require", host, port, user, password, database)

db, err := sql.Open("postgres", dsn)

db, err := sql.Open("postgres", schemaDsn)

dbi, err := postgres.WithInstance(db, &postgres.Config{})

m, err := migrate.NewWithInstance("iofs", migrations, "postgres", dbi)

opt(options)
}

dsn := fmt.Sprintf("host=%s port=%d user=%s password=%s dbname=%s sslmode=require", host, port, user, password, database)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

postgres specific change

@plunkettscott

Copy link
Copy Markdown

Any idea when this may be reviewed? I just stumbled across this and would really like postgres as a backend.

@@ -0,0 +1,70 @@
CREATE TABLE IF NOT EXISTS instances (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason NOT EXISTS is used here? In cases like migration (especially creation) being critical path, this may skip creating a table when one already exists with different schema. Reentrancy here has too much of an edge case.

Comment thread backend/postgres/postgres.go Outdated
"github.com/golang-migrate/migrate/v4/database/postgres"
"github.com/golang-migrate/migrate/v4/source/iofs"
"github.com/google/uuid"
_ "github.com/lib/pq"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lib/pq is largely stale at this point, jackc/pgx is actively worked on. it is compatible with database/sql` as well, see https://pkg.go.dev/github.com/jackc/pgx/v5/stdlib

}
defer tx.Rollback()

row := tx.QueryRowContext(ctx, SQLReplacer("SELECT state FROM instances WHERE instance_id = ? AND execution_id = ? LIMIT 1"), instance.InstanceID, instance.ExecutionID)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erma07 I think you are fine to use positional parameters when using pgx, e.g.

Suggested change
row := tx.QueryRowContext(ctx, SQLReplacer("SELECT state FROM instances WHERE instance_id = ? AND execution_id = ? LIMIT 1"), instance.InstanceID, instance.ExecutionID)
row := tx.QueryRowContext(
ctx,
"SELECT state FROM instances WHERE instance_id = $1 AND execution_id = $2 LIMIT 1",
instance.InstanceID,
instance.ExecutionID,
)

@emmanuelay

Copy link
Copy Markdown

Is this being worked on?

@Smithx10

Copy link
Copy Markdown

Any word on this?

@vr009

vr009 commented Sep 16, 2025

Copy link
Copy Markdown
Contributor

@cschleiden This PR seems stuck. Do you mind if I finish it so that it can make it into the next release?

@cschleiden

Copy link
Copy Markdown
Owner

Closing in favor of #441

@cschleiden cschleiden closed this Oct 10, 2025
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

Successfully merging this pull request may close these issues.

7 participants