> On Feb 21, 2025, at 9:07 AM, Mark Dilger <mark.dilger@enterprisedb.com> wrote:
>
> I infer that you intend to make v34-0004, v34-0006, and v35-0001 apply cleanly without the other patches and commit
itthat way. If that is correct, be advised that I'm doing a review and will respond back shortly, maybe in a few
hours.
Ok, here is my review:
v34-0001 looks fine
v34-0002 refactoring is needed by the gin patches, so I kept it in the patchset for review purposes
v34-0004 can mostly be applied without v34-0003, but a few changes are needed to make it apply cleanly.
v34-0006 looks fine
v35-0001 applies cleanly
I find the token quoting and capitalization patterns in sql/check_gin.sql somewhat confusing, but I tried to follow
whatis already there in extending that test to also check gin indexes over jsonb data using jsonb_path_ops. I think
thisis a common enough usage of gin that we should have test coverage for it.
After extending the test a bit, I ran the tests and checked lcov:
verify_common.c 86.3%
verify_gin.c 38.4%
verify_heapam.c 57.2%
verify_nbtree.c 72.4%
Showing that verify_gin has the least coverage of all. The main areas lacking coverage have to do with posting list
treesand concurrent page splits never being exercised. My first attempt cover that with a TAP test using pgbench got
thenumber up to 56.8%, but while trying to get that higher, I started getting error reports from verify_gin(),
apparentlyout of function gin_check_parent_keys_consistency():
# at t/006_gin_concurrency.pl line 137.
# 'pgbench: error: client 14 script 1 aborted in command 0 query 0: ERROR: index "ginidx" has wrong
tupleorder on entry tree page, block 153, offset 8
# pgbench: error: client 0 script 1 aborted in command 0 query 0: ERROR: index "ginidx" has wrong tuple order on entry
treepage, block 153, offset 8
# pgbench: error: client 12 script 1 aborted in command 0 query 0: ERROR: index "ginidx" has wrong tuple order on
entrytree page, block 153, offset 8
# pgbench: error: client 7 script 1 aborted in command 0 query 0: ERROR: index "ginidx" has wrong tuple order on entry
treepage, block 153, offset 8
# pgbench: error: client 1 script 1 aborted in command 0 query 0: ERROR: index "ginidx" has wrong tuple order on entry
treepage, block 153, offset 8
<MORE LINES LIKE THE ABOVE SNIPPED>
The pgbench script is not corrupting anything overtly, so this looks to either be a bug in gin or a bug in the check.
Iam including a patchset with the original patches reworked plus the extra test cases. For completeness, I also added
ginindexes to t/002_cic.pl and t/003_cic_2pc.pl.
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
