Inconsistent interface of #paged_each / skip_transaction: true not working on postgres for #paged_each

[hwo411]

Complete Description of Issue

Hello, I recently ran into a problem that skip_transcation: true is not honored by #paged_each in postgres.

Transaction is executed regardless of presence of that flag.

According to https://rubydoc.info/github/jeremyevans/sequel/master/Sequel%2FDataset:paged_each passing skip_transaction is possible, but it doesn't describe that some adapters can ignore it.

It quicky became clear to me that postgres options are different and can be found there: https://rubydoc.info/github/jeremyevans/sequel/master/Sequel/Postgres/Dataset#paged_each-instance_method

However, this type of inheritance violates Liskov substitution principle, because you can't just replace original dataset with postgres dataset without breaking the logic.

It seems that the problem affects other parameters too.

I'd expect the following behavior:

If at least one of the not honored parameters (strategy, filter_values or skip_transaction) from original implementation is passed, it falls back to original implementation (or warns/raises). In addition to that I think it could be fine if skip_transaction: true without strategy works as hold: true.

WDYT?

Simplest Possible Self-Contained Example Showing the Bug

YourModel.order(:id).limit(100).paged_each(rows_per_fetch: 10, skip_transaction: true) {}

when connected to postgres database

Full Backtrace of Exception (if any)

No response

SQL Log (if any)

I, [2023-11-16T08:35:19.968351 #230] INFO -- : (0.002123s) BEGIN
I, [2023-11-16T08:35:19.973816 #230] INFO -- : (0.004602s) DECLARE "sequel_cursor" NO SCROLL CURSOR WITHOUT HOLD FOR SELECT * FROM "your_models" ORDER BY "id" LIMIT 100
I, [2023-11-16T08:35:19.977175 #230] INFO -- : (0.003132s) FETCH FORWARD 10 FROM "sequel_cursor"
I, [2023-11-16T08:35:19.982371 #230] INFO -- : (0.001761s) FETCH FORWARD 10 FROM "sequel_cursor"
I, [2023-11-16T08:35:19.987546 #230] INFO -- : (0.001863s) FETCH FORWARD 10 FROM "sequel_cursor"
I, [2023-11-16T08:35:19.992386 #230] INFO -- : (0.001850s) FETCH FORWARD 10 FROM "sequel_cursor"
I, [2023-11-16T08:35:19.997125 #230] INFO -- : (0.002012s) FETCH FORWARD 10 FROM "sequel_cursor"
I, [2023-11-16T08:35:20.011978 #230] INFO -- : (0.008135s) FETCH FORWARD 10 FROM "sequel_cursor"
I, [2023-11-16T08:35:20.023002 #230] INFO -- : (0.002465s) FETCH FORWARD 10 FROM "sequel_cursor"
I, [2023-11-16T08:35:20.028574 #230] INFO -- : (0.001911s) FETCH FORWARD 10 FROM "sequel_cursor"
I, [2023-11-16T08:35:20.036191 #230] INFO -- : (0.002397s) FETCH FORWARD 10 FROM "sequel_cursor"
I, [2023-11-16T08:35:20.041868 #230] INFO -- : (0.002629s) FETCH FORWARD 10 FROM "sequel_cursor"
I, [2023-11-16T08:35:20.049211 #230] INFO -- : (0.001587s) FETCH FORWARD 10 FROM "sequel_cursor"
I, [2023-11-16T08:35:20.051138 #230] INFO -- : (0.001698s) CLOSE "sequel_cursor"
I, [2023-11-16T08:35:20.053239 #230] INFO -- : (0.001901s) COMMIT

Ruby Version

3.1.4

Sequel Version

5.74.0

[jeremyevans]

Thank you very much for the detailed report. I agree with you that this is a bug, and will work on a fix shortly.

[jeremyevans]

I'm going to change things such that :skip_transaction is respected and automatically turns on :hold. The :strategy and :filter_values options are ignored by design, as the cursor approach is better than any of the supported generic strategies, and :filter_values is only needed for the filter strategy.