Message-ID: <0492c144-1e84-fd7e-e0ef-e8276ffaf97c@postgrespro.ru>
Date: Wed, 23 Feb 2022 12:09:08 +0500
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: Assert in pageinspect with NULL pages
Content-Language: en-US
To: Justin Pryzby <pryzby@telsasoft.com>
References: <561e187b-3549-c8d5-03f5-525c14e65bd0@postgrespro.ru>
 <20220218030020.GA1137@telsasoft.com>
From: Daria Lepikhova <d.lepikhova@postgrespro.ru>
Cc: pgsql-hackers@postgresql.org
In-Reply-To: <20220218030020.GA1137@telsasoft.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: <https://www.postgresql.org/message-id/0492c144-1e84-fd7e-e0ef-e8276ffaf97c%40postgrespro.ru>
Precedence: bulk


18.02.2022 08:00, Justin Pryzby пишет:
> BRIN can also crash if passed a non-brin index.
>
> I've been sitting on this one for awhile.  Feel free to include it in your
> patchset.
>
> commit 08010a6037fc4e24a9ba05e5386e766f4310d35e
> Author: Justin Pryzby <pryzbyj@telsasoft.com>
> Date:   Tue Jan 19 00:25:15 2021 -0600
>
>      pageinspect: brin_page_items(): check that given relation is not only an index, but a brin one
>
> diff --git a/contrib/pageinspect/brinfuncs.c b/contrib/pageinspect/brinfuncs.c
> index f1e64a39ef2..3de6dd943ef 100644
> --- a/contrib/pageinspect/brinfuncs.c
> +++ b/contrib/pageinspect/brinfuncs.c
> @@ -16,6 +16,7 @@
>   #include "access/brin_tuple.h"
>   #include "access/htup_details.h"
>   #include "catalog/index.h"
> +#include "catalog/pg_am.h"
>   #include "catalog/pg_type.h"
>   #include "funcapi.h"
>   #include "lib/stringinfo.h"
> @@ -59,7 +60,7 @@ brin_page_type(PG_FUNCTION_ARGS)
>   	if (raw_page_size != BLCKSZ)
>   		ereport(ERROR,
>   				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> -				 errmsg("input page too small"),
> +				 errmsg("input page wrong size"),
>   				 errdetail("Expected size %d, got %d",
>   						   BLCKSZ, raw_page_size)));
>   
> @@ -97,7 +98,7 @@ verify_brin_page(bytea *raw_page, uint16 type, const char *strtype)
>   	if (raw_page_size != BLCKSZ)
>   		ereport(ERROR,
>   				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> -				 errmsg("input page too small"),
> +				 errmsg("input page wrong size"),
>   				 errdetail("Expected size %d, got %d",
>   						   BLCKSZ, raw_page_size)));
>   
> @@ -169,7 +170,14 @@ brin_page_items(PG_FUNCTION_ARGS)
>   	MemoryContextSwitchTo(oldcontext);
>   
>   	indexRel = index_open(indexRelid, AccessShareLock);
> -	bdesc = brin_build_desc(indexRel);
> +
> +	/* Must be a BRIN index */
> +	if (indexRel->rd_rel->relkind != RELKIND_INDEX ||
> +			indexRel->rd_rel->relam != BRIN_AM_OID)
> +		ereport(ERROR,
> +				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
> +				 errmsg("\"%s\" is not a BRIN index",
> +					 RelationGetRelationName(indexRel))));
>   
>   	/* minimally verify the page we got */
>   	page = verify_brin_page(raw_page, BRIN_PAGETYPE_REGULAR, "regular");
> @@ -178,6 +186,7 @@ brin_page_items(PG_FUNCTION_ARGS)
>   	 * Initialize output functions for all indexed datatypes; simplifies
>   	 * calling them later.
>   	 */
> +	bdesc = brin_build_desc(indexRel);
>   	columns = palloc(sizeof(brin_column_state *) * RelationGetDescr(indexRel)->natts);
>   	for (attno = 1; attno <= bdesc->bd_tupdesc->natts; attno++)
>   	{


Thanks! This is a very valuable note. I think I will definitely add it 
to the next version of the patch, as soon as I deal with the error 
exception.

-- 

Daria Lepikhova
Postgres Professional