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

Remove Teradata CREATE args from Snowflake create_sql #1211

Closed
wants to merge 2 commits into from
Closed

Remove Teradata CREATE args from Snowflake create_sql #1211

wants to merge 2 commits into from

Conversation

treysp
Copy link
Collaborator

@treysp treysp commented Feb 21, 2023

@treysp
Copy link
Collaborator Author

treysp commented Feb 21, 2023

I've been meaning to pull most of those args into a new "properties" location so we can use the properties machinery and dialect-specific unsupported approach

@@ -327,3 +327,10 @@ def generatedasidentitycolumnconstraint_sql(
increment = expression.args.get("increment")
increment = f" INCREMENT {increment}" if increment else ""
return f"AUTOINCREMENT{start}{increment}"

def create_sql(self, expression: exp.Create) -> str:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delete this and make a new propety location unsupported, then snowflake just needs to override the properties_location

def create_sql(self, expression: exp.Create) -> str:
unsupported = ["set", "multiset", "global_temporary", "volatile"]
for prop in unsupported:
expression.set(prop, False)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't mutate any expression in the generator. Whenever something like this is needed, you first need to copy it.

Comment on lines +335 to +336
create_sql = super().create_sql(expression)
return create_sql
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
create_sql = super().create_sql(expression)
return create_sql
return super().create_sql(expression)

@treysp treysp closed this Feb 21, 2023
@treysp treysp deleted the td_snow_create branch February 24, 2023 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants