public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v5 2/3] Improve pg_ctl postmaster process check on Windows 15+ messages / 5 participants [nested] [flat]
* [PATCH v5 2/3] Improve pg_ctl postmaster process check on Windows @ 2023-10-24 05:46 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 15+ messages in thread From: Kyotaro Horiguchi @ 2023-10-24 05:46 UTC (permalink / raw) Currently pg_ctl on Windows does not verify that it actually executed a postmaster process due to lack of process ID knowledge. This can lead to false positives in cases where another pg_ctl instance starts a different server simultaneously. This patch adds the capability to identify the process ID of the launched postmaster on Windows, similar to other OS versions, ensuring more reliable detection of concurrent server startups. --- src/bin/pg_ctl/pg_ctl.c | 102 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 93 insertions(+), 9 deletions(-) diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c index 3ac2fcc004..221049db0e 100644 --- a/src/bin/pg_ctl/pg_ctl.c +++ b/src/bin/pg_ctl/pg_ctl.c @@ -132,6 +132,7 @@ static void adjust_data_dir(void); #ifdef WIN32 #include <versionhelpers.h> +#include <tlhelp32.h> static bool pgwin32_IsInstalled(SC_HANDLE); static char *pgwin32_CommandLine(bool); static void pgwin32_doRegister(void); @@ -142,6 +143,7 @@ static void WINAPI pgwin32_ServiceMain(DWORD, LPTSTR *); static void pgwin32_doRunAsService(void); static int CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo, bool as_service); static PTOKEN_PRIVILEGES GetPrivilegesToDelete(HANDLE hToken); +static pid_t pgwin32_find_postmaster_pid(pid_t shell_pid); #endif static pid_t get_pgpid(bool is_status_request); @@ -609,7 +611,11 @@ wait_for_postmaster_start(pid_t pm_pid, bool do_checkpoint) /* File is complete enough for us, parse it */ pid_t pmpid; time_t pmstart; - +#ifndef WIN32 + pid_t wait_pid = pm_pid; +#else + pid_t wait_pid = pgwin32_find_postmaster_pid(pm_pid); +#endif /* * Make sanity checks. If it's for the wrong PID, or the recorded * start time is before pg_ctl started, then either we are looking @@ -619,14 +625,8 @@ wait_for_postmaster_start(pid_t pm_pid, bool do_checkpoint) */ pmpid = atol(optlines[LOCK_FILE_LINE_PID - 1]); pmstart = atol(optlines[LOCK_FILE_LINE_START_TIME - 1]); - if (pmstart >= start_time - 2 && -#ifndef WIN32 - pmpid == pm_pid -#else - /* Windows can only reject standalone-backend PIDs */ - pmpid > 0 -#endif - ) + + if (pmstart >= start_time - 2 && pmpid == wait_pid) { /* * OK, seems to be a valid pidfile from our child. Check the @@ -1950,6 +1950,90 @@ GetPrivilegesToDelete(HANDLE hToken) return tokenPrivs; } + +/* + * Find the PID of the launched postmaster. + * + * On Windows, the cmd.exe doesn't support the exec command. As a result, we + * don't directly get the postmaster's PID. This function identifies the PID of + * the postmaster started by the child cmd.exe. + * + * Returns the postmaster's PID. If the shell is alive but the postmaster is + * missing, returns 0. Otherwise terminates this command with an error. + * + * This function uses PID 0 as an invalid value, assuming the system idle + * process occupies it and it won't be a PID for a shell or postmaster. + */ +static pid_t +pgwin32_find_postmaster_pid(pid_t shell_pid) +{ + HANDLE hSnapshot; + PROCESSENTRY32 ppe; + pid_t pm_pid = 0; /* abusing 0 as an invalid value */ + bool multiple_children = false; + DWORD last_error; + + /* create a process snapshot */ + hSnapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0); + if (hSnapshot == INVALID_HANDLE_VALUE) + { + write_stderr(_("%s: CreateToolhelp32Snapshot failed: error code %lu\n"), + progname, (unsigned long) GetLastError()); + exit(1); + } + + /* start iterating on the snapshot */ + ppe.dwSize = sizeof(PROCESSENTRY32); + if (!Process32First(hSnapshot, &ppe)) + { + write_stderr(_("%s: Process32First failed: error code %lu\n"), + progname, (unsigned long) GetLastError()); + exit(1); + } + + /* + * Iterate over the snapshot + * + * Check for duplicate processes to ensure reliability. + * + * The launcher shell might start other cmd.exe instances or programs + * besides postgres.exe. Veryfying the program file name is essential. + * + * The launcher shell process isn't checked in this function. It will be + * checked by the caller. + */ + do + { + if (ppe.th32ParentProcessID == shell_pid && + strcmp("postgres.exe", ppe.szExeFile) == 0) + { + if (pm_pid != ppe.th32ProcessID && pm_pid != 0) + multiple_children = true; + pm_pid = ppe.th32ProcessID; + } + } + while (Process32Next(hSnapshot, &ppe)); + + /* avoid multiple calls primary for clarity, not out of necessity */ + last_error = GetLastError(); + if (last_error != ERROR_NO_MORE_FILES) + { + write_stderr(_("%s: Process32Next failed: error code %lu\n"), + progname, (unsigned long) last_error); + exit(1); + } + CloseHandle(hSnapshot); + + /* assuming the launching shell executes a single process */ + if (multiple_children) + { + write_stderr(_("%s: multiple postmasters found\n"), + progname); + exit(1); + } + + return pm_pid; +} #endif /* WIN32 */ static void -- 2.39.3 ----Next_Part(Tue_Oct_24_17_25_36_2023_034)-- Content-Type: Text/X-Patch; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="v5-0003-Remove-short-sleep-from-001_start_stop.pl.patch" ^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v3] Add "Backpatch" regions in wait_event_names.txt @ 2024-03-18 08:34 Bertrand Drouvot <[email protected]> 0 siblings, 0 replies; 15+ messages in thread From: Bertrand Drouvot @ 2024-03-18 08:34 UTC (permalink / raw) When backpatching, adding an event will renumber others, which can make an extension report the wrong event until recompiled. This is due to the fact that generate-wait_event_types.pl sorts events to position them. The "Backpatch" region added here ensures no ordering is done for the wait events found after this delimiter. --- .../activity/generate-wait_event_types.pl | 26 ++++++++++++++++++- .../utils/activity/wait_event_names.txt | 18 ++++++++++++- 2 files changed, 42 insertions(+), 2 deletions(-) 100.0% src/backend/utils/activity/ diff --git a/src/backend/utils/activity/generate-wait_event_types.pl b/src/backend/utils/activity/generate-wait_event_types.pl index f1adf0e8e7..d129d94889 100644 --- a/src/backend/utils/activity/generate-wait_event_types.pl +++ b/src/backend/utils/activity/generate-wait_event_types.pl @@ -38,7 +38,9 @@ die "Not possible to specify --docs and --code simultaneously" open my $wait_event_names, '<', $ARGV[0] or die; +my @backpatch_lines; my @lines; +my $backpatch = 0; my $section_name; my $note; my $note_name; @@ -59,10 +61,26 @@ while (<$wait_event_names>) { $section_name = $_; $section_name =~ s/^.*- //; + $backpatch = 0; next; } - push(@lines, $section_name . "\t" . $_); + # Are we dealing with backpatch wait events? + if (/^Backpatch:$/) + { + $backpatch = 1; + next; + } + + # Backpatch wait events won't be sorted during code generation + if ($gen_code && $backpatch) + { + push(@backpatch_lines, $section_name . "\t" . $_); + } + else + { + push(@lines, $section_name . "\t" . $_); + } } # Sort the lines based on the second column. @@ -70,6 +88,12 @@ while (<$wait_event_names>) my @lines_sorted = sort { uc((split(/\t/, $a))[1]) cmp uc((split(/\t/, $b))[1]) } @lines; +# If we are generating code then concat @lines_sorted and @backpatch_lines. +if ($gen_code) +{ + push(@lines_sorted, @backpatch_lines); +} + # Read the sorted lines and populate the hash table foreach my $line (@lines_sorted) { diff --git a/src/backend/utils/activity/wait_event_names.txt b/src/backend/utils/activity/wait_event_names.txt index c08e00d1d6..0c4788fe77 100644 --- a/src/backend/utils/activity/wait_event_names.txt +++ b/src/backend/utils/activity/wait_event_names.txt @@ -24,7 +24,10 @@ # SGML tables of wait events for inclusion in the documentation. # # When adding a new wait event, make sure it is placed in the appropriate -# ClassName section. +# ClassName section. Put it in its natural position in the master branch, and +# then put it in the "Backpatch:" region for any other branch (should the wait +# event be backpatched). +# Ensure that the backpatch regions are always empty on the master branch. # # WaitEventLWLock and WaitEventLock have their own C code for their wait event # enums and function names. Hence, for these, only the SGML documentation is @@ -61,6 +64,7 @@ WAL_SENDER_MAIN "Waiting in main loop of WAL sender process." WAL_SUMMARIZER_WAL "Waiting in WAL summarizer for more WAL to be generated." WAL_WRITER_MAIN "Waiting in main loop of WAL writer process." +Backpatch: # # Wait Events - Client @@ -82,6 +86,7 @@ WAIT_FOR_STANDBY_CONFIRMATION "Waiting for WAL to be received and flushed by the WAL_SENDER_WAIT_FOR_WAL "Waiting for WAL to be flushed in WAL sender process." WAL_SENDER_WRITE_DATA "Waiting for any activity when processing replies from WAL receiver in WAL sender process." +Backpatch: # # Wait Events - IPC @@ -149,6 +154,7 @@ WAL_RECEIVER_WAIT_START "Waiting for startup process to send initial data for st WAL_SUMMARY_READY "Waiting for a new WAL summary to be generated." XACT_GROUP_UPDATE "Waiting for the group leader to update transaction status at end of a parallel operation." +Backpatch: # # Wait Events - Timeout @@ -169,6 +175,7 @@ VACUUM_DELAY "Waiting in a cost-based vacuum delay point." VACUUM_TRUNCATE "Waiting to acquire an exclusive lock to truncate off any empty pages at the end of a table vacuumed." WAL_SUMMARIZER_ERROR "Waiting after a WAL summarizer error." +Backpatch: # # Wait Events - IO @@ -256,6 +263,7 @@ WAL_SYNC "Waiting for a WAL file to reach durable storage." WAL_SYNC_METHOD_ASSIGN "Waiting for data to reach durable storage while assigning a new WAL sync method." WAL_WRITE "Waiting for a write to a WAL file." +Backpatch: # # Wait Events - Buffer Pin @@ -265,6 +273,7 @@ Section: ClassName - WaitEventBufferPin BUFFER_PIN "Waiting to acquire an exclusive pin on a buffer." +Backpatch: # # Wait Events - Extension @@ -274,6 +283,8 @@ Section: ClassName - WaitEventExtension Extension "Waiting in an extension." +Backpatch: + # # Wait Events - LWLock # @@ -330,6 +341,8 @@ DSMRegistry "Waiting to read or update the dynamic shared memory registry." InjectionPoint "Waiting to read or update information related to injection points." SerialControl "Waiting to read or update shared <filename>pg_serial</filename> state." +# Don't create a Backpatch region here. + # # END OF PREDEFINED LWLOCKS (DO NOT CHANGE THIS LINE) # @@ -377,6 +390,7 @@ SerialSLRU "Waiting to access the serializable transaction conflict SLRU cache." SubtransSLRU "Waiting to access the sub-transaction SLRU cache." XactSLRU "Waiting to access the transaction status SLRU cache." +# Don't create a Backpatch region here. # # Wait Events - Lock @@ -399,3 +413,5 @@ object "Waiting to acquire a lock on a non-relation database object." userlock "Waiting to acquire a user lock." advisory "Waiting to acquire an advisory user lock." applytransaction "Waiting to acquire a lock on a remote transaction being applied by a logical replication subscriber." + +Backpatch: -- 2.34.1 --m1fyrkxSXqjY+FO1-- ^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v2] Add "Backpatch" regions in wait_event_names.txt @ 2024-03-18 08:34 Bertrand Drouvot <[email protected]> 0 siblings, 0 replies; 15+ messages in thread From: Bertrand Drouvot @ 2024-03-18 08:34 UTC (permalink / raw) When backpatching, adding an event will renumber others, which can make an extension report the wrong event until recompiled. This is due to the fact that generate-wait_event_types.pl sorts events to position them. The "Backpatch" region added here ensures no ordering is done for the wait events found after this delimiter. --- .../activity/generate-wait_event_types.pl | 26 ++++++++++++++++++- .../utils/activity/wait_event_names.txt | 19 +++++++++++++- 2 files changed, 43 insertions(+), 2 deletions(-) 100.0% src/backend/utils/activity/ diff --git a/src/backend/utils/activity/generate-wait_event_types.pl b/src/backend/utils/activity/generate-wait_event_types.pl index f1adf0e8e7..d129d94889 100644 --- a/src/backend/utils/activity/generate-wait_event_types.pl +++ b/src/backend/utils/activity/generate-wait_event_types.pl @@ -38,7 +38,9 @@ die "Not possible to specify --docs and --code simultaneously" open my $wait_event_names, '<', $ARGV[0] or die; +my @backpatch_lines; my @lines; +my $backpatch = 0; my $section_name; my $note; my $note_name; @@ -59,10 +61,26 @@ while (<$wait_event_names>) { $section_name = $_; $section_name =~ s/^.*- //; + $backpatch = 0; next; } - push(@lines, $section_name . "\t" . $_); + # Are we dealing with backpatch wait events? + if (/^Backpatch:$/) + { + $backpatch = 1; + next; + } + + # Backpatch wait events won't be sorted during code generation + if ($gen_code && $backpatch) + { + push(@backpatch_lines, $section_name . "\t" . $_); + } + else + { + push(@lines, $section_name . "\t" . $_); + } } # Sort the lines based on the second column. @@ -70,6 +88,12 @@ while (<$wait_event_names>) my @lines_sorted = sort { uc((split(/\t/, $a))[1]) cmp uc((split(/\t/, $b))[1]) } @lines; +# If we are generating code then concat @lines_sorted and @backpatch_lines. +if ($gen_code) +{ + push(@lines_sorted, @backpatch_lines); +} + # Read the sorted lines and populate the hash table foreach my $line (@lines_sorted) { diff --git a/src/backend/utils/activity/wait_event_names.txt b/src/backend/utils/activity/wait_event_names.txt index c08e00d1d6..b6303a0fdb 100644 --- a/src/backend/utils/activity/wait_event_names.txt +++ b/src/backend/utils/activity/wait_event_names.txt @@ -24,7 +24,11 @@ # SGML tables of wait events for inclusion in the documentation. # # When adding a new wait event, make sure it is placed in the appropriate -# ClassName section. +# ClassName section. If the wait event is backpatched from master to a version +# >= 17 then put it under a "Backpatch:" delimiter at the end of the related +# ClassName section (on the non master branches) or at its natural position on +# the master branch. +# Ensure that the backpatch regions are always empty on the master branch. # # WaitEventLWLock and WaitEventLock have their own C code for their wait event # enums and function names. Hence, for these, only the SGML documentation is @@ -61,6 +65,7 @@ WAL_SENDER_MAIN "Waiting in main loop of WAL sender process." WAL_SUMMARIZER_WAL "Waiting in WAL summarizer for more WAL to be generated." WAL_WRITER_MAIN "Waiting in main loop of WAL writer process." +Backpatch: # # Wait Events - Client @@ -82,6 +87,7 @@ WAIT_FOR_STANDBY_CONFIRMATION "Waiting for WAL to be received and flushed by the WAL_SENDER_WAIT_FOR_WAL "Waiting for WAL to be flushed in WAL sender process." WAL_SENDER_WRITE_DATA "Waiting for any activity when processing replies from WAL receiver in WAL sender process." +Backpatch: # # Wait Events - IPC @@ -149,6 +155,7 @@ WAL_RECEIVER_WAIT_START "Waiting for startup process to send initial data for st WAL_SUMMARY_READY "Waiting for a new WAL summary to be generated." XACT_GROUP_UPDATE "Waiting for the group leader to update transaction status at end of a parallel operation." +Backpatch: # # Wait Events - Timeout @@ -169,6 +176,7 @@ VACUUM_DELAY "Waiting in a cost-based vacuum delay point." VACUUM_TRUNCATE "Waiting to acquire an exclusive lock to truncate off any empty pages at the end of a table vacuumed." WAL_SUMMARIZER_ERROR "Waiting after a WAL summarizer error." +Backpatch: # # Wait Events - IO @@ -256,6 +264,7 @@ WAL_SYNC "Waiting for a WAL file to reach durable storage." WAL_SYNC_METHOD_ASSIGN "Waiting for data to reach durable storage while assigning a new WAL sync method." WAL_WRITE "Waiting for a write to a WAL file." +Backpatch: # # Wait Events - Buffer Pin @@ -265,6 +274,7 @@ Section: ClassName - WaitEventBufferPin BUFFER_PIN "Waiting to acquire an exclusive pin on a buffer." +Backpatch: # # Wait Events - Extension @@ -274,6 +284,8 @@ Section: ClassName - WaitEventExtension Extension "Waiting in an extension." +Backpatch: + # # Wait Events - LWLock # @@ -330,6 +342,8 @@ DSMRegistry "Waiting to read or update the dynamic shared memory registry." InjectionPoint "Waiting to read or update information related to injection points." SerialControl "Waiting to read or update shared <filename>pg_serial</filename> state." +# Don't create a Backpatch region here. + # # END OF PREDEFINED LWLOCKS (DO NOT CHANGE THIS LINE) # @@ -377,6 +391,7 @@ SerialSLRU "Waiting to access the serializable transaction conflict SLRU cache." SubtransSLRU "Waiting to access the sub-transaction SLRU cache." XactSLRU "Waiting to access the transaction status SLRU cache." +# Don't create a Backpatch region here. # # Wait Events - Lock @@ -399,3 +414,5 @@ object "Waiting to acquire a lock on a non-relation database object." userlock "Waiting to acquire a user lock." advisory "Waiting to acquire an advisory user lock." applytransaction "Waiting to acquire a lock on a remote transaction being applied by a logical replication subscriber." + +Backpatch: -- 2.34.1 --Xg/TgUq/K7vNmQT2-- ^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v3] Add "Backpatch" regions in wait_event_names.txt @ 2024-03-18 08:34 Bertrand Drouvot <[email protected]> 0 siblings, 0 replies; 15+ messages in thread From: Bertrand Drouvot @ 2024-03-18 08:34 UTC (permalink / raw) When backpatching, adding an event will renumber others, which can make an extension report the wrong event until recompiled. This is due to the fact that generate-wait_event_types.pl sorts events to position them. The "Backpatch" region added here ensures no ordering is done for the wait events found after this delimiter. --- .../activity/generate-wait_event_types.pl | 26 ++++++++++++++++++- .../utils/activity/wait_event_names.txt | 18 ++++++++++++- 2 files changed, 42 insertions(+), 2 deletions(-) 100.0% src/backend/utils/activity/ diff --git a/src/backend/utils/activity/generate-wait_event_types.pl b/src/backend/utils/activity/generate-wait_event_types.pl index f1adf0e8e7..d129d94889 100644 --- a/src/backend/utils/activity/generate-wait_event_types.pl +++ b/src/backend/utils/activity/generate-wait_event_types.pl @@ -38,7 +38,9 @@ die "Not possible to specify --docs and --code simultaneously" open my $wait_event_names, '<', $ARGV[0] or die; +my @backpatch_lines; my @lines; +my $backpatch = 0; my $section_name; my $note; my $note_name; @@ -59,10 +61,26 @@ while (<$wait_event_names>) { $section_name = $_; $section_name =~ s/^.*- //; + $backpatch = 0; next; } - push(@lines, $section_name . "\t" . $_); + # Are we dealing with backpatch wait events? + if (/^Backpatch:$/) + { + $backpatch = 1; + next; + } + + # Backpatch wait events won't be sorted during code generation + if ($gen_code && $backpatch) + { + push(@backpatch_lines, $section_name . "\t" . $_); + } + else + { + push(@lines, $section_name . "\t" . $_); + } } # Sort the lines based on the second column. @@ -70,6 +88,12 @@ while (<$wait_event_names>) my @lines_sorted = sort { uc((split(/\t/, $a))[1]) cmp uc((split(/\t/, $b))[1]) } @lines; +# If we are generating code then concat @lines_sorted and @backpatch_lines. +if ($gen_code) +{ + push(@lines_sorted, @backpatch_lines); +} + # Read the sorted lines and populate the hash table foreach my $line (@lines_sorted) { diff --git a/src/backend/utils/activity/wait_event_names.txt b/src/backend/utils/activity/wait_event_names.txt index c08e00d1d6..0c4788fe77 100644 --- a/src/backend/utils/activity/wait_event_names.txt +++ b/src/backend/utils/activity/wait_event_names.txt @@ -24,7 +24,10 @@ # SGML tables of wait events for inclusion in the documentation. # # When adding a new wait event, make sure it is placed in the appropriate -# ClassName section. +# ClassName section. Put it in its natural position in the master branch, and +# then put it in the "Backpatch:" region for any other branch (should the wait +# event be backpatched). +# Ensure that the backpatch regions are always empty on the master branch. # # WaitEventLWLock and WaitEventLock have their own C code for their wait event # enums and function names. Hence, for these, only the SGML documentation is @@ -61,6 +64,7 @@ WAL_SENDER_MAIN "Waiting in main loop of WAL sender process." WAL_SUMMARIZER_WAL "Waiting in WAL summarizer for more WAL to be generated." WAL_WRITER_MAIN "Waiting in main loop of WAL writer process." +Backpatch: # # Wait Events - Client @@ -82,6 +86,7 @@ WAIT_FOR_STANDBY_CONFIRMATION "Waiting for WAL to be received and flushed by the WAL_SENDER_WAIT_FOR_WAL "Waiting for WAL to be flushed in WAL sender process." WAL_SENDER_WRITE_DATA "Waiting for any activity when processing replies from WAL receiver in WAL sender process." +Backpatch: # # Wait Events - IPC @@ -149,6 +154,7 @@ WAL_RECEIVER_WAIT_START "Waiting for startup process to send initial data for st WAL_SUMMARY_READY "Waiting for a new WAL summary to be generated." XACT_GROUP_UPDATE "Waiting for the group leader to update transaction status at end of a parallel operation." +Backpatch: # # Wait Events - Timeout @@ -169,6 +175,7 @@ VACUUM_DELAY "Waiting in a cost-based vacuum delay point." VACUUM_TRUNCATE "Waiting to acquire an exclusive lock to truncate off any empty pages at the end of a table vacuumed." WAL_SUMMARIZER_ERROR "Waiting after a WAL summarizer error." +Backpatch: # # Wait Events - IO @@ -256,6 +263,7 @@ WAL_SYNC "Waiting for a WAL file to reach durable storage." WAL_SYNC_METHOD_ASSIGN "Waiting for data to reach durable storage while assigning a new WAL sync method." WAL_WRITE "Waiting for a write to a WAL file." +Backpatch: # # Wait Events - Buffer Pin @@ -265,6 +273,7 @@ Section: ClassName - WaitEventBufferPin BUFFER_PIN "Waiting to acquire an exclusive pin on a buffer." +Backpatch: # # Wait Events - Extension @@ -274,6 +283,8 @@ Section: ClassName - WaitEventExtension Extension "Waiting in an extension." +Backpatch: + # # Wait Events - LWLock # @@ -330,6 +341,8 @@ DSMRegistry "Waiting to read or update the dynamic shared memory registry." InjectionPoint "Waiting to read or update information related to injection points." SerialControl "Waiting to read or update shared <filename>pg_serial</filename> state." +# Don't create a Backpatch region here. + # # END OF PREDEFINED LWLOCKS (DO NOT CHANGE THIS LINE) # @@ -377,6 +390,7 @@ SerialSLRU "Waiting to access the serializable transaction conflict SLRU cache." SubtransSLRU "Waiting to access the sub-transaction SLRU cache." XactSLRU "Waiting to access the transaction status SLRU cache." +# Don't create a Backpatch region here. # # Wait Events - Lock @@ -399,3 +413,5 @@ object "Waiting to acquire a lock on a non-relation database object." userlock "Waiting to acquire a user lock." advisory "Waiting to acquire an advisory user lock." applytransaction "Waiting to acquire a lock on a remote transaction being applied by a logical replication subscriber." + +Backpatch: -- 2.34.1 --m1fyrkxSXqjY+FO1-- ^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v2] Add "Backpatch" regions in wait_event_names.txt @ 2024-03-18 08:34 Bertrand Drouvot <[email protected]> 0 siblings, 0 replies; 15+ messages in thread From: Bertrand Drouvot @ 2024-03-18 08:34 UTC (permalink / raw) When backpatching, adding an event will renumber others, which can make an extension report the wrong event until recompiled. This is due to the fact that generate-wait_event_types.pl sorts events to position them. The "Backpatch" region added here ensures no ordering is done for the wait events found after this delimiter. --- .../activity/generate-wait_event_types.pl | 26 ++++++++++++++++++- .../utils/activity/wait_event_names.txt | 19 +++++++++++++- 2 files changed, 43 insertions(+), 2 deletions(-) 100.0% src/backend/utils/activity/ diff --git a/src/backend/utils/activity/generate-wait_event_types.pl b/src/backend/utils/activity/generate-wait_event_types.pl index f1adf0e8e7..d129d94889 100644 --- a/src/backend/utils/activity/generate-wait_event_types.pl +++ b/src/backend/utils/activity/generate-wait_event_types.pl @@ -38,7 +38,9 @@ die "Not possible to specify --docs and --code simultaneously" open my $wait_event_names, '<', $ARGV[0] or die; +my @backpatch_lines; my @lines; +my $backpatch = 0; my $section_name; my $note; my $note_name; @@ -59,10 +61,26 @@ while (<$wait_event_names>) { $section_name = $_; $section_name =~ s/^.*- //; + $backpatch = 0; next; } - push(@lines, $section_name . "\t" . $_); + # Are we dealing with backpatch wait events? + if (/^Backpatch:$/) + { + $backpatch = 1; + next; + } + + # Backpatch wait events won't be sorted during code generation + if ($gen_code && $backpatch) + { + push(@backpatch_lines, $section_name . "\t" . $_); + } + else + { + push(@lines, $section_name . "\t" . $_); + } } # Sort the lines based on the second column. @@ -70,6 +88,12 @@ while (<$wait_event_names>) my @lines_sorted = sort { uc((split(/\t/, $a))[1]) cmp uc((split(/\t/, $b))[1]) } @lines; +# If we are generating code then concat @lines_sorted and @backpatch_lines. +if ($gen_code) +{ + push(@lines_sorted, @backpatch_lines); +} + # Read the sorted lines and populate the hash table foreach my $line (@lines_sorted) { diff --git a/src/backend/utils/activity/wait_event_names.txt b/src/backend/utils/activity/wait_event_names.txt index c08e00d1d6..b6303a0fdb 100644 --- a/src/backend/utils/activity/wait_event_names.txt +++ b/src/backend/utils/activity/wait_event_names.txt @@ -24,7 +24,11 @@ # SGML tables of wait events for inclusion in the documentation. # # When adding a new wait event, make sure it is placed in the appropriate -# ClassName section. +# ClassName section. If the wait event is backpatched from master to a version +# >= 17 then put it under a "Backpatch:" delimiter at the end of the related +# ClassName section (on the non master branches) or at its natural position on +# the master branch. +# Ensure that the backpatch regions are always empty on the master branch. # # WaitEventLWLock and WaitEventLock have their own C code for their wait event # enums and function names. Hence, for these, only the SGML documentation is @@ -61,6 +65,7 @@ WAL_SENDER_MAIN "Waiting in main loop of WAL sender process." WAL_SUMMARIZER_WAL "Waiting in WAL summarizer for more WAL to be generated." WAL_WRITER_MAIN "Waiting in main loop of WAL writer process." +Backpatch: # # Wait Events - Client @@ -82,6 +87,7 @@ WAIT_FOR_STANDBY_CONFIRMATION "Waiting for WAL to be received and flushed by the WAL_SENDER_WAIT_FOR_WAL "Waiting for WAL to be flushed in WAL sender process." WAL_SENDER_WRITE_DATA "Waiting for any activity when processing replies from WAL receiver in WAL sender process." +Backpatch: # # Wait Events - IPC @@ -149,6 +155,7 @@ WAL_RECEIVER_WAIT_START "Waiting for startup process to send initial data for st WAL_SUMMARY_READY "Waiting for a new WAL summary to be generated." XACT_GROUP_UPDATE "Waiting for the group leader to update transaction status at end of a parallel operation." +Backpatch: # # Wait Events - Timeout @@ -169,6 +176,7 @@ VACUUM_DELAY "Waiting in a cost-based vacuum delay point." VACUUM_TRUNCATE "Waiting to acquire an exclusive lock to truncate off any empty pages at the end of a table vacuumed." WAL_SUMMARIZER_ERROR "Waiting after a WAL summarizer error." +Backpatch: # # Wait Events - IO @@ -256,6 +264,7 @@ WAL_SYNC "Waiting for a WAL file to reach durable storage." WAL_SYNC_METHOD_ASSIGN "Waiting for data to reach durable storage while assigning a new WAL sync method." WAL_WRITE "Waiting for a write to a WAL file." +Backpatch: # # Wait Events - Buffer Pin @@ -265,6 +274,7 @@ Section: ClassName - WaitEventBufferPin BUFFER_PIN "Waiting to acquire an exclusive pin on a buffer." +Backpatch: # # Wait Events - Extension @@ -274,6 +284,8 @@ Section: ClassName - WaitEventExtension Extension "Waiting in an extension." +Backpatch: + # # Wait Events - LWLock # @@ -330,6 +342,8 @@ DSMRegistry "Waiting to read or update the dynamic shared memory registry." InjectionPoint "Waiting to read or update information related to injection points." SerialControl "Waiting to read or update shared <filename>pg_serial</filename> state." +# Don't create a Backpatch region here. + # # END OF PREDEFINED LWLOCKS (DO NOT CHANGE THIS LINE) # @@ -377,6 +391,7 @@ SerialSLRU "Waiting to access the serializable transaction conflict SLRU cache." SubtransSLRU "Waiting to access the sub-transaction SLRU cache." XactSLRU "Waiting to access the transaction status SLRU cache." +# Don't create a Backpatch region here. # # Wait Events - Lock @@ -399,3 +414,5 @@ object "Waiting to acquire a lock on a non-relation database object." userlock "Waiting to acquire a user lock." advisory "Waiting to acquire an advisory user lock." applytransaction "Waiting to acquire a lock on a remote transaction being applied by a logical replication subscriber." + +Backpatch: -- 2.34.1 --Xg/TgUq/K7vNmQT2-- ^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v3] Add "Backpatch" regions in wait_event_names.txt @ 2024-03-18 08:34 Bertrand Drouvot <[email protected]> 0 siblings, 0 replies; 15+ messages in thread From: Bertrand Drouvot @ 2024-03-18 08:34 UTC (permalink / raw) When backpatching, adding an event will renumber others, which can make an extension report the wrong event until recompiled. This is due to the fact that generate-wait_event_types.pl sorts events to position them. The "Backpatch" region added here ensures no ordering is done for the wait events found after this delimiter. --- .../activity/generate-wait_event_types.pl | 26 ++++++++++++++++++- .../utils/activity/wait_event_names.txt | 18 ++++++++++++- 2 files changed, 42 insertions(+), 2 deletions(-) 100.0% src/backend/utils/activity/ diff --git a/src/backend/utils/activity/generate-wait_event_types.pl b/src/backend/utils/activity/generate-wait_event_types.pl index f1adf0e8e7..d129d94889 100644 --- a/src/backend/utils/activity/generate-wait_event_types.pl +++ b/src/backend/utils/activity/generate-wait_event_types.pl @@ -38,7 +38,9 @@ die "Not possible to specify --docs and --code simultaneously" open my $wait_event_names, '<', $ARGV[0] or die; +my @backpatch_lines; my @lines; +my $backpatch = 0; my $section_name; my $note; my $note_name; @@ -59,10 +61,26 @@ while (<$wait_event_names>) { $section_name = $_; $section_name =~ s/^.*- //; + $backpatch = 0; next; } - push(@lines, $section_name . "\t" . $_); + # Are we dealing with backpatch wait events? + if (/^Backpatch:$/) + { + $backpatch = 1; + next; + } + + # Backpatch wait events won't be sorted during code generation + if ($gen_code && $backpatch) + { + push(@backpatch_lines, $section_name . "\t" . $_); + } + else + { + push(@lines, $section_name . "\t" . $_); + } } # Sort the lines based on the second column. @@ -70,6 +88,12 @@ while (<$wait_event_names>) my @lines_sorted = sort { uc((split(/\t/, $a))[1]) cmp uc((split(/\t/, $b))[1]) } @lines; +# If we are generating code then concat @lines_sorted and @backpatch_lines. +if ($gen_code) +{ + push(@lines_sorted, @backpatch_lines); +} + # Read the sorted lines and populate the hash table foreach my $line (@lines_sorted) { diff --git a/src/backend/utils/activity/wait_event_names.txt b/src/backend/utils/activity/wait_event_names.txt index c08e00d1d6..0c4788fe77 100644 --- a/src/backend/utils/activity/wait_event_names.txt +++ b/src/backend/utils/activity/wait_event_names.txt @@ -24,7 +24,10 @@ # SGML tables of wait events for inclusion in the documentation. # # When adding a new wait event, make sure it is placed in the appropriate -# ClassName section. +# ClassName section. Put it in its natural position in the master branch, and +# then put it in the "Backpatch:" region for any other branch (should the wait +# event be backpatched). +# Ensure that the backpatch regions are always empty on the master branch. # # WaitEventLWLock and WaitEventLock have their own C code for their wait event # enums and function names. Hence, for these, only the SGML documentation is @@ -61,6 +64,7 @@ WAL_SENDER_MAIN "Waiting in main loop of WAL sender process." WAL_SUMMARIZER_WAL "Waiting in WAL summarizer for more WAL to be generated." WAL_WRITER_MAIN "Waiting in main loop of WAL writer process." +Backpatch: # # Wait Events - Client @@ -82,6 +86,7 @@ WAIT_FOR_STANDBY_CONFIRMATION "Waiting for WAL to be received and flushed by the WAL_SENDER_WAIT_FOR_WAL "Waiting for WAL to be flushed in WAL sender process." WAL_SENDER_WRITE_DATA "Waiting for any activity when processing replies from WAL receiver in WAL sender process." +Backpatch: # # Wait Events - IPC @@ -149,6 +154,7 @@ WAL_RECEIVER_WAIT_START "Waiting for startup process to send initial data for st WAL_SUMMARY_READY "Waiting for a new WAL summary to be generated." XACT_GROUP_UPDATE "Waiting for the group leader to update transaction status at end of a parallel operation." +Backpatch: # # Wait Events - Timeout @@ -169,6 +175,7 @@ VACUUM_DELAY "Waiting in a cost-based vacuum delay point." VACUUM_TRUNCATE "Waiting to acquire an exclusive lock to truncate off any empty pages at the end of a table vacuumed." WAL_SUMMARIZER_ERROR "Waiting after a WAL summarizer error." +Backpatch: # # Wait Events - IO @@ -256,6 +263,7 @@ WAL_SYNC "Waiting for a WAL file to reach durable storage." WAL_SYNC_METHOD_ASSIGN "Waiting for data to reach durable storage while assigning a new WAL sync method." WAL_WRITE "Waiting for a write to a WAL file." +Backpatch: # # Wait Events - Buffer Pin @@ -265,6 +273,7 @@ Section: ClassName - WaitEventBufferPin BUFFER_PIN "Waiting to acquire an exclusive pin on a buffer." +Backpatch: # # Wait Events - Extension @@ -274,6 +283,8 @@ Section: ClassName - WaitEventExtension Extension "Waiting in an extension." +Backpatch: + # # Wait Events - LWLock # @@ -330,6 +341,8 @@ DSMRegistry "Waiting to read or update the dynamic shared memory registry." InjectionPoint "Waiting to read or update information related to injection points." SerialControl "Waiting to read or update shared <filename>pg_serial</filename> state." +# Don't create a Backpatch region here. + # # END OF PREDEFINED LWLOCKS (DO NOT CHANGE THIS LINE) # @@ -377,6 +390,7 @@ SerialSLRU "Waiting to access the serializable transaction conflict SLRU cache." SubtransSLRU "Waiting to access the sub-transaction SLRU cache." XactSLRU "Waiting to access the transaction status SLRU cache." +# Don't create a Backpatch region here. # # Wait Events - Lock @@ -399,3 +413,5 @@ object "Waiting to acquire a lock on a non-relation database object." userlock "Waiting to acquire a user lock." advisory "Waiting to acquire an advisory user lock." applytransaction "Waiting to acquire a lock on a remote transaction being applied by a logical replication subscriber." + +Backpatch: -- 2.34.1 --m1fyrkxSXqjY+FO1-- ^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v2] Add "Backpatch" regions in wait_event_names.txt @ 2024-03-18 08:34 Bertrand Drouvot <[email protected]> 0 siblings, 0 replies; 15+ messages in thread From: Bertrand Drouvot @ 2024-03-18 08:34 UTC (permalink / raw) When backpatching, adding an event will renumber others, which can make an extension report the wrong event until recompiled. This is due to the fact that generate-wait_event_types.pl sorts events to position them. The "Backpatch" region added here ensures no ordering is done for the wait events found after this delimiter. --- .../activity/generate-wait_event_types.pl | 26 ++++++++++++++++++- .../utils/activity/wait_event_names.txt | 19 +++++++++++++- 2 files changed, 43 insertions(+), 2 deletions(-) 100.0% src/backend/utils/activity/ diff --git a/src/backend/utils/activity/generate-wait_event_types.pl b/src/backend/utils/activity/generate-wait_event_types.pl index f1adf0e8e7..d129d94889 100644 --- a/src/backend/utils/activity/generate-wait_event_types.pl +++ b/src/backend/utils/activity/generate-wait_event_types.pl @@ -38,7 +38,9 @@ die "Not possible to specify --docs and --code simultaneously" open my $wait_event_names, '<', $ARGV[0] or die; +my @backpatch_lines; my @lines; +my $backpatch = 0; my $section_name; my $note; my $note_name; @@ -59,10 +61,26 @@ while (<$wait_event_names>) { $section_name = $_; $section_name =~ s/^.*- //; + $backpatch = 0; next; } - push(@lines, $section_name . "\t" . $_); + # Are we dealing with backpatch wait events? + if (/^Backpatch:$/) + { + $backpatch = 1; + next; + } + + # Backpatch wait events won't be sorted during code generation + if ($gen_code && $backpatch) + { + push(@backpatch_lines, $section_name . "\t" . $_); + } + else + { + push(@lines, $section_name . "\t" . $_); + } } # Sort the lines based on the second column. @@ -70,6 +88,12 @@ while (<$wait_event_names>) my @lines_sorted = sort { uc((split(/\t/, $a))[1]) cmp uc((split(/\t/, $b))[1]) } @lines; +# If we are generating code then concat @lines_sorted and @backpatch_lines. +if ($gen_code) +{ + push(@lines_sorted, @backpatch_lines); +} + # Read the sorted lines and populate the hash table foreach my $line (@lines_sorted) { diff --git a/src/backend/utils/activity/wait_event_names.txt b/src/backend/utils/activity/wait_event_names.txt index c08e00d1d6..b6303a0fdb 100644 --- a/src/backend/utils/activity/wait_event_names.txt +++ b/src/backend/utils/activity/wait_event_names.txt @@ -24,7 +24,11 @@ # SGML tables of wait events for inclusion in the documentation. # # When adding a new wait event, make sure it is placed in the appropriate -# ClassName section. +# ClassName section. If the wait event is backpatched from master to a version +# >= 17 then put it under a "Backpatch:" delimiter at the end of the related +# ClassName section (on the non master branches) or at its natural position on +# the master branch. +# Ensure that the backpatch regions are always empty on the master branch. # # WaitEventLWLock and WaitEventLock have their own C code for their wait event # enums and function names. Hence, for these, only the SGML documentation is @@ -61,6 +65,7 @@ WAL_SENDER_MAIN "Waiting in main loop of WAL sender process." WAL_SUMMARIZER_WAL "Waiting in WAL summarizer for more WAL to be generated." WAL_WRITER_MAIN "Waiting in main loop of WAL writer process." +Backpatch: # # Wait Events - Client @@ -82,6 +87,7 @@ WAIT_FOR_STANDBY_CONFIRMATION "Waiting for WAL to be received and flushed by the WAL_SENDER_WAIT_FOR_WAL "Waiting for WAL to be flushed in WAL sender process." WAL_SENDER_WRITE_DATA "Waiting for any activity when processing replies from WAL receiver in WAL sender process." +Backpatch: # # Wait Events - IPC @@ -149,6 +155,7 @@ WAL_RECEIVER_WAIT_START "Waiting for startup process to send initial data for st WAL_SUMMARY_READY "Waiting for a new WAL summary to be generated." XACT_GROUP_UPDATE "Waiting for the group leader to update transaction status at end of a parallel operation." +Backpatch: # # Wait Events - Timeout @@ -169,6 +176,7 @@ VACUUM_DELAY "Waiting in a cost-based vacuum delay point." VACUUM_TRUNCATE "Waiting to acquire an exclusive lock to truncate off any empty pages at the end of a table vacuumed." WAL_SUMMARIZER_ERROR "Waiting after a WAL summarizer error." +Backpatch: # # Wait Events - IO @@ -256,6 +264,7 @@ WAL_SYNC "Waiting for a WAL file to reach durable storage." WAL_SYNC_METHOD_ASSIGN "Waiting for data to reach durable storage while assigning a new WAL sync method." WAL_WRITE "Waiting for a write to a WAL file." +Backpatch: # # Wait Events - Buffer Pin @@ -265,6 +274,7 @@ Section: ClassName - WaitEventBufferPin BUFFER_PIN "Waiting to acquire an exclusive pin on a buffer." +Backpatch: # # Wait Events - Extension @@ -274,6 +284,8 @@ Section: ClassName - WaitEventExtension Extension "Waiting in an extension." +Backpatch: + # # Wait Events - LWLock # @@ -330,6 +342,8 @@ DSMRegistry "Waiting to read or update the dynamic shared memory registry." InjectionPoint "Waiting to read or update information related to injection points." SerialControl "Waiting to read or update shared <filename>pg_serial</filename> state." +# Don't create a Backpatch region here. + # # END OF PREDEFINED LWLOCKS (DO NOT CHANGE THIS LINE) # @@ -377,6 +391,7 @@ SerialSLRU "Waiting to access the serializable transaction conflict SLRU cache." SubtransSLRU "Waiting to access the sub-transaction SLRU cache." XactSLRU "Waiting to access the transaction status SLRU cache." +# Don't create a Backpatch region here. # # Wait Events - Lock @@ -399,3 +414,5 @@ object "Waiting to acquire a lock on a non-relation database object." userlock "Waiting to acquire a user lock." advisory "Waiting to acquire an advisory user lock." applytransaction "Waiting to acquire a lock on a remote transaction being applied by a logical replication subscriber." + +Backpatch: -- 2.34.1 --Xg/TgUq/K7vNmQT2-- ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: tab complete for COPY populated materialized view TO @ 2025-04-09 09:25 Kirill Reshke <[email protected]> 0 siblings, 1 reply; 15+ messages in thread From: Kirill Reshke @ 2025-04-09 09:25 UTC (permalink / raw) To: jian he <[email protected]>; +Cc: pgsql-hackers On Wed, 9 Apr 2025 at 13:23, jian he <[email protected]> wrote: > > hi. > > we allow the "COPY table TO" command to copy rows from materialized > views in [1]. > The attached patch is to add a tab complete for it. > > [1] https://git.postgresql.org/cgit/postgresql.git/commit/?id=534874fac0b34535c9a5ab9257d6574f78423578 Hi! Patch works good for me, but I noticed that psql COPY <tab> suggests partitioned relation both with and without this patch. Maybe that's not a big problem, if [0] will be pushed. [0] https://www.postgresql.org/message-id/CACJufxHjBPrsbNZAp-DCmwvOE_Gkogb%2Brhfqqe1%3DS5cOHR-V7Q%40mail... -- Best regards, Kirill Reshke ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: tab complete for COPY populated materialized view TO @ 2025-04-09 09:45 Fujii Masao <[email protected]> parent: Kirill Reshke <[email protected]> 0 siblings, 1 reply; 15+ messages in thread From: Fujii Masao @ 2025-04-09 09:45 UTC (permalink / raw) To: Kirill Reshke <[email protected]>; jian he <[email protected]>; +Cc: pgsql-hackers On 2025/04/09 18:25, Kirill Reshke wrote: > On Wed, 9 Apr 2025 at 13:23, jian he <[email protected]> wrote: >> >> hi. >> >> we allow the "COPY table TO" command to copy rows from materialized >> views in [1]. >> The attached patch is to add a tab complete for it. >> >> [1] https://git.postgresql.org/cgit/postgresql.git/commit/?id=534874fac0b34535c9a5ab9257d6574f78423578 > > Hi! > Patch works good for me, but I noticed that psql COPY <tab> suggests > partitioned relation both with and without this patch. Maybe that's > not a big problem, if [0] will be pushed. Is the partitioned table currently tab-completed for the COPY FROM case? If we aim to support tab-completion for all valid targets of both COPY TO and COPY FROM, shouldn't foreign tables also be included? And what about views with INSTEAD OF INSERT triggers - though maybe that's overkill? Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: tab complete for COPY populated materialized view TO @ 2025-04-09 10:24 Kirill Reshke <[email protected]> parent: Fujii Masao <[email protected]> 0 siblings, 1 reply; 15+ messages in thread From: Kirill Reshke @ 2025-04-09 10:24 UTC (permalink / raw) To: Fujii Masao <[email protected]>; +Cc: jian he <[email protected]>; pgsql-hackers On Wed, 9 Apr 2025 at 14:45, Fujii Masao <[email protected]> wrote: > > > > On 2025/04/09 18:25, Kirill Reshke wrote: > > On Wed, 9 Apr 2025 at 13:23, jian he <[email protected]> wrote: > >> > >> hi. > >> > >> we allow the "COPY table TO" command to copy rows from materialized > >> views in [1]. > >> The attached patch is to add a tab complete for it. > >> > >> [1] https://git.postgresql.org/cgit/postgresql.git/commit/?id=534874fac0b34535c9a5ab9257d6574f78423578 > > > > Hi! > > Patch works good for me, but I noticed that psql COPY <tab> suggests > > partitioned relation both with and without this patch. Maybe that's > > not a big problem, if [0] will be pushed. > > Is the partitioned table currently tab-completed for the COPY FROM case? If I'm not mistaken, yes. I double checked. > INSTEAD OF INSERT triggers - though maybe that's overkill? That's wild to me, psql tab completions feature designed to support postgresql not fully, but in frequent cases. So maybe we should keep it stupud. -- Best regards, Kirill Reshke ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: tab complete for COPY populated materialized view TO @ 2025-04-10 15:07 Fujii Masao <[email protected]> parent: Kirill Reshke <[email protected]> 0 siblings, 1 reply; 15+ messages in thread From: Fujii Masao @ 2025-04-10 15:07 UTC (permalink / raw) To: Kirill Reshke <[email protected]>; +Cc: jian he <[email protected]>; pgsql-hackers On 2025/04/09 19:24, Kirill Reshke wrote: > On Wed, 9 Apr 2025 at 14:45, Fujii Masao <[email protected]> wrote: >> >> >> >> On 2025/04/09 18:25, Kirill Reshke wrote: >>> On Wed, 9 Apr 2025 at 13:23, jian he <[email protected]> wrote: >>>> >>>> hi. >>>> >>>> we allow the "COPY table TO" command to copy rows from materialized >>>> views in [1]. >>>> The attached patch is to add a tab complete for it. >>>> >>>> [1] https://git.postgresql.org/cgit/postgresql.git/commit/?id=534874fac0b34535c9a5ab9257d6574f78423578 >>> >>> Hi! >>> Patch works good for me, but I noticed that psql COPY <tab> suggests >>> partitioned relation both with and without this patch. Maybe that's >>> not a big problem, if [0] will be pushed. >> >> Is the partitioned table currently tab-completed for the COPY FROM case? > > If I'm not mistaken, yes. I double checked. > >> INSTEAD OF INSERT triggers - though maybe that's overkill? > > That's wild to me, psql tab completions feature designed to support > postgresql not fully, but in frequent cases. So maybe we should keep > it stupud. I agree that it's reasonable to exclude such rarely used objects from tab-completion. How about including just tables, partitioned tables, foreign tables, and materialized views? I've attached a patch for that. Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION From 7f14bf122427a30f62e98ad65d4856455578cfc3 Mon Sep 17 00:00:00 2001 From: Fujii Masao <[email protected]> Date: Wed, 9 Apr 2025 18:21:18 +0900 Subject: [PATCH v2] psql: Improve tab completion for COPY command. Previously, tab completion for COPY only suggested plain tables and partitioned tables, even though materialized views are also valid for COPY TO (since commit 534874fac0b), and foreign tables are valid for COPY FROM. This commit enhances tab completion for COPY to also include materialized views and foreign tables. Views with INSTEAD OF INSERT triggers are supported with COPY FROM but rarely used, so plain views are intentionally excluded from completion. --- src/bin/psql/tab-complete.in.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c index c916b9299a8..9846b086ef2 100644 --- a/src/bin/psql/tab-complete.in.c +++ b/src/bin/psql/tab-complete.in.c @@ -889,6 +889,14 @@ static const SchemaQuery Query_for_list_of_analyzables = { .result = "c.relname", }; +/* + * Relations supporting COPY TO/FROM are currently almost the same as + * those supporting ANALYZE. Although views with INSTEAD OF INSERT triggers + * can be used with COPY FROM, they are rarely used for this purpose, + * so plain views are intentionally excluded from this tab completion. + */ +#define Query_for_list_of_tables_for_copy Query_for_list_of_analyzables + /* Relations supporting index creation */ static const SchemaQuery Query_for_list_of_indexables = { .catname = "pg_catalog.pg_class c", @@ -3249,7 +3257,7 @@ match_previous_words(int pattern_id, * backslash command). */ else if (Matches("COPY|\\copy")) - COMPLETE_WITH_SCHEMA_QUERY_PLUS(Query_for_list_of_tables, "("); + COMPLETE_WITH_SCHEMA_QUERY_PLUS(Query_for_list_of_tables_for_copy, "("); /* Complete COPY ( with legal query commands */ else if (Matches("COPY|\\copy", "(")) COMPLETE_WITH("SELECT", "TABLE", "VALUES", "INSERT INTO", "UPDATE", "DELETE FROM", "MERGE INTO", "WITH"); -- 2.49.0 Attachments: [text/plain] v2-0001-psql-Improve-tab-completion-for-COPY-command.patch (2.0K, ../../[email protected]/2-v2-0001-psql-Improve-tab-completion-for-COPY-command.patch) download | inline diff: From 7f14bf122427a30f62e98ad65d4856455578cfc3 Mon Sep 17 00:00:00 2001 From: Fujii Masao <[email protected]> Date: Wed, 9 Apr 2025 18:21:18 +0900 Subject: [PATCH v2] psql: Improve tab completion for COPY command. Previously, tab completion for COPY only suggested plain tables and partitioned tables, even though materialized views are also valid for COPY TO (since commit 534874fac0b), and foreign tables are valid for COPY FROM. This commit enhances tab completion for COPY to also include materialized views and foreign tables. Views with INSTEAD OF INSERT triggers are supported with COPY FROM but rarely used, so plain views are intentionally excluded from completion. --- src/bin/psql/tab-complete.in.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c index c916b9299a8..9846b086ef2 100644 --- a/src/bin/psql/tab-complete.in.c +++ b/src/bin/psql/tab-complete.in.c @@ -889,6 +889,14 @@ static const SchemaQuery Query_for_list_of_analyzables = { .result = "c.relname", }; +/* + * Relations supporting COPY TO/FROM are currently almost the same as + * those supporting ANALYZE. Although views with INSTEAD OF INSERT triggers + * can be used with COPY FROM, they are rarely used for this purpose, + * so plain views are intentionally excluded from this tab completion. + */ +#define Query_for_list_of_tables_for_copy Query_for_list_of_analyzables + /* Relations supporting index creation */ static const SchemaQuery Query_for_list_of_indexables = { .catname = "pg_catalog.pg_class c", @@ -3249,7 +3257,7 @@ match_previous_words(int pattern_id, * backslash command). */ else if (Matches("COPY|\\copy")) - COMPLETE_WITH_SCHEMA_QUERY_PLUS(Query_for_list_of_tables, "("); + COMPLETE_WITH_SCHEMA_QUERY_PLUS(Query_for_list_of_tables_for_copy, "("); /* Complete COPY ( with legal query commands */ else if (Matches("COPY|\\copy", "(")) COMPLETE_WITH("SELECT", "TABLE", "VALUES", "INSERT INTO", "UPDATE", "DELETE FROM", "MERGE INTO", "WITH"); -- 2.49.0 ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: tab complete for COPY populated materialized view TO @ 2025-04-10 19:19 Kirill Reshke <[email protected]> parent: Fujii Masao <[email protected]> 0 siblings, 1 reply; 15+ messages in thread From: Kirill Reshke @ 2025-04-10 19:19 UTC (permalink / raw) To: Fujii Masao <[email protected]>; +Cc: jian he <[email protected]>; pgsql-hackers On Thu, 10 Apr 2025 at 20:07, Fujii Masao <[email protected]> wrote: > > > > On 2025/04/09 19:24, Kirill Reshke wrote: > > On Wed, 9 Apr 2025 at 14:45, Fujii Masao <[email protected]> wrote: > >> > >> > >> > >> On 2025/04/09 18:25, Kirill Reshke wrote: > >>> On Wed, 9 Apr 2025 at 13:23, jian he <[email protected]> wrote: > >>>> > >>>> hi. > >>>> > >>>> we allow the "COPY table TO" command to copy rows from materialized > >>>> views in [1]. > >>>> The attached patch is to add a tab complete for it. > >>>> > >>>> [1] https://git.postgresql.org/cgit/postgresql.git/commit/?id=534874fac0b34535c9a5ab9257d6574f78423578 > >>> > >>> Hi! > >>> Patch works good for me, but I noticed that psql COPY <tab> suggests > >>> partitioned relation both with and without this patch. Maybe that's > >>> not a big problem, if [0] will be pushed. > >> > >> Is the partitioned table currently tab-completed for the COPY FROM case? > > > > If I'm not mistaken, yes. I double checked. > > > >> INSTEAD OF INSERT triggers - though maybe that's overkill? > > > > That's wild to me, psql tab completions feature designed to support > > postgresql not fully, but in frequent cases. So maybe we should keep > > it stupud. > > I agree that it's reasonable to exclude such rarely used objects from > tab-completion. How about including just tables, partitioned tables, > foreign tables, and materialized views? > I've attached a patch for that. > > Regards, Patch is ok. However... > If we aim to support tab-completion for all valid targets of both COPY TO and COPY FROM, shouldn't foreign tables also be included? Ah.. Sorry I missed this part of your message initially. No, foreign tables are not supported: ``` reshke=# CREATE FOREIGN TABLE ft (i int) server zz OPTIONS ( filename 'zz.csv', format 'csv' ); reshke=# copy ft to stdout; ERROR: cannot copy from foreign table "ft" HINT: Try the COPY (SELECT ...) TO variant. ``` So we will tab complete for cases that yet to be supported (foreign tables and partitioned tables); What's funny is that copying foreign tables using MV works fine ``` reshke=# create materialized view mv as table ft; SELECT 1 reshke=# copy mv to stdout; 228 ``` -- Best regards, Kirill Reshke ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: tab complete for COPY populated materialized view TO @ 2025-04-10 19:33 David G. Johnston <[email protected]> parent: Kirill Reshke <[email protected]> 0 siblings, 1 reply; 15+ messages in thread From: David G. Johnston @ 2025-04-10 19:33 UTC (permalink / raw) To: Kirill Reshke <[email protected]>; +Cc: Fujii Masao <[email protected]>; jian he <[email protected]>; pgsql-hackers On Thursday, April 10, 2025, Kirill Reshke <[email protected]> wrote: > On Thu, 10 Apr 2025 at 20:07, Fujii Masao <[email protected]> > wrote: > > > > > > > > On 2025/04/09 19:24, Kirill Reshke wrote: > > > On Wed, 9 Apr 2025 at 14:45, Fujii Masao <[email protected]> > wrote: > > >> > > >> > > >> > > >> On 2025/04/09 18:25, Kirill Reshke wrote: > > >>> On Wed, 9 Apr 2025 at 13:23, jian he <[email protected]> > wrote: > > >>>> > > >>>> hi. > > >>>> > > >>>> we allow the "COPY table TO" command to copy rows from materialized > > >>>> views in [1]. > > >>>> The attached patch is to add a tab complete for it. > > >>>> > > >>>> [1] https://git.postgresql.org/cgit/postgresql.git/commit/?id= > 534874fac0b34535c9a5ab9257d6574f78423578 > > >>> > > >>> Hi! > > >>> Patch works good for me, but I noticed that psql COPY <tab> suggests > > >>> partitioned relation both with and without this patch. Maybe that's > > >>> not a big problem, if [0] will be pushed. > > >> > > >> Is the partitioned table currently tab-completed for the COPY FROM > case? > > > > > > If I'm not mistaken, yes. I double checked. > > > > > >> INSTEAD OF INSERT triggers - though maybe that's overkill? > > > > > > That's wild to me, psql tab completions feature designed to support > > > postgresql not fully, but in frequent cases. So maybe we should keep > > > it stupud. > > > > I agree that it's reasonable to exclude such rarely used objects from > > tab-completion. How about including just tables, partitioned tables, > > foreign tables, and materialized views? > > I've attached a patch for that. > > > > Regards, > > Patch is ok. However... I concur with the premise of the patch. Tab-complete is going to happen before we know whether to/from is specified so the syntax limits our smarts here. > > If we aim to support tab-completion for all valid targets of both COPY TO > and COPY FROM, shouldn't foreign tables also be included? > > Ah.. Sorry I missed this part of your message initially. No, foreign > tables are not supported: They are supported for the From variant; valid completions need only satisfy one of to/from, not both. > > What's funny is that copying foreign tables using MV works fine > > ``` > reshke=# create materialized view mv as table ft; > SELECT 1 > reshke=# copy mv to stdout; > 228 > ``` > I don’t get why this is “funny” or otherwise surprising. David J. ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: tab complete for COPY populated materialized view TO @ 2025-04-10 19:37 Kirill Reshke <[email protected]> parent: David G. Johnston <[email protected]> 0 siblings, 1 reply; 15+ messages in thread From: Kirill Reshke @ 2025-04-10 19:37 UTC (permalink / raw) To: David G. Johnston <[email protected]>; +Cc: Fujii Masao <[email protected]>; jian he <[email protected]>; pgsql-hackers On Fri, 11 Apr 2025 at 00:33, David G. Johnston <[email protected]> wrote: > > They are supported for the From variant; valid completions need only satisfy one of to/from, not both. > Thank you. If so, then WFM, and I don't have any more objections. -- Best regards, Kirill Reshke ^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: tab complete for COPY populated materialized view TO @ 2025-06-30 09:39 Fujii Masao <[email protected]> parent: Kirill Reshke <[email protected]> 0 siblings, 0 replies; 15+ messages in thread From: Fujii Masao @ 2025-06-30 09:39 UTC (permalink / raw) To: Kirill Reshke <[email protected]>; David G. Johnston <[email protected]>; +Cc: jian he <[email protected]>; pgsql-hackers On 2025/04/11 4:37, Kirill Reshke wrote: > On Fri, 11 Apr 2025 at 00:33, David G. Johnston > <[email protected]> wrote: >> >> They are supported for the From variant; valid completions need only satisfy one of to/from, not both. >> > > Thank you. If so, then WFM, and I don't have any more objections. I've pushed the patch. Thanks! Regards, -- Fujii Masao NTT DATA Japan Corporation ^ permalink raw reply [nested|flat] 15+ messages in thread
end of thread, other threads:[~2025-06-30 09:39 UTC | newest] Thread overview: 15+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2023-10-24 05:46 [PATCH v5 2/3] Improve pg_ctl postmaster process check on Windows Kyotaro Horiguchi <[email protected]> 2024-03-18 08:34 [PATCH v3] Add "Backpatch" regions in wait_event_names.txt Bertrand Drouvot <[email protected]> 2024-03-18 08:34 [PATCH v3] Add "Backpatch" regions in wait_event_names.txt Bertrand Drouvot <[email protected]> 2024-03-18 08:34 [PATCH v2] Add "Backpatch" regions in wait_event_names.txt Bertrand Drouvot <[email protected]> 2024-03-18 08:34 [PATCH v3] Add "Backpatch" regions in wait_event_names.txt Bertrand Drouvot <[email protected]> 2024-03-18 08:34 [PATCH v2] Add "Backpatch" regions in wait_event_names.txt Bertrand Drouvot <[email protected]> 2024-03-18 08:34 [PATCH v2] Add "Backpatch" regions in wait_event_names.txt Bertrand Drouvot <[email protected]> 2025-04-09 09:25 Re: tab complete for COPY populated materialized view TO Kirill Reshke <[email protected]> 2025-04-09 09:45 ` Re: tab complete for COPY populated materialized view TO Fujii Masao <[email protected]> 2025-04-09 10:24 ` Re: tab complete for COPY populated materialized view TO Kirill Reshke <[email protected]> 2025-04-10 15:07 ` Re: tab complete for COPY populated materialized view TO Fujii Masao <[email protected]> 2025-04-10 19:19 ` Re: tab complete for COPY populated materialized view TO Kirill Reshke <[email protected]> 2025-04-10 19:33 ` Re: tab complete for COPY populated materialized view TO David G. Johnston <[email protected]> 2025-04-10 19:37 ` Re: tab complete for COPY populated materialized view TO Kirill Reshke <[email protected]> 2025-06-30 09:39 ` Re: tab complete for COPY populated materialized view TO Fujii Masao <[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