search

transferwise / pipelinewise-tap-postgres

Singer.io Tap for PostgreSQL - PipelineWise compatible
https://transferwise.github.io/pipelinewise/
GNU Affero General Public License v3.0
41 stars 101 forks source link

replication slot names must be lower case #21

Closed andyoneal closed 5 years ago

andyoneal commented 5 years ago

the db I'm trying to replicate is named "DWH", all caps. If I try to create a replication slot with the correct naming scheme, I get this error: ERROR: replication slot name "pipelinewise_DWH" contains invalid character HINT: Replication slot names may only contain lower case letters, numbers, and the underscore character.

If I create it with lower case letters, logical replication can't find the replication slot: CRITICAL Unable to find replication slot pipelinewise_DWH with wal2json

If I use "dbname = dwh" in my config, discovery and presumably many other things fail.

Is the easiest fix just to .lower() the dbname when searching for the replication slot?

koszti commented 5 years ago

Yes, I think you're right and .lower() makes sense in this case but we also need to modify the tap-postgres fastsync component to keep the naming in sync. Postgres FastSync is creating replication slots automatically and it should use the same naming convention.

I think we need to modify at two places:

  1. tap-postgres: https://github.com/transferwise/pipelinewise-tap-postgres/blob/ae5540fa88c27702d2d67c9d0db3668cf638e6a9/tap_postgres/sync_strategies/logical_replication.py#L351

  2. FastSync tap-postgres: https://github.com/transferwise/pipelinewise/blob/7a60b9dbf0b8eb1804930a389d4240485d1c0e35/pipelinewise/fastsync/commons/tap_postgres.py#L105

Please, if you can prepare a quick PR we're happy to review and it can also speed up getting things merged.

koszti commented 5 years ago

fixed