-
Notifications
You must be signed in to change notification settings - Fork 6
adding a database field INFRA-606 #45
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
Conversation
pkg/plugin/duckdb_driver.go
Outdated
| return &ConfigError{"Path is empty"} | ||
| } | ||
| // if db has no quotes add them | ||
| if !((strings.HasPrefix(db, "'") && strings.HasSuffix(db, "'")) || (strings.HasPrefix(db, "\"") && strings.HasSuffix(db, "\""))) { |
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.
i think this condition should be removed.
The db path should always be escaped for single quotes. right now a user input like '/some/pa'th.ddb' will not escape the inner ' and yield an invalid attach query.
But i don't even think the outer quotes should be treated specially, as in i don't think the user should try to quote their paths at all.
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.
motherduck database can not, I will keep quoting if a prefix with md: has no quotes.
logger=plugin.motherduck-duckdb-datasource t=2025-09-18T16:11:23.520295756Z level=info msg="ATTACH IF NOT EXISTS md:sample_data;"
logger=plugin.motherduck-duckdb-datasource t=2025-09-18T16:11:23.521975964Z level=error msg="error querying the database: Parser Error: syntax error at or near "md""
pkg/plugin/duckdb_driver.go
Outdated
| if strings.TrimSpace(config.Path) != "" { | ||
| db := strings.TrimSpace(config.Path) | ||
| // if db has no quotes add them | ||
| if !strings.HasPrefix(db, "'") && !strings.HasSuffix(db, "'") && strings.HasPrefix(db, "md:") { | ||
| db = "'" + db + "'" | ||
| } | ||
| bootQueries = append(bootQueries, "ATTACH IF NOT EXISTS "+db+";") | ||
| backend.Logger.Info("ATTACH IF NOT EXISTS " + db + ";") | ||
| } |
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 should only be run if the db path is md:...
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.
when the db path is 'md:sample_data',
then duckdb.NewConnector(path, func(execer driver.ExecerContext) error { will receive path = "'md:sample_data'";
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.
let's keep strings.ReplaceAll(db, "'", "''")
Uh oh!
There was an error while loading. Please reload this page.