public inbox for [email protected]
help / color / mirror / Atom feedRe: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc?
13+ messages / 5 participants
[nested] [flat]
* Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc?
@ 2023-08-23 12:48 Daniel Gustafsson <[email protected]>
2023-08-23 19:22 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Andres Freund <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: Daniel Gustafsson @ 2023-08-23 12:48 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: pgsql-hackers; Thomas Munro <[email protected]>; Justin Pryzby <[email protected]>; Nazir Bilal Yavuz <[email protected]>
> On 23 Aug 2023, at 08:58, Andres Freund <[email protected]> wrote:
> I'm hoping to push this fairly soon, as I'll be on vacation the last week of
> August. I'll be online intermittently though, if there are issues, I can react
> (very limited connectivity for middday Aug 29th - midday Aug 31th though). I'd
> appreciate a quick review or two.
I've been reading over these and the thread, and while not within my area of
expertise, nothing really sticks out.
I'll do another pass, but below are a few small comments so far.
I don't know Windows to know the implications, but should the below file have
some sort of warning about not doing that for production/shared systems, only
for dedicated test instances?
+++ b/src/tools/ci/windows_write_cache.ps1
@@ -0,0 +1,20 @@
+# Define the write cache to be power protected. This reduces the rate of cache
+# flushes, which seems to help metadata heavy workloads on NTFS. We're just
+# testing here anyway, so ...
+#
+# Let's do so for all disks, this could be useful beyond cirrus-ci.
One thing in 0010 caught my eye, and while not introduced in this patchset it
might be of interest here. In the below hunks we loop X ticks around
system(psql), with the loop assuming the server can come up really quickly and
sleeping if it doesn't. On my systems I always reach the pg_usleep after
failing the check, but if I reverse the check such it first sleeps and then
checks I only need to check once instead of twice.
@@ -2499,7 +2502,7 @@ regression_main(int argc, char *argv[],
else
wait_seconds = 60;
- for (i = 0; i < wait_seconds; i++)
+ for (i = 0; i < wait_seconds * WAITS_PER_SEC; i++)
{
/* Done if psql succeeds */
fflush(NULL);
@@ -2519,7 +2522,7 @@ regression_main(int argc, char *argv[],
outputdir);
}
- pg_usleep(1000000L);
+ pg_usleep(1000000L / WAITS_PER_SEC);
}
if (i >= wait_seconds)
{
It's a micro-optimization, but if we're changing things here to chase cycles it
might perhaps be worth doing?
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc?
2023-08-23 12:48 Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Daniel Gustafsson <[email protected]>
@ 2023-08-23 19:22 ` Andres Freund <[email protected]>
2023-08-23 20:11 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Daniel Gustafsson <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: Andres Freund @ 2023-08-23 19:22 UTC (permalink / raw)
To: Daniel Gustafsson <[email protected]>; +Cc: pgsql-hackers; Thomas Munro <[email protected]>; Justin Pryzby <[email protected]>; Nazir Bilal Yavuz <[email protected]>
Hi,
On 2023-08-23 14:48:26 +0200, Daniel Gustafsson wrote:
> > On 23 Aug 2023, at 08:58, Andres Freund <[email protected]> wrote:
>
> > I'm hoping to push this fairly soon, as I'll be on vacation the last week of
> > August. I'll be online intermittently though, if there are issues, I can react
> > (very limited connectivity for middday Aug 29th - midday Aug 31th though). I'd
> > appreciate a quick review or two.
>
> I've been reading over these and the thread, and while not within my area of
> expertise, nothing really sticks out.
Thanks!
> I'll do another pass, but below are a few small comments so far.
>
> I don't know Windows to know the implications, but should the below file have
> some sort of warning about not doing that for production/shared systems, only
> for dedicated test instances?
Ah, I should have explained that: I'm not planning to apply
- regress: Check for postgres startup completion more often
- ci: windows: Disabling write cache flushing during test
right now. Compared to the other patches the wins are much smaller and/or more
work is needed to make them good.
I think it might be worth going for
- ci: switch tasks to debugoptimized build
because that provides a fair bit of gain. But it might be more hurtful than
helpful due to costing more when ccache doesn't work...
> +++ b/src/tools/ci/windows_write_cache.ps1
> @@ -0,0 +1,20 @@
> +# Define the write cache to be power protected. This reduces the rate of cache
> +# flushes, which seems to help metadata heavy workloads on NTFS. We're just
> +# testing here anyway, so ...
> +#
> +# Let's do so for all disks, this could be useful beyond cirrus-ci.
>
> One thing in 0010 caught my eye, and while not introduced in this patchset it
> might be of interest here. In the below hunks we loop X ticks around
> system(psql), with the loop assuming the server can come up really quickly and
> sleeping if it doesn't. On my systems I always reach the pg_usleep after
> failing the check, but if I reverse the check such it first sleeps and then
> checks I only need to check once instead of twice.
I think there's more effective ways to make this cheaper. The basic thing
would be to use libpq instead of forking of psql to make a connection
check. Medium term, I think we should invent a way for pg_ctl and other
tooling (including pg_regress) to wait for the service to come up. E.g. having
a named pipe that postmaster opens once the server is up, which should allow
multiple clients to use select/epoll/... to wait for it without looping.
ISTM making pg_regress use libpq w/ PQping() should be a pretty simple patch?
The non-polling approach obviously is even better, but also requires more
thought (and documentation and ...).
> @@ -2499,7 +2502,7 @@ regression_main(int argc, char *argv[],
> else
> wait_seconds = 60;
>
> - for (i = 0; i < wait_seconds; i++)
> + for (i = 0; i < wait_seconds * WAITS_PER_SEC; i++)
> {
> /* Done if psql succeeds */
> fflush(NULL);
> @@ -2519,7 +2522,7 @@ regression_main(int argc, char *argv[],
> outputdir);
> }
>
> - pg_usleep(1000000L);
> + pg_usleep(1000000L / WAITS_PER_SEC);
> }
> if (i >= wait_seconds)
> {
>
> It's a micro-optimization, but if we're changing things here to chase cycles it
> might perhaps be worth doing?
I wouldn't quite call not waiting for 1s for the server to start, when it does
so within a few ms, chasing cycles ;). For short tests it's a substantial
fraction of the overall runtime...
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc?
2023-08-23 12:48 Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Daniel Gustafsson <[email protected]>
2023-08-23 19:22 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Andres Freund <[email protected]>
@ 2023-08-23 20:11 ` Daniel Gustafsson <[email protected]>
2023-08-23 21:02 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Tom Lane <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: Daniel Gustafsson @ 2023-08-23 20:11 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: pgsql-hackers; Thomas Munro <[email protected]>; Justin Pryzby <[email protected]>; Nazir Bilal Yavuz <[email protected]>
> On 23 Aug 2023, at 21:22, Andres Freund <[email protected]> wrote:
> On 2023-08-23 14:48:26 +0200, Daniel Gustafsson wrote:
>> I'll do another pass, but below are a few small comments so far.
>>
>> I don't know Windows to know the implications, but should the below file have
>> some sort of warning about not doing that for production/shared systems, only
>> for dedicated test instances?
>
> Ah, I should have explained that: I'm not planning to apply
> - regress: Check for postgres startup completion more often
> - ci: windows: Disabling write cache flushing during test
> right now. Compared to the other patches the wins are much smaller and/or more
> work is needed to make them good.
>
> I think it might be worth going for
> - ci: switch tasks to debugoptimized build
> because that provides a fair bit of gain. But it might be more hurtful than
> helpful due to costing more when ccache doesn't work...
Gotcha.
>> +++ b/src/tools/ci/windows_write_cache.ps1
>> @@ -0,0 +1,20 @@
>> +# Define the write cache to be power protected. This reduces the rate of cache
>> +# flushes, which seems to help metadata heavy workloads on NTFS. We're just
>> +# testing here anyway, so ...
>> +#
>> +# Let's do so for all disks, this could be useful beyond cirrus-ci.
>>
>> One thing in 0010 caught my eye, and while not introduced in this patchset it
>> might be of interest here. In the below hunks we loop X ticks around
>> system(psql), with the loop assuming the server can come up really quickly and
>> sleeping if it doesn't. On my systems I always reach the pg_usleep after
>> failing the check, but if I reverse the check such it first sleeps and then
>> checks I only need to check once instead of twice.
>
> I think there's more effective ways to make this cheaper. The basic thing
> would be to use libpq instead of forking of psql to make a connection
> check.
I had it in my head that not using libpq in pg_regress was a deliberate choice,
but I fail to find a reference to it in the archives.
>> It's a micro-optimization, but if we're changing things here to chase cycles it
>> might perhaps be worth doing?
>
> I wouldn't quite call not waiting for 1s for the server to start, when it does
> so within a few ms, chasing cycles ;). For short tests it's a substantial
> fraction of the overall runtime...
Absolutely, I was referring to shifting the sleep before the test to avoid the
extra test, not the reduction of the pg_usleep. Reducing the sleep is a clear
win.
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc?
2023-08-23 12:48 Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Daniel Gustafsson <[email protected]>
2023-08-23 19:22 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Andres Freund <[email protected]>
2023-08-23 20:11 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Daniel Gustafsson <[email protected]>
@ 2023-08-23 21:02 ` Tom Lane <[email protected]>
2023-08-23 21:12 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Daniel Gustafsson <[email protected]>
2023-08-23 21:43 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Andres Freund <[email protected]>
0 siblings, 2 replies; 13+ messages in thread
From: Tom Lane @ 2023-08-23 21:02 UTC (permalink / raw)
To: Daniel Gustafsson <[email protected]>; +Cc: Andres Freund <[email protected]>; pgsql-hackers; Thomas Munro <[email protected]>; Justin Pryzby <[email protected]>; Nazir Bilal Yavuz <[email protected]>
Daniel Gustafsson <[email protected]> writes:
> On 23 Aug 2023, at 21:22, Andres Freund <[email protected]> wrote:
>> I think there's more effective ways to make this cheaper. The basic thing
>> would be to use libpq instead of forking of psql to make a connection
>> check.
> I had it in my head that not using libpq in pg_regress was a deliberate choice,
> but I fail to find a reference to it in the archives.
I have a vague feeling that you are right about that. Perhaps the
concern was that under "make installcheck", pg_regress might be
using a build-tree copy of libpq rather than the one from the
system under test. As long as we're just trying to ping the server,
that shouldn't matter too much I think ... unless we hit problems
with, say, a different default port number or socket path compiled into
one copy vs. the other? That seems like it's probably a "so don't
do that" case, though.
regards, tom lane
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc?
2023-08-23 12:48 Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Daniel Gustafsson <[email protected]>
2023-08-23 19:22 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Andres Freund <[email protected]>
2023-08-23 20:11 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Daniel Gustafsson <[email protected]>
2023-08-23 21:02 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Tom Lane <[email protected]>
@ 2023-08-23 21:12 ` Daniel Gustafsson <[email protected]>
2023-08-28 12:32 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Daniel Gustafsson <[email protected]>
1 sibling, 1 reply; 13+ messages in thread
From: Daniel Gustafsson @ 2023-08-23 21:12 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Andres Freund <[email protected]>; pgsql-hackers; Thomas Munro <[email protected]>; Justin Pryzby <[email protected]>; Nazir Bilal Yavuz <[email protected]>
> On 23 Aug 2023, at 23:02, Tom Lane <[email protected]> wrote:
>
> Daniel Gustafsson <[email protected]> writes:
>> On 23 Aug 2023, at 21:22, Andres Freund <[email protected]> wrote:
>>> I think there's more effective ways to make this cheaper. The basic thing
>>> would be to use libpq instead of forking of psql to make a connection
>>> check.
>
>> I had it in my head that not using libpq in pg_regress was a deliberate choice,
>> but I fail to find a reference to it in the archives.
>
> I have a vague feeling that you are right about that. Perhaps the
> concern was that under "make installcheck", pg_regress might be
> using a build-tree copy of libpq rather than the one from the
> system under test. As long as we're just trying to ping the server,
> that shouldn't matter too much I think ... unless we hit problems
> with, say, a different default port number or socket path compiled into
> one copy vs. the other? That seems like it's probably a "so don't
> do that" case, though.
Ah yes, that does ring a familiar bell. I agree that using it for pinging the
server should be safe either way, but we should document the use-with-caution
in pg_regress.c if/when we go down that path. I'll take a stab at changing the
psql retry loop for pinging tomorrow to see what it would look like.
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc?
2023-08-23 12:48 Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Daniel Gustafsson <[email protected]>
2023-08-23 19:22 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Andres Freund <[email protected]>
2023-08-23 20:11 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Daniel Gustafsson <[email protected]>
2023-08-23 21:02 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Tom Lane <[email protected]>
2023-08-23 21:12 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Daniel Gustafsson <[email protected]>
@ 2023-08-28 12:32 ` Daniel Gustafsson <[email protected]>
0 siblings, 0 replies; 13+ messages in thread
From: Daniel Gustafsson @ 2023-08-28 12:32 UTC (permalink / raw)
To: Tom Lane <[email protected]>; Andres Freund <[email protected]>; +Cc: pgsql-hackers; Thomas Munro <[email protected]>; Justin Pryzby <[email protected]>; Nazir Bilal Yavuz <[email protected]>
> On 23 Aug 2023, at 23:12, Daniel Gustafsson <[email protected]> wrote:
>
>> On 23 Aug 2023, at 23:02, Tom Lane <[email protected]> wrote:
>>
>> Daniel Gustafsson <[email protected]> writes:
>>> On 23 Aug 2023, at 21:22, Andres Freund <[email protected]> wrote:
>>>> I think there's more effective ways to make this cheaper. The basic thing
>>>> would be to use libpq instead of forking of psql to make a connection
>>>> check.
>>
>>> I had it in my head that not using libpq in pg_regress was a deliberate choice,
>>> but I fail to find a reference to it in the archives.
>>
>> I have a vague feeling that you are right about that. Perhaps the
>> concern was that under "make installcheck", pg_regress might be
>> using a build-tree copy of libpq rather than the one from the
>> system under test. As long as we're just trying to ping the server,
>> that shouldn't matter too much I think ... unless we hit problems
>> with, say, a different default port number or socket path compiled into
>> one copy vs. the other? That seems like it's probably a "so don't
>> do that" case, though.
>
> Ah yes, that does ring a familiar bell. I agree that using it for pinging the
> server should be safe either way, but we should document the use-with-caution
> in pg_regress.c if/when we go down that path. I'll take a stab at changing the
> psql retry loop for pinging tomorrow to see what it would look like.
Attached is a patch with a quick PoC for using PQPing instead of using psql for
connection checks in pg_regress. In order to see performance it also includes
a diag output for "Time to first test" which contains all setup costs. This
might not make it into a commit but it was quite helpful in hacking so I left
it in for now.
The patch incorporates Andres' idea for finer granularity of checks by checking
TICKS times per second rather than once per second, it also shifts the
pg_usleep around to require just one ping in most cases compard to two today.
On my relatively tired laptop this speeds up pg_regress setup with 100+ms with
much bigger wins on Windows in the CI. While it does add a dependency on
libpq, I think it's a fairly decent price to pay for running tests faster.
--
Daniel Gustafsson
Attachments:
[application/octet-stream] v1-0001-Speed-up-pg_regress-server-testing.patch (10.2K, ../../[email protected]/2-v1-0001-Speed-up-pg_regress-server-testing.patch)
download | inline diff:
From eb71567159e488e9f6ba5596f18939dfb83b9078 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <[email protected]>
Date: Mon, 28 Aug 2023 10:54:25 +0200
Subject: [PATCH v1] Speed up pg_regress server testing
Instead of connecting to the server with psql to check if it is ready
for running tests, pg_regress now use PQPing which avoids performing
expensive system() calls on Windows.
The frequency of tests is also increased in order to connect to the
server faster.
This patch is part of a larger effort to make testing consume fewer
resources in order to be able to fit more tests into the available
CI constraints.
Discussion: https://postgr.es/m/[email protected]
---
src/interfaces/ecpg/test/Makefile | 3 +-
src/interfaces/ecpg/test/meson.build | 2 +-
src/test/isolation/Makefile | 2 +-
src/test/isolation/meson.build | 2 +-
src/test/regress/GNUmakefile | 4 +-
src/test/regress/meson.build | 2 +-
src/test/regress/pg_regress.c | 87 ++++++++++++++++++++--------
7 files changed, 70 insertions(+), 32 deletions(-)
diff --git a/src/interfaces/ecpg/test/Makefile b/src/interfaces/ecpg/test/Makefile
index cf841a3a5b..ba6ca837b3 100644
--- a/src/interfaces/ecpg/test/Makefile
+++ b/src/interfaces/ecpg/test/Makefile
@@ -10,6 +10,7 @@ include $(top_builddir)/src/Makefile.global
override CPPFLAGS := \
'-I$(top_builddir)/src/port' \
'-I$(top_srcdir)/src/test/regress' \
+ '-I$(libpq_srcdir)' \
'-DHOST_TUPLE="$(host_tuple)"' \
'-DSHELLPROG="$(SHELL)"' \
$(CPPFLAGS)
@@ -45,7 +46,7 @@ clean distclean maintainer-clean:
all: pg_regress$(X)
pg_regress$(X): pg_regress_ecpg.o $(WIN32RES) $(top_builddir)/src/test/regress/pg_regress.o
- $(CC) $(CFLAGS) $^ $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@
+ $(CC) $(CFLAGS) $^ $(libpq_pgport) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@
$(top_builddir)/src/test/regress/pg_regress.o:
$(MAKE) -C $(dir $@) $(notdir $@)
diff --git a/src/interfaces/ecpg/test/meson.build b/src/interfaces/ecpg/test/meson.build
index 04c6819a79..b7a3fb4e0e 100644
--- a/src/interfaces/ecpg/test/meson.build
+++ b/src/interfaces/ecpg/test/meson.build
@@ -18,7 +18,7 @@ pg_regress_ecpg = executable('pg_regress_ecpg',
pg_regress_ecpg_sources,
c_args: pg_regress_cflags,
include_directories: [pg_regress_inc, include_directories('.')],
- dependencies: [frontend_code],
+ dependencies: [frontend_code, libpq],
kwargs: default_bin_args + {
'install': false
},
diff --git a/src/test/isolation/Makefile b/src/test/isolation/Makefile
index b8738b7c1b..e99602ae52 100644
--- a/src/test/isolation/Makefile
+++ b/src/test/isolation/Makefile
@@ -38,7 +38,7 @@ pg_regress.o: | submake-regress
rm -f $@ && $(LN_S) $(top_builddir)/src/test/regress/pg_regress.o .
pg_isolation_regress$(X): isolation_main.o pg_regress.o $(WIN32RES)
- $(CC) $(CFLAGS) $^ $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@
+ $(CC) $(CFLAGS) $^ $(libpq_pgport) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@
isolationtester$(X): $(OBJS) | submake-libpq submake-libpgport
$(CC) $(CFLAGS) $^ $(libpq_pgport) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@
diff --git a/src/test/isolation/meson.build b/src/test/isolation/meson.build
index a4439e8ad0..e6ebe62c1e 100644
--- a/src/test/isolation/meson.build
+++ b/src/test/isolation/meson.build
@@ -35,7 +35,7 @@ pg_isolation_regress = executable('pg_isolation_regress',
isolation_sources,
c_args: pg_regress_cflags,
include_directories: pg_regress_inc,
- dependencies: frontend_code,
+ dependencies: [frontend_code, libpq],
kwargs: default_bin_args + {
'install_dir': dir_pgxs / 'src/test/isolation',
},
diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile
index 38c3a1f85b..db0687f867 100644
--- a/src/test/regress/GNUmakefile
+++ b/src/test/regress/GNUmakefile
@@ -36,11 +36,11 @@ EXTRADEFS = '-DHOST_TUPLE="$(host_tuple)"' \
all: pg_regress$(X)
pg_regress$(X): pg_regress.o pg_regress_main.o $(WIN32RES) | submake-libpgport
- $(CC) $(CFLAGS) $^ $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@
+ $(CC) $(CFLAGS) $^ $(libpq_pgport) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@
# dependencies ensure that path changes propagate
pg_regress.o: pg_regress.c $(top_builddir)/src/port/pg_config_paths.h
-pg_regress.o: override CPPFLAGS += -I$(top_builddir)/src/port $(EXTRADEFS)
+pg_regress.o: override CPPFLAGS += -I$(top_builddir)/src/port -I$(libpq_srcdir) $(EXTRADEFS)
# note: because of the submake dependency, this rule's action is really a no-op
$(top_builddir)/src/port/pg_config_paths.h: | submake-libpgport
diff --git a/src/test/regress/meson.build b/src/test/regress/meson.build
index a045c00c1f..f0dfd85591 100644
--- a/src/test/regress/meson.build
+++ b/src/test/regress/meson.build
@@ -30,7 +30,7 @@ endif
pg_regress = executable('pg_regress',
regress_sources,
c_args: pg_regress_cflags,
- dependencies: [frontend_code],
+ dependencies: [frontend_code, libpq],
kwargs: default_bin_args + {
'install_dir': dir_pgxs / 'src/test/regress',
},
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index ec67588cf5..601fbb33d3 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -32,6 +32,7 @@
#include "common/username.h"
#include "getopt_long.h"
#include "lib/stringinfo.h"
+#include "libpq-fe.h"
#include "libpq/pqcomm.h" /* needed for UNIXSOCK_PATH() */
#include "pg_config_paths.h"
#include "pg_regress.h"
@@ -75,6 +76,12 @@ const char *pretty_diff_opts = "-w -U3";
*/
#define TESTNAME_WIDTH 36
+/*
+ * The number times per second that pg_regress checks to see if the test
+ * instance server has started and is available for connection.
+ */
+#define WAIT_TICKS_PER_SECOND 20
+
typedef enum TAPtype
{
DIAG = 0,
@@ -107,6 +114,7 @@ static bool nolocale = false;
static bool use_existing = false;
static char *hostname = NULL;
static int port = -1;
+static char portstr[16];
static bool port_specified_by_user = false;
static char *dlpath = PKGLIBDIR;
static char *user = NULL;
@@ -2107,7 +2115,10 @@ regression_main(int argc, char *argv[],
int i;
int option_index;
char buf[MAXPGPATH * 4];
- char buf2[MAXPGPATH * 4];
+ instr_time starttime;
+ instr_time stoptime;
+
+ INSTR_TIME_SET_CURRENT(starttime);
pg_logging_init(argv[0]);
progname = get_progname(argv[0]);
@@ -2296,6 +2307,8 @@ regression_main(int argc, char *argv[],
const char *env_wait;
int wait_seconds;
const char *initdb_template_dir;
+ const char *keywords[4];
+ const char *values[4];
/*
* Prepare the temp instance
@@ -2435,22 +2448,30 @@ regression_main(int argc, char *argv[],
}
#endif
+ sprintf(portstr, "%d", port);
+
/*
- * Check if there is a postmaster running already.
+ * Prepare the connection params for checking the state of the server
+ * before starting the tests.
*/
- snprintf(buf2, sizeof(buf2),
- "\"%s%spsql\" -X postgres <%s 2>%s",
- bindir ? bindir : "",
- bindir ? "/" : "",
- DEVNULL, DEVNULL);
+ keywords[0] = "dbname";
+ values[0] = "postgres";
+ keywords[1] = "port";
+ values[1] = portstr;
+ keywords[2] = "host";
+ values[2] = hostname ? hostname : sockdir;
+ keywords[3] = NULL;
+ values[3] = NULL;
+ /*
+ * Check if there is a postmaster running already.
+ */
for (i = 0; i < 16; i++)
{
- fflush(NULL);
- if (system(buf2) == 0)
- {
- char s[16];
+ PGPing rv = PQpingParams(keywords, values, 1);
+ if (rv == PQPING_OK)
+ {
if (port_specified_by_user || i == 15)
{
note("port %d apparently in use", port);
@@ -2461,8 +2482,8 @@ regression_main(int argc, char *argv[],
note("port %d apparently in use, trying %d", port, port + 1);
port++;
- sprintf(s, "%d", port);
- setenv("PGPORT", s, 1);
+ sprintf(portstr, "%d", port);
+ setenv("PGPORT", portstr, 1);
}
else
break;
@@ -2485,11 +2506,11 @@ regression_main(int argc, char *argv[],
bail("could not spawn postmaster: %s", strerror(errno));
/*
- * Wait till postmaster is able to accept connections; normally this
- * is only a second or so, but Cygwin is reportedly *much* slower, and
- * test builds using Valgrind or similar tools might be too. Hence,
- * allow the default timeout of 60 seconds to be overridden from the
- * PGCTLTIMEOUT environment variable.
+ * Wait till postmaster is able to accept connections; normally takes
+ * only a fraction of a second or so, but Cygwin is reportedly *much*
+ * slower, and test builds using Valgrind or similar tools might be
+ * too. Hence, allow the default timeout of 60 seconds to be
+ * overridden from the PGCTLTIMEOUT environment variable.
*/
env_wait = getenv("PGCTLTIMEOUT");
if (env_wait != NULL)
@@ -2501,13 +2522,24 @@ regression_main(int argc, char *argv[],
else
wait_seconds = 60;
- for (i = 0; i < wait_seconds; i++)
+ for (i = 0; i < wait_seconds * WAIT_TICKS_PER_SECOND; i++)
{
- /* Done if psql succeeds */
- fflush(NULL);
- if (system(buf2) == 0)
+ /*
+ * It's fairly unlikely that the server is responding immediately
+ * so we start with sleeping before checking instead of the other
+ * way around.
+ */
+ pg_usleep(1000000L / WAIT_TICKS_PER_SECOND);
+
+ PGPing rv = PQpingParams(keywords, values, 1);
+
+ /* Done if the server is running and accepts connections */
+ if (rv == PQPING_OK)
break;
+ if (rv == PQPING_NO_ATTEMPT)
+ bail("attempting to connect to postmaster failed");
+
/*
* Fail immediately if postmaster has exited
*/
@@ -2520,10 +2552,8 @@ regression_main(int argc, char *argv[],
bail("postmaster failed, examine \"%s/log/postmaster.log\" for the reason",
outputdir);
}
-
- pg_usleep(1000000L);
}
- if (i >= wait_seconds)
+ if (i >= wait_seconds * WAIT_TICKS_PER_SECOND)
{
diag("postmaster did not respond within %d seconds, examine \"%s/log/postmaster.log\" for the reason",
wait_seconds, outputdir);
@@ -2582,6 +2612,13 @@ regression_main(int argc, char *argv[],
create_role(sl->str, dblist);
}
+ /*
+ * Report how much time we spent during instance setup.
+ */
+ INSTR_TIME_SET_CURRENT(stoptime);
+ INSTR_TIME_SUBTRACT(stoptime, starttime);
+ diag("Time to first test: %.0f ms", INSTR_TIME_GET_MILLISEC(stoptime));
+
/*
* Ready to run the tests
*/
--
2.32.1 (Apple Git-133)
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc?
2023-08-23 12:48 Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Daniel Gustafsson <[email protected]>
2023-08-23 19:22 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Andres Freund <[email protected]>
2023-08-23 20:11 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Daniel Gustafsson <[email protected]>
2023-08-23 21:02 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Tom Lane <[email protected]>
@ 2023-08-23 21:43 ` Andres Freund <[email protected]>
2023-08-23 21:55 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Tom Lane <[email protected]>
1 sibling, 1 reply; 13+ messages in thread
From: Andres Freund @ 2023-08-23 21:43 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; pgsql-hackers; Thomas Munro <[email protected]>; Justin Pryzby <[email protected]>; Nazir Bilal Yavuz <[email protected]>
Hi,
On 2023-08-23 17:02:51 -0400, Tom Lane wrote:
> Daniel Gustafsson <[email protected]> writes:
> > On 23 Aug 2023, at 21:22, Andres Freund <[email protected]> wrote:
> >> I think there's more effective ways to make this cheaper. The basic thing
> >> would be to use libpq instead of forking of psql to make a connection
> >> check.
>
> > I had it in my head that not using libpq in pg_regress was a deliberate choice,
> > but I fail to find a reference to it in the archives.
>
> I have a vague feeling that you are right about that. Perhaps the
> concern was that under "make installcheck", pg_regress might be
> using a build-tree copy of libpq rather than the one from the
> system under test. As long as we're just trying to ping the server,
> that shouldn't matter too much I think
Or perhaps the opposite? That an installcheck pg_regress run might use the
system libpq, which doesn't have the symbols, or such?
Either way, with a function like PQping(), which existing in well beyond the
supported branches, that shouldn't be an issue?
> ... unless we hit problems with, say, a different default port number or
> socket path compiled into one copy vs. the other? That seems like it's
> probably a "so don't do that" case, though.
If we were to find such a case, it seems we could just add whatever missing
parameter to the connection string? I think we would likely already hit such
problems though, the psql started by an installcheck pg_regress might use the
system libpq, I think?
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc?
2023-08-23 12:48 Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Daniel Gustafsson <[email protected]>
2023-08-23 19:22 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Andres Freund <[email protected]>
2023-08-23 20:11 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Daniel Gustafsson <[email protected]>
2023-08-23 21:02 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Tom Lane <[email protected]>
2023-08-23 21:43 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Andres Freund <[email protected]>
@ 2023-08-23 21:55 ` Tom Lane <[email protected]>
2023-08-23 22:06 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Andres Freund <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: Tom Lane @ 2023-08-23 21:55 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; pgsql-hackers; Thomas Munro <[email protected]>; Justin Pryzby <[email protected]>; Nazir Bilal Yavuz <[email protected]>
Andres Freund <[email protected]> writes:
> On 2023-08-23 17:02:51 -0400, Tom Lane wrote:
>> ... unless we hit problems with, say, a different default port number or
>> socket path compiled into one copy vs. the other? That seems like it's
>> probably a "so don't do that" case, though.
> If we were to find such a case, it seems we could just add whatever missing
> parameter to the connection string? I think we would likely already hit such
> problems though, the psql started by an installcheck pg_regress might use the
> system libpq, I think?
The trouble with that approach is that in "make installcheck", we
don't really want to assume we know what the installed libpq's default
connection parameters are. So we don't explicitly know where that
libpq will connect.
As I said, we might be able to start treating installed-libpq-not-
compatible-with-build as a "don't do it" case. Another idea is to try
to ensure that pg_regress uses the same libpq that the psql-under-test
does; but I'm not sure how to implement that.
regards, tom lane
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc?
2023-08-23 12:48 Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Daniel Gustafsson <[email protected]>
2023-08-23 19:22 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Andres Freund <[email protected]>
2023-08-23 20:11 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Daniel Gustafsson <[email protected]>
2023-08-23 21:02 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Tom Lane <[email protected]>
2023-08-23 21:43 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Andres Freund <[email protected]>
2023-08-23 21:55 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Tom Lane <[email protected]>
@ 2023-08-23 22:06 ` Andres Freund <[email protected]>
2023-08-23 22:32 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Tom Lane <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: Andres Freund @ 2023-08-23 22:06 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; pgsql-hackers; Thomas Munro <[email protected]>; Justin Pryzby <[email protected]>; Nazir Bilal Yavuz <[email protected]>
Hi,
On 2023-08-23 17:55:53 -0400, Tom Lane wrote:
> Andres Freund <[email protected]> writes:
> > On 2023-08-23 17:02:51 -0400, Tom Lane wrote:
> >> ... unless we hit problems with, say, a different default port number or
> >> socket path compiled into one copy vs. the other? That seems like it's
> >> probably a "so don't do that" case, though.
>
> > If we were to find such a case, it seems we could just add whatever missing
> > parameter to the connection string? I think we would likely already hit such
> > problems though, the psql started by an installcheck pg_regress might use the
> > system libpq, I think?
>
> The trouble with that approach is that in "make installcheck", we
> don't really want to assume we know what the installed libpq's default
> connection parameters are. So we don't explicitly know where that
> libpq will connect.
Stepping back: I don't think installcheck matters for the concrete use of
libpq we're discussing - the only time we wait for server startup is the
non-installcheck case.
There are other potential uses for libpq in pg_regress though - I'd e.g. like
to have a "monitoring" session open, which we could use to detect that the
server crashed (by waiting for the FD to be become invalid). Where the
connection default issue could matter more?
I was wondering if we could create an unambiguous connection info, but that
seems like it'd be hard to do, without creating cross version hazards.
> As I said, we might be able to start treating installed-libpq-not-
> compatible-with-build as a "don't do it" case. Another idea is to try
> to ensure that pg_regress uses the same libpq that the psql-under-test
> does; but I'm not sure how to implement that.
I don't think that's likely to work, psql could use a libpq with a different
soversion. We could dlopen() libpq, etc, but that seems way too complicated.
What's the reason we don't force psql to come from the same build as
pg_regress?
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc?
2023-08-23 12:48 Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Daniel Gustafsson <[email protected]>
2023-08-23 19:22 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Andres Freund <[email protected]>
2023-08-23 20:11 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Daniel Gustafsson <[email protected]>
2023-08-23 21:02 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Tom Lane <[email protected]>
2023-08-23 21:43 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Andres Freund <[email protected]>
2023-08-23 21:55 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Tom Lane <[email protected]>
2023-08-23 22:06 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Andres Freund <[email protected]>
@ 2023-08-23 22:32 ` Tom Lane <[email protected]>
2023-08-23 22:56 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Andres Freund <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: Tom Lane @ 2023-08-23 22:32 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; pgsql-hackers; Thomas Munro <[email protected]>; Justin Pryzby <[email protected]>; Nazir Bilal Yavuz <[email protected]>
Andres Freund <[email protected]> writes:
> On 2023-08-23 17:55:53 -0400, Tom Lane wrote:
>> The trouble with that approach is that in "make installcheck", we
>> don't really want to assume we know what the installed libpq's default
>> connection parameters are. So we don't explicitly know where that
>> libpq will connect.
> Stepping back: I don't think installcheck matters for the concrete use of
> libpq we're discussing - the only time we wait for server startup is the
> non-installcheck case.
Oh, that's an excellent point. So for the immediately proposed use-case,
there's no issue. (We don't have a mode where we try to start a server
using already-installed executables.)
> There are other potential uses for libpq in pg_regress though - I'd e.g. like
> to have a "monitoring" session open, which we could use to detect that the
> server crashed (by waiting for the FD to be become invalid). Where the
> connection default issue could matter more?
Meh. I don't find that idea compelling enough to justify adding
restrictions on what test scenarios will work. It's seldom hard to
tell from the test output whether the server crashed.
> I was wondering if we could create an unambiguous connection info, but that
> seems like it'd be hard to do, without creating cross version hazards.
Hmm, we don't expect the regression test suite to work against other
server versions, so maybe that could be made to work --- that is, we
could run the psql under test and get a full set of connection
parameters out of it? But I'm still not finding this worth the
trouble.
> What's the reason we don't force psql to come from the same build as
> pg_regress?
Because the point of installcheck is to check the installed binaries
--- including the installed psql and libpq.
(Thinks for a bit...) Maybe we should add pg_regress to the installed
fileset, and use that copy not the in-tree copy for installcheck?
Then we could assume it's using the same libpq as psql. IIRC there
have already been suggestions to do that for the benefit of PGXS
testing.
regards, tom lane
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc?
2023-08-23 12:48 Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Daniel Gustafsson <[email protected]>
2023-08-23 19:22 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Andres Freund <[email protected]>
2023-08-23 20:11 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Daniel Gustafsson <[email protected]>
2023-08-23 21:02 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Tom Lane <[email protected]>
2023-08-23 21:43 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Andres Freund <[email protected]>
2023-08-23 21:55 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Tom Lane <[email protected]>
2023-08-23 22:06 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Andres Freund <[email protected]>
2023-08-23 22:32 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Tom Lane <[email protected]>
@ 2023-08-23 22:56 ` Andres Freund <[email protected]>
2023-08-24 06:10 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: Andres Freund @ 2023-08-23 22:56 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; pgsql-hackers; Thomas Munro <[email protected]>; Justin Pryzby <[email protected]>; Nazir Bilal Yavuz <[email protected]>
Hi,
On 2023-08-23 18:32:26 -0400, Tom Lane wrote:
> Andres Freund <[email protected]> writes:
> > There are other potential uses for libpq in pg_regress though - I'd e.g. like
> > to have a "monitoring" session open, which we could use to detect that the
> > server crashed (by waiting for the FD to be become invalid). Where the
> > connection default issue could matter more?
>
> Meh. I don't find that idea compelling enough to justify adding
> restrictions on what test scenarios will work. It's seldom hard to
> tell from the test output whether the server crashed.
I find it pretty painful to wade through a several-megabyte regression.diffs
to find the cause of a crash. I think we ought to use
restart_after_crash=false, since after a crash there's no hope for the tests
to succeed, but even in that case, we end up with a lot of pointless contents
in regression.diffs. If we instead realized that we shouldn't start further
tests, we'd limit that by a fair bit.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc?
2023-08-23 12:48 Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Daniel Gustafsson <[email protected]>
2023-08-23 19:22 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Andres Freund <[email protected]>
2023-08-23 20:11 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Daniel Gustafsson <[email protected]>
2023-08-23 21:02 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Tom Lane <[email protected]>
2023-08-23 21:43 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Andres Freund <[email protected]>
2023-08-23 21:55 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Tom Lane <[email protected]>
2023-08-23 22:06 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Andres Freund <[email protected]>
2023-08-23 22:32 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Tom Lane <[email protected]>
2023-08-23 22:56 ` Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Andres Freund <[email protected]>
@ 2023-08-24 06:10 ` Peter Eisentraut <[email protected]>
0 siblings, 0 replies; 13+ messages in thread
From: Peter Eisentraut @ 2023-08-24 06:10 UTC (permalink / raw)
To: Andres Freund <[email protected]>; Tom Lane <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; pgsql-hackers; Thomas Munro <[email protected]>; Justin Pryzby <[email protected]>; Nazir Bilal Yavuz <[email protected]>
On 24.08.23 00:56, Andres Freund wrote:
> Hi,
>
> On 2023-08-23 18:32:26 -0400, Tom Lane wrote:
>> Andres Freund <[email protected]> writes:
>>> There are other potential uses for libpq in pg_regress though - I'd e.g. like
>>> to have a "monitoring" session open, which we could use to detect that the
>>> server crashed (by waiting for the FD to be become invalid). Where the
>>> connection default issue could matter more?
>>
>> Meh. I don't find that idea compelling enough to justify adding
>> restrictions on what test scenarios will work. It's seldom hard to
>> tell from the test output whether the server crashed.
>
> I find it pretty painful to wade through a several-megabyte regression.diffs
> to find the cause of a crash. I think we ought to use
> restart_after_crash=false, since after a crash there's no hope for the tests
> to succeed, but even in that case, we end up with a lot of pointless contents
> in regression.diffs. If we instead realized that we shouldn't start further
> tests, we'd limit that by a fair bit.
I once coded it up so that if the server crashes during a test, it would
wait until it recovers before running the next test. I found that
useful. I agree the current behavior is not useful in any case.
^ permalink raw reply [nested|flat] 13+ messages in thread
* [PATCH v13 3/8] Row pattern recognition patch (rewriter).
@ 2024-01-22 09:45 Tatsuo Ishii <[email protected]>
0 siblings, 0 replies; 13+ messages in thread
From: Tatsuo Ishii @ 2024-01-22 09:45 UTC (permalink / raw)
---
src/backend/utils/adt/ruleutils.c | 90 +++++++++++++++++++++++++++++++
1 file changed, 90 insertions(+)
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 0b2a164057..baded88201 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -428,6 +428,10 @@ static void get_rule_groupingset(GroupingSet *gset, List *targetlist,
bool omit_parens, deparse_context *context);
static void get_rule_orderby(List *orderList, List *targetList,
bool force_colno, deparse_context *context);
+static void get_rule_pattern(List *patternVariable, List *patternRegexp,
+ bool force_colno, deparse_context *context);
+static void get_rule_define(List *defineClause, List *patternVariables,
+ bool force_colno, deparse_context *context);
static void get_rule_windowclause(Query *query, deparse_context *context);
static void get_rule_windowspec(WindowClause *wc, List *targetList,
deparse_context *context);
@@ -6459,6 +6463,64 @@ get_rule_orderby(List *orderList, List *targetList,
}
}
+/*
+ * Display a PATTERN clause.
+ */
+static void
+get_rule_pattern(List *patternVariable, List *patternRegexp,
+ bool force_colno, deparse_context *context)
+{
+ StringInfo buf = context->buf;
+ const char *sep;
+ ListCell *lc_var, *lc_reg = list_head(patternRegexp);
+
+ sep = "";
+ appendStringInfoChar(buf, '(');
+ foreach(lc_var, patternVariable)
+ {
+ char *variable = strVal((String *) lfirst(lc_var));
+ char *regexp = NULL;
+
+ if (lc_reg != NULL)
+ {
+ regexp = strVal((String *) lfirst(lc_reg));
+ lc_reg = lnext(patternRegexp, lc_reg);
+ }
+
+ appendStringInfo(buf, "%s%s", sep, variable);
+ if (regexp != NULL)
+ appendStringInfoString(buf, regexp);
+
+ sep = " ";
+ }
+ appendStringInfoChar(buf, ')');
+}
+
+/*
+ * Display a DEFINE clause.
+ */
+static void
+get_rule_define(List *defineClause, List *patternVariables,
+ bool force_colno, deparse_context *context)
+{
+ StringInfo buf = context->buf;
+ const char *sep;
+ ListCell *lc_var, *lc_def;
+
+ sep = " ";
+ Assert(list_length(patternVariables) == list_length(defineClause));
+
+ forboth(lc_var, patternVariables, lc_def, defineClause)
+ {
+ char *varName = strVal(lfirst(lc_var));
+ TargetEntry *te = (TargetEntry *) lfirst(lc_def);
+
+ appendStringInfo(buf, "%s%s AS ", sep, varName);
+ get_rule_expr((Node *) te->expr, context, false);
+ sep = ",\n ";
+ }
+}
+
/*
* Display a WINDOW clause.
*
@@ -6596,6 +6658,34 @@ get_rule_windowspec(WindowClause *wc, List *targetList,
appendStringInfoString(buf, "EXCLUDE GROUP ");
else if (wc->frameOptions & FRAMEOPTION_EXCLUDE_TIES)
appendStringInfoString(buf, "EXCLUDE TIES ");
+ /* RPR */
+ if (wc->rpSkipTo == ST_NEXT_ROW)
+ appendStringInfoString(buf, "\n AFTER MATCH SKIP TO NEXT ROW ");
+ else if (wc->rpSkipTo == ST_PAST_LAST_ROW)
+ appendStringInfoString(buf, "\n AFTER MATCH SKIP PAST LAST ROW ");
+ else if (wc->rpSkipTo == ST_FIRST_VARIABLE)
+ appendStringInfo(buf, "\n AFTER MATCH SKIP TO FIRST %s ", wc->rpSkipVariable);
+ else if (wc->rpSkipTo == ST_LAST_VARIABLE)
+ appendStringInfo(buf, "\n AFTER MATCH SKIP TO LAST %s ", wc->rpSkipVariable);
+ else if (wc->rpSkipTo == ST_VARIABLE)
+ appendStringInfo(buf, "\n AFTER MATCH SKIP TO %s ", wc->rpSkipVariable);
+
+ if (wc->initial)
+ appendStringInfoString(buf, "\n INITIAL");
+
+ if (wc->patternVariable)
+ {
+ appendStringInfoString(buf, "\n PATTERN ");
+ get_rule_pattern(wc->patternVariable, wc->patternRegexp, false, context);
+ }
+
+ if (wc->defineClause)
+ {
+ appendStringInfoString(buf, "\n DEFINE\n");
+ get_rule_define(wc->defineClause, wc->patternVariable, false, context);
+ appendStringInfoChar(buf, ' ');
+ }
+
/* we will now have a trailing space; remove it */
buf->len--;
}
--
2.25.1
----Next_Part(Mon_Jan_22_19_26_18_2024_011)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v13-0004-Row-pattern-recognition-patch-planner.patch"
^ permalink raw reply [nested|flat] 13+ messages in thread
end of thread, other threads:[~2024-01-22 09:45 UTC | newest]
Thread overview: 13+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-08-23 12:48 Re: Cirrus-ci is lowering free CI cycles - what to do with cfbot, etc? Daniel Gustafsson <[email protected]>
2023-08-23 19:22 ` Andres Freund <[email protected]>
2023-08-23 20:11 ` Daniel Gustafsson <[email protected]>
2023-08-23 21:02 ` Tom Lane <[email protected]>
2023-08-23 21:12 ` Daniel Gustafsson <[email protected]>
2023-08-28 12:32 ` Daniel Gustafsson <[email protected]>
2023-08-23 21:43 ` Andres Freund <[email protected]>
2023-08-23 21:55 ` Tom Lane <[email protected]>
2023-08-23 22:06 ` Andres Freund <[email protected]>
2023-08-23 22:32 ` Tom Lane <[email protected]>
2023-08-23 22:56 ` Andres Freund <[email protected]>
2023-08-24 06:10 ` Peter Eisentraut <[email protected]>
2024-01-22 09:45 [PATCH v13 3/8] Row pattern recognition patch (rewriter). Tatsuo Ishii <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox