postgresql-dart icon indicating copy to clipboard operation
postgresql-dart copied to clipboard

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

Open insinfo opened this issue 1 year ago • 7 comments

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 avatar Mar 04 '24 13:03 insinfo

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

insinfo avatar Mar 04 '24 13:03 insinfo

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?

isoos avatar Mar 04 '24 15:03 isoos

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

insinfo avatar Mar 05 '24 01:03 insinfo

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.

isoos avatar Mar 05 '24 09:03 isoos

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

insinfo avatar Mar 07 '24 18:03 insinfo

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: https://github.com/isoos/postgresql-dart/pull/313

@insinfo: what do you think?

isoos avatar Mar 13 '24 22:03 isoos

@isoos looks good to me, congratulations on the work

insinfo avatar Mar 14 '24 18:03 insinfo