Re: mprove tab completion for ALTER EXTENSION ADD/DROP
On Mon, 5 Dec 2022 at 06:53, Michael Paquier wrote:
>
> On Sat, Dec 03, 2022 at 05:34:57PM +, Matheus Alcantara wrote:
> > I've tested your patched on current master and seems to be working properly.
> >
> > I'm starting reviewing some patches here, let's see what more experience
> > hackers
> > has to say about this, but as far I can tell is that is working as expected.
>
> + /* ALTER EXTENSION ADD|DROP */
> + else if (Matches("ALTER", "EXTENSION", MatchAny, "ADD|DROP"))
> + COMPLETE_WITH("ACCESS METHOD", "AGGREGATE", "CAST",
> "COLLATION",
> + "CONVERSION", "DOMAIN", "EVENT
> TRIGGER", "FOREIGN",
> + "FUNCTION", "MATERIALIZED VIEW",
> "OPERATOR",
> + "PROCEDURAL LANGUAGE", "PROCEDURE",
> "LANGUAGE",
> + "ROUTINE", "SCHEMA", "SEQUENCE",
> "SERVER", "TABLE",
> + "TEXT SEARCH", "TRANSFORM FOR",
> "TYPE", "VIEW");
> +
> + /* ALTER EXTENSION ADD|DROP FOREIGN*/
> + else if (Matches("ALTER", "EXTENSION", MatchAny, "ADD|DROP",
> "FOREIGN"))
> + COMPLETE_WITH("DATA WRAPPER", "TABLE");
>
> The DROP could be matched with the objects that are actually part of
> the so-said extension?
The modified v2 version has the changes to handle the same. Sorry for
the delay as I was working on another project.
Regards,
Vignesh
From c2aeeadac88b709ce823a01c80b0b8455c62d3d9 Mon Sep 17 00:00:00 2001
From: Vignesh C
Date: Mon, 2 Jan 2023 08:01:45 +0530
Subject: [PATCH v2 2/2] Missing tab completion for ALTER EXTENSION DROP
Missing tab completion for ALTER EXTENSION DROP
---
src/bin/psql/tab-complete.c | 25 ++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 4d1a664bf7..a6f27ed9b8 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -224,6 +224,7 @@ static char *completion_ref_schema; /* schema name of reference object */
static bool completion_case_sensitive; /* completion is case sensitive */
static bool completion_verbatim; /* completion is verbatim */
static bool completion_force_quote; /* true to force-quote filenames */
+static bool completion_dont_quote; /* true to not quote the result */
/*
* A few macros to ease typing. You can use these to complete the given
@@ -1011,6 +1012,15 @@ static const SchemaQuery Query_for_trigger_of_table = {
"SELECT nspname FROM pg_catalog.pg_namespace "\
" WHERE nspname LIKE '%s'"
+#define Query_for_extension_objs \
+"SELECT distinct CASE WHEN p.type = 'foreign-data wrapper' "\
+" THEN 'FOREIGN DATA WRAPPER'"\
+" ELSE trim ('\"' from pg_catalog.upper(p.type::text)) END "\
+" FROM pg_depend, pg_identify_object(classid, objid, objsubid) AS p"\
+" WHERE refclassid = 'pg_extension'::regclass AND"\
+" pg_catalog.lower(p.type) LIKE pg_catalog.lower('%s') AND"\
+" refobjid = (SELECT oid FROM pg_extension WHERE extname = '%s')"
+
/* Use COMPLETE_WITH_QUERY_VERBATIM with these queries for GUC names: */
#define Query_for_list_of_alter_system_set_vars \
"SELECT pg_catalog.lower(name) FROM pg_catalog.pg_settings "\
@@ -1946,6 +1956,15 @@ psql_completion(const char *text, int start, int end)
"ROUTINE", "SCHEMA", "SEQUENCE", "SERVER", "TABLE",
"TEXT SEARCH", "TRANSFORM FOR", "TYPE", "VIEW");
+ /* ALTER EXTENSION DROP */
+ else if (Matches("ALTER", "EXTENSION", MatchAny, "DROP"))
+ {
+ set_completion_reference(prev2_wd);
+ completion_dont_quote = true;
+ COMPLETE_WITH_QUERY(Query_for_extension_objs);
+ completion_dont_quote = false;
+ }
+
/* ALTER EXTENSION ADD FOREIGN*/
else if (Matches("ALTER", "EXTENSION", MatchAny, "ADD", "FOREIGN"))
COMPLETE_WITH("DATA WRAPPER", "TABLE");
@@ -5313,7 +5332,7 @@ _complete_from_query(const char *simple_query,
* surprising. This restriction also dodges some odd behaviors of
* some versions of readline/libedit.
*/
- if (non_empty_object)
+ if (non_empty_object && !completion_dont_quote)
{
if (item && !objectquoted && identifier_needs_quotes(item))
continue;
@@ -5910,7 +5929,7 @@ requote_identifier(const char *schemaname, const char *objectname,
if (schemaname)
{
buflen += strlen(schemaname) + 1; /* +1 for the dot */
- if (!quote_schema)
+ if (!quote_schema && !completion_dont_quote)
quote_schema = identifier_needs_quotes(schemaname);
if (quote_schema)
{
@@ -5925,7 +5944,7 @@ requote_identifier(const char *schemaname, const char *objectname,
if (objectname)
{
buflen += strlen(objectname);
- if (!quote_object)
+ if (!quote_object && !completion_dont_quote)
quote_object = identifier_needs_quotes(objectname);
if (quote_object)
{
--
2.34.1
From 328509d60c1ec5489f5e29efbabc4c32466d1bac Mon Sep 17 00:00:00 2001
From: Vignesh C
Date: Sat, 26 Nov 2022
Re: Add index scan progress to pg_stat_progress_vacuum
>cfbot is complaining that this patch no longer applies. Sami, would you >mind rebasing it? Rebased patch attached. -- Sami Imseih Amazon Web Services: https://aws.amazon.com v18-0001-Add-2-new-columns-to-pg_stat_progress_vacuum.-Th.patch Description: v18-0001-Add-2-new-columns-to-pg_stat_progress_vacuum.-Th.patch
enable_timeout_every() and fin_time
Hi, I was looking using enable_timeout_every() in another place with Lukas just now, and noticed the fin_time argument. It seems odd for an interval firing interface to get an absolute timestamp as an argument. The only in-tree user of enable_timeout_every() computes fin_time explicitly using the interval time: startup_progress_phase_start_time = GetCurrentTimestamp(); fin_time = TimestampTzPlusMilliseconds(startup_progress_phase_start_time, log_startup_progress_interval); enable_timeout_every(STARTUP_PROGRESS_TIMEOUT, fin_time, log_startup_progress_interval); In https://postgr.es/m/CA%2BTgmoYqSF5sCNrgTom9r3Nh%3Dat4WmYFD%3DgsV-omStZ60S0ZUQ%40mail.gmail.com Robert said: > Apparently not, but here's a v2 anyway. In this version I made > enable_timeout_every() a three-argument function, so that the caller > can specify both the first time at which the timeout routine should be > called and the interval between them, instead of only the latter. That > seems to be more convenient for this use case, and is more powerful in > general. What is the use case for an absolute start time plus a relative interval? ISTM that this will just lead to every caller ending up with a calculation like the startup.c piece quoted above. Greetings, Andres Freund
Re: Announcing Release 15 of the PostgreSQL Buildfarm client
On 2022-12-31 Sa 21:11, Andrew Dunstan wrote: > On 2022-12-31 Sa 20:55, Noah Misch wrote: >> On Sat, Dec 31, 2022 at 10:02:32AM -0500, Andrew Dunstan wrote: >>> * check if a branch is up to date before trying to run it >>> This only applies if the |branches_to_build| setting is a keyword >>> rather than a list of branches. It reduces the number of useless >>> calls to |git pull| to almost zero. >> This new reliance on buildfarm.postgresql.org/branches_of_interest.json is >> trouble for non-SSL buildfarm animals. >> http://buildfarm.postgresql.org/branches_of_interest.txt has an exemption to >> allow serving over plain http, but the json URL just redirects the client to >> https. Can the json file get the same exemption-from-redirect that the txt >> file has? > > I didn't realize there were animals left other than mine which had this > issue. I asked the admins some weeks ago to fix this (I don't have > privilege to do so), but have not had a response yet. The temporary > workaround is to use a list of named branches, e.g. instead of 'ALL' use > [qw(REL_11_STABLE REL_12_STABLE REL_13_STABLE REL_14_STABLE > REL_15_STABLE HEAD)] > > Looks like this is fixed now (Thanks Magnus!), the workaround should no longer be necessary. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Re: Add a test to ldapbindpasswd
On 2023-01-01 Su 14:02, Thomas Munro wrote:
> On Mon, Jan 2, 2023 at 3:04 AM Andrew Dunstan wrote:
>> On 2022-12-19 Mo 11:16, Andrew Dunstan wrote:
>>> There is currently no test for the use of ldapbindpasswd in the
>>> pg_hba.conf file. This patch, mostly the work of John Naylor, remedies that.
>>>
>>>
>> This currently has failures on the cfbot for meson builds on FBSD13 and
>> Debian Bullseye, but it's not at all clear why. In both cases it fails
>> where the ldap server is started.
> I think it's failing when using meson. I guess it fails to fail on
> macOS only because you need to add a new path for Homebrew/ARM like
> commit 14d63dd2, so it's skipping (it'd be nice if we didn't need
> another copy of all that logic). Trying locally... it looks like
> slapd is failing silently, and with some tracing I can see it's
> sending an error message to my syslog daemon, which logged:
>
> 2023-01-02T07:50:20.853019+13:00 x1 slapd[153599]: main: TLS init def
> ctx failed: -1
>
> Ah, it looks like this test is relying on "slapd-certs", which doesn't exist:
>
> tmunro@x1:~/projects/postgresql/build$ ls testrun/ldap/001_auth/data/
> ldap.conf ldappassword openldap-data portlock slapd-certs slapd.conf
> tmunro@x1:~/projects/postgresql/build$ ls testrun/ldap/002_bindpasswd/data/
> portlock slapd.conf
>
> I didn't look closely, but apparently there is something wrong in the
> part that copies certs from the ssl test? Not sure why it works for
> autoconf...
Let's see how we fare with this patch.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
From dace23df29efb43aa5e4bddc99098203c0e5ed00 Mon Sep 17 00:00:00 2001
From: Andrew Dunstan
Date: Sun, 1 Jan 2023 18:27:30 -0500
Subject: [PATCH] Add a test for ldapbindpasswd
The existing LDAP tests don't cover the use of ldapbindpasswd in
pg_hba.conf, so remedy that.
Author: John Naylor
---
src/test/ldap/meson.build | 1 +
src/test/ldap/t/002_bindpasswd.pl | 207 ++
2 files changed, 208 insertions(+)
create mode 100644 src/test/ldap/t/002_bindpasswd.pl
diff --git a/src/test/ldap/meson.build b/src/test/ldap/meson.build
index 90d88138e7..7628a9c7c6 100644
--- a/src/test/ldap/meson.build
+++ b/src/test/ldap/meson.build
@@ -7,6 +7,7 @@ tests += {
'tap': {
'tests': [
't/001_auth.pl',
+ 't/002_bindpasswd.pl',
],
'env': {
'with_ldap': ldap.found() ? 'yes' : 'no',
diff --git a/src/test/ldap/t/002_bindpasswd.pl b/src/test/ldap/t/002_bindpasswd.pl
new file mode 100644
index 00..8296864209
--- /dev/null
+++ b/src/test/ldap/t/002_bindpasswd.pl
@@ -0,0 +1,207 @@
+
+# Copyright (c) 2022, PostgreSQL Global Development Group
+
+use strict;
+use warnings;
+use File::Copy;
+use PostgreSQL::Test::Utils;
+use PostgreSQL::Test::Cluster;
+use Test::More;
+
+
+my ($slapd, $ldap_bin_dir, $ldap_schema_dir);
+
+$ldap_bin_dir = undef;# usually in PATH
+
+if ($ENV{with_ldap} ne 'yes')
+{
+ plan skip_all => 'LDAP not supported by this build';
+}
+elsif ($ENV{PG_TEST_EXTRA} !~ /\bldap\b/)
+{
+ plan skip_all => 'Potentially unsafe test LDAP not enabled in PG_TEST_EXTRA';
+}
+elsif ($^O eq 'darwin' && -d '/usr/local/opt/openldap')
+{
+ # typical paths for Homebrew
+ $slapd = '/usr/local/opt/openldap/libexec/slapd';
+ $ldap_schema_dir = '/usr/local/etc/openldap/schema';
+}
+elsif ($^O eq 'darwin' && -d '/opt/homebrew/opt/openldap')
+{
+ # typical paths for Homebrew on ARM
+ $slapd = '/opt/homebrew/opt/openldap/libexec/slapd';
+ $ldap_schema_dir = '/opt/homebrew/etc/openldap/schema';
+}
+elsif ($^O eq 'darwin' && -d '/opt/local/etc/openldap')
+{
+ # typical paths for MacPorts
+ $slapd = '/opt/local/libexec/slapd';
+ $ldap_schema_dir = '/opt/local/etc/openldap/schema';
+}
+elsif ($^O eq 'linux')
+{
+ $slapd = '/usr/sbin/slapd';
+ $ldap_schema_dir = '/etc/ldap/schema' if -d '/etc/ldap/schema';
+ $ldap_schema_dir = '/etc/openldap/schema' if -d '/etc/openldap/schema';
+}
+elsif ($^O eq 'freebsd')
+{
+ $slapd = '/usr/local/libexec/slapd';
+ $ldap_schema_dir = '/usr/local/etc/openldap/schema';
+}
+elsif ($^O eq 'openbsd')
+{
+ $slapd = '/usr/local/libexec/slapd';
+ $ldap_schema_dir = '/usr/local/share/examples/openldap/schema';
+}
+else
+{
+ plan skip_all => "ldap tests not supported on $^O or dependencies not installed";
+}
+
+# make your own edits here
+#$slapd = '';
+#$ldap_bin_dir = '';
+#$ldap_schema_dir = '';
+
+$ENV{PATH} = "$ldap_bin_dir:$ENV{PATH}" if $ldap_bin_dir;
+
+my $ldap_datadir = "${PostgreSQL::Test::Utils::tmp_check}/openldap-data";
+my $slapd_certs = "${PostgreSQL::Test::Utils::tmp_check}/slapd-certs";
+my $slapd_conf= "${PostgreSQL::Test::Utils::tmp_check}/slapd.conf";
+my $slapd_pidfile = "${PostgreSQL::Test::Utils::tmp_check}/slapd.pid";
+my $slapd_logfile = "${PostgreSQL::Test::Utils::log_path}/slapd.log";
+my $ldap_conf = "${PostgreSQL::Test::Utils::tmp_check}/ldap.conf";
+my $ldap_server =
Re: postgres_fdw: using TABLESAMPLE to collect remote sample
On 12/31/22 18:23, Tom Lane wrote: > Tomas Vondra writes: >> On 12/31/22 05:42, Tom Lane wrote: >>> ERROR: TABLESAMPLE clause can only be applied to tables and materialized >>> views >>> I think the patch avoids that, but only accidentally, because >>> reltuples will be 0 or -1 for a view. Maybe it'd be a good >>> idea to pull back relkind along with reltuples, and check >>> that too? > >> Not sure. I guess we can rely on reltuples being 0 or -1 in such cases, >> but maybe it'd be good to at least mention that in a comment? We're not >> going to use other reltuples values for views etc. > > Would anyone ever point a foreign table at a sequence? I guess it > would be a weird use-case, but it's possible, or was till now. > Yeah, it's a weird use case. I can't quite imagine why would anyone do that, but I guess the mere possibility is sufficient reason to add the relkind check ... regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: Add a test to ldapbindpasswd
> On Jan 1, 2023, at 2:03 PM, Thomas Munro wrote: > > On Mon, Jan 2, 2023 at 3:04 AM Andrew Dunstan wrote: >>> On 2022-12-19 Mo 11:16, Andrew Dunstan wrote: >>> There is currently no test for the use of ldapbindpasswd in the >>> pg_hba.conf file. This patch, mostly the work of John Naylor, remedies that. >>> >>> >> >> This currently has failures on the cfbot for meson builds on FBSD13 and >> Debian Bullseye, but it's not at all clear why. In both cases it fails >> where the ldap server is started. > > I think it's failing when using meson. I guess it fails to fail on > macOS only because you need to add a new path for Homebrew/ARM like > commit 14d63dd2, so it's skipping (it'd be nice if we didn't need > another copy of all that logic). Trying locally... it looks like > slapd is failing silently, and with some tracing I can see it's > sending an error message to my syslog daemon, which logged: > > 2023-01-02T07:50:20.853019+13:00 x1 slapd[153599]: main: TLS init def > ctx failed: -1 > > Ah, it looks like this test is relying on "slapd-certs", which doesn't exist: > > tmunro@x1:~/projects/postgresql/build$ ls testrun/ldap/001_auth/data/ > ldap.conf ldappassword openldap-data portlock slapd-certs slapd.conf > tmunro@x1:~/projects/postgresql/build$ ls testrun/ldap/002_bindpasswd/data/ > portlock slapd.conf > > I didn't look closely, but apparently there is something wrong in the > part that copies certs from the ssl test? Not sure why it works for > autoconf... Thanks, I see the problem. Will post a revised patch shortly Cheers Andrew
Re: Is RecoveryConflictInterrupt() entirely safe in a signal handler?
On Sat, Dec 31, 2022 at 6:36 PM Noah Misch wrote: > On Sat, Dec 31, 2022 at 10:06:53AM +1300, Thomas Munro wrote: > > Idea #8 is a realisation that twisting oneself into a pretzel to avoid > > having to change the regexp code or its REG_CANCEL control flow may be > > a bit silly. If the only thing it really needs to do is free some > > memory, maybe the regexp module should provide a function that frees > > everything that is safe to call from our rcancelrequested callback, so > > we can do so before we longjmp back to Kansas. Then the REG_CANCEL > > code paths would be effectively unreachable in PostgreSQL. I don't > > know if it's better or worse than your idea #6, "use palloc instead, > > it already has garbage collection, duh", but it's a different take on > > the same realisation that this is just about free(). > > PG_TRY() isn't free, so it's nice that (6) doesn't add one. If (6) fails in > some not-yet-revealed way, (8) could get more relevant. > > > I guess idea #6 must be pretty easy to try: just point that MALLOC() > > macro to palloc(), and do a plain old CFI() in rcancelrequested(). It's not quite so easy: in RE_compile_and_cache we construct objects with arbitrary cache-managed lifetime, which suggests we need a cache memory context, but we could also fail mid construction, which suggests we'd need a dedicated per-regex object memory context that is made permanent with the MemoryContextSetParent() trick (as we see elsewhere for cached things that are constructed by code that might throw), or something like the try/catch thing from idea #8. Thinking...
Re: +infinity for dates and timestamps
On 1/1/23 20:21, Tom Lane wrote: Vik Fearing writes: Hmm. Somehow the .out test files were not included. Fixed with attached. Somehow you'd managed to duplicate some of the other changes, so the cfbot still didn't like that :-( Anyway, pushed with cosmetic changes. Notably, I left out the documentation changes after observing that we don't document "+infinity" separately for the numeric types. Given the lack of complaints about that I think it's fine to do the same here. Thanks, Tom! No objections to your changes. -- Vik Fearing
Re: +infinity for dates and timestamps
Vik Fearing writes: > Hmm. Somehow the .out test files were not included. > Fixed with attached. Somehow you'd managed to duplicate some of the other changes, so the cfbot still didn't like that :-( Anyway, pushed with cosmetic changes. Notably, I left out the documentation changes after observing that we don't document "+infinity" separately for the numeric types. Given the lack of complaints about that I think it's fine to do the same here. regards, tom lane
Re: Add a test to ldapbindpasswd
On Mon, Jan 2, 2023 at 3:04 AM Andrew Dunstan wrote: > On 2022-12-19 Mo 11:16, Andrew Dunstan wrote: > > There is currently no test for the use of ldapbindpasswd in the > > pg_hba.conf file. This patch, mostly the work of John Naylor, remedies that. > > > > > > This currently has failures on the cfbot for meson builds on FBSD13 and > Debian Bullseye, but it's not at all clear why. In both cases it fails > where the ldap server is started. I think it's failing when using meson. I guess it fails to fail on macOS only because you need to add a new path for Homebrew/ARM like commit 14d63dd2, so it's skipping (it'd be nice if we didn't need another copy of all that logic). Trying locally... it looks like slapd is failing silently, and with some tracing I can see it's sending an error message to my syslog daemon, which logged: 2023-01-02T07:50:20.853019+13:00 x1 slapd[153599]: main: TLS init def ctx failed: -1 Ah, it looks like this test is relying on "slapd-certs", which doesn't exist: tmunro@x1:~/projects/postgresql/build$ ls testrun/ldap/001_auth/data/ ldap.conf ldappassword openldap-data portlock slapd-certs slapd.conf tmunro@x1:~/projects/postgresql/build$ ls testrun/ldap/002_bindpasswd/data/ portlock slapd.conf I didn't look closely, but apparently there is something wrong in the part that copies certs from the ssl test? Not sure why it works for autoconf...
Re: Add a test to ldapbindpasswd
On 2022-12-19 Mo 11:16, Andrew Dunstan wrote: > There is currently no test for the use of ldapbindpasswd in the > pg_hba.conf file. This patch, mostly the work of John Naylor, remedies that. > > This currently has failures on the cfbot for meson builds on FBSD13 and Debian Bullseye, but it's not at all clear why. In both cases it fails where the ldap server is started. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
