MeltanoLabs / target-postgres

MIT License
11 stars 20 forks source link

bug: Uppercase stream names cause failures #119

Closed pnadolny13 closed 1 year ago

pnadolny13 commented 1 year ago

Originally raised in https://meltano.slack.com/archives/C013EKWA2Q1/p1680488722168999

I was able to replicate the behavior. I sent some dummy data to the target using an uppercase stream name Account, like this:

{"type": "RECORD", "stream": "Account", "record": {"timestamp": ...

I see the table created in the database but then immediately get this error:

2023-04-03 13:10:44,096 | INFO     | target-postgres      | Target 'target-postgres' is listening for input from tap.
2023-04-03 13:10:44,096 | INFO     | target-postgres      | Initializing 'target-postgres' target sink...
2023-04-03 13:10:44,096 | INFO     | target-postgres      | Initializing target sink for stream 'Account'...
2023-04-03 13:10:44,181 | INFO     | target-postgres      | Target 'target-postgres' completed reading 4 lines of input (2 records, (0 batch manifests, 1 state messages).
Traceback (most recent call last):
  File "/Users/pnadolny/Documents/Git/meltano_project/quickbooks_postgres/qb_pg/.meltano/loaders/target-postgres/venv/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 1900, in _execute_context
    self.dialect.do_execute(
  File "/Users/pnadolny/Documents/Git/meltano_project/quickbooks_postgres/qb_pg/.meltano/loaders/target-postgres/venv/lib/python3.9/site-packages/sqlalchemy/engine/default.py", line 736, in do_execute
    cursor.execute(statement, parameters)
psycopg2.errors.UndefinedTable: relation "test_schema_ml.account" does not exist
LINE 1: ...69d0_1b6e_4660_adde_5acda4c8925e AS SELECT * FROM test_schem...
                                                             ^

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/pnadolny/Documents/Git/meltano_project/quickbooks_postgres/qb_pg/.meltano/loaders/target-postgres/venv/bin/target-postgres", line 8, in <module>
    sys.exit(TargetPostgres.cli())
  File "/Users/pnadolny/Documents/Git/meltano_project/quickbooks_postgres/qb_pg/.meltano/loaders/target-postgres/venv/lib/python3.9/site-packages/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
  File "/Users/pnadolny/Documents/Git/meltano_project/quickbooks_postgres/qb_pg/.meltano/loaders/target-postgres/venv/lib/python3.9/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
  File "/Users/pnadolny/Documents/Git/meltano_project/quickbooks_postgres/qb_pg/.meltano/loaders/target-postgres/venv/lib/python3.9/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/Users/pnadolny/Documents/Git/meltano_project/quickbooks_postgres/qb_pg/.meltano/loaders/target-postgres/venv/lib/python3.9/site-packages/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
  File "/Users/pnadolny/Documents/Git/meltano_project/quickbooks_postgres/qb_pg/.meltano/loaders/target-postgres/venv/lib/python3.9/site-packages/singer_sdk/target_base.py", line 572, in cli
    target.listen(file_input)
  File "/Users/pnadolny/Documents/Git/meltano_project/quickbooks_postgres/qb_pg/.meltano/loaders/target-postgres/venv/lib/python3.9/site-packages/singer_sdk/io_base.py", line 35, in listen
    self._process_endofpipe()
  File "/Users/pnadolny/Documents/Git/meltano_project/quickbooks_postgres/qb_pg/.meltano/loaders/target-postgres/venv/lib/python3.9/site-packages/singer_sdk/target_base.py", line 287, in _process_endofpipe
    self.drain_all(is_endofpipe=True)
  File "/Users/pnadolny/Documents/Git/meltano_project/quickbooks_postgres/qb_pg/.meltano/loaders/target-postgres/venv/lib/python3.9/site-packages/singer_sdk/target_base.py", line 449, in drain_all
    self._drain_all(list(self._sinks_active.values()), self.max_parallelism)
  File "/Users/pnadolny/Documents/Git/meltano_project/quickbooks_postgres/qb_pg/.meltano/loaders/target-postgres/venv/lib/python3.9/site-packages/singer_sdk/target_base.py", line 475, in _drain_all
    self.drain_one(sink)
  File "/Users/pnadolny/Documents/Git/meltano_project/quickbooks_postgres/qb_pg/.meltano/loaders/target-postgres/venv/lib/python3.9/site-packages/singer_sdk/target_base.py", line 469, in drain_one
    sink.process_batch(draining_status)
  File "/Users/pnadolny/Documents/Git/meltano_project/quickbooks_postgres/qb_pg/.meltano/loaders/target-postgres/venv/lib/python3.9/site-packages/target_postgres/sinks.py", line 63, in process_batch
    self.connector.create_temp_table_from_table(
  File "/Users/pnadolny/Documents/Git/meltano_project/quickbooks_postgres/qb_pg/.meltano/loaders/target-postgres/venv/lib/python3.9/site-packages/target_postgres/connector.py", line 71, in create_temp_table_from_table
    self.connection.execute(ddl)
  File "/Users/pnadolny/Documents/Git/meltano_project/quickbooks_postgres/qb_pg/.meltano/loaders/target-postgres/venv/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 1380, in execute
    return meth(self, multiparams, params, _EMPTY_EXECUTION_OPTS)
  File "/Users/pnadolny/Documents/Git/meltano_project/quickbooks_postgres/qb_pg/.meltano/loaders/target-postgres/venv/lib/python3.9/site-packages/sqlalchemy/sql/ddl.py", line 80, in _execute_on_connection
    return connection._execute_ddl(
  File "/Users/pnadolny/Documents/Git/meltano_project/quickbooks_postgres/qb_pg/.meltano/loaders/target-postgres/venv/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 1472, in _execute_ddl
    ret = self._execute_context(
  File "/Users/pnadolny/Documents/Git/meltano_project/quickbooks_postgres/qb_pg/.meltano/loaders/target-postgres/venv/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 1943, in _execute_context
    self._handle_dbapi_exception(
  File "/Users/pnadolny/Documents/Git/meltano_project/quickbooks_postgres/qb_pg/.meltano/loaders/target-postgres/venv/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 2124, in _handle_dbapi_exception
    util.raise_(
  File "/Users/pnadolny/Documents/Git/meltano_project/quickbooks_postgres/qb_pg/.meltano/loaders/target-postgres/venv/lib/python3.9/site-packages/sqlalchemy/util/compat.py", line 211, in raise_
    raise exception
  File "/Users/pnadolny/Documents/Git/meltano_project/quickbooks_postgres/qb_pg/.meltano/loaders/target-postgres/venv/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 1900, in _execute_context
    self.dialect.do_execute(
  File "/Users/pnadolny/Documents/Git/meltano_project/quickbooks_postgres/qb_pg/.meltano/loaders/target-postgres/venv/lib/python3.9/site-packages/sqlalchemy/engine/default.py", line 736, in do_execute
    cursor.execute(statement, parameters)
sqlalchemy.exc.ProgrammingError: (psycopg2.errors.UndefinedTable) relation "test_schema_ml.account" does not exist
LINE 1: ...69d0_1b6e_4660_adde_5acda4c8925e AS SELECT * FROM test_schem...
                                                             ^

[SQL: CREATE TEMP TABLE temp_Account_faf469d0_1b6e_4660_adde_5acda4c8925e AS SELECT * FROM test_schema_ml.Account LIMIT 0]
(Background on this error at: https://sqlalche.me/e/14/f405)

cc @visch @aaronsteers @edgarrmondragon

aaronsteers commented 1 year ago

Is this a bug in the SDK? I think in general we should bias towards case-insensitive comparisons.

These streams should all land into the same table without error and without duplication:

MyStream
Mystream
mystream

And these tables should all be considered valid destination tables for a stream named "MyStream":

MyStream
Mystream
mystream

We can (optionally!) let the target decide how/if to normalize casing, but regardless of casing preferences of the destination system, we should find and load to existing tables even if casing is not a match.

visch commented 1 year ago

I'd bet on it being something on the postgres target specifically. I agree it should work regardless of casing but I do think the case should follow whatever was initially set, maybe I care about something that doesn't matter here though.

A lot of this comes to the normalization stuff I ripped out. Maybe just tolower and a replace dashes and underscores. That would be an easy fix here but wouldn't fix this generically

visch commented 1 year ago

Solved with https://github.com/MeltanoLabs/target-postgres/pull/138