diff options
Diffstat (limited to 'src')
37 files changed, 686 insertions, 697 deletions
diff --git a/src/Makefile.shlib b/src/Makefile.shlib index fa81f6ffdd6..3825af5b228 100644 --- a/src/Makefile.shlib +++ b/src/Makefile.shlib @@ -112,7 +112,7 @@ ifeq ($(PORTNAME), darwin) ifneq ($(SO_MAJOR_VERSION), 0) version_link = -compatibility_version $(SO_MAJOR_VERSION) -current_version $(SO_MAJOR_VERSION).$(SO_MINOR_VERSION) endif - LINK.shared = $(COMPILER) -dynamiclib -install_name '$(libdir)/lib$(NAME).$(SO_MAJOR_VERSION)$(DLSUFFIX)' $(version_link) $(exported_symbols_list) + LINK.shared = $(COMPILER) -dynamiclib -install_name '$(libdir)/lib$(NAME).$(SO_MAJOR_VERSION)$(DLSUFFIX)' $(version_link) shlib = lib$(NAME).$(SO_MAJOR_VERSION)$(DLSUFFIX) shlib_major = lib$(NAME).$(SO_MAJOR_VERSION)$(DLSUFFIX) else @@ -122,7 +122,7 @@ ifeq ($(PORTNAME), darwin) BUILD.exports = $(AWK) '/^[^\#]/ {printf "_%s\n",$$1}' $< >$@ exports_file = $(SHLIB_EXPORTS:%.txt=%.list) ifneq (,$(exports_file)) - exported_symbols_list = -exported_symbols_list $(exports_file) + LINK.shared += -exported_symbols_list $(exports_file) endif endif diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c index 03a1d7b027a..fdff960c130 100644 --- a/src/backend/access/nbtree/nbtree.c +++ b/src/backend/access/nbtree/nbtree.c @@ -417,6 +417,8 @@ btrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys, * way, so we might as well avoid wasting cycles on acquiring page LSNs. * * See nbtree/README section on making concurrent TID recycling safe. + * + * Note: so->dropPin should never change across rescans. */ so->dropPin = (!scan->xs_want_itup && IsMVCCSnapshot(scan->xs_snapshot) && diff --git a/src/backend/access/nbtree/nbtsearch.c b/src/backend/access/nbtree/nbtsearch.c index 070f14c8b91..36544ecfd58 100644 --- a/src/backend/access/nbtree/nbtsearch.c +++ b/src/backend/access/nbtree/nbtsearch.c @@ -2282,9 +2282,12 @@ _bt_readfirstpage(IndexScanDesc scan, OffsetNumber offnum, ScanDirection dir) * previously-saved right link or left link. lastcurrblkno is the page that * was current at the point where the blkno link was saved, which we use to * reason about concurrent page splits/page deletions during backwards scans. + * In the common case where seized=false, blkno is either so->currPos.nextPage + * or so->currPos.prevPage, and lastcurrblkno is so->currPos.currPage. * - * On entry, caller shouldn't hold any locks or pins on any page (we work - * directly off of blkno and lastcurrblkno instead). Parallel scan callers + * On entry, so->currPos shouldn't be locked by caller. so->currPos.buf must + * be InvalidBuffer/unpinned as needed by caller (note that lastcurrblkno + * won't need to be read again in almost all cases). Parallel scan callers * that seized the scan before calling here should pass seized=true; such a * caller's blkno and lastcurrblkno arguments come from the seized scan. * seized=false callers just pass us the blkno/lastcurrblkno taken from their @@ -2301,8 +2304,8 @@ _bt_readfirstpage(IndexScanDesc scan, OffsetNumber offnum, ScanDirection dir) * success exit (except during so->dropPin index scans, when we drop the pin * eagerly to avoid blocking VACUUM). * - * If there are no more matching records in the given direction, we drop all - * locks and pins, invalidate so->currPos, and return false. + * If there are no more matching records in the given direction, we invalidate + * so->currPos (while ensuring it retains no locks or pins), and return false. * * We always release the scan for a parallel scan caller, regardless of * success or failure; we'll call _bt_parallel_release as soon as possible. diff --git a/src/backend/access/nbtree/nbtutils.c b/src/backend/access/nbtree/nbtutils.c index 29f0dca1b08..c71d1b6f2e1 100644 --- a/src/backend/access/nbtree/nbtutils.c +++ b/src/backend/access/nbtree/nbtutils.c @@ -63,7 +63,7 @@ static bool _bt_check_compare(IndexScanDesc scan, ScanDirection dir, bool *continuescan, int *ikey); static bool _bt_check_rowcompare(ScanKey skey, IndexTuple tuple, int tupnatts, TupleDesc tupdesc, - ScanDirection dir, bool *continuescan); + ScanDirection dir, bool forcenonrequired, bool *continuescan); static void _bt_checkkeys_look_ahead(IndexScanDesc scan, BTReadPageState *pstate, int tupnatts, TupleDesc tupdesc); static int _bt_keep_natts(Relation rel, IndexTuple lastleft, @@ -2902,10 +2902,8 @@ _bt_check_compare(IndexScanDesc scan, ScanDirection dir, /* row-comparison keys need special processing */ if (key->sk_flags & SK_ROW_HEADER) { - Assert(!forcenonrequired); /* forbidden by _bt_set_startikey */ - if (_bt_check_rowcompare(key, tuple, tupnatts, tupdesc, dir, - continuescan)) + forcenonrequired, continuescan)) continue; return false; } @@ -3062,7 +3060,8 @@ _bt_check_compare(IndexScanDesc scan, ScanDirection dir, */ static bool _bt_check_rowcompare(ScanKey skey, IndexTuple tuple, int tupnatts, - TupleDesc tupdesc, ScanDirection dir, bool *continuescan) + TupleDesc tupdesc, ScanDirection dir, + bool forcenonrequired, bool *continuescan) { ScanKey subkey = (ScanKey) DatumGetPointer(skey->sk_argument); int32 cmpresult = 0; @@ -3102,7 +3101,11 @@ _bt_check_rowcompare(ScanKey skey, IndexTuple tuple, int tupnatts, if (isNull) { - if (subkey->sk_flags & SK_BT_NULLS_FIRST) + if (forcenonrequired) + { + /* treating scan's keys as non-required */ + } + else if (subkey->sk_flags & SK_BT_NULLS_FIRST) { /* * Since NULLs are sorted before non-NULLs, we know we have @@ -3156,8 +3159,12 @@ _bt_check_rowcompare(ScanKey skey, IndexTuple tuple, int tupnatts, */ Assert(subkey != (ScanKey) DatumGetPointer(skey->sk_argument)); subkey--; - if ((subkey->sk_flags & SK_BT_REQFWD) && - ScanDirectionIsForward(dir)) + if (forcenonrequired) + { + /* treating scan's keys as non-required */ + } + else if ((subkey->sk_flags & SK_BT_REQFWD) && + ScanDirectionIsForward(dir)) *continuescan = false; else if ((subkey->sk_flags & SK_BT_REQBKWD) && ScanDirectionIsBackward(dir)) @@ -3209,7 +3216,7 @@ _bt_check_rowcompare(ScanKey skey, IndexTuple tuple, int tupnatts, break; } - if (!result) + if (!result && !forcenonrequired) { /* * Tuple fails this qual. If it's a required qual for the current @@ -3323,24 +3330,26 @@ _bt_checkkeys_look_ahead(IndexScanDesc scan, BTReadPageState *pstate, * current page and killed tuples thereon (generally, this should only be * called if so->numKilled > 0). * - * The caller does not have a lock on the page and may or may not have the - * page pinned in a buffer. Note that read-lock is sufficient for setting - * LP_DEAD status (which is only a hint). - * - * We match items by heap TID before assuming they are the right ones to - * delete. We cope with cases where items have moved right due to insertions. - * If an item has moved off the current page due to a split, we'll fail to - * find it and do nothing (this is not an error case --- we assume the item - * will eventually get marked in a future indexscan). + * Caller should not have a lock on the so->currPos page, but must hold a + * buffer pin when !so->dropPin. When we return, it still won't be locked. + * It'll continue to hold whatever pins were held before calling here. * - * Note that if we hold a pin on the target page continuously from initially - * reading the items until applying this function, VACUUM cannot have deleted - * any items on the page, so the page's TIDs can't have been recycled by now. - * There's no risk that we'll confuse a new index tuple that happens to use a - * recycled TID with a now-removed tuple with the same TID (that used to be on - * this same page). We can't rely on that during scans that drop pins eagerly + * We match items by heap TID before assuming they are the right ones to set + * LP_DEAD. If the scan is one that holds a buffer pin on the target page + * continuously from initially reading the items until applying this function + * (if it is a !so->dropPin scan), VACUUM cannot have deleted any items on the + * page, so the page's TIDs can't have been recycled by now. There's no risk + * that we'll confuse a new index tuple that happens to use a recycled TID + * with a now-removed tuple with the same TID (that used to be on this same + * page). We can't rely on that during scans that drop buffer pins eagerly * (so->dropPin scans), though, so we must condition setting LP_DEAD bits on * the page LSN having not changed since back when _bt_readpage saw the page. + * We totally give up on setting LP_DEAD bits when the page LSN changed. + * + * We give up much less often during !so->dropPin scans, but it still happens. + * We cope with cases where items have moved right due to insertions. If an + * item has moved off the current page due to a split, we'll fail to find it + * and just give up on it. */ void _bt_killitems(IndexScanDesc scan) @@ -3353,6 +3362,7 @@ _bt_killitems(IndexScanDesc scan) OffsetNumber maxoff; int numKilled = so->numKilled; bool killedsomething = false; + Buffer buf; Assert(numKilled > 0); Assert(BTScanPosIsValid(so->currPos)); @@ -3369,11 +3379,11 @@ _bt_killitems(IndexScanDesc scan) * concurrent VACUUMs from recycling any of the TIDs on the page. */ Assert(BTScanPosIsPinned(so->currPos)); - _bt_lockbuf(rel, so->currPos.buf, BT_READ); + buf = so->currPos.buf; + _bt_lockbuf(rel, buf, BT_READ); } else { - Buffer buf; XLogRecPtr latestlsn; Assert(!BTScanPosIsPinned(so->currPos)); @@ -3391,10 +3401,9 @@ _bt_killitems(IndexScanDesc scan) } /* Unmodified, hinting is safe */ - so->currPos.buf = buf; } - page = BufferGetPage(so->currPos.buf); + page = BufferGetPage(buf); opaque = BTPageGetOpaque(page); minoff = P_FIRSTDATAKEY(opaque); maxoff = PageGetMaxOffsetNumber(page); @@ -3511,10 +3520,13 @@ _bt_killitems(IndexScanDesc scan) if (killedsomething) { opaque->btpo_flags |= BTP_HAS_GARBAGE; - MarkBufferDirtyHint(so->currPos.buf, true); + MarkBufferDirtyHint(buf, true); } - _bt_unlockbuf(rel, so->currPos.buf); + if (!so->dropPin) + _bt_unlockbuf(rel, buf); + else + _bt_relbuf(rel, buf); } diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 1914859b2ee..47ffc0a2307 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -7498,6 +7498,10 @@ CreateCheckPoint(int flags) if (PriorRedoPtr != InvalidXLogRecPtr) UpdateCheckPointDistanceEstimate(RedoRecPtr - PriorRedoPtr); +#ifdef USE_INJECTION_POINTS + INJECTION_POINT("checkpoint-before-old-wal-removal", NULL); +#endif + /* * Delete old log files, those no longer needed for last checkpoint to * prevent the disk holding the xlog from growing full. diff --git a/src/backend/executor/execGrouping.c b/src/backend/executor/execGrouping.c index 255bd795361..b5400749353 100644 --- a/src/backend/executor/execGrouping.c +++ b/src/backend/executor/execGrouping.c @@ -144,7 +144,7 @@ execTuplesHashPrepare(int numCols, * hashfunctions: FmgrInfos of datatype-specific hashing functions to use * collations: collations to use in comparisons * nbuckets: initial estimate of hashtable size - * additionalsize: size of data stored in ->additional + * additionalsize: size of data that may be stored along with the hash entry * metacxt: memory context for long-lived allocation, but not per-entry data * tablecxt: memory context in which to store table entries * tempcxt: short-lived context for evaluation hash and comparison functions @@ -288,7 +288,7 @@ ResetTupleHashTable(TupleHashTable hashtable) * * If isnew isn't NULL, then a new entry is created if no existing entry * matches. On return, *isnew is true if the entry is newly created, - * false if it existed already. ->additional_data in the new entry has + * false if it existed already. The additional data in the new entry has * been zeroed. */ TupleHashEntry diff --git a/src/backend/executor/nodeTidrangescan.c b/src/backend/executor/nodeTidrangescan.c index ab2eab9596e..26f7420b64b 100644 --- a/src/backend/executor/nodeTidrangescan.c +++ b/src/backend/executor/nodeTidrangescan.c @@ -128,9 +128,11 @@ TidExprListCreate(TidRangeScanState *tidrangestate) * TidRangeEval * * Compute and set node's block and offset range to scan by evaluating - * the trss_tidexprs. Returns false if we detect the range cannot + * node->trss_tidexprs. Returns false if we detect the range cannot * contain any tuples. Returns true if it's possible for the range to - * contain tuples. + * contain tuples. We don't bother validating that trss_mintid is less + * than or equal to trss_maxtid, as the scan_set_tidrange() table AM + * function will handle that. * ---------------------------------------------------------------- */ static bool diff --git a/src/backend/libpq/be-secure-gssapi.c b/src/backend/libpq/be-secure-gssapi.c index 3534f0b8111..5d98c58ffa8 100644 --- a/src/backend/libpq/be-secure-gssapi.c +++ b/src/backend/libpq/be-secure-gssapi.c @@ -121,9 +121,9 @@ be_gssapi_write(Port *port, const void *ptr, size_t len) * again, so if it offers a len less than that, something is wrong. * * Note: it may seem attractive to report partial write completion once - * we've successfully sent any encrypted packets. However, that can cause - * problems for callers; notably, pqPutMsgEnd's heuristic to send only - * full 8K blocks interacts badly with such a hack. We won't save much, + * we've successfully sent any encrypted packets. However, doing that + * expands the state space of this processing and has been responsible for + * bugs in the past (cf. commit d053a879b). We won't save much, * typically, by letting callers discard data early, so don't risk it. */ if (len < PqGSSSendConsumed) diff --git a/src/backend/nodes/gen_node_support.pl b/src/backend/nodes/gen_node_support.pl index c8595109b0e..9ecddb14231 100644 --- a/src/backend/nodes/gen_node_support.pl +++ b/src/backend/nodes/gen_node_support.pl @@ -1329,7 +1329,7 @@ _jumble${n}(JumbleState *jstate, Node *node) # Node type. Squash constants if requested. if ($query_jumble_squash) { - print $jff "\tJUMBLE_ELEMENTS($f);\n" + print $jff "\tJUMBLE_ELEMENTS($f, node);\n" unless $query_jumble_ignore; } else diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 25e08ba3426..eaf391fc2ab 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -653,6 +653,8 @@ _outA_Expr(StringInfo str, const A_Expr *node) WRITE_NODE_FIELD(lexpr); WRITE_NODE_FIELD(rexpr); + WRITE_LOCATION_FIELD(rexpr_list_start); + WRITE_LOCATION_FIELD(rexpr_list_end); WRITE_LOCATION_FIELD(location); } diff --git a/src/backend/nodes/queryjumblefuncs.c b/src/backend/nodes/queryjumblefuncs.c index ac3cb3d9caf..fb33e6931ad 100644 --- a/src/backend/nodes/queryjumblefuncs.c +++ b/src/backend/nodes/queryjumblefuncs.c @@ -61,9 +61,9 @@ static void AppendJumble(JumbleState *jstate, const unsigned char *value, Size size); static void FlushPendingNulls(JumbleState *jstate); static void RecordConstLocation(JumbleState *jstate, - int location, bool squashed); + int location, int len); static void _jumbleNode(JumbleState *jstate, Node *node); -static void _jumbleElements(JumbleState *jstate, List *elements); +static void _jumbleElements(JumbleState *jstate, List *elements, Node *node); static void _jumbleA_Const(JumbleState *jstate, Node *node); static void _jumbleList(JumbleState *jstate, Node *node); static void _jumbleVariableSetStmt(JumbleState *jstate, Node *node); @@ -373,15 +373,17 @@ FlushPendingNulls(JumbleState *jstate) /* - * Record location of constant within query string of query tree that is - * currently being walked. + * Record the location of some kind of constant within a query string. + * These are not only bare constants but also expressions that ultimately + * constitute a constant, such as those inside casts and simple function + * calls. * - * 'squashed' signals that the constant represents the first or the last - * element in a series of merged constants, and everything but the first/last - * element contributes nothing to the jumble hash. + * If length is -1, it indicates a single such constant element. If + * it's a positive integer, it indicates the length of a squashable + * list of them. */ static void -RecordConstLocation(JumbleState *jstate, int location, bool squashed) +RecordConstLocation(JumbleState *jstate, int location, int len) { /* -1 indicates unknown or undefined location */ if (location >= 0) @@ -396,9 +398,14 @@ RecordConstLocation(JumbleState *jstate, int location, bool squashed) sizeof(LocationLen)); } jstate->clocations[jstate->clocations_count].location = location; - /* initialize lengths to -1 to simplify third-party module usage */ - jstate->clocations[jstate->clocations_count].squashed = squashed; - jstate->clocations[jstate->clocations_count].length = -1; + + /* + * Lengths are either positive integers (indicating a squashable + * list), or -1. + */ + Assert(len > -1 || len == -1); + jstate->clocations[jstate->clocations_count].length = len; + jstate->clocations[jstate->clocations_count].squashed = (len > -1); jstate->clocations_count++; } } @@ -408,12 +415,12 @@ RecordConstLocation(JumbleState *jstate, int location, bool squashed) * deduce that the expression is a constant: * * - Ignore a possible wrapping RelabelType and CoerceViaIO. - * - If it's a FuncExpr, check that the function is an implicit + * - If it's a FuncExpr, check that the function is a builtin * cast and its arguments are Const. * - Otherwise test if the expression is a simple Const. */ static bool -IsSquashableConst(Node *element) +IsSquashableConstant(Node *element) { if (IsA(element, RelabelType)) element = (Node *) ((RelabelType *) element)->arg; @@ -421,32 +428,50 @@ IsSquashableConst(Node *element) if (IsA(element, CoerceViaIO)) element = (Node *) ((CoerceViaIO *) element)->arg; - if (IsA(element, FuncExpr)) + switch (nodeTag(element)) { - FuncExpr *func = (FuncExpr *) element; - ListCell *temp; + case T_FuncExpr: + { + FuncExpr *func = (FuncExpr *) element; + ListCell *temp; - if (func->funcformat != COERCE_IMPLICIT_CAST && - func->funcformat != COERCE_EXPLICIT_CAST) - return false; + if (func->funcformat != COERCE_IMPLICIT_CAST && + func->funcformat != COERCE_EXPLICIT_CAST) + return false; - if (func->funcid > FirstGenbkiObjectId) - return false; + if (func->funcid > FirstGenbkiObjectId) + return false; - foreach(temp, func->args) - { - Node *arg = lfirst(temp); + /* + * We can check function arguments recursively, being careful + * about recursing too deep. At each recursion level it's + * enough to test the stack on the first element. (Note that + * I wasn't able to hit this without bloating the stack + * artificially in this function: the parser errors out before + * stack size becomes a problem here.) + */ + foreach(temp, func->args) + { + Node *arg = lfirst(temp); + + if (!IsA(arg, Const)) + { + if (foreach_current_index(temp) == 0 && + stack_is_too_deep()) + return false; + else if (!IsSquashableConstant(arg)) + return false; + } + } + + return true; + } - if (!IsA(arg, Const)) /* XXX we could recurse here instead */ + default: + if (!IsA(element, Const)) return false; - } - - return true; } - if (!IsA(element, Const)) - return false; - return true; } @@ -461,35 +486,29 @@ IsSquashableConst(Node *element) * expressions. */ static bool -IsSquashableConstList(List *elements, Node **firstExpr, Node **lastExpr) +IsSquashableConstantList(List *elements) { ListCell *temp; - /* - * If squashing is disabled, or the list is too short, we don't try to - * squash it. - */ + /* If the list is too short, we don't try to squash it. */ if (list_length(elements) < 2) return false; foreach(temp, elements) { - if (!IsSquashableConst(lfirst(temp))) + if (!IsSquashableConstant(lfirst(temp))) return false; } - *firstExpr = linitial(elements); - *lastExpr = llast(elements); - return true; } #define JUMBLE_NODE(item) \ _jumbleNode(jstate, (Node *) expr->item) -#define JUMBLE_ELEMENTS(list) \ - _jumbleElements(jstate, (List *) expr->list) +#define JUMBLE_ELEMENTS(list, node) \ + _jumbleElements(jstate, (List *) expr->list, node) #define JUMBLE_LOCATION(location) \ - RecordConstLocation(jstate, expr->location, false) + RecordConstLocation(jstate, expr->location, -1) #define JUMBLE_FIELD(item) \ do { \ if (sizeof(expr->item) == 8) \ @@ -517,36 +536,36 @@ do { \ #include "queryjumblefuncs.funcs.c" /* - * We jumble lists of constant elements as one individual item regardless - * of how many elements are in the list. This means different queries - * jumble to the same query_id, if the only difference is the number of - * elements in the list. + * We try to jumble lists of expressions as one individual item regardless + * of how many elements are in the list. This is know as squashing, which + * results in different queries jumbling to the same query_id, if the only + * difference is the number of elements in the list. + * + * We allow constants to be squashed. To normalize such queries, we use + * the start and end locations of the list of elements in a list. */ static void -_jumbleElements(JumbleState *jstate, List *elements) +_jumbleElements(JumbleState *jstate, List *elements, Node *node) { - Node *first, - *last; + bool normalize_list = false; - if (IsSquashableConstList(elements, &first, &last)) + if (IsSquashableConstantList(elements)) { - /* - * If this list of elements is squashable, keep track of the location - * of its first and last elements. When reading back the locations - * array, we'll see two consecutive locations with ->squashed set to - * true, indicating the location of initial and final elements of this - * list. - * - * For the limited set of cases we support now (implicit coerce via - * FuncExpr, Const) it's fine to use exprLocation of the 'last' - * expression, but if more complex composite expressions are to be - * supported (e.g., OpExpr or FuncExpr as an explicit call), more - * sophisticated tracking will be needed. - */ - RecordConstLocation(jstate, exprLocation(first), true); - RecordConstLocation(jstate, exprLocation(last), true); + if (IsA(node, ArrayExpr)) + { + ArrayExpr *aexpr = (ArrayExpr *) node; + + if (aexpr->list_start > 0 && aexpr->list_end > 0) + { + RecordConstLocation(jstate, + aexpr->list_start + 1, + (aexpr->list_end - aexpr->list_start) - 1); + normalize_list = true; + } + } } - else + + if (!normalize_list) { _jumbleNode(jstate, (Node *) elements); } diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index 8c90ab54af8..48b5d13b9b6 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -526,6 +526,8 @@ _readA_Expr(void) READ_NODE_FIELD(lexpr); READ_NODE_FIELD(rexpr); + READ_LOCATION_FIELD(rexpr_list_start); + READ_LOCATION_FIELD(rexpr_list_end); READ_LOCATION_FIELD(location); READ_DONE(); diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index a16fdd65601..34f7c17f576 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -239,102 +239,23 @@ parse_sub_analyze(Node *parseTree, ParseState *parentParseState, } /* - * setQueryLocationAndLength - * Set query's location and length from statement and ParseState - * - * Some statements, like PreparableStmt, can be located within parentheses. - * For example "(SELECT 1)" or "COPY (UPDATE ...) to x;". For those, we - * cannot use the whole string from the statement's location or the SQL - * string would yield incorrectly. The parser will set stmt_len, reflecting - * the size of the statement within the parentheses. Thus, when stmt_len is - * available, we need to use it for the Query's stmt_len. - * - * For other cases, the parser can't provide the length of individual - * statements. However, we have the statement's location plus the length - * (p_stmt_len) and location (p_stmt_location) of the top level RawStmt, - * stored in pstate. Thus, the statement's length is the RawStmt's length - * minus how much we've advanced in the RawStmt's string. If p_stmt_len - * is 0, the SQL string is used up to its end. - */ -static void -setQueryLocationAndLength(ParseState *pstate, Query *qry, Node *parseTree) -{ - ParseLoc stmt_len = 0; - - switch (nodeTag(parseTree)) - { - case T_InsertStmt: - qry->stmt_location = ((InsertStmt *) parseTree)->stmt_location; - stmt_len = ((InsertStmt *) parseTree)->stmt_len; - break; - - case T_DeleteStmt: - qry->stmt_location = ((DeleteStmt *) parseTree)->stmt_location; - stmt_len = ((DeleteStmt *) parseTree)->stmt_len; - break; - - case T_UpdateStmt: - qry->stmt_location = ((UpdateStmt *) parseTree)->stmt_location; - stmt_len = ((UpdateStmt *) parseTree)->stmt_len; - break; - - case T_MergeStmt: - qry->stmt_location = ((MergeStmt *) parseTree)->stmt_location; - stmt_len = ((MergeStmt *) parseTree)->stmt_len; - break; - - case T_SelectStmt: - qry->stmt_location = ((SelectStmt *) parseTree)->stmt_location; - stmt_len = ((SelectStmt *) parseTree)->stmt_len; - break; - - case T_PLAssignStmt: - qry->stmt_location = ((PLAssignStmt *) parseTree)->location; - break; - - default: - qry->stmt_location = pstate->p_stmt_location; - break; - } - - if (stmt_len > 0) - { - /* Statement's length is known, use it */ - qry->stmt_len = stmt_len; - } - else if (pstate->p_stmt_len > 0) - { - /* - * The top RawStmt's length is known, so calculate the statement's - * length from the statement's location and the RawStmt's length and - * location. - */ - qry->stmt_len = pstate->p_stmt_len - (qry->stmt_location - pstate->p_stmt_location); - } - - /* The calculated statement length should be calculated as positive. */ - Assert(qry->stmt_len >= 0); -} - -/* * transformTopLevelStmt - * transform a Parse tree into a Query tree. * - * This function is just responsible for storing location data - * from the RawStmt into the ParseState. + * This function is just responsible for transferring statement location data + * from the RawStmt into the finished Query. */ Query * transformTopLevelStmt(ParseState *pstate, RawStmt *parseTree) { Query *result; - /* Store RawStmt's length and location in pstate */ - pstate->p_stmt_len = parseTree->stmt_len; - pstate->p_stmt_location = parseTree->stmt_location; - /* We're at top level, so allow SELECT INTO */ result = transformOptionalSelectInto(pstate, parseTree->stmt); + result->stmt_location = parseTree->stmt_location; + result->stmt_len = parseTree->stmt_len; + return result; } @@ -503,7 +424,6 @@ transformStmt(ParseState *pstate, Node *parseTree) /* Mark as original query until we learn differently */ result->querySource = QSRC_ORIGINAL; result->canSetTag = true; - setQueryLocationAndLength(pstate, result, parseTree); return result; } diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 0b5652071d1..50f53159d58 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -154,7 +154,6 @@ static void base_yyerror(YYLTYPE *yylloc, core_yyscan_t yyscanner, const char *msg); static RawStmt *makeRawStmt(Node *stmt, int stmt_location); static void updateRawStmtEnd(RawStmt *rs, int end_location); -static void updatePreparableStmtEnd(Node *n, int end_location); static Node *makeColumnRef(char *colname, List *indirection, int location, core_yyscan_t yyscanner); static Node *makeTypeCast(Node *arg, TypeName *typename, int location); @@ -178,13 +177,13 @@ static void insertSelectOptions(SelectStmt *stmt, SelectLimit *limitClause, WithClause *withClause, core_yyscan_t yyscanner); -static Node *makeSetOp(SetOperation op, bool all, Node *larg, Node *rarg, int location); +static Node *makeSetOp(SetOperation op, bool all, Node *larg, Node *rarg); static Node *doNegate(Node *n, int location); static void doNegateFloat(Float *v); static Node *makeAndExpr(Node *lexpr, Node *rexpr, int location); static Node *makeOrExpr(Node *lexpr, Node *rexpr, int location); static Node *makeNotExpr(Node *expr, int location); -static Node *makeAArrayExpr(List *elements, int location); +static Node *makeAArrayExpr(List *elements, int location, int end_location); static Node *makeSQLValueFunction(SQLValueFunctionOp op, int32 typmod, int location); static Node *makeXmlExpr(XmlExprOp op, char *name, List *named_args, @@ -523,7 +522,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type <defelt> def_elem reloption_elem old_aggr_elem operator_def_elem %type <node> def_arg columnElem where_clause where_or_current_clause a_expr b_expr c_expr AexprConst indirection_el opt_slice_bound - columnref in_expr having_clause func_table xmltable array_expr + columnref having_clause func_table xmltable array_expr OptWhereClause operator_def_arg %type <list> opt_column_and_period_list %type <list> rowsfrom_item rowsfrom_list opt_col_def_list @@ -3417,7 +3416,6 @@ CopyStmt: COPY opt_binary qualified_name opt_column_list { CopyStmt *n = makeNode(CopyStmt); - updatePreparableStmtEnd($3, @4); n->relation = NULL; n->query = $3; n->attlist = NIL; @@ -12240,7 +12238,6 @@ InsertStmt: $5->onConflictClause = $6; $5->returningClause = $7; $5->withClause = $1; - $5->stmt_location = @$; $$ = (Node *) $5; } ; @@ -12431,7 +12428,6 @@ DeleteStmt: opt_with_clause DELETE_P FROM relation_expr_opt_alias n->whereClause = $6; n->returningClause = $7; n->withClause = $1; - n->stmt_location = @$; $$ = (Node *) n; } ; @@ -12506,7 +12502,6 @@ UpdateStmt: opt_with_clause UPDATE relation_expr_opt_alias n->whereClause = $7; n->returningClause = $8; n->withClause = $1; - n->stmt_location = @$; $$ = (Node *) n; } ; @@ -12584,7 +12579,6 @@ MergeStmt: m->joinCondition = $8; m->mergeWhenClauses = $9; m->returningClause = $10; - m->stmt_location = @$; $$ = (Node *) m; } @@ -12825,20 +12819,7 @@ SelectStmt: select_no_parens %prec UMINUS ; select_with_parens: - '(' select_no_parens ')' - { - SelectStmt *n = (SelectStmt *) $2; - - /* - * As SelectStmt's location starts at the SELECT keyword, - * we need to track the length of the SelectStmt within - * parentheses to be able to extract the relevant part - * of the query. Without this, the RawStmt's length would - * be used and would include the closing parenthesis. - */ - n->stmt_len = @3 - @2; - $$ = $2; - } + '(' select_no_parens ')' { $$ = $2; } | '(' select_with_parens ')' { $$ = $2; } ; @@ -12960,7 +12941,6 @@ simple_select: n->groupDistinct = ($7)->distinct; n->havingClause = $8; n->windowClause = $9; - n->stmt_location = @1; $$ = (Node *) n; } | SELECT distinct_clause target_list @@ -12978,7 +12958,6 @@ simple_select: n->groupDistinct = ($7)->distinct; n->havingClause = $8; n->windowClause = $9; - n->stmt_location = @1; $$ = (Node *) n; } | values_clause { $$ = $1; } @@ -12999,20 +12978,19 @@ simple_select: n->targetList = list_make1(rt); n->fromClause = list_make1($2); - n->stmt_location = @1; $$ = (Node *) n; } | select_clause UNION set_quantifier select_clause { - $$ = makeSetOp(SETOP_UNION, $3 == SET_QUANTIFIER_ALL, $1, $4, @1); + $$ = makeSetOp(SETOP_UNION, $3 == SET_QUANTIFIER_ALL, $1, $4); } | select_clause INTERSECT set_quantifier select_clause { - $$ = makeSetOp(SETOP_INTERSECT, $3 == SET_QUANTIFIER_ALL, $1, $4, @1); + $$ = makeSetOp(SETOP_INTERSECT, $3 == SET_QUANTIFIER_ALL, $1, $4); } | select_clause EXCEPT set_quantifier select_clause { - $$ = makeSetOp(SETOP_EXCEPT, $3 == SET_QUANTIFIER_ALL, $1, $4, @1); + $$ = makeSetOp(SETOP_EXCEPT, $3 == SET_QUANTIFIER_ALL, $1, $4); } ; @@ -13590,7 +13568,6 @@ values_clause: { SelectStmt *n = makeNode(SelectStmt); - n->stmt_location = @1; n->valuesLists = list_make1($3); $$ = (Node *) n; } @@ -15287,49 +15264,50 @@ a_expr: c_expr { $$ = $1; } (Node *) list_make2($5, $7), @2); } - | a_expr IN_P in_expr + | a_expr IN_P select_with_parens { - /* in_expr returns a SubLink or a list of a_exprs */ - if (IsA($3, SubLink)) - { - /* generate foo = ANY (subquery) */ - SubLink *n = (SubLink *) $3; + /* generate foo = ANY (subquery) */ + SubLink *n = makeNode(SubLink); - n->subLinkType = ANY_SUBLINK; - n->subLinkId = 0; - n->testexpr = $1; - n->operName = NIL; /* show it's IN not = ANY */ - n->location = @2; - $$ = (Node *) n; - } - else - { - /* generate scalar IN expression */ - $$ = (Node *) makeSimpleA_Expr(AEXPR_IN, "=", $1, $3, @2); - } + n->subselect = $3; + n->subLinkType = ANY_SUBLINK; + n->subLinkId = 0; + n->testexpr = $1; + n->operName = NIL; /* show it's IN not = ANY */ + n->location = @2; + $$ = (Node *) n; } - | a_expr NOT_LA IN_P in_expr %prec NOT_LA + | a_expr IN_P '(' expr_list ')' { - /* in_expr returns a SubLink or a list of a_exprs */ - if (IsA($4, SubLink)) - { - /* generate NOT (foo = ANY (subquery)) */ - /* Make an = ANY node */ - SubLink *n = (SubLink *) $4; - - n->subLinkType = ANY_SUBLINK; - n->subLinkId = 0; - n->testexpr = $1; - n->operName = NIL; /* show it's IN not = ANY */ - n->location = @2; - /* Stick a NOT on top; must have same parse location */ - $$ = makeNotExpr((Node *) n, @2); - } - else - { - /* generate scalar NOT IN expression */ - $$ = (Node *) makeSimpleA_Expr(AEXPR_IN, "<>", $1, $4, @2); - } + /* generate scalar IN expression */ + A_Expr *n = makeSimpleA_Expr(AEXPR_IN, "=", $1, (Node *) $4, @2); + + n->rexpr_list_start = @3; + n->rexpr_list_end = @5; + $$ = (Node *) n; + } + | a_expr NOT_LA IN_P select_with_parens %prec NOT_LA + { + /* generate NOT (foo = ANY (subquery)) */ + SubLink *n = makeNode(SubLink); + + n->subselect = $4; + n->subLinkType = ANY_SUBLINK; + n->subLinkId = 0; + n->testexpr = $1; + n->operName = NIL; /* show it's IN not = ANY */ + n->location = @2; + /* Stick a NOT on top; must have same parse location */ + $$ = makeNotExpr((Node *) n, @2); + } + | a_expr NOT_LA IN_P '(' expr_list ')' + { + /* generate scalar NOT IN expression */ + A_Expr *n = makeSimpleA_Expr(AEXPR_IN, "<>", $1, (Node *) $5, @2); + + n->rexpr_list_start = @4; + n->rexpr_list_end = @6; + $$ = (Node *) n; } | a_expr subquery_Op sub_type select_with_parens %prec Op { @@ -16764,15 +16742,15 @@ type_list: Typename { $$ = list_make1($1); } array_expr: '[' expr_list ']' { - $$ = makeAArrayExpr($2, @1); + $$ = makeAArrayExpr($2, @1, @3); } | '[' array_expr_list ']' { - $$ = makeAArrayExpr($2, @1); + $$ = makeAArrayExpr($2, @1, @3); } | '[' ']' { - $$ = makeAArrayExpr(NIL, @1); + $$ = makeAArrayExpr(NIL, @1, @2); } ; @@ -16894,17 +16872,6 @@ trim_list: a_expr FROM expr_list { $$ = lappend($3, $1); } | expr_list { $$ = $1; } ; -in_expr: select_with_parens - { - SubLink *n = makeNode(SubLink); - - n->subselect = $1; - /* other fields will be filled later */ - $$ = (Node *) n; - } - | '(' expr_list ')' { $$ = (Node *) $2; } - ; - /* * Define SQL-style CASE clause. * - Full specification @@ -18748,47 +18715,6 @@ updateRawStmtEnd(RawStmt *rs, int end_location) rs->stmt_len = end_location - rs->stmt_location; } -/* - * Adjust a PreparableStmt to reflect that it doesn't run to the end of the - * string. - */ -static void -updatePreparableStmtEnd(Node *n, int end_location) -{ - if (IsA(n, SelectStmt)) - { - SelectStmt *stmt = (SelectStmt *) n; - - stmt->stmt_len = end_location - stmt->stmt_location; - } - else if (IsA(n, InsertStmt)) - { - InsertStmt *stmt = (InsertStmt *) n; - - stmt->stmt_len = end_location - stmt->stmt_location; - } - else if (IsA(n, UpdateStmt)) - { - UpdateStmt *stmt = (UpdateStmt *) n; - - stmt->stmt_len = end_location - stmt->stmt_location; - } - else if (IsA(n, DeleteStmt)) - { - DeleteStmt *stmt = (DeleteStmt *) n; - - stmt->stmt_len = end_location - stmt->stmt_location; - } - else if (IsA(n, MergeStmt)) - { - MergeStmt *stmt = (MergeStmt *) n; - - stmt->stmt_len = end_location - stmt->stmt_location; - } - else - elog(ERROR, "unexpected node type %d", (int) n->type); -} - static Node * makeColumnRef(char *colname, List *indirection, int location, core_yyscan_t yyscanner) @@ -19167,14 +19093,11 @@ insertSelectOptions(SelectStmt *stmt, errmsg("multiple WITH clauses not allowed"), parser_errposition(exprLocation((Node *) withClause)))); stmt->withClause = withClause; - - /* Update SelectStmt's location to the start of the WITH clause */ - stmt->stmt_location = withClause->location; } } static Node * -makeSetOp(SetOperation op, bool all, Node *larg, Node *rarg, int location) +makeSetOp(SetOperation op, bool all, Node *larg, Node *rarg) { SelectStmt *n = makeNode(SelectStmt); @@ -19182,7 +19105,6 @@ makeSetOp(SetOperation op, bool all, Node *larg, Node *rarg, int location) n->all = all; n->larg = (SelectStmt *) larg; n->rarg = (SelectStmt *) rarg; - n->stmt_location = location; return (Node *) n; } @@ -19300,12 +19222,14 @@ makeNotExpr(Node *expr, int location) } static Node * -makeAArrayExpr(List *elements, int location) +makeAArrayExpr(List *elements, int location, int location_end) { A_ArrayExpr *n = makeNode(A_ArrayExpr); n->elements = elements; n->location = location; + n->list_start = location; + n->list_end = location_end; return (Node *) n; } diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index 1f8e2d54673..d66276801c6 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -1223,6 +1223,8 @@ transformAExprIn(ParseState *pstate, A_Expr *a) newa->element_typeid = scalar_type; newa->elements = aexprs; newa->multidims = false; + newa->list_start = a->rexpr_list_start; + newa->list_end = a->rexpr_list_end; newa->location = -1; result = (Node *) make_scalar_array_op(pstate, @@ -2165,6 +2167,8 @@ transformArrayExpr(ParseState *pstate, A_ArrayExpr *a, /* array_collid will be set by parse_collate.c */ newa->element_typeid = element_type; newa->elements = newcoercedelems; + newa->list_start = a->list_start; + newa->list_end = a->list_end; newa->location = a->location; return (Node *) newa; diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c index 1d56d0c4ef3..f1eb798f3e9 100644 --- a/src/backend/replication/logical/logical.c +++ b/src/backend/replication/logical/logical.c @@ -29,6 +29,7 @@ #include "postgres.h" #include "access/xact.h" +#include "access/xlog_internal.h" #include "access/xlogutils.h" #include "fmgr.h" #include "miscadmin.h" @@ -41,6 +42,7 @@ #include "storage/proc.h" #include "storage/procarray.h" #include "utils/builtins.h" +#include "utils/injection_point.h" #include "utils/inval.h" #include "utils/memutils.h" @@ -1825,9 +1827,13 @@ LogicalConfirmReceivedLocation(XLogRecPtr lsn) { bool updated_xmin = false; bool updated_restart = false; + XLogRecPtr restart_lsn pg_attribute_unused(); SpinLockAcquire(&MyReplicationSlot->mutex); + /* remember the old restart lsn */ + restart_lsn = MyReplicationSlot->data.restart_lsn; + /* * Prevent moving the confirmed_flush backwards, as this could lead to * data duplication issues caused by replicating already replicated @@ -1881,6 +1887,18 @@ LogicalConfirmReceivedLocation(XLogRecPtr lsn) /* first write new xmin to disk, so we know what's up after a crash */ if (updated_xmin || updated_restart) { +#ifdef USE_INJECTION_POINTS + XLogSegNo seg1, + seg2; + + XLByteToSeg(restart_lsn, seg1, wal_segment_size); + XLByteToSeg(MyReplicationSlot->data.restart_lsn, seg2, wal_segment_size); + + /* trigger injection point, but only if segment changes */ + if (seg1 != seg2) + INJECTION_POINT("logical-replication-slot-advance-segment", NULL); +#endif + ReplicationSlotMarkDirty(); ReplicationSlotSave(); elog(DEBUG1, "updated xmin: %u restart: %u", updated_xmin, updated_restart); diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c index 600b87fa9cb..c64f020742f 100644 --- a/src/backend/replication/slot.c +++ b/src/backend/replication/slot.c @@ -424,6 +424,7 @@ ReplicationSlotCreate(const char *name, bool db_specific, slot->candidate_restart_valid = InvalidXLogRecPtr; slot->candidate_restart_lsn = InvalidXLogRecPtr; slot->last_saved_confirmed_flush = InvalidXLogRecPtr; + slot->last_saved_restart_lsn = InvalidXLogRecPtr; slot->inactive_since = 0; /* @@ -1165,20 +1166,41 @@ ReplicationSlotsComputeRequiredLSN(void) { ReplicationSlot *s = &ReplicationSlotCtl->replication_slots[i]; XLogRecPtr restart_lsn; + XLogRecPtr last_saved_restart_lsn; bool invalidated; + ReplicationSlotPersistency persistency; if (!s->in_use) continue; SpinLockAcquire(&s->mutex); + persistency = s->data.persistency; restart_lsn = s->data.restart_lsn; invalidated = s->data.invalidated != RS_INVAL_NONE; + last_saved_restart_lsn = s->last_saved_restart_lsn; SpinLockRelease(&s->mutex); /* invalidated slots need not apply */ if (invalidated) continue; + /* + * For persistent slot use last_saved_restart_lsn to compute the + * oldest LSN for removal of WAL segments. The segments between + * last_saved_restart_lsn and restart_lsn might be needed by a + * persistent slot in the case of database crash. Non-persistent + * slots can't survive the database crash, so we don't care about + * last_saved_restart_lsn for them. + */ + if (persistency == RS_PERSISTENT) + { + if (last_saved_restart_lsn != InvalidXLogRecPtr && + restart_lsn > last_saved_restart_lsn) + { + restart_lsn = last_saved_restart_lsn; + } + } + if (restart_lsn != InvalidXLogRecPtr && (min_required == InvalidXLogRecPtr || restart_lsn < min_required)) @@ -1216,7 +1238,9 @@ ReplicationSlotsComputeLogicalRestartLSN(void) { ReplicationSlot *s; XLogRecPtr restart_lsn; + XLogRecPtr last_saved_restart_lsn; bool invalidated; + ReplicationSlotPersistency persistency; s = &ReplicationSlotCtl->replication_slots[i]; @@ -1230,14 +1254,33 @@ ReplicationSlotsComputeLogicalRestartLSN(void) /* read once, it's ok if it increases while we're checking */ SpinLockAcquire(&s->mutex); + persistency = s->data.persistency; restart_lsn = s->data.restart_lsn; invalidated = s->data.invalidated != RS_INVAL_NONE; + last_saved_restart_lsn = s->last_saved_restart_lsn; SpinLockRelease(&s->mutex); /* invalidated slots need not apply */ if (invalidated) continue; + /* + * For persistent slot use last_saved_restart_lsn to compute the + * oldest LSN for removal of WAL segments. The segments between + * last_saved_restart_lsn and restart_lsn might be needed by a + * persistent slot in the case of database crash. Non-persistent + * slots can't survive the database crash, so we don't care about + * last_saved_restart_lsn for them. + */ + if (persistency == RS_PERSISTENT) + { + if (last_saved_restart_lsn != InvalidXLogRecPtr && + restart_lsn > last_saved_restart_lsn) + { + restart_lsn = last_saved_restart_lsn; + } + } + if (restart_lsn == InvalidXLogRecPtr) continue; @@ -1455,6 +1498,7 @@ ReplicationSlotReserveWal(void) Assert(slot != NULL); Assert(slot->data.restart_lsn == InvalidXLogRecPtr); + Assert(slot->last_saved_restart_lsn == InvalidXLogRecPtr); /* * The replication slot mechanism is used to prevent removal of required @@ -1766,6 +1810,8 @@ InvalidatePossiblyObsoleteSlot(uint32 possible_causes, */ SpinLockAcquire(&s->mutex); + Assert(s->data.restart_lsn >= s->last_saved_restart_lsn); + restart_lsn = s->data.restart_lsn; /* we do nothing if the slot is already invalid */ @@ -1835,7 +1881,10 @@ InvalidatePossiblyObsoleteSlot(uint32 possible_causes, * just rely on .invalidated. */ if (invalidation_cause == RS_INVAL_WAL_REMOVED) + { s->data.restart_lsn = InvalidXLogRecPtr; + s->last_saved_restart_lsn = InvalidXLogRecPtr; + } /* Let caller know */ *invalidated = true; @@ -2079,6 +2128,12 @@ CheckPointReplicationSlots(bool is_shutdown) SaveSlotToPath(s, path, LOG); } LWLockRelease(ReplicationSlotAllocationLock); + + /* + * Recompute the required LSN as SaveSlotToPath() updated + * last_saved_restart_lsn for slots. + */ + ReplicationSlotsComputeRequiredLSN(); } /* @@ -2354,6 +2409,7 @@ SaveSlotToPath(ReplicationSlot *slot, const char *dir, int elevel) if (!slot->just_dirtied) slot->dirty = false; slot->last_saved_confirmed_flush = cp.slotdata.confirmed_flush; + slot->last_saved_restart_lsn = cp.slotdata.restart_lsn; SpinLockRelease(&slot->mutex); LWLockRelease(&slot->io_in_progress_lock); @@ -2569,6 +2625,7 @@ RestoreSlotFromDisk(const char *name) slot->effective_xmin = cp.slotdata.xmin; slot->effective_catalog_xmin = cp.slotdata.catalog_xmin; slot->last_saved_confirmed_flush = cp.slotdata.confirmed_flush; + slot->last_saved_restart_lsn = cp.slotdata.restart_lsn; slot->candidate_catalog_xmin = InvalidTransactionId; slot->candidate_xmin_lsn = InvalidXLogRecPtr; diff --git a/src/backend/storage/aio/method_io_uring.c b/src/backend/storage/aio/method_io_uring.c index cc312b641ca..b78048328e1 100644 --- a/src/backend/storage/aio/method_io_uring.c +++ b/src/backend/storage/aio/method_io_uring.c @@ -400,9 +400,9 @@ pgaio_uring_wait_one(PgAioHandle *ioh, uint64 ref_generation) while (true) { pgaio_debug_io(DEBUG3, ioh, - "wait_one io_gen: %llu, ref_gen: %llu, cycle %d", - (long long unsigned) ioh->generation, - (long long unsigned) ref_generation, + "wait_one io_gen: %" PRIu64 ", ref_gen: %" PRIu64 ", cycle %d", + ioh->generation, + ref_generation, waited); if (pgaio_io_was_recycled(ioh, ref_generation, &state) || diff --git a/src/bin/pg_dump/pg_backup_directory.c b/src/bin/pg_dump/pg_backup_directory.c index 21b00792a8a..bc2a2fb4797 100644 --- a/src/bin/pg_dump/pg_backup_directory.c +++ b/src/bin/pg_dump/pg_backup_directory.c @@ -412,10 +412,15 @@ _LoadLOs(ArchiveHandle *AH, TocEntry *te) /* * Note: before archive v16, there was always only one BLOBS TOC entry, - * now there can be multiple. We don't need to worry what version we are - * reading though, because tctx->filename should be correct either way. + * now there can be multiple. Furthermore, although the actual filename + * was always "blobs.toc" before v16, the value of tctx->filename did not + * match that before commit 548e50976 fixed it. For simplicity we assume + * it must be "blobs.toc" in all archives before v16. */ - setFilePath(AH, tocfname, tctx->filename); + if (AH->version < K_VERS_1_16) + setFilePath(AH, tocfname, "blobs.toc"); + else + setFilePath(AH, tocfname, tctx->filename); CFH = ctx->LOsTocFH = InitDiscoverCompressFileHandle(tocfname, PG_BINARY_R); diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c index f2182e91825..c4b6214d618 100644 --- a/src/bin/pg_dump/pg_restore.c +++ b/src/bin/pg_dump/pg_restore.c @@ -712,9 +712,9 @@ usage(const char *progname) printf(_(" --use-set-session-authorization\n" " use SET SESSION AUTHORIZATION commands instead of\n" " ALTER OWNER commands to set ownership\n")); - printf(_(" --with-data dump the data\n")); - printf(_(" --with-schema dump the schema\n")); - printf(_(" --with-statistics dump the statistics\n")); + printf(_(" --with-data restore the data\n")); + printf(_(" --with-schema restore the schema\n")); + printf(_(" --with-statistics restore the statistics\n")); printf(_("\nConnection options:\n")); printf(_(" -h, --host=HOSTNAME database server host or socket directory\n")); diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 81a5ba844ba..1f7635d0c23 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -778,6 +778,7 @@ exec_command_conninfo(PsqlScanState scan_state, bool active_branch) int ssl_in_use, password_used, gssapi_used; + int version_num; char *paramval; if (!active_branch) @@ -793,7 +794,9 @@ exec_command_conninfo(PsqlScanState scan_state, bool active_branch) /* Get values for the parameters */ host = PQhost(pset.db); hostaddr = PQhostaddr(pset.db); - protocol_version = psprintf("%d", PQprotocolVersion(pset.db)); + version_num = PQfullProtocolVersion(pset.db); + protocol_version = psprintf("%d.%d", version_num / 10000, + version_num % 10000); ssl_in_use = PQsslInUse(pset.db); password_used = PQconnectionUsedPassword(pset.db); gssapi_used = PQconnectionUsedGSSAPI(pset.db); diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c index 47352b7faed..b53cd8ab698 100644 --- a/src/bin/psql/common.c +++ b/src/bin/psql/common.c @@ -1867,18 +1867,30 @@ ExecQueryAndProcessResults(const char *query, { FILE *copy_stream = NULL; - if (pset.piped_syncs > 1) + if (PQpipelineStatus(pset.db) != PQ_PIPELINE_OFF) { /* - * When reading COPY data, the backend ignores sync messages - * and will not send a matching ReadyForQuery response. Even - * if we adjust piped_syncs and requested_results, it is not - * possible to salvage this as the sync message would still be - * in libpq's command queue and we would be stuck in a busy - * pipeline state. Thus, we abort the connection to avoid - * this state. + * Running COPY within a pipeline can break the protocol + * synchronisation in multiple ways, and psql shows its limits + * when it comes to tracking this information. + * + * While in COPY mode, the backend process ignores additional + * Sync messages and will not send the matching ReadyForQuery + * expected by the frontend. + * + * Additionally, libpq automatically sends a Sync with the + * Copy message, creating an unexpected synchronisation point. + * A failure during COPY would leave the pipeline in an + * aborted state while the backend would be in a clean state, + * ready to process commands. + * + * Improving those issues would require modifications in how + * libpq handles pipelines and COPY. Hence, for the time + * being, we forbid the use of COPY within a pipeline, + * aborting the connection to avoid an inconsistent state on + * psql side if trying to use a COPY command. */ - pg_log_info("\\syncpipeline after COPY is not supported, aborting connection"); + pg_log_info("COPY in a pipeline is not supported, aborting connection"); exit(EXIT_BADCONN); } diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c index 403b51325a7..ce05b3a5132 100644 --- a/src/bin/psql/help.c +++ b/src/bin/psql/help.c @@ -463,8 +463,9 @@ helpVariables(unsigned short int pager) " VERSION_NAME\n" " VERSION_NUM\n" " psql's version (in verbose string, short string, or numeric format)\n"); - HELP0(" WATCH_INTERVAL\n" - " if set to a number, overrides the default two second \\watch interval\n"); + HELPN(" WATCH_INTERVAL\n" + " number of seconds \\watch waits between executions (default %s)\n", + DEFAULT_WATCH_INTERVAL); HELP0("\nDisplay settings:\n"); HELP0("Usage:\n"); diff --git a/src/bin/psql/t/001_basic.pl b/src/bin/psql/t/001_basic.pl index ae5c1d66405..f42c3961e09 100644 --- a/src/bin/psql/t/001_basic.pl +++ b/src/bin/psql/t/001_basic.pl @@ -483,8 +483,8 @@ psql_like($node, "copy (values ('foo'),('bar')) to stdout \\g | $pipe_cmd", my $c4 = slurp_file($g_file); like($c4, qr/foo.*bar/s); -# Tests with pipelines. These trigger FATAL failures in the backend, -# so they cannot be tested via SQL. +# Test COPY within pipelines. These abort the connection from +# the frontend so they cannot be tested via SQL. $node->safe_psql('postgres', 'CREATE TABLE psql_pipeline()'); my $log_location = -s $node->logfile; psql_fails_like( @@ -493,53 +493,41 @@ psql_fails_like( COPY psql_pipeline FROM STDIN; SELECT 'val1'; \\syncpipeline -\\getresults \\endpipeline}, - qr/server closed the connection unexpectedly/, - 'protocol sync loss in pipeline: direct COPY, SELECT, sync and getresult' -); + qr/COPY in a pipeline is not supported, aborting connection/, + 'COPY FROM in pipeline: fails'); $node->wait_for_log( qr/FATAL: .*terminating connection because protocol synchronization was lost/, $log_location); +# Remove \syncpipeline here. psql_fails_like( $node, qq{\\startpipeline -COPY psql_pipeline FROM STDIN \\bind \\sendpipeline -SELECT 'val1' \\bind \\sendpipeline -\\syncpipeline -\\getresults -\\endpipeline}, - qr/server closed the connection unexpectedly/, - 'protocol sync loss in pipeline: bind COPY, SELECT, sync and getresult'); - -# This time, test without the \getresults and \syncpipeline. -psql_fails_like( - $node, - qq{\\startpipeline -COPY psql_pipeline FROM STDIN; +COPY psql_pipeline TO STDOUT; SELECT 'val1'; \\endpipeline}, - qr/server closed the connection unexpectedly/, - 'protocol sync loss in pipeline: COPY, SELECT and sync'); + qr/COPY in a pipeline is not supported, aborting connection/, + 'COPY TO in pipeline: fails'); -# Tests sending a sync after a COPY TO/FROM. These abort the connection -# from the frontend. psql_fails_like( $node, qq{\\startpipeline -COPY psql_pipeline FROM STDIN; +\\copy psql_pipeline from stdin; +SELECT 'val1'; \\syncpipeline \\endpipeline}, - qr/\\syncpipeline after COPY is not supported, aborting connection/, - 'sending sync after COPY FROM'); + qr/COPY in a pipeline is not supported, aborting connection/, + '\copy from in pipeline: fails'); + +# Sync attempt after a COPY TO/FROM. psql_fails_like( $node, qq{\\startpipeline -COPY psql_pipeline TO STDOUT; +\\copy psql_pipeline to stdout; \\syncpipeline \\endpipeline}, - qr/\\syncpipeline after COPY is not supported, aborting connection/, - 'sending sync after COPY TO'); + qr/COPY in a pipeline is not supported, aborting connection/, + '\copy to in pipeline: fails'); done_testing(); diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c index ec65ab79fec..2c0b4f28c14 100644 --- a/src/bin/psql/tab-complete.in.c +++ b/src/bin/psql/tab-complete.in.c @@ -3289,7 +3289,7 @@ match_previous_words(int pattern_id, COMPLETE_WITH("FORMAT", "FREEZE", "DELIMITER", "NULL", "HEADER", "QUOTE", "ESCAPE", "FORCE_QUOTE", "FORCE_NOT_NULL", "FORCE_NULL", "ENCODING", "DEFAULT", - "ON_ERROR", "LOG_VERBOSITY"); + "ON_ERROR", "LOG_VERBOSITY", "REJECT_LIMIT"); /* Complete COPY <sth> FROM|TO filename WITH (FORMAT */ else if (Matches("COPY|\\copy", MatchAny, "FROM|TO", MatchAny, "WITH", "(", "FORMAT")) @@ -3664,9 +3664,10 @@ match_previous_words(int pattern_id, TailMatches("CREATE", "TEMP|TEMPORARY|UNLOGGED", "TABLE", MatchAny, "(*)", "AS")) COMPLETE_WITH("EXECUTE", "SELECT", "TABLE", "VALUES", "WITH"); /* Complete CREATE TABLE name (...) with supported options */ - else if (TailMatches("CREATE", "TABLE", MatchAny, "(*)") || - TailMatches("CREATE", "UNLOGGED", "TABLE", MatchAny, "(*)")) + else if (TailMatches("CREATE", "TABLE", MatchAny, "(*)")) COMPLETE_WITH("AS", "INHERITS (", "PARTITION BY", "USING", "TABLESPACE", "WITH ("); + else if (TailMatches("CREATE", "UNLOGGED", "TABLE", MatchAny, "(*)")) + COMPLETE_WITH("AS", "INHERITS (", "USING", "TABLESPACE", "WITH ("); else if (TailMatches("CREATE", "TEMP|TEMPORARY", "TABLE", MatchAny, "(*)")) COMPLETE_WITH("AS", "INHERITS (", "ON COMMIT", "PARTITION BY", "USING", "TABLESPACE", "WITH ("); diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index f2971485d8f..67fbe9c9292 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -57,6 +57,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 202506021 +#define CATALOG_VERSION_NO 202506121 #endif diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index dd00ab420b8..ba12678d1cb 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -351,6 +351,14 @@ typedef struct A_Expr List *name; /* possibly-qualified name of operator */ Node *lexpr; /* left argument, or NULL if none */ Node *rexpr; /* right argument, or NULL if none */ + + /* + * If rexpr is a list of some kind, we separately track its starting and + * ending location; it's not the same as the starting and ending location + * of the token itself. + */ + ParseLoc rexpr_list_start; + ParseLoc rexpr_list_end; ParseLoc location; /* token location, or -1 if unknown */ } A_Expr; @@ -506,6 +514,8 @@ typedef struct A_ArrayExpr { NodeTag type; List *elements; /* array element expressions */ + ParseLoc list_start; /* start of the element list */ + ParseLoc list_end; /* end of the elements list */ ParseLoc location; /* token location, or -1 if unknown */ } A_ArrayExpr; @@ -2100,8 +2110,6 @@ typedef struct InsertStmt ReturningClause *returningClause; /* RETURNING clause */ WithClause *withClause; /* WITH clause */ OverridingKind override; /* OVERRIDING clause */ - ParseLoc stmt_location; /* start location, or -1 if unknown */ - ParseLoc stmt_len; /* length in bytes; 0 means "rest of string" */ } InsertStmt; /* ---------------------- @@ -2116,8 +2124,6 @@ typedef struct DeleteStmt Node *whereClause; /* qualifications */ ReturningClause *returningClause; /* RETURNING clause */ WithClause *withClause; /* WITH clause */ - ParseLoc stmt_location; /* start location, or -1 if unknown */ - ParseLoc stmt_len; /* length in bytes; 0 means "rest of string" */ } DeleteStmt; /* ---------------------- @@ -2133,8 +2139,6 @@ typedef struct UpdateStmt List *fromClause; /* optional from clause for more tables */ ReturningClause *returningClause; /* RETURNING clause */ WithClause *withClause; /* WITH clause */ - ParseLoc stmt_location; /* start location, or -1 if unknown */ - ParseLoc stmt_len; /* length in bytes; 0 means "rest of string" */ } UpdateStmt; /* ---------------------- @@ -2150,8 +2154,6 @@ typedef struct MergeStmt List *mergeWhenClauses; /* list of MergeWhenClause(es) */ ReturningClause *returningClause; /* RETURNING clause */ WithClause *withClause; /* WITH clause */ - ParseLoc stmt_location; /* start location, or -1 if unknown */ - ParseLoc stmt_len; /* length in bytes; 0 means "rest of string" */ } MergeStmt; /* ---------------------- @@ -2221,8 +2223,6 @@ typedef struct SelectStmt bool all; /* ALL specified? */ struct SelectStmt *larg; /* left child */ struct SelectStmt *rarg; /* right child */ - ParseLoc stmt_location; /* start location, or -1 if unknown */ - ParseLoc stmt_len; /* length in bytes; 0 means "rest of string" */ /* Eventually add fields for CORRESPONDING spec here */ } SelectStmt; diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index 7d3b4198f26..01510b01b64 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -1397,6 +1397,10 @@ typedef struct ArrayExpr List *elements pg_node_attr(query_jumble_squash); /* true if elements are sub-arrays */ bool multidims pg_node_attr(query_jumble_ignore); + /* location of the start of the elements list */ + ParseLoc list_start; + /* location of the end of the elements list */ + ParseLoc list_end; /* token location, or -1 if unknown */ ParseLoc location; } ArrayExpr; diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h index 994284019fb..f7d07c84542 100644 --- a/src/include/parser/parse_node.h +++ b/src/include/parser/parse_node.h @@ -108,20 +108,6 @@ typedef Node *(*CoerceParamHook) (ParseState *pstate, Param *param, * byte-wise locations in parse structures to character-wise cursor * positions.) * - * p_stmt_location: location of the top level RawStmt's start. During - * transformation, the Query's location will be set to the statement's - * location if available. Otherwise, the RawStmt's start location will - * be used. Propagating the location through ParseState is needed for - * the Query length calculation (see p_stmt_len below). - * - * p_stmt_len: length of the top level RawStmt. Most of the time, the - * statement's length is not provided by the parser, with the exception - * of SelectStmt within parentheses and PreparableStmt in COPY. If the - * statement's location is provided by the parser, the top-level location - * and length are needed to accurately compute the Query's length. If the - * statement's location is not provided, the RawStmt's length can be used - * directly. - * * p_rtable: list of RTEs that will become the rangetable of the query. * Note that neither relname nor refname of these entries are necessarily * unique; searching the rtable by name is a bad idea. @@ -207,8 +193,6 @@ struct ParseState { ParseState *parentParseState; /* stack link */ const char *p_sourcetext; /* source text, or NULL if not available */ - ParseLoc p_stmt_location; /* start location, or -1 if unknown */ - ParseLoc p_stmt_len; /* length in bytes; 0 means "rest of string" */ List *p_rtable; /* range table so far */ List *p_rteperminfos; /* list of RTEPermissionInfo nodes for each * RTE_RELATION entry in rtable */ diff --git a/src/include/replication/slot.h b/src/include/replication/slot.h index eb0b93b1114..ffacba9d2ae 100644 --- a/src/include/replication/slot.h +++ b/src/include/replication/slot.h @@ -215,6 +215,14 @@ typedef struct ReplicationSlot * recently stopped. */ TimestampTz inactive_since; + + /* + * Latest restart_lsn that has been flushed to disk. For persistent slots + * the flushed LSN should be taken into account when calculating the + * oldest LSN for WAL segments removal. + */ + XLogRecPtr last_saved_restart_lsn; + } ReplicationSlot; #define SlotIsPhysical(slot) ((slot)->data.database == InvalidOid) diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c index c14e3c95250..dca44fdc5d2 100644 --- a/src/interfaces/libpq/fe-misc.c +++ b/src/interfaces/libpq/fe-misc.c @@ -553,9 +553,35 @@ pqPutMsgEnd(PGconn *conn) /* Make message eligible to send */ conn->outCount = conn->outMsgEnd; + /* If appropriate, try to push out some data */ if (conn->outCount >= 8192) { - int toSend = conn->outCount - (conn->outCount % 8192); + int toSend = conn->outCount; + + /* + * On Unix-pipe connections, it seems profitable to prefer sending + * pipe-buffer-sized packets not randomly-sized ones, so retain the + * last partial-8K chunk in our buffer for now. On TCP connections, + * the advantage of that is far less clear. Moreover, it flat out + * isn't safe when using SSL or GSSAPI, because those code paths have + * API stipulations that if they fail to send all the data that was + * offered in the previous write attempt, we mustn't offer less data + * in this write attempt. The previous write attempt might've been + * pqFlush attempting to send everything in the buffer, so we mustn't + * offer less now. (Presently, we won't try to use SSL or GSSAPI on + * Unix connections, so those checks are just Asserts. They'll have + * to become part of the regular if-test if we ever change that.) + */ + if (conn->raddr.addr.ss_family == AF_UNIX) + { +#ifdef USE_SSL + Assert(!conn->ssl_in_use); +#endif +#ifdef ENABLE_GSS + Assert(!conn->gssenc); +#endif + toSend -= toSend % 8192; + } if (pqSendSome(conn, toSend) < 0) return EOF; diff --git a/src/interfaces/libpq/fe-secure-gssapi.c b/src/interfaces/libpq/fe-secure-gssapi.c index 62d05f68496..bc9e1ce06fa 100644 --- a/src/interfaces/libpq/fe-secure-gssapi.c +++ b/src/interfaces/libpq/fe-secure-gssapi.c @@ -112,9 +112,9 @@ pg_GSS_write(PGconn *conn, const void *ptr, size_t len) * again, so if it offers a len less than that, something is wrong. * * Note: it may seem attractive to report partial write completion once - * we've successfully sent any encrypted packets. However, that can cause - * problems for callers; notably, pqPutMsgEnd's heuristic to send only - * full 8K blocks interacts badly with such a hack. We won't save much, + * we've successfully sent any encrypted packets. However, doing that + * expands the state space of this processing and has been responsible for + * bugs in the past (cf. commit d053a879b). We won't save much, * typically, by letting callers discard data early, so don't risk it. */ if (len < PqGSSSendConsumed) diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build index cb983766c67..92429d28402 100644 --- a/src/test/recovery/meson.build +++ b/src/test/recovery/meson.build @@ -54,6 +54,8 @@ tests += { 't/043_no_contrecord_switch.pl', 't/044_invalidate_inactive_slots.pl', 't/045_archive_restartpoint.pl', + 't/046_checkpoint_logical_slot.pl', + 't/047_checkpoint_physical_slot.pl' ], }, } diff --git a/src/test/recovery/t/046_checkpoint_logical_slot.pl b/src/test/recovery/t/046_checkpoint_logical_slot.pl new file mode 100644 index 00000000000..b4265c4a6a5 --- /dev/null +++ b/src/test/recovery/t/046_checkpoint_logical_slot.pl @@ -0,0 +1,139 @@ +# Copyright (c) 2025, PostgreSQL Global Development Group +# +# This test verifies the case when the logical slot is advanced during +# checkpoint. The test checks that the logical slot's restart_lsn still refers +# to an existed WAL segment after immediate restart. +# +use strict; +use warnings FATAL => 'all'; + +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; + +use Test::More; + +if ($ENV{enable_injection_points} ne 'yes') +{ + plan skip_all => 'Injection points not supported by this build'; +} + +my ($node, $result); + +$node = PostgreSQL::Test::Cluster->new('mike'); +$node->init; +$node->append_conf('postgresql.conf', + "shared_preload_libraries = 'injection_points'"); +$node->append_conf('postgresql.conf', "wal_level = 'logical'"); +$node->start; +$node->safe_psql('postgres', q(CREATE EXTENSION injection_points)); + +# Create a simple table to generate data into. +$node->safe_psql('postgres', + q{create table t (id serial primary key, b text)}); + +# Create the two slots we'll need. +$node->safe_psql('postgres', + q{select pg_create_logical_replication_slot('slot_logical', 'test_decoding')} +); +$node->safe_psql('postgres', + q{select pg_create_physical_replication_slot('slot_physical', true)}); + +# Advance both slots to the current position just to have everything "valid". +$node->safe_psql('postgres', + q{select count(*) from pg_logical_slot_get_changes('slot_logical', null, null)} +); +$node->safe_psql('postgres', + q{select pg_replication_slot_advance('slot_physical', pg_current_wal_lsn())} +); + +# Run checkpoint to flush current state to disk and set a baseline. +$node->safe_psql('postgres', q{checkpoint}); + +# Generate some transactions to get RUNNING_XACTS. +my $xacts = $node->background_psql('postgres'); +$xacts->query_until( + qr/run_xacts/, + q(\echo run_xacts +SELECT 1 \watch 0.1 +\q +)); + +# Insert 2M rows; that's about 260MB (~20 segments) worth of WAL. +$node->safe_psql('postgres', + q{insert into t (b) select md5(i::text) from generate_series(1,1000000) s(i)} +); + +# Run another checkpoint to set a new restore LSN. +$node->safe_psql('postgres', q{checkpoint}); + +# Another 2M rows; that's about 260MB (~20 segments) worth of WAL. +$node->safe_psql('postgres', + q{insert into t (b) select md5(i::text) from generate_series(1,1000000) s(i)} +); + +# Run another checkpoint, this time in the background, and make it wait +# on the injection point) so that the checkpoint stops right before +# removing old WAL segments. +note('starting checkpoint\n'); + +my $checkpoint = $node->background_psql('postgres'); +$checkpoint->query_safe( + q(select injection_points_attach('checkpoint-before-old-wal-removal','wait')) +); +$checkpoint->query_until( + qr/starting_checkpoint/, + q(\echo starting_checkpoint +checkpoint; +\q +)); + +# Wait until the checkpoint stops right before removing WAL segments. +note('waiting for injection_point\n'); +$node->wait_for_event('checkpointer', 'checkpoint-before-old-wal-removal'); +note('injection_point is reached'); + +# Try to advance the logical slot, but make it stop when it moves to the next +# WAL segment (this has to happen in the background, too). +my $logical = $node->background_psql('postgres'); +$logical->query_safe( + q{select injection_points_attach('logical-replication-slot-advance-segment','wait');} +); +$logical->query_until( + qr/get_changes/, + q( +\echo get_changes +select count(*) from pg_logical_slot_get_changes('slot_logical', null, null) \watch 1 +\q +)); + +# Wait until the slot's restart_lsn points to the next WAL segment. +note('waiting for injection_point\n'); +$node->wait_for_event('client backend', + 'logical-replication-slot-advance-segment'); +note('injection_point is reached'); + +# OK, we're in the right situation: time to advance the physical slot, which +# recalculates the required LSN, and then unblock the checkpoint, which +# removes the WAL still needed by the logical slot. +$node->safe_psql('postgres', + q{select pg_replication_slot_advance('slot_physical', pg_current_wal_lsn())} +); + +# Continue the checkpoint. +$node->safe_psql('postgres', + q{select injection_points_wakeup('checkpoint-before-old-wal-removal')}); + +# Abruptly stop the server (1 second should be enough for the checkpoint +# to finish; it would be better). +$node->stop('immediate'); + +$node->start; + +eval { + $node->safe_psql('postgres', + q{select count(*) from pg_logical_slot_get_changes('slot_logical', null, null);} + ); +}; +is($@, '', "Logical slot still valid"); + +done_testing(); diff --git a/src/test/recovery/t/047_checkpoint_physical_slot.pl b/src/test/recovery/t/047_checkpoint_physical_slot.pl new file mode 100644 index 00000000000..454e56b9bd2 --- /dev/null +++ b/src/test/recovery/t/047_checkpoint_physical_slot.pl @@ -0,0 +1,133 @@ +# Copyright (c) 2025, PostgreSQL Global Development Group +# +# This test verifies the case when the physical slot is advanced during +# checkpoint. The test checks that the physical slot's restart_lsn still refers +# to an existed WAL segment after immediate restart. +# +use strict; +use warnings FATAL => 'all'; + +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; + +use Test::More; + +if ($ENV{enable_injection_points} ne 'yes') +{ + plan skip_all => 'Injection points not supported by this build'; +} + +my ($node, $result); + +$node = PostgreSQL::Test::Cluster->new('mike'); +$node->init; +$node->append_conf('postgresql.conf', + "shared_preload_libraries = 'injection_points'"); +$node->append_conf('postgresql.conf', "wal_level = 'replica'"); +$node->start; +$node->safe_psql('postgres', q(CREATE EXTENSION injection_points)); + +# Create a simple table to generate data into. +$node->safe_psql('postgres', + q{create table t (id serial primary key, b text)}); + +# Create a physical replication slot. +$node->safe_psql('postgres', + q{select pg_create_physical_replication_slot('slot_physical', true)}); + +# Advance slot to the current position, just to have everything "valid". +$node->safe_psql('postgres', + q{select pg_replication_slot_advance('slot_physical', pg_current_wal_lsn())} +); + +# Run checkpoint to flush current state to disk and set a baseline. +$node->safe_psql('postgres', q{checkpoint}); + +# Insert 2M rows; that's about 260MB (~20 segments) worth of WAL. +$node->safe_psql('postgres', + q{insert into t (b) select md5(i::text) from generate_series(1,100000) s(i)} +); + +# Advance slot to the current position, just to have everything "valid". +$node->safe_psql('postgres', + q{select pg_replication_slot_advance('slot_physical', pg_current_wal_lsn())} +); + +# Run another checkpoint to set a new restore LSN. +$node->safe_psql('postgres', q{checkpoint}); + +# Another 2M rows; that's about 260MB (~20 segments) worth of WAL. +$node->safe_psql('postgres', + q{insert into t (b) select md5(i::text) from generate_series(1,1000000) s(i)} +); + +my $restart_lsn_init = $node->safe_psql('postgres', + q{select restart_lsn from pg_replication_slots where slot_name = 'slot_physical'} +); +chomp($restart_lsn_init); +note("restart lsn before checkpoint: $restart_lsn_init"); + +# Run another checkpoint, this time in the background, and make it wait +# on the injection point) so that the checkpoint stops right before +# removing old WAL segments. +note('starting checkpoint'); + +my $checkpoint = $node->background_psql('postgres'); +$checkpoint->query_safe( + q{select injection_points_attach('checkpoint-before-old-wal-removal','wait')} +); +$checkpoint->query_until( + qr/starting_checkpoint/, + q(\echo starting_checkpoint +checkpoint; +\q +)); + +# Wait until the checkpoint stops right before removing WAL segments. +note('waiting for injection_point'); +$node->wait_for_event('checkpointer', 'checkpoint-before-old-wal-removal'); +note('injection_point is reached'); + +# OK, we're in the right situation: time to advance the physical slot, which +# recalculates the required LSN and then unblock the checkpoint, which +# removes the WAL still needed by the physical slot. +$node->safe_psql('postgres', + q{select pg_replication_slot_advance('slot_physical', pg_current_wal_lsn())} +); + +# Continue the checkpoint. +$node->safe_psql('postgres', + q{select injection_points_wakeup('checkpoint-before-old-wal-removal')}); + +my $restart_lsn_old = $node->safe_psql('postgres', + q{select restart_lsn from pg_replication_slots where slot_name = 'slot_physical'} +); +chomp($restart_lsn_old); +note("restart lsn before stop: $restart_lsn_old"); + +# Abruptly stop the server (1 second should be enough for the checkpoint +# to finish; it would be better). +$node->stop('immediate'); + +$node->start; + +# Get the restart_lsn of the slot right after restarting. +my $restart_lsn = $node->safe_psql('postgres', + q{select restart_lsn from pg_replication_slots where slot_name = 'slot_physical'} +); +chomp($restart_lsn); +note("restart lsn: $restart_lsn"); + +# Get the WAL segment name for the slot's restart_lsn. +my $restart_lsn_segment = $node->safe_psql('postgres', + "SELECT pg_walfile_name('$restart_lsn'::pg_lsn)"); +chomp($restart_lsn_segment); + +# Check if the required wal segment exists. +note("required by slot segment name: $restart_lsn_segment"); +my $datadir = $node->data_dir; +ok( -f "$datadir/pg_wal/$restart_lsn_segment", + "WAL segment $restart_lsn_segment for physical slot's restart_lsn $restart_lsn exists" +); + +done_testing(); diff --git a/src/test/regress/expected/psql_pipeline.out b/src/test/regress/expected/psql_pipeline.out index a30dec088b9..e78e6bfa0ad 100644 --- a/src/test/regress/expected/psql_pipeline.out +++ b/src/test/regress/expected/psql_pipeline.out @@ -228,192 +228,6 @@ BEGIN \bind \sendpipeline INSERT INTO psql_pipeline VALUES ($1) \bind 1 \sendpipeline COMMIT \bind \sendpipeline \endpipeline --- COPY FROM STDIN --- with \sendpipeline and \bind -\startpipeline -SELECT $1 \bind 'val1' \sendpipeline -COPY psql_pipeline FROM STDIN \bind \sendpipeline -\endpipeline - ?column? ----------- - val1 -(1 row) - --- with semicolon -\startpipeline -SELECT 'val1'; -COPY psql_pipeline FROM STDIN; -\endpipeline - ?column? ----------- - val1 -(1 row) - --- COPY FROM STDIN with \flushrequest + \getresults --- with \sendpipeline and \bind -\startpipeline -SELECT $1 \bind 'val1' \sendpipeline -COPY psql_pipeline FROM STDIN \bind \sendpipeline -\flushrequest -\getresults - ?column? ----------- - val1 -(1 row) - -message type 0x5a arrived from server while idle -\endpipeline --- with semicolon -\startpipeline -SELECT 'val1'; -COPY psql_pipeline FROM STDIN; -\flushrequest -\getresults - ?column? ----------- - val1 -(1 row) - -message type 0x5a arrived from server while idle -\endpipeline --- COPY FROM STDIN with \syncpipeline + \getresults --- with \bind and \sendpipeline -\startpipeline -SELECT $1 \bind 'val1' \sendpipeline -COPY psql_pipeline FROM STDIN \bind \sendpipeline -\syncpipeline -\getresults - ?column? ----------- - val1 -(1 row) - -\endpipeline --- with semicolon -\startpipeline -SELECT 'val1'; -COPY psql_pipeline FROM STDIN; -\syncpipeline -\getresults - ?column? ----------- - val1 -(1 row) - -\endpipeline --- COPY TO STDOUT --- with \bind and \sendpipeline -\startpipeline -SELECT $1 \bind 'val1' \sendpipeline -copy psql_pipeline TO STDOUT \bind \sendpipeline -\endpipeline - ?column? ----------- - val1 -(1 row) - -1 \N -2 test2 -20 test2 -3 test3 -30 test3 -4 test4 -40 test4 --- with semicolon -\startpipeline -SELECT 'val1'; -copy psql_pipeline TO STDOUT; -\endpipeline - ?column? ----------- - val1 -(1 row) - -1 \N -2 test2 -20 test2 -3 test3 -30 test3 -4 test4 -40 test4 --- COPY TO STDOUT with \flushrequest + \getresults --- with \bind and \sendpipeline -\startpipeline -SELECT $1 \bind 'val1' \sendpipeline -copy psql_pipeline TO STDOUT \bind \sendpipeline -\flushrequest -\getresults - ?column? ----------- - val1 -(1 row) - -1 \N -2 test2 -20 test2 -3 test3 -30 test3 -4 test4 -40 test4 -\endpipeline --- with semicolon -\startpipeline -SELECT 'val1'; -copy psql_pipeline TO STDOUT; -\flushrequest -\getresults - ?column? ----------- - val1 -(1 row) - -1 \N -2 test2 -20 test2 -3 test3 -30 test3 -4 test4 -40 test4 -\endpipeline --- COPY TO STDOUT with \syncpipeline + \getresults --- with \bind and \sendpipeline -\startpipeline -SELECT $1 \bind 'val1' \sendpipeline -copy psql_pipeline TO STDOUT \bind \sendpipeline -\syncpipeline -\getresults - ?column? ----------- - val1 -(1 row) - -1 \N -2 test2 -20 test2 -3 test3 -30 test3 -4 test4 -40 test4 -\endpipeline --- with semicolon -\startpipeline -SELECT 'val1'; -copy psql_pipeline TO STDOUT; -\syncpipeline -\getresults - ?column? ----------- - val1 -(1 row) - -1 \N -2 test2 -20 test2 -3 test3 -30 test3 -4 test4 -40 test4 -\endpipeline -- Use \parse and \bind_named \startpipeline SELECT $1 \parse '' @@ -740,7 +554,7 @@ SELECT COUNT(*) FROM psql_pipeline \bind \sendpipeline count ------- - 7 + 1 (1 row) -- After an error, pipeline is aborted and requires \syncpipeline to be diff --git a/src/test/regress/sql/psql_pipeline.sql b/src/test/regress/sql/psql_pipeline.sql index 16e1e1e84cd..5945eca1ef7 100644 --- a/src/test/regress/sql/psql_pipeline.sql +++ b/src/test/regress/sql/psql_pipeline.sql @@ -105,106 +105,6 @@ INSERT INTO psql_pipeline VALUES ($1) \bind 1 \sendpipeline COMMIT \bind \sendpipeline \endpipeline --- COPY FROM STDIN --- with \sendpipeline and \bind -\startpipeline -SELECT $1 \bind 'val1' \sendpipeline -COPY psql_pipeline FROM STDIN \bind \sendpipeline -\endpipeline -2 test2 -\. --- with semicolon -\startpipeline -SELECT 'val1'; -COPY psql_pipeline FROM STDIN; -\endpipeline -20 test2 -\. - --- COPY FROM STDIN with \flushrequest + \getresults --- with \sendpipeline and \bind -\startpipeline -SELECT $1 \bind 'val1' \sendpipeline -COPY psql_pipeline FROM STDIN \bind \sendpipeline -\flushrequest -\getresults -3 test3 -\. -\endpipeline --- with semicolon -\startpipeline -SELECT 'val1'; -COPY psql_pipeline FROM STDIN; -\flushrequest -\getresults -30 test3 -\. -\endpipeline - --- COPY FROM STDIN with \syncpipeline + \getresults --- with \bind and \sendpipeline -\startpipeline -SELECT $1 \bind 'val1' \sendpipeline -COPY psql_pipeline FROM STDIN \bind \sendpipeline -\syncpipeline -\getresults -4 test4 -\. -\endpipeline --- with semicolon -\startpipeline -SELECT 'val1'; -COPY psql_pipeline FROM STDIN; -\syncpipeline -\getresults -40 test4 -\. -\endpipeline - --- COPY TO STDOUT --- with \bind and \sendpipeline -\startpipeline -SELECT $1 \bind 'val1' \sendpipeline -copy psql_pipeline TO STDOUT \bind \sendpipeline -\endpipeline --- with semicolon -\startpipeline -SELECT 'val1'; -copy psql_pipeline TO STDOUT; -\endpipeline - --- COPY TO STDOUT with \flushrequest + \getresults --- with \bind and \sendpipeline -\startpipeline -SELECT $1 \bind 'val1' \sendpipeline -copy psql_pipeline TO STDOUT \bind \sendpipeline -\flushrequest -\getresults -\endpipeline --- with semicolon -\startpipeline -SELECT 'val1'; -copy psql_pipeline TO STDOUT; -\flushrequest -\getresults -\endpipeline - --- COPY TO STDOUT with \syncpipeline + \getresults --- with \bind and \sendpipeline -\startpipeline -SELECT $1 \bind 'val1' \sendpipeline -copy psql_pipeline TO STDOUT \bind \sendpipeline -\syncpipeline -\getresults -\endpipeline --- with semicolon -\startpipeline -SELECT 'val1'; -copy psql_pipeline TO STDOUT; -\syncpipeline -\getresults -\endpipeline - -- Use \parse and \bind_named \startpipeline SELECT $1 \parse '' |