Re: BUG #16619: Amcheck detects corruption in hstore' btree index (ver 2)

On 16.09.2020 13:34, PG Bug reporting form wrote:
> The following bug has been logged on the website:
>
> Bug reference:      16619
> Logged by:          Andrew Bille
> Email address:      andrewbille@gmail.com
> PostgreSQL version: 13beta3
> Operating system:   Ubuntu-20.04
> Description:
>
> Hello all.
> Index corruption detected after upgrade from v 9.6/10:
>
> mkdir v10
> cd v10
> git clone https://github.com/postgres/postgres.git ./
> git checkout REL_10_STABLE
>   ./configure >/dev/null && make -j8 >/dev/null && make -j8 contrib
>> /dev/null
> echo "SELECT 1;" >> contrib/hstore/sql/hstore.sql
> make check -C contrib/hstore
> cd ..
> mkdir v13
> cd v13
> git clone https://github.com/postgres/postgres.git ./
> git checkout REL_13_STABLE
> ./configure >/dev/null && make -j8 >/dev/null && make -j8 contrib
>> /dev/null
> echo "EXTRA_INSTALL = contrib/amcheck contrib/pageinspect"
>>> contrib/hstore/Makefile
> make check -C contrib/hstore
> export LD_LIBRARY_PATH=tmp_install/usr/local/pgsql/lib
> tmp_install/usr/local/pgsql/bin/initdb -D /tmp/db
> tmp_install/usr/local/pgsql/bin/pg_upgrade -d
> "../v10/contrib/hstore/tmp_check/data" -D /tmp/db -b
> "../v10/tmp_install/usr/local/pgsql/bin" -B
> tmp_install/usr/local/pgsql/bin
> tmp_install/usr/local/pgsql/bin/pg_ctl -D /tmp/db -l l.log start
> tmp_install/usr/local/pgsql/bin/psql -dcontrib_regression -c "create
> extension if not exists amcheck;"
> tmp_install/usr/local/pgsql/bin/psql -dcontrib_regression -c "SELECT
> bt_index_parent_check('hidx', true);"
>
> I get the following error:
> ERROR:  mismatch between parent key and child high key in index "hidx"
> DETAIL:  Target block=3 child block=1 target page lsn=0/16D5758.
>
> (The same procedure with v11/12 doesn't produce the error.)
>
> Thank you.

Good catch.

This check fails at v13 with any upgraded index that have BTREE_VERSION 2.

I think it is an amcheck problem. This check was introduced by commit 
d114cc5387.

The bt_pivot_tuple_identical() function excludes block number from 
comparison:

/*
  * Check if two tuples are binary identical except the block number.  So,
  * this function is capable to compare pivot keys on different levels.
  */
static bool
bt_pivot_tuple_identical(highkey, itup)

But it compares offset number. Which is fine for versions > 2, because 
since covering were introduced we store natts in this field and it is 
always initialized.

  * We store the number of columns present inside pivot tuples by abusing
  * their t_tid offset field, since pivot tuples never need to store a real
  * offset (pivot tuples generally store a downlink in t_tid, though).

For v2 indexes pivot tuple's offset can contain any random number which 
will lead to bt_pivot_tuple_identical() failure.

The fix is pretty simple - only compare data part of the tuples. I think 
we can skip offnum check for all index versions to keep the code simple.

I also noticed a typo in a comment and added minor correction to the patch.

-- 
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company