Horton

Re: new heapcheck contrib module (typos)

Mark Dilger <mark.dilger@enterprisedb.com>

View thread View on postgr.es
From: Mark Dilger <mark.dilger@enterprisedb.com>
To: Erik Rijkers <er@xs4all.nl>
Cc: Dilip Kumar <dilipbalaut@gmail.com>, Robert Haas <robertmhaas@gmail.com>, Peter Geoghegan <pg@bowt.ie>, Andres Freund <andres@anarazel.de>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2020-06-13T22:11:42Z
Lists: pgsql-hackers

Attachments


> On Jun 13, 2020, at 2:13 PM, Erik Rijkers <er@xs4all.nl> wrote:

Thanks for the review!

> On 2020-06-12 23:06, Mark Dilger wrote:
> 
>> [v7-0001-Adding-verify_heapam-and-pg_amcheck.patch]
>> [v7-0002-Adding-checks-o...ations-of-hint-bit.patch]
> 
> I came across these typos in the sgml:
> 
> --exclude-scheam   should be
> --exclude-schema
> 
> <option>table</option>     should be
> <option>--table</option>

Yeah, I agree and have made these changes for v8.

> I found this connection problem (or perhaps it is as designed):
> 
> $ env | grep ^PG
> PGPORT=6965
> PGPASSFILE=/home/aardvark/.pg_aardvark
> PGDATABASE=testdb
> PGDATA=/home/aardvark/pg_stuff/pg_installations/pgsql.amcheck/data
> 
> -- just to show that psql is connecting (via $PGPASSFILE and $PGPORT and $PGDATABASE):
> -- and showing a  table t  that I made earlier
> 
> $ psql
> SET
> Timing is on.
> psql (14devel_amcheck_0612_2f48)
> Type "help" for help.
> 
> testdb=# \dt+ t
>                           List of relations
> Schema | Name | Type  |  Owner   | Persistence |  Size  | Description
> --------+------+-------+----------+-------------+--------+-------------
> public | t    | table | aardvark | permanent   | 346 MB |
> (1 row)
> 
> testdb=# \q
> 
> I think this should work:
> 
> $ pg_amcheck -i -t t
> pg_amcheck: error: no matching tables were found
> 
> It seems a bug that I have to add  '-d testdb':
> 
> This works OK:
> pg_amcheck -i -t t -d testdb
> 
> Is that error as expected?

It was expected, but looking more broadly at other tools, your expectations seem to be more typical.  I've changed it in v8.  Thanks again for having a look at this patch!

Note that I've merge the two patches (v7-0001 and v7-0002) back into a single patch, since the separation introduced in v7 was only for illustration of changes in v7.

Commits

  1. Add pg_amcheck, a CLI for contrib/amcheck.

  2. Refactor and generalize the ParallelSlot machinery.

  3. Generalize parallel slot result handling.

  4. Move some code from src/bin/scripts to src/fe_utils to permit reuse.

  5. Factor pattern-construction logic out of processSQLNamePattern.

  6. Doc: clean up verify_heapam() documentation.

  7. Fix more portability issues in new amcheck code.

  8. Fix portability issues in new amcheck test.

  9. Try to avoid a compiler warning about using fxid uninitialized.

  10. Extend amcheck to check heap pages.

  11. Adjust walsender usage of xlogreader, simplify APIs

  12. Improve checking of child pages in contrib/amcheck.

  13. Sanitize line pointers within contrib/amcheck.

  14. Fix possible sorting error when aborting use of abbreviated keys.