Re: Support logical replication of DDLs

2023-01-02 Thread vignesh C
On Wed, 7 Dec 2022 at 17:50, Alvaro Herrera  wrote:
>
> I think this patch is split badly.
>
> You have:
>
> 0001 an enormous patch including some required infrastructure, plus the
> DDL deparsing bits themselves.
>
> 0002 another enormous (though not as much) patch, this time for
> DDL replication using the above.
>
> 0003 a bugfix for 0001, which includes changes in both the
> infrastructure and the deparsing bits.
>
> 0004 test stuff for 0002.
>
> 0005 Another bugfix for 0001
>
> 0006 Another bugfix for 0001
>
> As presented, I think it has very little chance of being reviewed
> usefully.  A better way to go about this, I think, would be:
>
> 0001 - infrastructure bits to support the DDL deparsing parts (all these
> new functions in ruleutils.c, sequence.c, etc).  That means, everything
> (?) that's currently in your 0001 except ddl_deparse.c and friends.
> Clearly there are several independent changes here; maybe it is possible
> to break it down even further.  This patch or these patches should also
> include the parts of 0003, 0005, 0006 that require changes outside of
> ddl_deparse.c.
> I expect that this patch should be fairly small.

Modified

> 0002 - ddl_deparse.c and its very close friends.  This should not have
> any impact on places such as ruleutils.c, sequence.c, etc.  The parts of
> the bugfixes (0001, 0005, 0006) that touch this could should be merged
> here as well; there's no reason to have them as separate patches.  Some
> test code should be here also, though it probably doesn't need to aim to
> be complete.
> This one is likely to be very large, but also self-contained.

Modified, I have currently kept the testing of deparse as a separate
patch, we are planning to add more tests to it. We can later merge it
to 0002 if required or keep it as 0003 if it is too large.

> 0003 - ddlmessage.c and friends.  I understand that DDL-messaging is
> supporting infrastructure for DDL replication; I think it should be its
> own patch.  Probably include its own simple-ish test bits.
> Not a very large patch.

Modified

> 0004 - DDL replication proper, including 0004.
> Probably not a very large patch either, not sure.

 Modified

> Some review comments, just skimming:
> - 0002 adds some functions to event_trigger.c, but that doesn't seem to
> be their place.  Maybe some new file in src/backend/replication/logical
> would make more sense.

Modified

> - publication_deparse_ddl_command_end has a long strcmp() list; why?
> Maybe change things so that it compares some object type enum instead.

Modified wherever possible

> - CreatePublication has a long list of command tags; is that good?
> Maybe it'd be better to annotate the list in cmdtaglist.h somehow.



> - The change in pg_dump's getPublications needs updated to 16.
Modified

> - Don't "git add" src/bin/pg_waldump/logicalddlmsgdesc.c, just update
> its Makefile and meson.build

Modified

>
> - I think psql's \dRp should not have the new column at the end.
> Maybe one of:
> + Name | Owner | DDL | All tables | Inserts | Updates | Deletes | Truncates | 
> Via root
> + Name | Owner | All tables | DDL | Inserts | Updates | Deletes | Truncates | 
> Via root
> + Name | Owner | All tables | Inserts | Updates | Deletes | Truncates | DDL | 
> Via root
> (I would not add the "s" at the end of that column title, also).

Modified it to:
Name |  Owner  | All tables | DDL | Inserts | Updates | Deletes |
Truncates | Via root

These issues were addressed as part of v55 at [1], v56 at [2]  and v58
at [3] posted.

[1] - 
https://www.postgresql.org/message-id/CALDaNm2V6YL6H4P9ZT95Ua_RDJaeDTUf6V0UDfrz4_vxhM5pMg%40mail.gmail.com
[2] - 
https://www.postgresql.org/message-id/CALDaNm0uXh%3DRgGD%3DoB1p83GONb5%3DL2n3nbpiLGVaMd57TimdZA%40mail.gmail.com
[3] - 
https://www.postgresql.org/message-id/CAAD30ULrZB-RNmuD3NMr1jGNUt15ZpPgFdFRX53HbcAB76hefw%40mail.gmail.com

Regards,
Vignesh




Re: [DOCS] Stats views and functions not in order?

2023-01-02 Thread Peter Eisentraut

On 08.12.22 03:30, Peter Smith wrote:

PSA patches for v9*

v9-0001 - Now the table rows are ordered per PeterE's suggestions [1]


committed


v9-0002 - All the review comments from DavidJ [2] are addressed


I'm not sure about this one.  It removes the "see [link] for details" 
phrases and instead makes the view name a link.  I think this loses the 
cue that there is more information elsewhere.  Otherwise, one could 
think that, say, the entry about pg_stat_activity is the primary source 
and the link just links to itself.  Also keep in mind that people use 
media where links are not that apparent (PDF), so the presence of a link 
by itself cannot be the only cue about the flow of the information.






Re: Allow placeholders in ALTER ROLE w/o superuser

2023-01-02 Thread Pavel Borisov
Hi, Justin!

On Wed, 28 Dec 2022 at 21:28, Justin Pryzby  wrote:
>
> On Tue, Dec 27, 2022 at 11:29:40PM -0500, Tom Lane wrote:
> > Justin Pryzby  writes:
> > > This fails when run more than once:
> > > time meson test --setup running --print 
> > > test_pg_db_role_setting-running/regress
> >
> > Ah.
> >
> > > It didn't fail for you because it says:
> > > ./src/test/modules/test_pg_db_role_setting/Makefile
> > > +# disable installcheck for now
> > > +NO_INSTALLCHECK = 1
> >
> > So ... exactly why is the meson infrastructure failing to honor that?
> > This test looks sufficiently careless about its side-effects that
> > I completely agree with the decision to not run it in pre-existing
> > installations.  Failing to drop a created superuser is just one of
> > its risk factors --- it also leaves around pg_db_role_setting entries.
>
> Meson doesn't try to parse the Makefiles (like the MSVC scripts) but
> (since 3f0e786ccb) has its own implementation, which involves setting
> 'runningcheck': false.
>
> 096dd80f3c seems to have copied the NO_INSTALLCHECK from oat's makefile,
> but didn't copy "runningcheck" from oat's meson.build (but did copy its
> regress_args).

I completely agree with your analysis. Fixes by 3f0e786ccbf5 to oat
and the other modules tests came just a couple of days before
committing the main pg_db_role_setting commit 096dd80f3c so they were
forgotten to be included into it.

I support committing the same fix to pg_db_role_setting test and added
this minor fix as a patch to this thread.

Kind regards, and happy New year!
Pavel Borisov,
Supabase.


0001-meson-Add-running-test-setup-as-a-replacement-for-in.patch
Description: Binary data


Re: heapgettup refactoring

2023-01-02 Thread Peter Eisentraut

On 30.11.22 23:34, Melanie Plageman wrote:

I have attached a patchset with only the code changes contained in the
previous patch 0003. I have broken the refactoring down into many
smaller pieces for ease of review.


To keep this moving along a bit, I have committed your 0002, which I 
think is a nice little improvement on its own.






Sampling-based timing for EXPLAIN ANALYZE

2023-01-02 Thread Lukas Fittl
Hi,

Since EXPLAIN ANALYZE with TIMING ON still carries noticeable overhead on
modern hardware (despite time sources being faster), I'd like to propose a
new setting EXPLAIN ANALYZE, called "TIMING SAMPLING", as compared to
TIMING ON.

This new timing mode uses a timer on a fixed recurring frequency (e.g. 100
or 1000 Hz) to gather a sampled timestamp on a predefined schedule, instead
of getting the time on-demand when InstrStartNode/InstrStopNode is called.
To implement the timer, we can use the existing timeout infrastructure,
which is backed by a wall clock timer (ITIMER_REAL).

Conceptually this is inspired by how sampling profilers work (e.g. "perf"),
but it ties into the existing per-plan node instrumentation done by EXPLAIN
ANALYZE, and simply provides a lower accuracy version of the total time for
each plan node.

In EXPLAIN output this is marked as "sampled time", and scaled to the total
wall clock time (to adjust for the sampling undercounting):

=# EXPLAIN (ANALYZE, BUFFERS, TIMING SAMPLING, SAMPLEFREQ 100) SELECT ...;
 QUERY PLAN

-
 HashAggregate  (cost=201747.90..201748.00 rows=10 width=12) (actual
sampled time=5490.974 rows=9 loops=1)
   ...
   ->  Hash Join  (cost=0.23..199247.90 rows=49 width=4) (actual
sampled time=3738.619 rows=900 loops=1)
 ...
 ->  Seq Scan on large  (cost=0.00..144247.79 rows=979 width=4)
(actual sampled time=1004.671 rows=1001 loops=1)
   ...
 ->  Hash  (cost=0.10..0.10 rows=10 width=4) (actual sampled
time=0.000 rows=10 loops=1)
   ...
 Execution Time: 5491.475 ms
---

In simple query tests like this on my local machine, this shows a
consistent benefit over TIMING ON (and behaves close to ANALYZE with TIMING
OFF), whilst providing a "good enough" accuracy to identify which part of
the query was problematic.

Attached is a prototype patch for early feedback on the concept, with tests
and documentation to come in a follow-up. Since the January commitfest is
still marked as open I'll register it there, but note that my assumption is
this is *not* Postgres 16 material.

As an open item, note that in the patch the requested sampling frequency is
not yet passed to parallel workers (it always defaults to 1000 Hz when
sampling is enabled). Also, note the timing frequency is limited to a
maximum of 1000 Hz (1ms) due to current limitations of the timeout
infrastructure.

With thanks to Andres Freund for help on refining the idea, collaborating
on early code and finding the approach to hook into the timeout API.

Thanks,
Lukas

-- 
Lukas Fittl


v1-0001-Add-TIMING-SAMPLING-option-for-EXPLAIN-ANALYZE.patch
Description: Binary data


Re: Add BufFileRead variants with short read and EOF detection

2023-01-02 Thread Amit Kapila
On Wed, Dec 28, 2022 at 4:17 PM Peter Eisentraut
 wrote:
>
> Most callers of BufFileRead() want to check whether they read the full
> specified length.  Checking this at every call site is very tedious.
> This patch provides additional variants BufFileReadExact() and
> BufFileReadMaybeEOF() that include the length checks.
>
> I considered changing BufFileRead() itself, but this function is also
> used in extensions, and so changing the behavior like this would create
> a lot of problems there.  The new names are analogous to the existing
> LogicalTapeReadExact().
>

+1 for the new APIs. I have noticed that some of the existing places
use %m and the file path in messages which are not used in the new
common function.

-- 
With Regards,
Amit Kapila.




Bug in check for unreachable MERGE WHEN clauses

2023-01-02 Thread Dean Rasheed
Re-reading my latest MERGE patch, I realised there is a trivial,
pre-existing bug in the check for unreachable WHEN clauses, which
means it won't spot an unreachable WHEN clause if it doesn't have an
AND condition.

So the checks need to be re-ordered, as in the attached.

Regards,
Dean
diff --git a/src/backend/parser/parse_merge.c b/src/backend/parser/parse_merge.c
new file mode 100644
index 3844f2b..47d266b
--- a/src/backend/parser/parse_merge.c
+++ b/src/backend/parser/parse_merge.c
@@ -155,12 +155,12 @@ transformMergeStmt(ParseState *pstate, M
 		/*
 		 * Check for unreachable WHEN clauses
 		 */
-		if (mergeWhenClause->condition == NULL)
-			is_terminal[when_type] = true;
-		else if (is_terminal[when_type])
+		if (is_terminal[when_type])
 			ereport(ERROR,
 	(errcode(ERRCODE_SYNTAX_ERROR),
 	 errmsg("unreachable WHEN clause specified after unconditional WHEN clause")));
+		if (mergeWhenClause->condition == NULL)
+			is_terminal[when_type] = true;
 	}
 
 	/*
diff --git a/src/test/regress/expected/merge.out b/src/test/regress/expected/merge.out
new file mode 100644
index 6c8a18f..bc53b21
--- a/src/test/regress/expected/merge.out
+++ b/src/test/regress/expected/merge.out
@@ -659,7 +659,7 @@ USING source AS s
 ON t.tid = s.sid
 WHEN MATCHED THEN /* Terminal WHEN clause for MATCHED */
 	DELETE
-WHEN MATCHED AND s.delta > 0 THEN
+WHEN MATCHED THEN
 	UPDATE SET balance = t.balance - s.delta;
 ERROR:  unreachable WHEN clause specified after unconditional WHEN clause
 ROLLBACK;
diff --git a/src/test/regress/sql/merge.sql b/src/test/regress/sql/merge.sql
new file mode 100644
index 98fe104..fdbcd70
--- a/src/test/regress/sql/merge.sql
+++ b/src/test/regress/sql/merge.sql
@@ -438,7 +438,7 @@ USING source AS s
 ON t.tid = s.sid
 WHEN MATCHED THEN /* Terminal WHEN clause for MATCHED */
 	DELETE
-WHEN MATCHED AND s.delta > 0 THEN
+WHEN MATCHED THEN
 	UPDATE SET balance = t.balance - s.delta;
 ROLLBACK;
 


Re: Data loss on logical replication, 12.12 to 14.5, ALTER SUBSCRIPTION

2023-01-02 Thread Amit Kapila
On Wed, Dec 28, 2022 at 4:52 PM Michail Nikolaev
 wrote:
>
> Hello.
>
> > None of these entries are from the point mentioned by you [1]
> > yesterday where you didn't find the corresponding data in the
> > subscriber. How did you identify that the entries corresponding to
> > that timing were missing?
>
> Some of the before the interval, some after... But the source database
> was generating a lot of WAL during logical replication
> - some of these log entries from time AFTER completion of initial sync
> of B but (probably) BEFORE finishing B table catch up (entering
> streaming mode).
>
...
...
>
> So, shortly the story looks like:
>
> * initial sync of A (and other tables) started and completed, they are
> in streaming mode
> * B and C initial sync started (by altering PUBLICATION and SUBSCRIPTION)
> * B sync completed, but new changes are still applying to the tables
> to catch up primary
>

The point which is not completely clear from your description is the
timing of missing records. In one of your previous emails, you seem to
have indicated that the data missed from Table B is from the time when
the initial sync for Table B was in-progress, right? Also, from your
description, it seems there is no error or restart that happened
during the time of initial sync for Table B. Is that understanding
correct?

> * logical replication apply worker is restarted because IO error on WAL 
> receive
> * Postgres killed
> * Postgres restarted
> * C initial sync restarted
> * logical replication apply worker few times restarted because IO
> error on WAL receive
> * finally every table in streaming mode but with small gap in B
>

I am not able to see how these steps can lead to the problem. If the
problem is reproducible at your end, you might want to increase LOG
verbosity to DEBUG1 and see if there is additional information in the
LOGs that can help or it would be really good if there is a
self-sufficient test to reproduce it.

-- 
With Regards,
Amit Kapila.




Re: Reduce timing overhead of EXPLAIN ANALYZE using rdtsc?

2023-01-02 Thread David Geier

Hi,

I re-based again on master and applied the following changes:

I removed the fallback for obtaining the TSC frequency from /proc/cpu as 
suggested by Andres. Worst-case we fall back to clock_gettime().


I added code to obtain the TSC frequency via CPUID when under a 
hypervisor. I had to use __cpuid() directly instead of __get_cpuid(), 
because __get_cpuid() returns an error if the leaf is > 0x8000 
(probably the implementation pre-dates the hypervisor timing leafs). 
Unfortunately, while testing my implementation under VMWare, I found 
that RDTSC runs awfully slow there (like 30x slower). [1] indicates that 
we cannot generally rely on RDTSC being actually fast on VMs. However, 
the same applies to clock_gettime(). It runs as slow as RDTSC on my 
VMWare setup. Hence, using RDTSC is not at disadvantage. I'm not 
entirely sure if there aren't cases where e.g. clock_gettime() is 
actually faster than RDTSC and it would be advantageous to use 
clock_gettime(). We could add a GUC so that the user can decide which 
clock source to use. Any thoughts?


I also somewhat improved the accuracy of the cycles to milli- and 
microseconds conversion functions by having two more multipliers with 
higher precision. For microseconds we could also keep the computation 
integer-only. I'm wondering what to best do for seconds and 
milliseconds. I'm currently leaning towards just keeping it as is, 
because the durations measured and converted are usually long enough 
that precision shouldn't be a problem.


In vacuum_lazy.c we do if ((INSTR_TIME_GET_MICROSEC(elapsed) / 1000). I 
changed that to use INSTR_TIME_GET_MILLISECS() instead. Additionally, I 
initialized a few variables of type instr_time which otherwise resulted 
in warnings due to use of potentially uninitialized variables.


I also couldn't reproduce the reported timing discrepancy. For me the 
runtime reported by \timing is just slightly higher than the time 
reported by EXPLAIN ANALYZE, which is expected.


Beyond that:

What about renaming INSTR_TIME_GET_DOUBLE() to INSTR_TIME_GET_SECS() so 
that it's consistent with the _MILLISEC() and _MICROSEC() variants?


The INSTR_TIME_GET_MICROSEC() returns a uint64 while the other variants 
return double. This seems error prone. What about renaming the function 
or also have the function return a double and cast where necessary at 
the call site?


If no one objects I would also re-register this patch in the commit fest.

[1] 
https://vmware.com/content/dam/digitalmarketing/vmware/en/pdf/techpaper/Timekeeping-In-VirtualMachines.pdf 
(page 11 "Virtual TSC")


--
David Geier
(ServiceNow)
From 321d00ae5dd1bcffc8fbdb39879b7f5c78e3930f Mon Sep 17 00:00:00 2001
From: David Geier 
Date: Thu, 17 Nov 2022 10:22:01 +0100
Subject: [PATCH 1/3] Change instr_time to just store nanoseconds, that's
 cheaper.

---
 src/include/portability/instr_time.h | 62 
 1 file changed, 26 insertions(+), 36 deletions(-)

diff --git a/src/include/portability/instr_time.h b/src/include/portability/instr_time.h
index 22bcf3d288..4bd555113b 100644
--- a/src/include/portability/instr_time.h
+++ b/src/include/portability/instr_time.h
@@ -80,63 +80,53 @@
 #define PG_INSTR_CLOCK	CLOCK_REALTIME
 #endif
 
-typedef struct timespec instr_time;
+typedef int64 instr_time;
+#define NS_PER_S INT64CONST(10)
+#define US_PER_S INT64CONST(100)
+#define MS_PER_S INT64CONST(1000)
 
-#define INSTR_TIME_IS_ZERO(t)	((t).tv_nsec == 0 && (t).tv_sec == 0)
+#define NS_PER_MS INT64CONST(100)
+#define NS_PER_US INT64CONST(1000)
 
-#define INSTR_TIME_SET_ZERO(t)	((t).tv_sec = 0, (t).tv_nsec = 0)
+#define INSTR_TIME_IS_ZERO(t)	((t) == 0)
 
-#define INSTR_TIME_SET_CURRENT(t)	((void) clock_gettime(PG_INSTR_CLOCK, &(t)))
+#define INSTR_TIME_SET_ZERO(t)	((t) = 0)
+
+static inline instr_time pg_clock_gettime_ns(void)
+{
+	struct timespec tmp;
+
+	clock_gettime(PG_INSTR_CLOCK, &tmp);
+
+	return tmp.tv_sec * NS_PER_S + tmp.tv_nsec;
+}
+
+#define INSTR_TIME_SET_CURRENT(t) \
+	(t) = pg_clock_gettime_ns()
 
 #define INSTR_TIME_ADD(x,y) \
 	do { \
-		(x).tv_sec += (y).tv_sec; \
-		(x).tv_nsec += (y).tv_nsec; \
-		/* Normalize */ \
-		while ((x).tv_nsec >= 10) \
-		{ \
-			(x).tv_nsec -= 10; \
-			(x).tv_sec++; \
-		} \
+		(x) += (y); \
 	} while (0)
 
 #define INSTR_TIME_SUBTRACT(x,y) \
 	do { \
-		(x).tv_sec -= (y).tv_sec; \
-		(x).tv_nsec -= (y).tv_nsec; \
-		/* Normalize */ \
-		while ((x).tv_nsec < 0) \
-		{ \
-			(x).tv_nsec += 10; \
-			(x).tv_sec--; \
-		} \
+		(x) -= (y); \
 	} while (0)
 
 #define INSTR_TIME_ACCUM_DIFF(x,y,z) \
 	do { \
-		(x).tv_sec += (y).tv_sec - (z).tv_sec; \
-		(x).tv_nsec += (y).tv_nsec - (z).tv_nsec; \
-		/* Normalize after each add to avoid overflow/underflow of tv_nsec */ \
-		while ((x).tv_nsec < 0) \
-		{ \
-			(x).tv_nsec += 10; \
-			(x).tv_sec--; \
-		} \
-		while ((x).tv_nsec >= 10) \
-		{ \
-			(x).tv_nsec -= 10; \
-			(x).tv_sec++; \
-		} \
+		(x) += 

Re: Add a test to ldapbindpasswd

2023-01-02 Thread Andrew Dunstan

On 2023-01-01 Su 18:31, Andrew Dunstan wrote:
> 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.
>
>

Not so well :-(. This version tries to make the tests totally
independent, as they should be. That's an attempt to get the cfbot to go
green, but I am intending to refactor this code substantially so the
common bits are in a module each test file will load.


cheers


andrew



--
Andrew Dunstan
EDB: https://www.enterprisedb.com
From c2bedfd8a5b326ffb563da49b7b4b4006ddac361 Mon Sep 17 00:00:00 2001
From: Andrew Dunstan 
Date: Mon, 2 Jan 2023 09:41:06 -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.

Authors: John Naylor and Andrew Dunstan
---
 src/test/ldap/meson.build |   1 +
 src/test/ldap/t/002_bindpasswd.pl | 223 ++
 2 files changed, 224 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..330a2b7dad
--- /dev/null
+++ b/src/test/ldap/t/002_bindpasswd.pl
@@ -0,0 +1,223 @@
+
+# Copyright (c) 2022, PostgreSQL Global Development Group
+
+use strict;
+use warnings;
+use File::Copy;
+use File::Basename;
+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 '/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 '/usr/local/opt/openldap')
+{
+	# typical paths for Homebrew on Intel
+	$slapd   = '/usr/local/opt/openldap/libexec/slapd';
+	$ldap_schema_dir = '/usr/local/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 $tst_name = basename(__FILE__,'.pl');
+my $test_temp = Po

Re: Allow placeholders in ALTER ROLE w/o superuser

2023-01-02 Thread Alexander Korotkov
Justin, Tom, Pavel, thank you for catching this.

On Mon, Jan 2, 2023 at 11:54 AM Pavel Borisov  wrote:
> I completely agree with your analysis. Fixes by 3f0e786ccbf5 to oat
> and the other modules tests came just a couple of days before
> committing the main pg_db_role_setting commit 096dd80f3c so they were
> forgotten to be included into it.
>
> I support committing the same fix to pg_db_role_setting test and added
> this minor fix as a patch to this thread.

I'm going to push this if no objections.

> Kind regards, and happy New year!

Thanks, and happy New Year to you!

--
Regards,
Alexander Korotkov




verbose mode for pg_input_error_message?

2023-01-02 Thread Andrew Dunstan
I've been wondering if it might be a good idea to have a third parameter
for pg_input_error_message() which would default to false, but which if
true would cause it to emit the detail and hint fields, if any,  as well
as the message field from the error_data.

Thoughts?


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com





Re: Allow placeholders in ALTER ROLE w/o superuser

2023-01-02 Thread Justin Pryzby
On Mon, Jan 02, 2023 at 06:14:48PM +0300, Alexander Korotkov wrote:
> I'm going to push this if no objections.

I also suggest that meson.build should not copy regress_args.

-- 
Justin




Re: verbose mode for pg_input_error_message?

2023-01-02 Thread Tom Lane
Andrew Dunstan  writes:
> I've been wondering if it might be a good idea to have a third parameter
> for pg_input_error_message() which would default to false, but which if
> true would cause it to emit the detail and hint fields, if any, as well
> as the message field from the error_data.

I don't think that just concatenating those strings would make for a
pleasant API.  More sensible, perhaps, to have a separate function
that returns a record.  Or we could redefine the existing function
that way, but I suspect that "just the primary error" will be a
principal use-case.

Being able to get the SQLSTATE is likely to be interesting too.

regards, tom lane




Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions

2023-01-02 Thread Tom Lane
Corey Huinker  writes:
> The proposed changes are as follows:
> CAST(expr AS typename)
> continues to behave as before.
> CAST(expr AS typename ERROR ON ERROR)
> has the identical behavior as the unadorned CAST() above.
> CAST(expr AS typename NULL ON ERROR)
> will use error-safe functions to do the cast of expr, and will return
> NULL if the cast fails.
> CAST(expr AS typename DEFAULT expr2 ON ERROR)
> will use error-safe functions to do the cast of expr, and will return
> expr2 if the cast fails.

While I approve of trying to get some functionality in this area,
I'm not sure that extending CAST is a great idea, because I'm afraid
that the SQL committee will do something that conflicts with it.
If we know that they are about to standardize exactly this syntax,
where is that information available?  If we don't know that,
I'd prefer to invent some kind of function or other instead of
extending the grammar.

regards, tom lane




Re: [RFC] Add jit deform_counter

2023-01-02 Thread Dmitry Dolgov
> On Sun, Dec 25, 2022 at 06:55:02PM +0100, Pavel Stehule wrote:
> there are some problems with stability of regress tests
>
> http://cfbot.cputube.org/dmitry-dolgov.html

Looks like this small change predates moving to meson, the attached
version should help.
>From 1b19af29b4a71e008d9d4ac7ca39d06dc89d6237 Mon Sep 17 00:00:00 2001
From: Dmitrii Dolgov <9erthali...@gmail.com>
Date: Sat, 11 Jun 2022 11:54:40 +0200
Subject: [PATCH v2 1/2] Add deform_counter

At the moment generation_counter includes time spent on both JITing
expressions and tuple deforming. Those are configured independently via
corresponding options (jit_expressions and jit_tuple_deforming), which
means there is no way to find out what fraction of time tuple deforming
alone is taking.

Add deform_counter dedicated to tuple deforming, which allows seeing
more directly the influence jit_tuple_deforming is having on the query.
---
 src/backend/commands/explain.c  | 7 ++-
 src/backend/jit/jit.c   | 1 +
 src/backend/jit/llvm/llvmjit_expr.c | 6 ++
 src/include/jit/jit.h   | 3 +++
 4 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index f86983c660..2f23602a87 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -882,6 +882,7 @@ ExplainPrintJIT(ExplainState *es, int jit_flags, 
JitInstrumentation *ji)
 
/* calculate total time */
INSTR_TIME_SET_ZERO(total_time);
+   /* don't add deform_counter, it's included into generation_counter */
INSTR_TIME_ADD(total_time, ji->generation_counter);
INSTR_TIME_ADD(total_time, ji->inlining_counter);
INSTR_TIME_ADD(total_time, ji->optimization_counter);
@@ -909,8 +910,9 @@ ExplainPrintJIT(ExplainState *es, int jit_flags, 
JitInstrumentation *ji)
{
ExplainIndentText(es);
appendStringInfo(es->str,
-"Timing: %s %.3f ms, 
%s %.3f ms, %s %.3f ms, %s %.3f ms, %s %.3f ms\n",
+"Timing: %s %.3f ms, 
%s %.3f ms, %s %.3f ms, %s %.3f ms, %s %.3f ms, %s %.3f ms\n",
 "Generation", 1000.0 * 
INSTR_TIME_GET_DOUBLE(ji->generation_counter),
+"Deforming", 1000.0 * 
INSTR_TIME_GET_DOUBLE(ji->deform_counter),
 "Inlining", 1000.0 * 
INSTR_TIME_GET_DOUBLE(ji->inlining_counter),
 "Optimization", 1000.0 
* INSTR_TIME_GET_DOUBLE(ji->optimization_counter),
 "Emission", 1000.0 * 
INSTR_TIME_GET_DOUBLE(ji->emission_counter),
@@ -937,6 +939,9 @@ ExplainPrintJIT(ExplainState *es, int jit_flags, 
JitInstrumentation *ji)
ExplainPropertyFloat("Generation", "ms",
 1000.0 * 
INSTR_TIME_GET_DOUBLE(ji->generation_counter),
 3, es);
+   ExplainPropertyFloat("Deforming", "ms",
+1000.0 * 
INSTR_TIME_GET_DOUBLE(ji->deform_counter),
+3, es);
ExplainPropertyFloat("Inlining", "ms",
 1000.0 * 
INSTR_TIME_GET_DOUBLE(ji->inlining_counter),
 3, es);
diff --git a/src/backend/jit/jit.c b/src/backend/jit/jit.c
index 91a6b2b63a..a6bdf03718 100644
--- a/src/backend/jit/jit.c
+++ b/src/backend/jit/jit.c
@@ -185,6 +185,7 @@ InstrJitAgg(JitInstrumentation *dst, JitInstrumentation 
*add)
 {
dst->created_functions += add->created_functions;
INSTR_TIME_ADD(dst->generation_counter, add->generation_counter);
+   INSTR_TIME_ADD(dst->deform_counter, add->deform_counter);
INSTR_TIME_ADD(dst->inlining_counter, add->inlining_counter);
INSTR_TIME_ADD(dst->optimization_counter, add->optimization_counter);
INSTR_TIME_ADD(dst->emission_counter, add->emission_counter);
diff --git a/src/backend/jit/llvm/llvmjit_expr.c 
b/src/backend/jit/llvm/llvmjit_expr.c
index f114337f8e..1ad384f15e 100644
--- a/src/backend/jit/llvm/llvmjit_expr.c
+++ b/src/backend/jit/llvm/llvmjit_expr.c
@@ -121,7 +121,9 @@ llvm_compile_expr(ExprState *state)
LLVMValueRef v_aggnulls;
 
instr_time  starttime;
+   instr_time  deform_starttime;
instr_time  endtime;
+   instr_time  deform_endtime;
 
llvm_enter_fatal_on_oom();
 
@@ -315,10 +317,14 @@ llvm_compile_expr(ExprState *state)
 */

Is OpenSSL AES-NI not available in pgcrypto?

2023-01-02 Thread aghart...@gmail.com

Hi all,

A question, may I wrong.

I've a Rocky Linux 8 with OpenSSL 1.1.1 FIPS  and Intel cpu with aes 
support (cat /proc/cpuinfo | grep aes)


Test made with openssl gives me a huge performance with aes enabled vs not:

"openssl speed -elapsed -evp aes-128-cbc" is about 5 time faster than 
"openssl speed -elapsed aes-128-cbc" or another "software calculated 
test", eg. "openssl speed -elapsed bf-cbc"


So OpenSSL is ok.

Postgresql 15 is compiled with openssl:

select name, setting from pg_settings where name = 'ssl_library';
    name | setting
-+-
 ssl_library | OpenSSL
(1 row)

So, a test with pgcrypto:

select pgp_sym_encrypt(data::text, 'pwd') --default to aes128
from generate_series('2022-01-01'::timestamp, '2022-12-31'::timestamp, 
'1 hour'::interval) data


vs

select pgp_sym_encrypt(data::text, 'pwd','cipher-algo=bf') -- blowfish
from generate_series('2022-01-01'::timestamp, '2022-12-31'::timestamp, 
'1 hour'::interval) data


In my test both queries execution is similaraes-128 was expected 
about  5 time faster.


So, why?

Pgcrypto use OpenSSL as backend, so, does it explicit force software aes 
calculation instead of AES-NI cpu ones?


Thanksfor support.

Best regards,

Agharta








Re: Allow pageinspect's bt_page_stats function to return a set of rows instead of a single row

2023-01-02 Thread Tom Lane
Hamid Akhtar  writes:
> I wasn't aware of the meson.build file. Attached is the latest version of
> the patch that contains the updated meson.build.

Pushed with minor corrections, plus one major one: you missed the
point of aeaaf520f, that pageinspect functions that touch relations
need to be parallel-restricted not parallel-safe.

regards, tom lane




Re: Infinite Interval

2023-01-02 Thread Joseph Koshakow
On Sat, Dec 31, 2022 at 12:09 AM jian he  wrote:
> In float8, select float8 'inf' / float8 'inf' return NaN. Now in your patch  
> select interval 'infinity' / float8 'infinity'; returns infinity.
> I am not sure it's right. I found this related post 
> (https://math.stackexchange.com/questions/181304/what-is-infinity-divided-by-infinity).

Good point, I agree this should return an error. We also need to
properly handle multiplication and division of infinite intervals by
float8 'nan'. My patch is returning an infinite interval, but it should
be returning an error. I'll upload a new patch shortly.

- Joe




Re: MERGE ... WHEN NOT MATCHED BY SOURCE

2023-01-02 Thread Dean Rasheed
On Fri, 30 Dec 2022 at 16:56, Dean Rasheed  wrote:
>
> Attached is a WIP patch.
>

Updated patch attached, now with updated docs and some other minor tidying up.

Regards,
Dean
diff --git a/doc/src/sgml/mvcc.sgml b/doc/src/sgml/mvcc.sgml
new file mode 100644
index b87ad5c..1482ede
--- a/doc/src/sgml/mvcc.sgml
+++ b/doc/src/sgml/mvcc.sgml
@@ -396,8 +396,8 @@
 originally matched appears later in the list of actions.
 On the other hand, if the row is concurrently updated or deleted so
 that the join condition fails, then MERGE will
-evaluate the condition's NOT MATCHED actions next,
-and execute the first one that succeeds.
+evaluate the condition's NOT MATCHED [BY TARGET]
+actions next, and execute the first one that succeeds.
 If MERGE attempts an INSERT
 and a unique index is present and a duplicate row is concurrently
 inserted, then a uniqueness violation error is raised;
diff --git a/doc/src/sgml/ref/merge.sgml b/doc/src/sgml/ref/merge.sgml
new file mode 100644
index 0995fe0..8ef121a
--- a/doc/src/sgml/ref/merge.sgml
+++ b/doc/src/sgml/ref/merge.sgml
@@ -33,7 +33,8 @@ USING dat
 and when_clause is:
 
 { WHEN MATCHED [ AND condition ] THEN { merge_update | merge_delete | DO NOTHING } |
-  WHEN NOT MATCHED [ AND condition ] THEN { merge_insert | DO NOTHING } }
+  WHEN NOT MATCHED BY SOURCE [ AND condition ] THEN { merge_update | merge_delete | DO NOTHING } |
+  WHEN NOT MATCHED [BY TARGET] [ AND condition ] THEN { merge_insert | DO NOTHING } }
 
 and merge_insert is:
 
@@ -70,7 +71,9 @@ DELETE
from data_source to
target_table_name
producing zero or more candidate change rows.  For each candidate change
-   row, the status of MATCHED or NOT MATCHED
+   row, the status of MATCHED,
+   NOT MATCHED BY SOURCE,
+   or NOT MATCHED [BY TARGET]
is set just once, after which WHEN clauses are evaluated
in the order specified.  For each candidate change row, the first clause to
evaluate as true is executed.  No more than one WHEN
@@ -226,16 +229,37 @@ DELETE
   At least one WHEN clause is required.
  
  
+  The WHEN clause may specify WHEN MATCHED,
+  WHEN NOT MATCHED BY SOURCE, or
+  WHEN NOT MATCHED [BY TARGET].
+  Note that the SQL standard only defines
+  WHEN MATCHED and WHEN NOT MATCHED
+  (which is defined to mean no matching target row).
+  WHEN NOT MATCHED BY SOURCE is an extension to the
+  SQL standard, as is the option to append
+  BY TARGET to WHEN NOT MATCHED, to
+  make its meaning more explicit.
+ 
+ 
   If the WHEN clause specifies WHEN MATCHED
-  and the candidate change row matches a row in the
+  and the candidate change row matches a row in the source to a row in the
   target_table_name,
   the WHEN clause is executed if the
   condition is
   absent or it evaluates to true.
  
  
-  Conversely, if the WHEN clause specifies
-  WHEN NOT MATCHED
+  If the WHEN clause specifies
+  WHEN NOT MATCHED BY SOURCE and the candidate change
+  row represents a row in the
+  target_table_name that does
+  not match a source row, the WHEN clause is executed
+  if the condition is
+  absent or it evaluates to true.
+ 
+ 
+  If the WHEN clause specifies
+  WHEN NOT MATCHED [BY TARGET]
   and the candidate change row does not match a row in the
   target_table_name,
   the WHEN clause is executed if the
@@ -257,7 +281,10 @@ DELETE
  
   A condition on a WHEN MATCHED clause can refer to columns
   in both the source and the target relations. A condition on a
-  WHEN NOT MATCHED clause can only refer to columns from
+  WHEN NOT MATCHED BY SOURCE clause can only refer to
+  columns from the target relation, since by definition there is no matching
+  source row. A condition on a WHEN NOT MATCHED [BY TARGET]
+  clause can only refer to columns from
   the source relation, since by definition there is no matching target row.
   Only the system attributes from the target table are accessible.
  
@@ -382,8 +409,10 @@ DELETE
   WHEN MATCHED clause, the expression can use values
   from the original row in the target table, and values from the
   data_source row.
-  If used in a WHEN NOT MATCHED clause, the
-  expression can use values from the data_source.
+  If used in a WHEN NOT MATCHED BY SOURCE clause, the
+  expression can only use values from the original row in the target table.
+  If used in a WHEN NOT MATCHED [BY TARGET] clause, the
+  expression can only use values from the data_source.
  
 

@@ -452,8 +481,9 @@ MERGE tot

 
  
-  Evaluate whether each row is MATCHED or
-  NOT MATCHED.
+  Evaluate whether each row is MATCHED,
+  NOT MATCHED BY SOURCE, or
+  NOT MATCHED [BY TARGET].
  
 
 
@@ -528,7 +558,8 @@ MERGE

Re: Fixing a couple of buglets in how VACUUM sets visibility map bits

2023-01-02 Thread Peter Geoghegan
On Sat, Dec 31, 2022 at 4:53 PM Peter Geoghegan  wrote:
> The first patch makes sure that the snapshotConflictHorizon cutoff
> (XID cutoff for recovery conflicts) is never a special XID, unless
> that XID is InvalidTransactionId, which is interpreted as a
> snapshotConflictHorizon value that will never need a recovery conflict
> (per the general convention for snapshotConflictHorizon values
> explained above ResolveRecoveryConflictWithSnapshot).

Pushed this just now.

Attached is another very simple refactoring patch for vacuumlazy.c. It
makes vacuumlazy.c save the result of get_database_name() in vacrel,
which matches what we already do with things like
get_namespace_name().

Would be helpful if I could get a +1 on
v1-0002-Never-just-set-the-all-frozen-bit-in-VM.patch, which is
somewhat more substantial than the others.

-- 
Peter Geoghegan


0001-Save-get_database_name-in-vacrel.patch
Description: Binary data


Re: Infinite Interval

2023-01-02 Thread Joseph Koshakow
On Mon, Jan 2, 2023 at 1:21 PM Joseph Koshakow  wrote:
>
> On Sat, Dec 31, 2022 at 12:09 AM jian he  wrote:
> > In float8, select float8 'inf' / float8 'inf' return NaN. Now in your patch 
> >  select interval 'infinity' / float8 'infinity'; returns infinity.
> > I am not sure it's right. I found this related post 
> > (https://math.stackexchange.com/questions/181304/what-is-infinity-divided-by-infinity).
>
> Good point, I agree this should return an error. We also need to
> properly handle multiplication and division of infinite intervals by
> float8 'nan'. My patch is returning an infinite interval, but it should
> be returning an error. I'll upload a new patch shortly.
>
> - Joe

Attached is the patch to handle these scenarios. Apparently dividing by
NaN is currently broken:
postgres=# SELECT INTERVAL '1 day' / float8 'nan';
 ?column?
---
 -178956970 years -8 mons -2562047788:00:54.775808
(1 row)

This patch will fix the issue, but we may want a separate patch that
handles this specific, existing issue. Any thoughts?

- Joe
From 2110bbe8be4b1c5c66eb48c35b958d84352a6287 Mon Sep 17 00:00:00 2001
From: Joseph Koshakow 
Date: Sat, 17 Dec 2022 14:21:26 -0500
Subject: [PATCH] This is WIP.

Following things are supported
1. Accepts '+/-infinity' as a valid string input for interval type.
2. Support interval_pl, interval_div
3. Tests in interval.sql for comparison operators working fine.

TODOs
1. Various TODOs in code
2. interval_pl: how to handle infinite values with opposite signs
3. timestamp, timestamptz, date and time arithmetic
4. Fix horology test.

Ashutosh Bapat
---
 src/backend/utils/adt/date.c   |  20 +
 src/backend/utils/adt/datetime.c   |  14 +-
 src/backend/utils/adt/timestamp.c  | 347 -
 src/include/datatype/timestamp.h   |  22 +
 src/test/regress/expected/horology.out |   6 +-
 src/test/regress/expected/interval.out | 993 -
 src/test/regress/sql/horology.sql  |   6 +-
 src/test/regress/sql/interval.sql  | 194 -
 8 files changed, 1527 insertions(+), 75 deletions(-)

diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index 1cf7c7652d..c6259cd9c1 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -2073,6 +2073,11 @@ time_pl_interval(PG_FUNCTION_ARGS)
 	Interval   *span = PG_GETARG_INTERVAL_P(1);
 	TimeADT		result;
 
+	if (INTERVAL_NOT_FINITE(span))
+		ereport(ERROR,
+(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("cannot add infinite interval to time")));
+
 	result = time + span->time;
 	result -= result / USECS_PER_DAY * USECS_PER_DAY;
 	if (result < INT64CONST(0))
@@ -2091,6 +2096,11 @@ time_mi_interval(PG_FUNCTION_ARGS)
 	Interval   *span = PG_GETARG_INTERVAL_P(1);
 	TimeADT		result;
 
+	if (INTERVAL_NOT_FINITE(span))
+		ereport(ERROR,
+(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("cannot subtract infinite interval from time")));
+
 	result = time - span->time;
 	result -= result / USECS_PER_DAY * USECS_PER_DAY;
 	if (result < INT64CONST(0))
@@ -2605,6 +2615,11 @@ timetz_pl_interval(PG_FUNCTION_ARGS)
 	Interval   *span = PG_GETARG_INTERVAL_P(1);
 	TimeTzADT  *result;
 
+	if (INTERVAL_NOT_FINITE(span))
+		ereport(ERROR,
+(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("cannot add infinite interval to time")));
+
 	result = (TimeTzADT *) palloc(sizeof(TimeTzADT));
 
 	result->time = time->time + span->time;
@@ -2627,6 +2642,11 @@ timetz_mi_interval(PG_FUNCTION_ARGS)
 	Interval   *span = PG_GETARG_INTERVAL_P(1);
 	TimeTzADT  *result;
 
+	if (INTERVAL_NOT_FINITE(span))
+		ereport(ERROR,
+(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("cannot subtract infinite interval from time")));
+
 	result = (TimeTzADT *) palloc(sizeof(TimeTzADT));
 
 	result->time = time->time - span->time;
diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index b5b117a8ca..b60d91dfb8 100644
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -70,7 +70,7 @@ static bool DetermineTimeZoneAbbrevOffsetInternal(pg_time_t t,
   const char *abbr, pg_tz *tzp,
   int *offset, int *isdst);
 static pg_tz *FetchDynamicTimeZone(TimeZoneAbbrevTable *tbl, const datetkn *tp,
-   DateTimeErrorExtra *extra);
+   DateTimeErrorExtra * extra);
 
 
 const int	day_tab[2][13] =
@@ -977,7 +977,7 @@ ParseDateTime(const char *timestr, char *workbuf, size_t buflen,
 int
 DecodeDateTime(char **field, int *ftype, int nf,
 			   int *dtype, struct pg_tm *tm, fsec_t *fsec, int *tzp,
-			   DateTimeErrorExtra *extra)
+			   DateTimeErrorExtra * extra)
 {
 	int			fmask = 0,
 tmask,
@@ -1927,7 +1927,7 @@ DetermineTimeZoneAbbrevOffsetInternal(pg_time_t t, const char *abbr, pg_tz *tzp,
 int
 DecodeTimeOnly(char **field, int *ftype, int nf,
 			   int *dtype, struct pg_tm *tm, fsec_t *fsec, int

Why is char an internal-use category

2023-01-02 Thread Dave Cramer
I see in v15 there is a note that there is a new category for "char"
however it is categorized as "internal use"

I would think that char and char(n) would be used by external programs as a
user type.

Dave Cramer


Re: Why is char an internal-use category

2023-01-02 Thread Tom Lane
Dave Cramer  writes:
> I see in v15 there is a note that there is a new category for "char"
> however it is categorized as "internal use"
> I would think that char and char(n) would be used by external programs as a
> user type.

"char" (with quotes) is not at all the same type as char without
quotes.

regards, tom lane




Re: New strategies for freezing, advancing relfrozenxid early

2023-01-02 Thread Peter Geoghegan
On Sat, Dec 31, 2022 at 12:45 PM Peter Geoghegan  wrote:
> On Sat, Dec 31, 2022 at 11:46 AM Jeff Davis  wrote:
> > "We have no freeze plans to execute, so there's no cost to following
> > the freeze path. This is important in the case where the page is
> > entirely frozen already, so that the page will be marked as such in the
> > VM."
>
> I'm happy to use your wording instead -- I'll come up with a patch for that.

What do you think of the wording adjustments in the attached patch?
It's based on your suggested wording.

--
Peter Geoghegan


0001-Tweak-page-level-freezing-comments.patch
Description: Binary data


Re: Reduce timing overhead of EXPLAIN ANALYZE using rdtsc?

2023-01-02 Thread Lukas Fittl
Hi David,

Thanks for continuing to work on this patch, and my apologies for silence
on the patch.

Its been hard to make time, and especially so because I typically develop
on an ARM-based macOS system where I can't test this directly - hence my
tests with virtualized EC2 instances, where I ran into the timing oddities.

On Mon, Jan 2, 2023 at 5:28 AM David Geier  wrote:

> The INSTR_TIME_GET_MICROSEC() returns a uint64 while the other variants
> return double. This seems error prone. What about renaming the function
> or also have the function return a double and cast where necessary at
> the call site?
>

Minor note, but in my understanding using a uint64 (where we can) is faster
for any simple arithmetic we do with the values.


> If no one objects I would also re-register this patch in the commit fest.
>

+1, and feel free to carry this patch forward - I'll try to make an effort
to review my earlier testing issues again, as well as your later
improvements to the patch.

Also, FYI, I just posted an alternate idea for speeding up EXPLAIN ANALYZE
with timing over in [0], using a sampling-based approach to reduce the
timing overhead.

[0]:
https://www.postgresql.org/message-id/CAP53PkxXMk0j-%2B0%3DYwRti2pFR5UB2Gu4v2Oyk8hhZS0DRART6g%40mail.gmail.com

Thanks,
Lukas

-- 
Lukas Fittl


Re: Reduce timing overhead of EXPLAIN ANALYZE using rdtsc?

2023-01-02 Thread Maciek Sakrejda
On Fri, Jul 15, 2022 at 11:21 AM Maciek Sakrejda  wrote:
> On Fri, Jul 1, 2022 at 10:26 AM Andres Freund  wrote:
> > On 2022-07-01 01:23:01 -0700, Lukas Fittl wrote:
> >...
> > > Known WIP problems with this patch version:
> > >
> > > * There appears to be a timing discrepancy I haven't yet worked out, where
> > >   the \timing data reported by psql doesn't match what EXPLAIN ANALYZE is
> > >   reporting. With Andres' earlier test case, I'm seeing a consistent 
> > > ~700ms
> > >   higher for \timing than for the EXPLAIN ANALYZE time reported on the
> > > server
> > >   side, only when rdtsc measurement is used -- its likely there is a 
> > > problem
> > >   somewhere with how we perform the cycles to time conversion
> >
> > Could you explain a bit more what you're seeing? I just tested your patches
> > and didn't see that here.
>
> I did not see this either, but I did see that the execution time
> reported by \timing is (for this test case) consistently 0.5-1ms
> *lower* than the Execution Time reported by EXPLAIN. I did not see
> that on master. Is that expected?

For what it's worth, I can no longer reproduce this. In fact, I went
back to master-as-of-around-then and applied Lukas' v2 patches again,
and I still can't reproduce that. I do remember it happening
consistently across several executions, but now \timing consistently
shows 0.5-1ms slower, as expected. This does not explain the different
timing issue Lukas was seeing in his tests, but I think we can assume
what I reported originally here is not an issue.




Re: An oversight in ExecInitAgg for grouping sets

2023-01-02 Thread Tom Lane
Richard Guo  writes:
> On Thu, Dec 22, 2022 at 2:02 PM Richard Guo  wrote:
>> If it is an empty grouping set, its length will be zero, and accessing
>> phasedata->eqfunctions[length - 1] is not right.

> Attached is a trivial patch for the fix.

Agreed, that's a latent bug.  It's only latent because the word just
before a palloc chunk will never be zero, either in our historical
palloc code or in v16's shiny new implementation.  Nonetheless it
is a horrible idea for ExecInitAgg to depend on that fact, so I
pushed your fix.

The thing that I find really distressing here is that it's been
like this for years and none of our automated testing caught it.
You'd have expected valgrind testing to do so ... but it does not,
because we've never marked that word NOACCESS.  Maybe we should
rethink that?  It'd require making mcxt.c do some valgrind flag
manipulations so it could access the hdrmask when appropriate.

regards, tom lane




TAP tests for psql \g piped into program

2023-01-02 Thread Daniel Verite
Hi,

This is a follow-up to commit d2a44904 from the 2022-11 CF [1]
The TAP tests were left out with the suggestion to use Perl instead of
cat (Unix) / findstr (Windows) as the program to pipe into.

PFA a patch implementing that suggestion.


[1] https://commitfest.postgresql.org/40/4000/

Best regards,
-- 
Daniel Vérité
https://postgresql.verite.pro/
Twitter: @DanielVerite
diff --git a/src/bin/psql/t/001_basic.pl b/src/bin/psql/t/001_basic.pl
index f447845717..9b908171e0 100644
--- a/src/bin/psql/t/001_basic.pl
+++ b/src/bin/psql/t/001_basic.pl
@@ -325,4 +325,31 @@ is($row_count, '10',
'client-side error commits transaction, no ON_ERROR_STOP and multiple 
-c switches'
 );
 
+# Test \g output piped into a program.
+# The program is perl -pe '' to simply copy the input to the output.
+my $g_file = "$tempdir/g_file_1.out";
+my $perlbin = PostgreSQL::Test::Utils::perl_binary();
+my $pipe_cmd = "$perlbin -pe '' >$g_file";
+
+psql_like($node, "SELECT 'one' \\g | $pipe_cmd", qr//, "one command \\g");
+my $c1 = slurp_file($g_file);
+like($c1, qr/one/);
+
+psql_like($node, "SELECT 'two' \\; SELECT 'three' \\g | $pipe_cmd", qr//, "two 
commands \\g");
+my $c2 = slurp_file($g_file);
+like($c2, qr/two.*three/s);
+
+
+psql_like($node, "\\set SHOW_ALL_RESULTS 0\nSELECT 'four' \\; SELECT 'five' 
\\g | $pipe_cmd", qr//,
+  "two commands \\g with only last result");
+my $c3 = slurp_file($g_file);
+like($c3, qr/five/);
+unlike($c3, qr/four/);
+
+psql_like($node, "copy (values ('foo'),('bar')) to stdout \\g | $pipe_cmd",
+ qr//,
+ "copy output passed to \\g pipe");
+my $c4 = slurp_file($g_file);
+like($c4, qr/foo.*bar/s);
+
 done_testing();
diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm 
b/src/test/perl/PostgreSQL/Test/Utils.pm
index b139190cc8..4e97c49dca 100644
--- a/src/test/perl/PostgreSQL/Test/Utils.pm
+++ b/src/test/perl/PostgreSQL/Test/Utils.pm
@@ -74,6 +74,7 @@ our @EXPORT = qw(
   run_log
   run_command
   pump_until
+  perl_binary
 
   command_ok
   command_fails
@@ -448,6 +449,23 @@ sub pump_until
 
 =pod
 
+=item perl_binary()
+
+Return the location of the currently running Perl interpreter.
+
+=cut
+
+sub perl_binary
+{
+   # Note: use of forward slashes here avoids any escaping problems
+   # that arise from use of backslashes.
+   my $perlbin = $^X;
+   $perlbin =~ s!\\!/!g if $windows_os;
+   return $perlbin;
+}
+
+=pod
+
 =item generate_ascii_string(from_char, to_char)
 
 Generate a string made of the given range of ASCII characters.
diff --git a/src/test/recovery/t/025_stuck_on_old_timeline.pl 
b/src/test/recovery/t/025_stuck_on_old_timeline.pl
index fd821242e8..9b04175ef2 100644
--- a/src/test/recovery/t/025_stuck_on_old_timeline.pl
+++ b/src/test/recovery/t/025_stuck_on_old_timeline.pl
@@ -25,12 +25,11 @@ my $node_primary = 
PostgreSQL::Test::Cluster->new('primary');
 # get there.
 $node_primary->init(allows_streaming => 1, has_archiving => 1);
 
+my $perlbin = PostgreSQL::Test::Utils::perl_binary();
+my $archivedir_primary = $node_primary->archive_dir;
 # Note: consistent use of forward slashes here avoids any escaping problems
 # that arise from use of backslashes. That means we need to double-quote all
 # the paths in the archive_command
-my $perlbin = $^X;
-$perlbin =~ s!\\!/!g if $PostgreSQL::Test::Utils::windows_os;
-my $archivedir_primary = $node_primary->archive_dir;
 $archivedir_primary =~ s!\\!/!g if $PostgreSQL::Test::Utils::windows_os;
 $node_primary->append_conf(
'postgresql.conf', qq(


Re: daitch_mokotoff module

2023-01-02 Thread Dag Lem
Alvaro Herrera  writes:

> Hello
>
> On 2022-Dec-23, Dag Lem wrote:
>

[...]

> So, yes, I'm proposing that we returns those as array elements and that
> @> is used to match them.

Looking into the array operators I guess that to match such arrays
directly one would actually use && (overlaps) rather than @> (contains),
but I digress.

The function is changed to return an array of soundex codes - I hope it
is now to your liking :-)

I also improved on the documentation example (using Full Text Search).
AFAIK you can't make general queries like that using arrays, however in
any case I must admit that text arrays seem like more natural building
blocks than space delimited text here.

Search to perform

is the best match for Daitch-Mokotoff, however

, but
in any case I've changed it into return arrays now. I hope it is to your
liking.

>
>> Daitch-Mokotoff Soundex indexes alternative sounds for the same name,
>> however if I understand correctly, you want to index names by single
>> sounds, linking all alike sounding names to the same soundex code. I
>> fail to see how that is useful - if you want to find matches for a name,
>> you simply match against all indexed names. If you only consider one
>> sound, you won't find all names that match.
>
> Hmm, I think we're saying the same thing, but from opposite points of
> view.  No, I want each name to return multiple codes, but that those
> multiple codes can be treated as a multiple-value array of codes, rather
> than as a single string of space-separated codes.
>
>> In any case, as explained in the documentation, the implementation is
>> intended to be a companion to Full Text Search, thus text is the natural
>> representation for the soundex codes.
>
> Hmm, I don't agree with this point.  The numbers are representations of
> the strings, but they don't necessarily have to be strings themselves.
>
>
>> BTW Vera 79 does not match Veras 794000, because they don't sound
>> the same (up to the maximum soundex code length).
>
> No, and maybe that's okay because they have different codes.  But they
> are both similar, in Daitch-Mokotoff, to Borja, which has two codes,
> 79 and 794000.  (Any Spanish speaker will readily tell you that
> neither Vera nor Veras are similar in any way to Borja, but D-M has
> chosen to say that each of them matches one of Borjas' codes.  So they
> *are* related, even though indirectly, and as a genealogist you *may* be
> interested in getting a match for a person called Vera when looking for
> relatives to a person called Veras.  And, as a Spanish speaker, that
> would make a lot of sense to me.)
>
>
> Now, it's true that I've chosen to use Spanish names for my silly little
> experiment.  Maybe this isn't terribly useful as a practical example,
> because this algorithm seems to have been designed for Jew surnames and
> perhaps not many (or not any) Jews had Spanish surnames.  I don't know;
> I'm not a Jew myself (though Noah Gordon tells the tale of a Spanish Jew
> called Josep Álvarez in his book "The Winemaker", so I guess it's not
> impossible).  Anyway, I suspect if you repeat the experiment with names
> of other origins, you'll find pretty much the same results apply there,
> and that is the whole reason D-M returns multiple codes and not just
> one.
>
>
> Merry Christmas :-)

-- 
Dag




Re: daitch_mokotoff module

2023-01-02 Thread Dag Lem
Sorry about the latest unfinished email - don't know what key
combination I managed to hit there.

Alvaro Herrera  writes:

> Hello
>
> On 2022-Dec-23, Dag Lem wrote:
>

[...]

>
> So, yes, I'm proposing that we returns those as array elements and that
> @> is used to match them.
>

Looking into the array operators I guess that to match such arrays
directly one would actually use && (overlaps) rather than @> (contains),
but I digress.

The function is changed to return an array of soundex codes - I hope it
is now to your liking :-)

I also improved on the documentation example (using Full Text Search).
AFAIK you can't make general queries like that using arrays, however in
any case I must admit that text arrays seem like more natural building
blocks than space delimited text here.

[...]

>> BTW Vera 79 does not match Veras 794000, because they don't sound
>> the same (up to the maximum soundex code length).
>
> No, and maybe that's okay because they have different codes.  But they
> are both similar, in Daitch-Mokotoff, to Borja, which has two codes,
> 79 and 794000.  (Any Spanish speaker will readily tell you that
> neither Vera nor Veras are similar in any way to Borja, but D-M has
> chosen to say that each of them matches one of Borjas' codes.  So they
> *are* related, even though indirectly, and as a genealogist you *may* be
> interested in getting a match for a person called Vera when looking for
> relatives to a person called Veras.  And, as a Spanish speaker, that
> would make a lot of sense to me.)

It is what it is - we can't call it Daitch-Mokotoff Soundex while
implementing something else. Having said that, one can always pre- or
postprocess to tweak the results.

Daitch-Mokotoff Soundex is known to produce false positives, but that is
in many cases not a problem.

Even though it's clearly tuned for Jewish names, the soundex algorithm
seems to work just fine for European names (we use it to match mostly
Norwegian names).


Best regards

Dag Lem

diff --git a/contrib/fuzzystrmatch/Makefile b/contrib/fuzzystrmatch/Makefile
index 0704894f88..12baf2d884 100644
--- a/contrib/fuzzystrmatch/Makefile
+++ b/contrib/fuzzystrmatch/Makefile
@@ -3,14 +3,15 @@
 MODULE_big = fuzzystrmatch
 OBJS = \
 	$(WIN32RES) \
+	daitch_mokotoff.o \
 	dmetaphone.o \
 	fuzzystrmatch.o
 
 EXTENSION = fuzzystrmatch
-DATA = fuzzystrmatch--1.1.sql fuzzystrmatch--1.0--1.1.sql
+DATA = fuzzystrmatch--1.1.sql fuzzystrmatch--1.1--1.2.sql fuzzystrmatch--1.0--1.1.sql
 PGFILEDESC = "fuzzystrmatch - similarities and distance between strings"
 
-REGRESS = fuzzystrmatch
+REGRESS = fuzzystrmatch fuzzystrmatch_utf8
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
@@ -22,3 +23,14 @@ top_builddir = ../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
+
+# Force this dependency to be known even without dependency info built:
+daitch_mokotoff.o: daitch_mokotoff.h
+
+daitch_mokotoff.h: daitch_mokotoff_header.pl
+	perl $< $@
+
+distprep: daitch_mokotoff.h
+
+maintainer-clean:
+	rm -f daitch_mokotoff.h
diff --git a/contrib/fuzzystrmatch/daitch_mokotoff.c b/contrib/fuzzystrmatch/daitch_mokotoff.c
new file mode 100644
index 00..2548903770
--- /dev/null
+++ b/contrib/fuzzystrmatch/daitch_mokotoff.c
@@ -0,0 +1,582 @@
+/*
+ * Daitch-Mokotoff Soundex
+ *
+ * Copyright (c) 2021 Finance Norway
+ * Author: Dag Lem 
+ *
+ * This implementation of the Daitch-Mokotoff Soundex System aims at high
+ * performance.
+ *
+ * - The processing of each phoneme is initiated by an O(1) table lookup.
+ * - For phonemes containing more than one character, a coding tree is traversed
+ *   to process the complete phoneme.
+ * - The (alternate) soundex codes are produced digit by digit in-place in
+ *   another tree structure.
+ *
+ *  References:
+ *
+ * https://www.avotaynu.com/soundex.htm
+ * https://www.jewishgen.org/InfoFiles/Soundex.html
+ * https://familypedia.fandom.com/wiki/Daitch-Mokotoff_Soundex
+ * https://stevemorse.org/census/soundex.html (dmlat.php, dmsoundex.php)
+ * https://github.com/apache/commons-codec/ (dmrules.txt, DaitchMokotoffSoundex.java)
+ * https://metacpan.org/pod/Text::Phonetic (DaitchMokotoff.pm)
+ *
+ * A few notes on other implementations:
+ *
+ * - All other known implementations have the same unofficial rules for "UE",
+ *   these are also adapted by this implementation (0, 1, NC).
+ * - The only other known implementation which is capable of generating all
+ *   correct soundex codes in all cases is the JOS Soundex Calculator at
+ *   https://www.jewishgen.org/jos/jossound.htm
+ * - "J" is considered (only) a vowel in dmlat.php
+ * - The official rules for "RS" are commented out in dmlat.php
+ * - Identical code digits for adjacent letters are not collapsed correctly in
+ *   dmsoundex.php when double digit codes are involved. E.g. "BESST" yields
+ *   744300 instead of 743000 as for "BEST".
+ * - "J" is considered (only) a consonant in DaitchMokotoffSoundex.java
+ * - "Y" is n

Re: perl 5.36, C99, -Wdeclaration-after-statement -Wshadow=compatible-local

2023-01-02 Thread Andres Freund
Hi,

On 2022-12-29 13:40:13 -0800, Andres Freund wrote:
> > > Should we backpatch this? Given the volume of warnings it's probably a 
> > > good
> > > idea. But I'd let it step in HEAD for a few days of buildfarm coverage 
> > > first.
> > 
> > +1 to both points.
> 
> Pushed to HEAD.

I haven't seen any problems in HEAD, so I'm working on backpatching.

Greetings,

Andres Freund




Doc: Rework contrib appendix -- informative titles, tweaked sentences

2023-01-02 Thread Karl O. Pinc
Hi,

Attached is a patch: contrib_v1.patch

It modifies Appendix F, the contrib directory.

It adds brief text into the titles shown in the
table of contents so it's easier to tell what
each module does.  It also suffixes [trusted] or [obsolete]
on the relevant titles.

I added the word "extension" into the appendix title
because I always have problems scanning through the
appendix and finding the one to do with extensions.

The sentences describing what the modules are and how
to build them have been reworked.  Some split in 2,
some words removed or replaced, etc.

I introduced the word "component" because the appendix
has build instructions for command line programs as well
as extensions and libraries loaded with shared_preload_libraries().
This involved removing most occurrences of the word
"module", although it is left in the section title.

Regards,

Karl 
Free Software:  "You don't pay back, you pay forward."
 -- Robert A. Heinlein
diff --git a/doc/src/sgml/adminpack.sgml b/doc/src/sgml/adminpack.sgml
index 184e96d7a0..04f3b52379 100644
--- a/doc/src/sgml/adminpack.sgml
+++ b/doc/src/sgml/adminpack.sgml
@@ -1,7 +1,7 @@
 
 
 
- adminpack
+ adminpack — pgAdmin support toolpack
 
  
   adminpack
diff --git a/doc/src/sgml/amcheck.sgml b/doc/src/sgml/amcheck.sgml
index 5d61a33936..48d72a24a3 100644
--- a/doc/src/sgml/amcheck.sgml
+++ b/doc/src/sgml/amcheck.sgml
@@ -1,7 +1,7 @@
 
 
 
- amcheck
+ amcheck — tools to verify index consistency
 
  
   amcheck
diff --git a/doc/src/sgml/auth-delay.sgml b/doc/src/sgml/auth-delay.sgml
index 3bc9cfb207..690774f86f 100644
--- a/doc/src/sgml/auth-delay.sgml
+++ b/doc/src/sgml/auth-delay.sgml
@@ -1,7 +1,7 @@
 
 
 
- auth_delay
+ auth_delay — pause on authentication failure
 
  
   auth_delay
diff --git a/doc/src/sgml/auto-explain.sgml b/doc/src/sgml/auto-explain.sgml
index 394fec94e8..a80910dab5 100644
--- a/doc/src/sgml/auto-explain.sgml
+++ b/doc/src/sgml/auto-explain.sgml
@@ -1,7 +1,7 @@
 
 
 
- auto_explain
+ auto_explain — log execution plans of slow queries
 
  
   auto_explain
diff --git a/doc/src/sgml/basebackup-to-shell.sgml b/doc/src/sgml/basebackup-to-shell.sgml
index b2ecc373eb..fd082ceb0b 100644
--- a/doc/src/sgml/basebackup-to-shell.sgml
+++ b/doc/src/sgml/basebackup-to-shell.sgml
@@ -1,7 +1,7 @@
 
 
 
- basebackup_to_shell
+ basebackup_to_shell — example "shell" pg_basebackup module
 
  
   basebackup_to_shell
diff --git a/doc/src/sgml/basic-archive.sgml b/doc/src/sgml/basic-archive.sgml
index 0b650f17a8..c412590dd6 100644
--- a/doc/src/sgml/basic-archive.sgml
+++ b/doc/src/sgml/basic-archive.sgml
@@ -1,7 +1,7 @@
 
 
 
- basic_archive
+ basic_archive — an example WAL archive module
 
  
   basic_archive
diff --git a/doc/src/sgml/bloom.sgml b/doc/src/sgml/bloom.sgml
index a3f51cfdc4..4a188ad5f1 100644
--- a/doc/src/sgml/bloom.sgml
+++ b/doc/src/sgml/bloom.sgml
@@ -1,7 +1,7 @@
 
 
 
- bloom
+ bloom — bloom filter index access
 
  
   bloom
diff --git a/doc/src/sgml/btree-gin.sgml b/doc/src/sgml/btree-gin.sgml
index 5bc5a054e8..5aafe856b5 100644
--- a/doc/src/sgml/btree-gin.sgml
+++ b/doc/src/sgml/btree-gin.sgml
@@ -1,7 +1,8 @@
 
 
 
- btree_gin
+ btree_gin —
+   sample GIN B-tree equalivent operator classes [trusted]
 
  
   btree_gin
diff --git a/doc/src/sgml/btree-gist.sgml b/doc/src/sgml/btree-gist.sgml
index b67f20a00f..67ff13c77f 100644
--- a/doc/src/sgml/btree-gist.sgml
+++ b/doc/src/sgml/btree-gist.sgml
@@ -1,7 +1,8 @@
 
 
 
- btree_gist
+ btree_gist —
+   B-tree equalivent GiST index operators [trusted]
 
  
   btree_gist
diff --git a/doc/src/sgml/citext.sgml b/doc/src/sgml/citext.sgml
index 5986601327..bf08e9e6a2 100644
--- a/doc/src/sgml/citext.sgml
+++ b/doc/src/sgml/citext.sgml
@@ -1,7 +1,8 @@
 
 
 
- citext
+ citext —
+   a case-insensitive character string type [trusted]
 
  
   citext
diff --git a/doc/src/sgml/contrib-spi.sgml b/doc/src/sgml/contrib-spi.sgml
index fed6f24932..edeb5b6346 100644
--- a/doc/src/sgml/contrib-spi.sgml
+++ b/doc/src/sgml/contrib-spi.sgml
@@ -1,7 +1,7 @@
 
 
 
- spi
+ spi — Server Programming Interface features/examples
 
  
   SPI
diff --git a/doc/src/sgml/contrib.sgml b/doc/src/sgml/contrib.sgml
index 4e7b87a42f..fd2e40980d 100644
--- a/doc/src/sgml/contrib.sgml
+++ b/doc/src/sgml/contrib.sgml
@@ -1,27 +1,31 @@
 
 
 
- Additional Supplied Modules
+ Additional Supplied Modules and Extensions
 
  
-  This appendix and the next one contain information regarding the modules that
-  can be found in the contrib directory of the
+  This appendix and the next one contain information on the
+  optional components
+  found in the contrib directory of the
   PostgreSQL distribution.
   These include porting tools, analysis utilities,
-  and plug-in features that are not part of the core PostgreSQL system,
-  mainly because they address a limited audience or are too experimental
+  and plug-in features that are not part of the core PostgreSQL system.
+  They are separate mainly
+  because they address a limite

Re: Infinite Interval

2023-01-02 Thread Joseph Koshakow
I have another patch, this one adds validations to operations that
return intervals and updated error messages. I tried to give all of the
error messages meaningful text, but I'm starting to think that almost all
of them should just say "interval out of range". The current approach
may reveal some implementation details and lead to confusion. For
example, some subtractions are converted to additions which would lead
to an error message about addition.

SELECT date 'infinity' - interval 'infinity';
ERROR:  cannot add infinite values with opposite signs

I've also updated the commit message to include the remaining TODOs,
which I've copied below

  1. Various TODOs in code.
  2. Correctly implement interval_part for infinite intervals.
  3. Test consolidation.
  4. Should we just use the months field to test for infinity?
From 65aceb25bc090375b60d140b1630cabcc90f1c9c Mon Sep 17 00:00:00 2001
From: Joseph Koshakow 
Date: Sat, 17 Dec 2022 14:21:26 -0500
Subject: [PATCH] This is WIP.

TODOs
1. Various TODOs in code.
2. Correctly implement interval_part for infinite intervals.
3. Test consolidation.
4. Should we just use the months field to test for infinity?

Ashutosh Bapat and Joe Koshakow
---
 src/backend/utils/adt/date.c   |   20 +
 src/backend/utils/adt/datetime.c   |   14 +-
 src/backend/utils/adt/timestamp.c  |  372 -
 src/include/datatype/timestamp.h   |   22 +
 src/test/regress/expected/horology.out |6 +-
 src/test/regress/expected/interval.out | 1006 +++-
 src/test/regress/sql/horology.sql  |6 +-
 src/test/regress/sql/interval.sql  |  200 -
 8 files changed, 1571 insertions(+), 75 deletions(-)

diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index 1cf7c7652d..c6259cd9c1 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -2073,6 +2073,11 @@ time_pl_interval(PG_FUNCTION_ARGS)
 	Interval   *span = PG_GETARG_INTERVAL_P(1);
 	TimeADT		result;
 
+	if (INTERVAL_NOT_FINITE(span))
+		ereport(ERROR,
+(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("cannot add infinite interval to time")));
+
 	result = time + span->time;
 	result -= result / USECS_PER_DAY * USECS_PER_DAY;
 	if (result < INT64CONST(0))
@@ -2091,6 +2096,11 @@ time_mi_interval(PG_FUNCTION_ARGS)
 	Interval   *span = PG_GETARG_INTERVAL_P(1);
 	TimeADT		result;
 
+	if (INTERVAL_NOT_FINITE(span))
+		ereport(ERROR,
+(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("cannot subtract infinite interval from time")));
+
 	result = time - span->time;
 	result -= result / USECS_PER_DAY * USECS_PER_DAY;
 	if (result < INT64CONST(0))
@@ -2605,6 +2615,11 @@ timetz_pl_interval(PG_FUNCTION_ARGS)
 	Interval   *span = PG_GETARG_INTERVAL_P(1);
 	TimeTzADT  *result;
 
+	if (INTERVAL_NOT_FINITE(span))
+		ereport(ERROR,
+(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("cannot add infinite interval to time")));
+
 	result = (TimeTzADT *) palloc(sizeof(TimeTzADT));
 
 	result->time = time->time + span->time;
@@ -2627,6 +2642,11 @@ timetz_mi_interval(PG_FUNCTION_ARGS)
 	Interval   *span = PG_GETARG_INTERVAL_P(1);
 	TimeTzADT  *result;
 
+	if (INTERVAL_NOT_FINITE(span))
+		ereport(ERROR,
+(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("cannot subtract infinite interval from time")));
+
 	result = (TimeTzADT *) palloc(sizeof(TimeTzADT));
 
 	result->time = time->time - span->time;
diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index b5b117a8ca..b60d91dfb8 100644
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -70,7 +70,7 @@ static bool DetermineTimeZoneAbbrevOffsetInternal(pg_time_t t,
   const char *abbr, pg_tz *tzp,
   int *offset, int *isdst);
 static pg_tz *FetchDynamicTimeZone(TimeZoneAbbrevTable *tbl, const datetkn *tp,
-   DateTimeErrorExtra *extra);
+   DateTimeErrorExtra * extra);
 
 
 const int	day_tab[2][13] =
@@ -977,7 +977,7 @@ ParseDateTime(const char *timestr, char *workbuf, size_t buflen,
 int
 DecodeDateTime(char **field, int *ftype, int nf,
 			   int *dtype, struct pg_tm *tm, fsec_t *fsec, int *tzp,
-			   DateTimeErrorExtra *extra)
+			   DateTimeErrorExtra * extra)
 {
 	int			fmask = 0,
 tmask,
@@ -1927,7 +1927,7 @@ DetermineTimeZoneAbbrevOffsetInternal(pg_time_t t, const char *abbr, pg_tz *tzp,
 int
 DecodeTimeOnly(char **field, int *ftype, int nf,
 			   int *dtype, struct pg_tm *tm, fsec_t *fsec, int *tzp,
-			   DateTimeErrorExtra *extra)
+			   DateTimeErrorExtra * extra)
 {
 	int			fmask = 0,
 tmask,
@@ -3232,7 +3232,7 @@ DecodeTimezone(const char *str, int *tzp)
 int
 DecodeTimezoneAbbrev(int field, const char *lowtoken,
 	 int *ftype, int *offset, pg_tz **tz,
-	 DateTimeErrorExtra *extra)
+	 DateTimeErrorExtra * extra)
 {
 	const datetkn *tp;
 
@@ -3634,6 +3634,8 @@ DecodeInterval(char **field, int *ftype, int nf, int range,
 			cas

Re: perl 5.36, C99, -Wdeclaration-after-statement -Wshadow=compatible-local

2023-01-02 Thread Andres Freund
On 2023-01-02 15:46:36 -0800, Andres Freund wrote:
> On 2022-12-29 13:40:13 -0800, Andres Freund wrote:
> > > > Should we backpatch this? Given the volume of warnings it's probably a 
> > > > good
> > > > idea. But I'd let it step in HEAD for a few days of buildfarm coverage 
> > > > first.
> > > 
> > > +1 to both points.
> > 
> > Pushed to HEAD.
> 
> I haven't seen any problems in HEAD, so I'm working on backpatching.

And done.




Re: Simplify standby state machine a bit in WaitForWALToBecomeAvailable()

2023-01-02 Thread Michael Paquier
On Fri, Dec 30, 2022 at 10:32:57AM -0800, Nathan Bossart wrote:
> This looks correct to me.  The only thing that stood out to me was the loop
> through 'tles' in XLogFileReadyAnyTLI.  With this change, we'd loop through
> the timelines for both XLOG_FROM_PG_ARCHIVE and XLOG_FROM_PG_WAL, whereas
> now we only loop through the timelines once.  However, I doubt this makes
> much difference in practice.  You'd only do the extra loop whenever
> restoring from the archives failed.

case XLOG_FROM_ARCHIVE:
+
+   /*
+* After failing to read from archive, we try to read from
+* pg_wal.
+*/
+   currentSource = XLOG_FROM_PG_WAL;
+   break;
In standby mode, the priority lookup order is pg_wal -> archive ->
stream.  With this change, we would do pg_wal -> archive -> pg_wal ->
stream, meaning that it could influence some recovery scenarios while
involving more lookups than necessary to the local pg_wal/ directory?

See, on failure where the current source is XLOG_FROM_ARCHIVE, we
would not switch anymore directly to XLOG_FROM_STREAM.
--
Michael


signature.asc
Description: PGP signature


Re: CFM for 2023-01

2023-01-02 Thread Michael Paquier
On Wed, Dec 28, 2022 at 10:52:38AM +0530, vignesh C wrote:
> If no one has volunteered for the upcoming (January 2023) commitfest.
> I would like to volunteer for it.

If you want to be up to the task, that would be great, of course.  For
now, I have switched the CF as in progress.
--
Michael


signature.asc
Description: PGP signature


Re: [PATCH] random_normal function

2023-01-02 Thread Michael Paquier
On Fri, Dec 30, 2022 at 09:58:04PM -0600, Justin Pryzby wrote:
> This is still failing tests - did you enable cirrusci on your own github
> account to run available checks on the patch ?

FYI, here is the failure:
[21:23:10.814] In file included from pg_prng.c:27:
[21:23:10.814] ../../src/include/utils/float.h:46:16: error: ‘struct
Node’ declared inside parameter list will not be visible outside of
this definition or declaration [-Werror] 
[21:23:10.814]46 | struct Node *escontext);

And a link to it, from the CF bot:
https://cirrus-ci.com/task/5969961391226880?logs=gcc_warning#L452
--
Michael


signature.asc
Description: PGP signature


Re: New strategies for freezing, advancing relfrozenxid early

2023-01-02 Thread Jeff Davis
On Mon, 2023-01-02 at 11:45 -0800, Peter Geoghegan wrote:
> What do you think of the wording adjustments in the attached patch?
> It's based on your suggested wording.

Great, thank you.


-- 
Jeff Davis
PostgreSQL Contributor Team - AWS






Re: [PATCH] random_normal function

2023-01-02 Thread Tom Lane
Michael Paquier  writes:
> FYI, here is the failure:
> [21:23:10.814] In file included from pg_prng.c:27:
> [21:23:10.814] ../../src/include/utils/float.h:46:16: error: ‘struct
> Node’ declared inside parameter list will not be visible outside of
> this definition or declaration [-Werror] 
> [21:23:10.814]46 | struct Node *escontext);

Hmm ... this looks an awful lot like it is the fault of ccff2d20e
not of the random_normal patch; that is, we probably need a
"struct Node" stub declaration in float.h.  However, why are we
not seeing any reports of this from elsewhere?  I'm concerned now
that there are more places also needing stub declarations, but
my test process didn't expose it.

regards, tom lane




Re: CFM for 2023-01

2023-01-02 Thread vignesh C
On Tue, 3 Jan 2023 at 07:52, Michael Paquier  wrote:
>
> On Wed, Dec 28, 2022 at 10:52:38AM +0530, vignesh C wrote:
> > If no one has volunteered for the upcoming (January 2023) commitfest.
> > I would like to volunteer for it.
>
> If you want to be up to the task, that would be great, of course.  For
> now, I have switched the CF as in progress.

Thanks, I will start working on this.

Regards,
Vignesh




Re: Todo: Teach planner to evaluate multiple windows in the optimal order

2023-01-02 Thread David Rowley
On Mon, 26 Dec 2022 at 02:04, Ankit Kumar Pandey  wrote:
> Point #1
>
> In the above query Oracle 10g performs 2 sorts, DB2 and Sybase perform 3
> sorts. We also perform 3.

This shouldn't be too hard to do. See the code in
select_active_windows().  You'll likely want to pay attention to the
DISTINCT pathkeys if they exist and just use the ORDER BY pathkeys if
the query has no DISTINCT clause. DISTINCT is evaluated after Window
and before ORDER BY.

One idea to implement this would be to adjust the loop in
select_active_windows() so that we record any WindowClauses which have
the pathkeys contained in the ORDER BY / DISTINCT pathkeys then record
those separately and append those onto the end of the actives array
after the sort.

I do think you'll likely want to put any WindowClauses which have
pathkeys which are a true subset or true superset of the ORDER BY /
DISTINCT pathkeys last.  If they're a superset then we won't need to
perform any additional ordering for the DISTINCT / ORDER BY clause.
If they're a subset then we might be able to perform an Incremental
Sort, which is likely much cheaper than a full sort.  The existing
code should handle that part. You just need to make
select_active_windows() more intelligent.

You might also think that we could perform additional optimisations
and also adjust the ORDER BY clause of a WindowClause if it contains
the pathkeys of the DISTINCT / ORDER BY clause. For example:

SELECT *,row_number() over (order by a,b) from tab order by a,b,c;

However, if you were to adjust the WindowClauses ORDER BY to become
a,b,c then you could produce incorrect results for window functions
that change their result based on peer rows.

Note the difference in results from:

create table ab(a int, b int);
insert into ab select x,y from generate_series(1,5) x, generate_Series(1,5)y;

select a,b,count(*) over (order by a) from ab order by a,b;
select a,b,count(*) over (order by a,b) from ab order by a,b;

> and Point #2
>
> Teach planner to decide which window to evaluate first based on costs.
> Currently the first window in the query is evaluated first, there may be no
> index to help sort the first window, but perhaps there are for other windows
> in the query. This may allow an index scan instead of a seqscan -> sort.

What Tom wrote about that in the first paragraph of [1] still applies.
The problem is that if the query contains many joins that to properly
find the cheapest way of executing the query we'd have to perform the
join search once for each unique sort order of each WindowClause.
That's just not practical to do from a performance standpoint. The
join search can be very expensive. There may be something that could
be done to better determine the most likely candidate for the first
WindowClause using some heuristics, but I've no idea what those would
be. You should look into point #1 first. Point #2 is significantly
more difficult to solve in a way that would be acceptable to the
project.

David

[1] https://www.postgresql.org/message-id/11535.1230501658%40sss.pgh.pa.us




Re: [PATCH] Improve ability to display optimizer analysis using OPTIMIZER_DEBUG

2023-01-02 Thread David Rowley
On Mon, 26 Dec 2022 at 08:05, Ankit Kumar Pandey  wrote:
> Also, inputs from other hackers are welcomed here.

I'm with Tom on this.  I've never once used this feature to try to
figure out why a certain plan was chosen or not chosen.

Do you actually have a need for this or are you just trying to tick
off some TODO items?

I'd really rather not see us compiling all that debug code in by
default unless it's actually going to be useful to a meaningful number
of people.

David




RADIUS tests and improvements

2023-01-02 Thread Thomas Munro
Hi,

Here's a draft patch to tackle a couple of TODOs in the RADIUS code in auth.c.

The first change is to replace select() with a standard latch loop
that responds to interrupts, postmaster death etc promptly.  It's not
really too much of a big deal because the timeout was only 3 seconds
(hardcoded), but it's not good to have places that ignore ProcSignal,
and it's good to move code to our modern pattern for I/O multiplexing.

We know from experience that we have to crank timeouts up to be able
to run tests reliably on slow/valgrind/etc systems, so the second
change is to change the timeout to a GUC, as also requested by a
comment.  One good side-effect is that it becomes easy and fast to
test the timed-out code path too, with a small value.  While adding
the GUC I couldn't help wondering why RADIUS even needs a timeout
separate from authentication_timeout; another way to go here would be
to remove it completely, but that'd be a policy change (removing the 3
second timeout we always had).  Thoughts?

The patch looks bigger than it really is because it changes the
indentation level.

But first, some basic tests to show that it works.  We can test before
and after the change and have a non-zero level of confidence about
whacking the code around.  Like existing similar tests, you need to
install an extra package (FreeRADIUS) and opt in with
PG_EXTRA_TESTS=radius.  I figured out how to do that for our 3 CI
Unixen, so cfbot should run the tests and pass once I add this to the
March commitfest.  FreeRADIUS claims to work on Windows too, but I
don't know how to set that up; maybe someday someone will fix that for
all the PG_EXTRA_TESTS tests.  I've also seen this work on a Mac with
MacPorts.  There's only one pathname in there that's a wild guess:
non-Debianoid Linux systems; if you know the answer there please LMK.
From 72a63c87b595a31f3d5e68722ca6bc7460190cc0 Mon Sep 17 00:00:00 2001
From: Thomas Munro 
Date: Sat, 31 Dec 2022 14:41:57 +1300
Subject: [PATCH 1/3] Add simple test for RADIUS authentication.

This is similar to the existing tests for ldap and kerberos.  It
requires FreeRADIUS to be installed, and opens ports that may be
considered insecure, so users have to opt in with PG_EXTRA_TESTS=radius.
---
 src/test/Makefile |   2 +-
 src/test/meson.build  |   1 +
 src/test/radius/Makefile  |  23 +
 src/test/radius/meson.build   |  12 +++
 src/test/radius/t/001_auth.pl | 187 ++
 5 files changed, 224 insertions(+), 1 deletion(-)
 create mode 100644 src/test/radius/Makefile
 create mode 100644 src/test/radius/meson.build
 create mode 100644 src/test/radius/t/001_auth.pl

diff --git a/src/test/Makefile b/src/test/Makefile
index dbd3192874..687164412c 100644
--- a/src/test/Makefile
+++ b/src/test/Makefile
@@ -12,7 +12,7 @@ subdir = src/test
 top_builddir = ../..
 include $(top_builddir)/src/Makefile.global
 
-SUBDIRS = perl regress isolation modules authentication recovery subscription
+SUBDIRS = perl regress isolation modules authentication recovery radius subscription
 
 ifeq ($(with_icu),yes)
 SUBDIRS += icu
diff --git a/src/test/meson.build b/src/test/meson.build
index 5f3c9c2ba2..b5da17b531 100644
--- a/src/test/meson.build
+++ b/src/test/meson.build
@@ -5,6 +5,7 @@ subdir('isolation')
 
 subdir('authentication')
 subdir('recovery')
+subdir('radius')
 subdir('subscription')
 subdir('modules')
 
diff --git a/src/test/radius/Makefile b/src/test/radius/Makefile
new file mode 100644
index 00..56768a3ca9
--- /dev/null
+++ b/src/test/radius/Makefile
@@ -0,0 +1,23 @@
+#-
+#
+# Makefile for src/test/radius
+#
+# Portions Copyright (c) 1996-2022, PostgreSQL Global Development Group
+# Portions Copyright (c) 1994, Regents of the University of California
+#
+# src/test/radius/Makefile
+#
+#-
+
+subdir = src/test/radius
+top_builddir = ../../..
+include $(top_builddir)/src/Makefile.global
+
+check:
+	$(prove_check)
+
+installcheck:
+	$(prove_installcheck)
+
+clean distclean maintainer-clean:
+	rm -rf tmp_check
diff --git a/src/test/radius/meson.build b/src/test/radius/meson.build
new file mode 100644
index 00..ea7afc4555
--- /dev/null
+++ b/src/test/radius/meson.build
@@ -0,0 +1,12 @@
+# Copyright (c) 2022, PostgreSQL Global Development Group
+
+tests += {
+  'name': 'radius',
+  'sd': meson.current_source_dir(),
+  'bd': meson.current_build_dir(),
+  'tap': {
+'tests': [
+  't/001_auth.pl',
+],
+  },
+}
diff --git a/src/test/radius/t/001_auth.pl b/src/test/radius/t/001_auth.pl
new file mode 100644
index 00..44db62a3d7
--- /dev/null
+++ b/src/test/radius/t/001_auth.pl
@@ -0,0 +1,187 @@
+
+# Copyright (c) 2021-2022, PostgreSQL Global Development Group
+
+# Debian: apt-get install freeradius
+# Homebrew: brew install freeradius-server
+# FreeBSD: pkg install freeradius3
+# MacPorts: po

Re: wake up logical workers after ALTER SUBSCRIPTION

2023-01-02 Thread Amit Kapila
On Thu, Dec 15, 2022 at 4:47 AM Nathan Bossart  wrote:
>
> On Wed, Dec 14, 2022 at 02:02:58PM -0500, Tom Lane wrote:
> > Maybe we could have workers that are exiting for that reason set a
> > flag saying "please restart me without delay"?
>
> That helps a bit, but there are still delays when starting workers for new
> subscriptions.  I think we'd need to create a new array in shared memory
> for subscription OIDs that need their workers started immediately.
>

That would be tricky because the list of subscription OIDs can be
longer than the workers. Can't we set a boolean variable
(check_immediate or something like that) in LogicalRepCtxStruct and
use that to traverse the subscriptions? So, when any worker will
restart because of a parameter change, we can set the variable and
send a signal to the launcher. The launcher can then check this
variable to decide whether to start the missing workers for enabled
subscriptions.

-- 
With Regards,
Amit Kapila.




Re: Add a test to ldapbindpasswd

2023-01-02 Thread Julien Rouhaud
Hi,

On Mon, Jan 02, 2023 at 09:45:27AM -0500, Andrew Dunstan wrote:
>
> On 2023-01-01 Su 18:31, Andrew Dunstan wrote:
> > Let's see how we fare with this patch.
> >
> >
>
> Not so well :-(. This version tries to make the tests totally
> independent, as they should be. That's an attempt to get the cfbot to go
> green, but I am intending to refactor this code substantially so the
> common bits are in a module each test file will load.

FTR you can run the same set of CI tests using your own GH account rather than
sedning patches, see src/tools/ci/README/




Re: wake up logical workers after ALTER SUBSCRIPTION

2023-01-02 Thread Amit Kapila
On Wed, Dec 7, 2022 at 11:42 PM Nathan Bossart  wrote:
>
> On Wed, Dec 07, 2022 at 02:07:11PM +0300, Melih Mutlu wrote:
> > Do we also need to wake up all sync workers too? Even if not, I'm not
> > actually sure whether doing that would harm anything though.
> > Just asking since currently the patch wakes up all workers including sync
> > workers if any still exists.
>
> After sleeping on this, I think we can do better.  IIUC we can simply check
> for AllTablesyncsReady() at the end of process_syncing_tables_for_apply()
> and wake up the logical replication workers (which should just consiѕt of
> setting the current process's latch) if we are ready for two_phase mode.
>

How just waking up will help with two_phase mode? For that, we need to
restart the apply worker as we are doing at the beginning of
process_syncing_tables_for_apply().

-- 
With Regards,
Amit Kapila.




Re: Allow placeholders in ALTER ROLE w/o superuser

2023-01-02 Thread Alexander Korotkov
On Mon, Jan 2, 2023 at 6:42 PM Justin Pryzby  wrote:
> On Mon, Jan 02, 2023 at 06:14:48PM +0300, Alexander Korotkov wrote:
> > I'm going to push this if no objections.
>
> I also suggest that meson.build should not copy regress_args.

Good point, thanks.

--
Regards,
Alexander Korotkov


0001-meson-Add-running-test-setup-as-a-replacement-for-v2.patch
Description: Binary data


Re: [PATCH] Improve ability to display optimizer analysis using OPTIMIZER_DEBUG

2023-01-02 Thread Ankit Kumar Pandey



On 03/01/23 08:38, David Rowley wrote:

I'm with Tom on this.  I've never once used this feature to try to
figure out why a certain plan was chosen or not chosen.

I'd really rather not see us compiling all that debug code in by
default unless it's actually going to be useful to a meaningful number
of people.


Okay this makes sense.




Do you actually have a need for this or are you just trying to tick
off some TODO items?

I would say Iatter but reason I picked it up was more on side of 
learning optimizer better. Currently, I am left with


explain analyze which does its job but for understanding internal 
working of optimizer, there are not much alternatives.


Again, if I know where to put breakpoint, I could see required 
path/states but point of this todo item is


ability to do this without need of developer tools.

Also from the thread,

https://www.postgresql.org/message-id/20120821.121611.501104647612634419.t-is...@sraoss.co.jp


+1. It would also be popular with our academic users.


There could be potential for this as well.


--
Regards,
Ankit Kumar Pandey





Re: Time delayed LR (WAS Re: logical replication restrictions)

2023-01-02 Thread vignesh C
On Tue, 27 Dec 2022 at 14:59, Hayato Kuroda (Fujitsu)
 wrote:
> Note that more than half of the modifications are done by Osumi-san.

1) This global variable can be removed as it is used only in
send_feedback which is called from maybe_delay_apply so we could pass
it as a function argument:
+ * delay, avoid having positions of the flushed and apply LSN overwritten by
+ * the latest LSN.
+ */
+static bool in_delaying_apply = false;
+static XLogRecPtr last_received = InvalidXLogRecPtr;
+

2) -1 gets converted to -1000

+int64
+interval2ms(const Interval *interval)
+{
+   int64   days;
+   int64   ms;
+   int64   result;
+
+   days = interval->month * INT64CONST(30);
+   days += interval->day;
+
+   /* Detect whether the value of interval can cause an overflow. */
+   if (pg_mul_s64_overflow(days, MSECS_PER_DAY, &result))
+   ereport(ERROR,
+   (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+errmsg("bigint out of range")));
+
+   /* Adds portion time (in ms) to the previous result. */
+   ms = interval->time / INT64CONST(1000);
+   if (pg_add_s64_overflow(result, ms, &result))
+   ereport(ERROR,
+   (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+errmsg("bigint out of range")));

create subscription sub7 connection 'dbname=regression host=localhost
port=5432' publication pub1 with (min_apply_delay = '-1');
ERROR:  -1000 ms is outside the valid range for parameter "min_apply_delay"


3) This can be slightly reworded:
+  
+   The length of time (ms) to delay the application of changes.
+  
to:
Delay applying the changes by a specified amount of time(ms).

4)  maybe_delay_apply can be moved from apply_handle_stream_prepare to
apply_spooled_messages so that it is consistent with
maybe_start_skipping_changes:
@@ -1120,6 +1240,19 @@ apply_handle_stream_prepare(StringInfo s)

elog(DEBUG1, "received prepare for streamed transaction %u",
prepare_data.xid);

+   /*
+* Should we delay the current prepared transaction?
+*
+* Although the delay is applied in BEGIN PREPARE messages, streamed
+* prepared transactions apply the delay in a STREAM PREPARE message.
+* That's ok because no changes have been applied yet
+* (apply_spooled_messages() will do it). The STREAM START message does
+* not contain a prepare time (it will be available when the in-progress
+* prepared transaction finishes), hence, it was not possible to apply a
+* delay at that time.
+*/
+   maybe_delay_apply(prepare_data.prepare_time);


That way the call from apply_handle_stream_commit can also be removed.


5) typo transfering should be transferring
+  publisher and the current time on the subscriber. Time
spent in logical
+  decoding and in transfering the transaction may reduce the
actual wait
+  time. If the system clocks on publisher and subscriber are not

6) feedbacks can be changed to feedback messages
+ * it's necessary to keep sending feedbacks during the delay from the worker
+ * process. Meanwhile, the feature delays the apply before starting the

7)
+ /*
+ * Suppress overwrites of flushed and writtten positions by the lastest
+ * LSN in send_feedback().
+ */

7a) typo writtten should be written

7b) lastest should latest

Regards,
Vignesh




Re: Todo: Teach planner to evaluate multiple windows in the optimal order

2023-01-02 Thread Ankit Kumar Pandey



On 03/01/23 08:21, David Rowley wrote:

On Mon, 26 Dec 2022 at 02:04, Ankit Kumar Pandey  wrote:

Point #1

In the above query Oracle 10g performs 2 sorts, DB2 and Sybase perform 3
sorts. We also perform 3.

This shouldn't be too hard to do. See the code in
select_active_windows().  You'll likely want to pay attention to the
DISTINCT pathkeys if they exist and just use the ORDER BY pathkeys if
the query has no DISTINCT clause. DISTINCT is evaluated after Window
and before ORDER BY.

One idea to implement this would be to adjust the loop in
select_active_windows() so that we record any WindowClauses which have
the pathkeys contained in the ORDER BY / DISTINCT pathkeys then record
those separately and append those onto the end of the actives array
after the sort.

I do think you'll likely want to put any WindowClauses which have
pathkeys which are a true subset or true superset of the ORDER BY /
DISTINCT pathkeys last.  If they're a superset then we won't need to
perform any additional ordering for the DISTINCT / ORDER BY clause.
If they're a subset then we might be able to perform an Incremental
Sort, which is likely much cheaper than a full sort.  The existing
code should handle that part. You just need to make
select_active_windows() more intelligent.

You might also think that we could perform additional optimisations
and also adjust the ORDER BY clause of a WindowClause if it contains
the pathkeys of the DISTINCT / ORDER BY clause. For example:

SELECT *,row_number() over (order by a,b) from tab order by a,b,c;

However, if you were to adjust the WindowClauses ORDER BY to become
a,b,c then you could produce incorrect results for window functions
that change their result based on peer rows.

Note the difference in results from:

create table ab(a int, b int);
insert into ab select x,y from generate_series(1,5) x, generate_Series(1,5)y;

select a,b,count(*) over (order by a) from ab order by a,b;
select a,b,count(*) over (order by a,b) from ab order by a,b;


Thanks, let me try this.



and Point #2

Teach planner to decide which window to evaluate first based on costs.
Currently the first window in the query is evaluated first, there may be no
index to help sort the first window, but perhaps there are for other windows
in the query. This may allow an index scan instead of a seqscan -> sort.

What Tom wrote about that in the first paragraph of [1] still applies.
The problem is that if the query contains many joins that to properly
find the cheapest way of executing the query we'd have to perform the
join search once for each unique sort order of each WindowClause.
That's just not practical to do from a performance standpoint. The
join search can be very expensive. There may be something that could
be done to better determine the most likely candidate for the first
WindowClause using some heuristics, but I've no idea what those would
be. You should look into point #1 first. Point #2 is significantly
more difficult to solve in a way that would be acceptable to the
project.


Okay, leaving this out for now.

--
Regards,
Ankit Kumar Pandey





Re: typos

2023-01-02 Thread Michael Paquier
On Fri, Dec 30, 2022 at 05:12:57PM -0600, Justin Pryzby wrote:

 # Use larger ccache cache, as this task compiles with multiple compilers /
 # flag combinations
-CCACHE_MAXSIZE: "1GB"
+CCACHE_MAXSIZE: "1G"

In 0006, I am not sure how much this matters.  Perhaps somebody more
fluent with Cirrus, though, has a different opinion..

  * pointer to this structure.  The information here must be sufficient to
  * properly initialize each new TableScanDesc as workers join the scan, and it
- * must act as a information what to scan for those workers.
+ * must provide information what to scan for those workers.

This comment in 0009 is obviously incorrect, but I am not sure whether
your new suggestion is an improvement.  Do workers provide such
information or has this structure some information that the workers
rely on?

Not sure that the whitespace issue in 0021 for the header of inval.c
is worth caring about.

0014 and 0013 do not reduce the translation workload, as the messages
include some stuff specific to the GUC names accessed to, or some
specific details about the code paths triggered.

The rest has been applied where they matter.
--
Michael


signature.asc
Description: PGP signature


Re: typos

2023-01-02 Thread Amit Kapila
On Tue, Jan 3, 2023 at 12:58 PM Michael Paquier  wrote:
>
> On Fri, Dec 30, 2022 at 05:12:57PM -0600, Justin Pryzby wrote:
>
>  # Use larger ccache cache, as this task compiles with multiple compilers 
> /
>  # flag combinations
> -CCACHE_MAXSIZE: "1GB"
> +CCACHE_MAXSIZE: "1G"
>
> In 0006, I am not sure how much this matters.
>

The other places in that file use M, so maybe, this is more consistent.

One minor comment:
-spoken in Belgium (BE), with a UTF-8 character set
+spoken in Belgium (BE), with a UTF-8 character set

Shouldn't this be UTF8 as we are using in func.sgml?

-- 
With Regards,
Amit Kapila.




[Commitfest 2023-01] has started

2023-01-02 Thread vignesh C
Hi All,

Just a reminder that Commitfest 2023-01 has started.
There are many patches based on the latest run from [1] which require
a) Rebased on top of head b) Fix compilation failures c) Fix test
failure, please have a look and rebase it so that it is easy for the
reviewers and committers:
1. TAP output format for pg_regress
2. Add BufFileRead variants with short read and EOF detection
3. Add SHELL_EXIT_CODE variable to psql
4. Add foreign-server health checks infrastructure
5. Add last_vacuum_index_scans in pg_stat_all_tables
6. Add index scan progress to pg_stat_progress_vacuum
7. Add the ability to limit the amount of memory that can be allocated
to backends.
8. Add tracking of backend memory allocated to pg_stat_activity
9. CAST( ... ON DEFAULT)
10. CF App: add "Returned: Needs more interest" close status
11. CI and test improvements
12. Cygwin cleanup
13. Expand character set for ltree labels
14. Fix tab completion MERGE
15. Force streaming every change in logical decoding
16. More scalable multixacts buffers and locking
17. Move SLRU data into the regular buffer pool
18. Move extraUpdatedCols out of RangeTblEntry
19.New [relation] options engine
20. Optimizing Node Files Support
21. PGDOCS - Stats views and functions not in order?
22. POC: Lock updated tuples in tuple_update() and tuple_delete()
23. Parallelize correlated subqueries that execute within each worker
24. Pluggable toaster
25. Prefetch the next tuple's memory during seqscans
26. Pulling up direct-correlated ANY_SUBLINK
27. Push aggregation down to base relations and joins
28. Reduce timing overhead of EXPLAIN ANALYZE using rdtsc
29. Refactor relation extension, faster COPY
30. Remove NEW placeholder entry from stored view query range table
31. TDE key management patches
32. Use AF_UNIX for tests on Windows (ie drop fallback TCP code)
33. Windows filesystem support improvements
34. making relfilenodes 56 bit
35. postgres_fdw: commit remote (sub)transactions in parallel during pre-commit
36.recovery modules

Commitfest status as of now:
Needs review:177
Waiting on Author:   47
Ready for Committer:  20
Committed:  31
Withdrawn:4
Rejected:   0
Returned with Feedback:  0
Total:  279

We will be needing more members to actively review the patches to get
more patches to the committed state. I would like to remind you that
each patch submitter is expected to review at least one patch from
another submitter during the CommitFest, those members who have not
picked up patch for review please pick someone else's patch to review
as soon as you can.
I'll send out reminders this week to get your patches rebased and
update the status of the patch accordingly.

[1] - http://cfbot.cputube.org/

Regards,
Vignesh