agora inbox for [email protected]
help / color / mirror / Atom feed[PATCH 3/8] Add rewrite rules and tupdesc flags
7+ messages / 3 participants
[nested] [flat]
* [PATCH 3/8] Add rewrite rules and tupdesc flags
@ 2018-06-18 12:34 Ildus Kurbangaliev <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Ildus Kurbangaliev @ 2018-06-18 12:34 UTC (permalink / raw)
---
src/backend/access/brin/brin_tuple.c | 2 +
src/backend/access/common/heaptuple.c | 27 +++++++++-
src/backend/access/common/indextuple.c | 2 +
src/backend/access/common/tupdesc.c | 20 ++++++++
src/backend/access/heap/heapam.c | 19 ++++---
src/backend/access/heap/tuptoaster.c | 2 +
src/backend/commands/copy.c | 2 +-
src/backend/commands/createas.c | 2 +-
src/backend/commands/matview.c | 2 +-
src/backend/commands/tablecmds.c | 68 ++++++++++++++++++++++++--
src/backend/utils/adt/expandedrecord.c | 1 +
src/backend/utils/cache/relcache.c | 4 ++
src/include/access/heapam.h | 3 +-
src/include/access/hio.h | 2 +
src/include/access/htup_details.h | 9 +++-
src/include/access/tupdesc.h | 7 +++
src/include/access/tuptoaster.h | 1 -
src/include/commands/event_trigger.h | 1 +
18 files changed, 156 insertions(+), 18 deletions(-)
diff --git a/src/backend/access/brin/brin_tuple.c b/src/backend/access/brin/brin_tuple.c
index 00316b899c..cb9024863d 100644
--- a/src/backend/access/brin/brin_tuple.c
+++ b/src/backend/access/brin/brin_tuple.c
@@ -96,6 +96,7 @@ brin_form_tuple(BrinDesc *brdesc, BlockNumber blkno, BrinMemTuple *tuple,
int keyno;
int idxattno;
uint16 phony_infomask = 0;
+ uint16 phony_infomask2 = 0;
bits8 *phony_nullbitmap;
Size len,
hoff,
@@ -187,6 +188,7 @@ brin_form_tuple(BrinDesc *brdesc, BlockNumber blkno, BrinMemTuple *tuple,
(char *) rettuple + hoff,
data_len,
&phony_infomask,
+ &phony_infomask2,
phony_nullbitmap);
/* done with these */
diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c
index 2ec7e6a439..680fc9df4b 100644
--- a/src/backend/access/common/heaptuple.c
+++ b/src/backend/access/common/heaptuple.c
@@ -214,6 +214,7 @@ fill_val(Form_pg_attribute att,
int *bitmask,
char **dataP,
uint16 *infomask,
+ uint16 *infomask2,
Datum datum,
bool isnull)
{
@@ -244,6 +245,9 @@ fill_val(Form_pg_attribute att,
**bit |= *bitmask;
}
+ if (OidIsValid(att->attcompression))
+ *infomask2 |= HEAP_HASCUSTOMCOMPRESSED;
+
/*
* XXX we use the att_align macros on the pointer value itself, not on an
* offset. This is a bit of a hack.
@@ -282,6 +286,15 @@ fill_val(Form_pg_attribute att,
/* no alignment, since it's short by definition */
data_length = VARSIZE_EXTERNAL(val);
memcpy(data, val, data_length);
+
+ if (VARATT_IS_EXTERNAL_ONDISK(val))
+ {
+ struct varatt_external toast_pointer;
+
+ VARATT_EXTERNAL_GET_POINTER(toast_pointer, val);
+ if (VARATT_EXTERNAL_IS_CUSTOM_COMPRESSED(toast_pointer))
+ *infomask2 |= HEAP_HASCUSTOMCOMPRESSED;
+ }
}
}
else if (VARATT_IS_SHORT(val))
@@ -305,6 +318,9 @@ fill_val(Form_pg_attribute att,
att->attalign);
data_length = VARSIZE(val);
memcpy(data, val, data_length);
+
+ if (VARATT_IS_CUSTOM_COMPRESSED(val))
+ *infomask2 |= HEAP_HASCUSTOMCOMPRESSED;
}
}
else if (att->attlen == -2)
@@ -341,7 +357,7 @@ void
heap_fill_tuple(TupleDesc tupleDesc,
Datum *values, bool *isnull,
char *data, Size data_size,
- uint16 *infomask, bits8 *bit)
+ uint16 *infomask, uint16 *infomask2, bits8 *bit)
{
bits8 *bitP;
int bitmask;
@@ -365,6 +381,7 @@ heap_fill_tuple(TupleDesc tupleDesc,
}
*infomask &= ~(HEAP_HASNULL | HEAP_HASVARWIDTH | HEAP_HASEXTERNAL);
+ *infomask2 &= ~HEAP_HASCUSTOMCOMPRESSED;
for (i = 0; i < numberOfAttributes; i++)
{
@@ -375,6 +392,7 @@ heap_fill_tuple(TupleDesc tupleDesc,
&bitmask,
&data,
infomask,
+ infomask2,
values ? values[i] : PointerGetDatum(NULL),
isnull ? isnull[i] : true);
}
@@ -793,6 +811,7 @@ expand_tuple(HeapTuple *targetHeapTuple,
int bitMask = 0;
char *targetData;
uint16 *infoMask;
+ uint16 *infoMask2;
Assert((targetHeapTuple && !targetMinimalTuple)
|| (!targetHeapTuple && targetMinimalTuple));
@@ -917,6 +936,7 @@ expand_tuple(HeapTuple *targetHeapTuple,
+ offsetof(HeapTupleHeaderData, t_bits));
targetData = (char *) (*targetHeapTuple)->t_data + hoff;
infoMask = &(targetTHeader->t_infomask);
+ infoMask2 = &(targetTHeader->t_infomask2);
}
else
{
@@ -935,6 +955,7 @@ expand_tuple(HeapTuple *targetHeapTuple,
+ offsetof(MinimalTupleData, t_bits));
targetData = (char *) *targetMinimalTuple + hoff;
infoMask = &((*targetMinimalTuple)->t_infomask);
+ infoMask2 = &((*targetMinimalTuple)->t_infomask2);
}
if (targetNullLen > 0)
@@ -987,6 +1008,7 @@ expand_tuple(HeapTuple *targetHeapTuple,
&bitMask,
&targetData,
infoMask,
+ infoMask2,
attrmiss[attnum].am_value,
false);
}
@@ -997,6 +1019,7 @@ expand_tuple(HeapTuple *targetHeapTuple,
&bitMask,
&targetData,
infoMask,
+ infoMask2,
(Datum) 0,
true);
}
@@ -1152,6 +1175,7 @@ heap_form_tuple(TupleDesc tupleDescriptor,
(char *) td + hoff,
data_len,
&td->t_infomask,
+ &td->t_infomask2,
(hasnull ? td->t_bits : NULL));
return tuple;
@@ -1855,6 +1879,7 @@ heap_form_minimal_tuple(TupleDesc tupleDescriptor,
(char *) tuple + hoff,
data_len,
&tuple->t_infomask,
+ &tuple->t_infomask2,
(hasnull ? tuple->t_bits : NULL));
return tuple;
diff --git a/src/backend/access/common/indextuple.c b/src/backend/access/common/indextuple.c
index 9a1cebbe2f..38fbdf7205 100644
--- a/src/backend/access/common/indextuple.c
+++ b/src/backend/access/common/indextuple.c
@@ -50,6 +50,7 @@ index_form_tuple(TupleDesc tupleDescriptor,
unsigned short infomask = 0;
bool hasnull = false;
uint16 tupmask = 0;
+ uint16 tupmask2 = 0;
int numberOfAttributes = tupleDescriptor->natts;
#ifdef TOAST_INDEX_HACK
@@ -162,6 +163,7 @@ index_form_tuple(TupleDesc tupleDescriptor,
(char *) tp + hoff,
data_size,
&tupmask,
+ &tupmask2,
(hasnull ? (bits8 *) tp + sizeof(IndexTupleData) : NULL));
#ifdef TOAST_INDEX_HACK
diff --git a/src/backend/access/common/tupdesc.c b/src/backend/access/common/tupdesc.c
index b0434b4672..0628416f0d 100644
--- a/src/backend/access/common/tupdesc.c
+++ b/src/backend/access/common/tupdesc.c
@@ -21,6 +21,7 @@
#include "access/hash.h"
#include "access/htup_details.h"
+#include "access/reloptions.h"
#include "access/tupdesc_details.h"
#include "catalog/pg_collation.h"
#include "catalog/pg_type.h"
@@ -74,6 +75,7 @@ CreateTemplateTupleDesc(int natts, bool hasoid)
desc->tdtypeid = RECORDOID;
desc->tdtypmod = -1;
desc->tdhasoid = hasoid;
+ desc->tdflags = 0;
desc->tdrefcount = -1; /* assume not reference-counted */
return desc;
@@ -96,8 +98,17 @@ CreateTupleDesc(int natts, bool hasoid, Form_pg_attribute *attrs)
desc = CreateTemplateTupleDesc(natts, hasoid);
for (i = 0; i < natts; ++i)
+ {
memcpy(TupleDescAttr(desc, i), attrs[i], ATTRIBUTE_FIXED_PART_SIZE);
+ /*
+ * If even one of attributes is compressed we save information about
+ * it to TupleDesc flags
+ */
+ if (OidIsValid(attrs[i]->attcompression))
+ desc->tdflags |= TD_ATTR_CUSTOM_COMPRESSED;
+ }
+
return desc;
}
@@ -138,6 +149,7 @@ CreateTupleDescCopy(TupleDesc tupdesc)
/* We can copy the tuple type identification, too */
desc->tdtypeid = tupdesc->tdtypeid;
desc->tdtypmod = tupdesc->tdtypmod;
+ desc->tdflags = tupdesc->tdflags;
return desc;
}
@@ -217,6 +229,7 @@ CreateTupleDescCopyConstr(TupleDesc tupdesc)
/* We can copy the tuple type identification, too */
desc->tdtypeid = tupdesc->tdtypeid;
desc->tdtypmod = tupdesc->tdtypmod;
+ desc->tdflags = tupdesc->tdflags;
return desc;
}
@@ -302,6 +315,7 @@ TupleDescCopyEntry(TupleDesc dst, AttrNumber dstAttno,
dstAtt->atthasdef = false;
dstAtt->atthasmissing = false;
dstAtt->attidentity = '\0';
+ dstAtt->attcompression = InvalidOid;
}
/*
@@ -418,6 +432,8 @@ equalTupleDescs(TupleDesc tupdesc1, TupleDesc tupdesc2)
return false;
if (tupdesc1->tdhasoid != tupdesc2->tdhasoid)
return false;
+ if (tupdesc1->tdflags != tupdesc2->tdflags)
+ return false;
for (i = 0; i < tupdesc1->natts; i++)
{
@@ -468,6 +484,8 @@ equalTupleDescs(TupleDesc tupdesc1, TupleDesc tupdesc2)
return false;
if (attr1->attcollation != attr2->attcollation)
return false;
+ if (attr1->attcompression != attr2->attcompression)
+ return false;
/* attacl, attoptions and attfdwoptions are not even present... */
}
@@ -553,6 +571,7 @@ equalTupleDescs(TupleDesc tupdesc1, TupleDesc tupdesc2)
}
else if (tupdesc2->constr != NULL)
return false;
+
return true;
}
@@ -659,6 +678,7 @@ TupleDescInitEntry(TupleDesc desc,
att->attalign = typeForm->typalign;
att->attstorage = typeForm->typstorage;
att->attcollation = typeForm->typcollation;
+ att->attcompression = InvalidOid;
ReleaseSysCache(tuple);
}
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index fdac4bbb67..0917019f72 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -96,7 +96,8 @@ static HeapScanDesc heap_beginscan_internal(Relation relation,
static void heap_parallelscan_startblock_init(HeapScanDesc scan);
static BlockNumber heap_parallelscan_nextpage(HeapScanDesc scan);
static HeapTuple heap_prepare_insert(Relation relation, HeapTuple tup,
- TransactionId xid, CommandId cid, int options);
+ TransactionId xid, CommandId cid, int options,
+ BulkInsertState bistate);
static XLogRecPtr log_heap_update(Relation reln, Buffer oldbuf,
Buffer newbuf, HeapTuple oldtup,
HeapTuple newtup, HeapTuple old_key_tup,
@@ -2361,13 +2362,14 @@ UpdateXmaxHintBits(HeapTupleHeader tuple, Buffer buffer, TransactionId xid)
* GetBulkInsertState - prepare status object for a bulk insert
*/
BulkInsertState
-GetBulkInsertState(void)
+GetBulkInsertState(HTAB *preserved_am_info)
{
BulkInsertState bistate;
bistate = (BulkInsertState) palloc(sizeof(BulkInsertStateData));
bistate->strategy = GetAccessStrategy(BAS_BULKWRITE);
bistate->current_buf = InvalidBuffer;
+ bistate->preserved_am_info = preserved_am_info;
return bistate;
}
@@ -2454,7 +2456,7 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
* Note: below this point, heaptup is the data we actually intend to store
* into the relation; tup is the caller's original untoasted data.
*/
- heaptup = heap_prepare_insert(relation, tup, xid, cid, options);
+ heaptup = heap_prepare_insert(relation, tup, xid, cid, options, bistate);
/*
* Find buffer to insert this tuple into. If the page is all visible,
@@ -2623,7 +2625,7 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
*/
static HeapTuple
heap_prepare_insert(Relation relation, HeapTuple tup, TransactionId xid,
- CommandId cid, int options)
+ CommandId cid, int options, BulkInsertState bistate)
{
/*
* Parallel operations are required to be strictly read-only in a parallel
@@ -2685,8 +2687,11 @@ heap_prepare_insert(Relation relation, HeapTuple tup, TransactionId xid,
return tup;
}
else if (HeapTupleHasExternal(tup)
+ || RelationGetDescr(relation)->tdflags & TD_ATTR_CUSTOM_COMPRESSED
+ || HeapTupleHasCustomCompressed(tup)
|| tup->t_len > TOAST_TUPLE_THRESHOLD)
- return toast_insert_or_update(relation, tup, NULL, options, NULL);
+ return toast_insert_or_update(relation, tup, NULL, options,
+ bistate ? bistate->preserved_am_info : NULL);
else
return tup;
}
@@ -2725,7 +2730,7 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
heaptuples = palloc(ntuples * sizeof(HeapTuple));
for (i = 0; i < ntuples; i++)
heaptuples[i] = heap_prepare_insert(relation, tuples[i],
- xid, cid, options);
+ xid, cid, options, bistate);
/*
* Allocate some memory to use for constructing the WAL record. Using
@@ -4028,6 +4033,8 @@ l2:
else
need_toast = (HeapTupleHasExternal(&oldtup) ||
HeapTupleHasExternal(newtup) ||
+ RelationGetDescr(relation)->tdflags & TD_ATTR_CUSTOM_COMPRESSED ||
+ HeapTupleHasCustomCompressed(newtup) ||
newtup->t_len > TOAST_TUPLE_THRESHOLD);
pagefree = PageGetHeapFreeSpace(page);
diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.c
index 413b0b2b9c..f75ca6d9b2 100644
--- a/src/backend/access/heap/tuptoaster.c
+++ b/src/backend/access/heap/tuptoaster.c
@@ -1197,6 +1197,7 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
(char *) new_data + new_header_len,
new_data_len,
&(new_data->t_infomask),
+ &(new_data->t_infomask2),
has_nulls ? new_data->t_bits : NULL);
}
else
@@ -1425,6 +1426,7 @@ toast_flatten_tuple_to_datum(HeapTupleHeader tup,
(char *) new_data + new_header_len,
new_data_len,
&(new_data->t_infomask),
+ &(new_data->t_infomask2),
has_nulls ? new_data->t_bits : NULL);
/*
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 3a66cb5025..253a454fbf 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2555,7 +2555,7 @@ CopyFrom(CopyState cstate)
values = (Datum *) palloc(tupDesc->natts * sizeof(Datum));
nulls = (bool *) palloc(tupDesc->natts * sizeof(bool));
- bistate = GetBulkInsertState();
+ bistate = GetBulkInsertState(NULL);
econtext = GetPerTupleExprContext(estate);
/* Set up callback to identify error line number */
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index 3d82edbf58..c7059d5f62 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -569,7 +569,7 @@ intorel_startup(DestReceiver *self, int operation, TupleDesc typeinfo)
*/
myState->hi_options = HEAP_INSERT_SKIP_FSM |
(XLogIsNeeded() ? 0 : HEAP_INSERT_SKIP_WAL);
- myState->bistate = GetBulkInsertState();
+ myState->bistate = GetBulkInsertState(NULL);
/* Not using WAL requires smgr_targblock be initially invalid */
Assert(RelationGetTargetBlock(intoRelationDesc) == InvalidBlockNumber);
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index e1eb7c374b..969c8160c2 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -465,7 +465,7 @@ transientrel_startup(DestReceiver *self, int operation, TupleDesc typeinfo)
myState->hi_options = HEAP_INSERT_SKIP_FSM | HEAP_INSERT_FROZEN;
if (!XLogIsNeeded())
myState->hi_options |= HEAP_INSERT_SKIP_WAL;
- myState->bistate = GetBulkInsertState();
+ myState->bistate = GetBulkInsertState(NULL);
/* Not using WAL requires smgr_targblock be initially invalid */
Assert(RelationGetTargetBlock(transientrel) == InvalidBlockNumber);
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 0c7c201952..e17a94a778 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -166,6 +166,8 @@ typedef struct AlteredTableInfo
/* Information saved by Phases 1/2 for Phase 3: */
List *constraints; /* List of NewConstraint */
List *newvals; /* List of NewColumnValue */
+ HTAB *preservedAmInfo; /* Hash table for preserved compression
+ * methods */
bool new_notnull; /* T if we added new NOT NULL constraints */
int rewrite; /* Reason for forced rewrite, if any */
Oid newTableSpace; /* new tablespace; 0 means no change */
@@ -4648,7 +4650,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
if (newrel)
{
mycid = GetCurrentCommandId(true);
- bistate = GetBulkInsertState();
+ bistate = GetBulkInsertState(tab->preservedAmInfo);
hi_options = HEAP_INSERT_SKIP_FSM;
if (!XLogIsNeeded())
@@ -4919,6 +4921,24 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
FreeExecutorState(estate);
+ /* Remove old compression options */
+ if (tab->rewrite & AT_REWRITE_ALTER_COMPRESSION)
+ {
+ AttrCmPreservedInfo *pinfo;
+ HASH_SEQ_STATUS status;
+
+ Assert(tab->preservedAmInfo);
+ hash_seq_init(&status, tab->preservedAmInfo);
+ while ((pinfo = (AttrCmPreservedInfo *) hash_seq_search(&status)) != NULL)
+ {
+ CleanupAttributeCompression(tab->relid, pinfo->attnum,
+ pinfo->preserved_amoids);
+ list_free(pinfo->preserved_amoids);
+ }
+
+ hash_destroy(tab->preservedAmInfo);
+ }
+
heap_close(oldrel, NoLock);
if (newrel)
{
@@ -9020,6 +9040,45 @@ createForeignKeyTriggers(Relation rel, Oid refRelOid, Constraint *fkconstraint,
CommandCounterIncrement();
}
+/*
+ * Initialize hash table used to keep rewrite rules for
+ * compression changes in ALTER commands.
+ */
+static void
+setupCompressionRewriteRules(AlteredTableInfo *tab, const char *column,
+ AttrNumber attnum, List *preserved_amoids)
+{
+ bool found;
+ AttrCmPreservedInfo *pinfo;
+
+ Assert(!IsBinaryUpgrade);
+ tab->rewrite |= AT_REWRITE_ALTER_COMPRESSION;
+
+ /* initialize hash for oids */
+ if (tab->preservedAmInfo == NULL)
+ {
+ HASHCTL ctl;
+
+ MemSet(&ctl, 0, sizeof(ctl));
+ ctl.keysize = sizeof(AttrNumber);
+ ctl.entrysize = sizeof(AttrCmPreservedInfo);
+ tab->preservedAmInfo =
+ hash_create("preserved access methods cache", 10, &ctl,
+ HASH_ELEM | HASH_BLOBS);
+ }
+ pinfo = (AttrCmPreservedInfo *) hash_search(tab->preservedAmInfo,
+ &attnum, HASH_ENTER, &found);
+
+ if (found)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot alter compression of column \"%s\" twice", column),
+ errhint("Remove one of statements from the command.")));
+
+ pinfo->attnum = attnum;
+ pinfo->preserved_amoids = preserved_amoids;
+}
+
/*
* ALTER TABLE DROP CONSTRAINT
*
@@ -9915,7 +9974,7 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
{
/* Set up rewrite of table */
attTup->attcompression = InvalidOid;
- /* TODO: call the rewrite function here */
+ setupCompressionRewriteRules(tab, colName, attOldTup->attnum, NIL);
}
else if (OidIsValid(attTup->attcompression))
{
@@ -9923,7 +9982,7 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
ColumnCompression *compression = MakeColumnCompression(attTup->attcompression);
if (!IsBuiltinCompression(attTup->attcompression))
- /* TODO: call the rewrite function here */;
+ setupCompressionRewriteRules(tab, colName, attOldTup->attnum, NIL);
attTup->attcompression = CreateAttributeCompression(attTup, compression, NULL, NULL);
}
@@ -13035,7 +13094,8 @@ ATExecSetCompression(AlteredTableInfo *tab,
* toast_insert_or_update
*/
if (need_rewrite)
- /* TODO: set up rewrite rules here */;
+ setupCompressionRewriteRules(tab, column, atttableform->attnum,
+ preserved_amoids);
atttableform->attcompression = acoid;
CatalogTupleUpdate(attrel, &atttuple->t_self, atttuple);
diff --git a/src/backend/utils/adt/expandedrecord.c b/src/backend/utils/adt/expandedrecord.c
index b1b6883c19..b0e6dfc996 100644
--- a/src/backend/utils/adt/expandedrecord.c
+++ b/src/backend/utils/adt/expandedrecord.c
@@ -814,6 +814,7 @@ ER_flatten_into(ExpandedObjectHeader *eohptr,
(char *) tuphdr + erh->hoff,
erh->data_len,
&tuphdr->t_infomask,
+ &tuphdr->t_infomask2,
(erh->hasnull ? tuphdr->t_bits : NULL));
}
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 6125421d39..44be85af82 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -577,6 +577,10 @@ RelationBuildTupleDesc(Relation relation)
ndef++;
}
+ /* mark tupledesc as it contains attributes with custom compression */
+ if (attp->attcompression)
+ relation->rd_att->tdflags |= TD_ATTR_CUSTOM_COMPRESSED;
+
/* Likewise for a missing value */
if (attp->atthasmissing)
{
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index ca5cad7497..755682a4f8 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -22,6 +22,7 @@
#include "storage/lockdefs.h"
#include "utils/relcache.h"
#include "utils/snapshot.h"
+#include "utils/hsearch.h"
/* "options" flag bits for heap_insert */
@@ -146,7 +147,7 @@ extern void heap_get_latest_tid(Relation relation, Snapshot snapshot,
ItemPointer tid);
extern void setLastTid(const ItemPointer tid);
-extern BulkInsertState GetBulkInsertState(void);
+extern BulkInsertState GetBulkInsertState(HTAB *);
extern void FreeBulkInsertState(BulkInsertState);
extern void ReleaseBulkInsertStatePin(BulkInsertState bistate);
diff --git a/src/include/access/hio.h b/src/include/access/hio.h
index 9993d5be70..b491bf6338 100644
--- a/src/include/access/hio.h
+++ b/src/include/access/hio.h
@@ -32,6 +32,8 @@ typedef struct BulkInsertStateData
{
BufferAccessStrategy strategy; /* our BULKWRITE strategy object */
Buffer current_buf; /* current insertion target page */
+ HTAB *preserved_am_info; /* hash table with preserved compression
+ * methods for attributes */
} BulkInsertStateData;
diff --git a/src/include/access/htup_details.h b/src/include/access/htup_details.h
index 1867a70f6f..9e255154ad 100644
--- a/src/include/access/htup_details.h
+++ b/src/include/access/htup_details.h
@@ -273,7 +273,9 @@ struct HeapTupleHeaderData
* information stored in t_infomask2:
*/
#define HEAP_NATTS_MASK 0x07FF /* 11 bits for number of attributes */
-/* bits 0x1800 are available */
+/* bit 0x800 is available */
+#define HEAP_HASCUSTOMCOMPRESSED 0x1000 /* tuple contains custom compressed
+ * varlena(s) */
#define HEAP_KEYS_UPDATED 0x2000 /* tuple was updated and key cols
* modified, or tuple deleted */
#define HEAP_HOT_UPDATED 0x4000 /* tuple was HOT-updated */
@@ -686,6 +688,9 @@ struct MinimalTupleData
#define HeapTupleHasExternal(tuple) \
(((tuple)->t_data->t_infomask & HEAP_HASEXTERNAL) != 0)
+#define HeapTupleHasCustomCompressed(tuple) \
+ (((tuple)->t_data->t_infomask2 & HEAP_HASCUSTOMCOMPRESSED) != 0)
+
#define HeapTupleIsHotUpdated(tuple) \
HeapTupleHeaderIsHotUpdated((tuple)->t_data)
@@ -801,7 +806,7 @@ extern Size heap_compute_data_size(TupleDesc tupleDesc,
extern void heap_fill_tuple(TupleDesc tupleDesc,
Datum *values, bool *isnull,
char *data, Size data_size,
- uint16 *infomask, bits8 *bit);
+ uint16 *infomask, uint16 *infomask2, bits8 *bit);
extern bool heap_attisnull(HeapTuple tup, int attnum, TupleDesc tupleDesc);
extern Datum nocachegetattr(HeapTuple tup, int attnum,
TupleDesc att);
diff --git a/src/include/access/tupdesc.h b/src/include/access/tupdesc.h
index 708160f645..5bac03168a 100644
--- a/src/include/access/tupdesc.h
+++ b/src/include/access/tupdesc.h
@@ -14,7 +14,9 @@
#ifndef TUPDESC_H
#define TUPDESC_H
+#include "postgres.h"
#include "access/attnum.h"
+#include "access/cmapi.h"
#include "catalog/pg_attribute.h"
#include "nodes/pg_list.h"
@@ -46,6 +48,10 @@ typedef struct tupleConstr
bool has_not_null;
} TupleConstr;
+/* tupledesc flags */
+#define TD_ATTR_CUSTOM_COMPRESSED 0x01 /* is TupleDesc contain attributes
+ * with custom compression? */
+
/*
* This struct is passed around within the backend to describe the structure
* of tuples. For tuples coming from on-disk relations, the information is
@@ -83,6 +89,7 @@ typedef struct tupleDesc
Oid tdtypeid; /* composite type ID for tuple type */
int32 tdtypmod; /* typmod for tuple type */
bool tdhasoid; /* tuple has oid attribute in its header */
+ char tdflags; /* tuple additional flags */
int tdrefcount; /* reference count, or -1 if not counting */
TupleConstr *constr; /* constraints, or NULL if none */
/* attrs[N] is the description of Attribute Number N+1 */
diff --git a/src/include/access/tuptoaster.h b/src/include/access/tuptoaster.h
index 793367ff58..50f20611ab 100644
--- a/src/include/access/tuptoaster.h
+++ b/src/include/access/tuptoaster.h
@@ -13,7 +13,6 @@
#ifndef TUPTOASTER_H
#define TUPTOASTER_H
-#include "access/cmapi.h"
#include "access/htup_details.h"
#include "storage/lockdefs.h"
#include "utils/relcache.h"
diff --git a/src/include/commands/event_trigger.h b/src/include/commands/event_trigger.h
index 0e1959462e..4fc2ed8c68 100644
--- a/src/include/commands/event_trigger.h
+++ b/src/include/commands/event_trigger.h
@@ -32,6 +32,7 @@ typedef struct EventTriggerData
#define AT_REWRITE_DEFAULT_VAL 0x02
#define AT_REWRITE_COLUMN_REWRITE 0x04
#define AT_REWRITE_ALTER_OID 0x08
+#define AT_REWRITE_ALTER_COMPRESSION 0x10
/*
* EventTriggerData is the node type that is passed as fmgr "context" info
--
2.18.0
--MP_/w9zXrmKKcGlmI.IAcoy33yI
Content-Type: text/x-patch
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename=0004-Add-pglz-compression-method-v17.patch
^ permalink raw reply [nested|flat] 7+ messages in thread
* [PoC] Add CANONICAL option to xmlserialize
@ 2023-02-27 13:16 Jim Jones <[email protected]>
2023-03-05 18:44 ` [PATCH] Add CANONICAL option to xmlserialize Jim Jones <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Jim Jones @ 2023-02-27 13:16 UTC (permalink / raw)
To: PostgreSQL Hackers <[email protected]>
Hi,
In order to compare pairs of XML documents for equivalence it is
necessary to convert them first to their canonical form, as described at
W3C Canonical XML 1.1.[1] This spec basically defines a standard
physical representation of xml documents that have more then one
possible representation, so that it is possible to compare them, e.g.
forcing UTF-8 encoding, entity reference replacement, attributes
normalization, etc.
Although it is not part of the XML/SQL standard, it would be nice to
have the option CANONICAL in xmlserialize. Additionally, we could also
add the attribute WITH [NO] COMMENTS to keep or remove xml comments from
the documents.
Something like this:
WITH t(col) AS (
VALUES
('<?xml version="1.0" encoding="ISO-8859-1"?>
<!DOCTYPE doc SYSTEM "doc.dtd" [
<!ENTITY val "42">
<!ATTLIST xyz attr CDATA "default">
]>
<!-- ordering of attributes -->
<foo ns:c = "3" ns:b = "2" ns:a = "1"
xmlns:ns="http://postgresql.org";
<!-- Normalization of whitespace in start and end tags -->
<!-- Elimination of superfluous namespace declarations,
as already declared in <foo> -->
<bar xmlns:ns="http://postgresql.org"; >&val;</bar >
<!-- Empty element conversion to start-end tag pair -->
<empty/>
<!-- Effect of transcoding from a sample encoding to UTF-8 -->
<iso8859>©</iso8859>
<!-- Addition of default attribute -->
<!-- Whitespace inside tag preserved -->
<xyz> 321 </xyz>
</foo>
<!-- comment outside doc -->'::xml)
)
SELECT xmlserialize(DOCUMENT col AS text CANONICAL) FROM t;
xmlserialize
--------------------------------------------------------------------------------------------------------------------------------------------------------
<foo xmlns:ns="http://postgresql.org"; ns:a="1" ns:b="2"
ns:c="3"><bar>42</bar><empty></empty><iso8859>©</iso8859><xyz
attr="default"> 321 </xyz></foo>
(1 row)
-- using WITH COMMENTS
WITH t(col) AS (
VALUES
(' <foo ns:c = "3" ns:b = "2" ns:a = "1"
xmlns:ns="http://postgresql.org";
<!-- very important comment -->
<xyz> 321 </xyz>
</foo>'::xml)
)
SELECT xmlserialize(DOCUMENT col AS text CANONICAL WITH COMMENTS) FROM t;
xmlserialize
------------------------------------------------------------------------------------------------------------------------
<foo xmlns:ns="http://postgresql.org"; ns:a="1" ns:b="2" ns:c="3"><!--
very important comment --><xyz> 321 </xyz></foo>
(1 row)
Another option would be to simply create a new function, e.g.
xmlcanonical(doc xml, keep_comments boolean), but I'm not sure if this
would be the right approach.
Attached a very short draft. What do you think?
Best, Jim
1- https://www.w3.org/TR/xml-c14n11/
Attachments:
[text/x-patch] Add-CANONICAL-Option-to-xmlserialize-draft.patch (8.4K, ../../[email protected]/2-Add-CANONICAL-Option-to-xmlserialize-draft.patch)
download | inline diff:
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index 19351fe34b..f8f10f0ed9 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -3829,6 +3829,8 @@ ExecEvalXmlExpr(ExprState *state, ExprEvalStep *op)
{
Datum *argvalue = op->d.xmlexpr.argvalue;
bool *argnull = op->d.xmlexpr.argnull;
+ XmlSerializeFormat format = op->d.xmlexpr.xexpr->format;
+ text *data;
/* argument type is known to be xml */
Assert(list_length(xexpr->args) == 1);
@@ -3837,9 +3839,15 @@ ExecEvalXmlExpr(ExprState *state, ExprEvalStep *op)
return;
value = argvalue[0];
- *op->resvalue = PointerGetDatum(xmltotext_with_xmloption(DatumGetXmlP(value),
- xexpr->xmloption));
*op->resnull = false;
+
+ data = xmltotext_with_xmloption(DatumGetXmlP(value),
+ xexpr->xmloption);
+
+ if (format == XMLDEFAULT_FORMAT)
+ *op->resvalue = PointerGetDatum(data);
+ else if (format == XMLCANONICAL || format == XMLCANONICAL_WITH_COMMENTS)
+ *op->resvalue = PointerGetDatum(xmlserialize_canonical(data,format));
}
break;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index a0138382a1..af5f3dfdfd 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -619,6 +619,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <defelt> xmltable_column_option_el
%type <list> xml_namespace_list
%type <target> xml_namespace_el
+%type <ival> opt_xml_serialize_format
%type <node> func_application func_expr_common_subexpr
%type <node> func_expr func_expr_windowless
@@ -676,7 +677,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
BACKWARD BEFORE BEGIN_P BETWEEN BIGINT BINARY BIT
BOOLEAN_P BOTH BREADTH BY
- CACHE CALL CALLED CASCADE CASCADED CASE CAST CATALOG_P CHAIN CHAR_P
+ CACHE CALL CALLED CANONICAL CASCADE CASCADED CASE CAST CATALOG_P CHAIN CHAR_P
CHARACTER CHARACTERISTICS CHECK CHECKPOINT CLASS CLOSE
CLUSTER COALESCE COLLATE COLLATION COLUMN COLUMNS COMMENT COMMENTS COMMIT
COMMITTED COMPRESSION CONCURRENTLY CONFIGURATION CONFLICT
@@ -15532,13 +15533,14 @@ func_expr_common_subexpr:
$$ = makeXmlExpr(IS_XMLROOT, NULL, NIL,
list_make3($3, $5, $6), @1);
}
- | XMLSERIALIZE '(' document_or_content a_expr AS SimpleTypename ')'
+ | XMLSERIALIZE '(' document_or_content a_expr AS SimpleTypename opt_xml_serialize_format ')'
{
XmlSerialize *n = makeNode(XmlSerialize);
n->xmloption = $3;
n->expr = $4;
n->typeName = $6;
+ n->format = $7;
n->location = @1;
$$ = (Node *) n;
}
@@ -15622,6 +15624,12 @@ xml_passing_mech:
| BY VALUE_P
;
+opt_xml_serialize_format:
+ CANONICAL { $$ = XMLCANONICAL; }
+ | CANONICAL WITH NO COMMENTS { $$ = XMLCANONICAL; }
+ | CANONICAL WITH COMMENTS { $$ = XMLCANONICAL_WITH_COMMENTS; }
+ | /*EMPTY*/ { $$ = XMLDEFAULT_FORMAT; }
+ ;
/*
* Aggregate decoration clauses
@@ -16737,6 +16745,7 @@ unreserved_keyword:
| CACHE
| CALL
| CALLED
+ | CANONICAL
| CASCADE
| CASCADED
| CATALOG_P
@@ -17259,6 +17268,7 @@ bare_label_keyword:
| CACHE
| CALL
| CALLED
+ | CANONICAL
| CASCADE
| CASCADED
| CASE
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 7ff41acb84..ddfbfe259d 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -2332,6 +2332,7 @@ transformXmlSerialize(ParseState *pstate, XmlSerialize *xs)
xexpr->xmloption = xs->xmloption;
xexpr->location = xs->location;
+ xexpr->format = xs->format;
/* We actually only need these to be able to parse back the expression. */
xexpr->type = targetType;
xexpr->typmod = targetTypmod;
diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index 079bcb1208..f5c4ee520c 100644
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -56,6 +56,7 @@
#include <libxml/xmlwriter.h>
#include <libxml/xpath.h>
#include <libxml/xpathInternals.h>
+#include <libxml/c14n.h>
/*
* We used to check for xmlStructuredErrorContext via a configure test; but
@@ -4818,3 +4819,59 @@ XmlTableDestroyOpaque(TableFuncScanState *state)
NO_XML_SUPPORT();
#endif /* not USE_LIBXML */
}
+
+xmltype *
+xmlserialize_canonical(text *data, XmlSerializeFormat format)
+{
+#ifdef USE_LIBXML
+
+ xmlDocPtr doc;
+ xmlChar *xmlbuf = NULL;
+ int nbytes;
+ StringInfoData buf;
+
+ if (format != XMLCANONICAL && format != XMLCANONICAL_WITH_COMMENTS)
+ elog(ERROR,"invalid canonical xml option");
+
+ doc = xml_parse(data, XMLOPTION_DOCUMENT, false, GetDatabaseEncoding(), NULL);
+
+ if(!doc)
+ elog(ERROR, "could not parse the given XML document");
+
+ /*
+ * int
+ * xmlC14NDocDumpMemory (
+ * xmlDocPtr doc, # the XML document for canonization
+ * xmlNodeSetPtr nodes, # the nodes set to be included in the canonized image
+ * or NULL if all document nodes should be included
+ * int mode, # 0 = Original C14N 1.0 (Outdated)
+ * 1 = Exclusive C14N 1.0 (Outdated)
+ * 2 = C14N 1.1
+ * xmlChar **inclusive_ns_prefixes, # the list of inclusive namespace prefixes ended with
+ * a NULL or NULL if there is no inclusive namespaces
+ * (only for exclusive canonicalization, ignored otherwise)
+ * int with_comments, # include comments in the result (!=0) or not (==0)
+ * xmlChar **xmlbuf # the memory pointer for allocated canonical XML text;
+ * )
+ * Returns: the number of bytes written on success or a negative value on fail.
+ */
+
+ nbytes = xmlC14NDocDumpMemory(doc, NULL, 2, NULL, format, &xmlbuf);
+
+ xmlFreeDoc(doc);
+
+ if(nbytes < 0)
+ elog(ERROR,"could not canonicalize the given XML document");
+
+ initStringInfo(&buf);
+ appendStringInfoString(&buf, (const char *) xmlbuf);
+
+ xmlFree(xmlbuf);
+
+ return stringinfo_to_xmltype(&buf);
+
+#else
+ NO_XML_SUPPORT();
+ return 0;
+#endif
+}
\ No newline at end of file
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index f7d7f10f7d..c75fec17ba 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -842,6 +842,7 @@ typedef struct XmlSerialize
Node *expr;
TypeName *typeName;
int location; /* token location, or -1 if unknown */
+ XmlSerializeFormat format;
} XmlSerialize;
/* Partitioning related definitions */
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index b4292253cc..f36d79fb14 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -1471,6 +1471,13 @@ typedef enum XmlOptionType
XMLOPTION_CONTENT
} XmlOptionType;
+typedef enum XmlSerializeFormat
+{
+ XMLCANONICAL,
+ XMLCANONICAL_WITH_COMMENTS,
+ XMLDEFAULT_FORMAT
+} XmlSerializeFormat;
+
typedef struct XmlExpr
{
Expr xpr;
@@ -1491,6 +1498,8 @@ typedef struct XmlExpr
int32 typmod pg_node_attr(query_jumble_ignore);
/* token location, or -1 if unknown */
int location;
+ /* serialization format: XMLCANONICAL, XMLCANONICAL_WITH_COMMENTS, XMLINDENT */
+ XmlSerializeFormat format pg_node_attr(query_jumble_ignore);
} XmlExpr;
/* ----------------
diff --git a/src/include/parser/kwlist.h b/src/include/parser/kwlist.h
index bb36213e6f..c1b1a720fe 100644
--- a/src/include/parser/kwlist.h
+++ b/src/include/parser/kwlist.h
@@ -67,6 +67,7 @@ PG_KEYWORD("by", BY, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("cache", CACHE, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("call", CALL, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("called", CALLED, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("canonical", CANONICAL, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("cascade", CASCADE, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("cascaded", CASCADED, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("case", CASE, RESERVED_KEYWORD, BARE_LABEL)
diff --git a/src/include/utils/xml.h b/src/include/utils/xml.h
index 311da06cd6..745ebefe24 100644
--- a/src/include/utils/xml.h
+++ b/src/include/utils/xml.h
@@ -90,4 +90,5 @@ extern PGDLLIMPORT int xmloption; /* XmlOptionType, but int for guc enum */
extern PGDLLIMPORT const TableFuncRoutine XmlTableRoutine;
+xmltype *xmlserialize_canonical(text *data, XmlSerializeFormat format);
#endif /* XML_H */
^ permalink raw reply [nested|flat] 7+ messages in thread
* [PATCH] Add CANONICAL option to xmlserialize
2023-02-27 13:16 [PoC] Add CANONICAL option to xmlserialize Jim Jones <[email protected]>
@ 2023-03-05 18:44 ` Jim Jones <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Jim Jones @ 2023-03-05 18:44 UTC (permalink / raw)
To: PostgreSQL Hackers <[email protected]>
On 27.02.23 14:16, I wrote:
> Hi,
>
> In order to compare pairs of XML documents for equivalence it is
> necessary to convert them first to their canonical form, as described
> at W3C Canonical XML 1.1.[1] This spec basically defines a standard
> physical representation of xml documents that have more then one
> possible representation, so that it is possible to compare them, e.g.
> forcing UTF-8 encoding, entity reference replacement, attributes
> normalization, etc.
>
> Although it is not part of the XML/SQL standard, it would be nice to
> have the option CANONICAL in xmlserialize. Additionally, we could also
> add the attribute WITH [NO] COMMENTS to keep or remove xml comments
> from the documents.
>
> Something like this:
>
> WITH t(col) AS (
> VALUES
> ('<?xml version="1.0" encoding="ISO-8859-1"?>
> <!DOCTYPE doc SYSTEM "doc.dtd" [
> <!ENTITY val "42">
> <!ATTLIST xyz attr CDATA "default">
> ]>
>
> <!-- ordering of attributes -->
> <foo ns:c = "3" ns:b = "2" ns:a = "1"
> xmlns:ns="http://postgresql.org";
>
> <!-- Normalization of whitespace in start and end tags -->
> <!-- Elimination of superfluous namespace declarations,
> as already declared in <foo> -->
> <bar xmlns:ns="http://postgresql.org"; >&val;</bar >
>
> <!-- Empty element conversion to start-end tag pair -->
> <empty/>
>
> <!-- Effect of transcoding from a sample encoding to UTF-8 -->
> <iso8859>©</iso8859>
>
> <!-- Addition of default attribute -->
> <!-- Whitespace inside tag preserved -->
> <xyz> 321 </xyz>
> </foo>
> <!-- comment outside doc -->'::xml)
> )
> SELECT xmlserialize(DOCUMENT col AS text CANONICAL) FROM t;
> xmlserialize
> --------------------------------------------------------------------------------------------------------------------------------------------------------
>
> <foo xmlns:ns="http://postgresql.org"; ns:a="1" ns:b="2"
> ns:c="3"><bar>42</bar><empty></empty><iso8859>©</iso8859><xyz
> attr="default"> 321 </xyz></foo>
> (1 row)
>
> -- using WITH COMMENTS
>
> WITH t(col) AS (
> VALUES
> (' <foo ns:c = "3" ns:b = "2" ns:a = "1"
> xmlns:ns="http://postgresql.org";
> <!-- very important comment -->
> <xyz> 321 </xyz>
> </foo>'::xml)
> )
> SELECT xmlserialize(DOCUMENT col AS text CANONICAL WITH COMMENTS) FROM t;
> xmlserialize
> ------------------------------------------------------------------------------------------------------------------------
>
> <foo xmlns:ns="http://postgresql.org"; ns:a="1" ns:b="2" ns:c="3"><!--
> very important comment --><xyz> 321 </xyz></foo>
> (1 row)
>
>
> Another option would be to simply create a new function, e.g.
> xmlcanonical(doc xml, keep_comments boolean), but I'm not sure if this
> would be the right approach.
>
> Attached a very short draft. What do you think?
>
> Best, Jim
>
> 1- https://www.w3.org/TR/xml-c14n11/
The attached version includes documentation and tests to the patch.
I hope things are clearer now :)
Best, Jim
Attachments:
[text/x-patch] v1-0001-Add-CANONICAL-format-to-xmlserialize.patch (31.6K, ../../[email protected]/2-v1-0001-Add-CANONICAL-format-to-xmlserialize.patch)
download | inline diff:
From 1a3b8bc66c451863ace0488d32f1c0a876ab8f04 Mon Sep 17 00:00:00 2001
From: Jim Jones <[email protected]>
Date: Tue, 28 Feb 2023 23:06:30 +0100
Subject: [PATCH v1] Add CANONICAL format to xmlserialize
This patch introduces the CANONICAL option to xmlserialize, which
serializes xml documents in their canonical form - as described in
the W3C Canonical XML Version 1.1 specification. This option can
be used with the additional parameter WITH [NO] COMMENTS to keep
or remove xml comments from the canonical xml output. This feature
is based on the function xmlC14NDocDumpMemory from the C14N module
of libxml2.
This patch also includes regression tests and documentation.
---
doc/src/sgml/datatype.sgml | 41 +++++++++-
src/backend/executor/execExprInterp.c | 12 ++-
src/backend/parser/gram.y | 14 +++-
src/backend/parser/parse_expr.c | 1 +
src/backend/utils/adt/xml.c | 60 ++++++++++++++
src/include/nodes/parsenodes.h | 1 +
src/include/nodes/primnodes.h | 9 +++
src/include/parser/kwlist.h | 1 +
src/include/utils/xml.h | 1 +
src/test/regress/expected/xml.out | 108 ++++++++++++++++++++++++++
src/test/regress/expected/xml_1.out | 104 +++++++++++++++++++++++++
src/test/regress/expected/xml_2.out | 108 ++++++++++++++++++++++++++
src/test/regress/sql/xml.sql | 61 +++++++++++++++
13 files changed, 516 insertions(+), 5 deletions(-)
diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml
index 467b49b199..46ec95dbb8 100644
--- a/doc/src/sgml/datatype.sgml
+++ b/doc/src/sgml/datatype.sgml
@@ -4460,7 +4460,7 @@ xml '<foo>bar</foo>'
<type>xml</type>, uses the function
<function>xmlserialize</function>:<indexterm><primary>xmlserialize</primary></indexterm>
<synopsis>
-XMLSERIALIZE ( { DOCUMENT | CONTENT } <replaceable>value</replaceable> AS <replaceable>type</replaceable> )
+XMLSERIALIZE ( { DOCUMENT | CONTENT } <replaceable>value</replaceable> AS <replaceable>type</replaceable> [ CANONICAL [ WITH [NO] COMMENTS ] ])
</synopsis>
<replaceable>type</replaceable> can be
<type>character</type>, <type>character varying</type>, or
@@ -4470,6 +4470,45 @@ XMLSERIALIZE ( { DOCUMENT | CONTENT } <replaceable>value</replaceable> AS <repla
you to simply cast the value.
</para>
+ <para>
+ The option <type>CANONICAL</type> converts a given
+ XML document to its <ulink url="https://www.w3.org/TR/xml-c14n11/#Terminology">canonical form</ulink>
+ based on the <ulink url="https://www.w3.org/TR/xml-c14n11/">W3C Canonical XML 1.1 Specification</ulink>.
+ It is basically designed to provide applications the ability to compare xml documents or test if they
+ have been changed. The optional parameter <type>WITH [NO] COMMENTS</type> removes or keeps XML comments
+ from the given document.
+ </para>
+
+ <para>
+ Example:
+
+<screen><![CDATA[
+SELECT
+ xmlserialize(DOCUMENT
+ '<foo>
+ <!-- a comment -->
+ <bar c="3" b="2" a="1">42</bar>
+ <empty/>
+ </foo>'::xml AS text CANONICAL);
+ xmlserialize
+-----------------------------------------------------------
+ <foo><bar a="1" b="2" c="3">42</bar><empty></empty></foo>
+(1 row)
+
+SELECT
+ xmlserialize(DOCUMENT
+ '<foo>
+ <!-- a comment -->
+ <bar c="3" b="2" a="1">42</bar>
+ <empty/>
+ </foo>'::xml AS text CANONICAL WITH COMMENTS);
+ xmlserialize
+-----------------------------------------------------------------------------
+ <foo><!-- a comment --><bar a="1" b="2" c="3">42</bar><empty></empty></foo>
+(1 row)
+
+]]></screen>
+ </para>
<para>
When a character string value is cast to or from type
<type>xml</type> without going through <type>XMLPARSE</type> or
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index 19351fe34b..f8f10f0ed9 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -3829,6 +3829,8 @@ ExecEvalXmlExpr(ExprState *state, ExprEvalStep *op)
{
Datum *argvalue = op->d.xmlexpr.argvalue;
bool *argnull = op->d.xmlexpr.argnull;
+ XmlSerializeFormat format = op->d.xmlexpr.xexpr->format;
+ text *data;
/* argument type is known to be xml */
Assert(list_length(xexpr->args) == 1);
@@ -3837,9 +3839,15 @@ ExecEvalXmlExpr(ExprState *state, ExprEvalStep *op)
return;
value = argvalue[0];
- *op->resvalue = PointerGetDatum(xmltotext_with_xmloption(DatumGetXmlP(value),
- xexpr->xmloption));
*op->resnull = false;
+
+ data = xmltotext_with_xmloption(DatumGetXmlP(value),
+ xexpr->xmloption);
+
+ if (format == XMLDEFAULT_FORMAT)
+ *op->resvalue = PointerGetDatum(data);
+ else if (format == XMLCANONICAL || format == XMLCANONICAL_WITH_COMMENTS)
+ *op->resvalue = PointerGetDatum(xmlserialize_canonical(data,format));
}
break;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index a0138382a1..af5f3dfdfd 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -619,6 +619,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <defelt> xmltable_column_option_el
%type <list> xml_namespace_list
%type <target> xml_namespace_el
+%type <ival> opt_xml_serialize_format
%type <node> func_application func_expr_common_subexpr
%type <node> func_expr func_expr_windowless
@@ -676,7 +677,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
BACKWARD BEFORE BEGIN_P BETWEEN BIGINT BINARY BIT
BOOLEAN_P BOTH BREADTH BY
- CACHE CALL CALLED CASCADE CASCADED CASE CAST CATALOG_P CHAIN CHAR_P
+ CACHE CALL CALLED CANONICAL CASCADE CASCADED CASE CAST CATALOG_P CHAIN CHAR_P
CHARACTER CHARACTERISTICS CHECK CHECKPOINT CLASS CLOSE
CLUSTER COALESCE COLLATE COLLATION COLUMN COLUMNS COMMENT COMMENTS COMMIT
COMMITTED COMPRESSION CONCURRENTLY CONFIGURATION CONFLICT
@@ -15532,13 +15533,14 @@ func_expr_common_subexpr:
$$ = makeXmlExpr(IS_XMLROOT, NULL, NIL,
list_make3($3, $5, $6), @1);
}
- | XMLSERIALIZE '(' document_or_content a_expr AS SimpleTypename ')'
+ | XMLSERIALIZE '(' document_or_content a_expr AS SimpleTypename opt_xml_serialize_format ')'
{
XmlSerialize *n = makeNode(XmlSerialize);
n->xmloption = $3;
n->expr = $4;
n->typeName = $6;
+ n->format = $7;
n->location = @1;
$$ = (Node *) n;
}
@@ -15622,6 +15624,12 @@ xml_passing_mech:
| BY VALUE_P
;
+opt_xml_serialize_format:
+ CANONICAL { $$ = XMLCANONICAL; }
+ | CANONICAL WITH NO COMMENTS { $$ = XMLCANONICAL; }
+ | CANONICAL WITH COMMENTS { $$ = XMLCANONICAL_WITH_COMMENTS; }
+ | /*EMPTY*/ { $$ = XMLDEFAULT_FORMAT; }
+ ;
/*
* Aggregate decoration clauses
@@ -16737,6 +16745,7 @@ unreserved_keyword:
| CACHE
| CALL
| CALLED
+ | CANONICAL
| CASCADE
| CASCADED
| CATALOG_P
@@ -17259,6 +17268,7 @@ bare_label_keyword:
| CACHE
| CALL
| CALLED
+ | CANONICAL
| CASCADE
| CASCADED
| CASE
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 7ff41acb84..ddfbfe259d 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -2332,6 +2332,7 @@ transformXmlSerialize(ParseState *pstate, XmlSerialize *xs)
xexpr->xmloption = xs->xmloption;
xexpr->location = xs->location;
+ xexpr->format = xs->format;
/* We actually only need these to be able to parse back the expression. */
xexpr->type = targetType;
xexpr->typmod = targetTypmod;
diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index 079bcb1208..e0119d5ce6 100644
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -56,6 +56,7 @@
#include <libxml/xmlwriter.h>
#include <libxml/xpath.h>
#include <libxml/xpathInternals.h>
+#include <libxml/c14n.h>
/*
* We used to check for xmlStructuredErrorContext via a configure test; but
@@ -4818,3 +4819,62 @@ XmlTableDestroyOpaque(TableFuncScanState *state)
NO_XML_SUPPORT();
#endif /* not USE_LIBXML */
}
+
+xmltype *
+xmlserialize_canonical(text *data, XmlSerializeFormat format)
+{
+#ifdef USE_LIBXML
+
+ xmlDocPtr doc;
+ xmlChar *xmlbuf = NULL;
+ int nbytes;
+ int with_comments = 0; /* 0 = remove xml comments (default) */
+ StringInfoData buf;
+
+ if (format != XMLCANONICAL && format != XMLCANONICAL_WITH_COMMENTS)
+ elog(ERROR,"invalid canonical xml option");
+ else if (format == XMLCANONICAL_WITH_COMMENTS)
+ with_comments = 1;
+
+ doc = xml_parse(data, XMLOPTION_DOCUMENT, false, GetDatabaseEncoding(), NULL);
+
+ if(!doc)
+ elog(ERROR, "could not parse the given XML document");
+
+ /*
+ * int
+ * xmlC14NDocDumpMemory (
+ * xmlDocPtr doc, # the XML document for canonization
+ * xmlNodeSetPtr nodes, # the nodes set to be included in the canonized image
+ * or NULL if all document nodes should be included
+ * int mode, # 0 = Original C14N 1.0 (Outdated)
+ * 1 = Exclusive C14N 1.0 (Outdated)
+ * 2 = C14N 1.1
+ * xmlChar **inclusive_ns_prefixes, # the list of inclusive namespace prefixes ended with
+ * a NULL or NULL if there is no inclusive namespaces
+ * (only for exclusive canonicalization, ignored otherwise)
+ * int with_comments, # include comments in the result (!=0) or not (==0)
+ * xmlChar **xmlbuf # the memory pointer for allocated canonical XML text;
+ * )
+ * Returns: the number of bytes written on success or a negative value on fail.
+ */
+
+ nbytes = xmlC14NDocDumpMemory(doc, NULL, 2, NULL, with_comments, &xmlbuf);
+
+ xmlFreeDoc(doc);
+
+ if(nbytes < 0)
+ elog(ERROR,"could not canonicalize the given XML document");
+
+ initStringInfo(&buf);
+ appendStringInfoString(&buf, (const char *) xmlbuf);
+
+ xmlFree(xmlbuf);
+
+ return stringinfo_to_xmltype(&buf);
+
+#else
+ NO_XML_SUPPORT();
+ return 0;
+#endif
+}
\ No newline at end of file
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index f7d7f10f7d..8ba0984266 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -842,6 +842,7 @@ typedef struct XmlSerialize
Node *expr;
TypeName *typeName;
int location; /* token location, or -1 if unknown */
+ XmlSerializeFormat format; /* serialization format */
} XmlSerialize;
/* Partitioning related definitions */
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index b4292253cc..79fdcc2650 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -1471,6 +1471,13 @@ typedef enum XmlOptionType
XMLOPTION_CONTENT
} XmlOptionType;
+typedef enum XmlSerializeFormat
+{
+ XMLCANONICAL, /* canonical form without xml comments */
+ XMLCANONICAL_WITH_COMMENTS, /* canonical form with xml comments */
+ XMLDEFAULT_FORMAT /* unformatted xml representation */
+} XmlSerializeFormat;
+
typedef struct XmlExpr
{
Expr xpr;
@@ -1491,6 +1498,8 @@ typedef struct XmlExpr
int32 typmod pg_node_attr(query_jumble_ignore);
/* token location, or -1 if unknown */
int location;
+ /* serialization format: XMLCANONICAL, XMLCANONICAL_WITH_COMMENTS, XMLINDENT */
+ XmlSerializeFormat format pg_node_attr(query_jumble_ignore);
} XmlExpr;
/* ----------------
diff --git a/src/include/parser/kwlist.h b/src/include/parser/kwlist.h
index bb36213e6f..c1b1a720fe 100644
--- a/src/include/parser/kwlist.h
+++ b/src/include/parser/kwlist.h
@@ -67,6 +67,7 @@ PG_KEYWORD("by", BY, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("cache", CACHE, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("call", CALL, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("called", CALLED, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("canonical", CANONICAL, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("cascade", CASCADE, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("cascaded", CASCADED, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("case", CASE, RESERVED_KEYWORD, BARE_LABEL)
diff --git a/src/include/utils/xml.h b/src/include/utils/xml.h
index 311da06cd6..745ebefe24 100644
--- a/src/include/utils/xml.h
+++ b/src/include/utils/xml.h
@@ -90,4 +90,5 @@ extern PGDLLIMPORT int xmloption; /* XmlOptionType, but int for guc enum */
extern PGDLLIMPORT const TableFuncRoutine XmlTableRoutine;
+xmltype *xmlserialize_canonical(text *data, XmlSerializeFormat format);
#endif /* XML_H */
diff --git a/src/test/regress/expected/xml.out b/src/test/regress/expected/xml.out
index 3c357a9c7e..de3bfabcef 100644
--- a/src/test/regress/expected/xml.out
+++ b/src/test/regress/expected/xml.out
@@ -486,6 +486,114 @@ SELECT xmlserialize(content 'good' as char(10));
SELECT xmlserialize(document 'bad' as text);
ERROR: not an XML document
+-- xmlserialize: canonical
+CREATE TABLE xmltest_serialize (id int, doc xml);
+INSERT INTO xmltest_serialize VALUES
+ (1,'<?xml version="1.0" encoding="ISO-8859-1"?>
+ <!DOCTYPE doc SYSTEM "doc.dtd" [
+ <!ENTITY val "42">
+ <!ATTLIST xyz attr CDATA "default">
+ ]>
+
+ <!-- attributes and namespces will be sorted -->
+ <foo a:attr="out" b:attr="sorted" attr2="all" attr="I am"
+ xmlns:b="http://www.ietf.org"
+ xmlns:a="http://www.w3.org"
+ xmlns="http://example.org">
+
+ <!-- Normalization of whitespace in start and end tags -->
+ <!-- Elimination of superfluous namespace declarations, as already declared in <foo> -->
+ <bar xmlns="" xmlns:a="http://www.w3.org" >&val;</bar >
+
+ <!-- empty element will be converted to start-end tag pair -->
+ <empty/>
+
+ <!-- text will be transcoded to UTF-8 -->
+ <transcode>£1</transcode>
+
+ <!-- default attribute will be added -->
+ <!-- whitespace inside tag will be preserved -->
+ <whitespace> 321 </whitespace>
+
+ <!-- empty namespace will be removed of child tag -->
+ <emptyns xmlns="" >
+ <emptyns_child xmlns=""></emptyns_child>
+ </emptyns>
+
+ <!-- CDATA section will be replaced by its value -->
+ <compute><![CDATA[value>"0" && value<"10" ?"valid":"error"]]></compute>
+ </foo>
+ <!-- comment outside doc -->'::xml),
+ (2,'<foo>
+ <bar>
+ <!-- important comment -->
+ <val x="y">42</val>
+ </bar>
+ </foo> '::xml);
+SELECT xmlserialize(DOCUMENT doc AS text CANONICAL) FROM xmltest_serialize WHERE id = 1;
+ xmlserialize
+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ <foo xmlns="http://example.org" xmlns:a="http://www.w3.org" xmlns:b="http://www.ietf.org" attr="I am" attr2="all" b:attr="sorted" a:attr="out"><bar xmlns="">42</bar><empty></empty><transcode>£1</transcode><whitespace> 321 </whitespace><emptyns xmlns=""><emptyns_child></emptyns_child></emptyns><compute>value>"0" && value<"10" ?"valid":"error"</compute></foo>
+(1 row)
+
+SELECT xmlserialize(DOCUMENT doc AS text CANONICAL WITH COMMENTS) FROM xmltest_serialize WHERE id = 2;
+ xmlserialize
+---------------------------------------------------------------------
+ <foo><bar><!-- important comment --><val x="y">42</val></bar></foo>
+(1 row)
+
+SELECT xmlserialize(DOCUMENT doc AS text CANONICAL) = xmlserialize(DOCUMENT doc AS text CANONICAL WITH NO COMMENTS) FROM xmltest_serialize;
+ ?column?
+----------
+ t
+ t
+(2 rows)
+
+SELECT xmlserialize(CONTENT doc AS text CANONICAL) FROM xmltest_serialize WHERE id = 1;
+ xmlserialize
+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ <foo xmlns="http://example.org" xmlns:a="http://www.w3.org" xmlns:b="http://www.ietf.org" attr="I am" attr2="all" b:attr="sorted" a:attr="out"><bar xmlns="">42</bar><empty></empty><transcode>£1</transcode><whitespace> 321 </whitespace><emptyns xmlns=""><emptyns_child></emptyns_child></emptyns><compute>value>"0" && value<"10" ?"valid":"error"</compute></foo>
+(1 row)
+
+SELECT xmlserialize(CONTENT doc AS text CANONICAL WITH COMMENTS) FROM xmltest_serialize WHERE id = 2;
+ xmlserialize
+---------------------------------------------------------------------
+ <foo><bar><!-- important comment --><val x="y">42</val></bar></foo>
+(1 row)
+
+SELECT xmlserialize(CONTENT doc AS text CANONICAL) = xmlserialize(CONTENT doc AS text CANONICAL WITH NO COMMENTS) FROM xmltest_serialize;
+ ?column?
+----------
+ t
+ t
+(2 rows)
+
+SELECT xmlserialize(DOCUMENT NULL AS text CANONICAL);
+ xmlserialize
+--------------
+
+(1 row)
+
+SELECT xmlserialize(CONTENT NULL AS text CANONICAL);
+ xmlserialize
+--------------
+
+(1 row)
+
+\set VERBOSITY terse
+SELECT xmlserialize(DOCUMENT '' AS text CANONICAL);
+ERROR: not an XML document
+SELECT xmlserialize(DOCUMENT ' ' AS text CANONICAL);
+ERROR: not an XML document
+SELECT xmlserialize(DOCUMENT 'foo' AS text CANONICAL);
+ERROR: not an XML document
+SELECT xmlserialize(CONTENT '' AS text CANONICAL);
+ERROR: invalid XML document
+SELECT xmlserialize(CONTENT ' ' AS text CANONICAL);
+ERROR: invalid XML document
+SELECT xmlserialize(CONTENT 'foo' AS text CANONICAL);
+ERROR: invalid XML document
+\set VERBOSITY default
SELECT xml '<foo>bar</foo>' IS DOCUMENT;
?column?
----------
diff --git a/src/test/regress/expected/xml_1.out b/src/test/regress/expected/xml_1.out
index 378b412db0..8b5be34c50 100644
--- a/src/test/regress/expected/xml_1.out
+++ b/src/test/regress/expected/xml_1.out
@@ -309,6 +309,110 @@ ERROR: unsupported XML feature
LINE 1: SELECT xmlserialize(document 'bad' as text);
^
DETAIL: This functionality requires the server to be built with libxml support.
+-- xmlserialize: canonical
+CREATE TABLE xmltest_serialize (id int, doc xml);
+INSERT INTO xmltest_serialize VALUES
+ (1,'<?xml version="1.0" encoding="ISO-8859-1"?>
+ <!DOCTYPE doc SYSTEM "doc.dtd" [
+ <!ENTITY val "42">
+ <!ATTLIST xyz attr CDATA "default">
+ ]>
+
+ <!-- attributes and namespces will be sorted -->
+ <foo a:attr="out" b:attr="sorted" attr2="all" attr="I am"
+ xmlns:b="http://www.ietf.org"
+ xmlns:a="http://www.w3.org"
+ xmlns="http://example.org">
+
+ <!-- Normalization of whitespace in start and end tags -->
+ <!-- Elimination of superfluous namespace declarations, as already declared in <foo> -->
+ <bar xmlns="" xmlns:a="http://www.w3.org" >&val;</bar >
+
+ <!-- empty element will be converted to start-end tag pair -->
+ <empty/>
+
+ <!-- text will be transcoded to UTF-8 -->
+ <transcode>£1</transcode>
+
+ <!-- default attribute will be added -->
+ <!-- whitespace inside tag will be preserved -->
+ <whitespace> 321 </whitespace>
+
+ <!-- empty namespace will be removed of child tag -->
+ <emptyns xmlns="" >
+ <emptyns_child xmlns=""></emptyns_child>
+ </emptyns>
+
+ <!-- CDATA section will be replaced by its value -->
+ <compute><![CDATA[value>"0" && value<"10" ?"valid":"error"]]></compute>
+ </foo>
+ <!-- comment outside doc -->'::xml),
+ (2,'<foo>
+ <bar>
+ <!-- important comment -->
+ <val x="y">42</val>
+ </bar>
+ </foo> '::xml);
+ERROR: unsupported XML feature
+LINE 2: (1,'<?xml version="1.0" encoding="ISO-8859-1"?>
+ ^
+DETAIL: This functionality requires the server to be built with libxml support.
+SELECT xmlserialize(DOCUMENT doc AS text CANONICAL) FROM xmltest_serialize WHERE id = 1;
+ xmlserialize
+--------------
+(0 rows)
+
+SELECT xmlserialize(DOCUMENT doc AS text CANONICAL WITH COMMENTS) FROM xmltest_serialize WHERE id = 2;
+ xmlserialize
+--------------
+(0 rows)
+
+SELECT xmlserialize(DOCUMENT doc AS text CANONICAL) = xmlserialize(DOCUMENT doc AS text CANONICAL WITH NO COMMENTS) FROM xmltest_serialize;
+ ?column?
+----------
+(0 rows)
+
+SELECT xmlserialize(CONTENT doc AS text CANONICAL) FROM xmltest_serialize WHERE id = 1;
+ xmlserialize
+--------------
+(0 rows)
+
+SELECT xmlserialize(CONTENT doc AS text CANONICAL WITH COMMENTS) FROM xmltest_serialize WHERE id = 2;
+ xmlserialize
+--------------
+(0 rows)
+
+SELECT xmlserialize(CONTENT doc AS text CANONICAL) = xmlserialize(CONTENT doc AS text CANONICAL WITH NO COMMENTS) FROM xmltest_serialize;
+ ?column?
+----------
+(0 rows)
+
+SELECT xmlserialize(DOCUMENT NULL AS text CANONICAL);
+ xmlserialize
+--------------
+
+(1 row)
+
+SELECT xmlserialize(CONTENT NULL AS text CANONICAL);
+ xmlserialize
+--------------
+
+(1 row)
+
+\set VERBOSITY terse
+SELECT xmlserialize(DOCUMENT '' AS text CANONICAL);
+ERROR: unsupported XML feature at character 30
+SELECT xmlserialize(DOCUMENT ' ' AS text CANONICAL);
+ERROR: unsupported XML feature at character 30
+SELECT xmlserialize(DOCUMENT 'foo' AS text CANONICAL);
+ERROR: unsupported XML feature at character 30
+SELECT xmlserialize(CONTENT '' AS text CANONICAL);
+ERROR: unsupported XML feature at character 29
+SELECT xmlserialize(CONTENT ' ' AS text CANONICAL);
+ERROR: unsupported XML feature at character 29
+SELECT xmlserialize(CONTENT 'foo' AS text CANONICAL);
+ERROR: unsupported XML feature at character 29
+\set VERBOSITY default
SELECT xml '<foo>bar</foo>' IS DOCUMENT;
ERROR: unsupported XML feature
LINE 1: SELECT xml '<foo>bar</foo>' IS DOCUMENT;
diff --git a/src/test/regress/expected/xml_2.out b/src/test/regress/expected/xml_2.out
index 42055c5003..9feeb9301e 100644
--- a/src/test/regress/expected/xml_2.out
+++ b/src/test/regress/expected/xml_2.out
@@ -466,6 +466,114 @@ SELECT xmlserialize(content 'good' as char(10));
SELECT xmlserialize(document 'bad' as text);
ERROR: not an XML document
+-- xmlserialize: canonical
+CREATE TABLE xmltest_serialize (id int, doc xml);
+INSERT INTO xmltest_serialize VALUES
+ (1,'<?xml version="1.0" encoding="ISO-8859-1"?>
+ <!DOCTYPE doc SYSTEM "doc.dtd" [
+ <!ENTITY val "42">
+ <!ATTLIST xyz attr CDATA "default">
+ ]>
+
+ <!-- attributes and namespces will be sorted -->
+ <foo a:attr="out" b:attr="sorted" attr2="all" attr="I am"
+ xmlns:b="http://www.ietf.org"
+ xmlns:a="http://www.w3.org"
+ xmlns="http://example.org">
+
+ <!-- Normalization of whitespace in start and end tags -->
+ <!-- Elimination of superfluous namespace declarations, as already declared in <foo> -->
+ <bar xmlns="" xmlns:a="http://www.w3.org" >&val;</bar >
+
+ <!-- empty element will be converted to start-end tag pair -->
+ <empty/>
+
+ <!-- text will be transcoded to UTF-8 -->
+ <transcode>£1</transcode>
+
+ <!-- default attribute will be added -->
+ <!-- whitespace inside tag will be preserved -->
+ <whitespace> 321 </whitespace>
+
+ <!-- empty namespace will be removed of child tag -->
+ <emptyns xmlns="" >
+ <emptyns_child xmlns=""></emptyns_child>
+ </emptyns>
+
+ <!-- CDATA section will be replaced by its value -->
+ <compute><![CDATA[value>"0" && value<"10" ?"valid":"error"]]></compute>
+ </foo>
+ <!-- comment outside doc -->'::xml),
+ (2,'<foo>
+ <bar>
+ <!-- important comment -->
+ <val x="y">42</val>
+ </bar>
+ </foo> '::xml);
+SELECT xmlserialize(DOCUMENT doc AS text CANONICAL) FROM xmltest_serialize WHERE id = 1;
+ xmlserialize
+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ <foo xmlns="http://example.org" xmlns:a="http://www.w3.org" xmlns:b="http://www.ietf.org" attr="I am" attr2="all" b:attr="sorted" a:attr="out"><bar xmlns="">42</bar><empty></empty><transcode>£1</transcode><whitespace> 321 </whitespace><emptyns xmlns=""><emptyns_child></emptyns_child></emptyns><compute>value>"0" && value<"10" ?"valid":"error"</compute></foo>
+(1 row)
+
+SELECT xmlserialize(DOCUMENT doc AS text CANONICAL WITH COMMENTS) FROM xmltest_serialize WHERE id = 2;
+ xmlserialize
+---------------------------------------------------------------------
+ <foo><bar><!-- important comment --><val x="y">42</val></bar></foo>
+(1 row)
+
+SELECT xmlserialize(DOCUMENT doc AS text CANONICAL) = xmlserialize(DOCUMENT doc AS text CANONICAL WITH NO COMMENTS) FROM xmltest_serialize;
+ ?column?
+----------
+ t
+ t
+(2 rows)
+
+SELECT xmlserialize(CONTENT doc AS text CANONICAL) FROM xmltest_serialize WHERE id = 1;
+ xmlserialize
+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ <foo xmlns="http://example.org" xmlns:a="http://www.w3.org" xmlns:b="http://www.ietf.org" attr="I am" attr2="all" b:attr="sorted" a:attr="out"><bar xmlns="">42</bar><empty></empty><transcode>£1</transcode><whitespace> 321 </whitespace><emptyns xmlns=""><emptyns_child></emptyns_child></emptyns><compute>value>"0" && value<"10" ?"valid":"error"</compute></foo>
+(1 row)
+
+SELECT xmlserialize(CONTENT doc AS text CANONICAL WITH COMMENTS) FROM xmltest_serialize WHERE id = 2;
+ xmlserialize
+---------------------------------------------------------------------
+ <foo><bar><!-- important comment --><val x="y">42</val></bar></foo>
+(1 row)
+
+SELECT xmlserialize(CONTENT doc AS text CANONICAL) = xmlserialize(CONTENT doc AS text CANONICAL WITH NO COMMENTS) FROM xmltest_serialize;
+ ?column?
+----------
+ t
+ t
+(2 rows)
+
+SELECT xmlserialize(DOCUMENT NULL AS text CANONICAL);
+ xmlserialize
+--------------
+
+(1 row)
+
+SELECT xmlserialize(CONTENT NULL AS text CANONICAL);
+ xmlserialize
+--------------
+
+(1 row)
+
+\set VERBOSITY terse
+SELECT xmlserialize(DOCUMENT '' AS text CANONICAL);
+ERROR: not an XML document
+SELECT xmlserialize(DOCUMENT ' ' AS text CANONICAL);
+ERROR: not an XML document
+SELECT xmlserialize(DOCUMENT 'foo' AS text CANONICAL);
+ERROR: not an XML document
+SELECT xmlserialize(CONTENT '' AS text CANONICAL);
+ERROR: invalid XML document
+SELECT xmlserialize(CONTENT ' ' AS text CANONICAL);
+ERROR: invalid XML document
+SELECT xmlserialize(CONTENT 'foo' AS text CANONICAL);
+ERROR: invalid XML document
+\set VERBOSITY default
SELECT xml '<foo>bar</foo>' IS DOCUMENT;
?column?
----------
diff --git a/src/test/regress/sql/xml.sql b/src/test/regress/sql/xml.sql
index ddff459297..32bc650a9d 100644
--- a/src/test/regress/sql/xml.sql
+++ b/src/test/regress/sql/xml.sql
@@ -132,6 +132,67 @@ SELECT xmlserialize(content data as character varying(20)) FROM xmltest;
SELECT xmlserialize(content 'good' as char(10));
SELECT xmlserialize(document 'bad' as text);
+-- xmlserialize: canonical
+CREATE TABLE xmltest_serialize (id int, doc xml);
+INSERT INTO xmltest_serialize VALUES
+ (1,'<?xml version="1.0" encoding="ISO-8859-1"?>
+ <!DOCTYPE doc SYSTEM "doc.dtd" [
+ <!ENTITY val "42">
+ <!ATTLIST xyz attr CDATA "default">
+ ]>
+
+ <!-- attributes and namespces will be sorted -->
+ <foo a:attr="out" b:attr="sorted" attr2="all" attr="I am"
+ xmlns:b="http://www.ietf.org"
+ xmlns:a="http://www.w3.org"
+ xmlns="http://example.org">
+
+ <!-- Normalization of whitespace in start and end tags -->
+ <!-- Elimination of superfluous namespace declarations, as already declared in <foo> -->
+ <bar xmlns="" xmlns:a="http://www.w3.org" >&val;</bar >
+
+ <!-- empty element will be converted to start-end tag pair -->
+ <empty/>
+
+ <!-- text will be transcoded to UTF-8 -->
+ <transcode>£1</transcode>
+
+ <!-- default attribute will be added -->
+ <!-- whitespace inside tag will be preserved -->
+ <whitespace> 321 </whitespace>
+
+ <!-- empty namespace will be removed of child tag -->
+ <emptyns xmlns="" >
+ <emptyns_child xmlns=""></emptyns_child>
+ </emptyns>
+
+ <!-- CDATA section will be replaced by its value -->
+ <compute><![CDATA[value>"0" && value<"10" ?"valid":"error"]]></compute>
+ </foo>
+ <!-- comment outside doc -->'::xml),
+ (2,'<foo>
+ <bar>
+ <!-- important comment -->
+ <val x="y">42</val>
+ </bar>
+ </foo> '::xml);
+
+SELECT xmlserialize(DOCUMENT doc AS text CANONICAL) FROM xmltest_serialize WHERE id = 1;
+SELECT xmlserialize(DOCUMENT doc AS text CANONICAL WITH COMMENTS) FROM xmltest_serialize WHERE id = 2;
+SELECT xmlserialize(DOCUMENT doc AS text CANONICAL) = xmlserialize(DOCUMENT doc AS text CANONICAL WITH NO COMMENTS) FROM xmltest_serialize;
+SELECT xmlserialize(CONTENT doc AS text CANONICAL) FROM xmltest_serialize WHERE id = 1;
+SELECT xmlserialize(CONTENT doc AS text CANONICAL WITH COMMENTS) FROM xmltest_serialize WHERE id = 2;
+SELECT xmlserialize(CONTENT doc AS text CANONICAL) = xmlserialize(CONTENT doc AS text CANONICAL WITH NO COMMENTS) FROM xmltest_serialize;
+SELECT xmlserialize(DOCUMENT NULL AS text CANONICAL);
+SELECT xmlserialize(CONTENT NULL AS text CANONICAL);
+\set VERBOSITY terse
+SELECT xmlserialize(DOCUMENT '' AS text CANONICAL);
+SELECT xmlserialize(DOCUMENT ' ' AS text CANONICAL);
+SELECT xmlserialize(DOCUMENT 'foo' AS text CANONICAL);
+SELECT xmlserialize(CONTENT '' AS text CANONICAL);
+SELECT xmlserialize(CONTENT ' ' AS text CANONICAL);
+SELECT xmlserialize(CONTENT 'foo' AS text CANONICAL);
+\set VERBOSITY default
SELECT xml '<foo>bar</foo>' IS DOCUMENT;
SELECT xml '<foo>bar</foo><bar>foo</bar>' IS DOCUMENT;
--
2.25.1
^ permalink raw reply [nested|flat] 7+ messages in thread
* [PATCH v9 04/21] heap_page_prune sets all_visible and visibility_cutoff_xid
@ 2024-03-25 22:31 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Melanie Plageman @ 2024-03-25 22:31 UTC (permalink / raw)
In order to combine the prune and freeze records, we must know if the
page is eligible to be opportunistically frozen before finishing
pruning. Save all_visible in the PruneResult and set it to false when we
see non-removable tuples which are not visible to everyone.
We will also need to ensure that the snapshotConflictHorizon for the combined
prune + freeze record is the more conservative of that calculated for each of
pruning and freezing. Calculate the visibility_cutoff_xid for the purposes of
freezing -- the newest xmin on the page -- in heap_page_prune() and save it in
PruneResult.visibility_cutoff_xid.
Note that these are only needed by vacuum callers of heap_page_prune(),
so don't update them for on-access pruning.
---
src/backend/access/heap/pruneheap.c | 131 +++++++++++++++++++++++++--
src/backend/access/heap/vacuumlazy.c | 113 +++++------------------
src/include/access/heapam.h | 21 +++++
3 files changed, 169 insertions(+), 96 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 7d7e1d2744c..52513fcdc90 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -63,8 +63,10 @@ static int heap_prune_chain(Buffer buffer,
static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
static void heap_prune_record_redirect(PruneState *prstate,
OffsetNumber offnum, OffsetNumber rdoffnum);
-static void heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum);
-static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum);
+static void heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult);
+static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult);
static void heap_prune_record_unused(PruneState *prstate, OffsetNumber offnum);
static void page_verify_redirects(Page page);
@@ -249,6 +251,14 @@ heap_page_prune(Relation relation, Buffer buffer,
presult->ndeleted = 0;
presult->nnewlpdead = 0;
+ /*
+ * Keep track of whether or not the page is all_visible in case the caller
+ * wants to use this information to update the VM.
+ */
+ presult->all_visible = true;
+ /* for recovery conflicts */
+ presult->visibility_cutoff_xid = InvalidTransactionId;
+
maxoff = PageGetMaxOffsetNumber(page);
tup.t_tableOid = RelationGetRelid(relation);
@@ -300,8 +310,101 @@ heap_page_prune(Relation relation, Buffer buffer,
presult->htsv[offnum] = heap_prune_satisfies_vacuum(&prstate, &tup,
buffer);
+
+ if (reason == PRUNE_ON_ACCESS)
+ continue;
+
+ switch (presult->htsv[offnum])
+ {
+ case HEAPTUPLE_DEAD:
+
+ /*
+ * Deliberately delay unsetting all_visible until later during
+ * pruning. Removable dead tuples shouldn't preclude freezing
+ * the page. After finishing this first pass of tuple
+ * visibility checks, initialize all_visible_except_removable
+ * with the current value of all_visible to indicate whether
+ * or not the page is all visible except for dead tuples. This
+ * will allow us to attempt to freeze the page after pruning.
+ * Later during pruning, if we encounter an LP_DEAD item or
+ * are setting an item LP_DEAD, we will unset all_visible. As
+ * long as we unset it before updating the visibility map,
+ * this will be correct.
+ */
+ break;
+ case HEAPTUPLE_LIVE:
+
+ /*
+ * Is the tuple definitely visible to all transactions?
+ *
+ * NB: Like with per-tuple hint bits, we can't set the
+ * PD_ALL_VISIBLE flag if the inserter committed
+ * asynchronously. See SetHintBits for more info. Check that
+ * the tuple is hinted xmin-committed because of that.
+ */
+ if (presult->all_visible)
+ {
+ TransactionId xmin;
+
+ if (!HeapTupleHeaderXminCommitted(htup))
+ {
+ presult->all_visible = false;
+ break;
+ }
+
+ /*
+ * The inserter definitely committed. But is it old enough
+ * that everyone sees it as committed? A
+ * FrozenTransactionId is seen as committed to everyone.
+ * Otherwise, we check if there is a snapshot that
+ * considers this xid to still be running, and if so, we
+ * don't consider the page all-visible.
+ */
+ xmin = HeapTupleHeaderGetXmin(htup);
+ if (xmin != FrozenTransactionId &&
+ !GlobalVisTestIsRemovableXid(vistest, xmin))
+ {
+ presult->all_visible = false;
+ break;
+ }
+
+ /* Track newest xmin on page. */
+ if (TransactionIdFollows(xmin, presult->visibility_cutoff_xid) &&
+ TransactionIdIsNormal(xmin))
+ presult->visibility_cutoff_xid = xmin;
+ }
+ break;
+ case HEAPTUPLE_RECENTLY_DEAD:
+ presult->all_visible = false;
+ break;
+ case HEAPTUPLE_INSERT_IN_PROGRESS:
+ presult->all_visible = false;
+ break;
+ case HEAPTUPLE_DELETE_IN_PROGRESS:
+ /* This is an expected case during concurrent vacuum */
+ presult->all_visible = false;
+ break;
+ default:
+ elog(ERROR, "unexpected HeapTupleSatisfiesVacuum result");
+ break;
+ }
}
+ /*
+ * For vacuum, if the whole page will become frozen, we consider
+ * opportunistically freezing tuples. Dead tuples which will be removed by
+ * the end of vacuuming should not preclude us from opportunistically
+ * freezing. We will not be able to freeze the whole page if there are
+ * tuples present which are not visible to everyone or if there are dead
+ * tuples which are not yet removable. We need all_visible to be false if
+ * LP_DEAD tuples remain after pruning so that we do not incorrectly
+ * update the visibility map or page hint bit. So, we will update
+ * presult->all_visible to reflect the presence of LP_DEAD items while
+ * pruning and keep all_visible_except_removable to permit freezing if the
+ * whole page will eventually become all visible after removing tuples.
+ */
+ presult->all_visible_except_removable = presult->all_visible;
+
/* Scan the page */
for (offnum = FirstOffsetNumber;
offnum <= maxoff;
@@ -565,10 +668,14 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
/*
* If the caller set mark_unused_now true, we can set dead line
* pointers LP_UNUSED now. We don't increment ndeleted here since
- * the LP was already marked dead.
+ * the LP was already marked dead. If it will not be marked
+ * LP_UNUSED, it will remain LP_DEAD, making the page not
+ * all_visible.
*/
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
+ else
+ presult->all_visible = false;
break;
}
@@ -705,7 +812,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* redirect the root to the correct chain member.
*/
if (i >= nchain)
- heap_prune_record_dead_or_unused(prstate, rootoffnum);
+ heap_prune_record_dead_or_unused(prstate, rootoffnum, presult);
else
heap_prune_record_redirect(prstate, rootoffnum, chainitems[i]);
}
@@ -718,7 +825,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* redirect item. We can clean up by setting the redirect item to
* DEAD state or LP_UNUSED if the caller indicated.
*/
- heap_prune_record_dead_or_unused(prstate, rootoffnum);
+ heap_prune_record_dead_or_unused(prstate, rootoffnum, presult);
}
return ndeleted;
@@ -755,13 +862,20 @@ heap_prune_record_redirect(PruneState *prstate,
/* Record line pointer to be marked dead */
static void
-heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum)
+heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult)
{
Assert(prstate->ndead < MaxHeapTuplesPerPage);
prstate->nowdead[prstate->ndead] = offnum;
prstate->ndead++;
Assert(!prstate->marked[offnum]);
prstate->marked[offnum] = true;
+
+ /*
+ * Setting the line pointer LP_DEAD means the page will definitely not be
+ * all_visible.
+ */
+ presult->all_visible = false;
}
/*
@@ -771,7 +885,8 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum)
* pointers LP_DEAD if mark_unused_now is true.
*/
static void
-heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum)
+heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult)
{
/*
* If the caller set mark_unused_now to true, we can remove dead tuples
@@ -782,7 +897,7 @@ heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum)
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
else
- heap_prune_record_dead(prstate, offnum);
+ heap_prune_record_dead(prstate, offnum, presult);
}
/* Record line pointer to be marked unused */
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index a7451743e25..17fb0b4f7b7 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1422,9 +1422,7 @@ lazy_scan_prune(LVRelState *vacrel,
recently_dead_tuples;
HeapPageFreeze pagefrz;
bool hastup = false;
- bool all_visible,
- all_frozen;
- TransactionId visibility_cutoff_xid;
+ bool all_frozen;
int64 fpi_before = pgWalUsage.wal_fpi;
OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
HeapTupleFreeze frozen[MaxHeapTuplesPerPage];
@@ -1465,17 +1463,16 @@ lazy_scan_prune(LVRelState *vacrel,
&presult, PRUNE_VACUUM_SCAN, &vacrel->offnum);
/*
- * We will update the VM after collecting LP_DEAD items and freezing
- * tuples. Keep track of whether or not the page is all_visible and
- * all_frozen and use this information to update the VM. all_visible
- * implies 0 lpdead_items, but don't trust all_frozen result unless
- * all_visible is also set to true.
+ * Now scan the page to collect LP_DEAD items and check for tuples
+ * requiring freezing among remaining tuples with storage. We will update
+ * the VM after collecting LP_DEAD items and freezing tuples. Pruning will
+ * have determined whether or not the page is all_visible. Keep track of
+ * whether or not the page is all_frozen and use this information to
+ * update the VM. all_visible implies lpdead_items == 0, but don't trust
+ * all_frozen result unless all_visible is also set to true.
*
- * Also keep track of the visibility cutoff xid for recovery conflicts.
*/
- all_visible = true;
all_frozen = true;
- visibility_cutoff_xid = InvalidTransactionId;
/*
* Now scan the page to collect LP_DEAD items and update the variables set
@@ -1516,11 +1513,6 @@ lazy_scan_prune(LVRelState *vacrel,
* will only happen every other VACUUM, at most. Besides, VACUUM
* must treat hastup/nonempty_pages as provisional no matter how
* LP_DEAD items are handled (handled here, or handled later on).
- *
- * Also deliberately delay unsetting all_visible until just before
- * we return to lazy_scan_heap caller, as explained in full below.
- * (This is another case where it's useful to anticipate that any
- * LP_DEAD items will become LP_UNUSED during the ongoing VACUUM.)
*/
deadoffsets[lpdead_items++] = offnum;
continue;
@@ -1558,46 +1550,6 @@ lazy_scan_prune(LVRelState *vacrel,
* what acquire_sample_rows() does.
*/
live_tuples++;
-
- /*
- * Is the tuple definitely visible to all transactions?
- *
- * NB: Like with per-tuple hint bits, we can't set the
- * PD_ALL_VISIBLE flag if the inserter committed
- * asynchronously. See SetHintBits for more info. Check that
- * the tuple is hinted xmin-committed because of that.
- */
- if (all_visible)
- {
- TransactionId xmin;
-
- if (!HeapTupleHeaderXminCommitted(htup))
- {
- all_visible = false;
- break;
- }
-
- /*
- * The inserter definitely committed. But is it old enough
- * that everyone sees it as committed? A
- * FrozenTransactionId is seen as committed to everyone.
- * Otherwise, we check if there is a snapshot that
- * considers this xid to still be running, and if so, we
- * don't consider the page all-visible.
- */
- xmin = HeapTupleHeaderGetXmin(htup);
- if (xmin != FrozenTransactionId &&
- !GlobalVisTestIsRemovableXid(vacrel->vistest, xmin))
- {
- all_visible = false;
- break;
- }
-
- /* Track newest xmin on page. */
- if (TransactionIdFollows(xmin, visibility_cutoff_xid) &&
- TransactionIdIsNormal(xmin))
- visibility_cutoff_xid = xmin;
- }
break;
case HEAPTUPLE_RECENTLY_DEAD:
@@ -1607,7 +1559,6 @@ lazy_scan_prune(LVRelState *vacrel,
* pruning.)
*/
recently_dead_tuples++;
- all_visible = false;
break;
case HEAPTUPLE_INSERT_IN_PROGRESS:
@@ -1618,16 +1569,13 @@ lazy_scan_prune(LVRelState *vacrel,
* results. This assumption is a bit shaky, but it is what
* acquire_sample_rows() does, so be consistent.
*/
- all_visible = false;
break;
case HEAPTUPLE_DELETE_IN_PROGRESS:
- /* This is an expected case during concurrent vacuum */
- all_visible = false;
/*
- * Count such rows as live. As above, we assume the deleting
- * transaction will commit and update the counters after we
- * report.
+ * This an expected case during concurrent vacuum. Count such
+ * rows as live. As above, we assume the deleting transaction
+ * will commit and update the counters after we report.
*/
live_tuples++;
break;
@@ -1670,7 +1618,7 @@ lazy_scan_prune(LVRelState *vacrel,
* page all-frozen afterwards (might not happen until final heap pass).
*/
if (pagefrz.freeze_required || tuples_frozen == 0 ||
- (all_visible && all_frozen &&
+ (presult.all_visible_except_removable && all_frozen &&
fpi_before != pgWalUsage.wal_fpi))
{
/*
@@ -1708,11 +1656,11 @@ lazy_scan_prune(LVRelState *vacrel,
* once we're done with it. Otherwise we generate a conservative
* cutoff by stepping back from OldestXmin.
*/
- if (all_visible && all_frozen)
+ if (presult.all_visible_except_removable && all_frozen)
{
/* Using same cutoff when setting VM is now unnecessary */
- snapshotConflictHorizon = visibility_cutoff_xid;
- visibility_cutoff_xid = InvalidTransactionId;
+ snapshotConflictHorizon = presult.visibility_cutoff_xid;
+ presult.visibility_cutoff_xid = InvalidTransactionId;
}
else
{
@@ -1748,17 +1696,19 @@ lazy_scan_prune(LVRelState *vacrel,
*/
#ifdef USE_ASSERT_CHECKING
/* Note that all_frozen value does not matter when !all_visible */
- if (all_visible && lpdead_items == 0)
+ if (presult.all_visible)
{
TransactionId debug_cutoff;
bool debug_all_frozen;
+ Assert(lpdead_items == 0);
+
if (!heap_page_is_all_visible(vacrel, buf,
&debug_cutoff, &debug_all_frozen))
Assert(false);
Assert(!TransactionIdIsValid(debug_cutoff) ||
- debug_cutoff == visibility_cutoff_xid);
+ debug_cutoff == presult.visibility_cutoff_xid);
}
#endif
@@ -1783,19 +1733,6 @@ lazy_scan_prune(LVRelState *vacrel,
Assert(dead_items->num_items <= dead_items->max_items);
pgstat_progress_update_param(PROGRESS_VACUUM_NUM_DEAD_TUPLES,
dead_items->num_items);
-
- /*
- * It was convenient to ignore LP_DEAD items in all_visible earlier on
- * to make the choice of whether or not to freeze the page unaffected
- * by the short-term presence of LP_DEAD items. These LP_DEAD items
- * were effectively assumed to be LP_UNUSED items in the making. It
- * doesn't matter which heap pass (initial pass or final pass) ends up
- * setting the page all-frozen, as long as the ongoing VACUUM does it.
- *
- * Now that freezing has been finalized, unset all_visible. It needs
- * to reflect the present state of things, as expected by our caller.
- */
- all_visible = false;
}
/* Finally, add page-local counts to whole-VACUUM counts */
@@ -1812,20 +1749,20 @@ lazy_scan_prune(LVRelState *vacrel,
/* Did we find LP_DEAD items? */
*has_lpdead_items = (lpdead_items > 0);
- Assert(!all_visible || !(*has_lpdead_items));
+ Assert(!presult.all_visible || !(*has_lpdead_items));
/*
* Handle setting visibility map bit based on information from the VM (as
* of last heap_vac_scan_next_block() call), and from all_visible and
* all_frozen variables
*/
- if (!all_visible_according_to_vm && all_visible)
+ if (!all_visible_according_to_vm && presult.all_visible)
{
uint8 flags = VISIBILITYMAP_ALL_VISIBLE;
if (all_frozen)
{
- Assert(!TransactionIdIsValid(visibility_cutoff_xid));
+ Assert(!TransactionIdIsValid(presult.visibility_cutoff_xid));
flags |= VISIBILITYMAP_ALL_FROZEN;
}
@@ -1845,7 +1782,7 @@ lazy_scan_prune(LVRelState *vacrel,
PageSetAllVisible(page);
MarkBufferDirty(buf);
visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr,
- vmbuffer, visibility_cutoff_xid,
+ vmbuffer, presult.visibility_cutoff_xid,
flags);
}
@@ -1893,7 +1830,7 @@ lazy_scan_prune(LVRelState *vacrel,
* it as all-frozen. Note that all_frozen is only valid if all_visible is
* true, so we must check both all_visible and all_frozen.
*/
- else if (all_visible_according_to_vm && all_visible &&
+ else if (all_visible_according_to_vm && presult.all_visible &&
all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer))
{
/*
@@ -1914,7 +1851,7 @@ lazy_scan_prune(LVRelState *vacrel,
* since a snapshotConflictHorizon sufficient to make everything safe
* for REDO was logged when the page's tuples were frozen.
*/
- Assert(!TransactionIdIsValid(visibility_cutoff_xid));
+ Assert(!TransactionIdIsValid(presult.visibility_cutoff_xid));
visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr,
vmbuffer, InvalidTransactionId,
VISIBILITYMAP_ALL_VISIBLE |
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index f1122453738..29daab7aeb8 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -199,6 +199,27 @@ typedef struct PruneResult
int ndeleted; /* Number of tuples deleted from the page */
int nnewlpdead; /* Number of newly LP_DEAD items */
+ /*
+ * The rest of the fields in PruneResult are only guaranteed to be
+ * initialized if heap_page_prune is passed PruneReason VACUUM_SCAN.
+ */
+
+ /*
+ * Whether or not the page is truly all-visible after pruning. If there
+ * are LP_DEAD items on the page which cannot be removed until vacuum's
+ * second pass, this will be false.
+ */
+ bool all_visible;
+
+ /*
+ * Whether or not the page is all-visible except for tuples which will be
+ * removed during vacuum's second pass. This is used by VACUUM to
+ * determine whether or not to consider opportunistically freezing the
+ * page.
+ */
+ bool all_visible_except_removable;
+ TransactionId visibility_cutoff_xid; /* Newest xmin on the page */
+
/*
* Tuple visibility is only computed once for each tuple, for correctness
* and efficiency reasons; see comment in heap_page_prune() for details.
--
2.40.1
--caj67xgx3lukmr5f
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v9-0005-Add-reference-to-VacuumCutoffs-in-HeapPageFreeze.patch"
^ permalink raw reply [nested|flat] 7+ messages in thread
* [PATCH v9 04/21] heap_page_prune sets all_visible and visibility_cutoff_xid
@ 2024-03-25 22:31 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Melanie Plageman @ 2024-03-25 22:31 UTC (permalink / raw)
In order to combine the prune and freeze records, we must know if the
page is eligible to be opportunistically frozen before finishing
pruning. Save all_visible in the PruneResult and set it to false when we
see non-removable tuples which are not visible to everyone.
We will also need to ensure that the snapshotConflictHorizon for the combined
prune + freeze record is the more conservative of that calculated for each of
pruning and freezing. Calculate the visibility_cutoff_xid for the purposes of
freezing -- the newest xmin on the page -- in heap_page_prune() and save it in
PruneResult.visibility_cutoff_xid.
Note that these are only needed by vacuum callers of heap_page_prune(),
so don't update them for on-access pruning.
---
src/backend/access/heap/pruneheap.c | 131 +++++++++++++++++++++++++--
src/backend/access/heap/vacuumlazy.c | 113 +++++------------------
src/include/access/heapam.h | 21 +++++
3 files changed, 169 insertions(+), 96 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 7d7e1d2744c..52513fcdc90 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -63,8 +63,10 @@ static int heap_prune_chain(Buffer buffer,
static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
static void heap_prune_record_redirect(PruneState *prstate,
OffsetNumber offnum, OffsetNumber rdoffnum);
-static void heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum);
-static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum);
+static void heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult);
+static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult);
static void heap_prune_record_unused(PruneState *prstate, OffsetNumber offnum);
static void page_verify_redirects(Page page);
@@ -249,6 +251,14 @@ heap_page_prune(Relation relation, Buffer buffer,
presult->ndeleted = 0;
presult->nnewlpdead = 0;
+ /*
+ * Keep track of whether or not the page is all_visible in case the caller
+ * wants to use this information to update the VM.
+ */
+ presult->all_visible = true;
+ /* for recovery conflicts */
+ presult->visibility_cutoff_xid = InvalidTransactionId;
+
maxoff = PageGetMaxOffsetNumber(page);
tup.t_tableOid = RelationGetRelid(relation);
@@ -300,8 +310,101 @@ heap_page_prune(Relation relation, Buffer buffer,
presult->htsv[offnum] = heap_prune_satisfies_vacuum(&prstate, &tup,
buffer);
+
+ if (reason == PRUNE_ON_ACCESS)
+ continue;
+
+ switch (presult->htsv[offnum])
+ {
+ case HEAPTUPLE_DEAD:
+
+ /*
+ * Deliberately delay unsetting all_visible until later during
+ * pruning. Removable dead tuples shouldn't preclude freezing
+ * the page. After finishing this first pass of tuple
+ * visibility checks, initialize all_visible_except_removable
+ * with the current value of all_visible to indicate whether
+ * or not the page is all visible except for dead tuples. This
+ * will allow us to attempt to freeze the page after pruning.
+ * Later during pruning, if we encounter an LP_DEAD item or
+ * are setting an item LP_DEAD, we will unset all_visible. As
+ * long as we unset it before updating the visibility map,
+ * this will be correct.
+ */
+ break;
+ case HEAPTUPLE_LIVE:
+
+ /*
+ * Is the tuple definitely visible to all transactions?
+ *
+ * NB: Like with per-tuple hint bits, we can't set the
+ * PD_ALL_VISIBLE flag if the inserter committed
+ * asynchronously. See SetHintBits for more info. Check that
+ * the tuple is hinted xmin-committed because of that.
+ */
+ if (presult->all_visible)
+ {
+ TransactionId xmin;
+
+ if (!HeapTupleHeaderXminCommitted(htup))
+ {
+ presult->all_visible = false;
+ break;
+ }
+
+ /*
+ * The inserter definitely committed. But is it old enough
+ * that everyone sees it as committed? A
+ * FrozenTransactionId is seen as committed to everyone.
+ * Otherwise, we check if there is a snapshot that
+ * considers this xid to still be running, and if so, we
+ * don't consider the page all-visible.
+ */
+ xmin = HeapTupleHeaderGetXmin(htup);
+ if (xmin != FrozenTransactionId &&
+ !GlobalVisTestIsRemovableXid(vistest, xmin))
+ {
+ presult->all_visible = false;
+ break;
+ }
+
+ /* Track newest xmin on page. */
+ if (TransactionIdFollows(xmin, presult->visibility_cutoff_xid) &&
+ TransactionIdIsNormal(xmin))
+ presult->visibility_cutoff_xid = xmin;
+ }
+ break;
+ case HEAPTUPLE_RECENTLY_DEAD:
+ presult->all_visible = false;
+ break;
+ case HEAPTUPLE_INSERT_IN_PROGRESS:
+ presult->all_visible = false;
+ break;
+ case HEAPTUPLE_DELETE_IN_PROGRESS:
+ /* This is an expected case during concurrent vacuum */
+ presult->all_visible = false;
+ break;
+ default:
+ elog(ERROR, "unexpected HeapTupleSatisfiesVacuum result");
+ break;
+ }
}
+ /*
+ * For vacuum, if the whole page will become frozen, we consider
+ * opportunistically freezing tuples. Dead tuples which will be removed by
+ * the end of vacuuming should not preclude us from opportunistically
+ * freezing. We will not be able to freeze the whole page if there are
+ * tuples present which are not visible to everyone or if there are dead
+ * tuples which are not yet removable. We need all_visible to be false if
+ * LP_DEAD tuples remain after pruning so that we do not incorrectly
+ * update the visibility map or page hint bit. So, we will update
+ * presult->all_visible to reflect the presence of LP_DEAD items while
+ * pruning and keep all_visible_except_removable to permit freezing if the
+ * whole page will eventually become all visible after removing tuples.
+ */
+ presult->all_visible_except_removable = presult->all_visible;
+
/* Scan the page */
for (offnum = FirstOffsetNumber;
offnum <= maxoff;
@@ -565,10 +668,14 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
/*
* If the caller set mark_unused_now true, we can set dead line
* pointers LP_UNUSED now. We don't increment ndeleted here since
- * the LP was already marked dead.
+ * the LP was already marked dead. If it will not be marked
+ * LP_UNUSED, it will remain LP_DEAD, making the page not
+ * all_visible.
*/
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
+ else
+ presult->all_visible = false;
break;
}
@@ -705,7 +812,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* redirect the root to the correct chain member.
*/
if (i >= nchain)
- heap_prune_record_dead_or_unused(prstate, rootoffnum);
+ heap_prune_record_dead_or_unused(prstate, rootoffnum, presult);
else
heap_prune_record_redirect(prstate, rootoffnum, chainitems[i]);
}
@@ -718,7 +825,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* redirect item. We can clean up by setting the redirect item to
* DEAD state or LP_UNUSED if the caller indicated.
*/
- heap_prune_record_dead_or_unused(prstate, rootoffnum);
+ heap_prune_record_dead_or_unused(prstate, rootoffnum, presult);
}
return ndeleted;
@@ -755,13 +862,20 @@ heap_prune_record_redirect(PruneState *prstate,
/* Record line pointer to be marked dead */
static void
-heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum)
+heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult)
{
Assert(prstate->ndead < MaxHeapTuplesPerPage);
prstate->nowdead[prstate->ndead] = offnum;
prstate->ndead++;
Assert(!prstate->marked[offnum]);
prstate->marked[offnum] = true;
+
+ /*
+ * Setting the line pointer LP_DEAD means the page will definitely not be
+ * all_visible.
+ */
+ presult->all_visible = false;
}
/*
@@ -771,7 +885,8 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum)
* pointers LP_DEAD if mark_unused_now is true.
*/
static void
-heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum)
+heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult)
{
/*
* If the caller set mark_unused_now to true, we can remove dead tuples
@@ -782,7 +897,7 @@ heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum)
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
else
- heap_prune_record_dead(prstate, offnum);
+ heap_prune_record_dead(prstate, offnum, presult);
}
/* Record line pointer to be marked unused */
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index a7451743e25..17fb0b4f7b7 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1422,9 +1422,7 @@ lazy_scan_prune(LVRelState *vacrel,
recently_dead_tuples;
HeapPageFreeze pagefrz;
bool hastup = false;
- bool all_visible,
- all_frozen;
- TransactionId visibility_cutoff_xid;
+ bool all_frozen;
int64 fpi_before = pgWalUsage.wal_fpi;
OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
HeapTupleFreeze frozen[MaxHeapTuplesPerPage];
@@ -1465,17 +1463,16 @@ lazy_scan_prune(LVRelState *vacrel,
&presult, PRUNE_VACUUM_SCAN, &vacrel->offnum);
/*
- * We will update the VM after collecting LP_DEAD items and freezing
- * tuples. Keep track of whether or not the page is all_visible and
- * all_frozen and use this information to update the VM. all_visible
- * implies 0 lpdead_items, but don't trust all_frozen result unless
- * all_visible is also set to true.
+ * Now scan the page to collect LP_DEAD items and check for tuples
+ * requiring freezing among remaining tuples with storage. We will update
+ * the VM after collecting LP_DEAD items and freezing tuples. Pruning will
+ * have determined whether or not the page is all_visible. Keep track of
+ * whether or not the page is all_frozen and use this information to
+ * update the VM. all_visible implies lpdead_items == 0, but don't trust
+ * all_frozen result unless all_visible is also set to true.
*
- * Also keep track of the visibility cutoff xid for recovery conflicts.
*/
- all_visible = true;
all_frozen = true;
- visibility_cutoff_xid = InvalidTransactionId;
/*
* Now scan the page to collect LP_DEAD items and update the variables set
@@ -1516,11 +1513,6 @@ lazy_scan_prune(LVRelState *vacrel,
* will only happen every other VACUUM, at most. Besides, VACUUM
* must treat hastup/nonempty_pages as provisional no matter how
* LP_DEAD items are handled (handled here, or handled later on).
- *
- * Also deliberately delay unsetting all_visible until just before
- * we return to lazy_scan_heap caller, as explained in full below.
- * (This is another case where it's useful to anticipate that any
- * LP_DEAD items will become LP_UNUSED during the ongoing VACUUM.)
*/
deadoffsets[lpdead_items++] = offnum;
continue;
@@ -1558,46 +1550,6 @@ lazy_scan_prune(LVRelState *vacrel,
* what acquire_sample_rows() does.
*/
live_tuples++;
-
- /*
- * Is the tuple definitely visible to all transactions?
- *
- * NB: Like with per-tuple hint bits, we can't set the
- * PD_ALL_VISIBLE flag if the inserter committed
- * asynchronously. See SetHintBits for more info. Check that
- * the tuple is hinted xmin-committed because of that.
- */
- if (all_visible)
- {
- TransactionId xmin;
-
- if (!HeapTupleHeaderXminCommitted(htup))
- {
- all_visible = false;
- break;
- }
-
- /*
- * The inserter definitely committed. But is it old enough
- * that everyone sees it as committed? A
- * FrozenTransactionId is seen as committed to everyone.
- * Otherwise, we check if there is a snapshot that
- * considers this xid to still be running, and if so, we
- * don't consider the page all-visible.
- */
- xmin = HeapTupleHeaderGetXmin(htup);
- if (xmin != FrozenTransactionId &&
- !GlobalVisTestIsRemovableXid(vacrel->vistest, xmin))
- {
- all_visible = false;
- break;
- }
-
- /* Track newest xmin on page. */
- if (TransactionIdFollows(xmin, visibility_cutoff_xid) &&
- TransactionIdIsNormal(xmin))
- visibility_cutoff_xid = xmin;
- }
break;
case HEAPTUPLE_RECENTLY_DEAD:
@@ -1607,7 +1559,6 @@ lazy_scan_prune(LVRelState *vacrel,
* pruning.)
*/
recently_dead_tuples++;
- all_visible = false;
break;
case HEAPTUPLE_INSERT_IN_PROGRESS:
@@ -1618,16 +1569,13 @@ lazy_scan_prune(LVRelState *vacrel,
* results. This assumption is a bit shaky, but it is what
* acquire_sample_rows() does, so be consistent.
*/
- all_visible = false;
break;
case HEAPTUPLE_DELETE_IN_PROGRESS:
- /* This is an expected case during concurrent vacuum */
- all_visible = false;
/*
- * Count such rows as live. As above, we assume the deleting
- * transaction will commit and update the counters after we
- * report.
+ * This an expected case during concurrent vacuum. Count such
+ * rows as live. As above, we assume the deleting transaction
+ * will commit and update the counters after we report.
*/
live_tuples++;
break;
@@ -1670,7 +1618,7 @@ lazy_scan_prune(LVRelState *vacrel,
* page all-frozen afterwards (might not happen until final heap pass).
*/
if (pagefrz.freeze_required || tuples_frozen == 0 ||
- (all_visible && all_frozen &&
+ (presult.all_visible_except_removable && all_frozen &&
fpi_before != pgWalUsage.wal_fpi))
{
/*
@@ -1708,11 +1656,11 @@ lazy_scan_prune(LVRelState *vacrel,
* once we're done with it. Otherwise we generate a conservative
* cutoff by stepping back from OldestXmin.
*/
- if (all_visible && all_frozen)
+ if (presult.all_visible_except_removable && all_frozen)
{
/* Using same cutoff when setting VM is now unnecessary */
- snapshotConflictHorizon = visibility_cutoff_xid;
- visibility_cutoff_xid = InvalidTransactionId;
+ snapshotConflictHorizon = presult.visibility_cutoff_xid;
+ presult.visibility_cutoff_xid = InvalidTransactionId;
}
else
{
@@ -1748,17 +1696,19 @@ lazy_scan_prune(LVRelState *vacrel,
*/
#ifdef USE_ASSERT_CHECKING
/* Note that all_frozen value does not matter when !all_visible */
- if (all_visible && lpdead_items == 0)
+ if (presult.all_visible)
{
TransactionId debug_cutoff;
bool debug_all_frozen;
+ Assert(lpdead_items == 0);
+
if (!heap_page_is_all_visible(vacrel, buf,
&debug_cutoff, &debug_all_frozen))
Assert(false);
Assert(!TransactionIdIsValid(debug_cutoff) ||
- debug_cutoff == visibility_cutoff_xid);
+ debug_cutoff == presult.visibility_cutoff_xid);
}
#endif
@@ -1783,19 +1733,6 @@ lazy_scan_prune(LVRelState *vacrel,
Assert(dead_items->num_items <= dead_items->max_items);
pgstat_progress_update_param(PROGRESS_VACUUM_NUM_DEAD_TUPLES,
dead_items->num_items);
-
- /*
- * It was convenient to ignore LP_DEAD items in all_visible earlier on
- * to make the choice of whether or not to freeze the page unaffected
- * by the short-term presence of LP_DEAD items. These LP_DEAD items
- * were effectively assumed to be LP_UNUSED items in the making. It
- * doesn't matter which heap pass (initial pass or final pass) ends up
- * setting the page all-frozen, as long as the ongoing VACUUM does it.
- *
- * Now that freezing has been finalized, unset all_visible. It needs
- * to reflect the present state of things, as expected by our caller.
- */
- all_visible = false;
}
/* Finally, add page-local counts to whole-VACUUM counts */
@@ -1812,20 +1749,20 @@ lazy_scan_prune(LVRelState *vacrel,
/* Did we find LP_DEAD items? */
*has_lpdead_items = (lpdead_items > 0);
- Assert(!all_visible || !(*has_lpdead_items));
+ Assert(!presult.all_visible || !(*has_lpdead_items));
/*
* Handle setting visibility map bit based on information from the VM (as
* of last heap_vac_scan_next_block() call), and from all_visible and
* all_frozen variables
*/
- if (!all_visible_according_to_vm && all_visible)
+ if (!all_visible_according_to_vm && presult.all_visible)
{
uint8 flags = VISIBILITYMAP_ALL_VISIBLE;
if (all_frozen)
{
- Assert(!TransactionIdIsValid(visibility_cutoff_xid));
+ Assert(!TransactionIdIsValid(presult.visibility_cutoff_xid));
flags |= VISIBILITYMAP_ALL_FROZEN;
}
@@ -1845,7 +1782,7 @@ lazy_scan_prune(LVRelState *vacrel,
PageSetAllVisible(page);
MarkBufferDirty(buf);
visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr,
- vmbuffer, visibility_cutoff_xid,
+ vmbuffer, presult.visibility_cutoff_xid,
flags);
}
@@ -1893,7 +1830,7 @@ lazy_scan_prune(LVRelState *vacrel,
* it as all-frozen. Note that all_frozen is only valid if all_visible is
* true, so we must check both all_visible and all_frozen.
*/
- else if (all_visible_according_to_vm && all_visible &&
+ else if (all_visible_according_to_vm && presult.all_visible &&
all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer))
{
/*
@@ -1914,7 +1851,7 @@ lazy_scan_prune(LVRelState *vacrel,
* since a snapshotConflictHorizon sufficient to make everything safe
* for REDO was logged when the page's tuples were frozen.
*/
- Assert(!TransactionIdIsValid(visibility_cutoff_xid));
+ Assert(!TransactionIdIsValid(presult.visibility_cutoff_xid));
visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr,
vmbuffer, InvalidTransactionId,
VISIBILITYMAP_ALL_VISIBLE |
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index f1122453738..29daab7aeb8 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -199,6 +199,27 @@ typedef struct PruneResult
int ndeleted; /* Number of tuples deleted from the page */
int nnewlpdead; /* Number of newly LP_DEAD items */
+ /*
+ * The rest of the fields in PruneResult are only guaranteed to be
+ * initialized if heap_page_prune is passed PruneReason VACUUM_SCAN.
+ */
+
+ /*
+ * Whether or not the page is truly all-visible after pruning. If there
+ * are LP_DEAD items on the page which cannot be removed until vacuum's
+ * second pass, this will be false.
+ */
+ bool all_visible;
+
+ /*
+ * Whether or not the page is all-visible except for tuples which will be
+ * removed during vacuum's second pass. This is used by VACUUM to
+ * determine whether or not to consider opportunistically freezing the
+ * page.
+ */
+ bool all_visible_except_removable;
+ TransactionId visibility_cutoff_xid; /* Newest xmin on the page */
+
/*
* Tuple visibility is only computed once for each tuple, for correctness
* and efficiency reasons; see comment in heap_page_prune() for details.
--
2.40.1
--caj67xgx3lukmr5f
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v9-0005-Add-reference-to-VacuumCutoffs-in-HeapPageFreeze.patch"
^ permalink raw reply [nested|flat] 7+ messages in thread
* [PATCH v7 04/16] heap_page_prune sets all_visible and visibility_cutoff_xid
@ 2024-03-25 22:31 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Melanie Plageman @ 2024-03-25 22:31 UTC (permalink / raw)
In order to combine the prune and freeze records, we must know if the
page is eligible to be opportunistically frozen before finishing
pruning. Save all_visible in the PruneResult and set it to false when we
see non-removable tuples which are not visible to everyone.
We will also need to ensure that the snapshotConflictHorizon for the combined
prune + freeze record is the more conservative of that calculated for each of
pruning and freezing. Calculate the visibility_cutoff_xid for the purposes of
freezing -- the newest xmin on the page -- in heap_page_prune() and save it in
PruneResult.visibility_cutoff_xid.
Note that these are only needed by vacuum callers of heap_page_prune(),
so don't update them for on-access pruning.
---
src/backend/access/heap/pruneheap.c | 131 +++++++++++++++++++++++++--
src/backend/access/heap/vacuumlazy.c | 113 +++++------------------
src/include/access/heapam.h | 21 +++++
3 files changed, 169 insertions(+), 96 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index ca4301bb8a9..5776ae84f4d 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -63,8 +63,10 @@ static int heap_prune_chain(Buffer buffer,
static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
static void heap_prune_record_redirect(PruneState *prstate,
OffsetNumber offnum, OffsetNumber rdoffnum);
-static void heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum);
-static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum);
+static void heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult);
+static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult);
static void heap_prune_record_unused(PruneState *prstate, OffsetNumber offnum);
static void page_verify_redirects(Page page);
@@ -249,6 +251,14 @@ heap_page_prune(Relation relation, Buffer buffer,
presult->ndeleted = 0;
presult->nnewlpdead = 0;
+ /*
+ * Keep track of whether or not the page is all_visible in case the caller
+ * wants to use this information to update the VM.
+ */
+ presult->all_visible = true;
+ /* for recovery conflicts */
+ presult->visibility_cutoff_xid = InvalidTransactionId;
+
maxoff = PageGetMaxOffsetNumber(page);
tup.t_tableOid = RelationGetRelid(relation);
@@ -300,8 +310,101 @@ heap_page_prune(Relation relation, Buffer buffer,
presult->htsv[offnum] = heap_prune_satisfies_vacuum(&prstate, &tup,
buffer);
+
+ if (reason == PRUNE_ON_ACCESS)
+ continue;
+
+ switch (presult->htsv[offnum])
+ {
+ case HEAPTUPLE_DEAD:
+
+ /*
+ * Deliberately delay unsetting all_visible until later during
+ * pruning. Removable dead tuples shouldn't preclude freezing
+ * the page. After finishing this first pass of tuple
+ * visibility checks, initialize all_visible_except_removable
+ * with the current value of all_visible to indicate whether
+ * or not the page is all visible except for dead tuples. This
+ * will allow us to attempt to freeze the page after pruning.
+ * Later during pruning, if we encounter an LP_DEAD item or
+ * are setting an item LP_DEAD, we will unset all_visible. As
+ * long as we unset it before updating the visibility map,
+ * this will be correct.
+ */
+ break;
+ case HEAPTUPLE_LIVE:
+
+ /*
+ * Is the tuple definitely visible to all transactions?
+ *
+ * NB: Like with per-tuple hint bits, we can't set the
+ * PD_ALL_VISIBLE flag if the inserter committed
+ * asynchronously. See SetHintBits for more info. Check that
+ * the tuple is hinted xmin-committed because of that.
+ */
+ if (presult->all_visible)
+ {
+ TransactionId xmin;
+
+ if (!HeapTupleHeaderXminCommitted(htup))
+ {
+ presult->all_visible = false;
+ break;
+ }
+
+ /*
+ * The inserter definitely committed. But is it old enough
+ * that everyone sees it as committed? A
+ * FrozenTransactionId is seen as committed to everyone.
+ * Otherwise, we check if there is a snapshot that
+ * considers this xid to still be running, and if so, we
+ * don't consider the page all-visible.
+ */
+ xmin = HeapTupleHeaderGetXmin(htup);
+ if (xmin != FrozenTransactionId &&
+ !GlobalVisTestIsRemovableXid(vistest, xmin))
+ {
+ presult->all_visible = false;
+ break;
+ }
+
+ /* Track newest xmin on page. */
+ if (TransactionIdFollows(xmin, presult->visibility_cutoff_xid) &&
+ TransactionIdIsNormal(xmin))
+ presult->visibility_cutoff_xid = xmin;
+ }
+ break;
+ case HEAPTUPLE_RECENTLY_DEAD:
+ presult->all_visible = false;
+ break;
+ case HEAPTUPLE_INSERT_IN_PROGRESS:
+ presult->all_visible = false;
+ break;
+ case HEAPTUPLE_DELETE_IN_PROGRESS:
+ /* This is an expected case during concurrent vacuum */
+ presult->all_visible = false;
+ break;
+ default:
+ elog(ERROR, "unexpected HeapTupleSatisfiesVacuum result");
+ break;
+ }
}
+ /*
+ * For vacuum, if the whole page will become frozen, we consider
+ * opportunistically freezing tuples. Dead tuples which will be removed by
+ * the end of vacuuming should not preclude us from opportunistically
+ * freezing. We will not be able to freeze the whole page if there are
+ * tuples present which are not visible to everyone or if there are dead
+ * tuples which are not yet removable. We need all_visible to be false if
+ * LP_DEAD tuples remain after pruning so that we do not incorrectly
+ * update the visibility map or page hint bit. So, we will update
+ * presult->all_visible to reflect the presence of LP_DEAD items while
+ * pruning and keep all_visible_except_removable to permit freezing if the
+ * whole page will eventually become all visible after removing tuples.
+ */
+ presult->all_visible_except_removable = presult->all_visible;
+
/* Scan the page */
for (offnum = FirstOffsetNumber;
offnum <= maxoff;
@@ -569,10 +672,14 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
/*
* If the caller set mark_unused_now true, we can set dead line
* pointers LP_UNUSED now. We don't increment ndeleted here since
- * the LP was already marked dead.
+ * the LP was already marked dead. If it will not be marked
+ * LP_UNUSED, it will remain LP_DEAD, making the page not
+ * all_visible.
*/
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
+ else
+ presult->all_visible = false;
break;
}
@@ -709,7 +816,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* redirect the root to the correct chain member.
*/
if (i >= nchain)
- heap_prune_record_dead_or_unused(prstate, rootoffnum);
+ heap_prune_record_dead_or_unused(prstate, rootoffnum, presult);
else
heap_prune_record_redirect(prstate, rootoffnum, chainitems[i]);
}
@@ -722,7 +829,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* redirect item. We can clean up by setting the redirect item to
* DEAD state or LP_UNUSED if the caller indicated.
*/
- heap_prune_record_dead_or_unused(prstate, rootoffnum);
+ heap_prune_record_dead_or_unused(prstate, rootoffnum, presult);
}
return ndeleted;
@@ -759,13 +866,20 @@ heap_prune_record_redirect(PruneState *prstate,
/* Record line pointer to be marked dead */
static void
-heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum)
+heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult)
{
Assert(prstate->ndead < MaxHeapTuplesPerPage);
prstate->nowdead[prstate->ndead] = offnum;
prstate->ndead++;
Assert(!prstate->marked[offnum]);
prstate->marked[offnum] = true;
+
+ /*
+ * Setting the line pointer LP_DEAD means the page will definitely not be
+ * all_visible.
+ */
+ presult->all_visible = false;
}
/*
@@ -775,7 +889,8 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum)
* pointers LP_DEAD if mark_unused_now is true.
*/
static void
-heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum)
+heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult)
{
/*
* If the caller set mark_unused_now to true, we can remove dead tuples
@@ -786,7 +901,7 @@ heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum)
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
else
- heap_prune_record_dead(prstate, offnum);
+ heap_prune_record_dead(prstate, offnum, presult);
}
/* Record line pointer to be marked unused */
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index a7451743e25..17fb0b4f7b7 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1422,9 +1422,7 @@ lazy_scan_prune(LVRelState *vacrel,
recently_dead_tuples;
HeapPageFreeze pagefrz;
bool hastup = false;
- bool all_visible,
- all_frozen;
- TransactionId visibility_cutoff_xid;
+ bool all_frozen;
int64 fpi_before = pgWalUsage.wal_fpi;
OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
HeapTupleFreeze frozen[MaxHeapTuplesPerPage];
@@ -1465,17 +1463,16 @@ lazy_scan_prune(LVRelState *vacrel,
&presult, PRUNE_VACUUM_SCAN, &vacrel->offnum);
/*
- * We will update the VM after collecting LP_DEAD items and freezing
- * tuples. Keep track of whether or not the page is all_visible and
- * all_frozen and use this information to update the VM. all_visible
- * implies 0 lpdead_items, but don't trust all_frozen result unless
- * all_visible is also set to true.
+ * Now scan the page to collect LP_DEAD items and check for tuples
+ * requiring freezing among remaining tuples with storage. We will update
+ * the VM after collecting LP_DEAD items and freezing tuples. Pruning will
+ * have determined whether or not the page is all_visible. Keep track of
+ * whether or not the page is all_frozen and use this information to
+ * update the VM. all_visible implies lpdead_items == 0, but don't trust
+ * all_frozen result unless all_visible is also set to true.
*
- * Also keep track of the visibility cutoff xid for recovery conflicts.
*/
- all_visible = true;
all_frozen = true;
- visibility_cutoff_xid = InvalidTransactionId;
/*
* Now scan the page to collect LP_DEAD items and update the variables set
@@ -1516,11 +1513,6 @@ lazy_scan_prune(LVRelState *vacrel,
* will only happen every other VACUUM, at most. Besides, VACUUM
* must treat hastup/nonempty_pages as provisional no matter how
* LP_DEAD items are handled (handled here, or handled later on).
- *
- * Also deliberately delay unsetting all_visible until just before
- * we return to lazy_scan_heap caller, as explained in full below.
- * (This is another case where it's useful to anticipate that any
- * LP_DEAD items will become LP_UNUSED during the ongoing VACUUM.)
*/
deadoffsets[lpdead_items++] = offnum;
continue;
@@ -1558,46 +1550,6 @@ lazy_scan_prune(LVRelState *vacrel,
* what acquire_sample_rows() does.
*/
live_tuples++;
-
- /*
- * Is the tuple definitely visible to all transactions?
- *
- * NB: Like with per-tuple hint bits, we can't set the
- * PD_ALL_VISIBLE flag if the inserter committed
- * asynchronously. See SetHintBits for more info. Check that
- * the tuple is hinted xmin-committed because of that.
- */
- if (all_visible)
- {
- TransactionId xmin;
-
- if (!HeapTupleHeaderXminCommitted(htup))
- {
- all_visible = false;
- break;
- }
-
- /*
- * The inserter definitely committed. But is it old enough
- * that everyone sees it as committed? A
- * FrozenTransactionId is seen as committed to everyone.
- * Otherwise, we check if there is a snapshot that
- * considers this xid to still be running, and if so, we
- * don't consider the page all-visible.
- */
- xmin = HeapTupleHeaderGetXmin(htup);
- if (xmin != FrozenTransactionId &&
- !GlobalVisTestIsRemovableXid(vacrel->vistest, xmin))
- {
- all_visible = false;
- break;
- }
-
- /* Track newest xmin on page. */
- if (TransactionIdFollows(xmin, visibility_cutoff_xid) &&
- TransactionIdIsNormal(xmin))
- visibility_cutoff_xid = xmin;
- }
break;
case HEAPTUPLE_RECENTLY_DEAD:
@@ -1607,7 +1559,6 @@ lazy_scan_prune(LVRelState *vacrel,
* pruning.)
*/
recently_dead_tuples++;
- all_visible = false;
break;
case HEAPTUPLE_INSERT_IN_PROGRESS:
@@ -1618,16 +1569,13 @@ lazy_scan_prune(LVRelState *vacrel,
* results. This assumption is a bit shaky, but it is what
* acquire_sample_rows() does, so be consistent.
*/
- all_visible = false;
break;
case HEAPTUPLE_DELETE_IN_PROGRESS:
- /* This is an expected case during concurrent vacuum */
- all_visible = false;
/*
- * Count such rows as live. As above, we assume the deleting
- * transaction will commit and update the counters after we
- * report.
+ * This an expected case during concurrent vacuum. Count such
+ * rows as live. As above, we assume the deleting transaction
+ * will commit and update the counters after we report.
*/
live_tuples++;
break;
@@ -1670,7 +1618,7 @@ lazy_scan_prune(LVRelState *vacrel,
* page all-frozen afterwards (might not happen until final heap pass).
*/
if (pagefrz.freeze_required || tuples_frozen == 0 ||
- (all_visible && all_frozen &&
+ (presult.all_visible_except_removable && all_frozen &&
fpi_before != pgWalUsage.wal_fpi))
{
/*
@@ -1708,11 +1656,11 @@ lazy_scan_prune(LVRelState *vacrel,
* once we're done with it. Otherwise we generate a conservative
* cutoff by stepping back from OldestXmin.
*/
- if (all_visible && all_frozen)
+ if (presult.all_visible_except_removable && all_frozen)
{
/* Using same cutoff when setting VM is now unnecessary */
- snapshotConflictHorizon = visibility_cutoff_xid;
- visibility_cutoff_xid = InvalidTransactionId;
+ snapshotConflictHorizon = presult.visibility_cutoff_xid;
+ presult.visibility_cutoff_xid = InvalidTransactionId;
}
else
{
@@ -1748,17 +1696,19 @@ lazy_scan_prune(LVRelState *vacrel,
*/
#ifdef USE_ASSERT_CHECKING
/* Note that all_frozen value does not matter when !all_visible */
- if (all_visible && lpdead_items == 0)
+ if (presult.all_visible)
{
TransactionId debug_cutoff;
bool debug_all_frozen;
+ Assert(lpdead_items == 0);
+
if (!heap_page_is_all_visible(vacrel, buf,
&debug_cutoff, &debug_all_frozen))
Assert(false);
Assert(!TransactionIdIsValid(debug_cutoff) ||
- debug_cutoff == visibility_cutoff_xid);
+ debug_cutoff == presult.visibility_cutoff_xid);
}
#endif
@@ -1783,19 +1733,6 @@ lazy_scan_prune(LVRelState *vacrel,
Assert(dead_items->num_items <= dead_items->max_items);
pgstat_progress_update_param(PROGRESS_VACUUM_NUM_DEAD_TUPLES,
dead_items->num_items);
-
- /*
- * It was convenient to ignore LP_DEAD items in all_visible earlier on
- * to make the choice of whether or not to freeze the page unaffected
- * by the short-term presence of LP_DEAD items. These LP_DEAD items
- * were effectively assumed to be LP_UNUSED items in the making. It
- * doesn't matter which heap pass (initial pass or final pass) ends up
- * setting the page all-frozen, as long as the ongoing VACUUM does it.
- *
- * Now that freezing has been finalized, unset all_visible. It needs
- * to reflect the present state of things, as expected by our caller.
- */
- all_visible = false;
}
/* Finally, add page-local counts to whole-VACUUM counts */
@@ -1812,20 +1749,20 @@ lazy_scan_prune(LVRelState *vacrel,
/* Did we find LP_DEAD items? */
*has_lpdead_items = (lpdead_items > 0);
- Assert(!all_visible || !(*has_lpdead_items));
+ Assert(!presult.all_visible || !(*has_lpdead_items));
/*
* Handle setting visibility map bit based on information from the VM (as
* of last heap_vac_scan_next_block() call), and from all_visible and
* all_frozen variables
*/
- if (!all_visible_according_to_vm && all_visible)
+ if (!all_visible_according_to_vm && presult.all_visible)
{
uint8 flags = VISIBILITYMAP_ALL_VISIBLE;
if (all_frozen)
{
- Assert(!TransactionIdIsValid(visibility_cutoff_xid));
+ Assert(!TransactionIdIsValid(presult.visibility_cutoff_xid));
flags |= VISIBILITYMAP_ALL_FROZEN;
}
@@ -1845,7 +1782,7 @@ lazy_scan_prune(LVRelState *vacrel,
PageSetAllVisible(page);
MarkBufferDirty(buf);
visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr,
- vmbuffer, visibility_cutoff_xid,
+ vmbuffer, presult.visibility_cutoff_xid,
flags);
}
@@ -1893,7 +1830,7 @@ lazy_scan_prune(LVRelState *vacrel,
* it as all-frozen. Note that all_frozen is only valid if all_visible is
* true, so we must check both all_visible and all_frozen.
*/
- else if (all_visible_according_to_vm && all_visible &&
+ else if (all_visible_according_to_vm && presult.all_visible &&
all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer))
{
/*
@@ -1914,7 +1851,7 @@ lazy_scan_prune(LVRelState *vacrel,
* since a snapshotConflictHorizon sufficient to make everything safe
* for REDO was logged when the page's tuples were frozen.
*/
- Assert(!TransactionIdIsValid(visibility_cutoff_xid));
+ Assert(!TransactionIdIsValid(presult.visibility_cutoff_xid));
visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr,
vmbuffer, InvalidTransactionId,
VISIBILITYMAP_ALL_VISIBLE |
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 368c570a0f4..8d0dd40ba6d 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -199,6 +199,27 @@ typedef struct PruneResult
int ndeleted; /* Number of tuples deleted from the page */
int nnewlpdead; /* Number of newly LP_DEAD items */
+ /*
+ * The rest of the fields in PruneResult are only guaranteed to be
+ * initialized if heap_page_prune is passed PruneReason VACUUM_SCAN.
+ */
+
+ /*
+ * Whether or not the page is truly all-visible after pruning. If there
+ * are LP_DEAD items on the page which cannot be removed until vacuum's
+ * second pass, this will be false.
+ */
+ bool all_visible;
+
+ /*
+ * Whether or not the page is all-visible except for tuples which will be
+ * removed during vacuum's second pass. This is used by VACUUM to
+ * determine whether or not to consider opportunistically freezing the
+ * page.
+ */
+ bool all_visible_except_removable;
+ TransactionId visibility_cutoff_xid; /* Newest xmin on the page */
+
/*
* Tuple visibility is only computed once for each tuple, for correctness
* and efficiency reasons; see comment in heap_page_prune() for details.
--
2.40.1
--ck6erxojvlx23byk
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0005-Add-reference-to-VacuumCutoffs-in-HeapPageFreeze.patch"
^ permalink raw reply [nested|flat] 7+ messages in thread
* [PATCH v7 04/16] heap_page_prune sets all_visible and visibility_cutoff_xid
@ 2024-03-25 22:31 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Melanie Plageman @ 2024-03-25 22:31 UTC (permalink / raw)
In order to combine the prune and freeze records, we must know if the
page is eligible to be opportunistically frozen before finishing
pruning. Save all_visible in the PruneResult and set it to false when we
see non-removable tuples which are not visible to everyone.
We will also need to ensure that the snapshotConflictHorizon for the combined
prune + freeze record is the more conservative of that calculated for each of
pruning and freezing. Calculate the visibility_cutoff_xid for the purposes of
freezing -- the newest xmin on the page -- in heap_page_prune() and save it in
PruneResult.visibility_cutoff_xid.
Note that these are only needed by vacuum callers of heap_page_prune(),
so don't update them for on-access pruning.
---
src/backend/access/heap/pruneheap.c | 131 +++++++++++++++++++++++++--
src/backend/access/heap/vacuumlazy.c | 113 +++++------------------
src/include/access/heapam.h | 21 +++++
3 files changed, 169 insertions(+), 96 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index ca4301bb8a9..5776ae84f4d 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -63,8 +63,10 @@ static int heap_prune_chain(Buffer buffer,
static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
static void heap_prune_record_redirect(PruneState *prstate,
OffsetNumber offnum, OffsetNumber rdoffnum);
-static void heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum);
-static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum);
+static void heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult);
+static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult);
static void heap_prune_record_unused(PruneState *prstate, OffsetNumber offnum);
static void page_verify_redirects(Page page);
@@ -249,6 +251,14 @@ heap_page_prune(Relation relation, Buffer buffer,
presult->ndeleted = 0;
presult->nnewlpdead = 0;
+ /*
+ * Keep track of whether or not the page is all_visible in case the caller
+ * wants to use this information to update the VM.
+ */
+ presult->all_visible = true;
+ /* for recovery conflicts */
+ presult->visibility_cutoff_xid = InvalidTransactionId;
+
maxoff = PageGetMaxOffsetNumber(page);
tup.t_tableOid = RelationGetRelid(relation);
@@ -300,8 +310,101 @@ heap_page_prune(Relation relation, Buffer buffer,
presult->htsv[offnum] = heap_prune_satisfies_vacuum(&prstate, &tup,
buffer);
+
+ if (reason == PRUNE_ON_ACCESS)
+ continue;
+
+ switch (presult->htsv[offnum])
+ {
+ case HEAPTUPLE_DEAD:
+
+ /*
+ * Deliberately delay unsetting all_visible until later during
+ * pruning. Removable dead tuples shouldn't preclude freezing
+ * the page. After finishing this first pass of tuple
+ * visibility checks, initialize all_visible_except_removable
+ * with the current value of all_visible to indicate whether
+ * or not the page is all visible except for dead tuples. This
+ * will allow us to attempt to freeze the page after pruning.
+ * Later during pruning, if we encounter an LP_DEAD item or
+ * are setting an item LP_DEAD, we will unset all_visible. As
+ * long as we unset it before updating the visibility map,
+ * this will be correct.
+ */
+ break;
+ case HEAPTUPLE_LIVE:
+
+ /*
+ * Is the tuple definitely visible to all transactions?
+ *
+ * NB: Like with per-tuple hint bits, we can't set the
+ * PD_ALL_VISIBLE flag if the inserter committed
+ * asynchronously. See SetHintBits for more info. Check that
+ * the tuple is hinted xmin-committed because of that.
+ */
+ if (presult->all_visible)
+ {
+ TransactionId xmin;
+
+ if (!HeapTupleHeaderXminCommitted(htup))
+ {
+ presult->all_visible = false;
+ break;
+ }
+
+ /*
+ * The inserter definitely committed. But is it old enough
+ * that everyone sees it as committed? A
+ * FrozenTransactionId is seen as committed to everyone.
+ * Otherwise, we check if there is a snapshot that
+ * considers this xid to still be running, and if so, we
+ * don't consider the page all-visible.
+ */
+ xmin = HeapTupleHeaderGetXmin(htup);
+ if (xmin != FrozenTransactionId &&
+ !GlobalVisTestIsRemovableXid(vistest, xmin))
+ {
+ presult->all_visible = false;
+ break;
+ }
+
+ /* Track newest xmin on page. */
+ if (TransactionIdFollows(xmin, presult->visibility_cutoff_xid) &&
+ TransactionIdIsNormal(xmin))
+ presult->visibility_cutoff_xid = xmin;
+ }
+ break;
+ case HEAPTUPLE_RECENTLY_DEAD:
+ presult->all_visible = false;
+ break;
+ case HEAPTUPLE_INSERT_IN_PROGRESS:
+ presult->all_visible = false;
+ break;
+ case HEAPTUPLE_DELETE_IN_PROGRESS:
+ /* This is an expected case during concurrent vacuum */
+ presult->all_visible = false;
+ break;
+ default:
+ elog(ERROR, "unexpected HeapTupleSatisfiesVacuum result");
+ break;
+ }
}
+ /*
+ * For vacuum, if the whole page will become frozen, we consider
+ * opportunistically freezing tuples. Dead tuples which will be removed by
+ * the end of vacuuming should not preclude us from opportunistically
+ * freezing. We will not be able to freeze the whole page if there are
+ * tuples present which are not visible to everyone or if there are dead
+ * tuples which are not yet removable. We need all_visible to be false if
+ * LP_DEAD tuples remain after pruning so that we do not incorrectly
+ * update the visibility map or page hint bit. So, we will update
+ * presult->all_visible to reflect the presence of LP_DEAD items while
+ * pruning and keep all_visible_except_removable to permit freezing if the
+ * whole page will eventually become all visible after removing tuples.
+ */
+ presult->all_visible_except_removable = presult->all_visible;
+
/* Scan the page */
for (offnum = FirstOffsetNumber;
offnum <= maxoff;
@@ -569,10 +672,14 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
/*
* If the caller set mark_unused_now true, we can set dead line
* pointers LP_UNUSED now. We don't increment ndeleted here since
- * the LP was already marked dead.
+ * the LP was already marked dead. If it will not be marked
+ * LP_UNUSED, it will remain LP_DEAD, making the page not
+ * all_visible.
*/
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
+ else
+ presult->all_visible = false;
break;
}
@@ -709,7 +816,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* redirect the root to the correct chain member.
*/
if (i >= nchain)
- heap_prune_record_dead_or_unused(prstate, rootoffnum);
+ heap_prune_record_dead_or_unused(prstate, rootoffnum, presult);
else
heap_prune_record_redirect(prstate, rootoffnum, chainitems[i]);
}
@@ -722,7 +829,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* redirect item. We can clean up by setting the redirect item to
* DEAD state or LP_UNUSED if the caller indicated.
*/
- heap_prune_record_dead_or_unused(prstate, rootoffnum);
+ heap_prune_record_dead_or_unused(prstate, rootoffnum, presult);
}
return ndeleted;
@@ -759,13 +866,20 @@ heap_prune_record_redirect(PruneState *prstate,
/* Record line pointer to be marked dead */
static void
-heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum)
+heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult)
{
Assert(prstate->ndead < MaxHeapTuplesPerPage);
prstate->nowdead[prstate->ndead] = offnum;
prstate->ndead++;
Assert(!prstate->marked[offnum]);
prstate->marked[offnum] = true;
+
+ /*
+ * Setting the line pointer LP_DEAD means the page will definitely not be
+ * all_visible.
+ */
+ presult->all_visible = false;
}
/*
@@ -775,7 +889,8 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum)
* pointers LP_DEAD if mark_unused_now is true.
*/
static void
-heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum)
+heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum,
+ PruneResult *presult)
{
/*
* If the caller set mark_unused_now to true, we can remove dead tuples
@@ -786,7 +901,7 @@ heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum)
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
else
- heap_prune_record_dead(prstate, offnum);
+ heap_prune_record_dead(prstate, offnum, presult);
}
/* Record line pointer to be marked unused */
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index a7451743e25..17fb0b4f7b7 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1422,9 +1422,7 @@ lazy_scan_prune(LVRelState *vacrel,
recently_dead_tuples;
HeapPageFreeze pagefrz;
bool hastup = false;
- bool all_visible,
- all_frozen;
- TransactionId visibility_cutoff_xid;
+ bool all_frozen;
int64 fpi_before = pgWalUsage.wal_fpi;
OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
HeapTupleFreeze frozen[MaxHeapTuplesPerPage];
@@ -1465,17 +1463,16 @@ lazy_scan_prune(LVRelState *vacrel,
&presult, PRUNE_VACUUM_SCAN, &vacrel->offnum);
/*
- * We will update the VM after collecting LP_DEAD items and freezing
- * tuples. Keep track of whether or not the page is all_visible and
- * all_frozen and use this information to update the VM. all_visible
- * implies 0 lpdead_items, but don't trust all_frozen result unless
- * all_visible is also set to true.
+ * Now scan the page to collect LP_DEAD items and check for tuples
+ * requiring freezing among remaining tuples with storage. We will update
+ * the VM after collecting LP_DEAD items and freezing tuples. Pruning will
+ * have determined whether or not the page is all_visible. Keep track of
+ * whether or not the page is all_frozen and use this information to
+ * update the VM. all_visible implies lpdead_items == 0, but don't trust
+ * all_frozen result unless all_visible is also set to true.
*
- * Also keep track of the visibility cutoff xid for recovery conflicts.
*/
- all_visible = true;
all_frozen = true;
- visibility_cutoff_xid = InvalidTransactionId;
/*
* Now scan the page to collect LP_DEAD items and update the variables set
@@ -1516,11 +1513,6 @@ lazy_scan_prune(LVRelState *vacrel,
* will only happen every other VACUUM, at most. Besides, VACUUM
* must treat hastup/nonempty_pages as provisional no matter how
* LP_DEAD items are handled (handled here, or handled later on).
- *
- * Also deliberately delay unsetting all_visible until just before
- * we return to lazy_scan_heap caller, as explained in full below.
- * (This is another case where it's useful to anticipate that any
- * LP_DEAD items will become LP_UNUSED during the ongoing VACUUM.)
*/
deadoffsets[lpdead_items++] = offnum;
continue;
@@ -1558,46 +1550,6 @@ lazy_scan_prune(LVRelState *vacrel,
* what acquire_sample_rows() does.
*/
live_tuples++;
-
- /*
- * Is the tuple definitely visible to all transactions?
- *
- * NB: Like with per-tuple hint bits, we can't set the
- * PD_ALL_VISIBLE flag if the inserter committed
- * asynchronously. See SetHintBits for more info. Check that
- * the tuple is hinted xmin-committed because of that.
- */
- if (all_visible)
- {
- TransactionId xmin;
-
- if (!HeapTupleHeaderXminCommitted(htup))
- {
- all_visible = false;
- break;
- }
-
- /*
- * The inserter definitely committed. But is it old enough
- * that everyone sees it as committed? A
- * FrozenTransactionId is seen as committed to everyone.
- * Otherwise, we check if there is a snapshot that
- * considers this xid to still be running, and if so, we
- * don't consider the page all-visible.
- */
- xmin = HeapTupleHeaderGetXmin(htup);
- if (xmin != FrozenTransactionId &&
- !GlobalVisTestIsRemovableXid(vacrel->vistest, xmin))
- {
- all_visible = false;
- break;
- }
-
- /* Track newest xmin on page. */
- if (TransactionIdFollows(xmin, visibility_cutoff_xid) &&
- TransactionIdIsNormal(xmin))
- visibility_cutoff_xid = xmin;
- }
break;
case HEAPTUPLE_RECENTLY_DEAD:
@@ -1607,7 +1559,6 @@ lazy_scan_prune(LVRelState *vacrel,
* pruning.)
*/
recently_dead_tuples++;
- all_visible = false;
break;
case HEAPTUPLE_INSERT_IN_PROGRESS:
@@ -1618,16 +1569,13 @@ lazy_scan_prune(LVRelState *vacrel,
* results. This assumption is a bit shaky, but it is what
* acquire_sample_rows() does, so be consistent.
*/
- all_visible = false;
break;
case HEAPTUPLE_DELETE_IN_PROGRESS:
- /* This is an expected case during concurrent vacuum */
- all_visible = false;
/*
- * Count such rows as live. As above, we assume the deleting
- * transaction will commit and update the counters after we
- * report.
+ * This an expected case during concurrent vacuum. Count such
+ * rows as live. As above, we assume the deleting transaction
+ * will commit and update the counters after we report.
*/
live_tuples++;
break;
@@ -1670,7 +1618,7 @@ lazy_scan_prune(LVRelState *vacrel,
* page all-frozen afterwards (might not happen until final heap pass).
*/
if (pagefrz.freeze_required || tuples_frozen == 0 ||
- (all_visible && all_frozen &&
+ (presult.all_visible_except_removable && all_frozen &&
fpi_before != pgWalUsage.wal_fpi))
{
/*
@@ -1708,11 +1656,11 @@ lazy_scan_prune(LVRelState *vacrel,
* once we're done with it. Otherwise we generate a conservative
* cutoff by stepping back from OldestXmin.
*/
- if (all_visible && all_frozen)
+ if (presult.all_visible_except_removable && all_frozen)
{
/* Using same cutoff when setting VM is now unnecessary */
- snapshotConflictHorizon = visibility_cutoff_xid;
- visibility_cutoff_xid = InvalidTransactionId;
+ snapshotConflictHorizon = presult.visibility_cutoff_xid;
+ presult.visibility_cutoff_xid = InvalidTransactionId;
}
else
{
@@ -1748,17 +1696,19 @@ lazy_scan_prune(LVRelState *vacrel,
*/
#ifdef USE_ASSERT_CHECKING
/* Note that all_frozen value does not matter when !all_visible */
- if (all_visible && lpdead_items == 0)
+ if (presult.all_visible)
{
TransactionId debug_cutoff;
bool debug_all_frozen;
+ Assert(lpdead_items == 0);
+
if (!heap_page_is_all_visible(vacrel, buf,
&debug_cutoff, &debug_all_frozen))
Assert(false);
Assert(!TransactionIdIsValid(debug_cutoff) ||
- debug_cutoff == visibility_cutoff_xid);
+ debug_cutoff == presult.visibility_cutoff_xid);
}
#endif
@@ -1783,19 +1733,6 @@ lazy_scan_prune(LVRelState *vacrel,
Assert(dead_items->num_items <= dead_items->max_items);
pgstat_progress_update_param(PROGRESS_VACUUM_NUM_DEAD_TUPLES,
dead_items->num_items);
-
- /*
- * It was convenient to ignore LP_DEAD items in all_visible earlier on
- * to make the choice of whether or not to freeze the page unaffected
- * by the short-term presence of LP_DEAD items. These LP_DEAD items
- * were effectively assumed to be LP_UNUSED items in the making. It
- * doesn't matter which heap pass (initial pass or final pass) ends up
- * setting the page all-frozen, as long as the ongoing VACUUM does it.
- *
- * Now that freezing has been finalized, unset all_visible. It needs
- * to reflect the present state of things, as expected by our caller.
- */
- all_visible = false;
}
/* Finally, add page-local counts to whole-VACUUM counts */
@@ -1812,20 +1749,20 @@ lazy_scan_prune(LVRelState *vacrel,
/* Did we find LP_DEAD items? */
*has_lpdead_items = (lpdead_items > 0);
- Assert(!all_visible || !(*has_lpdead_items));
+ Assert(!presult.all_visible || !(*has_lpdead_items));
/*
* Handle setting visibility map bit based on information from the VM (as
* of last heap_vac_scan_next_block() call), and from all_visible and
* all_frozen variables
*/
- if (!all_visible_according_to_vm && all_visible)
+ if (!all_visible_according_to_vm && presult.all_visible)
{
uint8 flags = VISIBILITYMAP_ALL_VISIBLE;
if (all_frozen)
{
- Assert(!TransactionIdIsValid(visibility_cutoff_xid));
+ Assert(!TransactionIdIsValid(presult.visibility_cutoff_xid));
flags |= VISIBILITYMAP_ALL_FROZEN;
}
@@ -1845,7 +1782,7 @@ lazy_scan_prune(LVRelState *vacrel,
PageSetAllVisible(page);
MarkBufferDirty(buf);
visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr,
- vmbuffer, visibility_cutoff_xid,
+ vmbuffer, presult.visibility_cutoff_xid,
flags);
}
@@ -1893,7 +1830,7 @@ lazy_scan_prune(LVRelState *vacrel,
* it as all-frozen. Note that all_frozen is only valid if all_visible is
* true, so we must check both all_visible and all_frozen.
*/
- else if (all_visible_according_to_vm && all_visible &&
+ else if (all_visible_according_to_vm && presult.all_visible &&
all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer))
{
/*
@@ -1914,7 +1851,7 @@ lazy_scan_prune(LVRelState *vacrel,
* since a snapshotConflictHorizon sufficient to make everything safe
* for REDO was logged when the page's tuples were frozen.
*/
- Assert(!TransactionIdIsValid(visibility_cutoff_xid));
+ Assert(!TransactionIdIsValid(presult.visibility_cutoff_xid));
visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr,
vmbuffer, InvalidTransactionId,
VISIBILITYMAP_ALL_VISIBLE |
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 368c570a0f4..8d0dd40ba6d 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -199,6 +199,27 @@ typedef struct PruneResult
int ndeleted; /* Number of tuples deleted from the page */
int nnewlpdead; /* Number of newly LP_DEAD items */
+ /*
+ * The rest of the fields in PruneResult are only guaranteed to be
+ * initialized if heap_page_prune is passed PruneReason VACUUM_SCAN.
+ */
+
+ /*
+ * Whether or not the page is truly all-visible after pruning. If there
+ * are LP_DEAD items on the page which cannot be removed until vacuum's
+ * second pass, this will be false.
+ */
+ bool all_visible;
+
+ /*
+ * Whether or not the page is all-visible except for tuples which will be
+ * removed during vacuum's second pass. This is used by VACUUM to
+ * determine whether or not to consider opportunistically freezing the
+ * page.
+ */
+ bool all_visible_except_removable;
+ TransactionId visibility_cutoff_xid; /* Newest xmin on the page */
+
/*
* Tuple visibility is only computed once for each tuple, for correctness
* and efficiency reasons; see comment in heap_page_prune() for details.
--
2.40.1
--ck6erxojvlx23byk
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0005-Add-reference-to-VacuumCutoffs-in-HeapPageFreeze.patch"
^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2024-03-25 22:31 UTC | newest]
Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-06-18 12:34 [PATCH 3/8] Add rewrite rules and tupdesc flags Ildus Kurbangaliev <[email protected]>
2023-02-27 13:16 [PoC] Add CANONICAL option to xmlserialize Jim Jones <[email protected]>
2023-03-05 18:44 ` [PATCH] Add CANONICAL option to xmlserialize Jim Jones <[email protected]>
2024-03-25 22:31 [PATCH v9 04/21] heap_page_prune sets all_visible and visibility_cutoff_xid Melanie Plageman <[email protected]>
2024-03-25 22:31 [PATCH v9 04/21] heap_page_prune sets all_visible and visibility_cutoff_xid Melanie Plageman <[email protected]>
2024-03-25 22:31 [PATCH v7 04/16] heap_page_prune sets all_visible and visibility_cutoff_xid Melanie Plageman <[email protected]>
2024-03-25 22:31 [PATCH v7 04/16] heap_page_prune sets all_visible and visibility_cutoff_xid Melanie Plageman <[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