Re: Add PQsendSyncMessage() to libpq

[Michael Paquier]

On Thu, Apr 27, 2023 at 01:06:27PM +0200, Denis Laxalde wrote:
> Thank you; this V2 looks good to me.
> Marking as ready for committer.

Please note that we are in a stabilization period for v16 and that the
first commit fest of v17 should start in July, so it will perhaps take
some time before this is looked at by a committer.

Speaking of which, what was the performance impact of your application
once PQflush() was moved out of the pipeline sync?  Just asking for
curiosity..
--
Michael


signature.asc
Description: PGP signature

Re: Initial Schema Sync for Logical Replication

[Masahiko Sawada]

On Thu, Apr 27, 2023 at 12:02 PM Wei Wang (Fujitsu)
 wrote:
>
> On Fri, Apr 21, 2023 at 16:48 PM Masahiko Sawada  
> wrote:
> > On Thu, Apr 20, 2023 at 8:16 PM Amit Kapila  wrote:
> > >
> > > On Mon, Apr 17, 2023 at 9:12 AM Masahiko Sawada
> >  wrote:
> > > >
> > > > On Fri, Apr 7, 2023 at 6:37 PM Amit Kapila  
> > > > wrote:
> > > > >
> > > > > On Thu, Apr 6, 2023 at 6:57 PM Masahiko Sawada
> >  wrote:
> > > > > >
> > > > > >
> > > > > > While writing a PoC patch, I found some difficulties in this idea.
> > > > > > First, I tried to add schemaname+relname to pg_subscription_rel but 
> > > > > > I
> > > > > > could not define the primary key of pg_subscription_rel. The primary
> > > > > > key on (srsubid, srrelid) doesn't work since srrelid could be NULL.
> > > > > > Similarly, the primary key on (srsubid, srrelid, schemaname, 
> > > > > > relname)
> > > > > > also doesn't work.
> > > > > >
> > > > >
> > > > > Can we think of having a separate catalog table say
> > > > > pg_subscription_remote_rel for this? You can have srsubid,
> > > > > remote_schema_name, remote_rel_name, etc. We may need some other
> > state
> > > > > to be maintained during the initial schema sync where this table can
> > > > > be used. Basically, this can be used to maintain the state till the
> > > > > initial schema sync is complete because we can create a relation entry
> > > > > in pg_subscritption_rel only after the initial schema sync is
> > > > > complete.
> > > >
> > > > It might not be ideal but I guess it works. But I think we need to
> > > > modify the name of replication slot for initial sync as it currently
> > > > includes OID of the table:
> > > >
> > > > void
> > > > ReplicationSlotNameForTablesync(Oid suboid, Oid relid,
> > > > char *syncslotname, Size szslot)
> > > > {
> > > > snprintf(syncslotname, szslot, "pg_%u_sync_%u_" UINT64_FORMAT,
> > suboid,
> > > >  relid, GetSystemIdentifier());
> > > > }
> > > >
> > > > If we use both schema name and table name, it's possible that slot
> > > > names are duplicated if schema and/or table names are long. Another
> > > > idea is to use the hash value of schema+table names, but it cannot
> > > > completely eliminate that possibility, and probably would make
> > > > investigation and debugging hard in case of any failure. Probably we
> > > > can use the OID of each entry in pg_subscription_remote_rel instead,
> > > > but I'm not sure it's a good idea, mainly because we will end up using
> > > > twice as many OIDs as before.
> > > >
> > >
> > > The other possibility is to use worker_pid. To make debugging easier,
> > > we may want to LOG schema_name+rel_name vs slot_name mapping at
> > DEBUG1
> > > log level.
> >
> > Since worker_pid changes after the worker restarts, a new worker
> > cannot find the slot that had been used, no?
> >
> > After thinking it over, a better solution would be that we add an oid
> > column, nspname column, and relname column to pg_subscription_rel and
> > the primary key on the oid. If the table is not present on the
> > subscriber we store the schema name and table name to the catalog, and
> > otherwise we store the local table oid same as today. The local table
> > oid will be filled after the schema sync. The names of origin and
> > replication slot the tablesync worker uses use the oid instead of the
> > table oid.
> >
> > I've attached a PoC patch of this idea (very rough patch and has many
> > TODO comments). It mixes the following changes:
> >
> > 1. Add oid column to the pg_subscription_rel. The oid is used as the
> > primary key and in the names of origin and slot the tablesync workers
> > use.
> >
> > 2. Add copy_schema = on/off option to CREATE SUBSCRIPTION (not yet
> > support for ALTER SUBSCRIPTION).
> >
> > 3. Add CRS_EXPORT_USE_SNAPSHOT new action in order to use the same
> > snapshot by both walsender and other processes (e.g. pg_dump). In this
> > patch, the snapshot is exported for pg_dump and is used by the
> > walsender for COPY.
> >
> > It seems to work well but there might be a pitfall as I've not fully
> > implemented it.
>
> Thanks for your POC patch.
> After reviewing this patch, I have a question below that want to confirm:
>
> 1. In the function synchronize_table_schema.
> I think some changes to GUC and table-related object SQLs are included in the
> pg_dump result. And in this POC, these SQLs will be executed. Do we need to
> alter the pg_dump results to only execute the table schema related SQLs?

Yes, in this approach, we need to dump/restore objects while
specifying with fine granularity. Ideally, the table sync worker dumps
and restores the table schema, does copy the initial data, and then
creates indexes, and triggers and table-related objects are created
after that. So if we go with the pg_dump approach to copy the schema
of individual tables, we need to change pg_dump (or libpgdump needs to
be able to do) to support it.

Regards,

-- 
Masahiko Sawada
Amazon Web Servi

Re: Add PQsendSyncMessage() to libpq

[Denis Laxalde]

Michael Paquier a écrit :

On Thu, Apr 27, 2023 at 01:06:27PM +0200, Denis Laxalde wrote:

Thank you; this V2 looks good to me.
Marking as ready for committer.


Please note that we are in a stabilization period for v16 and that the
first commit fest of v17 should start in July, so it will perhaps take
some time before this is looked at by a committer.


Yes, I am aware; totally fine by me.


Speaking of which, what was the performance impact of your application
once PQflush() was moved out of the pipeline sync?  Just asking for
curiosity..


I have no metrics for that; but maybe Anton has some?
(In Psycopg, we generally do not expect users to handle the sync 
operation themselves, it's done under the hood; and I only found one 
situation where the flush could be avoided, but that's largely because 
our design, there can be more in general I think.)

Re: Logging parallel worker draught

[Benoit Lobréau]

On 4/22/23 13:06, Amit Kapila wrote:

I don't think introducing a GUC for this is a good idea. We can
directly output this message in the server log either at LOG or DEBUG1
level.


Hi,

Sorry for the delayed answer, I was away from my computer for a few 
days. I don't mind removing the guc, but I would like to keep it at the 
LOG level. When I do audits most client are at that level and setting it 
to DEBUG1 whould add too much log for them on the long run.


I'll post the corresponding patch asap.

--
Benoit Lobréau
Consultant
http://dalibo.com

Re: Testing autovacuum wraparound (including failsafe)

[Daniel Gustafsson]

> On 28 Apr 2023, at 06:42, Tom Lane  wrote:
> John Naylor  writes:

>> If they're that slow, I'd worry more about generating 20GB of xact status
>> data. That's why the tests are disabled by default.
> 
> There is exactly zero chance that anyone will accept the introduction
> of such an expensive test into either check-world or the buildfarm
> sequence.

Even though the entire suite is disabled by default, shouldn't it also require
PG_TEST_EXTRA to be consistent with other off-by-default suites like for example
src/test/kerberos?

--
Daniel Gustafsson

Re: Add two missing tests in 035_standby_logical_decoding.pl

[Drouvot, Bertrand]

Hi,

On 4/28/23 5:55 AM, Amit Kapila wrote:

On Wed, Apr 26, 2023 at 7:53 PM Drouvot, Bertrand
 wrote:

+# Get the restart_lsn from an invalidated slot
+my $restart_lsn = $node_standby->safe_psql('postgres',
+ "SELECT restart_lsn from pg_replication_slots WHERE slot_name =
'vacuum_full_activeslot' and conflicting is true;"
+);
+
+chomp($restart_lsn);
+
+# Get the WAL file name associated to this lsn on the primary
+my $walfile_name = $node_primary->safe_psql('postgres',
+ "SELECT pg_walfile_name('$restart_lsn')");
+
+chomp($walfile_name);
+
+# Check the WAL file is still on the primary
+ok(-f $node_primary->data_dir . '/pg_wal/' . $walfile_name,
+ "WAL file still on the primary");

How is it guaranteed that the WAL file corresponding to the
invalidated slot on standby will still be present on primary?


The slot(s) have been invalidated by the "vacuum full" test just above
this one. So I think the WAL we are looking for is the last one being used
by the primary. As no activity happened on it since the vacuum full it looks to
me that it should still be present.

But I may have missed something and maybe that's not guarantee that this WAL is 
still there in all the cases.
In that case I think it's better to remove this test (it does not provide added 
value here).

Test removed in V7 attached.


Can you
please explain the logic behind this test a bit more like how the WAL
file switch helps you to achieve the purpose?



The idea was to generate enough "wal switch" on the primary to ensure
the WAL file has been removed.

I gave another thought on it and I think we can skip the test that the WAL is
not on the primary any more. That way, one "wal switch" seems to be enough
to see it removed on the standby.

It's done in V7.

V7 is not doing "extra tests" than necessary and I think it's probably better 
like this.

I can see V7 failing on "Cirrus CI / macOS - Ventura - Meson" only (other 
machines are not complaining).

It does fail on "invalidated logical slots do not lead to retaining WAL", see 
https://cirrus-ci.com/task/4518083541336064

I'm not sure why it is failing, any idea?

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.comFrom 2ab08214415023505244c954a6a5ebf42ec9aebb Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot 
Date: Fri, 28 Apr 2023 07:27:20 +
Subject: [PATCH v7] Add a test to verify that invalidated logical slots do not
 lead to retaining WAL.

---
 .../t/035_standby_logical_decoding.pl | 39 ++-
 1 file changed, 37 insertions(+), 2 deletions(-)
 100.0% src/test/recovery/t/

diff --git a/src/test/recovery/t/035_standby_logical_decoding.pl 
b/src/test/recovery/t/035_standby_logical_decoding.pl
index 66d264f230..b32c1002b0 100644
--- a/src/test/recovery/t/035_standby_logical_decoding.pl
+++ b/src/test/recovery/t/035_standby_logical_decoding.pl
@@ -500,9 +500,44 @@ $node_standby->restart;
 check_slots_conflicting_status(1);
 
 ##
-# Verify that invalidated logical slots do not lead to retaining WAL
+# Verify that invalidated logical slots do not lead to retaining WAL.
 ##
-# X TODO
+
+# Before removing WAL file(s), ensure the cascading standby catch up
+$node_standby->wait_for_replay_catchup($node_cascading_standby,
+   $node_primary);
+
+# Get the restart_lsn from an invalidated slot
+my $restart_lsn = $node_standby->safe_psql('postgres',
+   "SELECT restart_lsn from pg_replication_slots WHERE slot_name = 
'vacuum_full_activeslot' and conflicting is true;"
+);
+
+chomp($restart_lsn);
+
+# As pg_walfile_name() can not be executed on the standby,
+# get the WAL file name associated to this lsn from the primary
+my $walfile_name = $node_primary->safe_psql('postgres',
+   "SELECT pg_walfile_name('$restart_lsn')");
+
+chomp($walfile_name);
+
+# Generate some activity and switch WAL file on the primary
+$node_primary->safe_psql(
+   'postgres', "create table retain_test(a int);
+insert 
into retain_test values(1);
+select 
pg_switch_wal();
+
checkpoint;");
+
+# Wait for the standby to catch up
+$node_primary->wait_for_catchup($node_standby);
+
+# Request a checkpoint on the standby to trigger the WAL file(s) removal
+$node_standby->safe_psql('postgres', 'checkpoint;');
+
+# Verify that the wal file has not been retained on the standby
+my $standby_walfile = $node_standby->data_dir . '/pg_wal/' . $walfile_name;
+ok(!-f "$standby_walfile",
+   "invalidated logical slots do not lead to retaining WAL");
 
 ##
 # Recovery conflict: Invalidate conflicting slots, including in-use slots
-- 
2.34.1

Re: Perform streaming logical transactions by background workers and parallel apply

[Amit Kapila]

On Fri, Apr 28, 2023 at 11:48 AM Masahiko Sawada  wrote:
>
> On Fri, Apr 28, 2023 at 11:51 AM Amit Kapila  wrote:
> >
> > On Wed, Apr 26, 2023 at 4:11 PM Zhijie Hou (Fujitsu)
> >  wrote:
> > >
> > > On Wednesday, April 26, 2023 5:00 PM Alexander Lakhin 
> > >  wrote:
> > > >
> > > > IIUC, that assert will fail in case of any error raised between
> > > > ApplyWorkerMain()->logicalrep_worker_attach()->before_shmem_exit() and
> > > > ApplyWorkerMain()->InitializeApplyWorker()->BackgroundWorkerInitializeC
> > > > onnectionByOid()->InitPostgres().
> > >
> > > Thanks for reporting the issue.
> > >
> > > I think the problem is that it tried to release locks in
> > > logicalrep_worker_onexit() before the initialization of the process is 
> > > complete
> > > because this callback function was registered before the init phase. So I 
> > > think we
> > > can add a conditional statement before releasing locks. Please find an 
> > > attached
> > > patch.
> > >
> >
> > Alexander, does the proposed patch fix the problem you are facing?
> > Sawada-San, and others, do you see any better way to fix it than what
> > has been proposed?
>
> I'm concerned that the idea of relying on IsNormalProcessingMode()
> might not be robust since if we change the meaning of
> IsNormalProcessingMode() some day it would silently break again. So I
> prefer using something like InitializingApplyWorker,
>

I think if we change the meaning of IsNormalProcessingMode() then it
could also break the other places the similar check is being used.
However, I am fine with InitializingApplyWorker as that could be used
at other places as well. I just want to avoid adding another variable
by using IsNormalProcessingMode.

> or another idea
> would be to do cleanup work (e.g., fileset deletion and lock release)
> in a separate callback that is registered after connecting to the
> database.
>

Yeah, but not sure if it's worth having multiple callbacks for cleanup work.

-- 
With Regards,
Amit Kapila.

Re: run pgindent on a regular basis / scripted manner

[Alvaro Herrera]

On 2023-Feb-05, Andrew Dunstan wrote:

> So here's a diff made from running perltidy v20221112 with the additional
> setting --valign-exclusion-list=", = => || && if unless"

I ran this experimentally with perltidy 20230309, and compared that with
the --novalign behavior (not to propose the latter -- just to be aware
of what else is vertical alignment doing.)

Based on the differences between both, I think we'll definitely want to
include =~ and |= in this list, and I think we should discuss whether to
also include "or" (for "do_stuff or die()" type of constructs) and "qw"
(mainly used in 'use Foo qw(one two)' import lists).  All these have
effects (albeit smaller than the list you gave) on our existing code.


If you change from an exclusion list to --novalign then you lose
alignment of trailing # comments, which personally I find a loss, even
though they're still a multi-line effect.  Another change would be that
it ditches alignment of "{" but that only changes msvc/Install.pm, so I
think we shouldn't worry; and then there's this one:

-use PostgreSQL::Test::Utils  ();
+use PostgreSQL::Test::Utils ();
 use PostgreSQL::Test::BackgroundPsql ();

which I think we could just change to qw() if we cared enough (but I bet
we don't).


All in all, I think sticking to
--valign-exclusion-list=", = => =~ |= || && if or qw unless"
is a good deal.

-- 
Álvaro Herrera PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Ellos andaban todos desnudos como su madre los parió, y también las mujeres,
aunque no vi más que una, harto moza, y todos los que yo vi eran todos
mancebos, que ninguno vi de edad de más de XXX años" (Cristóbal Colón)

Re: Should vacuum process config file reload more often

[Daniel Gustafsson]

> On 27 Apr 2023, at 23:25, Daniel Gustafsson  wrote:
>> On 27 Apr 2023, at 16:53, Melanie Plageman  wrote:

>> Fix LGTM.
> 
> Thanks for review. I plan to push this in the morning.

Done, thanks.

--
Daniel Gustafsson

Build problem with square brackets in build path

[Nikolay Shaplov]

I am not sure it is really a bug, but nevertheless

If you do

mkdir [source]
git clone git://git.postgresql.org/git/postgresql.git [source]
mkdir build; cd build
../\[source\]/configure 
make

you will get

make[1]: *** No rule to make target 'generated-headers'.  Stop.

If there are no "[]" in the path to the source, everything is OK.

It would be OK for me, if it still does not work. But I would appreciate at 
least proper error message there (or at configure step), this would save time, 
for figuring out, why it suddenly stopped working.


-- 
Nikolay Shaplov aka Nataraj
Fuzzing Engineer at Postgres Professional
Matrix IM: @dhyan:nataraj.su


signature.asc
Description: This is a digitally signed message part.

Re: Support logical replication of DDLs

[Amit Kapila]

On Tue, Apr 25, 2023 at 9:28 AM Zhijie Hou (Fujitsu)
 wrote:
>

I have a few high-level comments on the deparsing approach used in the
patch. As per my understanding, we first build an ObjTree from the DDL
command, then convert the ObjTree to Jsonb which is then converted to
a JSON string.  Now, in the consecutive patch, via publication event
triggers, we get the JSON string via the conversions mentioned, WAL
log it, which then walsender will send to the subscriber, which will
convert the JSON string back to the DDL command and execute it.

Now, I think we can try to eliminate this entire ObjTree machinery and
directly from the JSON blob during deparsing. We have previously also
discussed this in an email chain at [1]. I think now the functionality
of JSONB has also been improved and we should investigate whether it
is feasible to directly use JSONB APIs to form the required blob.

The other general point is that one of the primary reasons to convert
DDL into JSONB blob is to allow replacing the elements. For example,
say on the publisher, the table is in Schema A and then on the
subscriber the same table is in Schema B, so, we would like to change
the Schema in the DDL before replaying it, or we want to change the
persistence of table to UNLOGGED before replaying the DDL on the
subscriber. Is it possible to have such an API exposed from this
module so that we can verify if that works? It can be a separate patch
though.

[1] - 
https://www.postgresql.org/message-id/CAB7nPqRX6w9UY%2B%3DOy2jqTVwi0hqT2y4%3DfUc7fNG4U-296JBvYQ%40mail.gmail.com

-- 
With Regards,
Amit Kapila.

Re: Add PQsendSyncMessage() to libpq

[Robert Haas]

On Fri, Mar 24, 2023 at 6:39 PM Anton Kirilov  wrote:
> I have attached a patch that introduces PQsendSyncMessage(), a
> function that is equivalent to PQpipelineSync(), except that it does
> not flush anything to the server; the user must subsequently call
> PQflush() instead. Alternatively, the new function is equivalent to
> PQsendFlushRequest(), except that it sends a sync message instead of a
> flush request. In addition to reducing the system call overhead of
> libpq's pipeline mode, it also makes it easier for the operating
> system to send as much of the pipeline as possible in a single TCP (or
> lower level protocol) packet when the database is running remotely.

I wonder whether this is the naming that we want. The two names are
significantly different. Something like PQpipelineSendSync() would be
more similar.

I also wonder, really even more, whether it would be better to do
something like PQpipelinePutSync(PGconn *conn, bool flush) with
PQpipelineSync(conn) just meaning PQpipelinePutSync(conn, true). We're
basically using the function name as a Boolean parameter to select the
behavior, which is fine if you only have one parameter and it's a
Boolean, but it's obviously unworkable if you have say 3 Boolean
parameters because you don't want 8 different functions, and what if
you need an integer parameter for some reason?

So I'd favor exposing a function that is effectively an extended
version of PQpipelineSendSync() with an additional Boolean parameter,
and that way if for some reason somebody needs to extend it again,
they can just make an even more extended version with yet another
parameter. That way, all the functionality is always available by
calling the newest function, and older ones are still there for older
applications.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

RE: [PATCH] Support % wildcard in extension upgrade filenames

[Regina Obe]

> 
> > As for Tom's concern about downgrades, I think it's valid but it's a
> > case that is easy to test for in Plpgsql and either handle or error.
> > For example, we use semver so testing for a downgrade at the top of
> > the upgrade script is trivial.
> 

I was thinking about this more.  The extension model as it stands doesn't
allow an extension author to define version hierarchy.  We handle this
internally in our scripts to detect a downgrade attempt and stop it similar
to what Mat is saying.

Most of that is basically converting our version string to a numeric number
which we largely do with a regex pattern.

I was thinking if there were some way to codify that regex pattern in our
control file, then wild cards can only be applied if such a pattern is
defined and the 

%--

The % has to be numerically before the target version.


Thanks,
Regina

Re: Possible regression setting GUCs on \connect

[Jonathan S. Katz]

On 4/27/23 8:04 PM, Alexander Korotkov wrote:

On Fri, Apr 28, 2023 at 2:30 AM Alexander Korotkov  wrote:

Additionally, I think if we start recording role OID, then we need a
full set of management clauses for each individual option ownership.
Otherwise, we would leave this new role OID without necessarily
management facilities.  But with them, the whole stuff will look like
awful overengineering.


I can also predict a lot of ambiguous cases.  For instance, we
existing setting can be overridden with a different role OID.  If it
has been overridden can the overwriter turn it back?


[RMT hat]

While the initial bug has been fixed, given there is discussion on 
reverting 096dd80f3, I've added this as an open item.


I want to study this a bit more before providing my own opinion on revert.

Thanks,

Jonathan




OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH] Support % wildcard in extension upgrade filenames

[Eric Ridge]

(I'm the developer of ZomboDB and pgrx, which while not an extension per se, 
allows others to make extensions that then need upgrade scripts.  So this topic 
is interesting to me.)

> On Mar 13, 2023, at 2:48 PM, Regina Obe  wrote:
> 
>>> I wonder if a solution to this problem might be to provide some kind
>>> of a version map file. Let's suppose that the map file is a bunch of
>>> lines of the form X Y Z, where X, Y, and Z are version numbers. The
>>> semantics could be: we (the extension authors) promise that if you
>>> want to upgrade from X to Z, it suffices to run the script that knows
>>> how to upgrade from Y to Z.
>>> This would address Tom's concern, because if you write a master
>>> upgrade script, you have to explicitly declare the versions to which
>>> it applies.
>> 
>> This would just move the problem from having 1968 files to having to write
>> 1968 lines in control files, 
> 
> 1968 lines in one control file, is still much nicer than 1968 files in my
> book.
> From a packaging standpoint also much cleaner, as that single file gets
> replaced with each upgrade and no need to uninstall more junk from prior
> install.


I tend to agree with this.  I like this mapping file idea.  Allowing an 
extension to declare what to use (Z) to get from X to Y is great.  In theory 
that's not much different than having a bunch of individual files, but in 
practice an extension likely could pare their actual file set down to just a 
few, and dealing with less files is a big win.  And then the mapping file would 
allow the extension author to make the tree as complex as necessary.

(I have some hand-wavy ideas for pgrx and autogenerating upgrade scripts and a 
file like this could help quite a bit.  Rust and rust-adjacent things prefer 
explicitness)

If this "wildcard in a filename" idea existed back in 2015 when I started 
ZomboDB I'm not sure I'd have used it, and I'm not sure I'd want to switch to 
using it now.  The ambiguities are too great when what I want is something 
that's obvious.  

Primarily, ZDB purposely doesn't support downgrade paths so I wouldn't want to 
use a pattern that implies it does.

ZomboDB has 137 releases over the past 8 years.  Each one of those adds an 
upgrade script from OLDVER--NEWVER.  Prior to the Rust rewrite, these were all 
hand-generated, and sometimes were just zero bytes.  I've since developed 
tooling to auto-generate upgrade scripts by diffing the full schemas for OLDVER 
and NEWVER and emitting whatever DDL can move OLDVER to NEWVER.  In practice 
this usually generates an upgrade script that just replaces 1 function... the 
"zdb.version()" function.  Of course, I've had to hand-edit these on occasion 
as well -- this is not a perfect system.

It might just be the nature of our extensions, but I don't recall ever needing 
DO statements in an upgrade script.  The extension.sql has a few, one of which 
is to optionally enable PostGIS support! haha  ZDB is fairly complex too.  
Hundreds of functions, dozens of types, an IAM implementation, a dozen views, a 
few tables, some operators.  I also don't see a lot of changes to ZDB's 
extension schema nowadays -- new releases are usually fixing some terrible bug.

(As an aside, I wish Postgres would show the line number in whatever .sql file 
when an ERROR occurs during CREATE EXTENSION or ALTER EXTENSION UPDATE.  That'd 
be a huge QoL improvement for me -- maybe it's my turn to put a patch together)

Just some musings from a guy hanging out here on the fringes of Postgres 
extension development.  Of the things I've seen in this thread I really like 
the mapping file idea and I don't have any original thoughts on the subject.

eric

Postgres Version want to update from 9.2 to 9.5 version in CentOS 7.9

[Gautham Raj]

Hi,

*Problem: Having multiple versions of Postgres installed in CentOS 7. I
Want to set the 9.5 version as default. Not able to access Postgres 9.5
through the terminal as well.*

   1. For Command *psql --version* I'm getting 9.5 as the version.
   2. For Command *sudo -u postgres psql *I'm getting 9.2 as the version.

Please look at the below screenshot.
[image: Screenshot from 2023-04-28 21-14-29.png]

*Background: *By default, the server has a 9.2 version I installed the
*rh-postgresql95* version from the below article.
Used the below command to install *rh-postgresql95*

> yum  
> --enablerepo=centos-sclo-rh
> -y install rh-postgresql95-postgresql-server
>
https://www.server-world.info/en/note?os=CentOS_7&p=postgresql95&f=1

Tried updating the PATH variable correctly with the latest version. But not
working.

[image: Screenshot from 2023-04-28 21-28-44.png]


Please share the steps or any guidance on how to resolve the issue.

Thankyou so much. Anything would be helpful.

Thanks & Regards,
Gautham

Re: Possible regression setting GUCs on \connect

[Pavel Borisov]

Hi!

On Fri, 28 Apr 2023 at 17:42, Jonathan S. Katz  wrote:
>
> On 4/27/23 8:04 PM, Alexander Korotkov wrote:
> > On Fri, Apr 28, 2023 at 2:30 AM Alexander Korotkov  
> > wrote:
> >> Additionally, I think if we start recording role OID, then we need a
> >> full set of management clauses for each individual option ownership.
> >> Otherwise, we would leave this new role OID without necessarily
> >> management facilities.  But with them, the whole stuff will look like
> >> awful overengineering.
> >
> > I can also predict a lot of ambiguous cases.  For instance, we
> > existing setting can be overridden with a different role OID.  If it
> > has been overridden can the overwriter turn it back?
>
> [RMT hat]
>
> While the initial bug has been fixed, given there is discussion on
> reverting 096dd80f3, I've added this as an open item.
>
> I want to study this a bit more before providing my own opinion on revert.

I see that 096dd80f3 is a lot simpler in implementation than
a0ffa885e, so I agree Alexander's opinion that it's good not to
overengineer what could be done simple. If we patched corner cases of
a0ffa885e before (by 13d838815), why not patch minor things in
096dd80f3 instead of reverting?

As I see in [1] there is some demand from users regarding this option.

Regards,
Pavel Borisov,
Supabase.

Re: Postgres Version want to update from 9.2 to 9.5 version in CentOS 7.9

[Tom Lane]

Gautham Raj  writes:
> *Problem: Having multiple versions of Postgres installed in CentOS 7. I
> Want to set the 9.5 version as default. Not able to access Postgres 9.5
> through the terminal as well.*

>1. For Command *psql --version* I'm getting 9.5 as the version.
>2. For Command *sudo -u postgres psql *I'm getting 9.2 as the version.

You'd need to read up on Red Hat's "SCL" packaging system to understand
how to use that rh-postgresql95 package.  SCL is since my time there,
but I'm pretty sure it's intentional that it's not in the default PATH.

But TBH, all the versions available from Red Hat for RHEL7/CentOS7 are
out of support as far as the upstream community is concerned; to us it's
pretty mystifying why you'd be trying to migrate to an already-years-dead
release branch.  I'd suggest getting some more modern release from
https://www.postgresql.org/download/

regards, tom lane

Re: Build problem with square brackets in build path

[Tom Lane]

Nikolay Shaplov  writes:
> If you do

> mkdir [source]
> git clone git://git.postgresql.org/git/postgresql.git [source]
> mkdir build; cd build
> ../\[source\]/configure 
> make

> you will get

> make[1]: *** No rule to make target 'generated-headers'.  Stop.

> If there are no "[]" in the path to the source, everything is OK.

It's generally quite unwise to use shell meta-characters in
file or directory names.  I give you one example:

$ ls ../[source]
COPYRIGHT   README.git  contrib/
GNUmakefile.in  aclocal.m4  doc/
HISTORY config/ meson.build
Makefileconfigure*  meson_options.txt
README  configure.acsrc/
$ ls ../[source]/*.ac
ls: ../[source]/*.ac: No such file or directory

This is expected behavior (I leave it as an exercise for the
student to figure out why).

While it might be possible to make the Postgres build scripts
proof against funny characters in the build paths, the effort
required would be far out of proportion to the value.  Not least
because manual operations in such a file tree would misbehave
often enough to convince you to change, even if the scripts were
all water-tight.

regards, tom lane

Re: run pgindent on a regular basis / scripted manner

[Bruce Momjian]

On Wed, Apr 26, 2023 at 03:44:47PM -0400, Andrew Dunstan wrote:
> On 2023-04-26 We 09:27, Tom Lane wrote:
> I doubt there's something like that. You can freeze arbitrary blocks of code
> like this (from the manual)
> 
> #<<<  format skipping: do not let perltidy change my nice formatting
>     my @list = (1,
>     1, 1,
>     1, 2, 1,
>     1, 3, 3, 1,
>     1, 4, 6, 4, 1,);
> #>>>   
> 
> 
> But that gets old and ugly pretty quickly.

Can those comments be added by a preprocessor before calling perltidy,
and then removed on completion?

-- 
  Bruce Momjian  https://momjian.us
  EDB  https://enterprisedb.com

  Embrace your flaws.  They make you human, rather than perfect,
  which you will never be.

Re: Postgres Version want to update from 9.2 to 9.5 version in CentOS 7.9

[Gautham Raj]

Thank you for the quick response.

Yes, I'm willing to get the latest version. I read some articles CentOS 7
doesn't support the latest versions. So was trying the old versions.

I tried the article shared but, got the below error at the step.

[image: image.png]
Something is wrong here.

Please suggest the steps for resolving this issue.

Thanks & Regards,
Gautham

On Fri, Apr 28, 2023 at 10:08 PM Tom Lane  wrote:

> Gautham Raj  writes:
> > *Problem: Having multiple versions of Postgres installed in CentOS 7. I
> > Want to set the 9.5 version as default. Not able to access Postgres 9.5
> > through the terminal as well.*
>
> >1. For Command *psql --version* I'm getting 9.5 as the version.
> >2. For Command *sudo -u postgres psql *I'm getting 9.2 as the version.
>
> You'd need to read up on Red Hat's "SCL" packaging system to understand
> how to use that rh-postgresql95 package.  SCL is since my time there,
> but I'm pretty sure it's intentional that it's not in the default PATH.
>
> But TBH, all the versions available from Red Hat for RHEL7/CentOS7 are
> out of support as far as the upstream community is concerned; to us it's
> pretty mystifying why you'd be trying to migrate to an already-years-dead
> release branch.  I'd suggest getting some more modern release from
> https://www.postgresql.org/download/
>
> regards, tom lane
>

Re: Postgres Version want to update from 9.2 to 9.5 version in CentOS 7.9

[Roberto Mello]

On Fri, Apr 28, 2023 at 12:10 PM Gautham Raj 
wrote:

> Thank you for the quick response.
>
> Yes, I'm willing to get the latest version. I read some articles CentOS 7
> doesn't support the latest versions. So was trying the old versions.
>
> I tried the article shared but, got the below error at the step.
>

You have bigger problems to solve with your setup. Basic programs are
failing to run, so there are other things that need fixing with your
package manager that are completely unrelated to PostgreSQL.

Although Tom kindly responded to your initial question, this list
(-hackers) is the wrong place to be asking that type of question, as its
purpose is to discuss the *development* of PostgreSQL, not installation and
upgrade issues.

There are many other places better suited to your question, including the
pgsql-general list [1], IRC channel [2] and Slack channel [3].

-Roberto

[1] https://www.postgresql.org/list/pgsql-general/
[2] https://www.postgresql.org/community/irc/
[3]
https://join.slack.com/t/postgresteam/shared_invite/zt-1qj14i9sj-E9WqIFlvcOiHsEk2yFEMjA

Re: Order changes in PG16 since ICU introduction

[Jeff Davis]

On Thu, 2023-04-27 at 14:23 +0200, Daniel Verite wrote:
> This should be pg_strcasecmp(...) == 0

Good catch, thank you! Fixed in updated patches.

> postgres=# create database lat9 locale 'fr_FR@euro' encoding LATIN9
> template
> 'template0';
> ERROR:  could not convert locale name "fr_FR@euro" to language tag:
> U_ILLEGAL_ARGUMENT_ERROR

ICU 63 and earlier convert it without error to the language tag 'fr-FR-
u-cu-eur', which is correct. ICU 64 removed support for transforming
some locale variants, because apparently they think those variants are
obsolete:

https://unicode-org.atlassian.net/browse/ICU-22268
https://unicode-org.atlassian.net/browse/ICU-20187

(Aside: how obsolete are those variants?)

It's frustrating that they'd remove such transformations from the
canonicalization process.

Fortunately, it looks like it's easy enough to do the transformation
ourselves. The only problematic format is '...@VARIANT'. The other
format 'fr_FR_EURO' doesn't seem to be a valid glibc locale name[1] and
windows seems to use BCP 47[2].

And there don't seem to be a lot of variants to handle. ICU 63 only
handles 3 variants, so that's what my patch does. Any unknown variant
between 5 and 8 characters won't throw an error. There could be more
problem cases, but I'm not sure how much of a practical problem they
are.

If we try to keep the meaning of LOCALE to only LC_COLLATE and
LC_CTYPE, that will continue to be confusing for anyone that uses
provider=icu.

Regards,
Jeff Davis

[1]
https://www.gnu.org/software/libc/manual/html_node/Locale-Names.html
[2]
https://learn.microsoft.com/en-us/windows/win32/intl/locale-names
From 6c0251c584edea64148604da52c8e55e43fe36e6 Mon Sep 17 00:00:00 2001
From: Jeff Davis 
Date: Fri, 21 Apr 2023 14:03:57 -0700
Subject: [PATCH v3 1/4] ICU: do not convert locale 'C' to 'en-US-u-va-posix'.

The conversion was intended to be for convenience, but it's more
likely to be confusing than useful.

The user can still directly specify 'en-US-u-va-posix' if desired.

Discussion: https://postgr.es/m/f83f089ee1e9acd5dbbbf3353294d24e1f196e95.ca...@j-davis.com
---
 src/backend/utils/adt/pg_locale.c | 19 +--
 src/bin/initdb/initdb.c   | 17 +
 .../regress/expected/collate.icu.utf8.out |  8 
 src/test/regress/sql/collate.icu.utf8.sql |  4 
 4 files changed, 14 insertions(+), 34 deletions(-)

diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index 51df570ce9..58c4c426bc 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -2782,26 +2782,10 @@ icu_language_tag(const char *loc_str, int elevel)
 {
 #ifdef USE_ICU
 	UErrorCode	 status;
-	char		 lang[ULOC_LANG_CAPACITY];
 	char		*langtag;
 	size_t		 buflen = 32;	/* arbitrary starting buffer size */
 	const bool	 strict = true;
 
-	status = U_ZERO_ERROR;
-	uloc_getLanguage(loc_str, lang, ULOC_LANG_CAPACITY, &status);
-	if (U_FAILURE(status))
-	{
-		if (elevel > 0)
-			ereport(elevel,
-	(errmsg("could not get language from locale \"%s\": %s",
-			loc_str, u_errorName(status;
-		return NULL;
-	}
-
-	/* C/POSIX locales aren't handled by uloc_getLanguageTag() */
-	if (strcmp(lang, "c") == 0 || strcmp(lang, "posix") == 0)
-		return pstrdup("en-US-u-va-posix");
-
 	/*
 	 * A BCP47 language tag doesn't have a clearly-defined upper limit
 	 * (cf. RFC5646 section 4.4). Additionally, in older ICU versions,
@@ -2889,8 +2873,7 @@ icu_validate_locale(const char *loc_str)
 
 	/* check for special language name */
 	if (strcmp(lang, "") == 0 ||
-		strcmp(lang, "root") == 0 || strcmp(lang, "und") == 0 ||
-		strcmp(lang, "c") == 0 || strcmp(lang, "posix") == 0)
+		strcmp(lang, "root") == 0 || strcmp(lang, "und") == 0)
 		found = true;
 
 	/* search for matching language within ICU */
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 2c208ead01..4086834458 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -2238,24 +2238,10 @@ icu_language_tag(const char *loc_str)
 {
 #ifdef USE_ICU
 	UErrorCode	 status;
-	char		 lang[ULOC_LANG_CAPACITY];
 	char		*langtag;
 	size_t		 buflen = 32;	/* arbitrary starting buffer size */
 	const bool	 strict = true;
 
-	status = U_ZERO_ERROR;
-	uloc_getLanguage(loc_str, lang, ULOC_LANG_CAPACITY, &status);
-	if (U_FAILURE(status))
-	{
-		pg_fatal("could not get language from locale \"%s\": %s",
- loc_str, u_errorName(status));
-		return NULL;
-	}
-
-	/* C/POSIX locales aren't handled by uloc_getLanguageTag() */
-	if (strcmp(lang, "c") == 0 || strcmp(lang, "posix") == 0)
-		return pstrdup("en-US-u-va-posix");
-
 	/*
 	 * A BCP47 language tag doesn't have a clearly-defined upper limit
 	 * (cf. RFC5646 section 4.4). Additionally, in older ICU versions,
@@ -2327,8 +2313,7 @@ icu_validate_locale(const char *loc_str)
 
 	/* check for special language name */
 	if (strcmp(lang, "") == 0 ||
-		strcmp(lang, "root") == 0 || strcmp(lang, "und") ==