-
Notifications
You must be signed in to change notification settings - Fork 623
database: Use database.ParseDialect to accept dialect aliases and return canonical dialect (closes #917) #1021
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
base: main
Are you sure you want to change the base?
Changes from 6 commits
09ac246
cf4bd31
3632c26
285412d
f489b60
84645f8
61e0b81
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,8 @@ import ( | |
| // Dialect is the type of database dialect. | ||
| type Dialect string | ||
|
|
||
| var ErrUnknownDialect = errors.New("unknown dialect") | ||
|
|
||
| const ( | ||
| DialectCustom Dialect = "" | ||
| DialectClickHouse Dialect = "clickhouse" | ||
|
|
@@ -32,7 +34,47 @@ const ( | |
| DialectVertica Dialect = "vertica" | ||
| ) | ||
|
|
||
| // NewStore returns a new [Store] implementation for the given dialect. | ||
| // ParseDialect returns the corresponding [Dialect], if any, for a given string with external | ||
| // origin. A supported [Dialect] value, like [DialectPostgres], may have multiple supported | ||
| // aliases, e.g. "postgres" or "pgx". Use this function to ensure that a [Dialect] instance | ||
| // passed to any function that accepts a [Dialect] is a valid [Dialect]. | ||
| func ParseDialect(s string) (d Dialect, err error) { | ||
| // Every non-error return value from this function must have an entry in the [NewStore] lookup map. | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To really prevent drift, instead of having a comment here, what do you think of having a registry? I drafted up a PR in globalchubby#1. |
||
| switch s { | ||
| case "postgres", "pgx": | ||
| d = DialectPostgres | ||
| case "mysql": | ||
| d = DialectMySQL | ||
| case "sqlite3", "sqlite": | ||
| d = DialectSQLite3 | ||
| case "spanner": | ||
| d = DialectSpanner | ||
| case "mssql", "azuresql", "sqlserver": | ||
| d = DialectMSSQL | ||
| case "redshift": | ||
| d = DialectRedshift | ||
| case "tidb": | ||
| d = DialectTiDB | ||
| case "clickhouse": | ||
| d = DialectClickHouse | ||
| case "vertica": | ||
| d = DialectVertica | ||
| case "ydb": | ||
| d = DialectYdB | ||
| case "turso": | ||
| d = DialectTurso | ||
| case "starrocks": | ||
| d = DialectStarrocks | ||
| case "dsql": | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This case is new. The lookup map in |
||
| d = DialectAuroraDSQL | ||
| default: | ||
| err = ErrUnknownDialect | ||
| } | ||
| return | ||
| } | ||
|
|
||
| // NewStore returns a new [Store] implementation for the given dialect. The | ||
| // dialect must be a valid [Dialect] value; see [ParseDialect]. | ||
| func NewStore(d Dialect, tableName string) (Store, error) { | ||
| if d == DialectCustom { | ||
| return nil, errors.New("custom dialect is not supported") | ||
|
|
@@ -54,7 +96,7 @@ func NewStore(d Dialect, tableName string) (Store, error) { | |
| } | ||
| querier, ok := lookup[d] | ||
| if !ok { | ||
| return nil, fmt.Errorf("unknown dialect: %q", d) | ||
| return nil, ErrUnknownDialect | ||
| } | ||
| return NewStoreFromQuerier(tableName, querier) | ||
| } | ||
|
|
||
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.
Since we're returning the same error in two places, I thought it better to have a sentinel error. I debated having an error type instead, but the only data I could think of it holding is the unknown dialect, but the caller already has this.