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

fix(electric): Rewrite PostgreSQL URI parser #706

Merged
merged 4 commits into from
Nov 29, 2023

Conversation

alco
Copy link
Member

@alco alco commented Nov 29, 2023

Main improvements:

  • support postgres:// scheme
  • default to using the username as the database name if the latter is missing from the URI (default behaviour of psql)

Fixes VAX-1264, VAX-1265.

Copy link

linear bot commented Nov 29, 2023

VAX-1265 Electric's PG URL parsing fails on `postgres://`

Electric can only parse the postgresql:// scheme. However, some hosting providers, such as Fly.io, may use postgres://. Electric chokes on that:

$ DATABASE_URL=postgres://electric:passwordlocalhost:54321/electric iex -S mix
Erlang/OTP 25 [erts-13.2] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit:ns]

** (MatchError) no match of right hand side value: "postgres"
    (postgresql_uri 0.1.0) lib/postgresql_uri.ex:89: PostgresqlUri.parse/1
    config/runtime.dev.exs:15: (file)
    /home/alco/code/electric-sql/electric/components/electric/config/runtime.exs:172: (file)

Copy link
Contributor

@magnetised magnetised left a comment

Choose a reason for hiding this comment

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

lovely stuff. nice to lose a dependency right.

components/electric/lib/electric/utils.ex Outdated Show resolved Hide resolved
Copy link

linear bot commented Nov 29, 2023

VAX-1264 Electric erroneously creates a replication slot named "electric_replication_out_test"

Electric generates a name for the replication slot that it creates along with a subcription by appending a custom suffix to the string "electric_replication_out". The suffix is derived from the database name which is looked up on the connection config. However, if the DATABASE_URL does not include an explicit database name, the suffix is set to "test".

I saw this when I was using Fly Postgres. The connection URI it provides looks as follows:

postgres://postgres:****ancient-pine-7827.flycast:5432

It does not include the database name because it is the same as the user name: postgres. So the replication slot name Electric generated in that case was "electric_replication_out_test".

@alco alco merged commit d9efe92 into main Nov 29, 2023
4 checks passed
@alco alco deleted the alco/vax-1265-postgresql-uri-parser branch November 29, 2023 13:14
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.

2 participants