Date: Sun, 1 Oct 2023 13:58:30 -0700
From: Noah Misch <noah@leadboat.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org
Subject: Re: pgstatindex vs. !indisready
Message-ID: <20231001205830.2f.nmisch@google.com>
References: <20231001195309.a3@google.com>
 <1867127.1696192645@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="3J3MVk1Faq2QMfjI"
Content-Disposition: inline
In-Reply-To: <1867127.1696192645@sss.pgh.pa.us>
User-Agent: Mutt/2.2.12 (2023-09-09)
Archived-At: 
 <https://www.postgresql.org/message-id/20231001205830.2f.nmisch%40google.com>
Precedence: bulk


--3J3MVk1Faq2QMfjI
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Sun, Oct 01, 2023 at 04:37:25PM -0400, Tom Lane wrote:
> Noah Misch <noah@leadboat.com> writes:
> > Running pgstatindex on some !indisready indexes fails with "ERROR:  XX001:
> > could not read block 0 in file".  This reproduces it:
> > ...
> > Since XX001 = ERRCODE_DATA_CORRUPTED appears in the "can't-happen" class, it's
> > not a good fit for this scenario.  I propose to have pgstatindex fail early on
> > !indisready indexes.
> 
> +1
> 
> > We could go a step further and also fail on
> > indisready&&!indisvalid indexes, which are complete enough to accept inserts
> > but not complete enough to answer queries.  I don't see a reason to do that,
> > but maybe someone else will.
> 
> Hmm.  Seems like the numbers pgstatindex would produce for a
> not-yet-complete index would be rather irrelevant, even if the case
> doesn't risk any outright problems.  I'd be inclined to be
> conservative and insist on indisvalid being true too.

Okay.  If no other preferences appear, I'll do pgstatindex that way.

> > This made me wonder about functions handling unlogged rels during recovery.  I
> > used the attached hack to test most regclass-arg functions.

I forgot to test the same battery of functions on !indisready indexes.  I've
now done that, using the attached script.  While I didn't get additional
class-XX errors, more should change:

[pgstatginindex pgstathashindex pgstattuple] currently succeed.  Like
pgstatindex, they should ERROR, including in the back branches.

[brin_desummarize_range brin_summarize_new_values brin_summarize_range
gin_clean_pending_list] currently succeed.  I propose to make them emit a
DEBUG1 message and return early, like amcheck does, except on !indisready.
This would allow users to continue running them on all indexes of the
applicable access method.  Doing these operations on an
indisready&&!indisvalid index is entirely reasonable, since they relate to
INSERT/UPDATE/DELETE operations.

[pg_freespace pg_indexes_size pg_prewarm] currently succeed, and I propose to
leave them that way.  No undefined behavior arises.  pg_freespace needs to be
resilient to garbage data anyway, given the lack of WAL for the FSM.  One
could argue for a relkind check in pg_indexes_size.  One could argue for
treating pg_prewarm like amcheck (DEBUG1 and return).

--3J3MVk1Faq2QMfjI
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="invalid-index-funcalls.sql"

rollback;
begin;
create extension if not exists btree_gin;
create or replace function error(text) returns text language plpgsql as $$
begin
	raise exception 'break index build';
	return $1;
end;
$$ immutable;
drop table if exists u;
create table u (c text);
insert into u values ('foo');
commit;
create index concurrently ubtree on u (error(c));
create index concurrently ugin on u using gin (error(c));
create index concurrently uhash on u using hash (error(c));
create index concurrently ubrin on u using brin (error(c));

\set utable '''ubtree''::regclass'
\set useq '''ubtree''::regclass'
\set VERBOSITY verbose
begin;
create or replace function error(text) returns text language sql
  as 'select $1' immutable;

SET log_error_verbosity = verbose;
SET log_min_messages = debug1; -- for amcheck
--SET client_min_messages = debug1;

create extension pg_visibility;
create extension amcheck;
create extension pgstattuple;
create extension pg_surgery;
create extension pg_prewarm;
create extension pg_freespacemap;
create extension pgrowlocks;

SELECT * FROM pgrowlocks(:utable::text);
SELECT brin_desummarize_range('ubrin',1);
SELECT brin_summarize_new_values('ubrin');
SELECT brin_summarize_range('ubrin',1);
SELECT bt_index_check('ubtree');
SELECT bt_index_check('ubtree',true);
SELECT bt_index_parent_check('ubtree');
SELECT bt_index_parent_check('ubtree',true);
SELECT bt_index_parent_check('ubtree',true,true);
SELECT gin_clean_pending_list('ugin');
SELECT heap_force_freeze(:utable,array['(0,1)'::tid]);
SELECT heap_force_kill(:utable,array['(0,1)'::tid]);
SELECT nextval(:useq);
SELECT currval(:useq);
SELECT pg_check_frozen(:utable);
SELECT pg_check_visible(:utable);
SELECT pg_column_is_updatable(:utable,'1',true);
--SELECT pg_extension_config_dump(:utable,'select 1');
SELECT pg_freespace(:utable);
SELECT pg_freespace(:utable,1);
SELECT pg_get_replica_identity_index(:utable);
SELECT pg_index_column_has_property(:utable,1,'asc');
SELECT pg_indexes_size(:utable);
SELECT pg_index_has_property('ubrin','asc');
--SELECT pg_nextoid(:utable,name,:utable);
SELECT pg_partition_ancestors(:utable);
SELECT pg_partition_root(:utable);
SELECT pg_partition_tree(:utable);
SELECT pg_prewarm(:utable);
SELECT pg_relation_filenode(:utable);
SELECT pg_relation_filepath(:utable);
SELECT pg_relation_is_publishable(:utable);
SELECT pg_relation_is_updatable(:utable,true);
SELECT pg_relation_size(:utable);
SELECT pg_relation_size(:utable,'main');
SELECT pg_relpages(:utable);
SELECT pg_sequence_last_value(:useq);
SELECT pgstatginindex('ugin');
SELECT pgstathashindex('uhash');
SELECT pgstatindex('ubtree');
SELECT pgstattuple_approx(:utable);
SELECT pgstattuple(:utable);
SELECT pg_table_size(:utable);
SELECT pg_total_relation_size(:utable);
SELECT pg_truncate_visibility_map(:utable);
SELECT pg_visibility_map(:utable);
SELECT pg_visibility_map(:utable,1);
SELECT pg_visibility_map_summary(:utable);
SELECT pg_visibility(:utable);
SELECT pg_visibility(:utable,1);
SELECT setval(:useq,1);
SELECT setval(:useq,1,true);
SELECT table_to_xml_and_xmlschema(:utable,true,true,'nsp');
SELECT table_to_xml(:utable,true,true,'nsp');
SELECT table_to_xmlschema(:utable,true,true,'nsp');
SELECT verify_heapam(:utable);
rollback;

--3J3MVk1Faq2QMfjI--