Skip to content
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

add connect event (callback?) for the pool that you can use to run custom commands on a connection prior to it being made available to app code #312

Open
insinfo opened this issue Mar 4, 2024 · 7 comments

Comments

@insinfo
Copy link

insinfo commented Mar 4, 2024

add a way to run queries before making the connection available, so that it is possible to run queries like set search_path to "schemaName" and SET client_encoding = 'utf8'
I'm having problems using the Pool without this feature

@insinfo
Copy link
Author

insinfo commented Mar 4, 2024

in node-pg-pool there is something like this

pool.on('connect', (client) => {client.query('set search_path...');});

https://github.com/brianc/node-pg-pool

@isoos
Copy link
Owner

isoos commented Mar 4, 2024

Is it fair to say that these SET parameters may go into ConnectionSettings and should be applied when we are creating the connection, regardless of pool or non-pool? Some may also go into SessionSettings?

I don't mind a callback, but the examples provided feel more like settings. What do you think?

@insinfo
Copy link
Author

insinfo commented Mar 5, 2024

I think putting these options in SessionSettings would be more difficult to implement because postgresql allows a considerable amount of configuration options at runtime, having the ability to run queries with a callback would perhaps be easier to implement and would be more flexible, but perhaps Could find a middle ground between putting the most used options like 'search_path' , 'datestyle', 'TIME ZONE', 'Client Encoding' in addition to having a way to pass a query to be executed, that would be very good.

From what I saw in the npgsql driver you can pass these settings through the connection string

"ConnectionStringPostgreSqlProvider": "User ID=xxx;Password=xxx;Host=localhost;Port=5432;Database=myDb;SearchPath=idsvr4config,public;Pooling=true;",

https://www.npgsql.org/doc/connection-string-parameters.html
https://www.postgresql.org/docs/current/runtime-config.html
https://www.postgresql.org/docs/current/sql-set.html

@isoos
Copy link
Owner

isoos commented Mar 5, 2024

I think the place of setting should reflect the expectation of the scope. I think we could make it work with run/runTx or even with individual execute calls, but it would need to run SET+RESET operations on each boundaries. It would add a non-trivial overhead, and scope nesting wouldn't work, because RESET <x> and RESET ALL won't restore the session-beginning value, rather it will restore the database-configured default(s).

I think the good balance here is to set these settings on the connection initialization (being in ConnectionSettings), and let the user override them without library support (assuming they know why and what they are doing).

This could be a simple Map<String, String> or a typed object, not yet sure which one is more preferable.

@insinfo
Copy link
Author

insinfo commented Mar 7, 2024

I think the good balance here is to set these settings on the connection initialization (being in ConnectionSettings), and let the user override them without library support (assuming they know why and what they are doing).

I completely agree, I think the Map<String, String> option is good

@isoos
Copy link
Owner

isoos commented Mar 13, 2024

After a bit back-and-forth, I'm backing out of my suggestion of Map-based settings, as it may become way too difficult to generalize it for e.g. cockroachdb's cluster-level variables. I also wanted to make it nice enough for named/typed variables, but that also complicates it further.

Instead, I'm suggesting to go ahead with a rather simple onOpen callback as implemented here:
#313

@insinfo: what do you think?

@insinfo
Copy link
Author

insinfo commented Mar 14, 2024

@isoos
looks good to me, congratulations on the work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants