F21 commented on a change in pull request #54:
URL: https://github.com/apache/calcite-avatica-go/pull/54#discussion_r470292931
##########
File path: dsn.go
##########
@@ -207,7 +207,8 @@ func ParseDSN(dsn string) (*Config, error) {
}
if parsed.Path != "" {
- conf.schema = strings.TrimPrefix(parsed.Path, "/")
+ s := strings.Split(parsed.Path, "/")
+ conf.schema = s[len(s)-1]
Review comment:
@joshelser Good catch! Yes, that is pretty subtle and could be difficult
to debug. I think there should not be a trailing slash after the schema (if
provided) as this is the convention used by a few drivers I checked. This is
the case for the MySQL driver you've linked as well as the postgres driver:
https://pkg.go.dev/github.com/lib/pq?tab=doc
In other words, `https://some.host/proxied/avatica` means the default schema
is `avatica` and `https://some.host/proxied/avatica/` means no default schema.
As you mentioned, this is definitely a subtle difference, but at least we are
following convention, so there's consistency across drivers.
I am not opposed to moving the default schema to a query string, but it
seems to be a break from the standard convention and I don't think it's really
necessary. In addition, that would be a breaking change as well, so if we do go
down this path, we'd have to consider if we're breaking anything else.
I think the `FormatDSN()` method is a good idea. We can change the
properties in the
[`Config`](https://godoc.org/github.com/apache/calcite-avatica-go#Config)
struct to public (breaking change, but additive) and add a `FormatDSN` method
to turn the config back to the DSN.
The above probably sounds a bit rambly, but I personally prefer the
following approach:
- `https://some.host/proxied/avatica` means the schema is `avatica` and
`https://some.host/proxied/avatica/` means no schema.
- Let's make the properties in `Config` public and add a `FormatDSN()`
method to it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
