ValueError: Unable to merge sql types: INTEGER, INTEGER #9
Comments
|
|
@visch - Looks like this may be resolve with : Can you confirm? |
|
@aaronsteers Yes Fixes this exception, then we immeditly hit a new one As upserts aren't implemented yet working on that now |
|
There's also a new error during the first run Not certain what this is from |
|
That is from your insert so you are past the table check and build out. Maybe check the |
@BuzzCutNorman The API is a WIP, and def generate_insert_statement(
self,
full_table_name: str,
schema: dict,
):
stmt = super().generate_insert_statement(full_table_name, schema)
stmt += "ON CONFLICT ...`Wdyt? |
|
@edgarrmondragon I wish I was good enough at python to have a definite educated opinion on that. it looks good to me. The only question I have is why manually create the insert statement when you could let
|
|
@visch sweet the title look like it is working for you now 🎈 🥳 🎈 |
I went with my previous iteration for checking Type differences! But it does work at least for target-postgres's use case |
|
That'd be ideal. Though it would require target devs to rely on dialect-specific SQLAlchemy functions, like @aaronsteers one thing that's perhaps not too clear is where responsibility of the from sqlalchemy.dialects.sqlite import insert
class SQLiteSink(SQLSink):
...
def generate_insert_statement(
self,
full_table_name: str,
schema: dict,
) -> str:
engine = self.connector.create_sqlalchemy_engine()
meta = sqlalchemy.MetaData(bind=engine)
table = sqlalchemy.Table(full_table_name, meta, autoload=True)
statement = insert(table)
if self.key_properties:
statement = statement.on_conflict_do_update(
index_elements=table.primary_key.columns,
set_={
column.name: getattr(statement.excluded, column.name)
for column in table.columns
},
)
return statement |
After working with this a little I thought maybe this might be a way. In the cookiecutter for a SQL target what if you asked what dialect they were going to use. Then add that as an import at the top of the |
|
@kgpayne, @BuzzCutNorman, @edgarrmondragon - Circling back on this... I believe this is resolved as of now, due to recent improvements and bug fixes on the SQL interfaces. Do I have that right? |
Created an issue #22 for this @aaronsteers I'm not certain if the main SDK fixes this now or not. I'll have to test a fresh cookiecutter and after I do that I"ll close up this issue. The key for this repo is this is fixed now :) |
|
From @edgarrmondragon:
I gave this tons of thought and sorry, I missed your question above at the time and did not reply directly. I propose the distinction:
This means that Wdyt? |
|
@visch - re:
Fantastic - thanks!
👍 Thanks! |
|
@aaronsteers this works well from a new cookiecutter target! |
To replicate run tap-smoke-test twice.
The text was updated successfully, but these errors were encountered: