agora inbox for [email protected]
help / color / mirror / Atom feedRe: make installcheck-world in a clean environment
11+ messages / 6 participants
[nested] [flat]
* Re: make installcheck-world in a clean environment
@ 2018-09-12 07:20 Michael Paquier <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Michael Paquier @ 2018-09-12 07:20 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Robert Haas <[email protected]>; Alexander Lakhin <[email protected]>; pgsql-hackers
On Mon, May 07, 2018 at 01:07:15PM -0400, Tom Lane wrote:
> Robert Haas <[email protected]> writes:
>> After thinking about this some more, I think the question here is
>> definitional. A first attempt at defining 'make installcheck' is to
>> say that it runs the tests from the build tree against the running
>> server. Certainly, we intend to use the SQL files that are present in
>> the build tree, not the ones that were present in the build tree where
>> the running server was built. But what about client programs that we
>> use to connect to the server? You're suggesting that we use the
>> pre-installed ones, but that is actually pretty problematic because
>> the ones we see as installed might correspond neither to the contents
>> of the build tree nor to the running server. Conceivably we could end
>> up having a mix of assets from three different places: (1) the running
>> server, (2) the build tree, (3) whatever is in our path at the moment.
>> That seems very confusing. So now I think it's probably right to
>> define 'make installcheck' as using the assets from the build tree to
>> test the running server. Under that definition, we're missing some
>> dependencies, but USE_INSTALLED_ASSETS isn't a thing we need.
>
> Nah, I disagree with this. To me, the purpose of "make installcheck"
> is to verify the correctness of an installation, which I take to include
> the client programs as well as the server. I think that "make
> installcheck" ought to use the installed version of any file that we
> actually install, and go to the build tree only for things we don't
> install (e.g. SQL test scripts).
>
> If the user has screwed up his PATH or other environmental aspects so that
> what he's testing isn't a single installation, that's his error, not
> something that "make installcheck" ought to work around. Indeed, maybe
> such aspects of his setup are intentional, and second-guessing them would
> completely defeat his purpose. In any case, if you want to test the
> build-tree assets, that's what "make check" is for.
I agree with Tom's position, and this is the behavior that Postgres is
using for ages for check and installcheck. If there are no objections,
I'll mark the patch as rejected and move on to other things.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: make installcheck-world in a clean environment
@ 2018-09-12 08:26 Alexander Lakhin <[email protected]>
parent: Michael Paquier <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Alexander Lakhin @ 2018-09-12 08:26 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; Tom Lane <[email protected]>; +Cc: Robert Haas <[email protected]>; pgsql-hackers
Hello Michael,
12.09.2018 10:20, Michael Paquier wrote:
> On Mon, May 07, 2018 at 01:07:15PM -0400, Tom Lane wrote:
>> Robert Haas <[email protected]> writes:
>>> After thinking about this some more, I think the question here is
>>> definitional. A first attempt at defining 'make installcheck' is to
>>> say that it runs the tests from the build tree against the running
>>> server. Certainly, we intend to use the SQL files that are present in
>>> the build tree, not the ones that were present in the build tree where
>>> the running server was built. But what about client programs that we
>>> use to connect to the server? You're suggesting that we use the
>>> pre-installed ones, but that is actually pretty problematic because
>>> the ones we see as installed might correspond neither to the contents
>>> of the build tree nor to the running server. Conceivably we could end
>>> up having a mix of assets from three different places: (1) the running
>>> server, (2) the build tree, (3) whatever is in our path at the moment.
>>> That seems very confusing. So now I think it's probably right to
>>> define 'make installcheck' as using the assets from the build tree to
>>> test the running server. Under that definition, we're missing some
>>> dependencies, but USE_INSTALLED_ASSETS isn't a thing we need.
>> Nah, I disagree with this. To me, the purpose of "make installcheck"
>> is to verify the correctness of an installation, which I take to include
>> the client programs as well as the server. I think that "make
>> installcheck" ought to use the installed version of any file that we
>> actually install, and go to the build tree only for things we don't
>> install (e.g. SQL test scripts).
>>
>> If the user has screwed up his PATH or other environmental aspects so that
>> what he's testing isn't a single installation, that's his error, not
>> something that "make installcheck" ought to work around. Indeed, maybe
>> such aspects of his setup are intentional, and second-guessing them would
>> completely defeat his purpose. In any case, if you want to test the
>> build-tree assets, that's what "make check" is for.
> I agree with Tom's position, and this is the behavior that Postgres is
> using for ages for check and installcheck. If there are no objections,
> I'll mark the patch as rejected and move on to other things.
> --
> Michael
It seems, that you miss a major part of the discussion (we have
discussed the issue from other positions later).
And I don't think that age of the behavior should prevail over it's
reasonability.
Best regards,
------
Alexander Lakhin
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: make installcheck-world in a clean environment
@ 2018-11-16 01:19 Tom Lane <[email protected]>
parent: Alexander Lakhin <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Tom Lane @ 2018-11-16 01:19 UTC (permalink / raw)
To: Alexander Lakhin <[email protected]>; +Cc: Michael Paquier <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
Alexander Lakhin <[email protected]> writes:
> 12.09.2018 10:20, Michael Paquier wrote:
>> On Mon, May 07, 2018 at 01:07:15PM -0400, Tom Lane wrote:
>>> Nah, I disagree with this. To me, the purpose of "make installcheck"
>>> is to verify the correctness of an installation, which I take to include
>>> the client programs as well as the server. I think that "make
>>> installcheck" ought to use the installed version of any file that we
>>> actually install, and go to the build tree only for things we don't
>>> install (e.g. SQL test scripts).
>> I agree with Tom's position, and this is the behavior that Postgres is
>> using for ages for check and installcheck. If there are no objections,
>> I'll mark the patch as rejected and move on to other things.
> It seems, that you miss a major part of the discussion (we have
> discussed the issue from other positions later).
I think the main reason this patch isn't moving forward is that it's not
clear to most people what you're trying to fix. And I'd lay a big part of
the blame for that on the fact that the patch includes no documentation
changes at all, not even additional Makefile comments. Perhaps the SGML
text about how to use "make installcheck" needs to be expanded to clarify
what we expect to be already installed.
The patch itself contains some pretty dubious/confusing things too.
For instance, the first hunk:
@@ -244,7 +244,13 @@ CPPFLAGS = @CPPFLAGS@
override CPPFLAGS := $(ICU_CFLAGS) $(CPPFLAGS)
+ifdef USE_INSTALLED_ASSETS
+USE_INCLUDEDIR = 1
+endif
ifdef PGXS
+USE_INCLUDEDIR = 1
+endif
+ifdef USE_INCLUDEDIR
override CPPFLAGS := -I$(includedir_server) -I$(includedir_internal) $(CPPFLAGS)
else # not PGXS
override CPPFLAGS := -I$(top_srcdir)/src/include $(CPPFLAGS)
immediately leads the reader to wonder where else USE_INCLUDEDIR is used,
and the answer is nowhere, so why'd you define it? I think it'd be better
to replace the first seven lines with the usual locution for OR:
ifneq (,$(PGXS)$(USE_INSTALLED_ASSETS))
and likewise further down for USE_LIBDIR. I also note you didn't fix the
comment you falsified about "# not PGXS".
It seems wrong to have changed src/interfaces/ecpg/Makefile the way you
did. Surely it is the responsibility of src/interfaces/ecpg/test/Makefile
to handle a "make installcheck" request correctly, whether it is issued
from the parent directory or locally. (Personally I do "make
installcheck" in the test/ subdirectory quite often, so I'd be unhappy
if it doesn't work right when started from there.)
The change in pgxs.mk doesn't make a lot of sense to me either; even if it's
not actually syntactically wrong, what's the point given what you did in
Makefile.global?
I do not believe that the changes in either plpgsql/src or test/isolation
are appropriate. If the former is needed then it would at least imply
that plperl, plpython, and pltcl are all broken too, and probably also
that all the contrib makefiles are broken, and I don't believe any of
that. As for test/isolation, there is nothing that it installs, so why
would it need a change in behavior?
Nor do I understand the need for changes in test/regress. I'm prepared to
believe that ECPG might need some work, because it's got a complicated
situation and few people pay any attention to it anyway. But all this
other stuff works perfectly fine under "make installcheck" today, and
has done for years, and you have not explained why changing it would
be an improvement.
regards, tom lane
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: make installcheck-world in a clean environment
@ 2018-11-18 07:30 Alexander Lakhin <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Alexander Lakhin @ 2018-11-18 07:30 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Michael Paquier <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
Hello Tom,
Thank you for the very detailed review.
Please look at the improved version of the patch (and the `make
installcheck` log with it).
16.11.2018 04:19, Tom Lane writes:
>> It seems, that you miss a major part of the discussion (we have
>> discussed the issue from other positions later).
> I think the main reason this patch isn't moving forward is that it's not
> clear to most people what you're trying to fix. And I'd lay a big part of
> the blame for that on the fact that the patch includes no documentation
> changes at all, not even additional Makefile comments. Perhaps the SGML
> text about how to use "make installcheck" needs to be expanded to clarify
> what we expect to be already installed.
Now I've tried to describe in regress.sgml the behavior, which I want to
implement.
I'm not sure that we should name all the assets, that we expect to be
installed, because it can change.
> The patch itself contains some pretty dubious/confusing things too.
> For instance, the first hunk:
>
> @@ -244,7 +244,13 @@ CPPFLAGS = @CPPFLAGS@
>
> override CPPFLAGS := $(ICU_CFLAGS) $(CPPFLAGS)
>
> +ifdef USE_INSTALLED_ASSETS
> +USE_INCLUDEDIR = 1
> +endif
> ifdef PGXS
> +USE_INCLUDEDIR = 1
> +endif
> +ifdef USE_INCLUDEDIR
> override CPPFLAGS := -I$(includedir_server) -I$(includedir_internal) $(CPPFLAGS)
> else # not PGXS
> override CPPFLAGS := -I$(top_srcdir)/src/include $(CPPFLAGS)
>
> immediately leads the reader to wonder where else USE_INCLUDEDIR is used,
> and the answer is nowhere, so why'd you define it? I think it'd be better
> to replace the first seven lines with the usual locution for OR:
>
> ifneq (,$(PGXS)$(USE_INSTALLED_ASSETS))
>
> and likewise further down for USE_LIBDIR. I also note you didn't fix the
> comment you falsified about "# not PGXS".
Thanks for the notice. I fixed it.
> It seems wrong to have changed src/interfaces/ecpg/Makefile the way you
> did. Surely it is the responsibility of src/interfaces/ecpg/test/Makefile
> to handle a "make installcheck" request correctly, whether it is issued
> from the parent directory or locally. (Personally I do "make
> installcheck" in the test/ subdirectory quite often, so I'd be unhappy
> if it doesn't work right when started from there.)
Thanks again, I've moved the change into ecpg/test.
> The change in pgxs.mk doesn't make a lot of sense to me either; even if it's
> not actually syntactically wrong, what's the point given what you did in
> Makefile.global?
I've modified the patch to use the installed version of pg_regress. It
simplifies a lot. The main idea of the change is to not build pg_regress.
> I do not believe that the changes in either plpgsql/src or test/isolation
> are appropriate. If the former is needed then it would at least imply
> that plperl, plpython, and pltcl are all broken too, and probably also
> that all the contrib makefiles are broken, and I don't believe any of
> that. As for test/isolation, there is nothing that it installs, so why
> would it need a change in behavior?
Regarding test/isolation, we need to change the behavior to build
pg_isolation_regress and isolationtester only. We don't need to build
all the libraries needed as they are already installed in $(DESTDIR)/lib/.
By the way, I wonder why "pg_isolation_regress" and "isolationtester"
binaries are not installed as "pg_regress" is.
As to plpgsql and other PLs, yes, it was my omission. We don't need to
build pg_regress when we use $(pg_regress_installcheck) for any of these,
In contrib/ I've found only one usage of pg_regress_installcheck
(contrib/test_decoding). Fixed it too. We don't need to build the
test-decoding lib as it's installed too.
Also there is an issue with src/test/regress/. We install refint.so and
autoinc.so into $(DESTDIR)/lib/, but don't install regress.so. It makes
impossible to pass "--dlpath='$(DESTDIR)$(libdir)/'" to pg_regress. So
for now I leave dlpath unchanged and it requires building of all the .so's.
> Nor do I understand the need for changes in test/regress. I'm prepared to
> believe that ECPG might need some work, because it's got a complicated
> situation and few people pay any attention to it anyway. But all this
> other stuff works perfectly fine under "make installcheck" today, and
> has done for years, and you have not explained why changing it would
> be an improvement.
The main purpose of the change is to make it possible to perform `make
installcheck` for the installation produced by the binary packages
(pgdg, for example).
The scenario is simple. If I've installed binary packages, how can I
check that the installation is working as expected? (Assuming that I
have corresponding source tarball.)
Currently, I can remove bin/ecpg, lib/libgport, lib/pgxs/, include/ from
the installation directory and `make installcheck-world` will succeed.
But it will not, if I remove bin/psql or lib/adminpack.so.
So the current behavior is at least inconsistent but moreover it's
unsuitable for the user-centric testing of installation. I do understand
that the developer-centric approach was working for years, but may be it
can (or should) be changed someday?
Best regards,
Alexander
Attachments:
[text/x-patch] make-installcheck-v3.patch (12.4K, ../../[email protected]/2-make-installcheck-v3.patch)
download | inline diff:
diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile
index afcab93..fd0e0ee 100644
--- a/contrib/test_decoding/Makefile
+++ b/contrib/test_decoding/Makefile
@@ -46,7 +46,7 @@ regresscheck: | submake-regress submake-test_decoding temp-install
--temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \
$(REGRESSCHECKS)
-regresscheck-install-force: | submake-regress submake-test_decoding temp-install
+regresscheck-install-force: | temp-install
$(pg_regress_installcheck) \
$(REGRESSCHECKS)
@@ -58,7 +58,7 @@ isolationcheck: | submake-isolation submake-test_decoding temp-install
--temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \
$(ISOLATIONCHECKS)
-isolationcheck-install-force: all | submake-isolation submake-test_decoding temp-install
+isolationcheck-install-force: | submake-isolation temp-install
$(pg_isolation_regress_installcheck) \
$(ISOLATIONCHECKS)
diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml
index 673a8c2..9444dee 100644
--- a/doc/src/sgml/regress.sgml
+++ b/doc/src/sgml/regress.sgml
@@ -111,7 +111,12 @@ make installcheck-parallel
default port number, unless directed otherwise by <envar>PGHOST</envar> and
<envar>PGPORT</envar> environment variables. The tests will be run in a
database named <literal>regression</literal>; any existing database by this name
- will be dropped.
+ will be dropped. The tests in the installcheck mode will use the executables,
+ libraries, and headers, which are already present in the installation tree,
+ not their copies in the source tree (if any). This mode is suitable for
+ testing distribution packages of PostgreSQL installed on user systems as
+ it allows to check the installation in its entirety without rebuilding
+ any parts of it.
</para>
<para>
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 956fd27..3f5b779 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -245,14 +245,16 @@ PG_SYSROOT = @PG_SYSROOT@
override CPPFLAGS := $(ICU_CFLAGS) $(CPPFLAGS)
-ifdef PGXS
+# For PGXS and in the installcheck mode (when we use the installed assets)
+# we need to target already installed headers and libraries
+ifneq (,$(PGXS)$(USE_INSTALLED_ASSETS))
override CPPFLAGS := -I$(includedir_server) -I$(includedir_internal) $(CPPFLAGS)
-else # not PGXS
+else # not PGXS and not USE_INSTALLED_ASSETS
override CPPFLAGS := -I$(top_srcdir)/src/include $(CPPFLAGS)
ifdef VPATH
override CPPFLAGS := -I$(top_builddir)/src/include $(CPPFLAGS)
endif
-endif # not PGXS
+endif # not PGXS and USE_INSTALLED_ASSETS
CC = @CC@
GCC = @GCC@
@@ -306,7 +308,7 @@ with_gnu_ld = @with_gnu_ld@
# "LDFLAGS := something" anywhere, ditto for LDFLAGS_INTERNAL.
# These initial assignments must be "=" type, and elsewhere we must only do
# "LDFLAGS += something" or "LDFLAGS_INTERNAL += something".
-ifdef PGXS
+ifneq (,$(PGXS)$(USE_INSTALLED_ASSETS))
LDFLAGS_INTERNAL = -L$(libdir)
else
LDFLAGS_INTERNAL = -L$(top_builddir)/src/port -L$(top_builddir)/src/common
@@ -379,6 +381,9 @@ endif
# install tree just once in any recursive "make check". The additional test
# on abs_top_builddir prevents doing anything foolish to the root directory.
+# In the installcheck mode we should use what is already installed
+# in the DESTDIR (namely, pg_regress).
+
check: temp-install
.PHONY: temp-install
@@ -421,7 +426,7 @@ ifeq ($(enable_tap_tests),yes)
define prove_installcheck
rm -rf '$(CURDIR)'/tmp_check
$(MKDIR_P) '$(CURDIR)'/tmp_check
-cd $(srcdir) && TESTDIR='$(CURDIR)' PATH="$(bindir):$$PATH" PGPORT='6$(DEF_PGPORT)' top_builddir='$(CURDIR)/$(top_builddir)' PG_REGRESS='$(CURDIR)/$(top_builddir)/src/test/regress/pg_regress' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) $(if $(PROVE_TESTS),$(PROVE_TESTS),t/*.pl)
+cd $(srcdir) && TESTDIR='$(CURDIR)' PATH="$(bindir):$$PATH" PGPORT='6$(DEF_PGPORT)' top_builddir='$(CURDIR)/$(top_builddir)' PG_REGRESS='$(DESTDIR)$(pgxsdir)/src/test/regress/pg_regress' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) $(if $(PROVE_TESTS),$(PROVE_TESTS),t/*.pl)
endef
define prove_check
@@ -564,6 +569,8 @@ endif
#
# Commonly used submake targets
+# build these libraries only when we are not going to use the installed ones
+ifndef USE_INSTALLED_ASSETS
submake-libpq: | submake-generated-headers
$(MAKE) -C $(libpq_builddir) all
@@ -575,6 +582,7 @@ submake-libpgfeutils: | submake-generated-headers
$(MAKE) -C $(top_builddir)/src/port all
$(MAKE) -C $(top_builddir)/src/common all
$(MAKE) -C $(top_builddir)/src/fe_utils all
+endif
.PHONY: submake-libpq submake-libpgport submake-libpgfeutils
@@ -617,7 +625,7 @@ pg_regress_check = \
$(TEMP_CONF) \
$(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS)
pg_regress_installcheck = \
- $(top_builddir)/src/test/regress/pg_regress \
+ '$(DESTDIR)$(pgxsdir)/src/test/regress/pg_regress' \
--inputdir=$(srcdir) \
--bindir='$(bindir)' \
$(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS)
diff --git a/src/interfaces/ecpg/test/Makefile b/src/interfaces/ecpg/test/Makefile
index c761a4d..0b12d1b 100644
--- a/src/interfaces/ecpg/test/Makefile
+++ b/src/interfaces/ecpg/test/Makefile
@@ -86,7 +86,11 @@ check: all
checktcp: all | temp-install
$(with_temp_install) ./pg_regress $(REGRESS_OPTS) --temp-instance=./tmp_check $(TEMP_CONF) --bindir= $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule --host=localhost sql/twophase connect/test1
-installcheck: all
+.PHONY: all-installcheck
+all-installcheck:
+ USE_INSTALLED_ASSETS=1 $(MAKE) all
+
+installcheck: all-installcheck
./pg_regress $(REGRESS_OPTS) --bindir='$(bindir)' $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule
# Versions of the check tests that include the twophase commit test.
diff --git a/src/interfaces/ecpg/test/Makefile.regress b/src/interfaces/ecpg/test/Makefile.regress
index 4da1bb8..99fb6fd 100644
--- a/src/interfaces/ecpg/test/Makefile.regress
+++ b/src/interfaces/ecpg/test/Makefile.regress
@@ -1,19 +1,29 @@
# This file is included into the Makefiles of subdirectories of ecpg/test/,
# so the file references have one more level of .. than you might expect.
+ifdef USE_INSTALLED_ASSETS
+# Standard way to invoke the installed ecpg preprocessor
+ECPG = '$(DESTDIR)$(bindir)/ecpg' --regression -I'$(DESTDIR)$(includedir)' \
+ -I'$(DESTDIR)$(pkgincludedir)/informix/esql'
+# Use installed headers and libs
+override CPPFLAGS := -I'$(DESTDIR)$(includedir)' \
+ -I'$(DESTDIR)$(pkgincludedir)/informix/esql' $(CPPFLAGS)
+LDFLAGS_INTERNAL += -lecpg -lpgtypes $(libpq)
+else
+# Standard way to invoke the ecpg preprocessor
+ECPG = ../../preproc/ecpg --regression -I$(srcdir)/../../include -I$(srcdir)
+
+ECPG_DEP = ../../preproc/ecpg$(X)
override CPPFLAGS := -I../../include -I$(top_srcdir)/src/interfaces/ecpg/include \
-I$(libpq_srcdir) $(CPPFLAGS)
-override CFLAGS += $(PTHREAD_CFLAGS)
-
LDFLAGS_INTERNAL += -L../../ecpglib -lecpg -L../../pgtypeslib -lpgtypes $(libpq)
+endif
+override CFLAGS += $(PTHREAD_CFLAGS)
override LIBS += $(PTHREAD_LIBS)
-# Standard way to invoke the ecpg preprocessor
-ECPG = ../../preproc/ecpg --regression -I$(srcdir)/../../include -I$(srcdir)
-
# Files that most or all ecpg preprocessor test outputs depend on
-ECPG_TEST_DEPENDENCIES = ../../preproc/ecpg$(X) \
+ECPG_TEST_DEPENDENCIES = $(ECPG_DEP) \
$(srcdir)/../regression.h \
$(srcdir)/../printf_hack.h \
$(srcdir)/../../include/sqlca.h \
diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk
index 070d151..1edef06 100644
--- a/src/makefiles/pgxs.mk
+++ b/src/makefiles/pgxs.mk
@@ -392,7 +392,7 @@ endif
# against installed postmaster
ifndef NO_INSTALLCHECK
-installcheck: submake $(REGRESS_PREP)
+installcheck: $(REGRESS_PREP)
$(pg_regress_installcheck) $(REGRESS_OPTS) $(REGRESS)
endif
diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile
index 9b1c514..da605f8 100644
--- a/src/pl/plperl/GNUmakefile
+++ b/src/pl/plperl/GNUmakefile
@@ -113,7 +113,7 @@ uninstall-data:
check: submake
$(pg_regress_check) $(REGRESS_OPTS) $(REGRESS)
-installcheck: submake
+installcheck:
$(pg_regress_installcheck) $(REGRESS_OPTS) $(REGRESS)
.PHONY: submake
diff --git a/src/pl/plpgsql/src/Makefile b/src/pl/plpgsql/src/Makefile
index 25a5a9d..5285eda 100644
--- a/src/pl/plpgsql/src/Makefile
+++ b/src/pl/plpgsql/src/Makefile
@@ -76,7 +76,7 @@ plerrcodes.h: $(top_srcdir)/src/backend/utils/errcodes.txt generate-plerrcodes.p
check: submake
$(pg_regress_check) $(REGRESS_OPTS) $(REGRESS)
-installcheck: submake
+installcheck:
$(pg_regress_installcheck) $(REGRESS_OPTS) $(REGRESS)
.PHONY: submake
diff --git a/src/pl/plpython/Makefile b/src/pl/plpython/Makefile
index 667a744..839f57d 100644
--- a/src/pl/plpython/Makefile
+++ b/src/pl/plpython/Makefile
@@ -142,7 +142,7 @@ include $(srcdir)/regress-python3-mangle.mk
check: submake-pg-regress
$(pg_regress_check) $(REGRESS_OPTS) $(REGRESS)
-installcheck: submake-pg-regress
+installcheck:
$(pg_regress_installcheck) $(REGRESS_OPTS) $(REGRESS)
diff --git a/src/pl/tcl/Makefile b/src/pl/tcl/Makefile
index ef61ee5..cc6216d 100644
--- a/src/pl/tcl/Makefile
+++ b/src/pl/tcl/Makefile
@@ -82,7 +82,7 @@ uninstall-data:
check: submake
$(pg_regress_check) $(REGRESS_OPTS) $(REGRESS)
-installcheck: submake
+installcheck:
$(pg_regress_installcheck) $(REGRESS_OPTS) $(REGRESS)
.PHONY: submake
diff --git a/src/test/isolation/Makefile b/src/test/isolation/Makefile
index c3c8280..4c3a937 100644
--- a/src/test/isolation/Makefile
+++ b/src/test/isolation/Makefile
@@ -15,6 +15,9 @@ OBJS = specparse.o isolationtester.o $(WIN32RES)
all: isolationtester$(X) pg_isolation_regress$(X)
+all-installcheck:
+ USE_INSTALLED_ASSETS=1 $(MAKE) all
+
# Though we don't install these binaries, build them during installation
# (including temp-install). Otherwise, "make -j check-world" and "make -j
# installcheck-world" would spawn multiple, concurrent builds in this
@@ -49,7 +52,7 @@ clean distclean:
maintainer-clean: distclean
rm -f specparse.c specscanner.c
-installcheck: all
+installcheck: all-installcheck
$(pg_isolation_regress_installcheck) --schedule=$(srcdir)/isolation_schedule
check: all
@@ -59,7 +62,7 @@ check: all
# It only makes sense to run these if set up to use prepared transactions,
# via TEMP_CONFIG for the check case, or via the postgresql.conf for the
# installcheck case.
-installcheck-prepared-txns: all temp-install
+installcheck-prepared-txns: all-installcheck temp-install
$(pg_isolation_regress_installcheck) --schedule=$(srcdir)/isolation_schedule prepared-transactions
check-prepared-txns: all temp-install
diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile
index 378bd01..595ffc8 100644
--- a/src/test/regress/GNUmakefile
+++ b/src/test/regress/GNUmakefile
@@ -34,11 +34,17 @@ EXTRADEFS = '-DHOST_TUPLE="$(host_tuple)"' \
# Build regression test driver
+ifndef USE_INSTALLED_ASSETS
all: pg_regress$(X)
+endif
pg_regress$(X): pg_regress.o pg_regress_main.o $(WIN32RES) | submake-libpgport
$(CC) $(CFLAGS) $^ $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@
+.PHONY: all-installcheck
+all-installcheck:
+ USE_INSTALLED_ASSETS=1 $(MAKE) all
+
# 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)
@@ -134,16 +140,16 @@ check: all tablespace-setup
check-tests: all tablespace-setup | temp-install
$(pg_regress_check) $(REGRESS_OPTS) $(MAXCONNOPT) $(TESTS) $(EXTRA_TESTS)
-installcheck: all tablespace-setup
+installcheck: all-installcheck tablespace-setup
$(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/serial_schedule $(EXTRA_TESTS)
-installcheck-parallel: all tablespace-setup
+installcheck-parallel: all-installcheck tablespace-setup
$(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(EXTRA_TESTS)
-installcheck-tests: all tablespace-setup
+installcheck-tests: all-installcheck tablespace-setup
$(pg_regress_installcheck) $(REGRESS_OPTS) $(TESTS) $(EXTRA_TESTS)
-standbycheck: all
+standbycheck: all-installcheck
$(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/standby_schedule --use-existing
# old interfaces follow...
@@ -152,7 +158,7 @@ runcheck: check
runtest: installcheck
runtest-parallel: installcheck-parallel
-bigtest: all tablespace-setup
+bigtest: all-installcheck tablespace-setup
$(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/serial_schedule numeric_big
bigcheck: all tablespace-setup | temp-install
[text/x-log] installcheck.log (150.8K, ../../[email protected]/3-installcheck.log)
download
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: make installcheck-world in a clean environment
@ 2018-11-30 20:59 Dmitry Dolgov <[email protected]>
parent: Alexander Lakhin <[email protected]>
0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Dolgov @ 2018-11-30 20:59 UTC (permalink / raw)
To: [email protected]; +Cc: Tom Lane <[email protected]>; Michael Paquier <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
> On Sun, Nov 18, 2018 at 8:31 AM Alexander Lakhin <[email protected]> wrote:
>
> I've modified the patch to use the installed version of pg_regress. It
> simplifies a lot. The main idea of the change is to not build pg_regress.
Hi,
I've noticed that for this patch cfbot show strange error
USE_INSTALLED_ASSETS=1 make all
make[2]: Entering directory
`/home/travis/build/postgresql-cfbot/postgresql/src/test/regress'
make -C ../../../contrib/spi
make[3]: Entering directory
`/home/travis/build/postgresql-cfbot/postgresql/contrib/spi'
make[3]: Nothing to be done for `all'.
make[3]: Leaving directory
`/home/travis/build/postgresql-cfbot/postgresql/contrib/spi'
make[2]: Leaving directory
`/home/travis/build/postgresql-cfbot/postgresql/src/test/regress'
rm -rf ./testtablespace
mkdir ./testtablespace
'/usr/local/pgsql/lib/pgxs/src/test/regress/pg_regress' --inputdir=.
--bindir='/usr/local/pgsql/bin'
--bindir='/home/travis/build/postgresql-cfbot/postgresql/src/bin/pg_upgrade/tmp_check/install//usr/local/pgsql/bin'
--port=54464 --dlpath=. --max-concurrent-tests=20
--bindir='/home/travis/build/postgresql-cfbot/postgresql/src/bin/pg_upgrade/tmp_check/install//usr/local/pgsql/bin'
--port=54464 --schedule=./parallel_schedule
make[1]: /usr/local/pgsql/lib/pgxs/src/test/regress/pg_regress:
Command not found
make[1]: *** [installcheck-parallel] Error 127
make[1]: Leaving directory
`/home/travis/build/postgresql-cfbot/postgresql/src/test/regress'
make: *** [installcheck-parallel] Error 2
make: Leaving directory `/home/travis/build/postgresql-cfbot/postgresql'
The same error I've got when tried to run it on my laptop, is it something
expected?
^ permalink raw reply [nested|flat] 11+ messages in thread
* [PATCH 06/10] Make XlogReadTwoPhaseData not use callback but call the function directly
@ 2019-04-18 06:02 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 11+ messages in thread
From: Kyotaro Horiguchi @ 2019-04-18 06:02 UTC (permalink / raw)
This patch replaces the call to the callback in XlogReadTwoPhaseData
with direct call to the original function. Then invalidate the
parameters callback and private for XLogReaderAllocate.
---
src/backend/access/transam/twophase.c | 8 ++------
src/backend/access/transam/xlogutils.c | 8 +++++---
src/backend/replication/logical/logicalfuncs.c | 8 ++++++--
src/include/access/xlogutils.h | 5 +----
4 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index a3573ad0af..caaa09d785 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1386,8 +1386,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
XLogReaderState *xlogreader;
char *errormsg;
- xlogreader = XLogReaderAllocate(wal_segment_size, &read_local_xlog_page,
- NULL);
+ xlogreader = XLogReaderAllocate(wal_segment_size, NULL, NULL);
if (!xlogreader)
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
@@ -1396,10 +1395,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
while (XLogReadRecord(xlogreader, lsn, &record, &errormsg) ==
XLREAD_NEED_DATA)
- xlogreader->read_page(xlogreader,
- xlogreader->loadPagePtr, xlogreader->loadLen,
- xlogreader->currRecPtr, xlogreader->readBuf,
- &xlogreader->readPageTLI);
+ read_local_xlog_page(xlogreader);
if (record == NULL)
ereport(ERROR,
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index c853e1f0e3..fd461f16fc 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -908,10 +908,12 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
* loop for now.
*/
void
-read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
- int reqLen, XLogRecPtr targetRecPtr, char *cur_page,
- TimeLineID *pageTLI)
+read_local_xlog_page(XLogReaderState *state)
{
+ XLogRecPtr targetPagePtr = state->loadPagePtr;
+ int reqLen = state->loadLen;
+ char *cur_page = state->readBuf;
+ TimeLineID *pageTLI = &state->readPageTLI;
XLogRecPtr read_upto,
loc;
int count;
diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index 4d09255504..240a375d8f 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -118,8 +118,12 @@ void
logical_read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
int reqLen, XLogRecPtr targetRecPtr, char *cur_page, TimeLineID *pageTLI)
{
- read_local_xlog_page(state, targetPagePtr, reqLen,
- targetRecPtr, cur_page, pageTLI);
+ Assert(targetPagePtr == state->loadPagePtr &&
+ reqLen == state->loadLen &&
+ targetRecPtr == state->currRecPtr &&
+ cur_page == state->readBuf &&
+ pageTLI == &state->readPageTLI);
+ read_local_xlog_page(state);
}
/*
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 5dba86b8b8..7f119837ce 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -47,10 +47,7 @@ extern Buffer XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum,
extern Relation CreateFakeRelcacheEntry(RelFileNode rnode);
extern void FreeFakeRelcacheEntry(Relation fakerel);
-extern void read_local_xlog_page(XLogReaderState *state,
- XLogRecPtr targetPagePtr, int reqLen,
- XLogRecPtr targetRecPtr, char *cur_page,
- TimeLineID *pageTLI);
+extern void read_local_xlog_page(XLogReaderState *state);
extern void XLogReadDetermineTimeline(XLogReaderState *state,
XLogRecPtr wantPage, uint32 wantLength);
--
2.16.3
----Next_Part(Thu_Apr_18_21_02_57_2019_406)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="0007-Make-logical-rep-stuff-not-use-callback-but-call-the.patch"
^ permalink raw reply [nested|flat] 11+ messages in thread
* [PATCH 06/10] Make XlogReadTwoPhaseData not use callback but call the function directly
@ 2019-04-18 06:02 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 11+ messages in thread
From: Kyotaro Horiguchi @ 2019-04-18 06:02 UTC (permalink / raw)
This patch replaces the call to the callback in XlogReadTwoPhaseData
with direct call to the original function. Then invalidate the
parameters callback and private for XLogReaderAllocate.
---
src/backend/access/transam/twophase.c | 8 ++------
src/backend/access/transam/xlogutils.c | 8 +++++---
src/backend/replication/logical/logicalfuncs.c | 8 ++++++--
src/include/access/xlogutils.h | 5 +----
4 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 6feca69126..f6d48368fe 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1386,8 +1386,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
XLogReaderState *xlogreader;
char *errormsg;
- xlogreader = XLogReaderAllocate(wal_segment_size, &read_local_xlog_page,
- NULL);
+ xlogreader = XLogReaderAllocate(wal_segment_size, NULL, NULL);
if (!xlogreader)
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
@@ -1396,10 +1395,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
while (XLogReadRecord(xlogreader, lsn, &record, &errormsg) ==
XLREAD_NEED_DATA)
- xlogreader->read_page(xlogreader,
- xlogreader->loadPagePtr, xlogreader->loadLen,
- xlogreader->currRecPtr, xlogreader->readBuf,
- &xlogreader->readPageTLI);
+ read_local_xlog_page(xlogreader);
if (record == NULL)
ereport(ERROR,
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index c853e1f0e3..fd461f16fc 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -908,10 +908,12 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
* loop for now.
*/
void
-read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
- int reqLen, XLogRecPtr targetRecPtr, char *cur_page,
- TimeLineID *pageTLI)
+read_local_xlog_page(XLogReaderState *state)
{
+ XLogRecPtr targetPagePtr = state->loadPagePtr;
+ int reqLen = state->loadLen;
+ char *cur_page = state->readBuf;
+ TimeLineID *pageTLI = &state->readPageTLI;
XLogRecPtr read_upto,
loc;
int count;
diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index 4d09255504..240a375d8f 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -118,8 +118,12 @@ void
logical_read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
int reqLen, XLogRecPtr targetRecPtr, char *cur_page, TimeLineID *pageTLI)
{
- read_local_xlog_page(state, targetPagePtr, reqLen,
- targetRecPtr, cur_page, pageTLI);
+ Assert(targetPagePtr == state->loadPagePtr &&
+ reqLen == state->loadLen &&
+ targetRecPtr == state->currRecPtr &&
+ cur_page == state->readBuf &&
+ pageTLI == &state->readPageTLI);
+ read_local_xlog_page(state);
}
/*
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index be0c79d18c..9724ce20b8 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -47,10 +47,7 @@ extern Buffer XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum,
extern Relation CreateFakeRelcacheEntry(RelFileNode rnode);
extern void FreeFakeRelcacheEntry(Relation fakerel);
-extern void read_local_xlog_page(XLogReaderState *state,
- XLogRecPtr targetPagePtr, int reqLen,
- XLogRecPtr targetRecPtr, char *cur_page,
- TimeLineID *pageTLI);
+extern void read_local_xlog_page(XLogReaderState *state);
extern void XLogReadDetermineTimeline(XLogReaderState *state,
XLogRecPtr wantPage, uint32 wantLength);
--
2.16.3
----Next_Part(Wed_Jul_10_13_18_10_2019_842)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v4-0007-Make-logical-rep-stuff-not-use-callback-but-call-the.patch"
^ permalink raw reply [nested|flat] 11+ messages in thread
* [PATCH 06/10] Make XlogReadTwoPhaseData not use callback but call the function directly
@ 2019-04-18 06:02 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 11+ messages in thread
From: Kyotaro Horiguchi @ 2019-04-18 06:02 UTC (permalink / raw)
This patch replaces the call to the callback in XlogReadTwoPhaseData
with direct call to the original function. Then invalidate the
parameters callback and private for XLogReaderAllocate.
---
src/backend/access/transam/twophase.c | 8 ++------
src/backend/access/transam/xlogutils.c | 8 +++++---
src/backend/replication/logical/logicalfuncs.c | 8 ++++++--
src/include/access/xlogutils.h | 5 +----
4 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index ad0d505e54..23424886dc 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1386,8 +1386,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
XLogReaderState *xlogreader;
char *errormsg;
- xlogreader = XLogReaderAllocate(wal_segment_size, &read_local_xlog_page,
- NULL);
+ xlogreader = XLogReaderAllocate(wal_segment_size, NULL, NULL);
if (!xlogreader)
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
@@ -1396,10 +1395,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
while (XLogReadRecord(xlogreader, lsn, &record, &errormsg) ==
XLREAD_NEED_DATA)
- xlogreader->read_page(xlogreader,
- xlogreader->loadPagePtr, xlogreader->loadLen,
- xlogreader->currRecPtr, xlogreader->readBuf,
- &xlogreader->readPageTLI);
+ read_local_xlog_page(xlogreader);
if (record == NULL)
ereport(ERROR,
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index c853e1f0e3..fd461f16fc 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -908,10 +908,12 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
* loop for now.
*/
void
-read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
- int reqLen, XLogRecPtr targetRecPtr, char *cur_page,
- TimeLineID *pageTLI)
+read_local_xlog_page(XLogReaderState *state)
{
+ XLogRecPtr targetPagePtr = state->loadPagePtr;
+ int reqLen = state->loadLen;
+ char *cur_page = state->readBuf;
+ TimeLineID *pageTLI = &state->readPageTLI;
XLogRecPtr read_upto,
loc;
int count;
diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index 4d09255504..240a375d8f 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -118,8 +118,12 @@ void
logical_read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
int reqLen, XLogRecPtr targetRecPtr, char *cur_page, TimeLineID *pageTLI)
{
- read_local_xlog_page(state, targetPagePtr, reqLen,
- targetRecPtr, cur_page, pageTLI);
+ Assert(targetPagePtr == state->loadPagePtr &&
+ reqLen == state->loadLen &&
+ targetRecPtr == state->currRecPtr &&
+ cur_page == state->readBuf &&
+ pageTLI == &state->readPageTLI);
+ read_local_xlog_page(state);
}
/*
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index be0c79d18c..9724ce20b8 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -47,10 +47,7 @@ extern Buffer XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum,
extern Relation CreateFakeRelcacheEntry(RelFileNode rnode);
extern void FreeFakeRelcacheEntry(Relation fakerel);
-extern void read_local_xlog_page(XLogReaderState *state,
- XLogRecPtr targetPagePtr, int reqLen,
- XLogRecPtr targetRecPtr, char *cur_page,
- TimeLineID *pageTLI);
+extern void read_local_xlog_page(XLogReaderState *state);
extern void XLogReadDetermineTimeline(XLogReaderState *state,
XLogRecPtr wantPage, uint32 wantLength);
--
2.16.3
----Next_Part(Fri_May_24_11_56_24_2019_374)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v3-0007-Make-logical-rep-stuff-not-use-callback-but-call-the.patch"
^ permalink raw reply [nested|flat] 11+ messages in thread
* [PATCH 06/10] Make XlogReadTwoPhaseData not use callback but call the function directly
@ 2019-04-18 06:02 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 11+ messages in thread
From: Kyotaro Horiguchi @ 2019-04-18 06:02 UTC (permalink / raw)
This patch replaces the call to the callback in XlogReadTwoPhaseData
with direct call to the original function. Then invalidate the
parameters callback and private for XLogReaderAllocate.
---
src/backend/access/transam/twophase.c | 8 ++------
src/backend/access/transam/xlogutils.c | 8 +++++---
src/backend/replication/logical/logicalfuncs.c | 8 ++++++--
src/include/access/xlogutils.h | 5 +----
4 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index a3573ad0af..caaa09d785 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1386,8 +1386,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
XLogReaderState *xlogreader;
char *errormsg;
- xlogreader = XLogReaderAllocate(wal_segment_size, &read_local_xlog_page,
- NULL);
+ xlogreader = XLogReaderAllocate(wal_segment_size, NULL, NULL);
if (!xlogreader)
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
@@ -1396,10 +1395,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
while (XLogReadRecord(xlogreader, lsn, &record, &errormsg) ==
XLREAD_NEED_DATA)
- xlogreader->read_page(xlogreader,
- xlogreader->loadPagePtr, xlogreader->loadLen,
- xlogreader->currRecPtr, xlogreader->readBuf,
- &xlogreader->readPageTLI);
+ read_local_xlog_page(xlogreader);
if (record == NULL)
ereport(ERROR,
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index c853e1f0e3..fd461f16fc 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -908,10 +908,12 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
* loop for now.
*/
void
-read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
- int reqLen, XLogRecPtr targetRecPtr, char *cur_page,
- TimeLineID *pageTLI)
+read_local_xlog_page(XLogReaderState *state)
{
+ XLogRecPtr targetPagePtr = state->loadPagePtr;
+ int reqLen = state->loadLen;
+ char *cur_page = state->readBuf;
+ TimeLineID *pageTLI = &state->readPageTLI;
XLogRecPtr read_upto,
loc;
int count;
diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index 4d09255504..240a375d8f 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -118,8 +118,12 @@ void
logical_read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
int reqLen, XLogRecPtr targetRecPtr, char *cur_page, TimeLineID *pageTLI)
{
- read_local_xlog_page(state, targetPagePtr, reqLen,
- targetRecPtr, cur_page, pageTLI);
+ Assert(targetPagePtr == state->loadPagePtr &&
+ reqLen == state->loadLen &&
+ targetRecPtr == state->currRecPtr &&
+ cur_page == state->readBuf &&
+ pageTLI == &state->readPageTLI);
+ read_local_xlog_page(state);
}
/*
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 5dba86b8b8..7f119837ce 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -47,10 +47,7 @@ extern Buffer XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum,
extern Relation CreateFakeRelcacheEntry(RelFileNode rnode);
extern void FreeFakeRelcacheEntry(Relation fakerel);
-extern void read_local_xlog_page(XLogReaderState *state,
- XLogRecPtr targetPagePtr, int reqLen,
- XLogRecPtr targetRecPtr, char *cur_page,
- TimeLineID *pageTLI);
+extern void read_local_xlog_page(XLogReaderState *state);
extern void XLogReadDetermineTimeline(XLogReaderState *state,
XLogRecPtr wantPage, uint32 wantLength);
--
2.16.3
----Next_Part(Fri_Apr_26_17_40_34_2019_888)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v2-0007-Make-logical-rep-stuff-not-use-callback-but-call-the.patch"
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: pg14 psql broke \d datname.nspname.relname
@ 2022-04-20 20:38 Thomas Munro <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Thomas Munro @ 2022-04-20 20:38 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Mark Dilger <[email protected]>; Greg Stark <[email protected]>; Tom Lane <[email protected]>; Andres Freund <[email protected]>; Julien Rouhaud <[email protected]>; pgsql-hackers; Justin Pryzby <[email protected]>; Stephen Frost <[email protected]>; Peter Geoghegan <[email protected]>
On Thu, Apr 21, 2022 at 7:35 AM Robert Haas <[email protected]> wrote:
> On Wed, Apr 20, 2022 at 3:08 PM Thomas Munro <[email protected]> wrote:
> > Looks like this somehow broke on a Windows box:
> >
> > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jacana&dt=2022-04-20%2016%3A34%3A19
>
> So the issue here is that we are running this command:
>
> pg_dumpall --exclude-database .*
>
> And on that Windows machine, .* is being expanded to .gitignore, so
> pg_dumpall prints:
>
> pg_dumpall: error: improper qualified name (too many dotted names): .gitignore
>
> Instead of:
>
> pg_dumpall: error: improper qualified name (too many dotted names): .*
>
> I don't know why that glob-expansion only happens on jacana, and I
> don't know how to fix it, either.
Perhaps bowerbird and jacana have different versions of IPC::Run? I
see some recent-ish changes to escaping logic in here from Noah:
https://github.com/toddr/IPC-Run/commits/master/lib/IPC/Run/Win32Helper.pm
Looks like the older version looks for meta characters not including
'*', and the later one uses Win32::ShellQuote::quote_native.
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: pg14 psql broke \d datname.nspname.relname
@ 2022-04-20 20:51 Thomas Munro <[email protected]>
parent: Thomas Munro <[email protected]>
0 siblings, 0 replies; 11+ messages in thread
From: Thomas Munro @ 2022-04-20 20:51 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Mark Dilger <[email protected]>; Greg Stark <[email protected]>; Tom Lane <[email protected]>; Andres Freund <[email protected]>; Julien Rouhaud <[email protected]>; pgsql-hackers; Justin Pryzby <[email protected]>; Stephen Frost <[email protected]>; Peter Geoghegan <[email protected]>; Andrew Dunstan <[email protected]>
On Thu, Apr 21, 2022 at 8:38 AM Thomas Munro <[email protected]> wrote:
> On Thu, Apr 21, 2022 at 7:35 AM Robert Haas <[email protected]> wrote:
> > On Wed, Apr 20, 2022 at 3:08 PM Thomas Munro <[email protected]> wrote:
> > > Looks like this somehow broke on a Windows box:
> > >
> > > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jacana&dt=2022-04-20%2016%3A34%3A19
> >
> > So the issue here is that we are running this command:
> >
> > pg_dumpall --exclude-database .*
> >
> > And on that Windows machine, .* is being expanded to .gitignore, so
> > pg_dumpall prints:
> >
> > pg_dumpall: error: improper qualified name (too many dotted names): .gitignore
> >
> > Instead of:
> >
> > pg_dumpall: error: improper qualified name (too many dotted names): .*
> >
> > I don't know why that glob-expansion only happens on jacana, and I
> > don't know how to fix it, either.
>
> Perhaps bowerbird and jacana have different versions of IPC::Run? I
> see some recent-ish changes to escaping logic in here from Noah:
>
> https://github.com/toddr/IPC-Run/commits/master/lib/IPC/Run/Win32Helper.pm
>
> Looks like the older version looks for meta characters not including
> '*', and the later one uses Win32::ShellQuote::quote_native.
This time with Andrew in CC.
^ permalink raw reply [nested|flat] 11+ messages in thread
end of thread, other threads:[~2022-04-20 20:51 UTC | newest]
Thread overview: 11+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-09-12 07:20 Re: make installcheck-world in a clean environment Michael Paquier <[email protected]>
2018-09-12 08:26 ` Alexander Lakhin <[email protected]>
2018-11-16 01:19 ` Tom Lane <[email protected]>
2018-11-18 07:30 ` Alexander Lakhin <[email protected]>
2018-11-30 20:59 ` Dmitry Dolgov <[email protected]>
2019-04-18 06:02 [PATCH 06/10] Make XlogReadTwoPhaseData not use callback but call the function directly Kyotaro Horiguchi <[email protected]>
2019-04-18 06:02 [PATCH 06/10] Make XlogReadTwoPhaseData not use callback but call the function directly Kyotaro Horiguchi <[email protected]>
2019-04-18 06:02 [PATCH 06/10] Make XlogReadTwoPhaseData not use callback but call the function directly Kyotaro Horiguchi <[email protected]>
2019-04-18 06:02 [PATCH 06/10] Make XlogReadTwoPhaseData not use callback but call the function directly Kyotaro Horiguchi <[email protected]>
2022-04-20 20:38 Re: pg14 psql broke \d datname.nspname.relname Thomas Munro <[email protected]>
2022-04-20 20:51 ` Re: pg14 psql broke \d datname.nspname.relname Thomas Munro <[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