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

SNOW-952817: Reflect doesn't work with mixed case schema #458

Open
fredrike opened this issue Oct 27, 2023 · 8 comments
Open

SNOW-952817: Reflect doesn't work with mixed case schema #458

fredrike opened this issue Oct 27, 2023 · 8 comments
Labels
bug Something isn't working duplicate This issue or pull request already exists status-triage_done Initial triage done, will be further handled by the driver team

Comments

@fredrike
Copy link

Please answer these questions before submitting your issue. Thanks!

  1. What version of Python are you using?
Python 3.8.16 (default, Jan 17 2023, 23:13:24) 
[GCC 11.2.0]
  1. What operating system and processor architecture are you using?

    Linux-5.4.0-1074-azure-x86_64-with-glibc2.17

  2. What are the component versions in the environment (pip freeze)?

snowflake-connector-python==2.7.9
snowflake-sqlalchemy==1.4.6
SQLAlchemy==1.4.35
  1. What did you do?

    from snowflake.sqlalchemy import URL
    from sqlalchemy import create_engine
    from sqlalchemy import MetaData
    
    engine = create_engine(
          URL(
              account=ACCOUNT,
              user=secrets[USR_KEY],
              password=secrets[PASS_KEY],
              database=DATABASE_NAME,
              schema=SCHEMA_NAME,
              warehouse=warehouse,
              role=ROLE,
          )
      )
    meta_data = MetaData(engine)
    meta_data.reflect(only=lambda l, _: 'control' in l, views=True, schema="EPS_CO_Technical")
    print(meta_data.tables.keys())
    
  2. What did you expect to see?

    The tables containing control should be listed. Instead i get this error:

    ProgrammingError: (snowflake.connector.errors.ProgrammingError) 002043 (02000): SQL compilation error:
    Object does not exist, or operation cannot be performed.
    [SQL: SHOW /* sqlalchemy:_get_table_comment */ TABLES LIKE '_controlTable_MapPocPrice' IN SCHEMA EPS_CO_Technical]
    (Background on this error at: https://sqlalche.me/e/14/f405)
    
  3. Can you set logging to DEBUG and collect the logs?

    Better, I have a suggested patch:

    diff --git a/snowdialect.py b/anaconda/envs/azureml_py38/lib/python3.8/site-packages/snowflake/sqlalchemy/snowdialect.py
    --- a/snowdialect.py
    +++ b/anaconda/envs/azureml_py38/lib/python3.8/site-packages/snowflake/sqlalchemy/snowdialect.py
    @@ -813,7 +813,7 @@ class SnowflakeDialect(default.DefaultDialect):
                 "SHOW /* sqlalchemy:_get_table_comment */ "
                 "TABLES LIKE '{}'{}".format(
                     table_name,
    -                f" IN SCHEMA {self.normalize_name(schema)}" if schema else "",
    +                f" IN SCHEMA \"{self.denormalize_name(schema)}\"" if schema else "",
                 )
             )
             cursor = connection.execute(text(sql_command))
    @@ -827,7 +827,7 @@ class SnowflakeDialect(default.DefaultDialect):
                 "SHOW /* sqlalchemy:_get_view_comment */ "
                 "VIEWS LIKE '{}'{}".format(
                     table_name,
    -                f" IN SCHEMA {self.normalize_name(schema)}" if schema else "",
    +                f" IN SCHEMA \"{self.denormalize_name(schema)}\"" if schema else "",
                 )
             )
             cursor = connection.execute(text(sql_command))

    So, qoute the schema name seems to do the trick.

@fredrike fredrike added bug Something isn't working needs triage labels Oct 27, 2023
@github-actions github-actions bot changed the title Reflect doesn't work with mixed case schema SNOW-952817: Reflect doesn't work with mixed case schema Oct 27, 2023
@fordhoka
Copy link

fordhoka commented Oct 31, 2023

I am also experiencing this issue. The suggested patch works for me, for mixed-case, lower-case, and case-insensitive schema names, but should also be applied to SnowflakeDialect.get_sequence_names().

I believe this patch would also fix #276 and #388.

fordhoka added a commit to pvcy/snowflake-sqlalchemy that referenced this issue Nov 27, 2023
fordhoka added a commit to pvcy/snowflake-sqlalchemy that referenced this issue Nov 30, 2023
@sfc-gh-dszmolka
Copy link

hi and thank you for submitting this issue and especially for sharing the patch ! i believe it could be caused by the same underlying cause which causes #388

can this be a possible duplicate ?

@sfc-gh-dszmolka sfc-gh-dszmolka self-assigned this Mar 13, 2024
@sfc-gh-dszmolka sfc-gh-dszmolka added duplicate This issue or pull request already exists status-triage_done Initial triage done, will be further handled by the driver team and removed needs triage labels Mar 13, 2024
@fordhoka
Copy link

This is a duplicate of both #388 and #276. We have been using the patch and it is working for us, with the same change applied to get_sequence_names():

    @reflection.cache
    def get_sequence_names(self, connection, schema=None, **kw):
        sql_command = "SHOW SEQUENCES {}".format(
            f" IN SCHEMA \"{self.denormalize_name(schema)}\"" if schema else "",
        )
        try:
            cursor = connection.execute(text(sql_command))

@sfc-gh-dszmolka
Copy link

thank you for confirming it @fordhoka (and also happy to hear you have a working workaround)
marking this as closed so the tracking could be focused on the existing tickets

@fordhoka
Copy link

Thanks @sfc-gh-dszmolka. When can we expect this patch to be included in a release?

@sfc-gh-dszmolka
Copy link

At this moment, I don't have any estimated timeline attached to this one unfortunately; but will keep the relevant open issues updated

@fredrike
Copy link
Author

I think the approach of closing issues without a patch is strange. It is much easier to track progress if issues are open until they are fixed.

What is the holdback in fixing this issue, I've already submitted a patch?

@sfc-gh-dszmolka
Copy link

sfc-gh-dszmolka commented Mar 22, 2024

only reason for closing this is because this is a duplicate but happy to reopen it.

is there a PR perhaps? if so, that would be more than appreciated and helpful - I can try to get the connector team to review it. if not, that's also not a problem and we'll get there.

there's quite a backlog for us to work through as you probably noticed, but we're now trying to dedicate more resources and love to this repo as well. so hoping that things will get better over time and thank you for bearing with us.

@sfc-gh-dszmolka sfc-gh-dszmolka removed their assignment Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working duplicate This issue or pull request already exists status-triage_done Initial triage done, will be further handled by the driver team
Projects
None yet
Development

No branches or pull requests

3 participants