From c2dc08ce7d41294cc52cc2820578e5a1668dddf7 Mon Sep 17 00:00:00 2001 From: kommih Date: Mon, 22 Oct 2018 16:06:10 +1100 Subject: [PATCH 1/2] Further fixes and cleanup 1. Remove the old slot interface file and also update the Makefile. 2. CREATE AS USING method grammer support This change was missed during earlier USING grammer support. 3. Remove the extra Tuple visibility check In heapgettup_pagemode the tuple visiblity check is added during the early devlopment of pluggable storage, but the visibility check is already carried out in heapgetpage function itself. regression fixes 1. scan start offset fix during analyze 2. Materialize the slot before they are processed using intorel_receive 3. ROW_MARK_COPY support by force store of heap tuple 4. partition prune extra heap page fix --- contrib/pg_visibility/pg_visibility.c | 5 ++-- src/backend/access/heap/heapam.c | 28 +++++++++-------------- src/backend/access/heap/heapam_handler.c | 6 +++-- src/backend/access/table/Makefile | 2 +- src/backend/access/table/tableam_common.c | 0 src/backend/commands/createas.c | 4 ++++ src/backend/executor/execExprInterp.c | 3 +++ src/backend/executor/execMain.c | 13 ++--------- src/backend/executor/execTuples.c | 21 +++++++++++++++++ src/backend/executor/nodeBitmapHeapscan.c | 12 ++++++++++ src/backend/executor/nodeModifyTable.c | 12 ++++++++-- src/backend/parser/gram.y | 11 +++++---- src/include/executor/tuptable.h | 1 + src/include/nodes/primnodes.h | 1 + 14 files changed, 79 insertions(+), 40 deletions(-) delete mode 100644 src/backend/access/table/tableam_common.c diff --git a/contrib/pg_visibility/pg_visibility.c b/contrib/pg_visibility/pg_visibility.c index dce5262e34..88ca4fd2af 100644 --- a/contrib/pg_visibility/pg_visibility.c +++ b/contrib/pg_visibility/pg_visibility.c @@ -563,12 +563,13 @@ collect_corrupt_items(Oid relid, bool all_visible, bool all_frozen) rel = relation_open(relid, AccessShareLock); + /* Only some relkinds have a visibility map */ + check_relation_relkind(rel); + if (rel->rd_rel->relam != HEAP_TABLE_AM_OID) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("only heap AM is supported"))); - /* Only some relkinds have a visibility map */ - check_relation_relkind(rel); nblocks = RelationGetNumberOfBlocks(rel); diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index ec99d0bcae..ef6b4c3e54 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -947,31 +947,25 @@ heapgettup_pagemode(HeapScanDesc scan, /* * if current tuple qualifies, return it. */ - if (HeapTupleSatisfies(tuple, scan->rs_scan.rs_snapshot, scan->rs_cbuf)) + if (key != NULL) { - /* - * if current tuple qualifies, return it. - */ - if (key != NULL) - { - bool valid; + bool valid; - HeapKeyTest(tuple, RelationGetDescr(scan->rs_scan.rs_rd), - nkeys, key, valid); - if (valid) - { - scan->rs_cindex = lineindex; - LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK); - return; - } - } - else + HeapKeyTest(tuple, RelationGetDescr(scan->rs_scan.rs_rd), + nkeys, key, valid); + if (valid) { scan->rs_cindex = lineindex; LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK); return; } } + else + { + scan->rs_cindex = lineindex; + LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK); + return; + } /* * otherwise move to the next item on the page diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index c3960dc91f..3254e30a45 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -1741,7 +1741,7 @@ heapam_scan_analyze_next_tuple(TableScanDesc sscan, TransactionId OldestXmin, do { HeapScanDesc scan = (HeapScanDesc) sscan; Page targpage; - OffsetNumber targoffset = scan->rs_cindex; + OffsetNumber targoffset; OffsetNumber maxoffset; BufferHeapTupleTableSlot *hslot; @@ -1751,7 +1751,9 @@ heapam_scan_analyze_next_tuple(TableScanDesc sscan, TransactionId OldestXmin, do maxoffset = PageGetMaxOffsetNumber(targpage); /* Inner loop over all tuples on the selected page */ - for (targoffset = scan->rs_cindex; targoffset <= maxoffset; targoffset++) + for (targoffset = scan->rs_cindex ? scan->rs_cindex : FirstOffsetNumber; + targoffset <= maxoffset; + targoffset++) { ItemId itemid; HeapTuple targtuple = &hslot->base.tupdata; diff --git a/src/backend/access/table/Makefile b/src/backend/access/table/Makefile index fe22bf9208..006ba99182 100644 --- a/src/backend/access/table/Makefile +++ b/src/backend/access/table/Makefile @@ -12,6 +12,6 @@ subdir = src/backend/access/table top_builddir = ../../../.. include $(top_builddir)/src/Makefile.global -OBJS = tableam.o tableamapi.o tableam_common.o +OBJS = tableam.o tableamapi.o include $(top_srcdir)/src/backend/common.mk diff --git a/src/backend/access/table/tableam_common.c b/src/backend/access/table/tableam_common.c deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c index 84de804175..d3ffe417ff 100644 --- a/src/backend/commands/createas.c +++ b/src/backend/commands/createas.c @@ -593,6 +593,10 @@ intorel_receive(TupleTableSlot *slot, DestReceiver *self) if (myState->rel->rd_rel->relhasoids) slot->tts_tupleOid = InvalidOid; + /* Materialize the slot */ + if (!TTS_IS_VIRTUAL(slot)) + ExecMaterializeSlot(slot); + table_insert(myState->rel, slot, myState->output_cid, diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index ef94ac4aa0..12651f5ceb 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -570,6 +570,9 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull) Assert(TTS_IS_HEAPTUPLE(scanslot) || TTS_IS_BUFFERTUPLE(scanslot)); + if (hslot->tuple == NULL) + ExecMaterializeSlot(scanslot); + d = heap_getsysattr(hslot->tuple, attnum, scanslot->tts_tupleDescriptor, op->resnull); diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index e055c0a7c6..34ef86a5bd 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -2594,7 +2594,7 @@ EvalPlanQual(EState *estate, EPQState *epqstate, * datums that may be present in copyTuple). As with the next step, this * is to guard against early re-use of the EPQ query. */ - if (!TupIsNull(slot)) + if (!TupIsNull(slot) && !TTS_IS_VIRTUAL(slot)) ExecMaterializeSlot(slot); #if FIXME @@ -2787,16 +2787,7 @@ EvalPlanQualFetchRowMarks(EPQState *epqstate) if (isNull) continue; - elog(ERROR, "frak, need to implement ROW_MARK_COPY"); -#ifdef FIXME - // FIXME: this should just deform the tuple and store it as a - // virtual one. - tuple = table_tuple_by_datum(erm->relation, datum, erm->relid); - - /* store tuple */ - EvalPlanQualSetTuple(epqstate, erm->rti, tuple); -#endif - + ExecForceStoreHeapTupleDatum(datum, slot); } } } diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c index 917bf80f71..74149cc3ad 100644 --- a/src/backend/executor/execTuples.c +++ b/src/backend/executor/execTuples.c @@ -1364,6 +1364,27 @@ ExecStoreAllNullTuple(TupleTableSlot *slot) return ExecFinishStoreSlotValues(slot); } +void +ExecForceStoreHeapTupleDatum(Datum data, TupleTableSlot *slot) +{ + HeapTuple tuple; + HeapTupleHeader td; + + td = DatumGetHeapTupleHeader(data); + + tuple = (HeapTuple) palloc(HEAPTUPLESIZE + HeapTupleHeaderGetDatumLength(td)); + tuple->t_len = HeapTupleHeaderGetDatumLength(td); + tuple->t_self = td->t_ctid; + tuple->t_data = (HeapTupleHeader) ((char *) tuple + HEAPTUPLESIZE); + memcpy((char *) tuple->t_data, (char *) td, tuple->t_len); + + ExecClearTuple(slot); + + heap_deform_tuple(tuple, slot->tts_tupleDescriptor, + slot->tts_values, slot->tts_isnull); + ExecFinishStoreSlotValues(slot); +} + /* -------------------------------- * ExecFetchSlotTuple * Fetch the slot's regular physical tuple. diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 56880e3d16..36ca07beb2 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -224,6 +224,18 @@ BitmapHeapNext(BitmapHeapScanState *node) BitmapAdjustPrefetchIterator(node, tbmres); + /* + * Ignore any claimed entries past what we think is the end of the + * relation. (This is probably not necessary given that we got at + * least AccessShareLock on the table before performing any of the + * indexscans, but let's be safe.) + */ + if (tbmres->blockno >= scan->rs_nblocks) + { + node->tbmres = tbmres = NULL; + continue; + } + /* * We can skip fetching the heap page if we don't need any fields * from the heap, and the bitmap entries don't need rechecking, diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 3cc9092413..307c12ee69 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -607,7 +607,7 @@ ExecDelete(ModifyTableState *mtstate, bool canSetTag, bool changingPart, bool *tupleDeleted, - TupleTableSlot **epqslot) + TupleTableSlot **epqreturnslot) { ResultRelInfo *resultRelInfo; Relation resultRelationDesc; @@ -632,7 +632,7 @@ ExecDelete(ModifyTableState *mtstate, bool dodelete; dodelete = ExecBRDeleteTriggers(estate, epqstate, resultRelInfo, - tupleid, oldtuple, epqslot); + tupleid, oldtuple, epqreturnslot); if (!dodelete) /* "do nothing" */ return NULL; @@ -724,6 +724,14 @@ ldelete:; /* Tuple no more passing quals, exiting... */ return NULL; } + + /**/ + if (epqreturnslot) + { + *epqreturnslot = epqslot; + return NULL; + } + goto ldelete; } } diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 54382aba88..f030ad25a2 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -4037,7 +4037,6 @@ CreateStatsStmt: * *****************************************************************************/ -// PBORKED: storage option CreateAsStmt: CREATE OptTemp TABLE create_as_target AS SelectStmt opt_with_data { @@ -4068,14 +4067,16 @@ CreateAsStmt: ; create_as_target: - qualified_name opt_column_list OptWith OnCommitOption OptTableSpace + qualified_name opt_column_list table_access_method_clause + OptWith OnCommitOption OptTableSpace { $$ = makeNode(IntoClause); $$->rel = $1; $$->colNames = $2; - $$->options = $3; - $$->onCommit = $4; - $$->tableSpaceName = $5; + $$->accessMethod = $3; + $$->options = $4; + $$->onCommit = $5; + $$->tableSpaceName = $6; $$->viewQuery = NULL; $$->skipData = false; /* might get changed later */ } diff --git a/src/include/executor/tuptable.h b/src/include/executor/tuptable.h index 05f38cfd0d..20fc425a27 100644 --- a/src/include/executor/tuptable.h +++ b/src/include/executor/tuptable.h @@ -476,6 +476,7 @@ extern TupleTableSlot *ExecCopySlot(TupleTableSlot *dstslot, extern void ExecForceStoreHeapTuple(HeapTuple tuple, TupleTableSlot *slot); +extern void ExecForceStoreHeapTupleDatum(Datum data, TupleTableSlot *slot); extern void slot_getmissingattrs(TupleTableSlot *slot, int startAttNum, int lastAttNum); extern Datum slot_getattr(TupleTableSlot *slot, int attnum, bool *isnull); diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index 40f6eb03d2..4d194a8c2a 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -111,6 +111,7 @@ typedef struct IntoClause RangeVar *rel; /* target relation name */ List *colNames; /* column names to assign, or NIL */ + char *accessMethod; /* table access method */ List *options; /* options from WITH clause */ OnCommitAction onCommit; /* what do we do at COMMIT? */ char *tableSpaceName; /* table space to use, or NULL */ -- 2.18.0.windows.1