> On Feb 21, 2025, at 9:07 AM, Mark Dilger <mark.dil...@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 it that 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 what is already there in extending
that test to also check gin indexes over jsonb data using jsonb_path_ops. I
think this is 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 trees and concurrent page splits never
being exercised. My first attempt cover that with a TAP test using pgbench got
the number up to 56.8%, but while trying to get that higher, I started getting
error reports from verify_gin(), apparently out 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 tuple order 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 tree page, block 153, offset 8
# pgbench: error: client 12 script 1 aborted in command 0 query 0: ERROR:
index "ginidx" has wrong tuple order on entry tree 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 tree page, 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 tree page, 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. I am including a patchset with the
original patches reworked plus the extra test cases. For completeness, I also
added gin indexes to t/002_cic.pl and t/003_cic_2pc.pl.
v36-0001-A-tiny-nitpicky-tweak-to-beautify-the-Amcheck-in.patch
Description: Binary data
v36-0002-Refactor-amcheck-internals-to-isolate-common-loc.patch
Description: Binary data
v36-0003-Add-gin_index_check-to-verify-GIN-index.patch
Description: Binary data
v36-0004-Fix-wording-in-GIN-README.patch
Description: Binary data
v36-0005-Fix-for-gin_index_check.patch
Description: Binary data
v36-0006-Add-gin-index-checking-test-for-jsonb-data.patch
Description: Binary data
v36-0007-Add-gin-to-the-create-index-concurrently-tap-tes.patch
Description: Binary data
v36-0008-Stress-test-verify_gin-using-pgbench.patch
Description: Binary data
— Mark Dilger EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
