-
Notifications
You must be signed in to change notification settings - Fork 30
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
Pass Dialect
to Column.DefaultValue
#536
Conversation
@@ -25,7 +25,7 @@ func BackfillColumn(cfg config.Config, dwh destination.DataWarehouse, column col | |||
} | |||
|
|||
additionalDateFmts := cfg.SharedTransferConfig.TypingSettings.AdditionalDateFormats | |||
defaultVal, err := column.DefaultValue(&columns.DefaultValueArgs{Escape: true, DestKind: dwh.Label()}, additionalDateFmts) | |||
defaultVal, err := column.DefaultValue(dwh.Dialect(), additionalDateFmts) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only place we call DefaultValue
so I removed the Escape
argument.
return fmt.Sprintf("JSON_PARSE(%s)", stringutil.Wrap(c.defaultValue, false)), nil | ||
case constants.Snowflake: | ||
case sql.SnowflakeDialect: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Tang8330 it doesn't seem like we're doing anything for MS SQL here, do we need a case for it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah we're not. We don't support backfill default values in MSSQL yet.
Do you want to leave a comment?
Dialog
to Column.DefaultValue
Dialect
to Column.DefaultValue
No description provided.