pgsql: Fix incorrect value for "strategy" with deflateParams() in walme
Fix incorrect value for "strategy" with deflateParams() in walmethods.c The zlib documentation mentions the values supported for the compression strategy, but this code has been using a hardcoded value of 0 rather than Z_DEFAULT_STRATEGY. This commit adjusts the code to use Z_DEFAULT_STRATEGY. Backpatch down to where this code has been added to ease the backport of any future patch touching this area. Reported-by: Tom Lane Discussion: https://postgr.es/m/1400032.1662217...@sss.pgh.pa.us Backpatch-through: 10 Branch -- REL_14_STABLE Details --- https://git.postgresql.org/pg/commitdiff/4b529f4697f3cb59e1a225a741f14a1afb6bccd5 Modified Files -- src/bin/pg_basebackup/walmethods.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-)
pgsql: Fix incorrect value for "strategy" with deflateParams() in walme
Fix incorrect value for "strategy" with deflateParams() in walmethods.c The zlib documentation mentions the values supported for the compression strategy, but this code has been using a hardcoded value of 0 rather than Z_DEFAULT_STRATEGY. This commit adjusts the code to use Z_DEFAULT_STRATEGY. Backpatch down to where this code has been added to ease the backport of any future patch touching this area. Reported-by: Tom Lane Discussion: https://postgr.es/m/1400032.1662217...@sss.pgh.pa.us Backpatch-through: 10 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/b447d6075db8041b870ac526525f6873df222db4 Modified Files -- src/bin/pg_basebackup/walmethods.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-)
pgsql: Fix incorrect value for "strategy" with deflateParams() in walme
Fix incorrect value for "strategy" with deflateParams() in walmethods.c The zlib documentation mentions the values supported for the compression strategy, but this code has been using a hardcoded value of 0 rather than Z_DEFAULT_STRATEGY. This commit adjusts the code to use Z_DEFAULT_STRATEGY. Backpatch down to where this code has been added to ease the backport of any future patch touching this area. Reported-by: Tom Lane Discussion: https://postgr.es/m/1400032.1662217...@sss.pgh.pa.us Backpatch-through: 10 Branch -- REL_11_STABLE Details --- https://git.postgresql.org/pg/commitdiff/f01cc02259df914db78e624babb2bc08fa9a9509 Modified Files -- src/bin/pg_basebackup/walmethods.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-)
pgsql: Fix incorrect value for "strategy" with deflateParams() in walme
Fix incorrect value for "strategy" with deflateParams() in walmethods.c The zlib documentation mentions the values supported for the compression strategy, but this code has been using a hardcoded value of 0 rather than Z_DEFAULT_STRATEGY. This commit adjusts the code to use Z_DEFAULT_STRATEGY. Backpatch down to where this code has been added to ease the backport of any future patch touching this area. Reported-by: Tom Lane Discussion: https://postgr.es/m/1400032.1662217...@sss.pgh.pa.us Backpatch-through: 10 Branch -- REL_12_STABLE Details --- https://git.postgresql.org/pg/commitdiff/adb371c9cd95ad48cbcedb21a8cb9b0208295dc6 Modified Files -- src/bin/pg_basebackup/walmethods.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-)
pgsql: Fix incorrect value for "strategy" with deflateParams() in walme
Fix incorrect value for "strategy" with deflateParams() in walmethods.c The zlib documentation mentions the values supported for the compression strategy, but this code has been using a hardcoded value of 0 rather than Z_DEFAULT_STRATEGY. This commit adjusts the code to use Z_DEFAULT_STRATEGY. Backpatch down to where this code has been added to ease the backport of any future patch touching this area. Reported-by: Tom Lane Discussion: https://postgr.es/m/1400032.1662217...@sss.pgh.pa.us Backpatch-through: 10 Branch -- REL_15_STABLE Details --- https://git.postgresql.org/pg/commitdiff/b3c630cc9230ef1ead0dab0ec1498fb4fd2d0de6 Modified Files -- src/bin/pg_basebackup/walmethods.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-)
pgsql: Fix incorrect value for "strategy" with deflateParams() in walme
Fix incorrect value for "strategy" with deflateParams() in walmethods.c The zlib documentation mentions the values supported for the compression strategy, but this code has been using a hardcoded value of 0 rather than Z_DEFAULT_STRATEGY. This commit adjusts the code to use Z_DEFAULT_STRATEGY. Backpatch down to where this code has been added to ease the backport of any future patch touching this area. Reported-by: Tom Lane Discussion: https://postgr.es/m/1400032.1662217...@sss.pgh.pa.us Backpatch-through: 10 Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/a9e99ff6b4ac46f78d4d173d2720ed453dda1331 Modified Files -- src/bin/pg_basebackup/walmethods.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-)
pgsql: Fix incorrect value for "strategy" with deflateParams() in walme
Fix incorrect value for "strategy" with deflateParams() in walmethods.c The zlib documentation mentions the values supported for the compression strategy, but this code has been using a hardcoded value of 0 rather than Z_DEFAULT_STRATEGY. This commit adjusts the code to use Z_DEFAULT_STRATEGY. Backpatch down to where this code has been added to ease the backport of any future patch touching this area. Reported-by: Tom Lane Discussion: https://postgr.es/m/1400032.1662217...@sss.pgh.pa.us Backpatch-through: 10 Branch -- REL_13_STABLE Details --- https://git.postgresql.org/pg/commitdiff/a94576c377b7934860d2fa5453d1cf17d8689810 Modified Files -- src/bin/pg_basebackup/walmethods.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-)
pgsql: Fix typo in pgbench.c.
Fix typo in pgbench.c. Author: Kyotaro Horiguchi Discussion: https://postgr.es/m/20220914.114608.1462991533784489178.horikyota@gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/d583036d68d6fe2fa7facd63eb6548583094fa96 Modified Files -- src/bin/pgbench/pgbench.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
pgsql: Bump minimum Perl version to 5.14
Bump minimum Perl version to 5.14 The oldest vendor-shipped Perl in the buildfarm is 5.14.2, which is the last version that Debian Wheezy shipped. That OS is EOL, but we keep it running because there is no other convenient way to test certain non-mainstream 32-bit platforms. There is no bugfix in the 5.14.2 release that is required, and yet it's also not the latest minor release -- that would be 5.14.4. To clarify the situation, we have thus arranged the buildfarm to test 5.14.0. That allows configure scripts and documentation to state 5.14 without fine print. The MSVC build didn't check the version, since our previous minimum 5.8.3 was considered too old to check for on Windows. We will need a check for Windows sometime during the v16 cycle, but that could be rendered moot by the impending Meson conversion, so it seems safe to just document the requirement for now. Reviewed by Tom Lane Discussion: https://www.postgresql.org/message-id/20220902181553.ev4pgzhubhdkg...@awork3.anarazel.de Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/4c1532763a00c21cbb737bc3855e9a31374b119d Modified Files -- config/perl.m4 | 4 ++-- configure| 6 +++--- doc/src/sgml/install-windows.sgml| 2 +- doc/src/sgml/installation.sgml | 4 ++-- src/pl/plperl/plc_perlboot.pl| 1 - src/test/perl/PostgreSQL/Test/Cluster.pm | 2 +- src/test/perl/README | 10 +++--- src/tools/msvc/gendef.pl | 1 - src/tools/pgindent/pgindent | 1 - 9 files changed, 12 insertions(+), 19 deletions(-)
Re: pgsql: process startup: Remove bootstrap / checker modes from AuxProcTy
Hi, On September 13, 2022 9:43:57 PM PDT, Peter Eisentraut wrote: >On 05.08.21 21:26, Andres Freund wrote: >> process startup: Remove bootstrap / checker modes from AuxProcType. >> >> Neither is actually initialized as an auxiliary process, so it does not >> really >> make sense to reserve a PGPROC etc for them. >> >> This keeps checker mode implemented by exiting partway through bootstrap >> mode. That might be worth changing at some point, perhaps if we ever extend >> checker mode to be a more general tool. > >I don't know if this was the original commit that added the --check option or >was just refactoring around it, but: > >I don't see any mention of the --check option in the postgres man page. That >should be added. > > > IIRC we discussed that at some point and considered it a sufficiently internal option that documenting it in user facing docs doesn't seem useful. Andres -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
Re: pgsql: process startup: Remove bootstrap / checker modes from AuxProcTy
On 05.08.21 21:26, Andres Freund wrote: process startup: Remove bootstrap / checker modes from AuxProcType. Neither is actually initialized as an auxiliary process, so it does not really make sense to reserve a PGPROC etc for them. This keeps checker mode implemented by exiting partway through bootstrap mode. That might be worth changing at some point, perhaps if we ever extend checker mode to be a more general tool. I don't know if this was the original commit that added the --check option or was just refactoring around it, but: I don't see any mention of the --check option in the postgres man page. That should be added.
pgsql: Expand palloc/pg_malloc API for more type safety
Expand palloc/pg_malloc API for more type safety This adds additional variants of palloc, pg_malloc, etc. that encapsulate common usage patterns and provide more type safety. Specifically, this adds palloc_object(), palloc_array(), and repalloc_array(), which take the type name of the object to be allocated as its first argument and cast the return as a pointer to that type. There are also palloc0_object() and palloc0_array() variants for initializing with zero, and pg_malloc_*() variants of all of the above. Inspired by the talloc library. This is backpatched from master so that future backpatchable code can make use of these APIs. This patch by itself does not contain any users of these APIs. Reviewed-by: Tom Lane Discussion: https://www.postgresql.org/message-id/flat/bb755632-2a43-d523-36f8-a1e7a389a...@enterprisedb.com Branch -- REL_14_STABLE Details --- https://git.postgresql.org/pg/commitdiff/b7f37af7c195519a041fcc6084cff95dc5e4a76f Modified Files -- src/include/common/fe_memutils.h | 28 src/include/utils/palloc.h | 22 ++ 2 files changed, 50 insertions(+)
pgsql: Expand palloc/pg_malloc API for more type safety
Expand palloc/pg_malloc API for more type safety This adds additional variants of palloc, pg_malloc, etc. that encapsulate common usage patterns and provide more type safety. Specifically, this adds palloc_object(), palloc_array(), and repalloc_array(), which take the type name of the object to be allocated as its first argument and cast the return as a pointer to that type. There are also palloc0_object() and palloc0_array() variants for initializing with zero, and pg_malloc_*() variants of all of the above. Inspired by the talloc library. This is backpatched from master so that future backpatchable code can make use of these APIs. This patch by itself does not contain any users of these APIs. Reviewed-by: Tom Lane Discussion: https://www.postgresql.org/message-id/flat/bb755632-2a43-d523-36f8-a1e7a389a...@enterprisedb.com Branch -- REL_15_STABLE Details --- https://git.postgresql.org/pg/commitdiff/7fe55d5e12b66c7807d8af1f45946d7d48f5d6db Modified Files -- src/include/common/fe_memutils.h | 28 src/include/utils/palloc.h | 22 ++ 2 files changed, 50 insertions(+)
pgsql: Expand palloc/pg_malloc API for more type safety
Expand palloc/pg_malloc API for more type safety This adds additional variants of palloc, pg_malloc, etc. that encapsulate common usage patterns and provide more type safety. Specifically, this adds palloc_object(), palloc_array(), and repalloc_array(), which take the type name of the object to be allocated as its first argument and cast the return as a pointer to that type. There are also palloc0_object() and palloc0_array() variants for initializing with zero, and pg_malloc_*() variants of all of the above. Inspired by the talloc library. This is backpatched from master so that future backpatchable code can make use of these APIs. This patch by itself does not contain any users of these APIs. Reviewed-by: Tom Lane Discussion: https://www.postgresql.org/message-id/flat/bb755632-2a43-d523-36f8-a1e7a389a...@enterprisedb.com Branch -- REL_11_STABLE Details --- https://git.postgresql.org/pg/commitdiff/e962235fe1f6ae79eff9b600bd37f945becec36e Modified Files -- src/include/common/fe_memutils.h | 28 src/include/utils/palloc.h | 22 ++ 2 files changed, 50 insertions(+)
pgsql: Expand palloc/pg_malloc API for more type safety
Expand palloc/pg_malloc API for more type safety This adds additional variants of palloc, pg_malloc, etc. that encapsulate common usage patterns and provide more type safety. Specifically, this adds palloc_object(), palloc_array(), and repalloc_array(), which take the type name of the object to be allocated as its first argument and cast the return as a pointer to that type. There are also palloc0_object() and palloc0_array() variants for initializing with zero, and pg_malloc_*() variants of all of the above. Inspired by the talloc library. This is backpatched from master so that future backpatchable code can make use of these APIs. This patch by itself does not contain any users of these APIs. Reviewed-by: Tom Lane Discussion: https://www.postgresql.org/message-id/flat/bb755632-2a43-d523-36f8-a1e7a389a...@enterprisedb.com Branch -- REL_13_STABLE Details --- https://git.postgresql.org/pg/commitdiff/1728822924511e792760737aab8749e294af2c1c Modified Files -- src/include/common/fe_memutils.h | 28 src/include/utils/palloc.h | 22 ++ 2 files changed, 50 insertions(+)
pgsql: Expand palloc/pg_malloc API for more type safety
Expand palloc/pg_malloc API for more type safety This adds additional variants of palloc, pg_malloc, etc. that encapsulate common usage patterns and provide more type safety. Specifically, this adds palloc_object(), palloc_array(), and repalloc_array(), which take the type name of the object to be allocated as its first argument and cast the return as a pointer to that type. There are also palloc0_object() and palloc0_array() variants for initializing with zero, and pg_malloc_*() variants of all of the above. Inspired by the talloc library. This is backpatched from master so that future backpatchable code can make use of these APIs. This patch by itself does not contain any users of these APIs. Reviewed-by: Tom Lane Discussion: https://www.postgresql.org/message-id/flat/bb755632-2a43-d523-36f8-a1e7a389a...@enterprisedb.com Branch -- REL_12_STABLE Details --- https://git.postgresql.org/pg/commitdiff/7dd9b469bc56f853ce2a092a6575a587a8c45ccf Modified Files -- src/include/common/fe_memutils.h | 28 src/include/utils/palloc.h | 22 ++ 2 files changed, 50 insertions(+)
pgsql: Expand palloc/pg_malloc API for more type safety
Expand palloc/pg_malloc API for more type safety This adds additional variants of palloc, pg_malloc, etc. that encapsulate common usage patterns and provide more type safety. Specifically, this adds palloc_object(), palloc_array(), and repalloc_array(), which take the type name of the object to be allocated as its first argument and cast the return as a pointer to that type. There are also palloc0_object() and palloc0_array() variants for initializing with zero, and pg_malloc_*() variants of all of the above. Inspired by the talloc library. This is backpatched from master so that future backpatchable code can make use of these APIs. This patch by itself does not contain any users of these APIs. Reviewed-by: Tom Lane Discussion: https://www.postgresql.org/message-id/flat/bb755632-2a43-d523-36f8-a1e7a389a...@enterprisedb.com Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/2864f77556110529f294951382a8d67201244758 Modified Files -- src/include/common/fe_memutils.h | 28 src/include/utils/palloc.h | 22 ++ 2 files changed, 50 insertions(+)
pgsql: Move gramparse.h to src/backend/parser
Move gramparse.h to src/backend/parser
This header is semi-private, being used only in files related to
raw parsing, so move to the backend directory where those files
live. This allows removal of Makefile rules that symlink gram.h to
src/include/parser, since gramparse.h can now include gram.h from
within the same directory. This has the side-effect of no longer
installing gram.h and gramparse.h, but there doesn't seem to be a
good reason to continue doing so.
Per suggestion from Andres Freund and Peter Eisentraut
Discussion:
https://www.postgresql.org/message-id/20220904181759.px6uosll6zbxcum5%40awork3.anarazel.de
Branch
--
master
Details
---
https://git.postgresql.org/pg/commitdiff/ecaf7c5df54f7fa9df2fdc7225d2bb4e283f0081
Modified Files
--
src/backend/Makefile| 7 +--
src/backend/parser/gram.y | 2 +-
src/{include => backend}/parser/gramparse.h | 4 ++--
src/backend/parser/parser.c | 2 +-
src/backend/parser/scan.l | 2 +-
src/include/Makefile| 4 ++--
src/include/parser/.gitignore | 1 -
src/tools/msvc/Install.pm | 4
src/tools/pginclude/cpluspluscheck | 1 -
src/tools/pginclude/headerscheck| 1 -
10 files changed, 8 insertions(+), 20 deletions(-)
pgsql: Simplify handling of compression level with compression specific
Simplify handling of compression level with compression specifications PG_COMPRESSION_OPTION_LEVEL is removed from the compression specification logic, and instead the compression level is always assigned with each library's default if nothing is directly given. This centralizes the checks on the compression methods supported by a given build, and always assigns a default compression level when parsing a compression specification. This results in complaining at an earlier stage than previously if a build supports a compression method or not, aka when parsing a specification in the backend or the frontend, and not when processing it. zstd, lz4 and zlib are able to handle in their respective routines setting up the compression level the case of a default value, hence the backend or frontend code (pg_receivewal or pg_basebackup) has now no need to know what the default compression level should be if nothing is specified: the logic is now done so as the specification parsing assigns it. It can also be enforced by passing down a "level" set to the default value, that the backend will accept (the replication protocol is for example able to handle a command like BASE_BACKUP (COMPRESSION_DETAIL 'gzip:level=-1')). This code simplification fixes an issue with pg_basebackup --gzip introduced by ffd5365, where the tarball of the streamed WAL segments would be created as of pg_wal.tar.gz with uncompressed contents, while the intention is to compress the segments with gzip at a default level. The origin of the confusion comes from the handling of the default compression level of gzip (-1 or Z_DEFAULT_COMPRESSION) and the value of 0 was getting assigned, which is what walmethods.c would consider as equivalent to no compression when streaming WAL segments with its tar methods. Assigning always the compression level removes the confusion of some code paths considering a value of 0 set in a specification as either no compression or a default compression level. Note that 010_pg_basebackup.pl has to be adjusted to skip a few tests where the shape of the compression detail string for client and server-side compression was checked using gzip. This is a result of the code simplification, as gzip specifications cannot be used if a build does not support it. Reported-by: Tom Lane Reviewed-by: Tom Lane Discussion: https://postgr.es/m/1400032.1662217...@sss.pgh.pa.us Backpatch-through: 15 Branch -- REL_15_STABLE Details --- https://git.postgresql.org/pg/commitdiff/53332eacaff6a391f79294193d0c39bff9d14b43 Modified Files -- doc/src/sgml/protocol.sgml | 10 +- src/backend/backup/basebackup_gzip.c | 10 +- src/backend/backup/basebackup_lz4.c | 9 +- src/backend/backup/basebackup_zstd.c | 13 +-- src/bin/pg_basebackup/bbstreamer_gzip.c | 4 +- src/bin/pg_basebackup/bbstreamer_lz4.c | 3 +- src/bin/pg_basebackup/bbstreamer_zstd.c | 15 ++- src/bin/pg_basebackup/pg_basebackup.c| 7 +- src/bin/pg_basebackup/pg_receivewal.c| 35 +-- src/bin/pg_basebackup/t/010_pg_basebackup.pl | 134 ++- src/common/compression.c | 105 - src/include/common/compression.h | 3 +- 12 files changed, 189 insertions(+), 159 deletions(-)
pgsql: Simplify handling of compression level with compression specific
Simplify handling of compression level with compression specifications PG_COMPRESSION_OPTION_LEVEL is removed from the compression specification logic, and instead the compression level is always assigned with each library's default if nothing is directly given. This centralizes the checks on the compression methods supported by a given build, and always assigns a default compression level when parsing a compression specification. This results in complaining at an earlier stage than previously if a build supports a compression method or not, aka when parsing a specification in the backend or the frontend, and not when processing it. zstd, lz4 and zlib are able to handle in their respective routines setting up the compression level the case of a default value, hence the backend or frontend code (pg_receivewal or pg_basebackup) has now no need to know what the default compression level should be if nothing is specified: the logic is now done so as the specification parsing assigns it. It can also be enforced by passing down a "level" set to the default value, that the backend will accept (the replication protocol is for example able to handle a command like BASE_BACKUP (COMPRESSION_DETAIL 'gzip:level=-1')). This code simplification fixes an issue with pg_basebackup --gzip introduced by ffd5365, where the tarball of the streamed WAL segments would be created as of pg_wal.tar.gz with uncompressed contents, while the intention is to compress the segments with gzip at a default level. The origin of the confusion comes from the handling of the default compression level of gzip (-1 or Z_DEFAULT_COMPRESSION) and the value of 0 was getting assigned, which is what walmethods.c would consider as equivalent to no compression when streaming WAL segments with its tar methods. Assigning always the compression level removes the confusion of some code paths considering a value of 0 set in a specification as either no compression or a default compression level. Note that 010_pg_basebackup.pl has to be adjusted to skip a few tests where the shape of the compression detail string for client and server-side compression was checked using gzip. This is a result of the code simplification, as gzip specifications cannot be used if a build does not support it. Reported-by: Tom Lane Reviewed-by: Tom Lane Discussion: https://postgr.es/m/1400032.1662217...@sss.pgh.pa.us Backpatch-through: 15 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/f352e2d08ac048d7407dd6098fc6b344ff85c2dd Modified Files -- doc/src/sgml/protocol.sgml | 10 +- src/backend/backup/basebackup_gzip.c | 10 +- src/backend/backup/basebackup_lz4.c | 9 +- src/backend/backup/basebackup_zstd.c | 13 +-- src/bin/pg_basebackup/bbstreamer_gzip.c | 4 +- src/bin/pg_basebackup/bbstreamer_lz4.c | 3 +- src/bin/pg_basebackup/bbstreamer_zstd.c | 15 ++- src/bin/pg_basebackup/pg_basebackup.c| 7 +- src/bin/pg_basebackup/pg_receivewal.c| 35 +-- src/bin/pg_basebackup/t/010_pg_basebackup.pl | 134 ++- src/common/compression.c | 105 - src/include/common/compression.h | 3 +- 12 files changed, 189 insertions(+), 159 deletions(-)
Re: pgsql: aix: No need to use mkldexport when we want to export all symbol
Andres Freund writes: > So we could work around the xlc 12.1 issue with something like the attached > patch. It passes at some of the tests, with both 32 and 64bit xlc 12.1, will > have to wait a while to see more Shouldn't that be more like + if test "$pgac_cv_prog_CC_cflags__qvisibility_hidden" != "yes"; then +CFLAGS_SL_MODULE='$CFLAGS_SL_MODULE -Wl,-b,expfull' + fi to avoid losing whatever we found out before that? > I think it'd be considerably better to just not support xlc < 13.1 though. A three-line patch doesn't seem like an unreasonable thing to carry, at least till these systems go out of support. We've jumped through much higher hoops in the past to support niche platforms. regards, tom lane
Re: pgsql: aix: No need to use mkldexport when we want to export all symbol
Hi, On 2022-09-13 11:25:30 -0700, Andres Freund wrote: > On 2022-09-11 11:19:25 -0700, Andres Freund wrote: > > We could also try to fall back to using -Wl,b,expall for extension modules. > > expall doesn't work, because of our use of identifiers in reserved namespaces, > e.g. _PG_init: > >expall > Exports all global symbols, except imported symbols, unreferenced > symbols defined in archive members, and symbols beginning with an underscore > (_). You can export additional symbols by listing them in an > export file or using the expfull option. This option does not affect symbols > exported by the autoexp option. > > However, there also is 'expfull' - IIRC that causes problems when used for > postgres, but appears to be fine for .so's. > > So we could work around the xlc 12.1 issue with something like the attached > patch. It passes at some of the tests, with both 32 and 64bit xlc 12.1, will > have to wait a while to see more Both passed check-world. - Andres
Re: pgsql: Fix perltidy breaking perlcritic
On 2022-09-13 Tu 05:25, John Naylor wrote: > On Mon, Sep 12, 2022 at 4:54 PM Dagfinn Ilmari Mannsåker > wrote: > >>> eval "\$hash_ref = $_"; ## no critic (ProhibitStringyEval) >> I didn't see this until it got committed, since I'm not subscribed to >> -committers, but I think it would be even better to rely on the fact >> that eval returns the value of the last expression in the string, which >> also gets rid of the ugly quoting and escaping, per the attached. > Hmm, interesting. I agree it's a slight stylistic improvement. I was trying to keep as close as possible to the original. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Re: pgsql: aix: No need to use mkldexport when we want to export all symbol
Hi, On 2022-09-11 11:19:25 -0700, Andres Freund wrote: > We could also try to fall back to using -Wl,b,expall for extension modules. expall doesn't work, because of our use of identifiers in reserved namespaces, e.g. _PG_init: expall Exports all global symbols, except imported symbols, unreferenced symbols defined in archive members, and symbols beginning with an underscore (_). You can export additional symbols by listing them in an export file or using the expfull option. This option does not affect symbols exported by the autoexp option. However, there also is 'expfull' - IIRC that causes problems when used for postgres, but appears to be fine for .so's. So we could work around the xlc 12.1 issue with something like the attached patch. It passes at some of the tests, with both 32 and 64bit xlc 12.1, will have to wait a while to see more I think it'd be considerably better to just not support xlc < 13.1 though. This won't fix sungazer, that just needs the perl wrapper removed... Greetings, Andres Freund >From 0f7503d7ef1053eeaa37e9aeb020502cc9078d57 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Tue, 13 Sep 2022 10:55:03 -0700 Subject: [PATCH] wip --- configure| 5 + configure.ac | 5 + 2 files changed, 10 insertions(+) diff --git a/configure b/configure index fd2a782454b..82fce7eccc4 100755 --- a/configure +++ b/configure @@ -7101,6 +7101,11 @@ if test x"$pgac_cv_prog_CXX_cxxflags__qvisibility_hidden" = x"yes"; then fi have_visibility_attribute=$pgac_cv_prog_CC_cflags__qvisibility_hidden + # Old xlc versions (<13.1) don't have support for -qvisibility. Use expfull to force + # all extension module symbols to be exported. + if test "$pgac_cv_prog_CC_cflags__qvisibility_hidden" != "yes"; then +CFLAGS_SL_MODULE='-Wl,-b,expfull' + fi fi if test "$have_visibility_attribute" = "yes"; then diff --git a/configure.ac b/configure.ac index 7792ae5bad3..93c015d6e0b 100644 --- a/configure.ac +++ b/configure.ac @@ -592,6 +592,11 @@ elif test "$PORTNAME" = "aix"; then PGAC_PROG_CC_VAR_OPT(CFLAGS_SL_MODULE, [-qvisibility=hidden]) PGAC_PROG_VARCXX_VARFLAGS_OPT(CXX, CXXFLAGS_SL_MODULE, [-qvisibility=hidden]) have_visibility_attribute=$pgac_cv_prog_CC_cflags__qvisibility_hidden + # Old xlc versions (<13.1) don't have support for -qvisibility. Use expfull to force + # all extension module symbols to be exported. + if test "$pgac_cv_prog_CC_cflags__qvisibility_hidden" != "yes"; then +CFLAGS_SL_MODULE='-Wl,-b,expfull' + fi fi if test "$have_visibility_attribute" = "yes"; then -- 2.37.3.542.gdd3f6c4cae
pgsql: Split up guc.c for better build speed and ease of maintenance.
Split up guc.c for better build speed and ease of maintenance. guc.c has grown to be one of our largest .c files, making it a bottleneck for compilation. It's also acquired a bunch of knowledge that'd be better kept elsewhere, because of our not very good habit of putting variable-specific check hooks here. Hence, split it up along these lines: * guc.c itself retains just the core GUC housekeeping mechanisms. * New file guc_funcs.c contains the SET/SHOW interfaces and some SQL-accessible functions for GUC manipulation. * New file guc_tables.c contains the data arrays that define the built-in GUC variables, along with some already-exported constant tables. * GUC check/assign/show hook functions are moved to the variable's home module, whenever that's clearly identifiable. A few hard- to-classify hooks ended up in commands/variable.c, which was already a home for miscellaneous GUC hook functions. To avoid cluttering a lot more header files with #include "guc.h", I also invented a new header file utils/guc_hooks.h and put all the GUC hook functions' declarations there, regardless of their originating module. That allowed removal of #include "guc.h" from some existing headers. The fallout from that (hopefully all caught here) demonstrates clearly why such inclusions are best minimized: there are a lot of files that, for example, were getting array.h at two or more levels of remove, despite not having any connection at all to GUCs in themselves. There is some very minor code beautification here, such as renaming a couple of inconsistently-named hook functions and improving some comments. But mostly this just moves code from point A to point B and deals with the ensuing needs for #include adjustments and exporting a few functions that previously weren't exported. Patch by me, per a suggestion from Andres Freund; thanks also to Michael Paquier for the idea to invent guc_funcs.c. Discussion: https://postgr.es/m/587607.1662836...@sss.pgh.pa.us Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/0a20ff54f5e66158930d5328f89f087d4e9ab400 Modified Files -- contrib/amcheck/verify_nbtree.c | 1 + contrib/ltree/_ltree_gist.c | 1 + contrib/ltree/_ltree_op.c | 1 + contrib/ltree/lquery_op.c | 1 + contrib/ltree/ltree_gist.c | 1 + contrib/pg_surgery/heap_surgery.c | 1 + contrib/pg_trgm/trgm_op.c | 1 + src/backend/access/brin/brin.c | 1 + src/backend/access/table/tableamapi.c | 1 + src/backend/access/transam/xlog.c | 176 +- src/backend/access/transam/xlogprefetcher.c | 2 +- src/backend/access/transam/xlogrecovery.c | 317 +- src/backend/backup/basebackup.c | 1 + src/backend/catalog/aclchk.c| 1 + src/backend/catalog/namespace.c | 2 +- src/backend/catalog/pg_parameter_acl.c | 1 + src/backend/commands/cluster.c | 1 + src/backend/commands/indexcmds.c| 1 + src/backend/commands/tablespace.c | 2 +- src/backend/commands/trigger.c |16 + src/backend/commands/variable.c | 271 +- src/backend/libpq/pqcomm.c | 104 +- src/backend/partitioning/partbounds.c | 1 + src/backend/port/sysv_shmem.c |19 +- src/backend/port/win32_shmem.c |16 + src/backend/postmaster/autovacuum.c |27 + src/backend/replication/repl_gram.y | 1 + src/backend/replication/repl_scanner.l | 1 + src/backend/replication/syncrep.c | 1 + src/backend/replication/syncrep_gram.y | 1 + src/backend/replication/syncrep_scanner.l | 1 + src/backend/storage/buffer/localbuf.c |20 +- src/backend/storage/ipc/ipci.c | 1 + src/backend/storage/lmgr/predicate.c| 4 +- src/backend/tcop/postgres.c |53 + src/backend/tsearch/dict.c | 1 + src/backend/utils/adt/pg_locale.c | 1 + src/backend/utils/adt/selfuncs.c| 1 + src/backend/utils/adt/varchar.c | 1 + src/backend/utils/adt/varlena.c | 1 + src/backend/utils/cache/ts_cache.c | 5 +- src/backend/utils/error/elog.c | 187 +- src/backend/utils/init/postinit.c |50 +- src/backend/utils/misc/Makefile | 2 + src/backend/utils/misc/guc.c| 17318 -- src/backend/utils/misc/guc_funcs.c | 1047 ++ src/backend/utils/misc/guc_tables.c | 4875 src/include/access/tableam.h| 4 +- src/include/access/xlog.h | 4 +- src/include/commands/variable.h |38 - src/include/replication/syncrep.h | 6 - src/include/tcop/tcopprot.h | 3
pgsql: pg_clean_ascii(): escape bytes rather than lose them
pg_clean_ascii(): escape bytes rather than lose them Rather than replace each unprintable byte with a '?' character, replace it with a hex escape instead. The API now allocates a copy rather than modifying the input in place. Author: Jacob Champion Discussion: https://www.postgresql.org/message-id/CAAWbhmgsvHrH9wLU2kYc3pOi1KSenHSLAHBbCVmmddW6-mc_=w...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/45b1a67a0fcb3f1588df596431871de4c93cb76f Modified Files -- src/backend/postmaster/postmaster.c | 6 + src/backend/utils/misc/guc.c| 22 ++-- src/common/string.c | 52 ++--- src/include/common/string.h | 2 +- 4 files changed, 65 insertions(+), 17 deletions(-)
pgsql: Don't reflect unescaped cert data to the logs
Don't reflect unescaped cert data to the logs Commit 3a0e385048 introduced a new path for unauthenticated bytes from the client certificate to be printed unescaped to the logs. There are a handful of these already, but it doesn't make sense to keep making the problem worse. \x-escape any unprintable bytes. The test case introduces a revoked UTF-8 certificate. This requires the addition of the `-utf8` flag to `openssl req`. Since the existing certificates all use an ASCII subset, this won't modify the existing certificates' subjects if/when they get regenerated; this was verified experimentally with $ make sslfiles-clean $ make sslfiles Unfortunately the test can't be run in the CI yet due to a test timing issue; see 55828a6b60. Author: Jacob Champion Discussion: https://www.postgresql.org/message-id/CAAWbhmgsvHrH9wLU2kYc3pOi1KSenHSLAHBbCVmmddW6-mc_=w...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/257eb57b50f7c65467bfc2f4d579622fa13f3370 Modified Files -- src/backend/libpq/be-secure-openssl.c | 57 ++--- src/test/ssl/conf/client-revoked-utf8.config| 13 ++ src/test/ssl/ssl/client-crldir/9bb9e3c3.r0 | 19 + src/test/ssl/ssl/client-revoked-utf8.crt| 18 src/test/ssl/ssl/client-revoked-utf8.key| 27 src/test/ssl/ssl/client.crl | 19 + src/test/ssl/ssl/root+client-crldir/9bb9e3c3.r0 | 19 + src/test/ssl/ssl/root+client.crl| 19 + src/test/ssl/sslfiles.mk| 10 +++-- src/test/ssl/t/001_ssltests.pl | 13 ++ src/test/ssl/t/SSL/Backend/OpenSSL.pm | 3 +- 11 files changed, 150 insertions(+), 67 deletions(-)
pgsql: Make locale option behavior more consistent
Make locale option behavior more consistent Locale options can be specified for initdb, createdb, and CREATE DATABASE. In initdb, it has always been possible to specify --locale and then some --lc-* option to override a category. CREATE DATABASE and createdb didn't allow that, requiring either the all-categories option or only per-category options. In f2553d43060edb210b36c63187d52a632448e1d2, this was changed in CREATE DATABASE (perhaps by accident?) to be more like the initdb behavior, but createdb still had the old behavior. Now we change createdb to match the behavior of CREATE DATABASE and initdb, and also update the documentation of CREATE DATABASE to match the new behavior, which was not done in the above commit. Author: Marina Polyakova Reviewed-by: Justin Pryzby Discussion: https://www.postgresql.org/message-id/7c99c132dc9c0ac630e0127f032ac...@postgrespro.ru Branch -- REL_15_STABLE Details --- https://git.postgresql.org/pg/commitdiff/3e694b318d20c876a3fd64f8e44d2ba5eab7a022 Modified Files -- doc/src/sgml/ref/create_database.sgml | 3 +-- src/bin/scripts/createdb.c| 10 -- 2 files changed, 5 insertions(+), 8 deletions(-)
pgsql: Make locale option behavior more consistent
Make locale option behavior more consistent Locale options can be specified for initdb, createdb, and CREATE DATABASE. In initdb, it has always been possible to specify --locale and then some --lc-* option to override a category. CREATE DATABASE and createdb didn't allow that, requiring either the all-categories option or only per-category options. In f2553d43060edb210b36c63187d52a632448e1d2, this was changed in CREATE DATABASE (perhaps by accident?) to be more like the initdb behavior, but createdb still had the old behavior. Now we change createdb to match the behavior of CREATE DATABASE and initdb, and also update the documentation of CREATE DATABASE to match the new behavior, which was not done in the above commit. Author: Marina Polyakova Reviewed-by: Justin Pryzby Discussion: https://www.postgresql.org/message-id/7c99c132dc9c0ac630e0127f032ac...@postgrespro.ru Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/da5d4ea5aaac4fc02f2e2aec272efe438dd4e171 Modified Files -- doc/src/sgml/ref/create_database.sgml | 3 +-- src/bin/scripts/createdb.c| 10 -- 2 files changed, 5 insertions(+), 8 deletions(-)
pgsql: Improve wal_decode_buffer_size description some more
Improve wal_decode_buffer_size description some more Per Thomas Munro Discussion: https://postgr.es/m/CA+hUKGJ9wP9kpvgoxHvqA=4g1d9-y_w3LhhdhFVU=mfiqjw...@mail.gmail.com Branch -- REL_15_STABLE Details --- https://git.postgresql.org/pg/commitdiff/892cac91249921af73cfe5a7209b64a16589ae18 Modified Files -- src/backend/utils/misc/guc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
pgsql: Improve wal_decode_buffer_size description some more
Improve wal_decode_buffer_size description some more Per Thomas Munro Discussion: https://postgr.es/m/CA+hUKGJ9wP9kpvgoxHvqA=4g1d9-y_w3LhhdhFVU=mfiqjw...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/8e7a0b4a36741934d1350de5646efd2f3a10951e Modified Files -- src/backend/utils/misc/guc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
pgsql: Remove useless pstrdups in untransformRelOptions
Remove useless pstrdups in untransformRelOptions The two strings are already a single palloc'd chunk, not freed; there's no reason to allocate separate copies that have the same lifetime. This code is only called in short-lived memory contexts (except in some cases in TopTransactionContext, which is still short-lived enough not to really matter), and typically only for short arrays, so the memory or computation saved is likely negligible. However, let's fix it to avoid leaving a bad example of code to copy. This is the only place I could find where we're doing this with makeDefElem(). Reported-by: Junwang Zhao Discussion: https://postgr.es/m/20220909142050.3vv2hjekppk265dd@alvherre.pgsql Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/6710e83a675eda798544fea4cdcb89eef7f39403 Modified Files -- src/backend/access/common/reloptions.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
pgsql: Adjust header exceptions for 0bd9c6297
Adjust header exceptions for 0bd9c6297 Per buildfarm animal crake Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/fcf7b3a9d42c3cf778dab0fc644f11f12684d184 Modified Files -- src/tools/pginclude/cpluspluscheck | 2 +- src/tools/pginclude/headerscheck | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
Re: pgsql: Fix perltidy breaking perlcritic
On Mon, Sep 12, 2022 at 4:54 PM Dagfinn Ilmari Mannsåker wrote: > > eval "\$hash_ref = $_"; ## no critic (ProhibitStringyEval) > > I didn't see this until it got committed, since I'm not subscribed to > -committers, but I think it would be even better to rely on the fact > that eval returns the value of the last expression in the string, which > also gets rid of the ugly quoting and escaping, per the attached. Hmm, interesting. -- John Naylor EDB: http://www.enterprisedb.com
pgsql: Treat Unicode codepoints of category "Format" as non-spacing
Treat Unicode codepoints of category "Format" as non-spacing Commit d8594d123 updated the list of non-spacing codepoints used for calculating display width, but in doing so inadvertently removed some, since the script used for that commit only considered combining characters. For complete coverage for zero-width characters, include codepoints in the category Cf (Format). To reflect the wider purpose, also rename files and update comments that referred specifically to combining characters. Some of these ranges have been missing since v12, but due to lack of field complaints it was determined not important enough to justify adding special-case logic the backbranches. Kyotaro Horiguchi Report by Pavel Stehule Discussion: https://www.postgresql.org/message-id/flat/CAFj8pRBE8yvpQ0FSkPCoe0Ny1jAAsAQ6j3qMgVwWvkqAoaaNmQ%40mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/0bd9c629732375e21d3ca6fba16c4a6a2808411a Modified Files -- src/common/unicode/Makefile| 4 +-- ...ble.pl => generate-unicode_nonspacing_table.pl} | 12 src/common/wchar.c | 8 +++--- ...ombining_table.h => unicode_nonspacing_table.h} | 33 ++ 4 files changed, 34 insertions(+), 23 deletions(-)
Re: pgsql: aix: No need to use mkldexport when we want to export all symbol
On 2022-09-13 00:15:55 -0700, Andres Freund wrote: > On 2022-09-12 23:39:04 -0700, Noah Misch wrote: > > On Mon, Sep 12, 2022 at 11:11:23PM -0700, Andres Freund wrote: > > > > > > -qvisibility option specifies visibility attributes for entities. > > > > > > Entity > > > > > > visibility attributes describe whether and how entities defined in > > > > > > one > > > > > > module can be referenced or used in other modules. Visibility > > > > > > attributes > > > > > > affect entities with external linkage only, and cannot increase the > > > > > > visibility of other entities. > > > > > > > > > > > > > > > Since xlc 13.1 supports all still supported AIX versions I'm inclined > > > > > to think > > > > > we should increase our requirement to 13.1 rather than revert back to > > > > > the > > > > > higher complexity way of building modules. > > > > > > > > Perhaps. That would demolish two buildfarm members. > > > > > > Or you could update them to a slightly newer xlc? IIRC the machine has > > > 13.x > > > available? > > > > I don't see 13.x, but I might not know where to look. > > Looks like it's not available on gcc111, just on gcc119, there it's at > /opt/IBM/xlc/13.1.3/bin/xlc > > > > > > What does PostgreSQL lose if you revert fe6a64a? > > > > > > A somewhat readable Makefile.shlib. The prior state is complicated, > > > specific > > > to AIX. I've several times spent quite a bit of time understanding it - > > > not > > > helped by the complete lack of comments. It's not frequent enough to stay > > > in > > > my brain unfortunately, so I'll have to relearn it again next time. And > > > we'll > > > need to continue supporting Makefile.shlib for quite a while, so ... > > > > I'm voting for a revert, then. Code beautification is nice when it just > > works, but desupporting this compiler to land this particular code > > beautification makes the net value go negative for me. I can understand > > your > > feeling differently based on your recent experiences. > > IMO this more than just a beautification - the difference is functional in > nature. I'd agree if it just were a reformating or such, but this isn't that. > > I just tested, and with the mlkdexport.sh approach we end up with all symbols > exported (-fvisibility=hidden) in extension .so's - AIX will be the only > platform unable to restrict symbol exports. That's bound to lead to problems > we'll only have on AIX. > > The more special-case-y some platform is, the higher the cost of supporting it > is. I'm somewhat ok with supporting niche stuff, but then the people desiring > that support also need to do the work of making that bearable for everyone > else. Worth noting that xlc 12.1 appears to be EOL: https://www.ibm.com/support/pages/unsupported-c-and-c-compilers-and-debuggers-links-latest-updates "This page contains links to the final updates (Fix Packs, PTFs, etc) for the non-supported IBM C and C++ compiler and debugger products. This document is provided "AS IS" since those compilers are no longer supported. ... XL C for AIX Fix Pack 23 (May 2021 PTF) for 12.1". The successor, 13.1.0 is from 06 June 2014, which in turn has been superceeded by 16.1 (16.1.0 14 December 2018) and 17.1 (17.1.0 17 September 2021). Greetings, Andres Freund
Re: pgsql: aix: No need to use mkldexport when we want to export all symbol
Hi, On 2022-09-13 00:02:53 -0700, Andres Freund wrote: > On 2022-09-12 23:11:23 -0700, Andres Freund wrote: > > I'll check it out tomorrow. I configured perl on the other AIX gcc compile > > farm machine building 64bit with both gcc and xlc, IIRC. I can't check rn, > > they seem to be down? > > Seems to just have been a temporary connectivity issue. > > After battling a bit with bison flex (system m4 seems incompatible with system > flex?), solved by using your binaries, I suceeded building and testing plperl > on 346990ae2e > > ~/src/postgres/configure FLEX=/home/nm/bin/flex > BISON=/home/nm/sw/bison/bin/bison CXX='g++ -maix64' CC='gcc -maix64' > PERL=perl64 --cache ../config-ac-gcc.cache --enable-cassert --enable-debug > --with-perl > > gmake -j8 -s world-bin && gmake -j8 -s checkprep && gmake -j8 -s temp-install > cd src/pl/plperl/ > gmake check NO_TEMP_INSTALL=1 > ... > == > All 13 tests passed. > == > > "my" linking of plperl.so: > > gcc -maix64 -Wall -Wmissing-prototypes -Wpointer-arith > -Wdeclaration-after-statement -Werror=vla -Wendif-labels > -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type > -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard > -Wno-format-truncation -Wno-stringop-truncation -g -O2 -fvisibility=hidden > -o plperl.so plperl.o SPI.o Util.o -L../../../src/port -L../../../src/common > > -Wl,-blibpath:'/usr/opt/perl5/lib64/5.28.1/aix-thread-multi-64all/CORE:/usr/lib:/lib:/opt/freeware/lib/gcc/powerpc-ibm-aix7.1.0.0/8.3.0/' > -Wl,-bnoentry -Wl,-H512 -Wl,-bM:SRE -shared -fvisibility=hidden > -L/usr/opt/perl5/lib64/5.28.1/aix-thread-multi-64all/CORE -lperl -lpthread > -lbind -lnsl -ldl -lld -lm -lcrypt -lpthreads -lc > -Wl,-bI:../../../src/backend/postgres.imp > > sungazer: > wrap-gcc-8.3 -D_THREAD_SAFE=1 -D_LARGE_FILES=1 -maix64 -Wall > -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement > -Werror=vla -Wendif-labels -Wmissing-format-attribute > -Wimplicit-fallthrough=3 -Wcast-function-type -Wformat-security > -fno-strict-aliasing -fwrapv -fexcess-precision=standard > -Wno-format-truncation -Wno-stringop-truncation -g -O2 -fvisibility=hidden > -o plperl.so plperl.o SPI.o Util.o -L../../../src/port -L../../../src/common > -L/home/nm/sw/nopath/libxml2-64/lib -L/home/nm/sw/nopath/icu58.2-64/lib > -L/home/nm/sw/nopath/uuid-64/lib -L/home/nm/sw/nopath/openldap-64/lib > -L/home/nm/sw/nopath/icu58.2-64/lib -L/home/nm/sw/nopath/libxml2-64/lib > -Wl,-blibpath:'/usr/opt/perl5/lib64/5.28.1/aix-thread-multi-64all/CORE:/home/nm/sw/nopath/libxml2-64/lib:/home/nm/sw/nopath/icu58.2-64/lib:/home/nm/sw/nopath/uuid-64/lib:/home/nm/sw/nopath/openldap-64/lib:/home/nm/sw/nopath/icu58.2-64/lib:/home/nm/sw/nopath/libxml2-64/lib:/usr/lib:/lib:/opt/freeware/lib/gcc/powerpc-ibm-aix7.1.0.0/8.3.0/' > -Wl,-bnoentry -Wl,-H512 -Wl,-bM:SRE -shared -fvisibility=hidden > -Wl,-bE:/usr/opt/perl5/lib64/5.28.1/aix-thread-multi-64all/CORE/perl.exp > -Wl,-bE:/usr/opt/perl5/lib64/5.28.1/aix-thread-multi-64all/CORE/perl.exp > -Wl,-brtl -Wl,-bdynamic -Wl,-b64 > -L/usr/opt/perl5/lib64/5.28.1/aix-thread-multi-64all/CORE -lperl -lpthread > -lbind -lnsl -ldl -lld -lm -lcrypt -lpthreads -lc > -Wl,-bI:../../../src/backend/postgres.imp > > I suspect the problem is the -Wl,-bE:/.../perl.exp - that'll obviously > restrict the exports of plperl.so to the ones of perl, which obviously is > wrong. I would have assumed that Peter's recent plperl changes should have > gotten rid of the -Wl,-bE:? The spurious -Wl,-bE's are the cause of all these warnings btw: ld: 0711-415 WARNING: Symbol ASCII_TO_NEED is already exported. ld: 0711-415 WARNING: Symbol boot_DynaLoader is already exported. ld: 0711-415 WARNING: Symbol NATIVE_TO_NEED is already exported. ld: 0711-415 WARNING: Symbol Perl__add_range_to_invlist is already exported. ld: 0711-415 WARNING: Symbol Perl__core_swash_init is already exported. [~1150 more] ld: 0711-415 WARNING: Symbol PL_Yes is already exported. ld: 0711-415 WARNING: Symbol PL_Zero is already exported. Greetings, Andres Freund
Re: pgsql: aix: No need to use mkldexport when we want to export all symbol
Hi,
On 2022-09-12 23:39:04 -0700, Noah Misch wrote:
> On Mon, Sep 12, 2022 at 11:11:23PM -0700, Andres Freund wrote:
> > On 2022-09-12 20:38:45 -0700, Noah Misch wrote:
> > > On Sun, Sep 11, 2022 at 11:19:25AM -0700, Andres Freund wrote:
> > > Both runs had the latest commits discussed above. tern (gcc32) passed,
> > > but it
> > > doesn't use --with-perl.
> >
> > I'll check it out tomorrow. I configured perl on the other AIX gcc compile
> > farm machine building 64bit with both gcc and xlc, IIRC. I can't check rn,
> > they seem to be down?
>
> They're up.
Yep, seems to have been temporary.
> > What is "perl64-for-gcc"?
>
> ===
> #! /bin/sh
>
> real_perl=perl64
>
> case $* in
> '-MExtUtils::Embed -e ldopts' | \
> '-MConfig -e print $Config{ccdlflags}')
> $real_perl "$@" | sed 's/-b/-Wl,-b/g' ;;
> *)
> exec $real_perl "$@" ;;
> esac
> ===
I guess that defeats our plperl flags logic - the wrapper shouldn't be needed
anymore today, see my parallel message about the problem likely being caused
by -Wl,-bE:.
> > > > > -qvisibility option specifies visibility attributes for entities.
> > > > > Entity
> > > > > visibility attributes describe whether and how entities defined in
> > > > > one
> > > > > module can be referenced or used in other modules. Visibility
> > > > > attributes
> > > > > affect entities with external linkage only, and cannot increase the
> > > > > visibility of other entities.
> > > >
> > > >
> > > > Since xlc 13.1 supports all still supported AIX versions I'm inclined
> > > > to think
> > > > we should increase our requirement to 13.1 rather than revert back to
> > > > the
> > > > higher complexity way of building modules.
> > >
> > > Perhaps. That would demolish two buildfarm members.
> >
> > Or you could update them to a slightly newer xlc? IIRC the machine has 13.x
> > available?
>
> I don't see 13.x, but I might not know where to look.
Looks like it's not available on gcc111, just on gcc119, there it's at
/opt/IBM/xlc/13.1.3/bin/xlc
> > > What does PostgreSQL lose if you revert fe6a64a?
> >
> > A somewhat readable Makefile.shlib. The prior state is complicated, specific
> > to AIX. I've several times spent quite a bit of time understanding it - not
> > helped by the complete lack of comments. It's not frequent enough to stay in
> > my brain unfortunately, so I'll have to relearn it again next time. And
> > we'll
> > need to continue supporting Makefile.shlib for quite a while, so ...
>
> I'm voting for a revert, then. Code beautification is nice when it just
> works, but desupporting this compiler to land this particular code
> beautification makes the net value go negative for me. I can understand your
> feeling differently based on your recent experiences.
IMO this more than just a beautification - the difference is functional in
nature. I'd agree if it just were a reformating or such, but this isn't that.
I just tested, and with the mlkdexport.sh approach we end up with all symbols
exported (-fvisibility=hidden) in extension .so's - AIX will be the only
platform unable to restrict symbol exports. That's bound to lead to problems
we'll only have on AIX.
The more special-case-y some platform is, the higher the cost of supporting it
is. I'm somewhat ok with supporting niche stuff, but then the people desiring
that support also need to do the work of making that bearable for everyone
else.
Greetings,
Andres Freund
Re: pgsql: aix: No need to use mkldexport when we want to export all symbol
Hi, On 2022-09-12 23:11:23 -0700, Andres Freund wrote: > I'll check it out tomorrow. I configured perl on the other AIX gcc compile > farm machine building 64bit with both gcc and xlc, IIRC. I can't check rn, > they seem to be down? Seems to just have been a temporary connectivity issue. After battling a bit with bison flex (system m4 seems incompatible with system flex?), solved by using your binaries, I suceeded building and testing plperl on 346990ae2e ~/src/postgres/configure FLEX=/home/nm/bin/flex BISON=/home/nm/sw/bison/bin/bison CXX='g++ -maix64' CC='gcc -maix64' PERL=perl64 --cache ../config-ac-gcc.cache --enable-cassert --enable-debug --with-perl gmake -j8 -s world-bin && gmake -j8 -s checkprep && gmake -j8 -s temp-install cd src/pl/plperl/ gmake check NO_TEMP_INSTALL=1 ... == All 13 tests passed. == "my" linking of plperl.so: gcc -maix64 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -g -O2 -fvisibility=hidden -o plperl.so plperl.o SPI.o Util.o -L../../../src/port -L../../../src/common -Wl,-blibpath:'/usr/opt/perl5/lib64/5.28.1/aix-thread-multi-64all/CORE:/usr/lib:/lib:/opt/freeware/lib/gcc/powerpc-ibm-aix7.1.0.0/8.3.0/' -Wl,-bnoentry -Wl,-H512 -Wl,-bM:SRE -shared -fvisibility=hidden -L/usr/opt/perl5/lib64/5.28.1/aix-thread-multi-64all/CORE -lperl -lpthread -lbind -lnsl -ldl -lld -lm -lcrypt -lpthreads -lc -Wl,-bI:../../../src/backend/postgres.imp sungazer: wrap-gcc-8.3 -D_THREAD_SAFE=1 -D_LARGE_FILES=1 -maix64 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -g -O2 -fvisibility=hidden -o plperl.so plperl.o SPI.o Util.o -L../../../src/port -L../../../src/common -L/home/nm/sw/nopath/libxml2-64/lib -L/home/nm/sw/nopath/icu58.2-64/lib -L/home/nm/sw/nopath/uuid-64/lib -L/home/nm/sw/nopath/openldap-64/lib -L/home/nm/sw/nopath/icu58.2-64/lib -L/home/nm/sw/nopath/libxml2-64/lib -Wl,-blibpath:'/usr/opt/perl5/lib64/5.28.1/aix-thread-multi-64all/CORE:/home/nm/sw/nopath/libxml2-64/lib:/home/nm/sw/nopath/icu58.2-64/lib:/home/nm/sw/nopath/uuid-64/lib:/home/nm/sw/nopath/openldap-64/lib:/home/nm/sw/nopath/icu58.2-64/lib:/home/nm/sw/nopath/libxml2-64/lib:/usr/lib:/lib:/opt/freeware/lib/gcc/powerpc-ibm-aix7.1.0.0/8.3.0/' -Wl,-bnoentry -Wl,-H512 -Wl,-bM:SRE -shared -fvisibility=hidden -Wl,-bE:/usr/opt/perl5/lib64/5.28.1/aix-thread-multi-64all/CORE/perl.exp -Wl,-bE:/usr/opt/perl5/lib64/5.28.1/aix-thread-multi-64all/CORE/perl.exp -Wl,-brtl -Wl,-bdynamic -Wl,-b64 -L/usr/opt/perl5/lib64/5.28.1/aix-thread-multi-64all/CORE -lperl -lpthread -lbind -lnsl -ldl -lld -lm -lcrypt -lpthreads -lc -Wl,-bI:../../../src/backend/postgres.imp I suspect the problem is the -Wl,-bE:/.../perl.exp - that'll obviously restrict the exports of plperl.so to the ones of perl, which obviously is wrong. I would have assumed that Peter's recent plperl changes should have gotten rid of the -Wl,-bE:? Greetings, Andres Freund
Re: pgsql: aix: No need to use mkldexport when we want to export all symbol
On Mon, Sep 12, 2022 at 11:11:23PM -0700, Andres Freund wrote:
> On 2022-09-12 20:38:45 -0700, Noah Misch wrote:
> > On Sun, Sep 11, 2022 at 11:19:25AM -0700, Andres Freund wrote:
> > > On 2022-09-10 01:19:44 -0700, Andres Freund wrote:
> > > > On 2022-09-09 22:57:36 -0700, Andres Freund wrote:
> > > > > On 2022-09-10 01:32:52 -0400, Tom Lane wrote:
> > > > > > Andres Freund writes:
> > > > > > > It seem worth applying the -qvisibility patch and seeing whether
> > > > > > > that fixes
> > > > > > > the buildfarm?
> > > > > >
> > > > > > Worth a try.
> > > > >
> > > > > Done now.
> > > >
> > > > Seems to have done the trick for at least hoverfly. And wrasse (sunpro)
> > > > is
> > > > still happy. Still need to wait for the older xlc's (mandril, hornet)
> > > > though...
> > >
> > > At least hornet isn't happy:
> > >
> > > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=hornet=2022-09-10%2021%3A44%3A00
> > >
> > > Looks like the support for -qvisibility came with xlc 13.1, released
> > > April 28, 2014:
> > > https://www.ibm.com/common/ssi/cgi-bin/ssialias?subtype=ca=an=iSource=897=ENUS214-162
> >
> > Last two sungazer (gcc64) runs failed with a suspicious "missing magic
> > block":
> > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=sungazer=2022-09-12%2006%3A24%3A53
> > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=sungazer=2022-09-10%2018%3A09%3A11
> >
> > Both runs had the latest commits discussed above. tern (gcc32) passed, but
> > it
> > doesn't use --with-perl.
>
> I'll check it out tomorrow. I configured perl on the other AIX gcc compile
> farm machine building 64bit with both gcc and xlc, IIRC. I can't check rn,
> they seem to be down?
They're up.
> What is "perl64-for-gcc"?
===
#! /bin/sh
real_perl=perl64
case $* in
'-MExtUtils::Embed -e ldopts' | \
'-MConfig -e print $Config{ccdlflags}')
$real_perl "$@" | sed 's/-b/-Wl,-b/g' ;;
*)
exec $real_perl "$@" ;;
esac
===
> > > > -qvisibility option specifies visibility attributes for entities. Entity
> > > > visibility attributes describe whether and how entities defined in one
> > > > module can be referenced or used in other modules. Visibility
> > > > attributes
> > > > affect entities with external linkage only, and cannot increase the
> > > > visibility of other entities.
> > >
> > >
> > > Since xlc 13.1 supports all still supported AIX versions I'm inclined to
> > > think
> > > we should increase our requirement to 13.1 rather than revert back to the
> > > higher complexity way of building modules.
> >
> > Perhaps. That would demolish two buildfarm members.
>
> Or you could update them to a slightly newer xlc? IIRC the machine has 13.x
> available?
I don't see 13.x, but I might not know where to look.
> > What does PostgreSQL lose if you revert fe6a64a?
>
> A somewhat readable Makefile.shlib. The prior state is complicated, specific
> to AIX. I've several times spent quite a bit of time understanding it - not
> helped by the complete lack of comments. It's not frequent enough to stay in
> my brain unfortunately, so I'll have to relearn it again next time. And we'll
> need to continue supporting Makefile.shlib for quite a while, so ...
I'm voting for a revert, then. Code beautification is nice when it just
works, but desupporting this compiler to land this particular code
beautification makes the net value go negative for me. I can understand your
feeling differently based on your recent experiences.
Re: pgsql: aix: No need to use mkldexport when we want to export all symbol
Hi, On 2022-09-12 20:38:45 -0700, Noah Misch wrote: > On Sun, Sep 11, 2022 at 11:19:25AM -0700, Andres Freund wrote: > > On 2022-09-10 01:19:44 -0700, Andres Freund wrote: > > > On 2022-09-09 22:57:36 -0700, Andres Freund wrote: > > > > On 2022-09-10 01:32:52 -0400, Tom Lane wrote: > > > > > Andres Freund writes: > > > > > > It seem worth applying the -qvisibility patch and seeing whether > > > > > > that fixes > > > > > > the buildfarm? > > > > > > > > > > Worth a try. > > > > > > > > Done now. > > > > > > Seems to have done the trick for at least hoverfly. And wrasse (sunpro) is > > > still happy. Still need to wait for the older xlc's (mandril, hornet) > > > though... > > > > At least hornet isn't happy: > > > > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=hornet=2022-09-10%2021%3A44%3A00 > > > > Looks like the support for -qvisibility came with xlc 13.1, released April > > 28, 2014: > > https://www.ibm.com/common/ssi/cgi-bin/ssialias?subtype=ca=an=iSource=897=ENUS214-162 > > Last two sungazer (gcc64) runs failed with a suspicious "missing magic block": > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=sungazer=2022-09-12%2006%3A24%3A53 > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=sungazer=2022-09-10%2018%3A09%3A11 > > Both runs had the latest commits discussed above. tern (gcc32) passed, but it > doesn't use --with-perl. I'll check it out tomorrow. I configured perl on the other AIX gcc compile farm machine building 64bit with both gcc and xlc, IIRC. I can't check rn, they seem to be down? What is "perl64-for-gcc"? > > > -qvisibility option specifies visibility attributes for entities. Entity > > > visibility attributes describe whether and how entities defined in one > > > module can be referenced or used in other modules. Visibility attributes > > > affect entities with external linkage only, and cannot increase the > > > visibility of other entities. > > > > > > Since xlc 13.1 supports all still supported AIX versions I'm inclined to > > think > > we should increase our requirement to 13.1 rather than revert back to the > > higher complexity way of building modules. > > Perhaps. That would demolish two buildfarm members. Or you could update them to a slightly newer xlc? IIRC the machine has 13.x available? > What does PostgreSQL lose if you revert fe6a64a? A somewhat readable Makefile.shlib. The prior state is complicated, specific to AIX. I've several times spent quite a bit of time understanding it - not helped by the complete lack of comments. It's not frequent enough to stay in my brain unfortunately, so I'll have to relearn it again next time. And we'll need to continue supporting Makefile.shlib for quite a while, so ... I do not understand what value it has to run an ancient compiler on a very niche, quite different, OS. I can see some value to supporting a larger number of operating systems and compiles - but I can't see any sort of benefit in also supporting old versions of those compilers. Greetings, Andres Freund
