sqlglot icon indicating copy to clipboard operation
sqlglot copied to clipboard

Snowflake Alter Table ... Alter Column ... DROP NOT NULL doesn't parse correctly

Open barino86 opened this issue 1 year ago • 6 comments
trafficstars

Before you file an issue

  • Make sure you specify the "read" dialect eg. parse_one(sql, read="spark"): snowflake
  • Make sure you specify the "write" dialect eg. ast.sql(dialect="duckdb"): snowflake
  • Check if the issue still exists on main: it does

Fully reproducible code snippet Please include a fully reproducible code snippet or the input sql, dialect, and expected output.

from sqlglot import parse_one

query = """
    ALTER TABLE DB_NAME.SCHMA_NAME.TBL_NAME
    ALTER COLUMN MY_COLUMN DROP NOT NULL;
"""
parsed = parse_one(query, read='snowflake')
print(repr(parsed))


'ALTER TABLE DB_NAME.SCHMA_NAME.TBL_NAME
            ALTER COLUMN MY_COLUMN DROP NOT NULL' contains unsupported syntax. Falling back to parsing as a 'Command'.
Command(this=ALTER, expression=TABLE DB_NAME.SCHMA_NAME.TBL_NAME
               ALTER COLUMN MY_COLUMN DROP NOT NULL)

I would have expected an AlterTable expression with actions.

Official Documentation Please include links to official SQL documentation related to your issue.

https://docs.snowflake.com/en/sql-reference/sql/alter-table-column

barino86 avatar May 22 '24 22:05 barino86

I would love to take on this issue if possible. I try to modify the query to

query = """
    ALTER TABLE DB_NAME.SCHMA_NAME.TBL_NAME
    ALTER COLUMN MY_COLUMN DROP DEFAULT;
"""

AlterTable(
  this=Table(
    this=Identifier(this=TBL_NAME, quoted=False),
    db=Identifier(this=SCHMA_NAME, quoted=False),
    catalog=Identifier(this=DB_NAME, quoted=False)),
  actions=[
    AlterColumn(
      this=Identifier(this=MY_COLUMN, quoted=False),
      drop=True)])

and the expression seems to be parsed correctly, so I guess the issue is that the DROP NOT NULL statement is not understood by sqlglot properly.

noklam avatar May 24 '24 01:05 noklam

https://github.com/tobymao/sqlglot/blob/59de4f6e0f48f5354e7b462fb65633018184aa3c/sqlglot/parser.py#L6024-L6038

This is as far as I can trace so far, so it seems like this is a Parser problem. Since this is a Snowflake specific syntax, this should probably goes to snowflake.py and I should add a new function _parse_alter_table_alter to override the default parser. I would like to get some comment before I proceed.

noklam avatar May 24 '24 02:05 noklam

https://github.com/tobymao/sqlglot/blob/59de4f6e0f48f5354e7b462fb65633018184aa3c/sqlglot/parser.py#L6024-L6038

This is as far as I can trace so far, so it seems like this is a Parser problem. Since this is a Snowflake specific syntax, this should probably goes to snowflake.py and I should add a new function _parse_alter_table_alter to override the default parser. I would like to get some comment before I proceed.

Let's add this logic to the base parser since it doesn't clash with the existing logic.

georgesittas avatar May 24 '24 13:05 georgesittas

Hey @noklam, let us know if you're planning to look into this.

georgesittas avatar May 25 '24 16:05 georgesittas

I will take a stab over this weekend.

noklam avatar May 25 '24 17:05 noklam

WIP https://github.com/tobymao/sqlglot/pull/3550, I need some help and suggest move the discussion there.

noklam avatar May 25 '24 23:05 noklam