Support options connection parameter

[rafiss]

This supports the connection parameter documented here:
https://www.postgresql.org/docs/9.1/libpq-connect.html#LIBPQ-CONNECT-OPTIONS

fixes #2214

[brianc]

wow that was fast! Do you know of a param I could send to the backend & then query the backend to verify it's taken effect? I'd like to add a test that does something like...

const client = new Client({ options: '--foo=bar' })
await client.connect()
const { rows } = await client.query('SELECT session_options')
assert(rows[0].key === 'foo')
assert(rows[0].value === 'bar')

or something like that. The specifics aren't really ironed out there I just always like to test "round tripping" the data when I can. I'll write this test if you know of how it could work?

[sehrope]

@brianc How about default_transaction_isolation? I have a feeling that one won't be going away:

$ psql \
    -c 'SHOW default_transaction_isolation'
 default_transaction_isolation 
-------------------------------
 read committed
(1 row)

$ PGOPTIONS='--default_transaction_isolation=serializable' psql \
    -c 'SHOW default_transaction_isolation'
 default_transaction_isolation 
-------------------------------
 serializable
(1 row)

[boromisp]

With recent postgres versions you can use custom namespaced variables:

PGOPTIONS="-c example.foo=bar" psql -c "SHOW example.foo"
 example.foo
-------------
 bar
(1 row)

psql -c "SHOW example.foo"
ERROR:  unrecognized configuration parameter "example.foo"

[rafiss]

Doing a round-trip test definitely sounds good to me. And thanks for taking on the task of writing it!

I'd agree with using a custom namespaced variable like how @boromisp posted. I think the -c name=value syntax is needed to set it.

[boromisp]

I think you will have to parse these parameters and include them in the startup message for the JavaScript client.

[rafiss]

I think you will have to parse these parameters and include them in the startup message for the JavaScript client.

Ah thanks, I'd missed that earlier. Just updated.

[boromisp]

The #2103 issue could be fixed by setting the statement_timeout for the native driver in the options parameter:

var escapeOptionValue = function (value) {
  return ('' + value).replace(/\\/g, '\\\\').replace(/ /g, '\\ ')
}

var addOption = function (options, config, optionName) {
  var value = config[optionName]
  if (value !== undefined && value !== null) {
    options.push('-c ' + optionName + '=' + escapeOptionValue(value))
  }
}

ConnectionParameters.prototype.getLibpqConnectionString = function (cb) {
  // ...
  var options = []
  if (this.options) {
    options.push(this.options)
  }
  addOption(options, this, 'statement_timeout')
  if (options.length > 0) {
    params.push('options=' + quoteParamValue(options.join(' ')))
  }
  // ...
}

An other (undocumented) config parameter that doesn't work with the native driver is idle_in_transaction_session_timeout. This could also be fixed the same way.

(The unsupported startup parameter: statement_timeout error in #2103 is probably related to this: https://www.pgbouncer.org/config.html#ignore_startup_parameters)

[brianc]

Hey sorry for the delay! I was out on vacation & just getting back...I'll take a look at this tomorrow! ❤️

[brianc]

@rafiss I have included a patch file for an integration test

https://cloudup.com/cmGDTMNSW9f

If you apply that patch to your branch I can merge this & release a new minor version w/ the support added. Thanks for your work here & sorry for the delay!

[brianc]

or actually i can just merge this & then add the test myself. :)

[brianc]

Thanks for the PR! I'll release this tomorrow morning so I can make sure it doesn't cause weird regressions in anyone's systems. (it shouldn't...but you never know)

[rafiss]

Thank you @brianc! Let me know if anything comes up.

[brianc]

Released as [email protected]