Suggestions : defining Headers constants as public and add split Config.PrestoURI into user & url

[Chaho12]

Mentioned in slack

1. defining Headers constants as public?

I was wondering why header constant is not public. People would need to use such headers but they would have to define their constants so that it matches presto-go-client const (which is bothersome)

• https://github.com/prestosql/presto-go-client/blob/master/presto/presto.go#L98-L113

presto-cli defines as public variable.

• https://github.com/prestosql/presto/blob/master/presto-client/src/main/java/io/prestosql/client/PrestoHeaders.java

2. Split PrestoURI with user info in Config to a DSN string

It is sort of confusing because PrestoURI is made up of default user and server addr
I think it would be better if we can split those two because URL consists of URL + user.

https://github.com/prestosql/presto-go-client/blob/master/presto/presto.go#L124

[Chaho12]

after reviewing golang url code, i take back second suggestion as URL schema has UserInfo.
It suggests general form as [scheme:][//[userinfo@]host][/]path[?query][#fragment], so separating default user with server addr would be unnecessary and would only make code dirtier

[Chaho12]

@losipiuk
Hey, am back to work with Presto :)

While I was working with query callback, I was wondering WDUT about this idea of changing header constants to public?
prestoUserHeader -> UserHeader (if it is changed, the word Presto name starts with package name, which is unnecessary in Goalng ref)

Other headers that needs to be fixed are as following:

	prestoSourceHeader      = "X-Presto-Source"
	prestoCatalogHeader     = "X-Presto-Catalog"
	prestoSchemaHeader      = "X-Presto-Schema"
	prestoSessionHeader     = "X-Presto-Session"

[findepi]

Ideally all the headers-related logic should be handled within the client library.
Exposing headers may be view as an escape valve, but I would want to make sure
this doesn't discourage us from providing proper APIs for everything that's needed.
How can we achieve that?

[Chaho12]

Am not that experienced engineer so it may say seem to be silly questions, btw is defining constants considered part of headers-related logic ? As user of the API, I would sometimes want to change user name, but then I would have to type or make a const for X-Presto-User which seems silly cause it is already used within presto go client.

rows, err := db.Query("SELECT current_user", sql.Named(PrestoUserHeader, string("TestUser")))

[losipiuk]

I think this is fine to pass extra configuration parameters via sql.Named(CONFIG_KEY, CONFIG_VALUE).
But I think that the set of valid values for the CONFIG_KEY values should be defined explicitly and not necessarily map 1:1 to internal set of http headers used by Presto communication protocol.

[Chaho12]

@losipiuk @nineinchnick Hi, I tagged you guys because I wanted to ask final question before closing this issue, opening headers as public constants like other client does, for convenience sake.

• python : https://github.com/trinodb/trino-python-client/blob/master/trino/constants.py
• node : https://github.com/tagomoris/presto-client-node/blob/master/lib/presto-client/headers.js

[nineinchnick]

I think the JDBC is the reference implementation. Python is not a good comparison, since you can't hide anything in Python anyway.

I don't mind keeping this issue open so that we could wait for others to report actual use cases where they need to pass headers. I had one use case recently to set X-Trino-Added-Prepare but I managed to add support for prepared statements in #44 so it's not an issue anymore.

[findepi]

I had one use case recently to set X-Trino-Added-Prepare but I managed to add support for prepared statements in #44 so it's not an issue anymore.

Awesome use-case!
But also example why we don't shouldn't tend to expose raw headers. This encourages people to hack around their use-cases, which is awesome for super advanced users, but also discourages people from actually improving the code.