Pass-through postgres connection option 'statement_timeout'

[amasad]

node-postgres recently added support for 'statement_timeout' -- a per connection query timeout.

See brianc/node-postgres#1436

[mention-bot]

@amasad, thanks for your PR! By analyzing the history of the files in this pull request, we identified @felixfbecker, @janmeier and @mickhansen to be potential reviewers.

[codecov]

Codecov Report

Merging #8342 into master will not change coverage.
The diff coverage is n/a.

[sushantdhiman]

There is a pending PR which handles this already #8254. PR tries to test statement_timeout but fails because Sequelize doesn't fully support pg@v7.

I think we should add this flag without testing statement_timeout, let pg worry about it. This will allow those on pg@v7 to use this feature.

cc @sequelize/core / @sequelize/trusted-contributors

[amasad]

Yes, I just realized that v7 isn't supported after trying to run it and hit the #7998 bug

[janmeier]

@sushantdhiman Yep, agreed

[mkaufmaner]

Yes, I just realized that v7 isn't supported after trying to run it and hit the #7998 bug

That being said, wouldnt we want to hold off on this PR? Also, not a huge fan of adding dialect specific options to abstract components.

[sushantdhiman]

That being said, wouldnt we want to hold off on this PR?

Dialect specific option so there should be no problem. Sequelize is incompatible with MULTIPLE statements type, this option not related

Also, not a huge fan of adding dialect specific options to abstract components.

Not sure I understood what you meant by abstract component, this is Postgres specific connection manager. This is a good place to set such options, we already have many pg specific options here like keepAlive etc and I think only way to pass these options is here.

We are not actually adding it, we are white listing it so users can pass it as dialectOptions

[jkennedy1980]

Model.upsert() fails with pg@7.3.0 due to the multiple result problem. Do we have a time frame for a fix on the multiple result issue?

[sushantdhiman]

@jkennedy1980 No we dont have a timeframe. Its an open source project, when someone is free enough to work on it, fix will land. Till then just wait :)

[jkennedy1980]

And you guys are sure you don't want to accept PR8029: #8029 - It basically converts the multiple results to the same thing you get with pg@6? If not, can you close PR8029 so people don't think it's being worked on?

[sushantdhiman]

That PR attempts to solve it but its not a good solution.

I haven't had chance to review that PR but as referred in #7998 we need a way to have MULTIPLE type statement define which result they need.

If someone is willing to solve that issue I can explain in details what we need

[jkennedy1980]

Ok. It seems that you have ideas about how you want the API to change to support multiple results. There are many issues/PRs open where people are proposing different solutions. I think it's time that you document what you're thinking about the API so that the community can discuss it and maybe even implement it. This would prevent people wasting their time on undesired solutions. I may even choose to implement if there is a good API design in place.