agora inbox for [email protected]help / color / mirror / Atom feed
[PATCH v7 14/16] Count tuples for vacuum logging in heap_page_prune 7+ messages / 2 participants [nested] [flat]
* [PATCH v7 14/16] Count tuples for vacuum logging in heap_page_prune @ 2024-03-26 14:04 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Melanie Plageman @ 2024-03-26 14:04 UTC (permalink / raw) lazy_scan_prune() loops through all of the tuple visibility information that was recorded in heap_page_prune() and then counts live and recently dead tuples. That information is available in heap_page_prune(), so just record it there. Add live and recently dead tuple counters to the PruneResult. Doing this counting in heap_page_prune() eliminates the need for saving the tuple visibility status information in the PruneResult. Instead, save it in the PruneState where it can be referenced by heap_prune_chain(). --- src/backend/access/heap/pruneheap.c | 98 ++++++++++++++++++++++++---- src/backend/access/heap/vacuumlazy.c | 93 +------------------------- src/include/access/heapam.h | 36 ++-------- 3 files changed, 97 insertions(+), 130 deletions(-) diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c index f8966d06cd2..ee557c9ed35 100644 --- a/src/backend/access/heap/pruneheap.c +++ b/src/backend/access/heap/pruneheap.c @@ -55,6 +55,18 @@ typedef struct */ bool marked[MaxHeapTuplesPerPage + 1]; + /* + * Tuple visibility is only computed once for each tuple, for correctness + * and efficiency reasons; see comment in heap_page_prune_and_freeze() for + * details. This is of type int8[], instead of HTSV_Result[], so we can + * use -1 to indicate no visibility has been computed, e.g. for LP_DEAD + * items. + * + * This needs to be MaxHeapTuplesPerPage + 1 long as FirstOffsetNumber is + * 1. Otherwise every access would need to subtract 1. + */ + int8 htsv[MaxHeapTuplesPerPage + 1]; + /* * One entry for every tuple that we may freeze. */ @@ -69,6 +81,7 @@ static int heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum, PruneState *prstate, PruneFreezeResult *presult); +static inline HTSV_Result htsv_get_valid_status(int status); static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid); static void heap_prune_record_redirect(PruneState *prstate, OffsetNumber offnum, OffsetNumber rdoffnum, @@ -273,7 +286,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer, memset(prstate.marked, 0, sizeof(prstate.marked)); /* - * presult->htsv is not initialized here because all ntuple spots in the + * prstate.htsv is not initialized here because all ntuple spots in the * array will be set either to a valid HTSV_Result value or -1. */ presult->ndeleted = 0; @@ -282,6 +295,9 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer, presult->hastup = false; + presult->live_tuples = 0; + presult->recently_dead_tuples = 0; + /* * Caller will update the VM after pruning, collecting LP_DEAD items, and * freezing tuples. Keep track of whether or not the page is all_visible @@ -340,7 +356,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer, /* Nothing to do if slot doesn't contain a tuple */ if (!ItemIdIsNormal(itemid)) { - presult->htsv[offnum] = -1; + prstate.htsv[offnum] = -1; continue; } @@ -356,13 +372,32 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer, if (off_loc) *off_loc = offnum; - presult->htsv[offnum] = heap_prune_satisfies_vacuum(&prstate, &tup, - buffer); + prstate.htsv[offnum] = heap_prune_satisfies_vacuum(&prstate, &tup, + buffer); if (reason == PRUNE_ON_ACCESS) continue; - switch (presult->htsv[offnum]) + /* + * The criteria for counting a tuple as live in this block need to + * match what analyze.c's acquire_sample_rows() does, otherwise VACUUM + * and ANALYZE may produce wildly different reltuples values, e.g. + * when there are many recently-dead tuples. + * + * The logic here is a bit simpler than acquire_sample_rows(), as + * VACUUM can't run inside a transaction block, which makes some cases + * impossible (e.g. in-progress insert from the same transaction). + * + * We treat LP_DEAD items (which are the closest thing to DEAD tuples + * that might be seen here) differently, too: we assume that they'll + * become LP_UNUSED before VACUUM finishes. This difference is only + * superficial. VACUUM effectively agrees with ANALYZE about DEAD + * items, in the end. VACUUM won't remember LP_DEAD items, but only + * because they're not supposed to be left behind when it is done. + * (Cases where we bypass index vacuuming will violate this optimistic + * assumption, but the overall impact of that should be negligible.) + */ + switch (prstate.htsv[offnum]) { case HEAPTUPLE_DEAD: @@ -382,6 +417,12 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer, break; case HEAPTUPLE_LIVE: + /* + * Count it as live. Not only is this natural, but it's also + * what acquire_sample_rows() does. + */ + presult->live_tuples++; + /* * Is the tuple definitely visible to all transactions? * @@ -423,13 +464,34 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer, } break; case HEAPTUPLE_RECENTLY_DEAD: + + /* + * If tuple is recently dead then we must not remove it from + * the relation. (We only remove items that are LP_DEAD from + * pruning.) + */ + presult->recently_dead_tuples++; all_visible_except_removable = false; break; case HEAPTUPLE_INSERT_IN_PROGRESS: + + /* + * We do not count these rows as live, because we expect the + * inserting transaction to update the counters at commit, and + * we assume that will happen only after we report our + * results. This assumption is a bit shaky, but it is what + * acquire_sample_rows() does, so be consistent. + */ all_visible_except_removable = false; break; case HEAPTUPLE_DELETE_IN_PROGRESS: - /* This is an expected case during concurrent vacuum */ + + /* + * 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. + */ + presult->live_tuples++; all_visible_except_removable = false; break; default: @@ -437,7 +499,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer, break; } - if (presult->htsv[offnum] != HEAPTUPLE_DEAD) + if (prstate.htsv[offnum] != HEAPTUPLE_DEAD) { /* * Deliberately don't set hastup for LP_DEAD items. We make the @@ -746,10 +808,24 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer) } +/* + * Pruning calculates tuple visibility once and saves the results in an array + * of int8. See PruneState.htsv for details. This helper function is meant to + * guard against examining visibility status array members which have not yet + * been computed. + */ +static inline HTSV_Result +htsv_get_valid_status(int status) +{ + Assert(status >= HEAPTUPLE_DEAD && + status <= HEAPTUPLE_DELETE_IN_PROGRESS); + return (HTSV_Result) status; +} + /* * Prune specified line pointer or a HOT chain originating at line pointer. * - * Tuple visibility information is provided in presult->htsv. + * Tuple visibility information is provided in prstate->htsv. * * If the item is an index-referenced tuple (i.e. not a heap-only tuple), * the HOT chain is pruned by removing all DEAD tuples at the start of the HOT @@ -800,7 +876,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum, */ if (ItemIdIsNormal(rootlp)) { - Assert(presult->htsv[rootoffnum] != -1); + Assert(prstate->htsv[rootoffnum] != -1); htup = (HeapTupleHeader) PageGetItem(dp, rootlp); if (HeapTupleHeaderIsHeapOnly(htup)) @@ -823,7 +899,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum, * either here or while following a chain below. Whichever path * gets there first will mark the tuple unused. */ - if (presult->htsv[rootoffnum] == HEAPTUPLE_DEAD && + if (prstate->htsv[rootoffnum] == HEAPTUPLE_DEAD && !HeapTupleHeaderIsHotUpdated(htup)) { heap_prune_record_unused(prstate, rootoffnum); @@ -924,7 +1000,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum, */ tupdead = recent_dead = false; - switch (htsv_get_valid_status(presult->htsv[offnum])) + switch (htsv_get_valid_status(prstate->htsv[offnum])) { case HEAPTUPLE_DEAD: tupdead = true; diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 68258d083ab..c28e786a1e0 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -1378,22 +1378,6 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno, * * Caller must hold pin and buffer cleanup lock on the buffer. * - * Prior to PostgreSQL 14 there were very rare cases where - * heap_page_prune_and_freeze() was allowed to disagree with our - * HeapTupleSatisfiesVacuum() call about whether or not a tuple should be - * considered DEAD. This happened when an inserting transaction concurrently - * aborted (after our heap_page_prune_and_freeze() call, before our - * HeapTupleSatisfiesVacuum() call). There was rather a lot of complexity just - * so we could deal with tuples that were DEAD to VACUUM, but nevertheless were - * left with storage after pruning. - * - * As of Postgres 17, we circumvent this problem altogether by reusing the - * result of heap_page_prune_and_freeze()'s visibility check. Without the - * second call to HeapTupleSatisfiesVacuum(), there is no new HTSV_Result and - * there can be no disagreement. We'll just handle such tuples as if they had - * become fully dead right after this operation completes instead of in the - * middle of it. - * * vmbuffer is the buffer containing the VM block with visibility information * for the heap block, blkno. all_visible_according_to_vm is the saved * visibility status of the heap block looked up earlier by the caller. We @@ -1415,10 +1399,8 @@ lazy_scan_prune(LVRelState *vacrel, OffsetNumber offnum, maxoff; ItemId itemid; + int lpdead_items = 0; PruneFreezeResult presult; - int lpdead_items, - live_tuples, - recently_dead_tuples; HeapPageFreeze pagefrz; OffsetNumber deadoffsets[MaxHeapTuplesPerPage]; @@ -1438,9 +1420,6 @@ lazy_scan_prune(LVRelState *vacrel, pagefrz.NoFreezePageRelfrozenXid = vacrel->NewRelfrozenXid; pagefrz.NoFreezePageRelminMxid = vacrel->NewRelminMxid; pagefrz.cutoffs = &vacrel->cutoffs; - lpdead_items = 0; - live_tuples = 0; - recently_dead_tuples = 0; /* * Prune all HOT-update chains and potentially freeze tuples on this page. @@ -1472,9 +1451,6 @@ lazy_scan_prune(LVRelState *vacrel, vacrel->offnum = offnum; itemid = PageGetItemId(page, offnum); - /* Redirect items mustn't be touched */ - if (ItemIdIsRedirected(itemid) || !ItemIdIsUsed(itemid)) - continue; if (ItemIdIsDead(itemid)) { @@ -1482,69 +1458,6 @@ lazy_scan_prune(LVRelState *vacrel, continue; } - Assert(ItemIdIsNormal(itemid)); - - /* - * The criteria for counting a tuple as live in this block need to - * match what analyze.c's acquire_sample_rows() does, otherwise VACUUM - * and ANALYZE may produce wildly different reltuples values, e.g. - * when there are many recently-dead tuples. - * - * The logic here is a bit simpler than acquire_sample_rows(), as - * VACUUM can't run inside a transaction block, which makes some cases - * impossible (e.g. in-progress insert from the same transaction). - * - * We treat LP_DEAD items (which are the closest thing to DEAD tuples - * that might be seen here) differently, too: we assume that they'll - * become LP_UNUSED before VACUUM finishes. This difference is only - * superficial. VACUUM effectively agrees with ANALYZE about DEAD - * items, in the end. VACUUM won't remember LP_DEAD items, but only - * because they're not supposed to be left behind when it is done. - * (Cases where we bypass index vacuuming will violate this optimistic - * assumption, but the overall impact of that should be negligible.) - */ - switch (htsv_get_valid_status(presult.htsv[offnum])) - { - case HEAPTUPLE_LIVE: - - /* - * Count it as live. Not only is this natural, but it's also - * what acquire_sample_rows() does. - */ - live_tuples++; - break; - case HEAPTUPLE_RECENTLY_DEAD: - - /* - * If tuple is recently dead then we must not remove it from - * the relation. (We only remove items that are LP_DEAD from - * pruning.) - */ - recently_dead_tuples++; - break; - case HEAPTUPLE_INSERT_IN_PROGRESS: - - /* - * We do not count these rows as live, because we expect the - * inserting transaction to update the counters at commit, and - * we assume that will happen only after we report our - * results. This assumption is a bit shaky, but it is what - * acquire_sample_rows() does, so be consistent. - */ - break; - case HEAPTUPLE_DELETE_IN_PROGRESS: - - /* - * 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; - default: - elog(ERROR, "unexpected HeapTupleSatisfiesVacuum result"); - break; - } } vacrel->offnum = InvalidOffsetNumber; @@ -1619,8 +1532,8 @@ lazy_scan_prune(LVRelState *vacrel, vacrel->tuples_deleted += presult.ndeleted; vacrel->tuples_frozen += presult.nfrozen; vacrel->lpdead_items += lpdead_items; - vacrel->live_tuples += live_tuples; - vacrel->recently_dead_tuples += recently_dead_tuples; + vacrel->live_tuples += presult.live_tuples; + vacrel->recently_dead_tuples += presult.recently_dead_tuples; /* Can't truncate this page */ if (presult.hastup) diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h index 567faa34664..06d75f2ad04 100644 --- a/src/include/access/heapam.h +++ b/src/include/access/heapam.h @@ -203,8 +203,14 @@ typedef struct PruneFreezeResult /* * The rest of the fields in PruneFreezeResult are only guaranteed to be - * initialized if heap_page_prune is passed PruneReason VACUUM_SCAN. + * initialized if heap_page_prune_and_freeze() is passed a PruneReason + * other than PRUNE_ON_ACCESS. */ + int live_tuples; + int recently_dead_tuples; + + /* Number of tuples we froze */ + int nfrozen; /* * Whether or not the page is truly all-visible after pruning. If there @@ -226,21 +232,6 @@ typedef struct PruneFreezeResult */ TransactionId vm_conflict_horizon; - /* - * Tuple visibility is only computed once for each tuple, for correctness - * and efficiency reasons; see comment in heap_page_prune_and_freeze() for - * details. This is of type int8[], instead of HTSV_Result[], so we can - * use -1 to indicate no visibility has been computed, e.g. for LP_DEAD - * items. - * - * This needs to be MaxHeapTuplesPerPage + 1 long as FirstOffsetNumber is - * 1. Otherwise every access would need to subtract 1. - */ - int8 htsv[MaxHeapTuplesPerPage + 1]; - - /* Number of tuples we may freeze */ - int nfrozen; - /* * One entry for every tuple that we may freeze. */ @@ -260,19 +251,6 @@ typedef enum PRUNE_VACUUM_CLEANUP, /* VACUUM 2nd heap pass */ } PruneReason; -/* - * Pruning calculates tuple visibility once and saves the results in an array - * of int8. See PruneFreezeResult.htsv for details. This helper function is meant to - * guard against examining visibility status array members which have not yet - * been computed. - */ -static inline HTSV_Result -htsv_get_valid_status(int status) -{ - Assert(status >= HEAPTUPLE_DEAD && - status <= HEAPTUPLE_DELETE_IN_PROGRESS); - return (HTSV_Result) status; -} /* ---------------- * function prototypes for heap access method -- 2.40.1 --ck6erxojvlx23byk Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v7-0015-Save-dead-tuple-offsets-during-heap_page_prune.patch" ^ permalink raw reply [nested|flat] 7+ messages in thread
* [PATCH v4 1/3] Refactor match_previous_words() to remove direct use of rl_completion_matches() @ 2025-06-05 00:38 Yugo Nagata <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Yugo Nagata @ 2025-06-05 00:38 UTC (permalink / raw) Most tab completions in match_previous_words() use COMPLETE_WITH* macros, which wrap rl_completion_matches(). However, some direct calls to rl_completion_matches() still remained. This commit replaces the remaining direct calls with the new macro, COMPLETE_WITH_FILES or COMPLETE_WITH_GENERATOR, for improved consistency and readability. --- src/bin/psql/tab-complete.in.c | 38 ++++++++++++++++------------------ 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c index 37524364290..1b4855edab6 100644 --- a/src/bin/psql/tab-complete.in.c +++ b/src/bin/psql/tab-complete.in.c @@ -443,6 +443,16 @@ do { \ matches = rl_completion_matches(text, complete_from_schema_query); \ } while (0) +#define COMPLETE_WITH_FILES(escape, force_quote) \ +do { \ + completion_charp = escape; \ + completion_force_quote = force_quote; \ + matches = rl_completion_matches(text, complete_from_files); \ +} while (0) + +#define COMPLETE_WITH_GENERATOR(function) \ + matches = rl_completion_matches(text, function) + /* * Assembly instructions for schema queries * @@ -2179,7 +2189,7 @@ match_previous_words(int pattern_id, /* for INDEX and TABLE/SEQUENCE, respectively */ "UNIQUE", "UNLOGGED"); else - matches = rl_completion_matches(text, create_command_generator); + COMPLETE_WITH_GENERATOR(create_command_generator); } /* complete with something you can create or replace */ else if (TailMatches("CREATE", "OR", "REPLACE")) @@ -2189,7 +2199,7 @@ match_previous_words(int pattern_id, /* DROP, but not DROP embedded in other commands */ /* complete with something you can drop */ else if (Matches("DROP")) - matches = rl_completion_matches(text, drop_command_generator); + COMPLETE_WITH_GENERATOR(drop_command_generator); /* ALTER */ @@ -2200,7 +2210,7 @@ match_previous_words(int pattern_id, /* ALTER something */ else if (Matches("ALTER")) - matches = rl_completion_matches(text, alter_command_generator); + COMPLETE_WITH_GENERATOR(alter_command_generator); /* ALTER TABLE,INDEX,MATERIALIZED VIEW ALL IN TABLESPACE xxx */ else if (TailMatches("ALL", "IN", "TABLESPACE", MatchAny)) COMPLETE_WITH("SET TABLESPACE", "OWNED BY"); @@ -3308,17 +3318,9 @@ match_previous_words(int pattern_id, COMPLETE_WITH("FROM", "TO"); /* Complete COPY <sth> FROM|TO with filename */ else if (Matches("COPY", MatchAny, "FROM|TO")) - { - completion_charp = ""; - completion_force_quote = true; /* COPY requires quoted filename */ - matches = rl_completion_matches(text, complete_from_files); - } + COMPLETE_WITH_FILES("", true); /* COPY requires quoted filename */ else if (Matches("\\copy", MatchAny, "FROM|TO")) - { - completion_charp = ""; - completion_force_quote = false; - matches = rl_completion_matches(text, complete_from_files); - } + COMPLETE_WITH_FILES("", false); /* Complete COPY <sth> TO <sth> */ else if (Matches("COPY|\\copy", MatchAny, "TO", MatchAny)) @@ -5417,9 +5419,9 @@ match_previous_words(int pattern_id, else if (TailMatchesCS("\\h|\\help", MatchAny)) { if (TailMatches("DROP")) - matches = rl_completion_matches(text, drop_command_generator); + COMPLETE_WITH_GENERATOR(drop_command_generator); else if (TailMatches("ALTER")) - matches = rl_completion_matches(text, alter_command_generator); + COMPLETE_WITH_GENERATOR(alter_command_generator); /* * CREATE is recognized by tail match elsewhere, so doesn't need to be @@ -5519,11 +5521,7 @@ match_previous_words(int pattern_id, else if (TailMatchesCS("\\cd|\\e|\\edit|\\g|\\gx|\\i|\\include|" "\\ir|\\include_relative|\\o|\\out|" "\\s|\\w|\\write|\\lo_import")) - { - completion_charp = "\\"; - completion_force_quote = false; - matches = rl_completion_matches(text, complete_from_files); - } + COMPLETE_WITH_FILES("\\", false); /* gen_tabcomplete.pl ends special processing here */ /* END GEN_TABCOMPLETE */ -- 2.43.0 --Multipart=_Thu__17_Jul_2025_10_57_36_+0900_dz7Bjsed2AEN=CJ+-- ^ permalink raw reply [nested|flat] 7+ messages in thread
* [PATCH v5 1/3] Refactor match_previous_words() to remove direct use of rl_completion_matches() @ 2025-06-05 00:38 Yugo Nagata <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Yugo Nagata @ 2025-06-05 00:38 UTC (permalink / raw) Most tab completions in match_previous_words() use COMPLETE_WITH* macros, which wrap rl_completion_matches(). However, some direct calls to rl_completion_matches() still remained. This commit replaces the remaining direct calls with the new macro, COMPLETE_WITH_FILES or COMPLETE_WITH_GENERATOR, for improved consistency and readability. --- src/bin/psql/tab-complete.in.c | 38 ++++++++++++++++------------------ 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c index 37524364290..1b4855edab6 100644 --- a/src/bin/psql/tab-complete.in.c +++ b/src/bin/psql/tab-complete.in.c @@ -443,6 +443,16 @@ do { \ matches = rl_completion_matches(text, complete_from_schema_query); \ } while (0) +#define COMPLETE_WITH_FILES(escape, force_quote) \ +do { \ + completion_charp = escape; \ + completion_force_quote = force_quote; \ + matches = rl_completion_matches(text, complete_from_files); \ +} while (0) + +#define COMPLETE_WITH_GENERATOR(function) \ + matches = rl_completion_matches(text, function) + /* * Assembly instructions for schema queries * @@ -2179,7 +2189,7 @@ match_previous_words(int pattern_id, /* for INDEX and TABLE/SEQUENCE, respectively */ "UNIQUE", "UNLOGGED"); else - matches = rl_completion_matches(text, create_command_generator); + COMPLETE_WITH_GENERATOR(create_command_generator); } /* complete with something you can create or replace */ else if (TailMatches("CREATE", "OR", "REPLACE")) @@ -2189,7 +2199,7 @@ match_previous_words(int pattern_id, /* DROP, but not DROP embedded in other commands */ /* complete with something you can drop */ else if (Matches("DROP")) - matches = rl_completion_matches(text, drop_command_generator); + COMPLETE_WITH_GENERATOR(drop_command_generator); /* ALTER */ @@ -2200,7 +2210,7 @@ match_previous_words(int pattern_id, /* ALTER something */ else if (Matches("ALTER")) - matches = rl_completion_matches(text, alter_command_generator); + COMPLETE_WITH_GENERATOR(alter_command_generator); /* ALTER TABLE,INDEX,MATERIALIZED VIEW ALL IN TABLESPACE xxx */ else if (TailMatches("ALL", "IN", "TABLESPACE", MatchAny)) COMPLETE_WITH("SET TABLESPACE", "OWNED BY"); @@ -3308,17 +3318,9 @@ match_previous_words(int pattern_id, COMPLETE_WITH("FROM", "TO"); /* Complete COPY <sth> FROM|TO with filename */ else if (Matches("COPY", MatchAny, "FROM|TO")) - { - completion_charp = ""; - completion_force_quote = true; /* COPY requires quoted filename */ - matches = rl_completion_matches(text, complete_from_files); - } + COMPLETE_WITH_FILES("", true); /* COPY requires quoted filename */ else if (Matches("\\copy", MatchAny, "FROM|TO")) - { - completion_charp = ""; - completion_force_quote = false; - matches = rl_completion_matches(text, complete_from_files); - } + COMPLETE_WITH_FILES("", false); /* Complete COPY <sth> TO <sth> */ else if (Matches("COPY|\\copy", MatchAny, "TO", MatchAny)) @@ -5417,9 +5419,9 @@ match_previous_words(int pattern_id, else if (TailMatchesCS("\\h|\\help", MatchAny)) { if (TailMatches("DROP")) - matches = rl_completion_matches(text, drop_command_generator); + COMPLETE_WITH_GENERATOR(drop_command_generator); else if (TailMatches("ALTER")) - matches = rl_completion_matches(text, alter_command_generator); + COMPLETE_WITH_GENERATOR(alter_command_generator); /* * CREATE is recognized by tail match elsewhere, so doesn't need to be @@ -5519,11 +5521,7 @@ match_previous_words(int pattern_id, else if (TailMatchesCS("\\cd|\\e|\\edit|\\g|\\gx|\\i|\\include|" "\\ir|\\include_relative|\\o|\\out|" "\\s|\\w|\\write|\\lo_import")) - { - completion_charp = "\\"; - completion_force_quote = false; - matches = rl_completion_matches(text, complete_from_files); - } + COMPLETE_WITH_FILES("\\", false); /* gen_tabcomplete.pl ends special processing here */ /* END GEN_TABCOMPLETE */ -- 2.43.0 --Multipart=_Fri__18_Jul_2025_16_49_28_+0900_ThDewLMcz6Ys6Zwo-- ^ permalink raw reply [nested|flat] 7+ messages in thread
* [PATCH v6 1/3] Refactor match_previous_words() to remove direct use of rl_completion_matches() @ 2025-06-05 00:38 Yugo Nagata <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Yugo Nagata @ 2025-06-05 00:38 UTC (permalink / raw) Most tab completions in match_previous_words() use COMPLETE_WITH* macros, which wrap rl_completion_matches(). However, some direct calls to rl_completion_matches() still remained. This commit replaces the remaining direct calls with the new macro, COMPLETE_WITH_FILES or COMPLETE_WITH_GENERATOR, for improved consistency and readability. --- src/bin/psql/tab-complete.in.c | 38 ++++++++++++++++------------------ 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c index 6b20a4404b2..6176741d20b 100644 --- a/src/bin/psql/tab-complete.in.c +++ b/src/bin/psql/tab-complete.in.c @@ -443,6 +443,16 @@ do { \ matches = rl_completion_matches(text, complete_from_schema_query); \ } while (0) +#define COMPLETE_WITH_FILES(escape, force_quote) \ +do { \ + completion_charp = escape; \ + completion_force_quote = force_quote; \ + matches = rl_completion_matches(text, complete_from_files); \ +} while (0) + +#define COMPLETE_WITH_GENERATOR(generator) \ + matches = rl_completion_matches(text, generator) + /* * Assembly instructions for schema queries * @@ -2182,7 +2192,7 @@ match_previous_words(int pattern_id, /* for INDEX and TABLE/SEQUENCE, respectively */ "UNIQUE", "UNLOGGED"); else - matches = rl_completion_matches(text, create_command_generator); + COMPLETE_WITH_GENERATOR(create_command_generator); } /* complete with something you can create or replace */ else if (TailMatches("CREATE", "OR", "REPLACE")) @@ -2192,7 +2202,7 @@ match_previous_words(int pattern_id, /* DROP, but not DROP embedded in other commands */ /* complete with something you can drop */ else if (Matches("DROP")) - matches = rl_completion_matches(text, drop_command_generator); + COMPLETE_WITH_GENERATOR(drop_command_generator); /* ALTER */ @@ -2203,7 +2213,7 @@ match_previous_words(int pattern_id, /* ALTER something */ else if (Matches("ALTER")) - matches = rl_completion_matches(text, alter_command_generator); + COMPLETE_WITH_GENERATOR(alter_command_generator); /* ALTER TABLE,INDEX,MATERIALIZED VIEW ALL IN TABLESPACE xxx */ else if (TailMatches("ALL", "IN", "TABLESPACE", MatchAny)) COMPLETE_WITH("SET TABLESPACE", "OWNED BY"); @@ -3316,17 +3326,9 @@ match_previous_words(int pattern_id, COMPLETE_WITH("FROM", "TO"); /* Complete COPY <sth> FROM|TO with filename */ else if (Matches("COPY", MatchAny, "FROM|TO")) - { - completion_charp = ""; - completion_force_quote = true; /* COPY requires quoted filename */ - matches = rl_completion_matches(text, complete_from_files); - } + COMPLETE_WITH_FILES("", true); /* COPY requires quoted filename */ else if (Matches("\\copy", MatchAny, "FROM|TO")) - { - completion_charp = ""; - completion_force_quote = false; - matches = rl_completion_matches(text, complete_from_files); - } + COMPLETE_WITH_FILES("", false); /* Complete COPY <sth> TO <sth> */ else if (Matches("COPY|\\copy", MatchAny, "TO", MatchAny)) @@ -5427,9 +5429,9 @@ match_previous_words(int pattern_id, else if (TailMatchesCS("\\h|\\help", MatchAny)) { if (TailMatches("DROP")) - matches = rl_completion_matches(text, drop_command_generator); + COMPLETE_WITH_GENERATOR(drop_command_generator); else if (TailMatches("ALTER")) - matches = rl_completion_matches(text, alter_command_generator); + COMPLETE_WITH_GENERATOR(alter_command_generator); /* * CREATE is recognized by tail match elsewhere, so doesn't need to be @@ -5529,11 +5531,7 @@ match_previous_words(int pattern_id, else if (TailMatchesCS("\\cd|\\e|\\edit|\\g|\\gx|\\i|\\include|" "\\ir|\\include_relative|\\o|\\out|" "\\s|\\w|\\write|\\lo_import")) - { - completion_charp = "\\"; - completion_force_quote = false; - matches = rl_completion_matches(text, complete_from_files); - } + COMPLETE_WITH_FILES("\\", false); /* gen_tabcomplete.pl ends special processing here */ /* END GEN_TABCOMPLETE */ -- 2.43.0 --Multipart=_Thu__18_Sep_2025_12_05_06_+0900_I2R/usu+NR/K2VUG-- ^ permalink raw reply [nested|flat] 7+ messages in thread
* [PATCH 1/3] Refactor match_previous_words() to remove direct use of rl_completion_matches() @ 2025-06-05 00:38 Yugo Nagata <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Yugo Nagata @ 2025-06-05 00:38 UTC (permalink / raw) Most tab completions in match_previous_words() use COMPLETE_WITH* macros, which wrap rl_completion_matches(). However, some direct calls to rl_completion_matches() still remained. This commit replaces the remaining direct calls with the new macro, COMPLETE_WITH_FILES or COMPLETE_WITH_GENERATOR, for improved consistency and readability. --- src/bin/psql/tab-complete.in.c | 38 ++++++++++++++++------------------ 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c index ec65ab79fec..8a85a285281 100644 --- a/src/bin/psql/tab-complete.in.c +++ b/src/bin/psql/tab-complete.in.c @@ -443,6 +443,16 @@ do { \ matches = rl_completion_matches(text, complete_from_schema_query); \ } while (0) +#define COMPLETE_WITH_FILES(escape, force_quote) \ +do { \ + completion_charp = escape; \ + completion_force_quote = force_quote; \ + matches = rl_completion_matches(text, complete_from_files); \ +} while (0) + +#define COMPLETE_WITH_GENERATOR(function) \ + matches = rl_completion_matches(text, function) + /* * Assembly instructions for schema queries * @@ -2158,7 +2168,7 @@ match_previous_words(int pattern_id, /* for INDEX and TABLE/SEQUENCE, respectively */ "UNIQUE", "UNLOGGED"); else - matches = rl_completion_matches(text, create_command_generator); + COMPLETE_WITH_GENERATOR(create_command_generator); } /* complete with something you can create or replace */ else if (TailMatches("CREATE", "OR", "REPLACE")) @@ -2168,7 +2178,7 @@ match_previous_words(int pattern_id, /* DROP, but not DROP embedded in other commands */ /* complete with something you can drop */ else if (Matches("DROP")) - matches = rl_completion_matches(text, drop_command_generator); + COMPLETE_WITH_GENERATOR(drop_command_generator); /* ALTER */ @@ -2179,7 +2189,7 @@ match_previous_words(int pattern_id, /* ALTER something */ else if (Matches("ALTER")) - matches = rl_completion_matches(text, alter_command_generator); + COMPLETE_WITH_GENERATOR(alter_command_generator); /* ALTER TABLE,INDEX,MATERIALIZED VIEW ALL IN TABLESPACE xxx */ else if (TailMatches("ALL", "IN", "TABLESPACE", MatchAny)) COMPLETE_WITH("SET TABLESPACE", "OWNED BY"); @@ -3264,17 +3274,9 @@ match_previous_words(int pattern_id, COMPLETE_WITH("FROM", "TO"); /* Complete COPY <sth> FROM|TO with filename */ else if (Matches("COPY", MatchAny, "FROM|TO")) - { - completion_charp = ""; - completion_force_quote = true; /* COPY requires quoted filename */ - matches = rl_completion_matches(text, complete_from_files); - } + COMPLETE_WITH_FILES("", true); /* COPY requires quoted filename */ else if (Matches("\\copy", MatchAny, "FROM|TO")) - { - completion_charp = ""; - completion_force_quote = false; - matches = rl_completion_matches(text, complete_from_files); - } + COMPLETE_WITH_FILES("", false); /* Complete COPY <sth> TO <sth> */ else if (Matches("COPY|\\copy", MatchAny, "TO", MatchAny)) @@ -5347,9 +5349,9 @@ match_previous_words(int pattern_id, else if (TailMatchesCS("\\h|\\help", MatchAny)) { if (TailMatches("DROP")) - matches = rl_completion_matches(text, drop_command_generator); + COMPLETE_WITH_GENERATOR(drop_command_generator); else if (TailMatches("ALTER")) - matches = rl_completion_matches(text, alter_command_generator); + COMPLETE_WITH_GENERATOR(alter_command_generator); /* * CREATE is recognized by tail match elsewhere, so doesn't need to be @@ -5449,11 +5451,7 @@ match_previous_words(int pattern_id, else if (TailMatchesCS("\\cd|\\e|\\edit|\\g|\\gx|\\i|\\include|" "\\ir|\\include_relative|\\o|\\out|" "\\s|\\w|\\write|\\lo_import")) - { - completion_charp = "\\"; - completion_force_quote = false; - matches = rl_completion_matches(text, complete_from_files); - } + COMPLETE_WITH_FILES("\\", false); /* gen_tabcomplete.pl ends special processing here */ /* END GEN_TABCOMPLETE */ -- 2.43.0 --Multipart=_Thu__5_Jun_2025_10_08_35_+0900_P.h1uSLN4yp_7Ild-- ^ permalink raw reply [nested|flat] 7+ messages in thread
* [PATCH v2 1/3] Refactor match_previous_words() to remove direct use of rl_completion_matches() @ 2025-06-05 00:38 Yugo Nagata <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Yugo Nagata @ 2025-06-05 00:38 UTC (permalink / raw) Most tab completions in match_previous_words() use COMPLETE_WITH* macros, which wrap rl_completion_matches(). However, some direct calls to rl_completion_matches() still remained. This commit replaces the remaining direct calls with the new macro, COMPLETE_WITH_FILES or COMPLETE_WITH_GENERATOR, for improved consistency and readability. --- src/bin/psql/tab-complete.in.c | 38 ++++++++++++++++------------------ 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c index ec65ab79fec..8a85a285281 100644 --- a/src/bin/psql/tab-complete.in.c +++ b/src/bin/psql/tab-complete.in.c @@ -443,6 +443,16 @@ do { \ matches = rl_completion_matches(text, complete_from_schema_query); \ } while (0) +#define COMPLETE_WITH_FILES(escape, force_quote) \ +do { \ + completion_charp = escape; \ + completion_force_quote = force_quote; \ + matches = rl_completion_matches(text, complete_from_files); \ +} while (0) + +#define COMPLETE_WITH_GENERATOR(function) \ + matches = rl_completion_matches(text, function) + /* * Assembly instructions for schema queries * @@ -2158,7 +2168,7 @@ match_previous_words(int pattern_id, /* for INDEX and TABLE/SEQUENCE, respectively */ "UNIQUE", "UNLOGGED"); else - matches = rl_completion_matches(text, create_command_generator); + COMPLETE_WITH_GENERATOR(create_command_generator); } /* complete with something you can create or replace */ else if (TailMatches("CREATE", "OR", "REPLACE")) @@ -2168,7 +2178,7 @@ match_previous_words(int pattern_id, /* DROP, but not DROP embedded in other commands */ /* complete with something you can drop */ else if (Matches("DROP")) - matches = rl_completion_matches(text, drop_command_generator); + COMPLETE_WITH_GENERATOR(drop_command_generator); /* ALTER */ @@ -2179,7 +2189,7 @@ match_previous_words(int pattern_id, /* ALTER something */ else if (Matches("ALTER")) - matches = rl_completion_matches(text, alter_command_generator); + COMPLETE_WITH_GENERATOR(alter_command_generator); /* ALTER TABLE,INDEX,MATERIALIZED VIEW ALL IN TABLESPACE xxx */ else if (TailMatches("ALL", "IN", "TABLESPACE", MatchAny)) COMPLETE_WITH("SET TABLESPACE", "OWNED BY"); @@ -3264,17 +3274,9 @@ match_previous_words(int pattern_id, COMPLETE_WITH("FROM", "TO"); /* Complete COPY <sth> FROM|TO with filename */ else if (Matches("COPY", MatchAny, "FROM|TO")) - { - completion_charp = ""; - completion_force_quote = true; /* COPY requires quoted filename */ - matches = rl_completion_matches(text, complete_from_files); - } + COMPLETE_WITH_FILES("", true); /* COPY requires quoted filename */ else if (Matches("\\copy", MatchAny, "FROM|TO")) - { - completion_charp = ""; - completion_force_quote = false; - matches = rl_completion_matches(text, complete_from_files); - } + COMPLETE_WITH_FILES("", false); /* Complete COPY <sth> TO <sth> */ else if (Matches("COPY|\\copy", MatchAny, "TO", MatchAny)) @@ -5347,9 +5349,9 @@ match_previous_words(int pattern_id, else if (TailMatchesCS("\\h|\\help", MatchAny)) { if (TailMatches("DROP")) - matches = rl_completion_matches(text, drop_command_generator); + COMPLETE_WITH_GENERATOR(drop_command_generator); else if (TailMatches("ALTER")) - matches = rl_completion_matches(text, alter_command_generator); + COMPLETE_WITH_GENERATOR(alter_command_generator); /* * CREATE is recognized by tail match elsewhere, so doesn't need to be @@ -5449,11 +5451,7 @@ match_previous_words(int pattern_id, else if (TailMatchesCS("\\cd|\\e|\\edit|\\g|\\gx|\\i|\\include|" "\\ir|\\include_relative|\\o|\\out|" "\\s|\\w|\\write|\\lo_import")) - { - completion_charp = "\\"; - completion_force_quote = false; - matches = rl_completion_matches(text, complete_from_files); - } + COMPLETE_WITH_FILES("\\", false); /* gen_tabcomplete.pl ends special processing here */ /* END GEN_TABCOMPLETE */ -- 2.43.0 --Multipart=_Thu__5_Jun_2025_16_52_00_+0900_d_5fKAN_xESbgyqx-- ^ permalink raw reply [nested|flat] 7+ messages in thread
* [PATCH v3 1/3] Refactor match_previous_words() to remove direct use of rl_completion_matches() @ 2025-06-05 00:38 Yugo Nagata <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Yugo Nagata @ 2025-06-05 00:38 UTC (permalink / raw) Most tab completions in match_previous_words() use COMPLETE_WITH* macros, which wrap rl_completion_matches(). However, some direct calls to rl_completion_matches() still remained. This commit replaces the remaining direct calls with the new macro, COMPLETE_WITH_FILES or COMPLETE_WITH_GENERATOR, for improved consistency and readability. --- src/bin/psql/tab-complete.in.c | 38 ++++++++++++++++------------------ 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c index 2c0b4f28c14..2716554aa0b 100644 --- a/src/bin/psql/tab-complete.in.c +++ b/src/bin/psql/tab-complete.in.c @@ -443,6 +443,16 @@ do { \ matches = rl_completion_matches(text, complete_from_schema_query); \ } while (0) +#define COMPLETE_WITH_FILES(escape, force_quote) \ +do { \ + completion_charp = escape; \ + completion_force_quote = force_quote; \ + matches = rl_completion_matches(text, complete_from_files); \ +} while (0) + +#define COMPLETE_WITH_GENERATOR(function) \ + matches = rl_completion_matches(text, function) + /* * Assembly instructions for schema queries * @@ -2158,7 +2168,7 @@ match_previous_words(int pattern_id, /* for INDEX and TABLE/SEQUENCE, respectively */ "UNIQUE", "UNLOGGED"); else - matches = rl_completion_matches(text, create_command_generator); + COMPLETE_WITH_GENERATOR(create_command_generator); } /* complete with something you can create or replace */ else if (TailMatches("CREATE", "OR", "REPLACE")) @@ -2168,7 +2178,7 @@ match_previous_words(int pattern_id, /* DROP, but not DROP embedded in other commands */ /* complete with something you can drop */ else if (Matches("DROP")) - matches = rl_completion_matches(text, drop_command_generator); + COMPLETE_WITH_GENERATOR(drop_command_generator); /* ALTER */ @@ -2179,7 +2189,7 @@ match_previous_words(int pattern_id, /* ALTER something */ else if (Matches("ALTER")) - matches = rl_completion_matches(text, alter_command_generator); + COMPLETE_WITH_GENERATOR(alter_command_generator); /* ALTER TABLE,INDEX,MATERIALIZED VIEW ALL IN TABLESPACE xxx */ else if (TailMatches("ALL", "IN", "TABLESPACE", MatchAny)) COMPLETE_WITH("SET TABLESPACE", "OWNED BY"); @@ -3264,17 +3274,9 @@ match_previous_words(int pattern_id, COMPLETE_WITH("FROM", "TO"); /* Complete COPY <sth> FROM|TO with filename */ else if (Matches("COPY", MatchAny, "FROM|TO")) - { - completion_charp = ""; - completion_force_quote = true; /* COPY requires quoted filename */ - matches = rl_completion_matches(text, complete_from_files); - } + COMPLETE_WITH_FILES("", true); /* COPY requires quoted filename */ else if (Matches("\\copy", MatchAny, "FROM|TO")) - { - completion_charp = ""; - completion_force_quote = false; - matches = rl_completion_matches(text, complete_from_files); - } + COMPLETE_WITH_FILES("", false); /* Complete COPY <sth> TO <sth> */ else if (Matches("COPY|\\copy", MatchAny, "TO", MatchAny)) @@ -5348,9 +5350,9 @@ match_previous_words(int pattern_id, else if (TailMatchesCS("\\h|\\help", MatchAny)) { if (TailMatches("DROP")) - matches = rl_completion_matches(text, drop_command_generator); + COMPLETE_WITH_GENERATOR(drop_command_generator); else if (TailMatches("ALTER")) - matches = rl_completion_matches(text, alter_command_generator); + COMPLETE_WITH_GENERATOR(alter_command_generator); /* * CREATE is recognized by tail match elsewhere, so doesn't need to be @@ -5450,11 +5452,7 @@ match_previous_words(int pattern_id, else if (TailMatchesCS("\\cd|\\e|\\edit|\\g|\\gx|\\i|\\include|" "\\ir|\\include_relative|\\o|\\out|" "\\s|\\w|\\write|\\lo_import")) - { - completion_charp = "\\"; - completion_force_quote = false; - matches = rl_completion_matches(text, complete_from_files); - } + COMPLETE_WITH_FILES("\\", false); /* gen_tabcomplete.pl ends special processing here */ /* END GEN_TABCOMPLETE */ -- 2.43.0 --Multipart=_Tue__17_Jun_2025_00_08_32_+0900_g9awdn.2U/+M5mrO-- ^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2025-06-05 00:38 UTC | newest] Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2024-03-26 14:04 [PATCH v7 14/16] Count tuples for vacuum logging in heap_page_prune Melanie Plageman <[email protected]> 2025-06-05 00:38 [PATCH v4 1/3] Refactor match_previous_words() to remove direct use of rl_completion_matches() Yugo Nagata <[email protected]> 2025-06-05 00:38 [PATCH v5 1/3] Refactor match_previous_words() to remove direct use of rl_completion_matches() Yugo Nagata <[email protected]> 2025-06-05 00:38 [PATCH v6 1/3] Refactor match_previous_words() to remove direct use of rl_completion_matches() Yugo Nagata <[email protected]> 2025-06-05 00:38 [PATCH 1/3] Refactor match_previous_words() to remove direct use of rl_completion_matches() Yugo Nagata <[email protected]> 2025-06-05 00:38 [PATCH v2 1/3] Refactor match_previous_words() to remove direct use of rl_completion_matches() Yugo Nagata <[email protected]> 2025-06-05 00:38 [PATCH v3 1/3] Refactor match_previous_words() to remove direct use of rl_completion_matches() Yugo Nagata <[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