Re: Adding SHOW CREATE TABLE

2023-05-21 Thread Kirk Wolak
On Fri, May 19, 2023 at 1:08 PM Andrew Dunstan  wrote:

> I think the ONLY place we should have this is in server side functions.
> More than ten years ago I did some work in this area (see below), but it's
> one of those things that have been on my ever growing personal TODO list
>
> See 
>  and
> 
> 
>
Andrew,
  Thanks for sharing that.  I reviewed your code.  10yrs, clearly it's not
working (as-is, but close), something interesting about the
structure you ended up in.  You check the type of the object and redirect
accordingly at the top level.  Hmmm...
What I liked was that each type gets handled (I was focused on "table"),
but I realized similarities.

  I don't know what the group would think, but I like the thought of
calling this, and having it "Correct" to call the appropriate function.
But not sure it will stand.  It does make obvious that some of these should
be spun out as "pg_get_typedef"..
pg_get_typedef
pg_get_domaindef
pg_get_sequencedef

  Finally, since you started this a while back, part of me is "leaning"
towards a function:
pg_get_columndef

  Which returns a properly formatted column for a table, type, or domain?
(one of the reasons for this, is that this is
the function with the highest probability to change, and potentially the
easiest to share reusability).

  Finally, I am curious about your opinion.  I noticed you used the
internal pg_ tables, versus the information_schema...
I am *thinking* that the information_schema will be more stable over
time... Thoughts?

Thank you for sharing your thoughts...
Kirk...


Re: pg_stat_io for the startup process

2023-05-21 Thread Kyotaro Horiguchi
At Sun, 21 May 2023 15:14:23 -0700, Andres Freund  wrote in 
> Hi,
> 
> I don't really feel confident we're going to get the timeout approach solid
> enough for something going into the tree around beta 1.
> 
> How about this, imo a lot simpler, approach: We flush stats whenever replaying
> a XLOG_RUNNING_XACTS record. Unless the primary is idle, it will log those at
> a regular interval. If the primary is idle, we don't need to flush stats in
> the startup process, because we'll not have done any io.
> 
> We only log XLOG_RUNNING_XACTS when wal_level >= replica, so stats wouldn't
> get regularly flushed if wal_level = minimal - but in that case the stats are
> also not accessible, so that's not a problem.

I concur with the three aspects, interval regularity, reliability and
about the case of the minimal wal_level. While I can't confidently
claim it is the best, its simplicilty gives us a solid reason to
proceed with it.  If necessary, we can switch to alternative timing
sources in the future without causing major disruptions for users, I
believe.

> It's not the prettiest solution, but I think the simplicity is worth a lot.
> 
> Greetings,

+1.


-- 
Kyotaro Horiguchi
NTT Open Source Software Center




Re: LLVM 16 (opaque pointers)

2023-05-21 Thread Thomas Munro
Oh, one important thing I forgot to mention: that patch is for LLVM 16
only, and I developed it with a local build of their "release/16.x"
branch on a FreeBSD box, and also tested with a released package for
16 on a Debian box.  Further changes are already needed for their
"main" branch (LLVM 17-to-be), so this won't quite be enough to shut
seawasp up.  At a glance, we will need to change from the "old pass
manager" API that has recently been vaporised[1]
(llvm-c/Transforms/PassManagerBuilder.h) to the new one[2][3]
(llvm-c/Transforms/PassBuilder.h), which I suspect/hope will be as
simple as changing llvmjit.c to call LLVMRunPasses() with a string
describing the passes we want in "opt -passes" format, instead of our
code that calls LLVMAddFunctionInlingPass() etc.  But that'll be a
topic for another day, and another thread.

[1] 
https://github.com/llvm/llvm-project/commit/0aac9a2875bad4f065367e4a6553fad78605f895
[2] https://llvm.org/docs/NewPassManager.html
[3] https://reviews.llvm.org/D102136




RE: [PoC] pg_upgrade: allow to upgrade publisher node

2023-05-21 Thread Wei Wang (Fujitsu)
On Tues, May 16, 2023 at 14:15 PM Kuroda, Hayato/黒田 隼人 
 wrote:
> Dear Peter,
> 
> Thanks for reviewing! PSA new version patchset.

Thanks for updating the patch set.

Here are some comments:
===
For patches 0001

1. The latest patch set fails to apply because the new commit (0245f8d) in HEAD.

~~~

2. In file pg_dump.h.
```
+/*
+ * The LogicalReplicationSlotInfo struct is used to represent replication
+ * slots.
+ *
+ * XXX: add more attributes if needed
+ */
+typedef struct _LogicalReplicationSlotInfo
+{
+   DumpableObject dobj;
+   char   *plugin;
+   char   *slottype;
+   booltwophase;
+} LogicalReplicationSlotInfo;
```

Do we need the structure member "slottype"? It seems we do not use "slottype"
because we only dump logical replication slot.

===
For patch 0002

3. In the function SaveSlotToPath
```
-   /* and don't do anything if there's nothing to write */
-   if (!was_dirty)
+   /*
+* and don't do anything if there's nothing to write, unless it's this 
is
+* called for a logical slot during a shutdown checkpoint, as we want to
+* persist the confirmed_flush_lsn in that case, even if that's the only
+* modification.
+*/
+   if (!was_dirty && !is_shutdown && !SlotIsLogical(slot))
```
It seems that the code isn't consistent with our expectation.
If this is called for a physical slot during a shutdown checkpoint and there's
nothing to write, I think it will also persist physical slots to disk.

===
For patch 0003

4. In the function check_for_parameter_settings
```
+   /* --include-logical-replication-slots can be used since PG16. */
+   if (GET_MAJOR_VERSION(new_cluster->major_version < 1600))
+   return;
```
It seems that there is a slight mistake (the input of GET_MAJOR_VERSION) in the
if-condition:
GET_MAJOR_VERSION(new_cluster->major_version < 1600)
->
GET_MAJOR_VERSION(new_cluster->major_version) <= 1500

Please also check the similar if-conditions in the below two functions
check_for_confirmed_flush_lsn (in 0003 patch)
check_are_logical_slots_active (in 0004 patch)

Regards,
Wang wei


Re: createuser --memeber and PG 16

2023-05-21 Thread Nathan Bossart
On Mon, May 22, 2023 at 09:11:18AM +0900, Michael Paquier wrote:
> On Sun, May 21, 2023 at 12:16:58PM -0700, Nathan Bossart wrote:
>> Alright.  Barring any additional feedback, I'll commit this tonight.
> 
> v2 passes the eye test, and I am not spotting any references to the
> past option names.  Thanks!

Committed.  Thanks for taking a look.  I'll keep an eye on the buildfarm
for a few.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com




Re: PG 16 draft release notes ready

2023-05-21 Thread Bruce Momjian
On Sun, May 21, 2023 at 10:13:41AM -0700, Andres Freund wrote:
> Hi,
> 
> Thanks for the release notes!
> 
> > 
> > 
> > 
> > 
> > Allow more efficient addition of multiple heap and index pages (Andres 
> > Freund)
> > 
> > 
> 
> While the case of extending by multiple pages improved the most, even
> extending by a single page at a time got a good bit more scalable. Maybe just
> "Improve efficiency of extending relations"?

Do average users know heap and index files are both relations?  That
seems too abstract so I spelled out heap and index pages.

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

  Only you can decide what is important to you.




Re: PG 16 draft release notes ready

2023-05-21 Thread Bruce Momjian
On Mon, May 22, 2023 at 09:03:11AM +0800, jian he wrote:
> In E.1.2. Migration to Version 16, probably need mention, some
> privilege command cannot restore.
> if new cluster bootstrap superuser name is not the same as old one. "GRANT x 
> TO
> y GRANTED BY no_bootstrap_superuser; " will have error.
> 
> ---pg15 dump content.
> CREATE ROLE jian;
> ALTER ROLE jian WITH SUPERUSER INHERIT CREATEROLE CREATEDB LOGIN REPLICATION
> BYPASSRLS;
> CREATE ROLE regress_priv_user1;
> ALTER ROLE regress_priv_user1 WITH NOSUPERUSER INHERIT NOCREATEROLE NOCREATEDB
> LOGIN NOREPLICATION NOBYPASSRLS;
> CREATE ROLE regress_priv_user2;
> ALTER ROLE regress_priv_user2 WITH NOSUPERUSER INHERIT NOCREATEROLE NOCREATEDB
> LOGIN NOREPLICATION NOBYPASSRLS;
> CREATE ROLE su1;
> ALTER ROLE su1 WITH SUPERUSER INHERIT CREATEROLE NOCREATEDB LOGIN 
> NOREPLICATION
> NOBYPASSRLS;
> GRANT regress_priv_user1 TO regress_priv_user2 GRANTED BY su1;
> 
> ---restore in pg16
> \i /home/jian/Desktop/dumpall_schema.sql
> 2023-05-22 08:46:00.170 CST [456584] ERROR:  permission denied to grant
> privileges as role "su1"
> 2023-05-22 08:46:00.170 CST [456584] DETAIL:  The grantor must have the ADMIN
> option on role "regress_priv_user1".
> 2023-05-22 08:46:00.170 CST [456584] STATEMENT:  GRANT regress_priv_user1 TO
> regress_priv_user2 GRANTED BY su1;
> psql:/home/jian/Desktop/dumpall_schema.sql:32: ERROR:  permission denied to
> grant privileges as role "su1"
> DETAIL:  The grantor must have the ADMIN option on role "regress_priv_user1".

Agreed, new text:





Prevent removal of superuser privileges for the bootstrap user (Robert 
Haas)



--> Restoring such users could lead to errors.



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

  Only you can decide what is important to you.




Re: PG 16 draft release notes ready

2023-05-21 Thread Bruce Momjian
On Sun, May 21, 2023 at 10:13:41AM -0700, Andres Freund wrote:
> Hi,
> 
> Thanks for the release notes!
> 
> > 
> > 
> > 
> > 
> > Allow more efficient addition of multiple heap and index pages (Andres 
> > Freund)
> > 
> > 
> 
> While the case of extending by multiple pages improved the most, even
> extending by a single page at a time got a good bit more scalable. Maybe just
> "Improve efficiency of extending relations"?

Okay, I made this change:

-Allow more efficient addition of multiple heap and index pages (Andres 
Freund)
+Allow more efficient addition of heap and index pages (Andres Freund)

> I think:
> 
> > 
> > 
> > 
> > 
> > Allow logical decoding on standbys (Bertrand Drouvot, Andres Freund, Amit 
> > Khandekar)
> > 
> > 
> 
> pretty much includes:
> 
> > 
> > 
> > 
> > 
> > Allow invalidation of replication slots due to row removal, wal_level, and 
> > conflicts (Bertrand Drouvot, Andres Freund, Amit Khandekar)
> > 
> 
> as it is a prerequisite.

Okay, I merged the commit entries and the authors, and removed the item.

> I'd probably also merge
> 
> > 
> > 
> > 
> > 
> > Add function pg_log_standby_snapshot() to force creation of a WAL snapshot 
> > (Bertrand Drouvot)
> > 
> > 
> > 
> > WAL snapshots are required for logical slot creation so this function 
> > speeds their creation on standbys.
> > 
> > 
> 
> As there really isn't a use case outside of logical decoding on a standby.

Okay new merged item is:





Allow logical decoding on standbys (Bertrand Drouvot, Andres Freund, 
Amit Khandekar, Bertrand Drouvot)





New function pg_log_standby_snapshot() forces creation of WAL snapshots.
Snapshots are required for logical slot creation so this function 
speeds their creation on standbys.



> > 
> > 
> > 
> > 
> > Prevent extension libraries from export their symbols by default (Andres 
> > Freund, Tom Lane)
> > 
> > 
> 
> s/export/exporting/?

Seems Tom's patch already fixed that.

> Looking through the release notes, I didn't see an entry for
> 
> commit c6e0fe1f2a08505544c410f613839664eea9eb21
> Author: David Rowley 
> Date:   2022-08-29 17:15:00 +1200
>  
> Improve performance of and reduce overheads of memory management
> 
> even though I think that's one of the more impactful improvements. What was
> the reason for leaving that out?

If you read my previous email:

> For the above two items, I mention items that would change user 
> like new features or changes that are significant enough that they would
> change user behavior.  For example, if a new join method increases
> performance by 5x, that could change user behavior.  Based on the quoted
> numbers above, I didn't think "hash now faster" would be appropriate to
> mention.  Right?

I can see this item as a big win, but I don't know how to describe it in a way
that is helpful for the user to know.

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

  Only you can decide what is important to you.




Re: Initial Schema Sync for Logical Replication

2023-05-21 Thread Masahiko Sawada
On Fri, Apr 28, 2023 at 4:16 PM Masahiko Sawada  wrote:
> 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.

We have been discussing how to sync schema but I'd like to step back a
bit and discuss use cases and requirements of this feature.

Suppose that a table belongs to a publication, what objects related to
the table we want to sync by the initial schema sync features? IOW, do
we want to sync table's ACLs, tablespace settings, triggers, and
security labels too?

If we want to replicate the whole database, e.g. when using logical
replication for major version upgrade, it would be convenient if it
synchronizes all table-related objects. However, if we have only this
option, it could be useless in some cases. For example, in a case
where users have different database users on the subscriber than the
publisher, they might want to sync only CREATE TABLE, and set ACL etc
by themselves. In this case, it would not be necessary to sync ACL and
security labels.

What use case do we want to support by this feature? I think the
implementation could be varied depending on how to select what objects
to sync.

One possible idea is to select objects to sync depending on how DDL
replication is set in the publisher. It's straightforward but I'm not
sure the design of DDL replication syntax has been decided. Also, even
if we create a publication with ddl = 'table' option, it's not clear
to me that we want to sync table-dependent triggers, indexes, and
rules too by the initial sync feature.

Second idea is to make it configurable by users so that they can
specify what objects to sync. But it would make the feature complex
and I'm not sure users can use it properly.

Third idea is that since the use case of synchronizing the whole
database can be achievable even by pg_dump(all), we support
synchronizing only tables (+ indexes) in the initial sync feature,
which can not be achievable by pg_dump.

Feedback is very welcome.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com




Re: PG 16 draft release notes ready

2023-05-21 Thread jian he
In E.1.2. Migration to Version 16, probably need mention, some
privilege command cannot restore.
if new cluster bootstrap superuser name is not the same as old one. "GRANT
x TO y GRANTED BY no_bootstrap_superuser; " will have error.

---pg15 dump content.
CREATE ROLE jian;
ALTER ROLE jian WITH SUPERUSER INHERIT CREATEROLE CREATEDB LOGIN
REPLICATION BYPASSRLS;
CREATE ROLE regress_priv_user1;
ALTER ROLE regress_priv_user1 WITH NOSUPERUSER INHERIT NOCREATEROLE
NOCREATEDB LOGIN NOREPLICATION NOBYPASSRLS;
CREATE ROLE regress_priv_user2;
ALTER ROLE regress_priv_user2 WITH NOSUPERUSER INHERIT NOCREATEROLE
NOCREATEDB LOGIN NOREPLICATION NOBYPASSRLS;
CREATE ROLE su1;
ALTER ROLE su1 WITH SUPERUSER INHERIT CREATEROLE NOCREATEDB LOGIN
NOREPLICATION NOBYPASSRLS;
GRANT regress_priv_user1 TO regress_priv_user2 GRANTED BY su1;

---restore in pg16
\i /home/jian/Desktop/dumpall_schema.sql
2023-05-22 08:46:00.170 CST [456584] ERROR:  permission denied to grant
privileges as role "su1"
2023-05-22 08:46:00.170 CST [456584] DETAIL:  The grantor must have the
ADMIN option on role "regress_priv_user1".
2023-05-22 08:46:00.170 CST [456584] STATEMENT:  GRANT regress_priv_user1
TO regress_priv_user2 GRANTED BY su1;
psql:/home/jian/Desktop/dumpall_schema.sql:32: ERROR:  permission denied to
grant privileges as role "su1"
DETAIL:  The grantor must have the ADMIN option on role
"regress_priv_user1".


Re: WAL Insertion Lock Improvements

2023-05-21 Thread Michael Paquier
On Fri, May 19, 2023 at 08:34:16PM +0530, Bharath Rupireddy wrote:
> I get it. How about the following similar to what
> ProcessProcSignalBarrier() has?
> 
> + * Note that pg_atomic_exchange_u64 is a full barrier, so we're 
> guaranteed
> + * that the variable is updated before waking up waiters.
> + */
> 
> + * Note that pg_atomic_exchange_u64 is a full barrier, so we're 
> guaranteed
> + * that the variable is updated before releasing the lock.
>   */
> 
> Please find the attached v8 patch with the above change.

Simpler and consistent, nice.  I don't have much more to add, so I
have switched the patch as RfC.
--
Michael


signature.asc
Description: PGP signature


Re: ERROR: wrong varnullingrels (b 5 7) (expected (b)) for Var 3/3

2023-05-21 Thread Michael Paquier
On Fri, May 19, 2023 at 03:46:26PM +0800, Richard Guo wrote:
> Thanks for testing.  I looked into it and figured out that it is the
> same issue discussed at the end of this thread:
> https://www.postgresql.org/message-id/CAMbWs4-EU9uBGSP7G-iTwLBhRQ%3DrnZKvFDhD%2Bn%2BxhajokyPCKg%40mail.gmail.com

OK, thanks for confirming.
--
Michael


signature.asc
Description: PGP signature


Re: Add PQsendSyncMessage() to libpq

2023-05-21 Thread Anton Kirilov

Hello,

On 02/05/2023 00:55, Michael Paquier wrote:
> Well, these are nice numbers.  At ~1% I am ready to buy the noise
> argument, but what would the range of the usual noise when it comes to
> multiple runs under the same conditions>

I managed to get my patch tested in the TechEmpower Framework Benchmarks 
continuous benchmarking environment, and even though it takes roughly a 
week to get a new set of results, now there had been a couple of runs 
both with and without my changes. All 4 database-bound Web application 
tests (single query, multiple queries, fortunes, and data updates) saw 
improvements, by approximately 8.94%, 0.64%, 9.54%, and 2.78% 
respectively. The standard errors were 0.65% or less, so there was 
practically no change in the second test. However, I have seen another 
implementation experience a much larger improvement (~6.69%) in that 
test from essentially the same optimization, so I think that my own code 
has another bottleneck. Note that these test runs were not in the same 
benchmarking environment as the one I used previously for a quick check, 
so the values differ. Also, another set of results should become 
available in a week or so (and would be based on my optimization).


Links to the test runs:
https://www.techempower.com/benchmarks/#section=test=1ecf679a-9686-4de7-a3b7-de16a1a84bb6=zik0zi-35r=zhb2tb-zik0zj-zik0zj-sf=db
https://www.techempower.com/benchmarks/#section=test=aab00736-445c-4b7f-83b5-451c47c83395=zik0zi-35r=zhb2tb-zik0zj-zik0zj-sf=db
https://www.techempower.com/benchmarks/#section=test=bc7f7570-a88e-48e3-9874-06d7dc0a0f74=zik0zi-35r=zhb2tb-zik0zj-zik0zj-sf=db
https://www.techempower.com/benchmarks/#section=test=e6dd1abd-7aa2-4846-9b44-d8fd8a23d385=zik0zi-35r=zhb2tb-zik0zj-zik0zj-sf=db
(ordered chronologically; the first 2 did not include my optimization)

Best wishes,
Anton Kirilov




Re: createuser --memeber and PG 16

2023-05-21 Thread Michael Paquier
On Sun, May 21, 2023 at 12:16:58PM -0700, Nathan Bossart wrote:
> On Sun, May 21, 2023 at 01:20:01PM -0400, Tom Lane wrote:
> > Nathan Bossart  writes:
>>> How do folks feel about keeping --role undocumented?  Should we give it a
>>> mention in the docs for --member-of?
>> 
>> I'm okay with leaving it undocumented, but I won't fight about it
>> if somebody wants to argue for the other.
> 
> Alright.  Barring any additional feedback, I'll commit this tonight.

v2 passes the eye test, and I am not spotting any references to the
past option names.  Thanks!

+$node->issues_sql_like(
+   [ 'createuser', 'regress_user11', '--role', 'regress_user1' ],
+   qr/statement: CREATE ROLE regress_user11 NOSUPERUSER NOCREATEDB 
NOCREATEROLE INHERIT LOGIN NOREPLICATION NOBYPASSRLS IN ROLE regress_user1;/,
+   '--role (for backward compatibility)');

Not sure I would have kept this test, still that's cheap enough to
test.
--
Michael


signature.asc
Description: PGP signature


Re: PG 16 draft release notes ready

2023-05-21 Thread Jonathan S. Katz

On 5/21/23 3:04 PM, Jonathan S. Katz wrote:

On 5/18/23 4:49 PM, Bruce Momjian wrote:

I have completed the first draft of the PG 16 release notes.


One thing that we could attempt for this beta is to include a 
prospective list of "major features + enhancements." Of course it can 
change before the GA, but it'll give readers some idea of things to test.


I'd propose the following (in no particular order):

* General performance improvements for read-heavy workloads (looking for 
clarification for that in[1])


Per [1] this sounds like it should be:

* Optimization to reduce overall memory usage, including general 
performance improvements.


We can get more specific for the GA.

Thanks,

Jonathan

[1] 
https://www.postgresql.org/message-id/5749E807-A5B7-4CC7-8282-84F6F0D4D1D0%40anarazel.de




OpenPGP_signature
Description: OpenPGP digital signature


Re: PG 16 draft release notes ready

2023-05-21 Thread Jonathan S. Katz

On 5/21/23 3:24 PM, Andres Freund wrote:

Hi,

On May 21, 2023 11:46:56 AM PDT, "Jonathan S. Katz"  
wrote:

On 5/21/23 1:13 PM, Andres Freund wrote:



Looking through the release notes, I didn't see an entry for

commit c6e0fe1f2a08505544c410f613839664eea9eb21
Author: David Rowley 
Date:   2022-08-29 17:15:00 +1200
Improve performance of and reduce overheads of memory management

even though I think that's one of the more impactful improvements. What was
the reason for leaving that out?


IIUC in[1], would this "just speed up" read-heavy workloads?


I don't think so. It can speed up write workloads as well. But more importantly 
it can noticeably reduce memory usage, including for things like the relcache.


Cool! I'll dive more into the thread later to learn more.

Jonathan


OpenPGP_signature
Description: OpenPGP digital signature


Re: pg_stat_io for the startup process

2023-05-21 Thread Andres Freund
Hi,

I don't really feel confident we're going to get the timeout approach solid
enough for something going into the tree around beta 1.

How about this, imo a lot simpler, approach: We flush stats whenever replaying
a XLOG_RUNNING_XACTS record. Unless the primary is idle, it will log those at
a regular interval. If the primary is idle, we don't need to flush stats in
the startup process, because we'll not have done any io.

We only log XLOG_RUNNING_XACTS when wal_level >= replica, so stats wouldn't
get regularly flushed if wal_level = minimal - but in that case the stats are
also not accessible, so that's not a problem.

It's not the prettiest solution, but I think the simplicity is worth a lot.

Greetings,

Andres Freund




Re: PG 16 draft release notes ready

2023-05-21 Thread Bruce Momjian
On Sun, May 21, 2023 at 01:11:05PM -0400, Tom Lane wrote:
> Bruce Momjian  writes:
> > On Sun, May 21, 2023 at 09:30:01PM +0900, Ian Lawrence Barwick wrote:
> >> 2ceea5adb Accept "+infinity" in date and timestamp[tz] input.
> 
> > I have this but didn't add that commit, added.
> 
> That's really not related to the commit you added it to...
> 
> I don't have time today to read through all the relnotes, but I went
> through those that have my name on them.  Suggested wording modifications
> attached.

These were all good, patch applied, thanks.

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

  Only you can decide what is important to you.




Re: Assert failure of the cross-check for nullingrels

2023-05-21 Thread Tom Lane
I wrote:
> If they have the same clause_relids, then clearly in its current
> form clause_is_computable_at() cannot distinguish them.  So what
> I now think we should do is have clause_is_computable_at() examine
> their required_relids instead.  Those will be different, by
> construction in deconstruct_distribute_oj_quals(), ensuring that
> we select only one of the group of clones.

Since we're hard up against the beta1 wrap deadline, I went ahead
and pushed the v5 patch.  I doubt that it's perfect yet, but it's
a small change and demonstrably fixes the cases we know about.

As I said in the commit message, the main knock I'd lay on v5
is "why not use required_relids all the time?".  I tried to do
that and soon found that the answer is that we're not maintaining
required_relids very accurately.  I found two causes so far:

1. equivclass.c sometimes generates placeholder constant-true
join clauses, and it's being sloppy about that.  It copies
the required_relids of the original clause, but fails to copy
is_pushed_down, making the clause look like it's been assigned
to the wrong side of the join-clause-vs-filter-clause divide.
I found that we need to copy has_clone/is_clone as well.  The
attached quick-hack patch avoids the bugs, but now I feel like
it was a mistake to not add has_clone/is_clone as full-fledged
arguments of make_restrictinfo.  I'm inclined to change that,
but not right before beta1 when we have no evidence of a reachable
bug.  (Mind you, there might *be* a reachable bug, but ...)

2. When distribute_qual_to_rels forces a qual up to a particular
syntactic level, it applies a relid set that very possibly refers
to rels the clause doesn't actually depend on.  This is problematic
because if the clause gets postponed to above some outer join that
nulls those rels, then it looks like it's being evaluated in an
unsafe location.  I think that when we detect commutability of two
outer joins, we need to adjust the relevant min_xxxhand sets more
thoroughly than we do now.  I've not managed to write a patch
for that yet.  One problem is that if we insist on removing all
unreferenced rels from required_relids, we might end up with a
set that mentions *none* of the RHS and therefore fails to keep
the clause from dropping into the LHS where it must not go.
Not sure about a nice way to handle that.

regards, tom lane

diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c
index 2db1bf6448..e0077aa1e4 100644
--- a/src/backend/optimizer/path/equivclass.c
+++ b/src/backend/optimizer/path/equivclass.c
@@ -1993,20 +1993,24 @@ reconsider_outer_join_clauses(PlannerInfo *root)
 			if (reconsider_outer_join_clause(root, ojcinfo, true))
 			{
 RestrictInfo *rinfo = ojcinfo->rinfo;
+RestrictInfo *ninfo;
 
 found = true;
 /* remove it from the list */
 root->left_join_clauses =
 	foreach_delete_current(root->left_join_clauses, cell);
 /* throw back a dummy replacement clause (see notes above) */
-rinfo = make_restrictinfo(root,
+ninfo = make_restrictinfo(root,
 		  (Expr *) makeBoolConst(true, false),
-		  true, /* is_pushed_down */
-		  false,	/* pseudoconstant */
+		  rinfo->is_pushed_down,
+		  false,	/* !pseudoconstant */
 		  0,	/* security_level */
 		  rinfo->required_relids,
 		  rinfo->outer_relids);
-distribute_restrictinfo_to_rels(root, rinfo);
+/* copy clone marking, too */
+ninfo->has_clone = rinfo->has_clone;
+ninfo->is_clone = rinfo->is_clone;
+distribute_restrictinfo_to_rels(root, ninfo);
 			}
 		}
 
@@ -2018,20 +2022,24 @@ reconsider_outer_join_clauses(PlannerInfo *root)
 			if (reconsider_outer_join_clause(root, ojcinfo, false))
 			{
 RestrictInfo *rinfo = ojcinfo->rinfo;
+RestrictInfo *ninfo;
 
 found = true;
 /* remove it from the list */
 root->right_join_clauses =
 	foreach_delete_current(root->right_join_clauses, cell);
 /* throw back a dummy replacement clause (see notes above) */
-rinfo = make_restrictinfo(root,
+ninfo = make_restrictinfo(root,
 		  (Expr *) makeBoolConst(true, false),
-		  true, /* is_pushed_down */
-		  false,	/* pseudoconstant */
+		  rinfo->is_pushed_down,
+		  false,	/* !pseudoconstant */
 		  0,	/* security_level */
 		  rinfo->required_relids,
 		  rinfo->outer_relids);
-distribute_restrictinfo_to_rels(root, rinfo);
+/* copy clone marking, too */
+ninfo->has_clone = rinfo->has_clone;
+ninfo->is_clone = rinfo->is_clone;
+distribute_restrictinfo_to_rels(root, ninfo);
 			}
 		}
 
@@ -2043,20 +2051,24 @@ reconsider_outer_join_clauses(PlannerInfo *root)
 			if (reconsider_full_join_clause(root, ojcinfo))
 			{
 RestrictInfo *rinfo = ojcinfo->rinfo;
+RestrictInfo *ninfo;
 
 found = true;
 /* remove it from the list */
 root->full_join_clauses =
 	

Re: PG 16 draft release notes ready

2023-05-21 Thread Andres Freund
Hi,

On May 21, 2023 11:46:56 AM PDT, "Jonathan S. Katz"  
wrote:
>On 5/21/23 1:13 PM, Andres Freund wrote:
>
>> 
>> Looking through the release notes, I didn't see an entry for
>> 
>> commit c6e0fe1f2a08505544c410f613839664eea9eb21
>> Author: David Rowley 
>> Date:   2022-08-29 17:15:00 +1200
>>Improve performance of and reduce overheads of memory management
>> 
>> even though I think that's one of the more impactful improvements. What was
>> the reason for leaving that out?
>
>IIUC in[1], would this "just speed up" read-heavy workloads?

I don't think so. It can speed up write workloads as well. But more importantly 
it can noticeably reduce memory usage, including for things like the relcache.

Andres
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.




Re: createuser --memeber and PG 16

2023-05-21 Thread Nathan Bossart
On Sun, May 21, 2023 at 01:20:01PM -0400, Tom Lane wrote:
> Nathan Bossart  writes:
>> Fixed.
> 
> v2 looks good to me, except the documentation wording for --with-role
> is needlessly inconsistent with --with-admin.  The --with-admin
> wording looks better, so I suggest
> 
> -Indicates the specified existing role should be automatically
> +Specifies an existing role that will be automatically
>  added as a member of the new role. Multiple existing roles can

Will do.

>> How do folks feel about keeping --role undocumented?  Should we give it a
>> mention in the docs for --member-of?
> 
> I'm okay with leaving it undocumented, but I won't fight about it
> if somebody wants to argue for the other.

Alright.  Barring any additional feedback, I'll commit this tonight.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com




Re: PG 16 draft release notes ready

2023-05-21 Thread Jonathan S. Katz

On 5/18/23 4:49 PM, Bruce Momjian wrote:

I have completed the first draft of the PG 16 release notes.


One thing that we could attempt for this beta is to include a 
prospective list of "major features + enhancements." Of course it can 
change before the GA, but it'll give readers some idea of things to test.


I'd propose the following (in no particular order):

* General performance improvements for read-heavy workloads (looking for 
clarification for that in[1])


* Parallel execution of queries that use `FULL` and `OUTER` joins

* Logical replication allowed from read-only standbys

* Logical replication subscribers can apply large transactions in parallel

* Monitoring of I/O statistics through the `pg_stat_io` view

* Addition of SQL/JSON constructors and identity functions

* Optimizations to the vacuum freezing strategy

* Support for regular expressions for matching usernames and databases 
names in `pg_hba.conf`, and user names in `pg_ident.conf`


The above is tossing items at the wall, and I'm OK with any or all being 
modified or replaced.


Thanks,

Jonathan

[1] 
https://postgr.es/m/CAApHDvpjauCRXcgcaL6+e3eqecEHoeRm9D-kcbuvBitgPnW=v...@mail.gmail.com


OpenPGP_signature
Description: OpenPGP digital signature


Re: PG 16 draft release notes ready

2023-05-21 Thread Jonathan S. Katz

On 5/21/23 1:13 PM, Andres Freund wrote:



Looking through the release notes, I didn't see an entry for

commit c6e0fe1f2a08505544c410f613839664eea9eb21
Author: David Rowley 
Date:   2022-08-29 17:15:00 +1200
  
 Improve performance of and reduce overheads of memory management


even though I think that's one of the more impactful improvements. What was
the reason for leaving that out?


IIUC in[1], would this "just speed up" read-heavy workloads?

Thanks,

Jonathan

[1] 
https://www.postgresql.org/message-id/CAApHDvpjauCRXcgcaL6%2Be3eqecEHoeRm9D-kcbuvBitgPnW%3Dvw%40mail.gmail.com




OpenPGP_signature
Description: OpenPGP digital signature


Re: createuser --memeber and PG 16

2023-05-21 Thread Tom Lane
Nathan Bossart  writes:
> Fixed.

v2 looks good to me, except the documentation wording for --with-role
is needlessly inconsistent with --with-admin.  The --with-admin
wording looks better, so I suggest

-Indicates the specified existing role should be automatically
+Specifies an existing role that will be automatically
 added as a member of the new role. Multiple existing roles can

> How do folks feel about keeping --role undocumented?  Should we give it a
> mention in the docs for --member-of?

I'm okay with leaving it undocumented, but I won't fight about it
if somebody wants to argue for the other.

regards, tom lane




Re: Add PQsendSyncMessage() to libpq

2023-05-21 Thread Anton Kirilov

Hello,

On 05/05/2023 16:02, Anton Kirilov wrote:
On Thu, 4 May 2023, 11:36 Alvaro Herrera, > wrote:


On 2023-May-04, Anton Kirilov wrote:
If you want to make sure it's fully flushed, your only option is to have
the call block.


Surely PQflush() returning 0 would signify that the output buffer has 
been fully flushed?
Since I haven't got any further comments, I assume that I am correct, so 
here is an updated version of the patch that should address all feedback 
that I have received so far and all issues that I have identified.


Thanks,
Anton KirilovFrom 8ba588e0356fd5310d34c5301d99676af8c34fdf Mon Sep 17 00:00:00 2001
From: Anton Kirilov 
Date: Wed, 22 Mar 2023 20:39:57 +
Subject: [PATCH v3] Add PQpipelinePutSync() to libpq

This new function is equivalent to PQpipelineSync(),
except that it lets the caller choose whether anything
is flushed to the server. Its purpose is to reduce the
system call overhead of pipeline mode.
---
 doc/src/sgml/libpq.sgml   | 52 ---
 src/interfaces/libpq/exports.txt  |  1 +
 src/interfaces/libpq/fe-exec.c| 29 ---
 src/interfaces/libpq/libpq-fe.h   |  6 +++
 .../modules/libpq_pipeline/libpq_pipeline.c   | 37 +
 .../traces/multi_pipelines.trace  | 11 
 6 files changed, 120 insertions(+), 16 deletions(-)

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index cce25d06e6..5aac3c9172 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -3490,8 +3490,9 @@ ExecStatusType PQresultStatus(const PGresult *res);
   

 The PGresult represents a
-synchronization point in pipeline mode, requested by
-.
+synchronization point in pipeline mode, requested by either
+ or
+.
 This status occurs only when pipeline mode has been selected.

   
@@ -5019,7 +5020,8 @@ int PQsendDescribePortal(PGconn *conn, const char *portalName);
,
,
,
-   , or
+   ,
+   , or

call, and returns it.
A null pointer is returned when the command is complete and there
@@ -5385,7 +5387,8 @@ int PQflush(PGconn *conn);
  or its prepared-query sibling
  .
  These requests are queued on the client-side until flushed to the server;
- this occurs when  is used to
+ this occurs when  (or optionally
+ ) is used to
  establish a synchronization point in the pipeline,
  or when  is called.
  The functions ,
@@ -5399,8 +5402,9 @@ int PQflush(PGconn *conn);
  client sends them.  The server will begin executing the commands in the
  pipeline immediately, not waiting for the end of the pipeline.
  Note that results are buffered on the server side; the server flushes
- that buffer when a synchronization point is established with
- PQpipelineSync, or when
+ that buffer when a synchronization point is established with either
+ PQpipelineSync or
+ PQpipelinePutSync, or when
  PQsendFlushRequest is called.
  If any statement encounters an error, the server aborts the current
  transaction and does not execute any subsequent command in the queue
@@ -5457,7 +5461,8 @@ int PQflush(PGconn *conn);
  PGresult types PGRES_PIPELINE_SYNC
  and PGRES_PIPELINE_ABORTED.
  PGRES_PIPELINE_SYNC is reported exactly once for each
- PQpipelineSync at the corresponding point
+ PQpipelineSync or
+ PQpipelinePutSync at the corresponding point
  in the pipeline.
  PGRES_PIPELINE_ABORTED is emitted in place of a normal
  query result for the first error and all subsequent results
@@ -5495,7 +5500,8 @@ int PQflush(PGconn *conn);
  PQresultStatus will report a
  PGRES_PIPELINE_ABORTED result for each remaining queued
  operation in an aborted pipeline. The result for
- PQpipelineSync is reported as
+ PQpipelineSync or
+ PQpipelinePutSync is reported as
  PGRES_PIPELINE_SYNC to signal the end of the aborted pipeline
  and resumption of normal result processing.
 
@@ -5727,6 +5733,36 @@ int PQsendFlushRequest(PGconn *conn);

   
  
+
+
+ PQpipelinePutSyncPQpipelinePutSync
+
+ 
+  
+   Marks a synchronization point in a pipeline by sending a
+   sync message.
+   This serves as the delimiter of an implicit transaction and
+   an error recovery point; see .
+
+
+int PQpipelinePutSync(PGconn *conn, int flags);
+
+  
+  
+   Returns 0 for success. Returns -1 if the connection is not in
+   pipeline mode or sending a
+   sync message
+   failed. Returns 1 if it was unable to send all the data in
+   the send queue yet (this case can only occur if the connection
+   is nonblocking and output data is flushed to the server).
+   The flags argument is a bitwise OR of
+   

Re: PG 16 draft release notes ready

2023-05-21 Thread Andres Freund
Hi,

Thanks for the release notes!

> 
> 
> 
> 
> Allow more efficient addition of multiple heap and index pages (Andres Freund)
> 
> 

While the case of extending by multiple pages improved the most, even
extending by a single page at a time got a good bit more scalable. Maybe just
"Improve efficiency of extending relations"?


I think:

> 
> 
> 
> 
> Allow logical decoding on standbys (Bertrand Drouvot, Andres Freund, Amit 
> Khandekar)
> 
> 

pretty much includes:

> 
> 
> 
> 
> Allow invalidation of replication slots due to row removal, wal_level, and 
> conflicts (Bertrand Drouvot, Andres Freund, Amit Khandekar)
> 

as it is a prerequisite.

I'd probably also merge

> 
> 
> 
> 
> Add function pg_log_standby_snapshot() to force creation of a WAL snapshot 
> (Bertrand Drouvot)
> 
> 
> 
> WAL snapshots are required for logical slot creation so this function speeds 
> their creation on standbys.
> 
> 

As there really isn't a use case outside of logical decoding on a standby.


> 
> 
> 
> 
> Prevent extension libraries from export their symbols by default (Andres 
> Freund, Tom Lane)
> 
> 

s/export/exporting/?


Looking through the release notes, I didn't see an entry for

commit c6e0fe1f2a08505544c410f613839664eea9eb21
Author: David Rowley 
Date:   2022-08-29 17:15:00 +1200
 
Improve performance of and reduce overheads of memory management

even though I think that's one of the more impactful improvements. What was
the reason for leaving that out?

Greetings,

Andres Freund




Re: PostgreSQL 16 Beta 1 release announcement draft

2023-05-21 Thread Nathan Bossart
On Sun, May 21, 2023 at 12:51:05PM -0400, Jonathan S. Katz wrote:
> On 5/19/23 10:57 AM, Nathan Bossart wrote:
>> [pg_use_]reserved_connections might also deserve a mention here.  AFAICT
>> it's the only new predefined role that isn't mentioned in the announcement.
>> I'm okay with leaving it out if folks don't think it should make the cut.
> 
> I'm not sure how widely used this one would be, so I left it out. However,
> open to other opinions.

Fair enough.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com




Re: PG 16 draft release notes ready

2023-05-21 Thread Tom Lane
Bruce Momjian  writes:
> On Sun, May 21, 2023 at 09:30:01PM +0900, Ian Lawrence Barwick wrote:
>> 2ceea5adb Accept "+infinity" in date and timestamp[tz] input.

> I have this but didn't add that commit, added.

That's really not related to the commit you added it to...

I don't have time today to read through all the relnotes, but I went
through those that have my name on them.  Suggested wording modifications
attached.

regards, tom lane

diff --git a/doc/src/sgml/release-16.sgml b/doc/src/sgml/release-16.sgml
index e156284b71..b31e31fccd 100644
--- a/doc/src/sgml/release-16.sgml
+++ b/doc/src/sgml/release-16.sgml
@@ -60,6 +60,8 @@ Change assignment rules for PL/pgSQL bound cursor variables (Tom Lane)
 
 
 Previously, the string value of such variables was set to match the variable name during cursor assignment;  now it will be assigned during OPEN, and will not match the variable name.
+To restore the previous behavior, assign the desired portal name to the cursor
+variable before OPEN.
 
 
 
@@ -257,7 +259,7 @@ Author: Tom Lane 
 
 
 
-Allow memoize atop of UNION ALL and partitions (Richard Guo)
+Allow memoize atop a UNION ALL (Richard Guo)
 
 
 
@@ -268,7 +270,8 @@ Author: Tom Lane 
 
 
 
-Allow anti-joins to be constructed on the right/outer side (Richard Guo)
+Allow anti-joins to be performed with the non-nullable input as the
+inner relation (Richard Guo)
 
 
 
@@ -924,7 +927,7 @@ Allow makeaclitem() to accept multiple privilege names (Robins Tharakan)
 
 
 
-Previously only a single privilege names, like SELECT, were supported.
+Previously only a single privilege name, like SELECT, was accepted.
 
 
 
@@ -972,11 +975,7 @@ Author: Tom Lane 
 
 
 
-Store server variables in a hash table (Tom Lane)
-
-
-
-This allows the faster addition of server variables.
+Improve performance of server variable management (Tom Lane)
 
 
 
@@ -1081,7 +1080,9 @@ Allow the postmaster to terminate children with an abort signal (Tom Lane)
 
 
 
-Abort normally creates a core dump.  This is controlled by send_abort_for_crash and send_abort_for_kill.  postmaster -T is now the same as setting send_abort_for_crash.
+This allows collection of a core dump for a stuck child process.
+This is controlled by send_abort_for_crash and send_abort_for_kill.
+The postmaster's -T switch is now the same as setting send_abort_for_crash.
 
 
 
@@ -1092,7 +1093,7 @@ Author: Tom Lane 
 
 
 
-Remove the unnecessary postmaster -n option (Tom Lane)
+Remove the non-functional postmaster -n option (Tom Lane)
 
 
 
@@ -1462,7 +1463,8 @@ Author: Tom Lane 
 
 
 
-Add EXPLAIN option GENERIC_PLAN to display the query's generic plan (Laurenz Albe)
+Add EXPLAIN option GENERIC_PLAN to display the generic plan
+for a parameterized query (Laurenz Albe)
 
 
 
@@ -1542,7 +1544,7 @@ Author: Tom Lane 
 
 
 
-Add VACUUM option to skip or update all frozen statistics (Tom Lane, Nathan Bossart)
+Add VACUUM options to skip or update all frozen statistics (Tom Lane, Nathan Bossart)
 
 
 
@@ -1635,13 +1637,23 @@ This can improve readability for long strings of digits.
 
+
+
+
+Accept the spelling "+infinity" in datetime input (Vik Fearing)
+
+
+
+
 
 
 
-Prevent the specification of "epoch" and "infinity" with other units in datetime strings (Joseph Koshakow)
+Prevent the specification of "epoch" and "infinity" together with other fields
+in datetime strings (Joseph Koshakow)
 
 
 
@@ -1652,7 +1664,9 @@ Author: Tom Lane 
 
 
 
-Remove support for datetime input that prefixes year-month-day by Y/M/D (Joseph Koshakow)
+Remove undocumented support for date input in the form
+"YyearMmonthDday"
+(Joseph Koshakow)
 
 
 
@@ -1909,7 +1923,7 @@ Author: Tom Lane 
 
 
 
-Allow to_reg* functions to accept OIDs parameters (Tom Lane)
+Allow to_reg* functions to accept numeric OIDs as input (Tom Lane)
 
 
 
@@ -2024,7 +2038,8 @@ Author: Tom Lane 
 
 
 
-Allow ECPG variable declarations to use type names which match SQL keywords (Tom Lane)
+Allow ECPG variable declarations to use typedef names that match unreserved
+SQL keywords (Tom Lane)
 
 
 
@@ -2130,7 +2145,8 @@ Author: Tom Lane 
 
 
 
-Allow psql to detect the exit status of shell commands and queries (Corey Huinker, Tom Lane)
+Allow psql scripts to obtain the exit status of shell commands and queries
+(Corey Huinker, Tom Lane)
 
 
 
@@ -2558,7 +2574,12 @@ Author: Andres Freund 
 
 
 
-Prevent extension libraries from export their symbols by default (Andres Freund, Tom Lane)
+Prevent extension libraries from exporting their symbols by default (Andres Freund, Tom Lane)
+
+
+
+Functions that need to be called from the core backend or other extensions
+must now be explicitly marked PGDLLEXPORT.
 
 
 
@@ -2980,7 +3001,8 @@ Author: Tom Lane 
 
 
 
-Allow the schemas of dependent extensions to be referenced using the new syntax @extschema:dependent_extension_name@ (Regina Obe)
+Allow the schemas of required extensions to be referenced in extension scripts
+using the new syntax 

Re: createuser --memeber and PG 16

2023-05-21 Thread Nathan Bossart
On Sun, May 21, 2023 at 11:45:24AM -0400, Tom Lane wrote:
> A few comments on the patch:

Thanks for taking a look.

>>>  Indicates an existing role that will be automatically added as a 
>>> member of the new
> 
> "Specifies" would be clearer than "indicates" (not your fault, but
> let's avoid the passive construction while we are here).  Likewise
> nearby.

Fixed.

>>> +   {"member-of", required_argument, NULL, 6},
> 
> Why didn't you just translate this as 'g' instead of inventing
> a new switch case?

Fixed.  *facepalm*

> I think clearer would be
> 
>>> +   printf(_("  -a, --with-admin=ROLE ROLE will be a member of new role 
>>> with admin\n"
> 
> Likewise
> 
>>> +   printf(_("  -g, --member-of=ROLE  new role will be a member of 
>>> ROLE\n"));
> 
> (I assume that's what this should say, it's backwards ATM)
> and
> 
>>> +   printf(_("  -m, --with-member=ROLEROLE will be a member of new 
>>> role\n"));

Fixed.

How do folks feel about keeping --role undocumented?  Should we give it a
mention in the docs for --member-of?

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
diff --git a/doc/src/sgml/ref/createuser.sgml b/doc/src/sgml/ref/createuser.sgml
index 58ed111642..448c28a056 100644
--- a/doc/src/sgml/ref/createuser.sgml
+++ b/doc/src/sgml/ref/createuser.sgml
@@ -82,10 +82,10 @@ PostgreSQL documentation
 
  
   -a role
-  --admin=role
+  --with-admin=role
   

-Indicates an existing role that will be automatically added as a member of the new
+Specifies an existing role that will be automatically added as a member of the new
 role with admin option, giving it the right to grant membership in the
 new role to others.  Multiple existing roles can be specified by
 writing multiple -a switches.
@@ -149,10 +149,10 @@ PostgreSQL documentation
 
  
   -g role
-  --role=role
+  --member-of=role
   

-Indicates the new role should be automatically added as a member
+Specifies the new role should be automatically added as a member
 of the specified existing role. Multiple existing roles can be
 specified by writing multiple -g switches.

@@ -222,10 +222,10 @@ PostgreSQL documentation
 
  
   -m role
-  --member=role
+  --with-member=role
   

-Indicates the specified existing role should be automatically
+Specifies the specified existing role should be automatically
 added as a member of the new role. Multiple existing roles can
 be specified by writing multiple -m switches.

diff --git a/src/bin/scripts/createuser.c b/src/bin/scripts/createuser.c
index 0c7f454be5..2d5e2452f7 100644
--- a/src/bin/scripts/createuser.c
+++ b/src/bin/scripts/createuser.c
@@ -28,19 +28,21 @@ int
 main(int argc, char *argv[])
 {
 	static struct option long_options[] = {
-		{"admin", required_argument, NULL, 'a'},
+		{"with-admin", required_argument, NULL, 'a'},
 		{"connection-limit", required_argument, NULL, 'c'},
 		{"createdb", no_argument, NULL, 'd'},
 		{"no-createdb", no_argument, NULL, 'D'},
 		{"echo", no_argument, NULL, 'e'},
 		{"encrypted", no_argument, NULL, 'E'},
-		{"role", required_argument, NULL, 'g'},
+		{"role", required_argument, NULL, 'g'}, /* kept for backward
+ * compatibility */
+		{"member-of", required_argument, NULL, 'g'},
 		{"host", required_argument, NULL, 'h'},
 		{"inherit", no_argument, NULL, 'i'},
 		{"no-inherit", no_argument, NULL, 'I'},
 		{"login", no_argument, NULL, 'l'},
 		{"no-login", no_argument, NULL, 'L'},
-		{"member", required_argument, NULL, 'm'},
+		{"with-member", required_argument, NULL, 'm'},
 		{"port", required_argument, NULL, 'p'},
 		{"pwprompt", no_argument, NULL, 'P'},
 		{"createrole", no_argument, NULL, 'r'},
@@ -414,19 +416,19 @@ help(const char *progname)
 	printf(_("Usage:\n"));
 	printf(_("  %s [OPTION]... [ROLENAME]\n"), progname);
 	printf(_("\nOptions:\n"));
-	printf(_("  -a, --admin=ROLE  this role will be a member of new role with admin\n"
+	printf(_("  -a, --with-admin=ROLE ROLE will be a member of new role with admin\n"
 			 "option\n"));
 	printf(_("  -c, --connection-limit=N  connection limit for role (default: no limit)\n"));
 	printf(_("  -d, --createdbrole can create new databases\n"));
 	printf(_("  -D, --no-createdb role cannot create databases (default)\n"));
 	printf(_("  -e, --echoshow the commands being sent to the server\n"));
-	printf(_("  -g, --role=ROLE   new role will be a member of this role\n"));
+	printf(_("  -g, --member-of=ROLE  new role will be a member of ROLE\n"));
 	printf(_("  -i, --inherit role inherits privileges of roles it is a\n"
 			 "member of (default)\n"));
 	printf(_("  -I, --no-inherit  role does not inherit privileges\n"));
 	printf(_("  

Re: PostgreSQL 16 Beta 1 release announcement draft

2023-05-21 Thread Jonathan S. Katz

On 5/19/23 12:17 AM, Jonathan S. Katz wrote:

Hi,

Attached is a draft of the release announcement for PostgreSQL 16 Beta 
1. The goal of this announcement is to get people excited about testing 
the beta and highlight many of the new features.


Please review for inaccuracies, omissions, and other suggestions / errors.

Please provide feedback no later than May 24, 0:00 AoE. This will give 
me enough time to incorporate the changes prior to the release the next 
day.


Thanks everyone for your feedback. Here is the updated text that 
combines all of the feedback from both -advocacy and -hackers.


Thanks,

Jonathan
The PostgreSQL Global Development Group announces that the first beta release of
PostgreSQL 16 is now [available for 
download](https://www.postgresql.org/download/).
This release contains previews of all features that will be available when
PostgreSQL 16 is made generally available, though some details of the release
can change during the beta period.

You can find information about all of the features and changes found in
PostgreSQL 16 in the [release 
notes](https://www.postgresql.org/docs/16/release-16.html):

  
[https://www.postgresql.org/docs/16/release-16.html](https://www.postgresql.org/docs/16/release-16.html)

In the spirit of the open source PostgreSQL community, we strongly encourage you
to test the new features of PostgreSQL 16 on your systems to help us eliminate
bugs or other issues that may exist. While we do not advise you to run
PostgreSQL 16 Beta 1 in production environments, we encourage you to find ways
to run your typical application workloads against this beta release.

Your testing and feedback will help the community ensure that the PostgreSQL 16
release upholds our standards of delivering a stable, reliable release of the
world's most advanced open source relational database. Please read more about
our [beta testing process](https://www.postgresql.org/developer/beta/) and how
you can contribute:

  
[https://www.postgresql.org/developer/beta/](https://www.postgresql.org/developer/beta/)

PostgreSQL 16 Feature Highlights


### Logical replication enhancements

Logical replication lets PostgreSQL users stream data in real-time to other
PostgreSQL or other external systems that implement the logical protocol. Until
PostgreSQL 16, users could only create logical replication publishers on primary
instances. PostgreSQL 16 adds the ability to perform logical decoding on a
standby instance, giving users more options to distribute their workload, for
example, use a standby that's less busy than a primary to logically replicate
changes.

PostgreSQL 16 also includes several performance improvements to logical
replication. This includes allowing the subscriber to apply large transactions
in parallel, use indexes other than the `PRIMARY KEY` to perform lookups during
`UPDATE` or `DELETE` operations, and allow for tables to be copied using binary
format during initialization.

### Performance

PostgreSQL 16 includes performance improvements in query execution. This release
adds more query parallelism, including allowing `FULL` and `OUTER` joins to
execute in parallel, and parallel execution of the `string_agg` and `array_agg`
aggregate functions. Additionally, PostgreSQL 16 can use incremental sorts in
`SELECT DISTINCT` queries. There are also several optimizations for
[window 
queries](https://www.postgresql.org/docs/devel/sql-expressions.html#SYNTAX-WINDOW-FUNCTIONS),
improvements in lookups for `RANGE` and `LIST` partitions, and support for
"anti-joins" in `RIGHT` and `OUTER` queries.

This release also introduces support for CPU acceleration using SIMD for both
x86 and ARM architectures, including optimizations for processing ASCII and JSON
strings, and array and substransaction searches. Additionally, PostgreSQL 16
introduces [load 
balancing](https://www.postgresql.org/docs/devel/libpq-connect.html#LIBPQ-CONNECT-LOAD-BALANCE-HOSTS)
to libpq, the client library for PostgreSQL.

### Developer Experience

PostgreSQL 16 continues to implement the 
[SQL/JSON](https://www.postgresql.org/docs/devel/functions-json.html)
standard for manipulating 
[JSON](https://www.postgresql.org/docs/devel/datatype-json.html)
data, including support for SQL/JSON constructors (e.g. `JSON_ARRAY()`,
`JSON_ARRAYAGG()` et al), and identity functions (`IS JSON`). This release also
adds the SQL standard 
[`ANY_VALUE`](https://www.postgresql.org/docs/devel/functions-aggregate.html#id-1.5.8.27.5.2.4.1.1.1.1)
aggregate function, which returns any arbitrary value from the aggregate set.
For convenience, PostgreSQL 16 now lets you specify non-decimal integer
literals, such as `0xff`, `0o777`, and `0b101010`, and use thousands separators,
such as `5_432`.

This release adds support for the extended query protocol to the 
[`psql`](https://www.postgresql.org/docs/devel/app-psql.html)
client. Users can execute a query, e.g. `SELECT $1 + $2`, and use the

Re: PostgreSQL 16 Beta 1 release announcement draft

2023-05-21 Thread Jonathan S. Katz

On 5/19/23 10:57 AM, Nathan Bossart wrote:

On Fri, May 19, 2023 at 12:17:50AM -0400, Jonathan S. Katz wrote:



PostgreSQL 16 continues to give users to the ability grant privileged access to
features without requiring superuser with new
[predefined roles](https://www.postgresql.org/docs/devel/predefined-roles.html).
These include `pg_maintain`, which enables execution of operations such as
`VACUUM`, `ANALYZE`, `REINDEX`, and others, and `pg_createsubscription`, which
allows users to create a logical replication subscription. Additionally,
starting with release, logical replication subscribers execute transactions on a
table as the table owner, not the superuser.


[pg_use_]reserved_connections might also deserve a mention here.  AFAICT
it's the only new predefined role that isn't mentioned in the announcement.
I'm okay with leaving it out if folks don't think it should make the cut.


I'm not sure how widely used this one would be, so I left it out. 
However, open to other opinions.


Thanks,

Jonathan



OpenPGP_signature
Description: OpenPGP digital signature


Re: PostgreSQL 16 Beta 1 release announcement draft

2023-05-21 Thread Jonathan S. Katz

On 5/19/23 1:42 AM, Erik Rijkers wrote:

Op 5/19/23 om 06:17 schreef Jonathan S. Katz:

Hi,

Attached is a draft of the release announcement for PostgreSQL 16 Beta 


Hi,


The usual small fry:


'continues to to'  should be
'continues to'

'continues to give users to the ability'  should be
'continues to give users the ability to'

'pg_createsubscription'  should be
'pg_create_subscription'

'starting with release'  should be
'starting with this release'

'credentials to connected to other services'  should be
'credentials to connect to other services'


Thanks Erik. I made all of these changes and will upload them in the 
next review.


Jonathan



OpenPGP_signature
Description: OpenPGP digital signature


Re: walsender performance regression due to logical decoding on standby changes

2023-05-21 Thread Andres Freund
Hi,

On 2023-05-18 20:11:11 +0530, Bharath Rupireddy wrote:
> Please find the attached v4 patch addressing the review comment (not
> the fastpath one).

I pushed a mildly edited version of this. I didn't like the name of the CVs
much, so I renamed them to wal_flush_cv/wal_replay_cv. I also did some minor
comment polishing.

Greetings,

Andres Freund




Re: walsender performance regression due to logical decoding on standby changes

2023-05-21 Thread Andres Freund
Hi,

On 2023-05-19 12:07:56 +0900, Kyotaro Horiguchi wrote:
> At Thu, 18 May 2023 20:11:11 +0530, Bharath Rupireddy 
>  wrote in 
> > > > + ConditionVariableInit(>physicalWALSndCV);
> > > > + ConditionVariableInit(>logicalWALSndCV);
> > >
> > > It's not obvious to me that it's worth having two CVs, because it's more
> > > expensive to find no waiters in two CVs than to find no waiters in one CV.
> > 
> > I disagree. In the tight per-WAL record recovery loop, WalSndWakeup
> > wakes up logical walsenders for every WAL record, but it wakes up
> > physical walsenders only if the applied WAL record causes a TLI
> > switch. Therefore, the extra cost of spinlock acquire-release for per
> > WAL record applies only for logical walsenders. On the other hand, if
> > we were to use a single CV, we would be unnecessarily waking up (if at
> > all they are sleeping) physical walsenders for every WAL record -
> > which is costly IMO.
> 
> As I was reading this, I start thinking that one reason for the
> regression could be to exccessive frequency of wakeups during logical
> replication. In physical replication, we make sure to avoid exccessive
> wakeups when the stream is tightly packed.  I'm just wondering why
> logical replication doesn't (or can't) do the same thing, IMHO.

It's possible we could try to reduce the frequency by issuing wakeups only at
specific points. The most obvious thing to do would be to wake only when
waiting for more WAL or when crossing a page boundary, or such. Unfortunately
that could easily lead to deadlocks, because the startup process might be
blocked waiting for a lock, held by a backend doing logical decoding - which
can't progress until the startup process wakes the backend up.

So I don't think this is promising avenue in the near term.

Greetings,

Andres Freund




Re: Naming of gss_accept_deleg

2023-05-21 Thread Stephen Frost
Greetings,

* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Abhijit Menon-Sen  writes:
> > At 2023-05-20 23:21:57 -0400, t...@sss.pgh.pa.us wrote:
> >> I thought the plan was to also rename the libpq "gssdeleg" connection
> >> parameter and so on?  I can look into that tomorrow, if nobody beats
> >> me to it.
> 
> > I was trying the change to see if it would be better to name it
> > "gssdelegate" instead (as in delegate on one side, and accept the
> > delegation on the other), but decided that "gssdelegation=enable"
> > reads better than "gssdelegate=enable".
> 
> Yeah, agreed.
> 
> > Here's the diff.
> 
> Thanks for doing that legwork!  I found a couple other places where
> "deleg" had escaped notice, and changed the lot.  Watching the
> buildfarm now ...

Thanks all for taking this up over a weekend.

Stephen


signature.asc
Description: PGP signature


Re: PG 16 draft release notes ready

2023-05-21 Thread Bruce Momjian
On Sun, May 21, 2023 at 09:30:01PM +0900, Ian Lawrence Barwick wrote:
> 2023年5月19日(金) 5:49 Bruce Momjian :
> >
> > I have completed the first draft of the PG 16 release notes.  You can
> > see the output here:
> >
> > https://momjian.us/pgsql_docs/release-16.html
> >
> > I will adjust it to the feedback I receive;  that URL will quickly show
> > all updates.
> 
> Hi
> 
> Below are a few commits which are not referenced in the current iteration
> of the release notes, but which seem worthy of inclusion.
> Apologies if they have been previously discussed, or I'm overlooking something
> obvious.
> 
> d09dbeb9b Speedup hash index builds by skipping needless binary searches
>   "Testing has shown that this can improve hash index build speeds by 5-15%
>with a unique set of integer values."
> 
> e09d7a126 Improve speed of hash index build.
>   "This seems to be good for overall
>   speedup of 5%-9%, depending on the incoming data."

For the above two items, I mention items that would change user behavior
like new features or changes that are significant enough that they would
change user behavior.  For example, if a new join method increases
performance by 5x, that could change user behavior.  Based on the quoted
numbers above, I didn't think "hash now faster" would be appropriate to
mention.  Right?

> 594f8d377 Allow batching of inserts during cross-partition updates.
>   seems reasonable to mention this as it's related to 97da48246, which
>   is mentioned in the notes

I wasn't sure if that was significant, based on the above logic, but
97da48246 has a user API to control it so I mentioned that one.

> 1349d2790 Improve performance of ORDER BY / DISTINCT aggregates
>   This is the basis for da5800d5, which is mentioned in the notes, but AFAICS
>the latter is an implementation fix for the former (haven't looked
> into either
>in detail though).

I have added this commit to the existing entry, thanks.

> The following are probably not headline features, but are the kind of
> behavioural
> changes I'd expect to find in the release notes (when, at some point
> in the far and
> distant future, trying to work out when they were introduced when considering
> application compatibility etc.):
> 
> 13a185f54 Allow publications with schema and table of the same schema.

This seemed like a rare enough case that I did not add it.

> 2ceea5adb Accept "+infinity" in date and timestamp[tz] input.

I have this but didn't add that commit, added.

> d540a02a7 Display the leader apply worker's PID for parallel apply workers.

Parallelism of apply is a new feature and I don't normally mention
output _additions_ that are related to new features.

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

  Only you can decide what is important to you.




Re: createuser --memeber and PG 16

2023-05-21 Thread Tom Lane
Bruce Momjian  writes:
> On Sun, May 21, 2023 at 08:22:05AM -0700, Nathan Bossart wrote:
>> I've attached a draft patch for this.  I also changed --admin to
>> --with-admin.

> If we want to go forward with this, the big question is whether we want
> to get this in before beta1.  FYI, the release notes don't mention the
> option names.

+1 for doing it before beta1.

A few comments on the patch:

>>  Indicates an existing role that will be automatically added as a 
>> member of the new

"Specifies" would be clearer than "indicates" (not your fault, but
let's avoid the passive construction while we are here).  Likewise
nearby.

>> +{"member-of", required_argument, NULL, 6},

Why didn't you just translate this as 'g' instead of inventing
a new switch case?

>> -printf(_("  -a, --admin=ROLE  this role will be a member of new 
>> role with admin\n"
>> +printf(_("  -a, --with-admin=ROLE this role will be a member of new 
>> role with admin\n"

I think clearer would be

>> +printf(_("  -a, --with-admin=ROLE ROLE will be a member of new role 
>> with admin\n"

Likewise

>> +printf(_("  -g, --member-of=ROLE  new role will be a member of 
>> ROLE\n"));

(I assume that's what this should say, it's backwards ATM)
and

>> +printf(_("  -m, --with-member=ROLEROLE will be a member of new 
>> role\n"));

regards, tom lane




Re: createuser --memeber and PG 16

2023-05-21 Thread Bruce Momjian
On Sun, May 21, 2023 at 08:22:05AM -0700, Nathan Bossart wrote:
> On Sun, May 21, 2023 at 07:44:49AM -0700, Nathan Bossart wrote:
> > On Sun, May 21, 2023 at 08:00:15AM +0200, Peter Eisentraut wrote:
> >> Maybe
> >> 
> >> createuser --with-members
> >> 
> >> and
> >> 
> >> createuser --member-of
> >> 
> >> would be clearer.
> > 
> > Those seem like reasonable choices to me.  I suspect we'll want to keep
> > --role around for backward compatibility.
> 
> I've attached a draft patch for this.  I also changed --admin to
> --with-admin.

If we want to go forward with this, the big question is whether we want
to get this in before beta1.  FYI, the release notes don't mention the
option names.

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

  Only you can decide what is important to you.




Re: createuser --memeber and PG 16

2023-05-21 Thread Nathan Bossart
On Sun, May 21, 2023 at 07:44:49AM -0700, Nathan Bossart wrote:
> On Sun, May 21, 2023 at 08:00:15AM +0200, Peter Eisentraut wrote:
>> Maybe
>> 
>> createuser --with-members
>> 
>> and
>> 
>> createuser --member-of
>> 
>> would be clearer.
> 
> Those seem like reasonable choices to me.  I suspect we'll want to keep
> --role around for backward compatibility.

I've attached a draft patch for this.  I also changed --admin to
--with-admin.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
diff --git a/doc/src/sgml/ref/createuser.sgml b/doc/src/sgml/ref/createuser.sgml
index 58ed111642..062509deb8 100644
--- a/doc/src/sgml/ref/createuser.sgml
+++ b/doc/src/sgml/ref/createuser.sgml
@@ -82,7 +82,7 @@ PostgreSQL documentation
 
  
   -a role
-  --admin=role
+  --with-admin=role
   

 Indicates an existing role that will be automatically added as a member of the new
@@ -149,7 +149,7 @@ PostgreSQL documentation
 
  
   -g role
-  --role=role
+  --member-of=role
   

 Indicates the new role should be automatically added as a member
@@ -222,7 +222,7 @@ PostgreSQL documentation
 
  
   -m role
-  --member=role
+  --with-member=role
   

 Indicates the specified existing role should be automatically
diff --git a/src/bin/scripts/createuser.c b/src/bin/scripts/createuser.c
index 0c7f454be5..77cb0bb8e1 100644
--- a/src/bin/scripts/createuser.c
+++ b/src/bin/scripts/createuser.c
@@ -28,7 +28,7 @@ int
 main(int argc, char *argv[])
 {
 	static struct option long_options[] = {
-		{"admin", required_argument, NULL, 'a'},
+		{"with-admin", required_argument, NULL, 'a'},
 		{"connection-limit", required_argument, NULL, 'c'},
 		{"createdb", no_argument, NULL, 'd'},
 		{"no-createdb", no_argument, NULL, 'D'},
@@ -40,7 +40,7 @@ main(int argc, char *argv[])
 		{"no-inherit", no_argument, NULL, 'I'},
 		{"login", no_argument, NULL, 'l'},
 		{"no-login", no_argument, NULL, 'L'},
-		{"member", required_argument, NULL, 'm'},
+		{"with-member", required_argument, NULL, 'm'},
 		{"port", required_argument, NULL, 'p'},
 		{"pwprompt", no_argument, NULL, 'P'},
 		{"createrole", no_argument, NULL, 'r'},
@@ -56,6 +56,7 @@ main(int argc, char *argv[])
 		{"interactive", no_argument, NULL, 3},
 		{"bypassrls", no_argument, NULL, 4},
 		{"no-bypassrls", no_argument, NULL, 5},
+		{"member-of", required_argument, NULL, 6},
 		{NULL, 0, NULL, 0}
 	};
 
@@ -123,6 +124,7 @@ main(int argc, char *argv[])
 			case 'E':
 /* no-op, accepted for backward compatibility */
 break;
+			case 6:
 			case 'g':
 simple_string_list_append(, optarg);
 break;
@@ -414,19 +416,19 @@ help(const char *progname)
 	printf(_("Usage:\n"));
 	printf(_("  %s [OPTION]... [ROLENAME]\n"), progname);
 	printf(_("\nOptions:\n"));
-	printf(_("  -a, --admin=ROLE  this role will be a member of new role with admin\n"
+	printf(_("  -a, --with-admin=ROLE this role will be a member of new role with admin\n"
 			 "option\n"));
 	printf(_("  -c, --connection-limit=N  connection limit for role (default: no limit)\n"));
 	printf(_("  -d, --createdbrole can create new databases\n"));
 	printf(_("  -D, --no-createdb role cannot create databases (default)\n"));
 	printf(_("  -e, --echoshow the commands being sent to the server\n"));
-	printf(_("  -g, --role=ROLE   new role will be a member of this role\n"));
+	printf(_("  -g, --member-of=ROLE  new role will be a member of this role\n"));
 	printf(_("  -i, --inherit role inherits privileges of roles it is a\n"
 			 "member of (default)\n"));
 	printf(_("  -I, --no-inherit  role does not inherit privileges\n"));
 	printf(_("  -l, --login   role can login (default)\n"));
 	printf(_("  -L, --no-loginrole cannot login\n"));
-	printf(_("  -m, --member=ROLE this role will be a member of new role\n"));
+	printf(_("  -m, --with-member=ROLEthis role will be a member of new role\n"));
 	printf(_("  -P, --pwpromptassign a password to new role\n"));
 	printf(_("  -r, --createrole  role can create new roles\n"));
 	printf(_("  -R, --no-createrole   role cannot create roles (default)\n"));
diff --git a/src/bin/scripts/t/040_createuser.pl b/src/bin/scripts/t/040_createuser.pl
index da99d0ccb9..7530a9f007 100644
--- a/src/bin/scripts/t/040_createuser.pl
+++ b/src/bin/scripts/t/040_createuser.pl
@@ -64,4 +64,21 @@ $node->issues_sql_like(
 $node->command_fails([ 'createuser', 'regress_user1' ],
 	'fails if role already exists');
 
+$node->issues_sql_like(
+	[ 'createuser', 'regress_user9', '--with-admin', 'regress_user1' ],
+	qr/statement: CREATE ROLE regress_user9 NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT LOGIN NOREPLICATION NOBYPASSRLS ADMIN regress_user1;/,
+	'--with-admin');
+$node->issues_sql_like(
+	[ 'createuser', 

Re: Naming of gss_accept_deleg

2023-05-21 Thread Tom Lane
Abhijit Menon-Sen  writes:
> At 2023-05-20 23:21:57 -0400, t...@sss.pgh.pa.us wrote:
>> I thought the plan was to also rename the libpq "gssdeleg" connection
>> parameter and so on?  I can look into that tomorrow, if nobody beats
>> me to it.

> I was trying the change to see if it would be better to name it
> "gssdelegate" instead (as in delegate on one side, and accept the
> delegation on the other), but decided that "gssdelegation=enable"
> reads better than "gssdelegate=enable".

Yeah, agreed.

> Here's the diff.

Thanks for doing that legwork!  I found a couple other places where
"deleg" had escaped notice, and changed the lot.  Watching the
buildfarm now ...

regards, tom lane




Re: createuser --memeber and PG 16

2023-05-21 Thread Nathan Bossart
On Sun, May 21, 2023 at 08:00:15AM +0200, Peter Eisentraut wrote:
> Maybe
> 
> createuser --with-members
> 
> and
> 
> createuser --member-of
> 
> would be clearer.

Those seem like reasonable choices to me.  I suspect we'll want to keep
--role around for backward compatibility.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com




Re: RFI: Extending the TOAST Pointer

2023-05-21 Thread Aleksander Alekseev
Hi,

> We'd need to stop using the va_tag as length indicator, but I don't
> think it's currently assumed to be a length indicator anyway (see
> VARSIZE_EXTERNAL(ptr)). By not using the varatt_external struct
> currently in use, we could be able to get down to <18B toast pointers
> as well, though I'd consider that unlikely.

Agree.

Another thing we have to decide is what to do exactly in the scope of
this thread.

I imagine it as a refactoring that will find all the places that deal
with current TOAST pointer and changes them to something like:

```
switch(va_tag) {
  case DEFAULT_VA_TAG( equals 18 ):
default_toast_process_case_abc(...);
  default:
elog(ERROR, "Unknown TOAST tag")
}
```

So that next time somebody is going to need another type of TOAST
pointer this person will have only to add a corresponding tag and
handlers. (Something like "virtual methods" will produce a cleaner
code but will also break branch prediction, so I don't think we should
use those.)

I don't think we need an example of adding a new TOAST tag in scope of
this work since the default one is going to end up being such an
example.

Does it make sense?

-- 
Best regards,
Aleksander Alekseev




Re: PG 16 draft release notes ready

2023-05-21 Thread Ian Lawrence Barwick
2023年5月19日(金) 5:49 Bruce Momjian :
>
> I have completed the first draft of the PG 16 release notes.  You can
> see the output here:
>
> https://momjian.us/pgsql_docs/release-16.html
>
> I will adjust it to the feedback I receive;  that URL will quickly show
> all updates.

Hi

Below are a few commits which are not referenced in the current iteration
of the release notes, but which seem worthy of inclusion.
Apologies if they have been previously discussed, or I'm overlooking something
obvious.

d09dbeb9b Speedup hash index builds by skipping needless binary searches
  "Testing has shown that this can improve hash index build speeds by 5-15%
   with a unique set of integer values."

e09d7a126 Improve speed of hash index build.
  "This seems to be good for overall
  speedup of 5%-9%, depending on the incoming data."

594f8d377 Allow batching of inserts during cross-partition updates.
  seems reasonable to mention this as it's related to 97da48246, which
  is mentioned in the notes

1349d2790 Improve performance of ORDER BY / DISTINCT aggregates
  This is the basis for da5800d5, which is mentioned in the notes, but AFAICS
   the latter is an implementation fix for the former (haven't looked
into either
   in detail though).

The following are probably not headline features, but are the kind of
behavioural
changes I'd expect to find in the release notes (when, at some point
in the far and
distant future, trying to work out when they were introduced when considering
application compatibility etc.):

13a185f54 Allow publications with schema and table of the same schema.
2ceea5adb Accept "+infinity" in date and timestamp[tz] input.
d540a02a7 Display the leader apply worker's PID for parallel apply workers.


Regards

Ian Barwick




Re: "38.10.10. Shared Memory and LWLocks" may require a clarification

2023-05-21 Thread Aleksander Alekseev
Hi,

> However it's not clear when a race-condition may happen. The rest of
> the text gives an overall impression that the shmem_startup_hook will
> be called by postmaster once (unless an extension places several hooks
> in series). Thus there is no real need to ackquire AddinShmemInitLock
> and it should be safe to store LWLock* in local process memory. This
> memory will be inherited from postmaster by child processes and the
> overall memory usage is going to be the same due to copy-on-write.

I added some logs and comments to my toy extension [1] to demonstrate
this. Additionally I added a sleep() call to the shmem_startup_hook to
make sure there are no concurrent processes at the moment when the
hook is called (this change is not committed to the GitHub
repository):

```
@@ -35,6 +35,9 @@ experiment_shmem_request(void)
RequestNamedLWLockTranche("experiment", 1);
 }

+#include 
+#include 
+
 static void
 experiment_shmem_startup(void)
 {
@@ -43,6 +46,8 @@ experiment_shmem_startup(void)
elog(LOG, "experiment_shmem_startup(): pid = %d, postmaster = %d\n",
MyProcPid, !IsUnderPostmaster);

+sleep(30);
+
if(prev_shmem_startup_hook)
prev_shmem_startup_hook();
```

If we do `make && make install && make installcheck` and examine
.//tmp_check/log/001_basic_main.log we will see:

```
[6288] LOG:  _PG_init(): pid = 6288, postmaster = 1
[6288] LOG:  experiment_shmem_request(): pid = 6288, postmaster = 1
[6288] LOG:  experiment_shmem_startup(): pid = 6288, postmaster = 1
```

Also we can make sure that there is only one process running when
shmem_startup_hook is called.

So it looks like acquiring AddinShmemInitLock in the hook is redundant
and also placing LWLock* in local process memory instead of shared
memory is safe.

Unless I missed something, I suggest updating the documentation and
pg_stat_statements.c accordingly.

[1]: 
https://github.com/afiskon/postgresql-extensions/blob/main/006-shared-memory/experiment.c#L38

-- 
Best regards,
Aleksander Alekseev




Re: POC PATCH: copy from ... exceptions to: (was Re: VLDB Features)

2023-05-21 Thread Alena Rybakina

I'm sorry I was unable to respond right away.

On 09.05.2023 17:23, torikoshia wrote:
You may already understand it, but these variable names are given in 
imitation of FREEZE and BINARY cases:


  --- a/src/include/commands/copy.h
  +++ b/src/include/commands/copy.h
  @@ -42,6 +42,7 @@ typedef struct CopyFormatOptions
   * -1 if not specified */
  bool    binary; /* binary format? */
  bool    freeze; /* freeze rows on loading? */
  +   bool    ignore_datatype_errors;  /* ignore rows with 
datatype errors */


  --- a/src/backend/commands/copy.c
  +++ b/src/backend/commands/copy.c
  @@ -419,6 +419,7 @@ ProcessCopyOptions(ParseState *pstate,
  bool    format_specified = false;
  bool    freeze_specified = false;
  bool    header_specified = false;
  +   bool    ignore_datatype_errors_specified = false;

I think it would be sane to align the names with the FREEZE and BINARY 
options.


I agree with the name is too long and we once used the name 
'ignore_errors'.
However, current implementation does not ignore all errors but just 
data type error, so I renamed it.

There may be a better name, but I haven't come up with one.


Yes, you are right, I saw it.



As far as I take a quick look at on PostgreSQL source code, there're 
few variable name with "_counter". It seems to be used for function names.

Something like "ignored_errors_count" might be better.
I noticed that many variables are named with the "_counter" postfix, and 
most of them are used as a counter. For example, PgStat_StatTabEntry or 
JitInstrumentation structures consisted of many such variables. Despite 
this, I agree with your suggested name, because I found many similar 
variables that are used in the program as a counter, but it seems to me 
that the most of them are still used by local variables in the function.





Re: Naming of gss_accept_deleg

2023-05-21 Thread Abhijit Menon-Sen
At 2023-05-20 23:21:57 -0400, t...@sss.pgh.pa.us wrote:
>
> Nathan Bossart  writes:
> > On Sat, May 20, 2023 at 09:33:44PM -0400, Bruce Momjian wrote:
> >> With less then 48 hours to beta 1 packaging, I have made this change and
> >> adjusted internal variable to match.
> 
> > The buildfarm and cfbot seem unhappy with 9c0a0e2.  It looks like there are
> > a few remaining uses of gss_accept_deleg to rename.  I'm planning to commit
> > the attached patch shortly.
> 
> I thought the plan was to also rename the libpq "gssdeleg" connection
> parameter and so on?  I can look into that tomorrow, if nobody beats
> me to it.

I was trying the change to see if it would be better to name it
"gssdelegate" instead (as in delegate on one side, and accept the
delegation on the other), but decided that "gssdelegation=enable"
reads better than "gssdelegate=enable".

Here's the diff.

-- Abhijit
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 826baac9f1..c8c4614b54 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -172,7 +172,7 @@ ALTER SERVER testserver1 OPTIONS (
 	--requirepeer 'value',
 	krbsrvname 'value',
 	gsslib 'value',
-	gssdeleg 'value'
+	gssdelegation 'value'
 	--replication 'value'
 );
 -- Error, invalid list syntax
diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c
index fe40d50c6d..5c301e0ef3 100644
--- a/contrib/postgres_fdw/option.c
+++ b/contrib/postgres_fdw/option.c
@@ -289,10 +289,10 @@ InitPgFdwOptions(void)
 		{"sslkey", UserMappingRelationId, true},
 
 		/*
-		 * gssdeleg is also a libpq option but should be allowed in a user
-		 * mapping context too
+		 * gssdelegation is also a libpq option but should be allowed in
+		 * a user mapping context too
 		 */
-		{"gssdeleg", UserMappingRelationId, true},
+		{"gssdelegation", UserMappingRelationId, true},
 
 		{NULL, InvalidOid, false}
 	};
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 15f3af6c29..b54903ad8f 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -186,7 +186,7 @@ ALTER SERVER testserver1 OPTIONS (
 	--requirepeer 'value',
 	krbsrvname 'value',
 	gsslib 'value',
-	gssdeleg 'value'
+	gssdelegation 'value'
 	--replication 'value'
 );
 
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index cce25d06e6..e38a7debc3 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -2054,8 +2054,8 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
   
  
 
- 
-  gssdeleg
+ 
+  gssdelegation
   

 Forward (delegate) GSS credentials to the server.  The default is
@@ -8271,10 +8271,10 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
 
  
   
-   PGGSSDELEG
+   PGGSSDELEGATION
   
-  PGGSSDELEG behaves the same as the  connection parameter.
+  PGGSSDELEGATION behaves the same as the  connection parameter.
  
 
 
diff --git a/src/backend/foreign/foreign.c b/src/backend/foreign/foreign.c
index 6e1977fa62..ca3ad55b62 100644
--- a/src/backend/foreign/foreign.c
+++ b/src/backend/foreign/foreign.c
@@ -574,7 +574,7 @@ static const struct ConnectionOption libpq_conninfo_options[] = {
 	{"requiressl", ForeignServerRelationId},
 	{"sslmode", ForeignServerRelationId},
 	{"gsslib", ForeignServerRelationId},
-	{"gssdeleg", ForeignServerRelationId},
+	{"gssdelegation", ForeignServerRelationId},
 	{NULL, InvalidOid}
 };
 
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 0dc31988b4..de0e13e50d 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -97,7 +97,7 @@ pg_GSS_continue(PGconn *conn, int payloadlen)
 	if (!pg_GSS_have_cred_cache(>gcred))
 		conn->gcred = GSS_C_NO_CREDENTIAL;
 
-	if (conn->gssdeleg && pg_strcasecmp(conn->gssdeleg, "enable") == 0)
+	if (conn->gssdelegation && pg_strcasecmp(conn->gssdelegation, "enable") == 0)
 		gss_flags |= GSS_C_DELEG_FLAG;
 
 	maj_stat = gss_init_sec_context(_stat,
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 30486c59ba..786d22a770 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -343,9 +343,9 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
 		"GSS-library", "", 7,	/* sizeof("gssapi") == 7 */
 	offsetof(struct pg_conn, gsslib)},
 
-	{"gssdeleg", "PGGSSDELEG", NULL, NULL,
+	{"gssdelegation", "PGGSSDELEGATION", NULL, NULL,
 		"GSS-delegation", "", 8,	/* sizeof("disable") == 8 */
-	offsetof(struct pg_conn, gssdeleg)},
+	offsetof(struct pg_conn, gssdelegation)},
 
 	{"replication", NULL, NULL, NULL,
 		"Replication", "D", 5,
@@ -4453,7 +4453,7 @@ freePGconn(PGconn *conn)
 	free(conn->gssencmode);
 	free(conn->krbsrvname);
 	free(conn->gsslib);
-	free(conn->gssdeleg);
+	

Re: createuser --memeber and PG 16

2023-05-21 Thread Peter Eisentraut

On 15.05.23 22:11, Nathan Bossart wrote:

On Mon, May 15, 2023 at 04:27:04PM +0900, Michael Paquier wrote:

On Fri, May 12, 2023 at 04:35:34PM +0200, Peter Eisentraut wrote:

it's not intuitive whether foo becomes a member of bar or bar becomes a
member of foo.  Maybe something more verbose like --member-of would help?


Indeed, presented like that it could be confusing, and --member-of
sounds like it could be a good idea instead of --member.


--member specifieѕ an existing role that will be given membership to the
new role (i.e., GRANT newrole TO existingrole).  IMO --member-of sounds
like the new role will be given membership to the specified existing role
(i.e., GRANT existingrole TO newrole).  IOW a command like

createuser newrole --member-of existingrole

would make existingrole a "member of" newrole according to \du.  Perhaps
--role should be --member-of because it makes the new role a member of the
existing role.


Yeah, that's exactly my confusion.

Maybe

createuser --with-members

and

createuser --member-of

would be clearer.