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

bug: SQL compilation error...unexpected '.' when table name is ORDER #230

Closed
pnadolny13 opened this issue Jul 24, 2024 · 9 comments · Fixed by #231 or #242 · May be fixed by #232
Closed

bug: SQL compilation error...unexpected '.' when table name is ORDER #230

pnadolny13 opened this issue Jul 24, 2024 · 9 comments · Fixed by #231 or #242 · May be fixed by #232
Assignees
Labels
bug Something isn't working

Comments

@pnadolny13
Copy link
Contributor

The target is generating invalid SQL when the stream name is ORDER because thats a reserved word and its not getting double quoted. For example a stream named tap_jaffle_shop-ORDER

sqlalchemy.exc.ProgrammingError: (snowflake.connector.errors.ProgrammingError) 001003 (42000): 01b5e42c-0705-1c49-0051-5c8309be0026: SQL compilation error:
syntax error line 1 at position 38 unexpected '.'.
[SQL: merge into SCHEMA_NAME.TAP_JAFFLE_SHOP.ORDER d using (select $1:id::VARCHAR as id, $1:customer::VARCHAR as customer, $1:ordered_at::TIMESTAMP_NTZ as ordered_at, $1:store_id::VARCHAR as store_id, $1:subtotal::DECIMAL as subtotal, $1:tax_paid::DECIMAL as tax_paid, $1:order_total::DECIMAL as order_total, $1:_sdc_extracted_at::TIMESTAMP_NTZ as _sdc_extracted_at, $1:_sdc_received_at::TIMESTAMP_NTZ as _sdc_received_at, $1:_sdc_batched_at::TIMESTAMP_NTZ as _sdc_batched_at, $1:_sdc_deleted_at::TIMESTAMP_NTZ as _sdc_deleted_at, $1:_sdc_sequence::DECIMAL as _sdc_sequence, $1:_sdc_table_version::DECIMAL as _sdc_table_version, $1:_sdc_sync_started_at::DECIMAL as _sdc_sync_started_at from '@~/target-snowflake/tap_jaffle_shop-ORDER-6a024391-605f-4849-ae8f-7467c11f5166'(file_format => SCHEMA_NAME.TAP_JAFFLE_SHOP."tap_jaffle_shop-ORDER-6a024391-605f-4849-ae8f-7467c11f5166") QUALIFY ROW_NUMBER() OVER (PARTITION BY id ORDER BY SEQ8() DESC) = 1) s on d.id = s.id when matched then update set d.id = s.id, d.customer = s.customer, d.ordered_at = s.ordered_at, d.store_id = s.store_id, d.subtotal = s.subtotal, d.tax_paid = s.tax_paid, d.order_total = s.order_total, d._sdc_extracted_at = s._sdc_extracted_at, d._sdc_received_at = s._sdc_received_at, d._sdc_batched_at = s._sdc_batched_at, d._sdc_deleted_at = s._sdc_deleted_at, d._sdc_sequence = s._sdc_sequence, d._sdc_table_version = s._sdc_table_version, d._sdc_sync_started_at = s._sdc_sync_started_at when not matched then insert (id, customer, ordered_at, store_id, subtotal, tax_paid, order_total, _sdc_extracted_at, _sdc_received_at, _sdc_batched_at, _sdc_deleted_at, _sdc_sequence, _sdc_table_version, _sdc_sync_started_at) values (s.id, s.customer, s.ordered_at, s.store_id, s.subtotal, s.tax_paid, s.order_total, s._sdc_extracted_at, s._sdc_received_at, s._sdc_batched_at, s._sdc_deleted_at, s._sdc_sequence, s._sdc_table_version, s._sdc_sync_started_at)]
@pnadolny13
Copy link
Contributor Author

cc @cjohnhanson @edgarrmondragon

@edgarrmondragon
Copy link
Member

Fixed in 0.9.0

@pnadolny13
Copy link
Contributor Author

pnadolny13 commented Jul 26, 2024

@edgarrmondragon I'm still getting errors when I try to sync a table called "ORDER". When I debug, strangely it doesnt seem like the quoted_name function is doing anything for the fully qualified name or even just the table name.

Screenshot 2024-07-26 at 11 34 07 AM

I'm also remembering now how tricky this casing and quoting logic is. What I'm observing is that full_table_name should be where we add these quotes so that its reflected across the merge/alter/update/etc. statements but the problem is that it seems the SQL is somehow being executed using different methods where some cases automatically add these quotes (i.e. the create table statement) but others do not (merge/alter/etc.). This makes it tricky because I tried changing the full_table_name property to add quotes when needed but that results in create statements like CREATE TABLE STAGING_RAW.TAP_JAFFLE_SHOP."""ORDER""".

@pnadolny13 pnadolny13 reopened this Jul 26, 2024
@pnadolny13
Copy link
Contributor Author

pnadolny13 commented Jul 26, 2024

For example I hard coded the merge statement to properly quote 'merge into STAGING_RAW.TAP_JAFFLE_SHOP."ORDER" d using ' and everything works but if I then drop a column from the table and run the sync again I get this error because theres other places like altering that are building SQL that arent yet handled:

sqlalchemy.exc.ProgrammingError: (snowflake.connector.errors.ProgrammingError) 001003 (42000): 01b5edf2-0705-1fe2-0051-5c8309cf89f2: SQL compilation error:
syntax error line 1 at position 40 unexpected 'ORDER'.
[SQL: ALTER TABLE STAGING_RAW.TAP_JAFFLE_SHOP.ORDER ADD COLUMN store_id VARCHAR]
(Background on this error at: https://sqlalche.me/e/14/f405)

@pnadolny13
Copy link
Contributor Author

pnadolny13 commented Jul 26, 2024

When the table is created at this point in the SDK connector class the full_table_name is unquoted 'STAGING_RAW.TAP_JAFFLE_SHOP.ORDER' (and the results of parse_full_table_name are also still not quoted) but the result is an info log showing that it automatically quoted the schema and table name 2024-07-26 12:01:14,077 | INFO | snowflake.connector.cursor | query: [CREATE TABLE "TAP_JAFFLE_SHOP"."ORDER" ( id VARCHAR NOT NULL, customer VARCHAR, ...].

I think the SDK needs to either use native sqlalchemy methods that automatically quote (i.e. stop building SQL statements manually) or stop using the native method thats auto quoting. I think the only place auto quoting is the create table statement and as a user its much easier to manually build SQL strings like we do for merge/copy/etc. in this target, so I'd prefer to avoid auto quoting in the create table statement. This way I can handle the quoting myself in the table_name or conform_name methods. What do you think?

@edgarrmondragon edgarrmondragon self-assigned this Jul 30, 2024
@edgarrmondragon edgarrmondragon added the bug Something isn't working label Jul 30, 2024
@pnadolny13
Copy link
Contributor Author

@edgarrmondragon I gave this another shot and it doesnt seem like an easy fix. I was attempting to escape the reserved word consistently but now the problem is that full_table_name is being passed around and parsed to retrieve the table name string. That string would have a double quote around it so using it in any sqlalchemy functions like inspect arent working because the table isnt found. Again because sqlalchemy expects to do the escaping but we've already done it.

@edgarrmondragon
Copy link
Member

Yeah I've also been trying to replace our stringified SQL constructions with SQLAlchemy-native ones, but it's indeed painful to work around the full_table_name being passed instead of a more workable Table instance without a larger refactor 😬.

@edgarrmondragon
Copy link
Member

@pnadolny13 I think I got at least a path towards a solution in meltano/sdk#2601 and #242 🎉

@edgarrmondragon
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment