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

ALTER WAREHOUSE Snowflaken statements do no support variables or single quotes #4937

Closed
3 tasks done
Orangemic opened this issue Jun 29, 2023 · 3 comments · Fixed by #5264
Closed
3 tasks done

ALTER WAREHOUSE Snowflaken statements do no support variables or single quotes #4937

Orangemic opened this issue Jun 29, 2023 · 3 comments · Fixed by #5264
Labels
bug Something isn't working snowflake Issues related to the Snowflake dialect

Comments

@Orangemic
Copy link

Search before asking

  • I searched the issues and found no similar issues.

What Happened

Using the SQLParse Snowflake dialect, PRS errors are given on ALTER WAREHOUSE statements whereby either variables are used to describe the warehouse (e.g. IDENTIFIER($var)) or when using single quotes after a SET statement (e.g. SET WAREHOUSE_TYPE = 'STANDARD').

Expected Behaviour

Using IDENTIFIER($var) and single quotes after a SET statement should not give PRS issues.

Observed Behaviour

Running this code gives PRS errors in SQLFluff using the Snowflake dialect, whereby the code is completed without errors in Snowflake itself:

ALTER WAREHOUSE IDENTIFIER($var_wh) SET WAREHOUSE_TYPE = STANDARD;

ALTER WAREHOUSE WH_TEST SET SCALING_POLICY = 'ECONOMY';

Running the code without the 'IDENTIFIER($var_wh)' or single quotes around ECONOMY, gives no PRS issues:

ALTER WAREHOUSE WH_TEST SET WAREHOUSE_TYPE = STANDARD;

ALTER WAREHOUSE WH_TEST SET SCALING_POLICY = ECONOMY;

How to reproduce

See here a link to where the code breaks.

In this example the code would run without PRS errors.

Dialect

I use the Snowflake dialect

Version

sqlfluff version 2.1.1
Python version 3.10.12

Configuration

Honestly, our config is still pretty empty at this point:

[sqlfluff]
dialect = snowflake

Are you willing to work on and submit a PR to address the issue?

  • Yes I am willing to submit a PR!

Code of Conduct

@Orangemic Orangemic added the bug Something isn't working label Jun 29, 2023
@Orangemic
Copy link
Author

I'm seeing that the ALTER WAREHOUSE statement uses Ref("NakedIdentifierSegment", optional=True) in the code.

match_grammar = Sequence(
        "ALTER",
        "WAREHOUSE",
        Sequence("IF", "EXISTS", optional=True),
        OneOf(
            Sequence(
                Ref("NakedIdentifierSegment", optional=True),
                OneOf(
                    "SUSPEND",
                    Sequence(
                        "RESUME",
                        Sequence("IF", "SUSPENDED", optional=True),
                    ),
                ),
            ),

However, I'm seeing that other ALTER STATEMENTS (e.g. alter storage integration) use Ref("ObjectReferenceSegment") instead. Like the example here:

match_grammar = Sequence(
        "ALTER",
        Ref.keyword("STORAGE", optional=True),
        "INTEGRATION",
        Ref("IfExistsGrammar", optional=True),
        Ref("ObjectReferenceSegment"),
        OneOf(
            Sequence(
                "SET",
                OneOf(
                    Ref("TagEqualsSegment", optional=True),
                    AnySetOf(
                        Sequence(
                            "COMMENT", Ref("EqualsSegment"), Ref("QuotedLiteralSegment")
                        ),

Could it be the case that we need to use ObjectReferenceSegment for ALTER WAREHOUSE statements instead of NakedIdentifierSegment?

@greg-finley greg-finley added the snowflake Issues related to the Snowflake dialect label Jul 6, 2023
@moreaupascal56
Copy link
Contributor

moreaupascal56 commented Jul 19, 2023

Hi @Orangemic,

For the ALTER WAREHOUSE WH_TEST SET SCALING_POLICY = 'ECONOMY';, the issue is on the WarehouseObjectPropertiesSegment where you see this:

        Sequence(
            "SCALING_POLICY",
            Ref("EqualsSegment"),
            OneOf(
                "STANDARD",
                "ECONOMY",
            ),
        ),

There is no possibility for quotes. I think you can add the possibility quite easily, take inspiration from other places in the code :)

PS: All the clauses after the SET are done in this part of the ALTER WAREHOUSE fonction

@moreaupascal56
Copy link
Contributor

For the ALTER WAREHOUSE IDENTIFIER($var_wh) SET WAREHOUSE_TYPE = STANDARD;, I think your intuition is great, the issue is that indeed in all the possibilities given in the AlterWarehouseStatementSegment are always using the NakedIdentifierSegment for the warehouse "name".

You can find NakedIdentifierSegment code here (and below):

    NakedIdentifierSegment=SegmentGenerator(
        # Generate the anti template from the set of reserved keywords
        lambda dialect: RegexParser(
            # See https://docs.snowflake.com/en/sql-reference/identifiers-syntax.html
            r"[a-zA-Z_][a-zA-Z0-9_$]*",
            ansi.IdentifierSegment,
            type="naked_identifier",
            anti_template=r"^(" + r"|".join(dialect.sets("reserved_keywords")) + r")$",
        )
    ),

So there is no mention of any IDENTIFIER thing, I think ObjectReferenceSegment (this is defined in dialect_ansi.py because it is not overwritten in dialect_snowflake.py) will work, the key being the use of SingleIdentifierGrammar in ObjectReferenceSegment !

Have a try at it if you like and I can help if needed :)
Have a great day !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working snowflake Issues related to the Snowflake dialect
Projects
None yet
3 participants