Re: [PATCH] Add regression tests for btree skip scan support functions

2026-06-05 Thread Baji Shaik
On Thu, Jun 4, 2026 at 10:32 PM Michael Paquier  wrote:

> Could you add this patch to the next commit fest please at [1]?
> That's to make sure that we don't forget about it when v20 opens for
> business.  I am sure that nobody would complain it this gets applied
> on HEAD today (argument that this is an open item for v19 due to
> v19?), but that's a coverage improvement so let's just do that once
> REL_19_STABLE is forked.
>
> Perhaps we could do something for the overflow cases in the increment
> functions of oid, oid8, char, int8 and int4.  int2_increment is
> lacking entirely.  For the decrement functions, we still lack
> something for the overflow of int2, int8, char.  You can run a
> coverage report to check all that:
> https://www.postgresql.org/docs/devel/regress-coverage.htm
> l


Thanks for the review.  Added to commitfest 59 today [1] and posting v2
which addresses the coverage gaps.

v2 adds:

  - basic int2_increment coverage (was lacking entirely as you noted)
  - boundary tables for int2/int4/int8/oid/oid8/"char" around MIN/MAX,
with forward and backward Index Only Scans, exercising the
*_increment overflow / *_decrement underflow comparisons

Coverage on src/backend/access/nbtree/nbtcompare.c (--enable-coverage
build, regression suite):

  master:  58.75 % lines, 63.10 % branches taken at least once
  v1:  89.06 % lines, 73.81 % branches taken at least once
  v2:  90.94 % lines, 82.14 % branches taken at least once

A couple of *_increment overflow code blocks (the body of the
"if (val == TYPE_MAX)" check) still read as unreached. The scan
walks the rows at the boundary value and exits without probing past
the end, so the increment function gets called with intermediate
values rather than the boundary value itself.  The boundary
comparison is exercised either way, which is what the +8 pp branch
delta over v1 reflects.  If you or anyone else has ideas for forcing
the few remaining branches I'm happy to extend.

All 245 regression tests pass; all 129 isolation tests pass.

Patch attached.

[1]: https://commitfest.postgresql.org/patch/6852/

Thanks,
Baji Shaik


v2-0001-Add-regression-tests-for-btree-skip-scan-support-fun.patch
Description: Binary data


Re: [PATCH] Add regression tests for btree skip scan support functions

2026-06-04 Thread Michael Paquier
On Wed, Jun 03, 2026 at 07:16:15PM -0500, Baji Shaik wrote:
> These functions feed into the skip-scan optimization added in
> commit 92fe23d93aa for PG18, but the existing regression coverage
> only exercises a few types (mostly int4 and varchar) on the leading
> column of multi-column indexes.

Thanks a lot for this patch.  FWIW, I've noticed the gap a couple of
months ago when adding oid8 but I have not been able to get back to
it, noticing that the gap was wider than only oid8.

> The attached patch adds a small section to btree_index.sql that
> creates a two-column index (a, b) for each missing type, then runs
> forward and backward Index Only Scans with a predicate on the
> non-leading column (b).  This forces the planner into a skip scan,
> which in turn calls the per-type skip-support function plus the
> increment/decrement helpers as it generates skip-array elements.

Could you add this patch to the next commit fest please at [1]?
That's to make sure that we don't forget about it when v20 opens for
business.  I am sure that nobody would complain it this gets applied
on HEAD today (argument that this is an open item for v19 due to
v19?), but that's a coverage improvement so let's just do that once
REL_19_STABLE is forked.

Perhaps we could do something for the overflow cases in the increment
functions of oid, oid8, char, int8 and int4.  int2_increment is
lacking entirely.  For the decrement functions, we still lack
something for the overflow of int2, int8, char.  You can run a
coverage report to check all that:
https://www.postgresql.org/docs/devel/regress-coverage.html

[1]: https://commitfest.postgresql.org/59/
--
Michael


signature.asc
Description: PGP signature