Re: Rewriting the test of pg_upgrade as a TAP test - take three - remastered set

[Michael Paquier]

On Sat, Oct 02, 2021 at 11:34:38PM -0400, Tom Lane wrote:
> Maybe we could leave test.sh in place for awhile?  I'd rather
> not cause a flag day for buildfarm owners.  (Also, how do we
> see this working in the back branches?)

I would be fine with test.sh staying around for now.

If we do that, though, I think that we had better remove the support
for upgrades across different major versions in test.sh, and keep this
capability in the new script.  I am not sure that a lot of people use
that to begin with, but it would be weird to support that with a
different configuration layer for both at the same time (test.sh uses
a combination of bin/ and lib/ paths, while TAP uses just installation
path to accomodate with what PostgresNode.pm is able to do).  The
patch of this thread also adds support for the load of an old dump
instead of an installcheck run of the old instance, which is something
the buildfarm could use.

I also looked two days ago at a proposal to move all the
pg_upgrade-specific SQLs into a new, separate, file that makes use of
psql's \if to do the job encoded now in test.sh.  I think that it
would be strange to duplicate this logic in a the pg_upgrade TAP test
and test.sh if we finish by keeping both around for now.  So that's a
second item we had better deal with first, in my opinion:
https://www.postgresql.org/message-id/YVa/se5gxr1ps...@paquier.xyz

Thoughts?
--
Michael


signature.asc
Description: PGP signature

Re: 2021-09 Commitfest

[Michael Paquier]

On Sat, Oct 02, 2021 at 11:32:01AM -0400, Tom Lane wrote:
> Right.  Michael and Jaime have been doing some of that too in the last
> few days, but obviously a CFM should only do that unilaterally in very
> clear-cut cases of patch abandonment.  I was intending to go after some
> where maybe a bit of community consensus is needed for rejection.

One thing I have used in this process is what I'd call the two-week
rule: if a patch is listed in the CF app as waiting on author for two
weeks at the middle of the CF, and if it has stalled with the same
state by the end of the commit fest with the thread remaining idle, it
is rather safe to switch the patch as returned with feedback.  I have
tried to follow this rule for the last couple of years and received
few complains when done this way.  The CF patch tester has proved to
be really helpful regarding that, even if some patches have sometimes
a state in the CF app that does not reflect what the thread tells.  In
short, it is important to check the state of the patches mid-CF
pinging the related threads if necessary, and at the end of the CF.
--
Michael


signature.asc
Description: PGP signature

Re: 2021-09 Commitfest

[Magnus Hagander]

On Sat, Oct 2, 2021 at 7:31 AM Michael Paquier  wrote:

> On Fri, Oct 01, 2021 at 08:29:08PM +0200, Daniel Gustafsson wrote:
> > Correct, if one looks at the activity log for an old entry the pattern of
> > moving to needs review, then to the next CF, then WoA is clearly visible.
>
> That's the tricky part.  It does not really make sense either to keep
> moving patches that are waiting on author for months.  The scan of the
> CF app I have done was about those idle patches waiting on author for
> months.  It takes time as authors and/or reviewers tend to sometimes
> not update the status of a patch so the state in the app does not
> reflect the reality, but this vacuuming limits the noise in for the
> next CFs.
>

I'm pretty sure this is the original reason for adding this -- to enforce
that this review happens.

Prior to this being added, all patches moved would end up in "needs review"
status. When we changed it so that the patch would keep it's status in the
next CF, we explicitly wanted to avoid having lots of patches in WoA status
in the new CF.

But this was 5 years ago, and the feature was new at the time. This may
have been wrong already then, or it may simply be that we use the system in
a different way now (and we for example did not have the cfbot back then).
Either one of those is a good reason to re-visit the decision. And it
certainly sounds from this thread that nobody is actually arguing to keep
that behaviour -- unless that changes knowing the original reason?

-- 
 Magnus Hagander
 Me: https://www.hagander.net/ 
 Work: https://www.redpill-linpro.com/ 

Re: pgsql: Document XLOG_INCLUDE_XID a little better

[Dilip Kumar]

On Sat, Oct 2, 2021 at 8:10 PM Alvaro Herrera  wrote:
>
> On 2021-Oct-02, Dilip Kumar wrote:
>
> > I have written two patches, Approach1 is as you described using a
> > static boolean and Approach2 as a local variable to XLogAssembleRecord
> > as described by Amit, attached both of them for your reference.
> > IMHO, either of these approaches looks cleaner.
>
> Thanks!  I haven't read these patches carefully, but I think the
> variable is about assigning the *subxid*, not the topxid.  Amit can
> confirm ...

IIRC, this variable is for logging the top xid in the first WAL by
each subtransaction. So that during logical decoding, while creating
the ReorderBufferTxn for the subtransaction we can associate it to the
top transaction without seeing the commit WAL.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

Re: Rewriting the test of pg_upgrade as a TAP test - take three - remastered set

[Andrew Dunstan]

On 10/2/21 11:34 PM, Tom Lane wrote:
> Andrew Dunstan  writes:
>> On 10/2/21 5:03 PM, Tom Lane wrote:
>>> IIUC, the only problem for a non-updated animal would be that it'd
>>> run the test twice?  Or would it actually fail?  If the latter,
>>> we'd need to sit on the patch rather longer.
>> The patch removes test.sh, so yes it would break.
> Maybe we could leave test.sh in place for awhile?  I'd rather
> not cause a flag day for buildfarm owners.  (Also, how do we
> see this working in the back branches?)
>
>   


Actually, I was wrong. The module just does "make check" for non-MSVC.
For MSVC it calls vcregress.pl, which the patch doesn't touch (it
should, I think).


cheers


andrew



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

Re: very long record lines in expanded psql output

[Platon Pronko]

On 2021-09-24 14:42, Andrew Dunstan wrote:


On 9/24/21 12:49 AM, Platon Pronko wrote:

On 2021-09-23 22:28, Andrew Dunstan wrote:


2. It would possibly be better to pass the relevant parts of the options
to print_aligned_vertical_line() rather than the whole options
structure. It feels odd to pass both that and opt_border.


What do you think about doing it the other way around - passing only
whole
options structure? That way we will roll 4 parameters (opt_border,
printTextFormat,
and two xheader ones) into only one argument.
This increases code coupling a bit, but I'm not sure if that's
relevant here.



Sure, as long as it doesn't result in duplicated computation.


Hi!

Please find attached the updated patch - with fixed braces and
adjusted parameters to print_aligned_vertical_line().

Best regards,
Platon Pronkodiff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 14e0a4dbe3..78d27caefa 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -2799,6 +2799,32 @@ lo_import 152801
   
   
 
+  
+  xheader_width
+  
+  
+  Sets expanded header width to one of full,
+  column,
+  page,
+  or integer value.
+  
+
+  full is the default option - expanded header
+  is not truncated.
+  
+
+  column: don't print header line past the first
+  column.
+  
+
+  page: fit header line to terminal width.
+  
+
+  integer value: specify exact width of header line.
+  
+  
+  
+
   
   fieldsep
   
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 49d4c0e3ce..ebba14d082 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -4310,6 +4310,32 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 			popt->topt.expanded = !popt->topt.expanded;
 	}
 
+	/* header line width in expanded mode */
+	else if (strcmp(param, "xheader_width") == 0)
+	{
+		if (value && pg_strcasecmp(value, "full") == 0)
+			popt->topt.expanded_header_width_type = PRINT_XHEADER_FULL;
+		else if (value && pg_strcasecmp(value, "column") == 0)
+			popt->topt.expanded_header_width_type = PRINT_XHEADER_COLUMN;
+		else if (value && pg_strcasecmp(value, "page") == 0)
+			popt->topt.expanded_header_width_type = PRINT_XHEADER_PAGE;
+		else if (value)
+		{
+			popt->topt.expanded_header_width_type = PRINT_XHEADER_EXACT_WIDTH;
+			popt->topt.expanded_header_exact_width = atoi(value);
+			if (popt->topt.expanded_header_exact_width == 0)
+			{
+pg_log_error("\\pset: allowed xheader_width values are full (default), column, page, or an number specifying exact width.");
+return false;
+			}
+		}
+		else
+		{
+			/* reset to default if value is empty */
+			popt->topt.expanded_header_width_type = PRINT_XHEADER_FULL;
+		}
+	}
+
 	/* field separator for CSV format */
 	else if (strcmp(param, "csv_fieldsep") == 0)
 	{
@@ -4502,6 +4528,19 @@ printPsetInfo(const char *param, printQueryOpt *popt)
 			printf(_("Expanded display is off.\n"));
 	}
 
+	/* show xheader width type */
+	else if (strcmp(param, "xheader_width") == 0)
+	{
+		if (popt->topt.expanded_header_width_type == PRINT_XHEADER_FULL)
+			printf(_("Expanded header width is 'full'.\n"));
+		else if (popt->topt.expanded_header_width_type == PRINT_XHEADER_COLUMN)
+			printf(_("Expanded header width is 'column'.\n"));
+		else if (popt->topt.expanded_header_width_type == PRINT_XHEADER_PAGE)
+			printf(_("Expanded header width is 'page'.\n"));
+		else if (popt->topt.expanded_header_width_type == PRINT_XHEADER_EXACT_WIDTH)
+			printf(_("Expanded header width is %d.\n"), popt->topt.expanded_header_exact_width);
+	}
+
 	/* show field separator for CSV format */
 	else if (strcmp(param, "csv_fieldsep") == 0)
 	{
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 5cd5838668..e3e3bdc709 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -4168,13 +4168,16 @@ psql_completion(const char *text, int start, int end)
 		 "tableattr", "title", "tuples_only",
 		 "unicode_border_linestyle",
 		 "unicode_column_linestyle",
-		 "unicode_header_linestyle");
+		 "unicode_header_linestyle",
+		 "xheader_width");
 	else if (TailMatchesCS("\\pset", MatchAny))
 	{
 		if (TailMatchesCS("format"))
 			COMPLETE_WITH_CS("aligned", "asciidoc", "csv", "html", "latex",
 			 "latex-longtable", "troff-ms", "unaligned",
 			 "wrapped");
+		else if (TailMatchesCS("xheader_width"))
+			COMPLETE_WITH_CS("full", "column", "page");
 		else if (TailMatchesCS("linestyle"))
 			COMPLETE_WITH_CS("ascii", "old-ascii", "unicode");
 		else if (TailMatchesCS("pager"))
diff --git a/src/fe_utils/print.c b/src/fe_utils/print.c
index d48fcc4a03..1e94772b8b 100644
--- a/src/fe_utils/print.c
+++ b/src/fe_utils/print.c
@@ -1152,15 +1152,16 @@ cleanup:
 
 
 st

Re: 2021-09 Commitfest

[Tom Lane]

Magnus Hagander  writes:
> On Sat, Oct 2, 2021 at 7:31 AM Michael Paquier  wrote:
>> That's the tricky part.  It does not really make sense either to keep
>> moving patches that are waiting on author for months.

> I'm pretty sure this is the original reason for adding this -- to enforce
> that this review happens.

The CF tool is in no way able to enforce that, though.  All it's doing
is making extra work for the CFM.

regards, tom lane

Re: very long record lines in expanded psql output

[Zhihong Yu]

On Sun, Oct 3, 2021 at 5:57 AM Platon Pronko 
wrote:

> On 2021-09-24 14:42, Andrew Dunstan wrote:
> >
> > On 9/24/21 12:49 AM, Platon Pronko wrote:
> >> On 2021-09-23 22:28, Andrew Dunstan wrote:
> >>>
> >>> 2. It would possibly be better to pass the relevant parts of the
> options
> >>> to print_aligned_vertical_line() rather than the whole options
> >>> structure. It feels odd to pass both that and opt_border.
> >>
> >> What do you think about doing it the other way around - passing only
> >> whole
> >> options structure? That way we will roll 4 parameters (opt_border,
> >> printTextFormat,
> >> and two xheader ones) into only one argument.
> >> This increases code coupling a bit, but I'm not sure if that's
> >> relevant here.
> >>
> >
> > Sure, as long as it doesn't result in duplicated computation.
>
> Hi!
>
> Please find attached the updated patch - with fixed braces and
> adjusted parameters to print_aligned_vertical_line().
>
> Best regards,
> Platon Pronko


Hi,

+   pg_log_error("\\pset: allowed xheader_width values are full
(default), column, page, or an number specifying exact width.");

 an number specifying -> a number specifying

Cheers

Re: 2021-09 Commitfest

[Jaime Casanova]

On Sat, Oct 02, 2021 at 11:32:01AM -0400, Tom Lane wrote:
> Alvaro Herrera  writes:
> > On 2021-Oct-02, Tom Lane wrote:
> >> Yeah.  I have been thinking of looking through the oldest CF entries
> >> and proposing that we just reject any that look permanently stalled.
> 
> > I was just going to say the same thing yesterday, and reference [1]
> > when I did it once in 2019.  I think it was a useful cleanup exercise.
> > [1] https://postgr.es/m/20190930182818.GA25331@alvherre.pgsql
> 
> Right.  Michael and Jaime have been doing some of that too in the last
> few days, but obviously a CFM should only do that unilaterally in very
> clear-cut cases of patch abandonment.  I was intending to go after some
> where maybe a bit of community consensus is needed for rejection.
> 

I have done so with 2 or 3 patches that has been stalled more than one
month and after asking in the thread if I receive no answer for 2 or 3
weeks.

-- 
Jaime Casanova
Director de Servicios Profesionales
SystemGuards - Consultores de PostgreSQL

Re: 2021-09 Commitfest

[Jaime Casanova]

On Sat, Oct 02, 2021 at 12:20:09PM -0300, Alvaro Herrera wrote:
> On 2021-Oct-02, Tom Lane wrote:
> 
> > Yeah.  I have been thinking of looking through the oldest CF entries
> > and proposing that we just reject any that look permanently stalled.
> > It doesn't do much good to leave things in the list when there's
> > no apparent interest in pushing them to conclusion.  But I've not
> > done the legwork yet, and I'm a little worried about the push-back
> > that will inevitably result.
> 
> I was just going to say the same thing yesterday, and reference [1]
> when I did it once in 2019.  I think it was a useful cleanup exercise.
> In hindsight, some of these patches were resubmitted later, and those
> are either still ongoing or are already committed.
> [1] https://postgr.es/m/20190930182818.GA25331@alvherre.pgsql
> 
> 
> (I did have the luxury of a local copy of the commitfest database, which
> is perhaps a service we could offer to CFMs to make their lives easier.)
> 

Right now, an option to bulk move everything in their current states to
Next CF would be handy... There are still 139 remaining patches to move.

11 of them "Ready for Committer"

-- 
Jaime Casanova
Director de Servicios Profesionales
SystemGuards - Consultores de PostgreSQL

Re: 2021-09 Commitfest

[Jaime Casanova]

On Sun, Oct 03, 2021 at 11:20:21AM -0500, Jaime Casanova wrote:
> On Sat, Oct 02, 2021 at 11:32:01AM -0400, Tom Lane wrote:
> > Alvaro Herrera  writes:
> > > On 2021-Oct-02, Tom Lane wrote:
> > >> Yeah.  I have been thinking of looking through the oldest CF entries
> > >> and proposing that we just reject any that look permanently stalled.
> > 
> > > I was just going to say the same thing yesterday, and reference [1]
> > > when I did it once in 2019.  I think it was a useful cleanup exercise.
> > > [1] https://postgr.es/m/20190930182818.GA25331@alvherre.pgsql
> > 
> > Right.  Michael and Jaime have been doing some of that too in the last
> > few days, but obviously a CFM should only do that unilaterally in very
> > clear-cut cases of patch abandonment.  I was intending to go after some
> > where maybe a bit of community consensus is needed for rejection.
> > 
> 
> I have done so with 2 or 3 patches that has been stalled more than one
> month and after asking in the thread if I receive no answer for 2 or 3
> weeks.
> 

Actually it should be some kind of rule of thumb (that could be used as
guide) for doing so. Keeping around patches that has no expectation of
being worked on makes us no favor and the queue keeps growing. 

-- 
Jaime Casanova
Director de Servicios Profesionales
SystemGuards - Consultores de PostgreSQL

Re: Reduce lock level for ALTER TABLE ... ADD CHECK .. NOT VALID

[Simon Riggs]

On Sat, 4 Sept 2021 at 20:28, Tom Lane  wrote:

> We have two things that we need to worry about when considering
> reducing ALTER TABLE lock levels:
>
> 1. Is it semantically okay (which is what you claim above)?
>
> 2. Will onlooker processes see sufficiently-consistent catalog data
> if they look at the table during the change?
>
> Trying to reduce the lock level for ADD CHECK fails the second
> test, because it has to alter two different catalogs.  It has
> to increment pg_class.relchecks, and it has to make an entry in
> pg_constraint.  This patch makes it possible for onlookers to
> see a value of pg_class.relchecks that is inconsistent with what
> they find in pg_constraint, and then they will blow up.

Thanks for the review. I will check this consideration for any future patches.

> That happens because the systable_beginscan() in CheckConstraintFetch
> will get a new snapshot, so now it sees the new entry in pg_constraint,
> making the count of entries inconsistent with what it found in pg_class.

This is clearly important and we must now return the patch with feedback.

I've looked at other similar cases and can't find any bugs in other areas, phew!

> It's possible that this could be made safe if we replaced the exact
> "relchecks" count with a boolean "relhaschecks", analogous to the
> way indexes are handled.  It's not clear to me though that the effort,
> and ensuing compatibility costs for applications that look at pg_class,
> would be repaid by having a bit more concurrency here.  One could
> also worry about whether we really want to give up this consistency
> cross-check between pg_class and pg_constraint.

I will work on a patch for this and see how complex it is.

At very least I will add a longer comment patch to mention this for the future.

-- 
Simon Riggshttp://www.EnterpriseDB.com/

ssl tests fail on windows / slurp_file() offset doesn't work on win

[Andres Freund]

Hi,

On 2021-10-02 21:05:17 -0700, Andres Freund wrote:
> Got the build part working (although the state of msvc compatible openssl
> distribution on windows seems a bit scary). However the ssl tests don't
> fully succeed:
> 
> https://cirrus-ci.com/task/6264790323560448?logs=ssl#L655
> 
>  I didn't see code in the bf client code running the test so perhaps that's
>  not too surprising :/
> 
> Did you run those tests on windows?

As you can see in the test output, every mismatch prints the whole file,
despite only intending to show the tail. Which appears to be because the
windows portion of 3c5b0685b921 doesn't actually work.  The reason for that in
turn is that afaict the setFilePointer doesn't change the file position in a
way that affects perl.

Consequently, if I force the !win32 path, the tests pass.

At first I assumed the cause of this is that while the setFilePointer() 
modifies the
state of the underlying handle, it doesn't actually let perl know about
that. Due to buffering etc perl likely has its own bookeeping about the
position in the file. There's some pretty clear hints in
https://perldoc.perl.org/functions/seek

But the problem turns out to be that it's bogus to pass $fh to
setFilePointer(). That's a perl handle, not an win32 handle. Fixing that seems
to make the tests pass.


Why did 3c5b0685b921 choose to use setFilePointer() in the first place? At
this point it's a perl filehandle, so we should just use perl seek?


Leaving the concrete breakage aside, I'm somewhat unhappy that there's not a
single comment explaining why TestLib.pm is trying to use native windows
APIs.

Isn't the code as-is also "leaking" an open IO::Handle? There's a
CloseHandle($fHandle), but nothing is done to $fh. But perhaps there's some
perl magic cleaning things up? Even if so, loks like just closing $fh will
close the handle as well...

Greetings,

Andres Freund

Re: ssl tests fail on windows / slurp_file() offset doesn't work on win

[Andres Freund]

Hi,

On 2021-10-03 10:18:31 -0700, Andres Freund wrote:
> As you can see in the test output, every mismatch prints the whole file,
> despite only intending to show the tail. Which appears to be because the
> windows portion of 3c5b0685b921 doesn't actually work.  The reason for that in
> turn is that afaict the setFilePointer doesn't change the file position in a
> way that affects perl.
> 
> Consequently, if I force the !win32 path, the tests pass.
> 
> At first I assumed the cause of this is that while the setFilePointer() 
> modifies the
> state of the underlying handle, it doesn't actually let perl know about
> that. Due to buffering etc perl likely has its own bookeeping about the
> position in the file. There's some pretty clear hints in
> https://perldoc.perl.org/functions/seek
> 
> But the problem turns out to be that it's bogus to pass $fh to
> setFilePointer(). That's a perl handle, not an win32 handle. Fixing that seems
> to make the tests pass.

It does (I only let it run to the ssl test, then pushed a newer revision):
https://cirrus-ci.com/task/5345293928497152?logs=ssl#L5


> Why did 3c5b0685b921 choose to use setFilePointer() in the first place? At
> this point it's a perl filehandle, so we should just use perl seek?
> 
> 
> Leaving the concrete breakage aside, I'm somewhat unhappy that there's not a
> single comment explaining why TestLib.pm is trying to use native windows
> APIs.
> 
> Isn't the code as-is also "leaking" an open IO::Handle? There's a
> CloseHandle($fHandle), but nothing is done to $fh. But perhaps there's some
> perl magic cleaning things up? Even if so, loks like just closing $fh will
> close the handle as well...

I think something roughly like the attached might be a good idea. Runs locally
on linux, and hopefully still on windows

https://cirrus-ci.com/build/4857291573821440

Greetings,

Andres Freund
>From e84841e58acb20ba0f17ed88d6455deefb265a57 Mon Sep 17 00:00:00 2001
From: Andres Freund 
Date: Sun, 3 Oct 2021 10:07:42 -0700
Subject: [PATCH] WIP: Fix TestLib::slurp_file() with offset on windows.

---
 src/test/perl/TestLib.pm | 34 +-
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index 15f4e6f56e3..2171e93d8ff 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -492,33 +492,33 @@ sub slurp_file
 	my ($filename, $offset) = @_;
 	local $/;
 	my $contents;
+	my $fh;
+
+	# On windows open file using win32 APIs, to allow us to set the
+	# FILE_SHARE_DELETE flag ("d" below), otherwise other accesses to the file
+	# may fail.
 	if ($Config{osname} ne 'MSWin32')
 	{
-		open(my $in, '<', $filename)
+		open($fh, '<', $filename)
 		  or croak "could not read \"$filename\": $!";
-		if (defined($offset))
-		{
-			seek($in, $offset, SEEK_SET)
-			  or croak "could not seek \"$filename\": $!";
-		}
-		$contents = <$in>;
-		close $in;
 	}
 	else
 	{
 		my $fHandle = createFile($filename, "r", "rwd")
 		  or croak "could not open \"$filename\": $^E";
-		OsFHandleOpen(my $fh = IO::Handle->new(), $fHandle, 'r')
+		OsFHandleOpen($fh = IO::Handle->new(), $fHandle, 'r')
 		  or croak "could not read \"$filename\": $^E\n";
-		if (defined($offset))
-		{
-			setFilePointer($fh, $offset, qw(FILE_BEGIN))
-			  or croak "could not seek \"$filename\": $^E\n";
-		}
-		$contents = <$fh>;
-		CloseHandle($fHandle)
-		  or croak "could not close \"$filename\": $^E\n";
 	}
+
+	if (defined($offset))
+	{
+		seek($fh, $offset, SEEK_SET)
+		  or croak "could not seek \"$filename\": $!";
+	}
+
+	$contents = <$fh>;
+	close $fh;
+
 	$contents =~ s/\r\n/\n/g if $Config{osname} eq 'msys';
 	return $contents;
 }
-- 
2.32.0.rc2

Re: [HACKERS] GSoC 2017: Foreign Key Arrays

[Stefan Keller]

Dear all

Just for my understanding - and perhaps as input for the documentation of this:
Are Foreign Key Arrays a means to implement "Generic Foreign Keys" as
in Oracle [1] and Django [2], and of "Polymorphic Associations" as
they call this in Ruby on Rails?

Yours, Stefan

[1] Steven Feuerstein and Debby Russell (1995): Oracle Extension
"PLVfk - Generic Foreign Key Lookups" in: "Advanced Oracle Pl/Sql:
Programming With Packages" (Nutshell Handbook), Oreilly, ISBN
B6AVR6. Webaccess: https://flylib.com/books/en/2.408.1.159/1/
[2] Stackoverflow:
https://stackoverflow.com/questions/14333460/django-generic-foreign-keys-good-or-bad-considering-the-sql-performance

Am Di., 14. Sept. 2021 um 21:00 Uhr schrieb Mark Rofail
:
>
> Dear Alvaro,
>
> We just need to rewrite the scope of the patch so I work on the next 
> generation. I do not know what was "out of scope" in the current version
>
> /Mark
>
> On Tue, 14 Sep 2021, 8:55 pm Alvaro Herrera,  wrote:
>>
>> On 2021-Sep-14, Daniel Gustafsson wrote:
>>
>> > Given the above, and that nothing has happened on this thread since this 
>> > note,
>> > I think we should mark this Returned with Feedback and await a new 
>> > submission.
>> > Does that seem reasonable Alvaro?
>>
>> It seems reasonable to me.
>>
>> --
>> Álvaro Herrera  Valdivia, Chile  —  https://www.EnterpriseDB.com/

Triage on old commitfest entries

[Tom Lane]

As I threatened in another thread, I've looked through all of the
oldest commitfest entries to see which ones should maybe be tossed,
on the grounds that they're unlikely to ever get committed so we
should stop pushing them forward to the next CF.

An important note to make here is that we don't have any explicit
mechanism for saying "sorry, this patch is perhaps useful but it
seems that nobody is going to take an interest in it".  Closing
such a patch as "rejected" seems harsh, but R-W-F isn't very
appropriate either if the patch never got any real review.
Perhaps we should create a new closure state?

I looked at entries that are at least 10 CFs old, as indicated by
the handy sort field.  That's a pretty small population: 16 items
out of the 317 listed in the 2021-09 CF.  A quick look in recent
CFs shows that it's very rare that we commit entries older than
10 CFs.

Here's what I found, along with some commentary about each one.

Patch   Age in CFs

Protect syscache from bloating with negative cache entries  23
Last substantive discussion 2021-01, currently passing cfbot

It's well known that I've never liked this patch, so I can't
claim to be unbiased.  But what I see here is a lot of focus
on specific test scenarios with little concern for the
possibility that other scenarios will be made worse.
I think we need some new ideas to make progress.
Proposed action: RWF

Transactions involving multiple postgres foreign servers18
Last substantive discussion 2021-07, currently failing cfbot

This has been worked on fairly recently, but frankly I'm
dubious that we want to integrate a 2PC XM into Postgres.
Proposed action: Reject

schema variables, LET command   18
Last substantive discussion 2021-09, currently passing cfbot

Seems to be actively worked on, but is it ever going to get
committed?

Remove self join on a unique column 16
Last substantive discussion 2021-07, currently passing cfbot

I'm not exactly sold that this has a good planning-cost-to-
usefulness ratio.
Proposed action: RWF

Index Skip Scan 16
Last substantive discussion 2021-05, currently passing cfbot

Seems possibly useful, but we're not making progress.

standby recovery fails when re-replaying due to missing directory which was 
removed in previous replay  13
Last substantive discussion 2021-09, currently passing cfbot

This is a bug fix, so we shouldn't drop it.

Remove page-read callback from XLogReaderState  12
Last substantive discussion 2021-04, currently failing cfbot

Not sure what to think about this one, but given that it
was pushed and later reverted, I'm suspicious of it.

Incremental Materialized View Maintenance   12
Last substantive discussion 2021-09, currently passing cfbot

Seems to be actively worked on.

pg_upgrade fails with non-standard ACL  12
Last substantive discussion 2021-03, currently passing cfbot

This is a bug fix, so we shouldn't drop it.

Fix up partitionwise join on how equi-join conditions between the partition 
keys are identified 11
Last substantive discussion 2021-07, currently passing cfbot

This is another one where I feel we need new ideas to make
progress.
Proposed action: RWF

A hook for path-removal decision on add_path11
Last substantive discussion 2021-03, currently passing cfbot

I don't think this is a great idea: a hook there will be
costly, and it's very unclear how multiple extensions could
interact correctly.
Proposed action: Reject

Implement INSERT SET syntax 11
Last substantive discussion 2020-03, currently passing cfbot

This one is clearly stalled.  I don't think it's necessarily
a bad idea, but we seem not to be very interested.
Proposed action: Reject for lack of interest

SQL:2011 application time   11
Last substantive discussion 2021-10, currently failing cfbot

Actively worked on, and it's a big feature so long gestation
isn't surprising.

WITH SYSTEM VERSIONING Temporal Tables  11
Last substantive discussion 2021-09, currently failing cfbot

Actively worked on, and it's a big feature so long gestation
isn't surprising.

psql - add SHOW_ALL_RESULTS option  11
Last substantive discussion 2021-09, currently passing cfbot

This got committed and reverted once already.  I have to be
suspicious of whether this is a good design.

Split StdRdOptions into HeapOptions and ToastOptions10
Last substantive discussion 2021-06, currently failing cfbot

I think the author has despaired of anyone else taking an
interest here.  Unless somebody intends to take an interest,
we should put this one out of its misery.
   

Re: Triage on old commitfest entries - JSON_PATH

[Erik Rijkers]

Op 03-10-2021 om 21:14 schreef Tom Lane:

As I threatened in another thread, I've looked through all of the
oldest commitfest entries to see which ones should maybe be tossed,
on the grounds that they're unlikely to ever get committed so we
should stop pushing them forward to the next CF.

An important note to make here is that we don't have any explicit
mechanism for saying "sorry, this patch is perhaps useful but it
seems that nobody is going to take an interest in it".  Closing
such a patch as "rejected" seems harsh, but R-W-F isn't very
appropriate either if the patch never got any real review.
Perhaps we should create a new closure state?

I looked at entries that are at least 10 CFs old, as indicated by
the handy sort field.  That's a pretty small population: 16 items
out of the 317 listed in the 2021-09 CF.  A quick look in recent
CFs shows that it's very rare that we commit entries older than
10 CFs.

Here's what I found, along with some commentary about each one.

Patch   Age in CFs


May I add one more?

SQL/JSON: JSON_TABLE started 2018 (the commitfest page shows only 4 as 
'Age in CFs' but that obviously can't be right)


Although I like the patch & new functionality and Andrew Dunstan has 
worked to keep it up-to-date, there seems to be very little further 
discussion. I makes me a little worried that the time I put in will end 
up sunk in a dead project.



Erik Rijkers



Protect syscache from bloating with negative cache entries  23
Last substantive discussion 2021-01, currently passing cfbot

It's well known that I've never liked this patch, so I can't
claim to be unbiased.  But what I see here is a lot of focus
on specific test scenarios with little concern for the
possibility that other scenarios will be made worse.
I think we need some new ideas to make progress.
Proposed action: RWF

Transactions involving multiple postgres foreign servers18
Last substantive discussion 2021-07, currently failing cfbot

This has been worked on fairly recently, but frankly I'm
dubious that we want to integrate a 2PC XM into Postgres.
Proposed action: Reject

schema variables, LET command   18
Last substantive discussion 2021-09, currently passing cfbot

Seems to be actively worked on, but is it ever going to get
committed?

Remove self join on a unique column 16
Last substantive discussion 2021-07, currently passing cfbot

I'm not exactly sold that this has a good planning-cost-to-
usefulness ratio.
Proposed action: RWF

Index Skip Scan 16
Last substantive discussion 2021-05, currently passing cfbot

Seems possibly useful, but we're not making progress.

standby recovery fails when re-replaying due to missing directory which was 
removed in previous replay  13
Last substantive discussion 2021-09, currently passing cfbot

This is a bug fix, so we shouldn't drop it.

Remove page-read callback from XLogReaderState  12
Last substantive discussion 2021-04, currently failing cfbot

Not sure what to think about this one, but given that it
was pushed and later reverted, I'm suspicious of it.

Incremental Materialized View Maintenance   12
Last substantive discussion 2021-09, currently passing cfbot

Seems to be actively worked on.

pg_upgrade fails with non-standard ACL  12
Last substantive discussion 2021-03, currently passing cfbot

This is a bug fix, so we shouldn't drop it.

Fix up partitionwise join on how equi-join conditions between the partition 
keys are identified 11
Last substantive discussion 2021-07, currently passing cfbot

This is another one where I feel we need new ideas to make
progress.
Proposed action: RWF

A hook for path-removal decision on add_path11
Last substantive discussion 2021-03, currently passing cfbot

I don't think this is a great idea: a hook there will be
costly, and it's very unclear how multiple extensions could
interact correctly.
Proposed action: Reject

Implement INSERT SET syntax 11
Last substantive discussion 2020-03, currently passing cfbot

This one is clearly stalled.  I don't think it's necessarily
a bad idea, but we seem not to be very interested.
Proposed action: Reject for lack of interest

SQL:2011 application time   11
Last substantive discussion 2021-10, currently failing cfbot

Actively worked on, and it's a big feature so long gestation
isn't surprising.

WITH SYSTEM VERSIONING Temporal Tables  11
Last substantive discussion 2021-09, currently failing cfbot

Actively worked on, and it's a big feature so long gestation
isn't surprising.

psql - add SHOW_ALL_RESULTS option  11
Last substantive discussion 2021-09, currently passing c

Re: very long record lines in expanded psql output

[Platon Pronko]

Hi,

+   pg_log_error("\\pset: allowed xheader_width values are full
(default), column, page, or an number specifying exact width.");

  an number specifying -> a number specifying

Cheers



Fixed, attaching the updated patch. Thank you!

Best regards,
Platon Pronkodiff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 14e0a4dbe3..78d27caefa 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -2799,6 +2799,32 @@ lo_import 152801
   
   
 
+  
+  xheader_width
+  
+  
+  Sets expanded header width to one of full,
+  column,
+  page,
+  or integer value.
+  
+
+  full is the default option - expanded header
+  is not truncated.
+  
+
+  column: don't print header line past the first
+  column.
+  
+
+  page: fit header line to terminal width.
+  
+
+  integer value: specify exact width of header line.
+  
+  
+  
+
   
   fieldsep
   
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 49d4c0e3ce..801810aba9 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -4310,6 +4310,32 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 			popt->topt.expanded = !popt->topt.expanded;
 	}
 
+	/* header line width in expanded mode */
+	else if (strcmp(param, "xheader_width") == 0)
+	{
+		if (value && pg_strcasecmp(value, "full") == 0)
+			popt->topt.expanded_header_width_type = PRINT_XHEADER_FULL;
+		else if (value && pg_strcasecmp(value, "column") == 0)
+			popt->topt.expanded_header_width_type = PRINT_XHEADER_COLUMN;
+		else if (value && pg_strcasecmp(value, "page") == 0)
+			popt->topt.expanded_header_width_type = PRINT_XHEADER_PAGE;
+		else if (value)
+		{
+			popt->topt.expanded_header_width_type = PRINT_XHEADER_EXACT_WIDTH;
+			popt->topt.expanded_header_exact_width = atoi(value);
+			if (popt->topt.expanded_header_exact_width == 0)
+			{
+pg_log_error("\\pset: allowed xheader_width values are full (default), column, page, or a number specifying exact width.");
+return false;
+			}
+		}
+		else
+		{
+			/* reset to default if value is empty */
+			popt->topt.expanded_header_width_type = PRINT_XHEADER_FULL;
+		}
+	}
+
 	/* field separator for CSV format */
 	else if (strcmp(param, "csv_fieldsep") == 0)
 	{
@@ -4502,6 +4528,19 @@ printPsetInfo(const char *param, printQueryOpt *popt)
 			printf(_("Expanded display is off.\n"));
 	}
 
+	/* show xheader width type */
+	else if (strcmp(param, "xheader_width") == 0)
+	{
+		if (popt->topt.expanded_header_width_type == PRINT_XHEADER_FULL)
+			printf(_("Expanded header width is 'full'.\n"));
+		else if (popt->topt.expanded_header_width_type == PRINT_XHEADER_COLUMN)
+			printf(_("Expanded header width is 'column'.\n"));
+		else if (popt->topt.expanded_header_width_type == PRINT_XHEADER_PAGE)
+			printf(_("Expanded header width is 'page'.\n"));
+		else if (popt->topt.expanded_header_width_type == PRINT_XHEADER_EXACT_WIDTH)
+			printf(_("Expanded header width is %d.\n"), popt->topt.expanded_header_exact_width);
+	}
+
 	/* show field separator for CSV format */
 	else if (strcmp(param, "csv_fieldsep") == 0)
 	{
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 5cd5838668..e3e3bdc709 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -4168,13 +4168,16 @@ psql_completion(const char *text, int start, int end)
 		 "tableattr", "title", "tuples_only",
 		 "unicode_border_linestyle",
 		 "unicode_column_linestyle",
-		 "unicode_header_linestyle");
+		 "unicode_header_linestyle",
+		 "xheader_width");
 	else if (TailMatchesCS("\\pset", MatchAny))
 	{
 		if (TailMatchesCS("format"))
 			COMPLETE_WITH_CS("aligned", "asciidoc", "csv", "html", "latex",
 			 "latex-longtable", "troff-ms", "unaligned",
 			 "wrapped");
+		else if (TailMatchesCS("xheader_width"))
+			COMPLETE_WITH_CS("full", "column", "page");
 		else if (TailMatchesCS("linestyle"))
 			COMPLETE_WITH_CS("ascii", "old-ascii", "unicode");
 		else if (TailMatchesCS("pager"))
diff --git a/src/fe_utils/print.c b/src/fe_utils/print.c
index d48fcc4a03..1e94772b8b 100644
--- a/src/fe_utils/print.c
+++ b/src/fe_utils/print.c
@@ -1152,15 +1152,16 @@ cleanup:
 
 
 static void
-print_aligned_vertical_line(const printTextFormat *format,
-			const unsigned short opt_border,
+print_aligned_vertical_line(const printTableOpt *topt,
 			unsigned long record,
 			unsigned int hwidth,
 			unsigned int dwidth,
+			int output_columns,
 			printTextRule pos,
 			FILE *fout)
 {
-	const printTextLineFormat *lformat = &format->lrule[pos];
+	const printTextLineFormat *lformat = &get_line_style(topt)->lrule[pos];
+	const unsigned short opt_border = topt->border;
 	unsigned int i;
 	int			reclen =

Re: Triage on old commitfest entries - JSON_PATH

[Tom Lane]

Erik Rijkers  writes:
> Op 03-10-2021 om 21:14 schreef Tom Lane:
>> I looked at entries that are at least 10 CFs old, as indicated by
>> the handy sort field.  That's a pretty small population: 16 items
>> out of the 317 listed in the 2021-09 CF.  A quick look in recent
>> CFs shows that it's very rare that we commit entries older than
>> 10 CFs.

> May I add one more?

> SQL/JSON: JSON_TABLE started 2018 (the commitfest page shows only 4 as 
> 'Age in CFs' but that obviously can't be right)

Hm.  It's being actively worked on, so I wouldn't have proposed
killing it even if its age had been shown correctly.  Unless you
think it has no hope of ever reaching committability?

regards, tom lane

Re: Triage on old commitfest entries

[Peter Geoghegan]

On Sun, Oct 3, 2021 at 12:15 PM Tom Lane  wrote:
> An important note to make here is that we don't have any explicit
> mechanism for saying "sorry, this patch is perhaps useful but it
> seems that nobody is going to take an interest in it".  Closing
> such a patch as "rejected" seems harsh, but R-W-F isn't very
> appropriate either if the patch never got any real review.
> Perhaps we should create a new closure state?

We don't reject patches, except in very rare cases where the whole
concept is wildly unreasonable, or when the patch author decides to
mark their own patch rejected. In other words, we only reject patches
where the formal status of being rejected hardly matters at all. I
have to wonder what the point of the status of "rejected" really is.
Ambiguity about what the best way forward is seems to be the thing
that kills patches -- it is seldom mistakes or design problems. They
can usually be corrected easily. Sometimes the ambiguity is very
broad, other times it's just one aspect of the design (e.g., the
planner aspects).

I'd rather go in the opposite direction here: merge "Rejected" and
"Returned with Feedback" into a single "Patch Returned" category
(without adding a third category). The odds of a CF entry that gets
marked R-W-F eventually being committed is, in general, totally
unclear, or seems to be. I myself have zero faith that that status
alone predicts anything, good or bad. I think that under-specifying
why a patch has been returned like this would actually be *more*
informative. Less experienced contributors wouldn't have to waste
their time looking for some signal, when in fact there is little more
than noise.

> Index Skip Scan 16
> Last substantive discussion 2021-05, currently passing cfbot
>
> Seems possibly useful, but we're not making progress.

This feature is definitely useful. My pet theory is that it hasn't
made more progress because it requires expertise in two fairly
distinct areas of the system. There is a lot of B-Tree stuff here,
which is clearly my thing. But I know that I personally am much less
likely to work on a patch that requires significant changes to the
planner. Maybe this is a coordination problem.

-- 
Peter Geoghegan

Re: Triage on old commitfest entries

[Tom Lane]

Peter Geoghegan  writes:
> On Sun, Oct 3, 2021 at 12:15 PM Tom Lane  wrote:
>> Perhaps we should create a new closure state?

> I'd rather go in the opposite direction here: merge "Rejected" and
> "Returned with Feedback" into a single "Patch Returned" category
> (without adding a third category).

Hm, perhaps.  You're right that the classification might be slippery.
I do feel it's useful to distinguish "this is a bad idea overall,
we don't want to see follow-on patches" from "this needs work, please
send a follow-on patch when you've done the work".  But maybe more
thought could get an idea out of the first category and into the
second.

>> Index Skip Scan 16
>> Last substantive discussion 2021-05, currently passing cfbot
>> 
>> Seems possibly useful, but we're not making progress.

> This feature is definitely useful. My pet theory is that it hasn't
> made more progress because it requires expertise in two fairly
> distinct areas of the system. There is a lot of B-Tree stuff here,
> which is clearly my thing. But I know that I personally am much less
> likely to work on a patch that requires significant changes to the
> planner. Maybe this is a coordination problem.

Fair.  My concern here is mostly that we not just keep kicking the
can down the road.  If we see that a patch has been hanging around
this long without reaching commit, we should either kill it or
form a specific plan for how to advance it.

regards, tom lane

Re: Adding CI to our tree

[Daniel Gustafsson]

> On 3 Oct 2021, at 06:05, Andres Freund  wrote:

> Did you run those tests on windows?

Sorry, failed to mention I only compile it for now, I hadn't reached trying to
run the tests yet.  I see you started on that in this thread, so thank you for
that!

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

Re: Better context for "TAP tests not enabled" error message

[Kevin Burke]

Updated patch that removes the "Maybe"


--
Kevin Burke
phone: 925-271-7005 | kevin.burke.dev


On Sat, Oct 2, 2021 at 11:19 AM Tom Lane  wrote:

> Daniel Gustafsson  writes:
> >> On 2 Oct 2021, at 02:09, Kevin Burke  wrote:
> >> This patch amends the error message to give help to the user.
>
> > I think this makes sense,
>
> +1.  I'd take out the "Maybe"; the diagnosis seems pretty certain.
>
> regards, tom lane
>


0001-Makefile-provide-better-help-if-TAP-tests-are-not-en.patch
Description: Binary data

Re: Triage on old commitfest entries

[Peter Geoghegan]

On Sun, Oct 3, 2021 at 1:30 PM Tom Lane  wrote:
> Hm, perhaps.  You're right that the classification might be slippery.
> I do feel it's useful to distinguish "this is a bad idea overall,
> we don't want to see follow-on patches" from "this needs work, please
> send a follow-on patch when you've done the work".  But maybe more
> thought could get an idea out of the first category and into the
> second.

I agree in principle, but experience suggests that there is
approximately zero practical difference.

My whole approach is to filter aggressively. I can only speak for
myself, but I have to imagine that this is what most committers do, in
one way or another. I am focussed on what I can understand with a high
degree of confidence, that seems likely to be relatively beneficial to
users -- nothing more. So patch authors that receive no feedback from
me ought to assume that that means absolutely nothing, even in areas
where my input might be expected. I'm not saying that I *never*
mentally write-off patches without saying anything, but it's rare, and
when it happens it tends to be in the least interesting, most obvious
cases -- cases where speaking up is clearly unnecessary. I would hate
to think that less experienced patch authors are taking radio silence
as a meaningful signal, whether it's from me or from somebody else --
because it's really not like that at all.

My argument boils down to this: I think that less experienced
contributors are better served by a system that plainly admits this
uncertainty. At the same time I think that old patches need to get
bumped for the good of all patch authors collectively. We have a hard
time bumping patches today because we seem to feel the need to justify
it, based on facts about the patch. The reality has always been that
Postgres patches are rejected by default, not accepted by default. We
should be clear about this.

> Fair.  My concern here is mostly that we not just keep kicking the
> can down the road.  If we see that a patch has been hanging around
> this long without reaching commit, we should either kill it or
> form a specific plan for how to advance it.

Also fair.

The pandemic has made the kind of coordination I refer to harder in
practice. It's the kind of thing that face to face communication
really helps with.

-- 
Peter Geoghegan

Re: BUG #17212: pg_amcheck fails on checking temporary relations

[Mark Dilger]

> On Oct 3, 2021, at 10:04 AM, Mark Dilger  wrote:
> 
>> On Oct 2, 2021, at 10:32 AM, Peter Geoghegan  wrote:
>> 
>> On Sat, Oct 2, 2021 at 4:49 AM PG Bug reporting form
>>  wrote:
>>> Although you can add --exclude-relation=*.pg_temp*.*, this behaviour differs
>>> from the behaviour of pg_dump and friends, which skip such relations
>>> silently.
>> 
>> I agree -- this behavior is a bug.
>> 
>> Can you propose a fix, Mark?
> 
> The attached patch includes a test case for this, which shows the problems 
> against the current pg_amcheck.c, and a new version of pg_amcheck.c which 
> fixes the bug.  Could you review it?
> 
> Thanks for bringing this to my attention.

Reposting to pgsql-hackers in preparation for making a commitfest entry.



v1-0001-Bug-fix-do-not-check-temp-tables-from-pg_amcheck.patch
Description: Binary data


—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: Skipping logical replication transactions on subscriber side

[Masahiko Sawada]

On Fri, Oct 1, 2021 at 5:32 PM Amit Kapila  wrote:
>
> On Fri, Oct 1, 2021 at 6:30 AM Masahiko Sawada  wrote:
> >
> > On Fri, Oct 1, 2021 at 5:05 AM Peter Eisentraut
> >  wrote:
> > >
> > > On 30.09.21 07:45, Masahiko Sawada wrote:
> > > > I've attached updated patches that incorporate all comments I got so
> > > > far. Please review them.
> > >
> > > I'm uneasy about the way the xids-to-be-skipped are presented as
> > > subscriptions options, similar to settings such as "binary".  I see how
> > > that is convenient, but it's not really the same thing, in how you use
> > > it, is it?  Even if we share some details internally, I feel that there
> > > should be a separate syntax somehow.
> >
> > Since I was thinking that ALTER SUBSCRIPTION ... SET is used to alter
> > parameters originally set by CREATE SUBSCRIPTION, in the first several
> > version patches it added a separate syntax for this feature like ALTER
> > SUBSCRIPTION ... SET SKIP TRANSACTION xxx. But Amit was concerned
> > about an additional syntax and consistency with disable_on_error[1]
> > which is proposed by Mark Diliger[2], so I’ve changed it to a
> > subscription option.
> >
>
> Yeah, the basic idea is that this is not the only option we will
> support for taking actions on error/conflict. For example, we might
> want to disable subscriptions or allow skipping transactions based on
> XID, LSN, etc.

I guess disabling subscriptions on error/conflict and skipping the
particular transactions are somewhat different types of functions.
Disabling subscriptions on error/conflict seems likes a setting
parameter of subscriptions. The users might want to specify this
option at creation time. Whereas, skipping the particular transaction
is a repair function that the user might want to use on the spot in
case of a failure. I’m concerned a bit that combining these functions
to one syntax could confuse the users.

Regards,

-- 
Masahiko Sawada
EDB:  https://www.enterprisedb.com/

Re: (LOCK TABLE options) “ONLY” and “NOWAIT” are not yet implemented

[bt21tanigaway]

else if (Matches("LOCK", MatchAny, "IN", "ACCESS|ROW") ||
-Matches("LOCK", "TABLE", MatchAny, "IN", "ACCESS|ROW"))
+Matches("LOCK", "TABLE", MatchAny, "IN", "ACCESS|ROW") 
||
+Matches("LOCK", "ONLY", MatchAny, "IN", "ACCESS|ROW") 
||
+Matches("LOCK", "TABLE", "ONLY", MatchAny, "IN", 
"ACCESS|ROW"))

I think this code is redundant, so I change following.
---
else if (HeadMatches("LOCK") && TailMatches("IN", "ACCESS|ROW"))
---
I created the patch, and attached it. Do you think?

Thank you for update!
I think that your code is more concise than mine.
There seems to be no problem.

Regards,
Koyu Tanigawa

Re: Triage on old commitfest entries

[Pavel Stehule]

Hi

schema variables, LET command   18
> Last substantive discussion 2021-09, currently passing cfbot
>
> Seems to be actively worked on, but is it ever going to get
> committed?
>
>
This patch was originally very dirty with a strange design - something
between command and query. But on second hand, these issues are real and
there was a lot of work to have good performance for CALL statements and
still CALL statements is limited to using just simple expressions.

In January of this year I completely rewrote this feature (significant
part). So the implementation is very new, and I hope it can be better
included in Postgres concepts.

This feature is interesting mainly for RLS - it allows secure space in
memory, and it is available from all environments in Postgres. Second usage
can be emulation of package variables. Current emulations are very slow or
require extensions. The schema variables (session variables) can be used
badly or well. I migrated one Oracle's application, where it was an hell,
but when you do migration, then is not too much possibility for complex
redesign. I hope so this feature can be nice for users who need to write
SQL scripts, because it reduce an necessary work for pushing values to
server side. It can be used for parametrisation of "DO" blocks.

The current patch is trimmed to implementation not transactional variables,
what I think should be default behaviour (like any other databases do it).
This limit is just for reducing of necessity work with maintaining of this
patch. I have prepared patch with support transactional behaviour too (that
can have nice uses cases too). But is hard to maintain this part of patch
to be applicable every week, so I postponed this part of patch.

Regards

Pavel

Re: Triage on old commitfest entries - JSON_PATH

[Pavel Stehule]

ne 3. 10. 2021 v 22:16 odesílatel Tom Lane  napsal:

> Erik Rijkers  writes:
> > Op 03-10-2021 om 21:14 schreef Tom Lane:
> >> I looked at entries that are at least 10 CFs old, as indicated by
> >> the handy sort field.  That's a pretty small population: 16 items
> >> out of the 317 listed in the 2021-09 CF.  A quick look in recent
> >> CFs shows that it's very rare that we commit entries older than
> >> 10 CFs.
>
> > May I add one more?
>
> > SQL/JSON: JSON_TABLE started 2018 (the commitfest page shows only 4 as
> > 'Age in CFs' but that obviously can't be right)
>
> Hm.  It's being actively worked on, so I wouldn't have proposed
> killing it even if its age had been shown correctly.  Unless you
> think it has no hope of ever reaching committability?
>

This is a pretty important feature and a nice patch.

Unfortunately, it is a pretty complex patch - JSON_TABLE is a really
complex function, and this patch does complete implementation. I checked
this patch more times, and I think it is good. There is only one problem -
the size (there are not any problems in code, or in behaviour) . In MySQL
or MariaDB, there is a much more simple implementation, that covers maybe
10% of standard. But it is available, and people can use it. Isn't it
possible to reduce this patch to some basic functionality, and commit it
quickly, and later commit step by step all parts.

Regards

Pavel




> regards, tom lane
>
>
>

Re: BufferAlloc: don't take two simultaneous locks

[Yura Sokolov]

В Пт, 01/10/2021 в 15:46 -0700, Zhihong Yu wrote:
> 
> 
> On Fri, Oct 1, 2021 at 3:26 PM Yura Sokolov 
> wrote:
> > Good day.
> > 
> > I found some opportunity in Buffer Manager code in BufferAlloc
> > function:
> > - When valid buffer is evicted, BufferAlloc acquires two partition
> > lwlocks: for partition for evicted block is in and partition for new
> > block placement.
> > 
> > It doesn't matter if there is small number of concurrent
> > replacements.
> > But if there are a lot of concurrent backends replacing buffers,
> > complex dependency net quickly arose.
> > 
> > It could be easily seen with select-only pgbench with scale 100 and
> > shared buffers 128MB: scale 100 produces 1.5GB tables, and it
> > certainly
> > doesn't fit shared buffers. This way performance starts to degrade
> > at
> > ~100 connections. Even with shared buffers 1GB it slowly degrades
> > after
> > 150 connections. 
> > 
> > But strictly speaking, there is no need to hold both lock
> > simultaneously. Buffer is pinned so other processes could not select
> > it
> > for eviction. If tag is cleared and buffer removed from old
> > partition
> > then other processes will not find it. Therefore it is safe to
> > release
> > old partition lock before acquiring new partition lock.
> > 
> > If other process concurrently inserts same new buffer, then old
> > buffer
> > is placed to bufmanager's freelist.
> > 
> > Additional optimisation: in case of old buffer is reused, there is
> > no
> > need to put its BufferLookupEnt into dynahash's freelist. That
> > reduces
> > lock contention a bit more. To acomplish this FreeListData.nentries
> > is
> > changed to pg_atomic_u32/pg_atomic_u64 and atomic
> > increment/decrement
> > is used.
> > 
> > Remark: there were bug in the `hash_update_hash_key`: nentries were
> > not
> > kept in sync if freelist partitions differ. This bug were never
> > triggered because single use of `hash_update_hash_key` doesn't move
> > entry between partitions.
> > 
> > There is some tests results.
> > 
> > - pgbench with scale 100 were tested with --select-only (since we
> > want
> > to test buffer manager alone). It produces 1.5GB table.
> > - two shared_buffers values were tested: 128MB and 1GB.
> > - second best result were taken among five runs
> > 
> > Test were made in three system configurations:
> > - notebook with i7-1165G7 (limited to 2.8GHz to not overheat)
> > - Xeon X5675 6 core 2 socket NUMA system (12 cores/24 threads).
> > - same Xeon X5675 but restricted to single socket
> >   (with numactl -m 0 -N 0)
> > 
> > Results for i7-1165G7:
> > 
> >   conns | master |patched |  master 1G | patched 1G 
> > ++++
> >   1 |  29667 |  29079 |  29425 |  29411 
> >   2 |  55577 |  3 |  57974 |  57223 
> >   3 |  87393 |  87924 |  87246 |  89210 
> >   5 | 136222 | 136879 | 133775 | 133949 
> >   7 | 179865 | 176734 | 178297 | 175559 
> >  17 | 215953 | 214708 | 222908 | 223651 
> >  27 | 211162 | 213014 | 220506 | 219752 
> >  53 | 211620 | 218702 | 220906 | 225218 
> >  83 | 213488 | 221799 | 219075 | 228096 
> > 107 | 212018 | 222110 | 222502 | 227825 
> > 139 | 207068 | 220812 | 218191 | 226712 
> > 163 | 203716 | 220793 | 213498 | 226493 
> > 191 | 199248 | 217486 | 210994 | 221026 
> > 211 | 195887 | 217356 | 209601 | 219397 
> > 239 | 193133 | 215695 | 209023 | 218773 
> > 271 | 190686 | 213668 | 207181 | 219137 
> > 307 | 188066 | 214120 | 205392 | 218782 
> > 353 | 185449 | 213570 | 202120 | 217786 
> > 397 | 182173 | 212168 | 201285 | 216489 
> > 
> > Results for 1 socket X5675
> > 
> >   conns | master |patched |  master 1G | patched 1G 
> > ++++
> >   1 |  16864 |  16584 |  17419 |  17630 
> >   2 |  32764 |  32735 |  34593 |  34000 
> >   3 |  47258 |  46022 |  49570 |  47432 
> >   5 |  64487 |  64929 |  68369 |  68885 
> >   7 |  81932 |  82034 |  87543 |  87538 
> >  17 | 114502 | 114218 | 127347 | 127448 
> >  27 | 116030 | 115758 | 130003 | 128890 
> >  53 | 116814 | 117197 | 131142 | 131080 
> >  83 | 114438 | 116704 | 130198 | 130985 
> > 107 | 113255 | 116910 | 129932 | 131468 
> > 139 | 111577 | 116929 | 129012 | 131782 
> > 163 | 110477 | 116818 | 128628 | 131697 
> > 191 | 109237 | 116672 | 127833 | 131586 
> > 211 | 108248 |  

Re: pgsql: Document XLOG_INCLUDE_XID a little better

[Amit Kapila]

On Sun, Oct 3, 2021 at 5:05 PM Dilip Kumar  wrote:
>
> On Sat, Oct 2, 2021 at 8:10 PM Alvaro Herrera  wrote:
> >
> > On 2021-Oct-02, Dilip Kumar wrote:
> >
> > > I have written two patches, Approach1 is as you described using a
> > > static boolean and Approach2 as a local variable to XLogAssembleRecord
> > > as described by Amit, attached both of them for your reference.
> > > IMHO, either of these approaches looks cleaner.
> >
> > Thanks!  I haven't read these patches carefully, but I think the
> > variable is about assigning the *subxid*, not the topxid.  Amit can
> > confirm ...
>
> IIRC, this variable is for logging the top xid in the first WAL by
> each subtransaction. So that during logical decoding, while creating
> the ReorderBufferTxn for the subtransaction we can associate it to the
> top transaction without seeing the commit WAL.
>

This is correct.

-- 
With Regards,
Amit Kapila.

Re: (LOCK TABLE options) “ONLY” and “NOWAIT” are not yet implemented

[Fujii Masao]

On 2021/10/04 11:17, bt21tanigaway wrote:

else if (Matches("LOCK", MatchAny, "IN", "ACCESS|ROW") ||
- Matches("LOCK", "TABLE", MatchAny, "IN", "ACCESS|ROW"))
+ Matches("LOCK", "TABLE", MatchAny, "IN", "ACCESS|ROW") ||
+ Matches("LOCK", "ONLY", MatchAny, "IN", "ACCESS|ROW") ||
+ Matches("LOCK", "TABLE", "ONLY", MatchAny, "IN", "ACCESS|ROW"))

I think this code is redundant, so I change following.
---
else if (HeadMatches("LOCK") && TailMatches("IN", "ACCESS|ROW"))
---
I created the patch, and attached it. Do you think?

Thank you for update!
I think that your code is more concise than mine.
There seems to be no problem.


The patch looks good to me, too. I applied cosmetic changes to it.
Attached is the updated version of the patch. Barring any objection,
I will commit it.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 5cd5838668..ecae9df8ed 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3599,40 +3599,49 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH("(");
 
 /* LOCK */
-   /* Complete LOCK [TABLE] with a list of tables */
+   /* Complete LOCK [TABLE] [ONLY] with a list of tables */
else if (Matches("LOCK"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables,
-  " UNION 
SELECT 'TABLE'");
+  " UNION 
SELECT 'TABLE'"
+  " UNION 
SELECT 'ONLY'");
else if (Matches("LOCK", "TABLE"))
-   COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, "");
-
+   COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables,
+  " UNION 
SELECT 'ONLY'");
+   else if (Matches("LOCK", "TABLE", "ONLY") || Matches("LOCK", "ONLY"))
+   COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL);
/* For the following, handle the case of a single table only for now */
 
-   /* Complete LOCK [TABLE]  with "IN" */
-   else if (Matches("LOCK", MatchAnyExcept("TABLE")) ||
-Matches("LOCK", "TABLE", MatchAny))
-   COMPLETE_WITH("IN");
+   /* Complete LOCK [TABLE] [ONLY]  with IN or NOWAIT */
+   else if (Matches("LOCK", MatchAnyExcept("TABLE|ONLY")) ||
+Matches("LOCK", "TABLE", MatchAnyExcept("ONLY")) ||
+Matches("LOCK", "ONLY", MatchAny) ||
+Matches("LOCK", "TABLE", "ONLY", MatchAny))
+   COMPLETE_WITH("IN", "NOWAIT");
 
-   /* Complete LOCK [TABLE]  IN with a lock mode */
-   else if (Matches("LOCK", MatchAny, "IN") ||
-Matches("LOCK", "TABLE", MatchAny, "IN"))
+   /* Complete LOCK [TABLE] [ONLY]  IN with a lock mode */
+   else if (HeadMatches("LOCK") && TailMatches("IN"))
COMPLETE_WITH("ACCESS SHARE MODE",
  "ROW SHARE MODE", "ROW EXCLUSIVE 
MODE",
  "SHARE UPDATE EXCLUSIVE MODE", "SHARE 
MODE",
  "SHARE ROW EXCLUSIVE MODE",
  "EXCLUSIVE MODE", "ACCESS EXCLUSIVE 
MODE");
 
-   /* Complete LOCK [TABLE]  IN ACCESS|ROW with rest of lock mode */
-   else if (Matches("LOCK", MatchAny, "IN", "ACCESS|ROW") ||
-Matches("LOCK", "TABLE", MatchAny, "IN", "ACCESS|ROW"))
+   /*
+* Complete LOCK [TABLE][ONLY]  IN ACCESS|ROW with rest of lock
+* mode
+*/
+   else if (HeadMatches("LOCK") && TailMatches("IN", "ACCESS|ROW"))
COMPLETE_WITH("EXCLUSIVE MODE", "SHARE MODE");
 
-   /* Complete LOCK [TABLE]  IN SHARE with rest of lock mode */
-   else if (Matches("LOCK", MatchAny, "IN", "SHARE") ||
-Matches("LOCK", "TABLE", MatchAny, "IN", "SHARE"))
+   /* Complete LOCK [TABLE] [ONLY]  IN SHARE with rest of lock mode 
*/
+   else if (HeadMatches("LOCK") && TailMatches("IN", "SHARE"))
COMPLETE_WITH("MODE", "ROW EXCLUSIVE MODE",
  "UPDATE EXCLUSIVE MODE");
 
+   /* Complete LOCK [TABLE] [ONLY]  [IN lockmode MODE] with 
"NOWAIT" */
+   else if (HeadMatches("LOCK") && TailMatches("MODE"))
+   COMPLETE_WITH("NOWAIT");
+
 /* NOTIFY --- can be inside EXPLAIN, RULE, etc */
else if (TailMatches("NOTIFY"))
COMPLETE_WITH_QUERY("SELECT pg_catalog.quote_ident(channel) 
FROM pg_catalog.pg_listening_channels() AS channel WHERE 
substring(pg_catalog.quote_ident(channel),1,%d)='%s'");

Re: pgsql: Document XLOG_INCLUDE_XID a little better

[Amit Kapila]

On Fri, Oct 1, 2021 at 6:23 PM Alvaro Herrera  wrote:
>
> On 2021-Oct-01, Amit Kapila wrote:
>
> > AFAICS, there are two possibilities w.r.t global variables: (a) use
> > curinsert_flags which we are doing now, (b) another is to introduce a
> > new global variable, set it after we make the association, and then
> > reset it after we mark SubTransaction assigned and on error. I have
> > also thought of passing it via XLogCtlInsert but as that is shared by
> > different processes, it can be set by one process and be read by
> > another process which we don't want here.
>
> So, in my mind, curinsert_flags is a way for the high-level user of
> XLogInsert to pass info about the record being inserted to the low-level
> xloginsert.c infrastructure.  In contrast, XLOG_INCLUDE_XID is being
> used solely within xloginsert.c, by one piece of low-level
> infrastructure to communicate to another piece of low-level
> infrastructure that some cleanup is needed.  Nothing outside of
> xloginsert.c needs to know anything about XLOG_INCLUDE_XID, in contrast
> with the other bits that can be set by XLogSetRecordFlags.  You could
> move the #define to xloginsert.c and everything would compile fine.
>
> Another tell-tale sign that things are not fitting right is that
> XLOG_INCLUDE_XID is not set via XLogSetRecordFlags, contrary the comment
> above those defines.
>
> (Aside: I wonder why do we have XLogSetRecordFlags at all -- I think we
> could just pass the other two flags via XLogBeginInsert).
>

Agreed, I think we can do that if we want but we still need to set
curinsert_flags or some other similar variable in xloginsert.c so that
we can later use and reset it.

> Regarding XLOG_INCLUDE_XID, I don't think replacing it with a bit in
> shared memory is a good idea, since it only applies to the insertion
> being carried out by the current process, right?
>

Right. Ideally, we can set this in a local variable via
XLogRecordAssemble() and then use it in XLogInsertRecord() as is done
in 0001-Refactor-code-for-logging-the-top-transaction-id-in-Approach2.
Basically, we just need to ensure that we mark the
CurrentTransactionState for this flag once we are sure that the
function XLogInsertRecord() will perform the insertion and won't
return InvalidXLogRecPtr. OTOH, I see the point in using a global
static variable to achieve this purpose as that allows to do the
required work only in xloginsert.c.

> I think a straight standalone variable (probably a static boolean in
> xloginsert.c) might be less confusing.
>
> ... so, reading the xact.c code again, TransactionState->assigned really
> means "whether the subXID-to-topXID association has been wal-logged",
> which is a completely different meaning from what the term 'assigned'
> means in all other comments in xact.c ...
>

I think you have interpreted it correctly and we make this association
logged with the first WAL of each subtransaction if the WAL level is
logical.

-- 
With Regards,
Amit Kapila.

Re: Add some tests for pg_stat_statements compatibility verification under contrib

[Michael Paquier]

On Thu, Sep 30, 2021 at 11:12:21AM +0900, Michael Paquier wrote:
> There is also no need for tests on 1.9, which is the latest version.
> Tests for this one should be added once we bump the code to the next
> version.  At the end I finish with the attached, counting for the
> back-and-forth game with pg_read_all_stats.

Done as of 2b0da03.
--
Michael


signature.asc
Description: PGP signature

Re: Triage on old commitfest entries

[Fabien COELHO]

Hello Tom,


As I threatened in another thread, I've looked through all of the
oldest commitfest entries to see which ones should maybe be tossed,
on the grounds that they're unlikely to ever get committed so we
should stop pushing them forward to the next CF.




psql - add SHOW_ALL_RESULTS option  11
Last substantive discussion 2021-09, currently passing cfbot

This got committed and reverted once already.  I have to be
suspicious of whether this is a good design.



Thoughts?


ISTM that the main problem with this patch is that it touches a barely 
tested piece of software, aka "psql":-( The second problem is that the 
initial code is fragile because it handles different modes with pretty 
intricate code.


So, on the first commit it broke a few untested things, among the many 
untested things.


This resulted in more tests being added (sql, tap) so that the relevant 
features are covered, so my point of view is that this patch is currently 
a net improvement both from an engineering perspective and for features 
and enabling other features. Also, there is some interest to get it in.


So I do not think that it deserves to be dropped.

--
Fabien.

RE: (LOCK TABLE options) “ONLY” and “NOWAIT” are not yet implemented

[Shinya11.Kato]

>-Original Message-
>From: Fujii Masao 
>Sent: Monday, October 4, 2021 1:59 PM
>To: bt21tanigaway ; RDH 加藤 慎也/Kato,
>Shinya (NTT DATA) 
>Cc: pgsql-hackers@lists.postgresql.org
>Subject: Re: (LOCK TABLE options) “ONLY” and “NOWAIT” are not yet
>implemented
>
>
>
>On 2021/10/04 11:17, bt21tanigaway wrote:
 else if (Matches("LOCK", MatchAny, "IN", "ACCESS|ROW") ||
 - Matches("LOCK", "TABLE", MatchAny, "IN",
 "ACCESS|ROW"))
 + Matches("LOCK", "TABLE", MatchAny, "IN",
>"ACCESS|ROW")
 +||
 + Matches("LOCK", "ONLY", MatchAny, "IN",
>"ACCESS|ROW")
 +||
 + Matches("LOCK", "TABLE", "ONLY", MatchAny, "IN",
 +"ACCESS|ROW"))
>>> I think this code is redundant, so I change following.
>>> ---
>>> else if (HeadMatches("LOCK") && TailMatches("IN", "ACCESS|ROW"))
>>> ---
>>> I created the patch, and attached it. Do you think?
>> Thank you for update!
>> I think that your code is more concise than mine.
>> There seems to be no problem.
>
>The patch looks good to me, too. I applied cosmetic changes to it.
>Attached is the updated version of the patch. Barring any objection, I will 
>commit
>it.
Thank you for the patch!
It looks good to me.

Regards,
Shinya Kato

Re: Skipping logical replication transactions on subscriber side

[Amit Kapila]

On Mon, Oct 4, 2021 at 6:01 AM Masahiko Sawada  wrote:
>
> On Fri, Oct 1, 2021 at 5:32 PM Amit Kapila  wrote:
> >
> > On Fri, Oct 1, 2021 at 6:30 AM Masahiko Sawada  
> > wrote:
> > >
> > > On Fri, Oct 1, 2021 at 5:05 AM Peter Eisentraut
> > >  wrote:
> > > >
> > > > On 30.09.21 07:45, Masahiko Sawada wrote:
> > > > > I've attached updated patches that incorporate all comments I got so
> > > > > far. Please review them.
> > > >
> > > > I'm uneasy about the way the xids-to-be-skipped are presented as
> > > > subscriptions options, similar to settings such as "binary".  I see how
> > > > that is convenient, but it's not really the same thing, in how you use
> > > > it, is it?  Even if we share some details internally, I feel that there
> > > > should be a separate syntax somehow.
> > >
> > > Since I was thinking that ALTER SUBSCRIPTION ... SET is used to alter
> > > parameters originally set by CREATE SUBSCRIPTION, in the first several
> > > version patches it added a separate syntax for this feature like ALTER
> > > SUBSCRIPTION ... SET SKIP TRANSACTION xxx. But Amit was concerned
> > > about an additional syntax and consistency with disable_on_error[1]
> > > which is proposed by Mark Diliger[2], so I’ve changed it to a
> > > subscription option.
> > >
> >
> > Yeah, the basic idea is that this is not the only option we will
> > support for taking actions on error/conflict. For example, we might
> > want to disable subscriptions or allow skipping transactions based on
> > XID, LSN, etc.
>
> I guess disabling subscriptions on error/conflict and skipping the
> particular transactions are somewhat different types of functions.
> Disabling subscriptions on error/conflict seems likes a setting
> parameter of subscriptions. The users might want to specify this
> option at creation time.
>

Okay, but they can still specify it by using "On Error" syntax.

> Whereas, skipping the particular transaction
> is a repair function that the user might want to use on the spot in
> case of a failure. I’m concerned a bit that combining these functions
> to one syntax could confuse the users.
>

Fair enough, I was mainly trying to combine the syntax for all actions
that we can take "On Error". We can allow to set them either at Create
Subscription or Alter Subscription time.

I think here the main point is that does this addresses Peter's
concern for this Patch to use a separate syntax? Peter E., can you
please confirm? Do let us know if you have something else going in
your mind?

-- 
With Regards,
Amit Kapila.

Re: [PATCH] document

[Fujii Masao]

On 2021/08/26 1:39, Justin Pryzby wrote:

On Wed, Aug 25, 2021 at 09:50:13AM -0400, Tom Lane wrote:

Fujii Masao  writes:

When I applied the patch to the master, I found that the table entries for
those function were added into the table for aclitem functions in the docs.
I think this is not valid position and needs to be moved to the proper one
(maybe the table for system catalog information functions?).


You have to be very careful these days when applying stale patches to
func.sgml --- there's enough duplicate boilerplate that "patch' can easily
be fooled into dumping an addition into the wrong place.  I doubt that
the submitter meant the doc addition to go there.


I suppose one solution to this is to use git format-patch -U11 or similar, at
least for doc/


Yes. I moved the desriptions of the function into the table for
system catalog information functions, and made the patch by using
git diff -U6. Patch attached. Barring any objection, I'm thinking
to commit it.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 384e6eaa3b..fd6910ddbe 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -2816,22 +2816,24 @@ SCRAM-SHA-256$:&l
 
  
   
conforencoding int4
   
   
-   Source encoding ID
+   Source encoding ID (pg_encoding_to_char()
+   can translate this number to the encoding name)
   
  
 
  
   
contoencoding int4
   
   
-   Destination encoding ID
+   Destination encoding ID (pg_encoding_to_char()
+   can translate this number to the encoding name)
   
  
 
  
   
conproc regproc
@@ -2924,13 +2926,13 @@ SCRAM-SHA-256$:&l
  
   
encoding int4
   
   
Character encoding for this database
-   (pg_encoding_to_char() can translate
+   (pg_encoding_to_char() 
can translate
this number to the encoding name)
   
  
 
  
   
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 78812b2dbe..42dff83e16 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -23325,12 +23325,42 @@ SELECT pg_type_is_visible('myschema.widget'::regtype);
 Returns the SQL name for a data type that is identified by its type
 OID and possibly a type modifier.  Pass NULL for the type modifier if
 no specific modifier is known.

   
 
+  
+   
+
+ pg_char_to_encoding
+
+pg_char_to_encoding ( 
encoding name )
+integer
+   
+   
+Converts the supplied encoding name into an integer representing the
+internal identifier used in some system catalog tables.
+Returns -1 if an unknown encoding name is provided.
+   
+  
+
+  
+   
+
+ pg_encoding_to_char
+
+pg_encoding_to_char ( 
encoding integer )
+name
+   
+   
+Converts the integer used as the internal identifier of an encoding in 
some
+system catalog tables into a human-readable string.
+Returns an empty string if an invalid encoding number is provided.
+   
+  
+
   

 
  pg_get_catalog_foreign_keys
 
 pg_get_catalog_foreign_keys ()