public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 2/2] fix
5+ messages / 4 participants
[nested] [flat]
* [PATCH 2/2] fix
@ 2020-08-01 02:24 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: Justin Pryzby @ 2020-08-01 02:24 UTC (permalink / raw)
---
src/backend/access/heap/vacuumlazy.c | 29 +++++++++++++++-------------
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 78d1db9ae2..b6015c9297 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1860,7 +1860,6 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats)
tblk = ItemPointerGetBlockNumber(&vacrelstats->dead_tuples->itemptrs[tupindex]);
vacrelstats->blkno = tblk;
- vacrelstats->offnum = ItemPointerGetOffsetNumber(&vacrelstats->dead_tuples->itemptrs[tupindex]);
buf = ReadBufferExtended(onerel, MAIN_FORKNUM, tblk, RBM_NORMAL,
vac_strategy);
if (!ConditionalLockBufferForCleanup(buf))
@@ -1937,7 +1936,6 @@ lazy_vacuum_page(Relation onerel, BlockNumber blkno, Buffer buffer,
if (tblk != blkno)
break; /* past end of tuples for this block */
toff = ItemPointerGetOffsetNumber(&dead_tuples->itemptrs[tupindex]);
- vacrelstats->offnum = toff;
itemid = PageGetItemId(page, toff);
ItemIdSetUnused(itemid);
unused[uncnt++] = toff;
@@ -2022,6 +2020,7 @@ lazy_check_needs_freeze(Buffer buf, bool *hastup, LVRelStats *vacrelstats)
OffsetNumber offnum,
maxoff;
HeapTupleHeader tupleheader;
+ LVSavedErrInfo saved_err_info;
*hastup = false;
@@ -2034,6 +2033,11 @@ lazy_check_needs_freeze(Buffer buf, bool *hastup, LVRelStats *vacrelstats)
if (PageIsNew(page) || PageIsEmpty(page))
return false;
+ /* Update error traceback information */
+ update_vacuum_error_info(vacrelstats, &saved_err_info,
+ VACUUM_ERRCB_PHASE_SCAN_HEAP, vacrelstats->blkno,
+ InvalidOffsetNumber);
+
maxoff = PageGetMaxOffsetNumber(page);
for (offnum = FirstOffsetNumber;
offnum <= maxoff;
@@ -2056,10 +2060,13 @@ lazy_check_needs_freeze(Buffer buf, bool *hastup, LVRelStats *vacrelstats)
if (heap_tuple_needs_freeze(tupleheader, FreezeLimit,
MultiXactCutoff, buf))
- return true;
+ break;
} /* scan along page */
- return false;
+ /* Revert to the previous phase information for error traceback */
+ restore_vacuum_error_info(vacrelstats, &saved_err_info);
+
+ return offnum <= maxoff ? true : false;
}
/*
@@ -2501,7 +2508,7 @@ lazy_cleanup_index(Relation indrel,
*stats = index_vacuum_cleanup(&ivinfo, *stats);
- /* Revert back to the old phase information for error traceback */
+ /* Revert to the old phase information for error traceback */
restore_vacuum_error_info(vacrelstats, &saved_err_info);
pfree(vacrelstats->indname);
vacrelstats->indname = NULL;
@@ -3590,8 +3597,8 @@ vacuum_error_callback(void *arg)
if (BlockNumberIsValid(errinfo->blkno))
{
if (OffsetNumberIsValid(errinfo->offnum))
- errcontext("while scanning block %u and offset %u of relation \"%s.%s\"",
- errinfo->blkno, errinfo->offnum, errinfo->relnamespace, errinfo->relname);
+ errcontext("while scanning block %u of relation \"%s.%s\", item offset %u",
+ errinfo->blkno, errinfo->relnamespace, errinfo->relname, errinfo->offnum);
else
errcontext("while scanning block %u of relation \"%s.%s\"",
errinfo->blkno, errinfo->relnamespace, errinfo->relname);
@@ -3601,12 +3608,8 @@ vacuum_error_callback(void *arg)
case VACUUM_ERRCB_PHASE_VACUUM_HEAP:
if (BlockNumberIsValid(errinfo->blkno))
{
- if (OffsetNumberIsValid(errinfo->offnum))
- errcontext("while vacuuming block %u and offset %u of relation \"%s.%s\"",
- errinfo->blkno, errinfo->offnum, errinfo->relnamespace, errinfo->relname);
- else
- errcontext("while vacuuming block %u of relation \"%s.%s\"",
- errinfo->blkno, errinfo->relnamespace, errinfo->relname);
+ errcontext("while vacuuming block %u of relation \"%s.%s\"",
+ errinfo->blkno, errinfo->relnamespace, errinfo->relname);
}
break;
--
2.17.0
--OQhbRXNHSL5w/5po--
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Assert in pageinspect with NULL pages
@ 2022-02-18 03:00 Justin Pryzby <[email protected]>
2022-02-18 03:07 ` Re: Assert in pageinspect with NULL pages Michael Paquier <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Justin Pryzby @ 2022-02-18 03:00 UTC (permalink / raw)
To: Daria Lepikhova <[email protected]>; +Cc: pgsql-hackers
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 <[email protected]>
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++)
{
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Assert in pageinspect with NULL pages
2022-02-18 03:00 Re: Assert in pageinspect with NULL pages Justin Pryzby <[email protected]>
@ 2022-02-18 03:07 ` Michael Paquier <[email protected]>
2022-02-21 07:00 ` Re: Assert in pageinspect with NULL pages Alexander Lakhin <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Michael Paquier @ 2022-02-18 03:07 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Daria Lepikhova <[email protected]>; pgsql-hackers
On Thu, Feb 17, 2022 at 09:00:20PM -0600, Justin Pryzby wrote:
> 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.
Ugh. Thanks! I am keeping a note about this one.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Assert in pageinspect with NULL pages
2022-02-18 03:00 Re: Assert in pageinspect with NULL pages Justin Pryzby <[email protected]>
2022-02-18 03:07 ` Re: Assert in pageinspect with NULL pages Michael Paquier <[email protected]>
@ 2022-02-21 07:00 ` Alexander Lakhin <[email protected]>
2022-02-21 08:04 ` Re: Assert in pageinspect with NULL pages Michael Paquier <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Alexander Lakhin @ 2022-02-21 07:00 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; Justin Pryzby <[email protected]>; +Cc: Daria Lepikhova <[email protected]>; pgsql-hackers
Hello Michael,
18.02.2022 06:07, Michael Paquier wrpte:
> On Thu, Feb 17, 2022 at 09:00:20PM -0600, Justin Pryzby wrote:
>> 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.
> Ugh. Thanks! I am keeping a note about this one.
Could you please confirm before committing the patchset that it fixes
the bug #16527 [1]? Or maybe I could check it?
(Original patch proposed by Daria doesn't cover that case, but if the
patch going to be improved, probably it's worth fixing that bug too.)
[1]
https://www.postgresql.org/message-id/flat/16527-ef7606186f0610a1%40postgresql.org
Best regards,
Alexander
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Assert in pageinspect with NULL pages
2022-02-18 03:00 Re: Assert in pageinspect with NULL pages Justin Pryzby <[email protected]>
2022-02-18 03:07 ` Re: Assert in pageinspect with NULL pages Michael Paquier <[email protected]>
2022-02-21 07:00 ` Re: Assert in pageinspect with NULL pages Alexander Lakhin <[email protected]>
@ 2022-02-21 08:04 ` Michael Paquier <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: Michael Paquier @ 2022-02-21 08:04 UTC (permalink / raw)
To: Alexander Lakhin <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Daria Lepikhova <[email protected]>; pgsql-hackers
On Mon, Feb 21, 2022 at 10:00:00AM +0300, Alexander Lakhin wrote:
> Could you please confirm before committing the patchset that it fixes
> the bug #16527 [1]? Or maybe I could check it?
> (Original patch proposed by Daria doesn't cover that case, but if the
> patch going to be improved, probably it's worth fixing that bug too.)
I have not directly checked, but that looks like the same issue to
me. By the way, I was waiting for an updated patch set, based on the
review provided upthread:
https://www.postgresql.org/message-id/Yg8MPrV49/[email protected]
And it seems better to group everything in a single commit as the same
areas are touched.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2022-02-21 08:04 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-08-01 02:24 [PATCH 2/2] fix Justin Pryzby <[email protected]>
2022-02-18 03:00 Re: Assert in pageinspect with NULL pages Justin Pryzby <[email protected]>
2022-02-18 03:07 ` Re: Assert in pageinspect with NULL pages Michael Paquier <[email protected]>
2022-02-21 07:00 ` Re: Assert in pageinspect with NULL pages Alexander Lakhin <[email protected]>
2022-02-21 08:04 ` Re: Assert in pageinspect with NULL pages Michael Paquier <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox