Re: Problem with moderation of messages with patched attached.

[Aleksander Alekseev]

Hi hackers,

> Confirm

Here are my two cents.

My last email to pgsql-jobs@ was moderated in a similar fashion. To my
knowledge that mailing list is not pre-moderated. So it may have the same
problem, and not only with patches. (We use regular Google Workspace.)

The pgsql-hackers@ thread under question seems to have two mailing list
addresses in cc:. Maybe this is the reason [1]:

> Cross-posted emails will be moderated and therefore will also take longer
to reach the subscribers if approved.

Although it's strange that only emails with attachments seem to be affected.

[1]: https://www.postgresql.org/list/

-- 
Best regards,
Aleksander Alekseev

Re: Mingw task for Cirrus CI

[Andrew Dunstan]

On 3/3/22 05:16, Melih Mutlu wrote:
> Hi Andres, 
>  
>
> This presumably is due to using mingw's python rather than
> python.org 's
> python. Seems like a reasonable thing to support for the mingw build.
>
> Melih, you could try to build against the python.org
>  python (i installed in
> the CI container).
>
>
> I tried to use the installed python from python.org
>  in the container. 
> The problem with this is that "LIBDIR" and "LDLIBRARY" configs of
> python for windows from python.org  are empty.
> Therefore python_libdir or other related variables in configure file
> are not set correctly.



Yeah, here's what it has:


# python -c "import sysconfig; import pprint; pp =
pprint.PrettyPrinter(); pp.pprint(sysconfig.get_config_vars())"
{'BINDIR': 'C:\\prog\\python310',
 'BINLIBDEST': 'C:\\prog\\python310\\Lib',
 'EXE': '.exe',
 'EXT_SUFFIX': '.cp310-win_amd64.pyd',
 'INCLUDEPY': 'C:\\prog\\python310\\Include',
 'LIBDEST': 'C:\\prog\\python310\\Lib',
 'SO': '.cp310-win_amd64.pyd',
 'TZPATH': '',
 'VERSION': '310',
 'abiflags': '',
 'base': 'C:\\prog\\python310',
 'exec_prefix': 'C:\\prog\\python310',
 'installed_base': 'C:\\prog\\python310',
 'installed_platbase': 'C:\\prog\\python310',
 'platbase': 'C:\\prog\\python310',
 'platlibdir': 'lib',
 'prefix': 'C:\\prog\\python310',
 'projectbase': 'C:\\prog\\python310',
 'py_version': '3.10.2',
 'py_version_nodot': '310',
 'py_version_nodot_plat': '310',
 'py_version_short': '3.10',
 'srcdir': 'C:\\prog\\python310',
 'userbase': 'C:\\Users\\Administrator\\AppData\\Roaming\\Python'}


The DLL lives in the BINDIR, so maybe I guess we should search there if
we can't get the other things.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Re: Commitfest 2022-03 Patch Triage Part 1a.i

[Daniel Gustafsson]

> On 3 Mar 2022, at 10:11, Yugo NAGATA  wrote:

> I think this patch set needs more reviews to be commitable in 15, so I
> returned the target version to blank. I'll change it to 16 later.

I've added 16 as a target version in the CF app now.

--
Daniel Gustafsson   https://vmware.com/

Re: Problem with moderation of messages with patched attached.

[Maxim Orlov]

On Thu, Mar 3, 2022 at 3:31 PM Pavel Borisov  wrote:

> Hi, hackers!
>
> Around 2 months ago I've noticed a problem that messages containing
> patches in the thread [1] were always processed with manual moderation.
> They appear in hackers' thread hours after posting None of them are from
> new CF members and personally, I don't see a reason for such inconvenience.
> The problem still exists as of today.
>
> Can someone make changes in a moderation engine to make it more liberal
> and convenient for authors?
>
> [1]
> https://www.postgresql.org/message-id/flat/CACG%3DezZe1NQSCnfHOr78AtAZxJZeCvxrts0ygrxYwe%3DpyyjVWA%40mail.gmail.com
>
> --
> Best regards,
> Pavel Borisov
>
> Postgres Professional: http://postgrespro.com 
>

Confirm

-- 
Best regards,
Maxim Orlov.

Problem with moderation of messages with patched attached.

[Pavel Borisov]

Hi, hackers!

Around 2 months ago I've noticed a problem that messages containing patches
in the thread [1] were always processed with manual moderation. They appear
in hackers' thread hours after posting None of them are from new CF members
and personally, I don't see a reason for such inconvenience. The problem
still exists as of today.

Can someone make changes in a moderation engine to make it more liberal and
convenient for authors?

[1]
https://www.postgresql.org/message-id/flat/CACG%3DezZe1NQSCnfHOr78AtAZxJZeCvxrts0ygrxYwe%3DpyyjVWA%40mail.gmail.com

-- 
Best regards,
Pavel Borisov

Postgres Professional: http://postgrespro.com 

Re: Add 64-bit XIDs into PostgreSQL 15

[Pavel Borisov]

BTW messages with patches in this thread are always invoke manual spam
moderation and we need to wait for ~3 hours before the message with patch
becomes visible in the hackers thread. Now when I've already answered
Alexander's letter with v10 patch the very message (and a patch) I've
answered is still not visible in the thread and to CFbot.

Can something be done in hackers' moderation engine to make new versions
patches become visible hassle-free?

-- 
Best regards,
Pavel Borisov

Postgres Professional: http://postgrespro.com 

Re: Add 64-bit XIDs into PostgreSQL 15

[Pavel Borisov]

>
> > The patch doesn't apply - I suppose the patch is relative a forked
> postgres
>
> No, the authors just used a little outdated `master` branch. I
> successfully applied it against 31d8d474 and then rebased to the
> latest master (62ce0c75). The new version is attached.

Not 100% sure if my rebase is correct since I didn't invest too much
> time into reviewing the code. But at least it passes `make
> installcheck` locally. Let's see what cfbot will tell us.
>
Thank you very much! We'll do the same rebase and check this soon. Let's
use v10 now.


> > I encourage trying to break down the patch into smaller incrementally
> useful
> > pieces. E.g. making all the SLRUs 64bit would be a substantial and
> > independently committable piece.
>
> Completely agree. And the changes like:
>
> +#if 0 /* XXX remove unit tests */
>
> ... suggest that the patch is pretty raw in its current state.
>
> Pavel, Maxim, don't you mind me splitting the patchset, or would you
> like to do it yourself and/or maybe include more changes? I don't know
> how actively you are working on this.
>
I don't mind and appreciate you joining this. If you split this we'll just
make the next versions based on it. Of course, there is much to do and we
work on this patch, including pg_upgrade test fail reported by Justin,
which we haven't had time to concentrate on before. We try to do changes in
small batches so I consider we can manage parallel changes. At least I read
this thread very often and can answer soon, even if our new versions of
patches are not ready.

Again I consider the work you propose useful and big thanks to you,
Alexander!

-- 
Best regards,
Pavel Borisov

Postgres Professional: http://postgrespro.com 

Re: row filtering for logical replication

[Amit Kapila]

On Thu, Mar 3, 2022 at 11:18 AM shiy.f...@fujitsu.com
 wrote:
>
> On Thu, Mar 3, 2022 10:40 AM Amit Kapila  wrote:
> >
> > On Wed, Mar 2, 2022 at 5:42 PM Euler Taveira  wrote:
> > >
> > > On Wed, Mar 2, 2022, at 8:45 AM, Tomas Vondra wrote:
> > >
> > > While working on the column filtering patch, which touches about the
> > > same places, I noticed two minor gaps in testing:
> > >
> > > 1) The regression tests do perform multiple ALTER PUBLICATION commands,
> > > tweaking the row filter. But there are no checks the row filter was
> > > actually modified / stored in the catalog. It might be just thrown away
> > > and no one would notice.
> > >
> > > The test that row filter was modified is available in a previous section. 
> > > The
> > > one that you modified (0001) is testing the supported objects.
> > >
> >
> > Right. But if Tomas thinks it is good to add for these ones as well
> > then I don't mind.
> >
> > > 153 ALTER PUBLICATION testpub5 ADD TABLE testpub_rf_tbl3 WHERE (e > 1000
> > AND e < 2000);
> > > 154 \dRp+ testpub5
> > > 155 ALTER PUBLICATION testpub5 DROP TABLE testpub_rf_tbl2;
> > > 156 \dRp+ testpub5
> > > 157 -- remove testpub_rf_tbl1 and add testpub_rf_tbl3 again (another WHERE
> > expression)
> > > 158 ALTER PUBLICATION testpub5 SET TABLE testpub_rf_tbl3 WHERE (e > 300
> > AND e < 500);
> > > 159 \dRp+ testpub5
> > >
> > > IIRC this test was written before adding the row filter information into 
> > > the
> > > psql. We could add \d+ testpub_rf_tbl3 before and after the modification.
> > >
> >
> >
> > Agreed. We can use \d instead of \d+ as row filter is available with \d.
> >
> > > 2) There are no pg_dump tests.
> > >
> > > WFM.
> > >
> >
> > This is a miss. I feel we can add a few more.
> >
>
> Agree that we can add some tests, attach the patch which fixes these two 
> points.
>

LGTM. I'll push this tomorrow unless Tomas or Euler feels otherwise.

-- 
With Regards,
Amit Kapila.

Re: Mingw task for Cirrus CI

[Melih Mutlu]

Hi Andres,


> This presumably is due to using mingw's python rather than python.org's
> python. Seems like a reasonable thing to support for the mingw build.
>
> Melih, you could try to build against the python.org python (i installed
> in
> the CI container).
>

I tried to use the installed python from python.org in the container.
The problem with this is that "LIBDIR" and "LDLIBRARY" configs of python
for windows from python.org are empty. Therefore python_libdir or other
related variables in configure file are not set correctly.


Seems we're doing something wrong and end up with a 4 byte off_t, whereas
> python ends up with an 8byte one. We probably need to fix that. But it's
> not
> the cause of this problem.
>
>
> You could take out -s from the make flags and see whether plpython3.dll is
> being built and installed, and where to.
>

Here is a run without -s: https://cirrus-ci.com/task/4569363104661504
I couldn't get what's wrong from these logs to be honest. But I see that
plpython3.dll exists under src/pl/plpython after build when I run these
steps locally.

Best,
Melih

Re: Proposal: Support custom authentication methods using hooks

[Peter Eisentraut]

On 02.03.22 21:49, samay sharma wrote:
I think we are discussing two topics in this thread which in my opinion 
are orthogonal.


(a) Should we make authentication methods pluggable by exposing these 
hooks? - These will allow users to add plugins of their own to support 
whatever auth method they like. One immediate use case (and what 
prompted me to start looking at this) is Azure Active Directory 
integration which is a common request from Azure customers. We could 
also, over time, move some of our existing auth methods into extensions 
if we don’t want to maintain them in core.


I don't think people are necessarily opposed to that.

At the moment, it is not possible to judge whether the hook interface 
you have chosen is appropriate.


I suggest you actually implement the Azure provider, then make the hook 
interface, and then show us both and we can see what to do with it.


One thing that has been requested, and I would support that, is that a 
plugged-in authentication method should look like a built-in one.  So 
for example it should be able to register a real name, instead of 
"custom".  I think a fair bit of refactoring work might be appropriate 
in order to make the authentication code more modular.

Re: Make mesage at end-of-recovery less scary.

[Ashutosh Sharma]

On Wed, Mar 2, 2022 at 7:47 AM Kyotaro Horiguchi
 wrote:
>
> At Sat, 19 Feb 2022 09:31:33 +0530, Ashutosh Sharma  
> wrote in
> > The changes looks good. thanks.!
>
> Thanks!
>
> Some recent core change changed WAL insertion speed during the TAP
> test and revealed one forgotton case of EndOfWAL.  When a record
> header flows into the next page, XLogReadRecord does separate check
> from ValidXLogRecordHeader by itself.
>

The new changes made in the patch look good. Thanks to the recent
changes to speed WAL insertion that have helped us catch this bug.

One small comment:

record = (XLogRecord *) (state->readBuf + RecPtr % XLOG_BLCKSZ);
-   total_len = record->xl_tot_len;

Do you think we need to change the position of the comments written
for above code that says:

/*
 * Read the record length.
 *
...
...

--
With Regards,
Ashutosh Sharma.

Re: Proposal: Support custom authentication methods using hooks

[Peter Eisentraut]

On 02.03.22 15:16, Jonathan S. Katz wrote:
What are the reasons they are still purposely using it? The ones I have 
seen/heard are:


- Using an older driver
- On a pre-v10 PG
- Unaware of SCRAM


Another reason is that SCRAM presents subtle operational issues in 
distributed systems.  As someone who is involved with products such as 
pgbouncer and bdr, I am aware that there are still unresolved problems 
and ongoing research in that area.  Maybe they can all be solved 
eventually, even if it is concluding "you can't do that anymore" in 
certain cases, but it's not all solved yet, and falling back to the 
best-method-before-this-one is a useful workaround.


I'm thinking there might be room for an authentication method between 
plain and scram that is less complicated and allows distributed systems 
to be set up more easily.  I don't know what that would be, but I don't 
think we should prohibit the consideration of "anything less than SCRAM".


I notice that a lot of internet services are promoting "application 
passwords" nowadays.  I don't know the implementation details of that, 
but it appears that the overall idea is to have instead of one 
high-value password have many frequently generated medium-value 
passwords.  We also have a recent proposal to store multiple passwords 
per user.  (Obviously that could apply to SCRAM and not-SCRAM equally.) 
That's the kind of direction I would like to explore.

Re: Proposal: Support custom authentication methods using hooks

[Peter Eisentraut]

On 02.03.22 16:45, Jonathan S. Katz wrote:
By that argument, we should have kept "password" (plain) as an 
authentication method.


For comparison, the time between adding md5 and removing password was 16 
years.  It has been 5 years since scram was added.

Re: Proposal: Support custom authentication methods using hooks

[Peter Eisentraut]

On 02.03.22 21:26, Stephen Frost wrote:

Part of the point, for my part anyway, of dropping support for plaintext
transmission would be to remove support for that from libpq, otherwise a
compromised server could still potentially convince a client to provide
a plaintext password be sent to it.


I think there should be a generalized feature for client-side selecting 
or filtering of authentication methods.  As long as there exists more 
than one method, there will be tradeoffs and users might want to avoid 
one or the other.  I don't think removing a method outright is the right 
solution for that.

Re: Support for grabbing multiple consecutive values with nextval()

[Jille Timmermans]

On 2022-03-03 10:10, Peter Eisentraut wrote:

On 02.03.22 20:12, Jille Timmermans wrote:

On 2022-02-28 11:13, Peter Eisentraut wrote:

On 27.02.22 10:42, Jille Timmermans wrote:
I wanted to be able to allocate a bunch of numbers from a sequence 
at once. Multiple people seem to be struggling with this 
(https://stackoverflow.com/questions/896274/select-multiple-ids-from-a-postgresql-sequence, 
https://www.depesz.com/2008/03/20/getting-multiple-values-from-sequences/). 
I propose to add an extra argument to nextval() that specifies how 
many numbers you want to allocate (default 1).


What is the use of this?

I note that the stackoverflow question wanted to return multiple
sequence values as a result set, whereas your implementation just
skips a number of values and returns the last one.  At least we 
should

be clear about what we are trying to achieve.
Both would work for me actually. I'm using COPY FROM to insert many 
rows and need to know their ids and COPY FROM doesn't support 
RETURNING.


I don't understand this use case.  COPY FROM copies from a file.  So
you want to preallocate the sequence numbers before you copy the new
data in?

Yes


How do you know how many rows are in the file?
I'm using https://pkg.go.dev/github.com/jackc/pgx/v4#Conn.CopyFrom, 
which uses the COPY FROM protocol but doesn't actually have to originate 
from a file.

RE: Logical replication timeout problem

[kuroda.hay...@fujitsu.com]

Dear Wang,

> Attach the new patch. [suggestion by Kuroda-San]
> 1. Fix the typo.
> 2. Improve comment style.
> 3. Fix missing consideration.
> 4. Add comments to clarifies above functions and function calls.

Thank you for updating the patch! I confirmed they were fixed.

```
case REORDER_BUFFER_CHANGE_INVALIDATION:
-   /* Execute the invalidation messages 
locally */
-   ReorderBufferExecuteInvalidations(
-   
  change->data.inval.ninvalidations,
-   
  change->data.inval.invalidations);
-   break;
+   {
+   LogicalDecodingContext *ctx = 
rb->private_data;
+
+   Assert(!ctx->fast_forward);
+
+   /* Set output state. */
+   ctx->accept_writes = true;
+   ctx->write_xid = txn->xid;
+   ctx->write_location = 
change->lsn;
```

Some codes were added in ReorderBufferProcessTXN() for treating DDL, 




I'm also happy if you give the version number :-).


Best Regards,
Hayato Kuroda
FUJITSU LIMITED

> -Original Message-
> From: Wang, Wei/王 威 
> Sent: Wednesday, March 2, 2022 11:06 AM
> To: Kuroda, Hayato/黒田 隼人 
> Cc: Fabrice Chapuis ; Simon Riggs
> ; Petr Jelinek
> ; Tang, Haiying/唐 海英
> ; Amit Kapila ;
> PostgreSQL Hackers ; Ajin Cherian
> 
> Subject: RE: Logical replication timeout problem
> 
> On Mon, Feb 28, 2022 at 6:58 PM Kuroda, Hayato/黒田 隼人
>  wrote:
> > Dear Wang,
> >
> > > Attached a new patch that addresses following improvements I have got
> > > so far as
> > > comments:
> > > 1. Consider other changes that need to be skipped(truncate, DDL and
> > > function calls pg_logical_emit_message). [suggestion by Ajin, Amit]
> > > (Add a new function SendKeepaliveIfNecessary for trying to send
> > > keepalive
> > > message.)
> > > 2. Set the threshold conservatively to a static value of
> > > 1.[suggestion by Amit, Kuroda-San] 3. Reset sendTime in function
> > > WalSndUpdateProgress when send_keep_alive is false. [suggestion by
> > > Amit]
> >
> > Thank you for giving a good patch! I'll check more detail later, but it can 
> > be
> > applied my codes and passed check world.
> > I put some minor comments:
> Thanks for your comments.
> 
> > ```
> > + * Try to send keepalive message
> > ```
> >
> > Maybe missing "a"?
> Fixed. Add missing "a".
> 
> > ```
> > +   /*
> > +   * After continuously skipping SKIPPED_CHANGES_THRESHOLD
> changes, try
> > to send a
> > +   * keepalive message.
> > +   */
> > ```
> >
> > This comments does not follow preferred style:
> > https://www.postgresql.org/docs/devel/source-format.html
> Fixed. Correct wrong comment style.
> 
> > ```
> > @@ -683,12 +683,12 @@ OutputPluginWrite(struct LogicalDecodingContext
> *ctx,
> > bool last_write)
> >   * Update progress tracking (if supported).
> >   */
> >  void
> > -OutputPluginUpdateProgress(struct LogicalDecodingContext *ctx)
> > +OutputPluginUpdateProgress(struct LogicalDecodingContext *ctx, bool
> > +send_keep_alive)
> > ```
> >
> > This function is no longer doing just tracking.
> > Could you update the code comment above?
> Fixed. Update the comment above function OutputPluginUpdateProgress.
> 
> > ```
> > if (!is_publishable_relation(relation))
> > return;
> > ```
> >
> > I'm not sure but it seems that the function exits immediately if relation 
> > is a
> > sequence, view, temporary table and so on. Is it OK? Does it never happen?
> I did some checks to confirm this. After my confirmation, there are several
> situations that can cause a timeout. For example, if I insert many date into
> table sql_features in a long transaction, subscriber-side will time out.
> Although I think users should not modify these tables arbitrarily, it could
> happen. To be conservative, I think this use case should be addressed as well.
> Fixed. Invoke function SendKeepaliveIfNecessary before return.
> 
> > ```
> > +   SendKeepaliveIfNecessary(ctx, false);
> > ```
> >
> > I think a comment is needed above which clarifies sending a keepalive
> message.
> Fixed. Before invoking function SendKeepaliveIfNecessary, add the
> corresponding
> comment.
> 
> Attach the new patch. [suggestion by Kuroda-San]
> 1. Fix the typo.
> 2. Improve comment style.
> 3. Fix missing consideration.
> 4. Add comments to clarifies above functions and function calls.
> 
> Regards,
> Wang wei

Re: Commitfest 2022-03 Patch Triage Part 1a.i

[Yugo NAGATA]

On Tue, 1 Mar 2022 13:39:45 -0500
Greg Stark  wrote:

> As Justin suggested I CC the authors from these patches I'm adding
> them here. Some of the patches have multiple "Authors" listed in the
> commitfest which may just be people who posted updated patches so I
> may have added more people than necessary.
> 
> On Tue, 1 Mar 2022 at 11:16, Greg Stark  wrote:
> >
> > Last November Daniel Gustafsson  did a patch triage. It took him three
> > emails to get through the patches in the commitfest back then. Since
> > then we've had the November and the January commitfests so I was
> > interested to see how many of these patches had advanced
> >
> > I'm only part way through the first email but so far only two patches
> > have changed status -- and both to "Returned with feedback" :(
> >
> > So I'm going to post updates but I'm going to break it up into smaller
> > batches because otherwise it'll take me a month before I post
> > anything.
> >
> >
> >
> > > 1608: schema variables, LET command
> > > ===
> > > After 18 CF's and two very long threads it seems to be nearing completion
> > > judging by Tomas' review.  There is an ask in that review for a second 
> > > pass
> > > over the docs by a native speaker, any takers?
> >
> > Patch has a new name, "session variables, LET command"
> >
> > There's been a *lot* of work on this patch so I'm loath to bump it.
> > The last review was from Julien Rouhaud which had mostly code style
> > comments but it had one particular concern about using xact callback
> > in core and about EOX cleanup.
> >
> > Pavel, do you have a plan to improve this or are you looking for
> > suggestions from someone about how you should solve this problem?
> >
> > > 1741: Index Skip Scan
> > > =
> > > An often requested feature which has proven hard to reach consensus on an
> > > implementation for.  The thread(s) have stalled since May, is there any 
> > > hope of
> > > taking this further?  Where do we go from here with this patch?
> >
> > "Often requested indeed! I would love to be able to stop explaining to
> > people that Postgres can't handle this case well.
> >
> > It seems there are multiple patch variants around and suggestions from
> > Heikki and Peter G about fundamental interface choices. It would be
> > nice to have a good summary from someone who is involved about what's
> > actually left unresolved.
> >
> >
> > > 1712: Remove self join on a unique column
> > > =
> > > This has moved from "don't write bad queries" to "maybe we should do 
> > > something
> > > about this".  It seems like there is concensus that it can be worth 
> > > paying the
> > > added planner cost for this, especially if gated by a GUC to keep it out 
> > > of
> > > installations where it doesn't apply.  The regression on large join 
> > > queries
> > > hasn't been benchmarked it seems (or I missed it), but the patch seems to 
> > > have
> > > matured and be quite ready.  Any takers on getting it across the finish 
> > > line?
> >
> > There hasn't been any review since the v29 patch was posted in July.
> > That said, to my eye it seemed like pretty basic functionality errors
> > were being corrected quite late. All the bugs got patch updates
> > quickly but does this have enough tests to be sure it's working right?
> >
> > The only real objection was about whether the planning time justified
> > the gains since the gains are small. But I think they were resolved by
> > making the optimization optional. Do we have consensus that that
> > resolved the issue or do we still need benchmarks showing the planning
> > time hit is reasonable?
> >
> > > 2161: standby recovery fails when re-replaying due to missing directory 
> > > which
> > > was removed in previous replay.
> > > =
> > > Tom and Robert seem to be in agreement that parts of this patchset are 
> > > good,
> > > and that some parts are not.  The thread has stalled and the patch no 
> > > longer
> > > apply, so unless someone feels like picking up the good pieces this seems 
> > > like
> > > a contender to close for now.
> >
> > Tom's feedback seems to have been acted on last November. And the last
> > update in January was that it was passing CI now. Is this ready to
> > commit now?
> >
> >
> > > 2138: Incremental Materialized View Maintenance
> > > ===
> > > The size of the
> > > patchset and the length of the thread make it hard to gauge just far away 
> > > it
> > > is, maybe the author or a reviewer can summarize the current state and 
> > > outline
> > > what is left for it to be committable.
> >
> > There is an updated patch set as of February but I have the same
> > difficulty wrapping my head around the amount of info here.
> >
> > Is this one really likely to be commitable in 15? If not I think we
> > should move this to 16 now and conc

Re: Support for grabbing multiple consecutive values with nextval()

[Peter Eisentraut]

On 02.03.22 20:12, Jille Timmermans wrote:

On 2022-02-28 11:13, Peter Eisentraut wrote:

On 27.02.22 10:42, Jille Timmermans wrote:
I wanted to be able to allocate a bunch of numbers from a sequence at 
once. Multiple people seem to be struggling with this 
(https://stackoverflow.com/questions/896274/select-multiple-ids-from-a-postgresql-sequence, 
https://www.depesz.com/2008/03/20/getting-multiple-values-from-sequences/). 



I propose to add an extra argument to nextval() that specifies how 
many numbers you want to allocate (default 1).


What is the use of this?

I note that the stackoverflow question wanted to return multiple
sequence values as a result set, whereas your implementation just
skips a number of values and returns the last one.  At least we should
be clear about what we are trying to achieve.
Both would work for me actually. I'm using COPY FROM to insert many rows 
and need to know their ids and COPY FROM doesn't support RETURNING.


I don't understand this use case.  COPY FROM copies from a file.  So you 
want to preallocate the sequence numbers before you copy the new data 
in?  How do you know how many rows are in the file?

Re: libpq compression (part 2)

[Daniil Zakhlystov]

Ok, thanks


> On 3 Mar 2022, at 02:33, Justin Pryzby  wrote:
> 
> If there's no objection, I'd like to move this to the next CF for 
> consideration
> in PG16.
> 
> On Mon, Jan 17, 2022 at 10:39:19PM -0600, Justin Pryzby wrote:
>> On Tue, Jan 18, 2022 at 02:06:32AM +0500, Daniil Zakhlystov wrote:
 => Since March, errmsg doesn't need extra parenthesis around it (e3a87b4).
>> 
>>> I’ve resolved the stuck tests and added zlib support for CI Windows builds 
>>> to patch 0003-*.  Thanks
>>> for the suggestion, all tests seem to be OK now, except the macOS one.  It 
>>> won't schedule in Cirrus
>>> CI for some reason, but I guess it happens because of my GitHub account 
>>> limitation.
>> 
>> I don't know about your github account, but it works for cfbot, which is now
>> green.
>> 
>> Thanks for implementing zlib for windows.  Did you try this with default
>> compressions set to lz4 and zstd ?
>> 
>> The thread from 2019 is very long, and starts off with the guidance that
>> compression had been implemented at the wrong layer.  It looks like this 
>> hasn't
>> changed since then.  secure_read/write are passed as function pointers to the
>> ZPQ interface, which then calls back to them to read and flush its 
>> compression
>> buffers.  As I understand, the suggestion was to leave the socket reads and
>> writes alone.  And then conditionally de/compress buffers after reading /
>> before writing from the socket if compression was negotiated.
>> 
>> It's currently done like this
>> pq_recvbuf() => secure_read() - when compression is disabled 
>> pq_recvbuf() => ZPQ => secure_read() - when compression is enabled 
>> 
>> Dmitri sent a partial, POC patch which changes the de/compression to happen 
>> in
>> secure_read/write, which is changed to call ZPQ:  
>> https://www.postgresql.org/message-id/ca+q6zcuprssnars+fyobsd-f2stk1roa-4sahfofajowlzi...@mail.gmail.com
>> pq_recvbuf() => secure_read() => ZPQ
>> 
>> The same thing is true of the frontend: function pointers to
>> pqsecure_read/write are being passed to zpq_create, and then the ZPQ 
>> interface
>> called instead of the original functions.  Those are the functions which read
>> using SSL, so they should also handle compression.
>> 
>> That's where SSL is handled, and it seems like the right place to handle
>> compression.  Have you evaluated that way to do things ?
>> 
>> Konstantin said he put ZPQ at that layer seems to 1) avoid code duplication
>> between client/server; and, 2) to allow compression to happen before SSL, to
>> allow both (if the admin decides it's okay).  But I don't see why compression
>> can't happen before sending to SSL, or after reading from it?

Re: Skipping logical replication transactions on subscriber side

[Amit Kapila]

On Tue, Mar 1, 2022 at 8:31 PM Masahiko Sawada  wrote:
>
> I’ve considered a plan for the skipping logical replication
> transaction feature toward PG15. Several ideas and patches have been
> proposed here and another related thread[1][2] for the skipping
> logical replication transaction feature as follows:
>
> A. Change pg_stat_subscription_workers (committed 7a8507329085)
> B. Add origin name and commit-LSN to logical replication worker
> errcontext (proposed[2])
> C. Store error information (e.g., the error message and commit-LSN) to
> the system catalog
> D. Introduce ALTER SUBSCRIPTION SKIP
> E. Record the skipped data somewhere: server logs or a table
>
> Given the remaining time for PG15, it’s unlikely to complete all of
> them for PG15 by the feature freeze. The most realistic plan for PG15
> in my mind is to complete B and D. With these two items, the LSN of
> the error-ed transaction is shown in the server log, and we can ask
> users to check server logs for the LSN and use it with ALTER
> SUBSCRIPTION SKIP command.
>

It makes sense to me to try to finish B and D from the above list for
PG-15. I can review the patch for D in detail if others don't have an
objection to it.

Peter E., others, any opinion on this matter?

> If the community agrees with B+D, we will
> have a user-visible feature for PG15 which can be further
> extended/improved in PG16 by adding C and E.

Agreed.

>
> I've attached an updated patch for D and here is the summary:
>
> * Introduce a new command ALTER SUBSCRIPTION ... SKIP (lsn =
> '0/1234'). The user can get the commit-LSN of the transaction in
> question from the server logs thanks to B[2].
> * The user-specified LSN (say skip-LSN) is stored in the
> pg_subscription catalog.
> * The apply worker skips the whole transaction if the transaction's
> commit-LSN exactly matches to skip-LSN.
> * The skip-LSN has an effect on only the first non-empty transaction
> since the worker started to apply changes. IOW it's cleared after
> either skipping the whole transaction or successfully committing a
> non-empty transaction, preventing the skip-LSN to remain in the
> catalog. Also, since the latter case means that the user set the wrong
> skip-LSN we clear it with a warning.
>

As this will be displayed only in server logs and by background apply
worker, should it be LOG or WARNING?

-- 
With Regards,
Amit Kapila.

Re: shared-memory based stats collector

[Kyotaro Horiguchi]

At Wed, 2 Mar 2022 18:16:00 -0800, Andres Freund  wrote in 
> Hi,
> 
> On 2021-07-26 18:27:54 -0700, Andres Freund wrote:
> > I had intended to post a rebase by now. But while I did mostly finish
> > that (see [1]) I unfortunately encountered a new issue around
> > partitioned tables, see [2]. Currently I'm hoping for a few thoughts on
> > that thread about the best way to address the issues.
> 
> Now that 
> https://postgr.es/m/20220125063131.4cmvsxbz2tdg6g65%40alap3.anarazel.de
> is resolved, here's a rebased version. With a good bit of further cleanup.
> 
> One "big" thing that I'd like to figure out is a naming policy for the
> different types prefixed with PgStat. We have different groups of types:
> 
> - "pending statistics", that are accumulated but not yet submitted to the
>   shared stats system, like PgStat_TableStatus, PgStat_BackendFunctionEntry
>   etc
> - accumulated statistics like PgStat_StatDBEntry, PgStat_SLRUStats. About 
> half are
>   prefixed with PgStat_Stat, the other just with PgStat_
> - random other types like PgStat_Single_Reset_Type, ...
> 
> To me it's very confusing to have these all in an essentially undistinguishing
> namespace, particularly the top two items.

Profoundly agreed.  It was always a pain in the neck.

> I think we should at least do s/PgStat_Stat/PgStat_/. Perhaps we should use a
> distinct PgStatPending_* for the pending item? I can't quite come up with a
> good name for the "accumulated" ones.

How about naming "pending stats" as just "Stats" and the "acculumated
stats" as "counts" or "counters"?  "Counter" doesn't reflect the
characteristics so exactly but I think the discriminability of the two
is more significant.  Specifically;

- PgStat_TableStatus
+ PgStat_TableStats
- PgStat_BackendFunctionEntry
+ PgStat_FunctionStats

- PgStat_GlobalStats
+ PgStat_GlobalCounts
- PgStat_ArchiverStats
+ PgStat_ArchiverCounts
- PgStat_BgWriterStats
+ PgStat_BgWriterCounts

Moving to shared stats collector turns them into attributed by "Local"
and "Shared". (I don't consider the details at this stage.)

PgStatLocal_TableStats
PgStatLocal_FunctionStats
PgStatLocal_GlobalCounts
PgStatLocal_ArchiverCounts
PgStatLocal_BgWriterCounts

PgStatShared_TableStats
PgStatShared_FunctionStats
PgStatShared_GlobalCounts
PgStatShared_ArchiverCounts
PgStatShared_BgWriterCounts

PgStatLocal_GlobalCounts somewhat looks odd, but doesn't matter much, maybe.

> I'd like that get resolved first because I think that'd allow commiting the
> prepatory split and reordering patches.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center