Re: request for database identifier in the startup packet

[David G. Johnston]

On Thursday, May 9, 2024, Dave Cramer  wrote:

> Greetings,
>
> The JDBC driver is currently keeping a per connection cache of types in
> the driver. We are seeing cases where the number of columns is quite high.
> In one case Prevent fetchFieldMetaData() from being run when unnecessary.
> · Issue #3241 · pgjdbc/pgjdbc (github.com)
>  2.6 Million columns.
>
> If we knew that we were connecting to the same database we could use a
> single cache across connections.
>
> I think we would require a server/database identifier in the startup
> message.
>

I feel like pgbouncer ruins this plan.

But maybe you can construct a lookup key from some combination of data
provided by these functions:
https://www.postgresql.org/docs/current/functions-info.html#FUNCTIONS-INFO-SESSION

David J.

Is there an undocumented Syntax Check in Meson?

[David G. Johnston]

$subject

Make has one:
https://www.postgresql.org/docs/current/docguide-build.html#DOCGUIDE-BUILD-SYNTAX-CHECK

This needs updating:
https://www.postgresql.org/docs/current/docguide-build-meson.html

I've been using "ninja html" which isn't shown here.  Also, as a sanity
check, running that command takes my system 1 minute.  Any idea what
percentile that falls into?

David J.

Re: Is there an undocumented Syntax Check in Meson?

[David G. Johnston]

On Thu, May 9, 2024 at 1:16 PM Andres Freund  wrote:

> Hi,
>
> On 2024-05-09 09:23:37 -0700, David G. Johnston wrote:
> > This needs updating:
> > https://www.postgresql.org/docs/current/docguide-build-meson.html
>
> You mean it should have a syntax target? Or that something else is out of
> date?
>
>
v17 looks good, I like the auto-generation.  I failed to notice I was
looking at v16 when searching for a check docs target.


> Also, as a sanity check, running that command takes my system 1 minute.
> Any
> > idea what percentile that falls into?
>
> I think that's on the longer end - what OS/environment is this on? Even on
> ~5yo CPU with turbo boost disabled it's 48s for me.  FWIW, the single-page
> html is a good bit faster, 29s on the same system.
>

Ubuntu 22.04 running in AWS Workspaces Power.

Amazon EC2 t3.xlarge
Intel® Xeon(R) Platinum 8259CL CPU @ 2.50GHz × 4
16GiB Ram

David J.

Re: Is there an undocumented Syntax Check in Meson?

[David G. Johnston]

On Thu, May 9, 2024 at 12:12 PM Dagfinn Ilmari Mannsåker 
wrote:

> "David G. Johnston"  writes:
>
> > I've been using "ninja html" which isn't shown here.
>
> The /devel/ version has a link to the full list of doc targets:
>
>
> https://www.postgresql.org/docs/devel/install-meson.html#TARGETS-MESON-DOCUMENTATION


I knew I learned about the html target from the docs.  Forgot to use the
devel ones this time around.


> Attached is a patch which adds a check-docs target for meson, which
> takes 0.3s on my laptop.
>

Thanks.

David J.

Re: Document NULL

[David G. Johnston]

On Fri, May 3, 2024 at 1:14 AM jian he  wrote:

> On Fri, May 3, 2024 at 2:47 PM Laurenz Albe 
> wrote:
> >
> > On Thu, 2024-05-02 at 08:23 -0700, David G. Johnston wrote:
> > > Version 2 attached.  Still a draft, focused on topic picking and
> overall structure.
> >
> > I'm fine with most of the material (ignoring ellipses and typos), except
> this:
> >
> > +The NOT NULL column constraint is largely syntax sugar for the
> corresponding
> > +column IS NOT NULL check constraint, though there are metadata
> differences
> > +described in create table.
> >
>
> the system does not translate (check constraint column IS NOT NULL)
> to NOT NULL constraint,
> at least in domain.
>
>
I'll change this but I was focusing on the fact you get identical
user-visible behavior with not null and a check(col is not null).  Chain of
thought being we discuss the is not null operator (indirectly) already and
so not null, which is syntax as opposed to an operation/expression, can
leverage that explanation as opposed to getting its own special case.  I'll
consider this some more and maybe mention the catalog dynamics a bit as
well, or at least point to them.


> drop domain connotnull cascade;
> create domain connotnull integer;
> alter domain connotnull add check (value is not null);
> \dD
>

This reminds me, I forgot to add commentary regarding defining a not null
constraint on a domain but the domain type surviving a left join but having
a null value.

David J.

Re: Document NULL

[David G. Johnston]

On Fri, May 3, 2024 at 8:44 AM Tom Lane  wrote:

> "David G. Johnston"  writes:
> > On Fri, May 3, 2024 at 7:10 AM Peter Eisentraut 
> > wrote:
> >> On 02.05.24 17:23, David G. Johnston wrote:
> >>> I chose to add a new sect1 in the user guide (The SQL Language)
> chapter,
> >>> "Data".
>
> >> Please, let's not.
>
> > If a committer wants to state the single place in the documentation to
> put
> > this I'm content to put it there while leaving my reasoning of choices in
> > place for future bike-shedding.  My next options to decide between are
> the
> > appendix or the lead chapter in Data Types. It really doesn't fit inside
> > DDL IMO which is the only other suggestion I've seen (and an uncertain,
> or
> > at least unsubstantiated, one) and a new chapter meets both criteria Tom
> > laid out, so long as this is framed as more than just having to document
> > null values.
>
> I could see going that route if we actually had a chapter's worth of
> material to put into "Data".  But we don't, there's really only one
> not-very-long section.  Robert has justifiably complained about that
> sort of thing elsewhere in the docs, and I don't want to argue with
> him about why it'd be OK here.
>

OK.  I was hopeful that once the Chapter existed the annoyance of it being
short would be solved by making it longer.  If we ever do that, moving this
section under there at that point would be an option.


> Having said that, I reiterate my proposal that we make it a new
>  under DDL, before 5.2 Default Values which is the first
> place in ddl.sgml that assumes you have heard of nulls.


I will go with this and remove the "Data Basics" section I wrote, leaving
it to be just a discussion about null values.  The tutorial is the only
section that really needs unique wording to fit in.  No matter where we
decide to place it otherwise the core content will be the same, with maybe
a different section preface to tie it in.

Putting it in an appendix is similarly throwing
> to the wind any idea that you can read the documentation in order.
>

I think we can keep the entire camel out of the tent while letting it get a
whiff of what is inside.  It would be a summary reference linked to from
the various places that mention null values.
https://en.wikipedia.org/wiki/Camel%27s_nose


> I suppose we could address the nonlinearity gripe with a bunch
> of cross-reference links, in which case maybe something under
> Data Types is the least bad approach.
>
>
Yeah, there is circularity here that is probably impossible to
completely resolve.

David J.

Re: Document NULL

[David G. Johnston]

On Fri, May 3, 2024 at 7:10 AM Peter Eisentraut 
wrote:

> On 02.05.24 17:23, David G. Johnston wrote:
> > Version 2 attached.  Still a draft, focused on topic picking and overall
> > structure.  Examples and links planned plus the usual semantic markup
> stuff.
> >
> > I chose to add a new sect1 in the user guide (The SQL Language) chapter,
> > "Data".
>
> Please, let's not.
>

If a committer wants to state the single place in the documentation to put
this I'm content to put it there while leaving my reasoning of choices in
place for future bike-shedding.  My next options to decide between are the
appendix or the lead chapter in Data Types. It really doesn't fit inside
DDL IMO which is the only other suggestion I've seen (and an uncertain, or
at least unsubstantiated, one) and a new chapter meets both criteria Tom
laid out, so long as this is framed as more than just having to document
null values.


> A stylistic note: "null" is an adjective.  You can talk about a "null
> value" or a value "is null".
>

Will do.

David J.

Document NULL

[David G. Johnston]

Hi,

Over in [1] it was rediscovered that our documentation assumes the reader
is familiar with NULL.  It seems worthwhile to provide both an introduction
to the topic and an overview of how this special value gets handled
throughout the system.

Attached is a very rough draft attempting this, based on my own thoughts
and those expressed by Tom in [1], which largely align with mine.

I'll flesh this out some more once I get support for the goal, content, and
placement.  On that point, NULL is a fundamental part of the SQL language
and so having it be a section in a Chapter titled "SQL Language" seems to
fit well, even if that falls into our tutorial.  Framing this up as
tutorial content won't be that hard, though I've skipped on examples and
such pending feedback.  It really doesn't fit as a top-level chapter under
part II nor really under any of the other chapters there.  The main issue
with the tutorial is the forward references to concepts not yet discussed
but problem points there can be addressed.

I do plan to remove the entity reference and place the content into
query.sgml directly in the final version.  It is just much easier to write
an entire new section in its own file.

David J.

[1] https://www.postgresql.org/message-id/1859814.1714532025%40sss.pgh.pa.us
From a068247e92e620455a925a0ae746adc225ae1339 Mon Sep 17 00:00:00 2001
From: "David G. Johnston" 
Date: Wed, 1 May 2024 07:45:48 -0700
Subject: [PATCH] Document NULL

---
 doc/src/sgml/filelist.sgml |  1 +
 doc/src/sgml/null.sgml | 79 ++
 doc/src/sgml/query.sgml|  2 +
 3 files changed, 82 insertions(+)
 create mode 100644 doc/src/sgml/null.sgml

diff --git a/doc/src/sgml/filelist.sgml b/doc/src/sgml/filelist.sgml
index 38ec362d8f..ac4fd52978 100644
--- a/doc/src/sgml/filelist.sgml
+++ b/doc/src/sgml/filelist.sgml
@@ -10,6 +10,7 @@
 
 
 
+
 
 
 
diff --git a/doc/src/sgml/null.sgml b/doc/src/sgml/null.sgml
new file mode 100644
index 00..5f95b2494e
--- /dev/null
+++ b/doc/src/sgml/null.sgml
@@ -0,0 +1,79 @@
+
+ Handling Unkowns (NULL)
+
+ 
+  NULL
+ 
+
+ 
+  Looking again at our example weather data you will note that we do not know
+  the amount of precipitation Hayward.  We communicated that implicitly by
+  not including the prcp column in the insert.  Explicitly, we can communicate
+  this fact by writing.  [example using null].  When a column is not specified
+  in an insert the default value for that column is recorded and the default
+  default value is NULL.
+ 
+
+ 
+  As a value NULL crops up all throughout the database and interacts with many
+  features.  The main portion of this book will detail the interactions specific
+  features have with NULL but it is helpful to have a reference page where one
+  can get an overview.
+ 
+
+ 
+  First, like all values, NULLs are typed.  But since any value can be unknown
+  NULL is a valid value for all data types.
+ 
+
+ 
+  Second, when speaking generally NULL is assumed to mean unknown.  However,
+  in practice meaning comes from context and so a model design may state that
+  NULL is to be used to represent "not applicable" - i.e., that a value is not
+  even possible.  SQL has only the single value NULL while there are multiple
+  concepts that people have chosen to apply it to.  In any case the behavior
+  of the system when dealing with NULL is the same regardless of the meaning
+  the given to it in the surrounding context.
+ 
+
+ 
+  The cardinal rule, NULL is never equal or unequal to any non-null
+  value; and when asked to be combined with a known value in an operation the
+  result of the operation becomes unknown.  e.g., both 1 = NULL and 1 + NULL
+  result in NULL.  Exceptions to this are documented.  See [chapter] for
+  details on how to test for null.  Specifically, note that concept of
+  distinctness is introduced to allow for true/false equality tests.
+ 
+
+ 
+  Extending from the previous point, function calls are truly a mixed bag.
+  Aggregate functions in particular will usually just ignore NULL inputs
+  instead of forcing the entire aggregate result to NULL.  Function
+  specifications has a "strictness" attribute that, when set to "strict"
+  (a.k.a. "null on null input") will tell the executor to return NULL for any
+  function call having at least one NULL input value, without executing the
+  function.
+ 
+
+ 
+  A WHERE clause that evaluates to NULL for a given row will exclude that row.
+  This was demonstrated in the tutorial query where cities with prcp > 0 were
+  requested and Hayward was not returned due to this and the cardinal rule.
+ 
+
+ 
+  While not yet discussed, it is possible to define validation expressions on
+  tables that ensure only values passing those expressions are inserted.  While
+  this seems like it would behave as a WHERE clause on a table, the choice here
+  when an expression evaulates to NULL is allow the row to be in

Re: EXPLAN redundant options

[David G. Johnston]

On Thu, May 2, 2024 at 6:17 AM jian he  wrote:

> explain (verbose, verbose off, analyze on, analyze off, analyze on)
>
>
I would just update this paragraph to note the last one wins behavior.

"When the option list is surrounded by parentheses, the options can be
written in any order.  However, if options are repeated the last one listed
is used."

I have no desire to introduce breakage here.  The implemented concept is
actually quite common.  The inconsistency with COPY seems like a minor
point.  It would influence my green field choice but not enough for
changing long-standing behavior.

David J.

Re: Document NULL

[David G. Johnston]

On Wed, May 1, 2024 at 9:47 PM Tom Lane  wrote:

> David Rowley  writes:
> > Let's bash it into shape a bit more before going any further on actual
> wording.
>
> FWIW, I want to push back on the idea of making it a tutorial section.
> I too considered that, but in the end I think it's a better idea to
> put it into the "main" docs, for two reasons:
>
>
Version 2 attached.  Still a draft, focused on topic picking and overall
structure.  Examples and links planned plus the usual semantic markup stuff.

I chose to add a new sect1 in the user guide (The SQL Language) chapter,
"Data".  Don't tell Robert.

The "Data Basics" sub-section lets us readily slide this Chapter into the
main flow and here the NULL discussion feels like a natural fit.  In
hindsight, the lack of a Data chapter in a Database manual seems like an
oversight.  One easily made because we assume if you are here you "know"
what data is, but there is still stuff to be discussed, if nothing else to
establish a common understanding between us and our users.

David J.
From 7798121992154edab4768d7eab5a89be04730b2f Mon Sep 17 00:00:00 2001
From: "David G. Johnston" 
Date: Wed, 1 May 2024 07:45:48 -0700
Subject: [PATCH] Document NULL

---
 doc/src/sgml/data.sgml | 169 +
 doc/src/sgml/filelist.sgml |   1 +
 doc/src/sgml/postgres.sgml |   1 +
 3 files changed, 171 insertions(+)
 create mode 100644 doc/src/sgml/data.sgml

diff --git a/doc/src/sgml/data.sgml b/doc/src/sgml/data.sgml
new file mode 100644
index 00..2b09382494
--- /dev/null
+++ b/doc/src/sgml/data.sgml
@@ -0,0 +1,169 @@
+
+ Data
+
+ 
+  This chapter provides definitions for, and an overview of, data.
+  It discusses the basic design of the metadata related to values
+  and then goes on to describe the special value NULL which typically
+  represents unknown
+ 
+
+ 
+  Data Basics
+  
+   All literals, columns, variables, and expression results in PostgreSQL
+   are typed, which are listed in the next chapter.  Literals and columns
+   must only use one of the concrete types while variables can use
+   either a concrete type or a pseudo-type.  Expression results
+   are limited to concrete types and the pseudo-type record described below.
+  
+  
+   The pseudo-types prefixed with any implement polymorphism
+   in PostgreSQL.  Polymorphism allows a single function specification
+   to act on multiple concrete types.  At runtime, the function body
+   associates concrete types to all polymorphic types based upon the
+   conrete argument of its inputs. See ... for more details.
+  
+  
+   The record pseudo-type is also polymorphic in nature but allows
+   the caller of the function to specify the row-like structure of
+   output within the containing query. See ... for more details.
+   The ROW(...) expression (see ...) will also produce a record
+   result comprised of the named columns.
+  
+ 
+
+  
+   Unknown Values (NULL)
+   
+This section first introduces the meaning of NULL and then goes
+on to explain how different parts of the system behave when faced
+with NULL input.
+   
+
+  
+   NULL in Data Models
+   
+Generally NULL is assumed to mean "unknown".  However,
+in practice meaning comes from context and so a model design may state that
+NULL is to be used to represent "not applicable" - i.e., that a value is not
+even possible.  SQL has only the single value NULL while there are multiple
+concepts that people have chosen to apply it to.  In any case the behavior
+of the system when dealing with NULL is the same regardless of the meaning
+the given to it in the surrounding context.
+   
+   
+NULL also takes on a literal meaning of "not found" when produced as the
+result of an outer join.
+   
+  
+
+  
+   NULL Usage
+   
+As NULL is treated as a data value it, like all values, must have
+a data type.  NULL is a valid value for all data types.
+   
+
+   
+A NULL literal is written as unquoted NULL.  Its type is unknown but
+can be cast to any concrete data type.  The [type 'string'] syntax
+however will not work as there is no way to express NULL using single
+quotes and unlike.
+   
+
+   
+The presence of NULL in the system results in three-valued logic.
+In binary logic every outcome is either true or false.  In
+three-valued logic unknown, represented using a NULL value, is
+also an outcome.  Aspects of the system that branch based upon
+whether a condition variable is true or false thus must also
+decide how to behave when then condition is NULL.  The remaining
+sub-sections summarize these decisions.
+   
+  
+
+  
+The Cardinal Rule of NULL
+   
+The cardinal rule, NULL is never equal or unequal to any non-null
+value (or itself).  [NULL = anything yields NULL].
+Checking for NULL has an explicit tes

Re: Proposal for CREATE OR REPLACE EVENT TRIGGER in PostgreSQL

[David G. Johnston]

On Friday, May 3, 2024, Peter Burbery  wrote:

> Dear pgsql-hackers,
>
> One-line Summary:
> Proposal to introduce the CREATE OR REPLACE syntax for EVENT TRIGGER in
> PostgreSQL.
>
> Business Use-case:
> Currently, to modify an EVENT TRIGGER, one must drop and recreate it. This
> proposal aims to introduce a CREATE OR REPLACE syntax for EVENT TRIGGER,
> similar to other database objects in PostgreSQL, to simplify this process
> and improve usability.
>
> Contact Information:
> Peter Burbery
> peter.cullen.burb...@gmail.com
>
> I look forward to your feedback on this proposal.
>

Its doesn’t seem like that big an omission, especially since unlike views
and functions you can’t actually affect dependencies on an event trigger.
But the same can be said of normal triggers and they already have an “or
replace” option.  So, sure, if you come along and offer a patch for this it
seems probable it would be accepted.  I’d just be a bit sad it probably
would take away from time that could spent on more desirable features.

As for your proposal format, it is a quite confusing presentation to send
to an open source project.  Also, your examples are overly complex, and
crammed together, for the purpose and not enough effort is spent actually
trying to demonstrate why this is needed when “drop if exists” already is
present.  A much better flow is to conditionally drop the existing object
and create the new one all in the same transaction.

David J.

Re: Document NULL

[David G. Johnston]

On Saturday, May 11, 2024, Thom Brown  wrote:

>
>  Sat, May 11, 2024, 16:34 David G. Johnston 
> wrote:
>


> My plan is to have a v4 out next week, without or without a review of this
>> draft, but then the subsequent few weeks will probably be a bit quiet.
>>
>
> +   The cardinal rule, a given null value is never
> +   equal or unequal
> +   to any other non-null.
>
> Again, doesn't this imply it tends to be equal to another null by its
> omission?
>
>
I still agree, it’s just a typo now…

…is never equal or unequal to any value.

Though I haven’t settled on a phrasing I really like.  But I’m trying to
avoid a parenthetical.

David J.

Re: Document NULL

[David G. Johnston]

On Fri, May 3, 2024 at 9:00 AM David G. Johnston 
wrote:

> On Fri, May 3, 2024 at 8:44 AM Tom Lane  wrote:
>
>> Having said that, I reiterate my proposal that we make it a new
>>
>  under DDL, before 5.2 Default Values which is the first
>> place in ddl.sgml that assumes you have heard of nulls.
>
>
> I will go with this and remove the "Data Basics" section I wrote, leaving
> it to be just a discussion about null values.  The tutorial is the only
> section that really needs unique wording to fit in.  No matter where we
> decide to place it otherwise the core content will be the same, with maybe
> a different section preface to tie it in.
>
>
v3 Attached.

Probably at the 90% complete mark.  Minimal index entries, not as thorough
a look-about of the existing documentation as I'd like.  Probably some
wording and style choices to tweak.  Figured better to get feedback now
before I go into polish mode.  In particular, tweaking and re-running the
examples.

Yes, I am aware of my improper indentation for programlisting and screen. I
wanted to be able to use the code folding features of my editor.  Those can
be readily un-indented in the final version.

The changes to func.sgml is basically one change repeated something like 20
times with tweaks for true/false.  Plus moving the discussion regarding the
SQL specification into the new null handling section.

It took me doing this to really understand the difference between row
constructors and composite typed values, especially since array
constructors produce array typed values and the constructor is just an
unimportant implementation option while row constructors introduce
meaningfully different behaviors when used.

My plan is to have a v4 out next week, without or without a review of this
draft, but then the subsequent few weeks will probably be a bit quiet.

David J.
From bea784bd683f7e022dbfb3d72832d09fc7754913 Mon Sep 17 00:00:00 2001
From: "David G. Johnston" 
Date: Wed, 1 May 2024 07:45:48 -0700
Subject: [PATCH] Document NULL

---
 doc/src/sgml/ddl.sgml|   2 +
 doc/src/sgml/filelist.sgml   |   1 +
 doc/src/sgml/func.sgml   | 268 ++---
 doc/src/sgml/nullvalues.sgml | 719 +++
 4 files changed, 837 insertions(+), 153 deletions(-)
 create mode 100644 doc/src/sgml/nullvalues.sgml

diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 026bfff70f..68a0fe698d 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -168,6 +168,8 @@ DROP TABLE products;
   
  
 
+ 
+
  
   Default Values
 
diff --git a/doc/src/sgml/filelist.sgml b/doc/src/sgml/filelist.sgml
index 38ec362d8f..882752e88f 100644
--- a/doc/src/sgml/filelist.sgml
+++ b/doc/src/sgml/filelist.sgml
@@ -21,6 +21,7 @@
 
 
 
+
 
 
 
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 17c44bc338..98fba7742c 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -23295,7 +23295,8 @@ MERGE INTO products p
This section describes the SQL-compliant subquery
expressions available in PostgreSQL.
All of the expression forms documented in this section return
-   Boolean (true/false) results.
+   three-valued typed
+   results (true, false, or null).
   
 
   
@@ -23357,19 +23358,17 @@ WHERE EXISTS (SELECT 1 FROM tab2 WHERE col2 = tab1.col2);
 
   
The right-hand side is a parenthesized
-   subquery, which must return exactly one column.  The left-hand expression
+   subquery, which must return exactly one column.  The result of IN
+   is false if the subquery returns no rows, otherwise the left-hand expression
is evaluated and compared to each row of the subquery result.
-   The result of IN is true if any equal subquery row is found.
-   The result is false if no equal row is found (including the
-   case where the subquery returns no rows).
+   The result is true if any equal subquery row is found.
+   The result is false if no equal row is found.
   
 
   
-   Note that if the left-hand expression yields null, or if there are
-   no equal right-hand values and at least one right-hand row yields
-   null, the result of the IN construct will be null, not false.
-   This is in accordance with SQL's normal rules for Boolean combinations
-   of null values.
+   As explained in , it is not possible to see
+   a false result in the presence of both rows and null values since the multiple equality
+   tests are AND'd together.
   
 
   
@@ -23386,21 +23385,18 @@ WHERE EXISTS (SELECT 1 FROM tab2 WHERE col2 = tab1.col2);
as described in .
The right-hand side is a parenthesized
subquery, which must return exactly as many columns as there are
-   expressions in the left-hand row.  The left-hand expressions are
+   expressions in the left-hand row.
+   The result of IN is false if the subquery returns no rows,
+   otherwise the left-hand expressions are
evaluated and compared row-wise to each row of the subque

Re: PERIOD foreign key feature

[David G. Johnston]

On Tue, May 7, 2024 at 7:54 AM Bruce Momjian  wrote:

> In this commit:
>
> commit 34768ee3616
> Author: Peter Eisentraut 
> Date:   Sun Mar 24 07:37:13 2024 +0100
>
> Add temporal FOREIGN KEY contraints
>
> Add PERIOD clause to foreign key constraint definitions.  This
> is
> supported for range and multirange types.  Temporal foreign
> keys check
> for range containment instead of equality.
>
> This feature matches the behavior of the SQL standard temporal
> foreign
> keys, but it works on PostgreSQL's native ranges instead of
> SQL's
> "periods", which don't exist in PostgreSQL (yet).
>
> Reference actions ON {UPDATE,DELETE} {CASCADE,SET NULL,SET
> DEFAULT}
> are not supported yet.
>
> Author: Paul A. Jungwirth 
> Reviewed-by: Peter Eisentraut 
> Reviewed-by: jian he 
> Discussion:
> https://www.postgresql.org/message-id/flat/CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mdhcy4_qq0+noc...@mail.gmail.com
>
> this text was added to create_table.sgml:
>
> In addition, the referenced table must have a primary
> key or unique constraint declared with WITHOUT
> --> OVERLAPS.  Finally, if one side of the foreign key
> --> uses PERIOD, the other side must too.  If the
> refcolumn list is
> omitted, the WITHOUT OVERLAPS part of the
> primary key is treated as if marked with PERIOD.
>
> In the two marked lines, it says "if one side of the foreign key uses
> PERIOD, the other side must too."  However, looking at the example
> queries, it seems like if the foreign side has PERIOD, the primary side
> must have WITHOUT OVERLAPS, not PERIOD.
>
> Does this doc text need correcting?
>
>
The text is factually correct, though a bit hard to parse.

"the other side" refers to the part after "REFERENCES":

FOREIGN KEY ( column_name [, ... ] [, PERIOD column_name ] ) REFERENCES
reftable [ ( refcolumn [, ... ] [, PERIOD column_name ] ) ]

***(shouldn't the second occurrence be [, PERIOD refcolum] ?)

The text is pointing out that since the refcolumn specification is optional
you may very well not see a second PERIOD keyword in the clause.  Instead
it will be inferred from the PK.

Maybe:

Finally, if the foreign key has a PERIOD column_name specification the
corresponding refcolumn, if present, must also be marked PERIOD.  If the
refcolumn clause is omitted, and thus the reftable's primary key constraint
chosen, the primary key must have its final column marked WITHOUT OVERLAPS.

David J.

Re: Things I don't like about \du's "Attributes" column

[David G. Johnston]

On Tue, May 14, 2024 at 9:03 AM Robert Haas  wrote:

> On Tue, Apr 16, 2024 at 3:06 AM Pavel Luzanov 
> wrote:
> > As for the Login column and its values.
> > I'm not sure about using "Can" instead of "yes" to represent true.
> > In other psql commands, boolean values are always shown as yes/no.
> > NULL instead of false might be possible, but I'd rather check if this
> approach
> > has been used elsewhere. I prefer consistency everywhere.
>
> I don't think we can use "Can" to mean "yes". That's going to be
> really confusing.
>

Agreed


> If I see that the connection limit is labelled as (irrelevant)
> I don't know why it's labelled that way and, if it were me, I'd likely
> end up looking at the source code to figure out why it's showing it
> that way.
>

Or we'd document what we've done and users that don't want to go looking at
source code can just read our specification.


> I think we should go back to the v4 version of this patch, minus the
> (ignored) stuff.
>
>
Agreed, I'm past the point of wanting to have this behave more
intelligently rather than a way for people to avoid having to go write a
catalog using query themselves.

David J.

Re: Postgres and --config-file option

[David G. Johnston]

On Wed, May 15, 2024 at 2:49 AM Peter Eisentraut 
wrote:

> On 15.05.24 04:07, Michael Paquier wrote:
> > Not sure that these additions in --help or the docs are necessary.
> > The rest looks OK.
> >
> > -"You must specify the --config-file or -D invocation "
> > +"You must specify the --config-file (or equivalent -c) or -D
> invocation "
> >
> > How about "You must specify the --config-file, -c
> > \"config_file=VALUE\" or -D invocation"?  There is some practice for
> > --opt=VALUE in .po files.
>
> Yeah, some of this is becoming quite unwieldy, if we document and
> mention each spelling variant of each option everywhere.
>

Where else would this need to be added that was missed?  Largely we don't
discuss how to bring a setting into effect - rather there is a single
reference area that discusses how, and everywhere else just assumes you
have read it and goes on to name the setting.  On this grounds the
proper fix here is probably to not put the how into the message:

"You must specify the config_file option, the -D argument, or the PGDATA
environment variable."

And this is only unwieldy because while -D and --config-file both can get
to the same result they are not substitutes for each other.  Namely if the
configuration file is not in the data directory, as is the case on Debian,
the choice to use -D is not going to work.

This isn't an error message, I'm not all that worried if we output a wall
of text in lieu of pointing the user to the reference page.


> Maybe if the original problem is that the option --config-file is not
> explicitly in the --help output, let's add it to the --help output?
>
>
I'm not opposed to this.  Though maybe it is sufficient to do:

--NAME=VALUE (e.g., --config-file='...')

I would do this in addition to removing the explicit how of setting
config_file above.

We also don't mention environment variables in the help but that message
refers to PGDATA...so the complaint and fix if done on that basis seems a
bit selective.

David J.

Re: add function argument names to regex* functions.

[David G. Johnston]

On Wed, May 15, 2024 at 12:07 PM Robert Haas  wrote:

> On Wed, May 15, 2024 at 3:01 PM David G. Johnston
>  wrote:
> > I think this confusion goes to show that replacing N with count doesn't
> work.
> >
> > "replace_at" comes to mind as a better name.
>
> I do not agree with that at all. It shows that a literal
> search-and-replace changing N to count does not work, but it does not
> show that count is a bad name for the concept, and I don't think it
> is. I believe that if I were reading the documentation, count would be
> clearer to me than N, N would probably still be clear enough, and
> replace_at wouldn't be clear at all. I'd expect replace_at to be a
> character position or something, not an occurrence count.
>
>
The function replaces matches, not random characters.  And if you are
reading the documentation I find it implausible that the wording I
suggested would cause one to think in terms of characters instead of
matches.

If I choose not to read the documentation "count" seems like it behaves as
a qualified "g".  I don't want all matches replaced, I want the first
"count" matches only replaced.

"occurrence" probably is the best choice but I agree the spelling issues
are a big negative.

count - how many things there are.  This isn't a count.  I'd rather stick
with N, at least it actually has the desired meaning as a pointer to an
item in a list.

N - The label provides zero context as to what the number you place there
is going to be used for.  Labels ideally do more work than this especially
if someone takes the time to spell them out.  Otherwise why use "pattern"
instead of "p".

David J.

Re: [PATCH] Add --syntax to postgres for SQL syntax checking

[David G. Johnston]

On Wed, May 15, 2024 at 12:18 PM  wrote:

> Tom Lane:
> >> This is really what is missing for the ecosystem. A libpqparser for
> >> tools to use: Formatters, linters, query rewriters, simple syntax
> >> checkers... they are all missing access to postgres' own parser.
> >
> > To get to that, you'd need some kind of agreement on what the syntax
> > tree is.  I doubt our existing implementation would be directly useful
> > to very many tools, and even if it is, do they want to track constant
> > version-to-version changes?
>
> Correct, on top of what the syntax tree currently has, one would
> probably need:
> - comments
> - locations (line number / character) for everything, including those of
> comments
>
>
I'm with the original patch idea at this point.  A utility that simply runs
text through the parser, not parse analysis, and answers the question:
"Were you able to parse this?" has both value and seems like something that
can be patched into core in a couple of hundred lines, not thousands, as
has already been demonstrated.

Sure, other questions are valid and other goals exist in the ecosystem, but
that doesn't diminish this one which is sufficiently justified for my +1 on
the idea.

Now, in my ideal world something like this could be made as an extension so
that it can work on older versions and not have to be maintained by core.
And likely grow more features over time.  Is there anything fundamental
about this that prevents it being implemented in an extension and, if so,
what can we add to core in v18 to alleviate that limitation?

David J.

Re: add function argument names to regex* functions.

[David G. Johnston]

On Wed, May 15, 2024 at 1:19 PM Robert Haas  wrote:

>
> So my point was: to me, N is more self-documenting than replace_at,
> and less self-documenting than count or occurrence.
>
> If your mileage varies on that point, so be it!
>
>
Maybe just "match" instead of "replace_match".

Reading this it strikes me that any of these parameter names can and
probably should be read as having "replace" in front of them:

replace N
replace count
replace occurrence
replace match

Saying replace becomes redundant:
replace replace at
replace replace match

David J.

Re: [PATCH] Add --syntax to postgres for SQL syntax checking

[David G. Johnston]

On Wed, May 15, 2024 at 12:35 PM Josef Šimánek 
wrote:

> st 15. 5. 2024 v 21:33 odesílatel David G. Johnston
>  napsal:
>
> > Now, in my ideal world something like this could be made as an extension
> so that it can work on older versions and not have to be maintained by
> core.  And likely grow more features over time.  Is there anything
> fundamental about this that prevents it being implemented in an extension
> and, if so, what can we add to core in v18 to alleviate that limitation?
>
> Like extension providing additional binary? Or what kind of extension
> do you mean? One of the original ideas was to be able to do so (parse
> query) without running postgres itself. Could extension be useful
> without running postgres backend?
>
>
Pushing beyond my experience level here...but yes a separately installed
binary (extension is being used conceptually here, this doesn't involve
"create extension") that can inspect pg_config to find out where
backend/postmaster library objects are installed and link to them.

David J.

Re: add function argument names to regex* functions.

[David G. Johnston]

On Wed, May 15, 2024 at 12:52 PM Robert Haas  wrote:

> On Wed, May 15, 2024 at 3:25 PM David G. Johnston
>  wrote:
> > The function replaces matches, not random characters.  And if you are
> reading the documentation I find it implausible that the wording I
> suggested would cause one to think in terms of characters instead of
> matches.
>
> I mean I just told you what my reaction to it was. If you find that
> reaction "implausible" then I guess you think I was lying when I said
> that?
>
>
You just broke my brain when you say that you read:

By default, only the first match of the pattern is replaced.  If replace_at
is specified and greater than zero, then the first "replace_at - 1" matches
are skipped before making a single replacement (i.e., the g flag is ignored
when replace_at is specified.)

And then say:

I'd expect replace_at to be a character position or something, not an
occurrence count.

I guess it isn't a claim you are lying, rather I simply don't follow your
mental model of all this and in my mental model behind the proposal I don't
believe the typical reader will become confused on that point.  I guess
that means I don't find you to be the typical reader, at least so far as
this specific topic goes.  But hey, maybe I'm the one in the minority.  In
either case we disagree and that was my main point.

David J.

Re: add function argument names to regex* functions.

[David G. Johnston]

On Wed, May 15, 2024 at 12:07 PM Robert Haas  wrote:

> On Wed, May 15, 2024 at 3:01 PM David G. Johnston
>  wrote:
> > I think this confusion goes to show that replacing N with count doesn't
> work.
> >
> > "replace_at" comes to mind as a better name.
> I'd expect replace_at to be a
> character position or something, not an occurrence count.
>
>
I'll amend the name to:  "replace_match"

I do now see that since the immediately preceding parameter, "start", deals
with characters instead of matches that making it clear this parameter
deals in matches in the name work.  The singular 'match' has all the same
benefits as 'at' plus this point of clarity.


David J.

Re: add function argument names to regex* functions.

[David G. Johnston]

On Wed, May 15, 2024 at 11:46 AM Robert Haas  wrote:

> On Thu, Apr 4, 2024 at 9:55 AM jian he 
> wrote:
> > in the regexp_replace explanation section.
> > changing "N" to lower-case would be misleading for regexp_replace?
> > so I choose "count".
>
> I don't see why that would be confusing for regexp_replace
> specifically, but I think N => count is a reasonable change to make.
> However, I don't think this quite works:
>
> + then the count'th match of the pattern
>
> An English speaker is more likely to understand what is meant by
> "N'th" than what is meant by "count'th". Even if they can guess, it's
> kinda strange-looking. I think it needs to be rephrased somehow, but
> I'm not sure exactly how.
>
>
I think this confusion goes to show that replacing N with count doesn't
work.

"replace_at" comes to mind as a better name.

By default, only the first match of the pattern is replaced.  If replace_at
is specified and greater than zero, then the first "replace_at - 1" matches
are skipped before making a single replacement (i.e., the g flag is ignored
when replace_at is specified.)

David J.

Re: [PATCH] Add --syntax to postgres for SQL syntax checking

[David G. Johnston]

On Wed, May 15, 2024 at 1:00 PM Robert Haas  wrote:

> On Wed, May 15, 2024 at 3:28 PM Tom Lane  wrote:
> > Sorry: "make sense" was a poorly chosen phrase there.  What I was
> > doubting, and continue to doubt, is that 100% checking of what
> > you can check without catalog access and 0% checking of the rest
> > is a behavior that end users will find especially useful.
>
> You might be right, but my guess is that you're wrong. Syntax
> highlighting is very popular, and seems like a more sophisticated
> version of that same concept.


The proposed seems distinctly less sophisticated though would be a starting
point.

I think the documentation for --syntax check would read something like this:

postgres --syntax-check={filename | -}

Performs a pass over the lexical structure of the script supplied in
filename or, if - is specified, standard input, then exits.  The exit code
is zero if no errors were found, otherwise it is 1, and the errors, at full
verbosity, are printed to standard error.  This does not involve reading
the configuration file and, by extension, will not detect errors that
require knowledge of a database schema, including the system catalogs, to
manifest.  There will be no false positives, but due to the prior rule,
false negatives must be factored into its usage.  Thus this option is most
useful as an initial triage point, quickly rejecting SQL code without
requiring a running PostgreSQL service.

Note: This is exposed as a convenient way to get access to the outcome of
performing a raw parse within the specific version of the postgres binary
being executed.  The specific implementation of that parse is still
non-public.  Likewise, PostgreSQL doesn't itself have a use for a raw parse
output beyond sending it to post-parse analysis.  All of the catalog
required checks, and potentially some that don't obviously need the
catalogs, happen in this post-parse step; which the syntax checking API
does not expose.  In short, the API here doesn't include any guarantees
regarding the specific errors one should expect to see output, only the no
false positive test result of performing the first stage raw parse.

David J.

If in core I would still want to expose this as say a contrib module binary
instead of hacking it into postgres.  It would be our first server program
entry there.

Re: [PATCH] Add --syntax to postgres for SQL syntax checking

[David G. Johnston]

On Wed, May 15, 2024 at 6:35 PM David G. Johnston <
david.g.johns...@gmail.com> wrote:

>
> If in core I would still want to expose this as say a contrib module
> binary instead of hacking it into postgres.  It would be our first server
> program entry there.
>
>
Sorry for self-reply but:

Maybe name it "pg_script_check" with a, for now mandatory, "--syntax-only"
option that enables this raw parse mode.  Leaving room for executing this
in an environment where there is, or it can launch, a running instance that
then performs post-parse analysis as well.

David J.

Re: First draft of PG 17 release notes

[David G. Johnston]

On Wednesday, May 15, 2024, jian he  wrote:

> On Thu, May 9, 2024 at 12:04 PM Bruce Momjian  wrote:
> >
> > I have committed the first draft of the PG 17 release notes;  you can
> > see the results here:
> >
> > https://momjian.us/pgsql_docs/release-17.html
> >
>
> in section: E.1.2. Migration to Version 17
>
> >> Rename I/O block read/write timing statistics columns of
> pg_stat_statement (Nazir Bilal Yavuz)
> >> This renames "blk_read_time" to "shared_blk_read_time", and
> "blk_write_time" to "shared_blk_write_time".
>
> we only mentioned, pg_stat_statements some columns name changed
> in "E.1.2. Migration to Version 17"
> but if you look at the release note pg_stat_statements section,
> we added a bunch of columns, which are far more incompatibile than
> just colunm name changes.
>
> not sure we need add these in section "E.1.2. Migration to Version 17"
>
>
New columns are not a migration issue since nothing being migrated forward
ever referenced them.  Its the ones that existing code knows about that
we’ve removed (including renames) that matter from a migration perspective.

David J.

Re: commitfest.postgresql.org is no longer fit for purpose

[David G. Johnston]

On Thu, May 16, 2024 at 1:46 PM Melanie Plageman 
wrote:

>
> I should probably simply
> withdraw and re-register them. My justification was that I'll lose
> them if I don't keep them in the commitfest app. But, I could just,
> you know, save them somewhere myself instead of polluting the
> commitfest app with them. I don't know if others are in this
> situation. Anyway, I'm definitely currently guilty of parking.
>
>
I use a personal JIRA to track the stuff that I hope makes it into the
codebase, as well as just starring the corresponding emails in the
short-term.  Every patch ever submitted sits in the mailing list archive so
I have no real need to preserve git branches with my submitted work on
them.  At lot of my work comes down to lucky timing so I'm mostly content
with just pinging my draft patches on the email thread once in a while
hoping there is interest and time from someone.  For stuff that I would be
OK committing as submitted I'll add it to the commitfest and wait for
someone to either agree or point out where I can improve things.

Adding both these kinds to WIP appeals to me, particularly with something
akin to a "Collaboration Wanted" category in addition to "Needs Review" for
when I think it is ready, and "Waiting on Author" for stuff that has
pending feedback to resolve - or the author isn't currently fishing for
reviewer time for whatever reason.  Ideally there would be no rejections,
only constructive feedback that convinces the author that, whether for now
or forever, the proposed patch should be withdrawn pending some change in
circumstances that suggests the world is ready for it.

David J.

Re: CREATE TABLE creates a composite type corresponding to the table row, which is and is not there

[David G. Johnston]

On Wed, May 15, 2024 at 8:46 AM Robert Haas  wrote:

> On Thu, Apr 4, 2024 at 12:41 AM Erik Wienhold  wrote:
> > Thanks, fixed in v4.  Looks like American English prefers that comma and
> > it's also more common in our docs.
>
> Reviewing this patch:
>
> -  Creates a typed table, which takes its
> -  structure from the specified composite type (name optionally
> -  schema-qualified).  A typed table is tied to its type; for
> -  example the table will be dropped if the type is dropped
> -  (with DROP TYPE ... CASCADE).
> +  Creates a typed table, which takes its
> structure
> +  from an existing (name optionally schema-qualified) stand-alone
> composite
> +  type (i.e., created using ) though
> it
> +  still produces a new composite type as well.  The table will have
> +  a dependency on the referenced type such that cascaded alter and
> drop
> +  actions on the type will propagate to the table.
>
> It would be better if this diff didn't reflow the unchanged portions
> of the paragraph.
>
> I agree that it's a good idea to mention that the table must have been
> created using CREATE TYPE .. AS here, but I disagree with the rest of
> the rewording in this hunk. I think we could just add "creating using
> CREATE TYPE" to the end of the first sentence, with an xref, and leave
> the rest as it is.



> I don't see a reason to mention that the typed
> table also spawns a rowtype; that's just standard CREATE TABLE
> behavior and not really relevant here.


I figured it wouldn't be immediately obvious that the system would create a
second type with identical structure.  Of course, in order for SELECT tbl
FROM tbl; to work it must indeed do so.  I'm not married to pointing out
this dynamic explicitly though.


> And I don't understand what the
> rest of the rewording does for us.
>

It calls out the explicit behavior that the table's columns can change due
to actions on the underlying type.  Mentioning this unique behavior seems
worth a sentence.


>   
> -  When a typed table is created, then the data types of the
> -  columns are determined by the underlying composite type and are
> -  not specified by the CREATE TABLE command.
> +  A typed table always has the same column names and data types as the
> +  type it is derived from, and you cannot specify additional columns.
>But the CREATE TABLE command can add defaults
> -  and constraints to the table and can specify storage parameters.
> +  and constraints to the table, as well as specify storage parameters.
>   
>
> I don't see how this is better.
>

I'll agree this is more of a stylistic change, but mainly because the talk
about data types reasonably implies the other items the patch explicitly
mentions - names and additional columns.


> - errmsg("type %s is not a composite type",
> + errmsg("type %s is not a stand-alone composite type",
>
> I agree with Peter's complaint that people aren't going to understand
> what a stand-alone composite type means when they see the revised
> error message; to really help people, we're going to need to do better
> than this, I think.
>
>
We have a glossary.

That said, leave the wording as-is and add a conditional hint: The
composite type must not also be a table.

David J.

Re: commitfest.postgresql.org is no longer fit for purpose

[David G. Johnston]

On Thu, May 16, 2024 at 11:30 AM Robert Haas  wrote:

> Hi,
>
> The original intent of CommitFests, and of commitfest.postgresql.org
> by extension, was to provide a place where patches could be registered
> to indicate that they needed to be reviewed, thus enabling patch
> authors and patch reviewers to find each other in a reasonably
> efficient way. I don't think it's working any more. I spent a good
> deal of time going through the CommitFest this week, and I didn't get
> through a very large percentage of it, and what I found is that the
> status of the patches registered there is often much messier than can
> be captured by a simple "Needs Review" or "Waiting on Author," and the
> number of patches that are actually in need of review is not all that
> large. For example, there are:
>
> - patches parked there by a committer who will almost certainly do
> something about them after we branch
> - patches parked there by a committer who probably won't do something
> about them after we branch, but maybe they will, or maybe somebody
> else will, and anyway this way we at least run CI
> - patches parked there by a committer who may well do something about
> them before we even branch, because they're not actually subject to
> the feature freeze
>

If a committer has a patch in the CF that is going to be committed in the
future unless there is outside action those should be put under a "Pending
Commit" status.

- patches that we've said we don't want but the author thinks we do
> (sometimes i agree with the author, sometimes not)
> - patches that have long-unresolved difficulties which the author
> either doesn't know how to solve or is in no hurry to solve
> - patches that have already been reviewed by multiple people, often
> including several committers, and which have been updated multiple
> times, but for one reason or another, not committed
>

Use the same software but a different endpoint - Collaboration.  Or even
the same endpoint just add an always open slot named "Work In Process"
(WIP).  If items can be moved from there to another open or future
commitfest slot so much the better.

- patches that actually do need to be reviewed
>

If we can distinguish between needs to be reviewed by a committer
(commitfest dated slots - bimonthlies) and reviewed by someone other than
the author (work in process slot) that should help everyone.  Of course,
anyone is welcome to review a patch that has been marked ready to commit.
I suppose "ready to commit" already sorta does this without the need for
WIP but a quick sanity check would be that ready to commit shouldn't (not
mustn't) be seen in WIP and needs review shouldn't be seen in the
bimonthlies.  A needs review in WIP means that the patch has been seen by a
committer and sent back for more work but that the scope and intent are
such that it will make it into the upcoming major release.  Or is something
like a bug fix that just goes right into the bimonthly instead of starting
out as a WIP item.


> I think there are a couple of things that have led to this state of
> affairs. First, we got tired of making people mad by booting their
> stuff out of the CommitFest, so we mostly just stopped doing it, even
> if it had 0% chance of being committed this CommitFest, and really
> even if it had a 0% chance of being committed ever.


Those likely never get out of the new WIP slot discussed above.  Your patch
tracker basically.  And there should be less angst in moving something in
the bimonthly into WIP rather than dropping it outright.  There is
discussion to be had regarding WIP/patch tracking should we go this
direction but even if it is just movIng clutter from one room to another
there seems like a clear benefit and need to tighten up what it means to be
in the bimonthly slot - to have qualifications laid out for a patch to earn
its way there, either by effort (authored and reviewed) or need (i.e., bug
fixes), or, realistically, being authored by a committer and being mostly
trivial in nature.


> Second, we added
> CI, which means that there is positive value to registering the patch
> in the CommitFest even if committing it is not in the cards.


The new slot retains this benefit.

And those
> things together have created a third problem, which is that the list
> is now so long and so messy that even the CommitFest managers probably
> don't manage to go through the whole thing thoroughly in a month.
>

The new slot wouldn't be subject to this.

We'll still have a problem with too many WIP patches and not enough ability
or desire to resolve them.  But setting a higher bar to get onto the
bimonthly slot while still providing a place for collaboration is a step
forward that configuring technology can help with.  As for WIP, maybe
adding thumbs-up and thumbs-down support tracking widgets will help draw
attention to more desired things.

David J.

Re: Postgres and --config-file option

[David G. Johnston]

On Thu, May 16, 2024 at 4:11 PM Michael Paquier  wrote:

> On Thu, May 16, 2024 at 11:57:10AM +0300, Aleksander Alekseev wrote:
> > I propose my original v1 patch for correcting the --help output of
> > 'postgres' too. I agree with the above comments that corresponding
> > changes in v4 became somewhat unwieldy.
>
> Thanks for compiling the rest.
>
> -printf(_("  --NAME=VALUE   set run-time parameter\n"));
> +printf(_("  --NAME=VALUE   set run-time parameter, a shorter form
> of -c\n"));
>
> This part with cross-references in the output is still meh to me, for
> same reason as for the doc changes I've argued to discard upthread.
>

I'm fine with leaving these alone.  If we did change this I'd want to
change both, not just --NAME.


>  write_stderr("%s does not know where to find the server
> configuration file.\n"
> - "You must specify the --config-file or -D invocation
> "
> + "You must specify the --config-file (or equivalent
> -c) or -D invocation "
>
>
I would rather just do this:

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 3fb6803998..f827086489 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1828,8 +1828,8 @@ SelectConfigFiles(const char *userDoption, const char
*progname)
else
{
write_stderr("%s does not know where to find the server
configuration file.\n"
-"You must specify the
--config-file or -D invocation "
-"option or set the PGDATA
environment variable.\n",
+"You must specify either the
config_file run-time parameter or "
+"provide the database directory\n",
 progname);
return false;
}

Both "run-time parameter" and "database directory" are words present in the
help and the user can find the correct argument if that is the option they
want to use.  The removal of the mention of the PGDATA environment variable
doesn't seem to be a great loss here.  This error message doesn't seem to
be the correct place to teach the user about all of their options so long
as they choose to read the documentation they learn about them there; and
we need not prescribe the specific means by which they supply either of
those pieces of information - which is the norm.  If someone simply runs
"postgres" at the command line this message and --help gives them
sufficient information to proceed.

David J.

Re: confusing `case when` column name

[David G. Johnston]

On Tuesday, March 12, 2024, adjk...@126.com  wrote:

>
> Nee we change the title of the case-when output column?
>
>
Choosing either a or b as the label seems wrong and probably worth changing
to something that has no meaning and encourages the application of a column
alias.

David J.

Re: REVOKE FROM warning on grantor

[David G. Johnston]

On Thursday, March 14, 2024, Étienne BERSAC 
wrote:

>
> However, I'd prefer if Postgres fails properly. Because the GRANT is
> actually not revoked. This prevent ldap2pg to report an issue in
> handling privileges on such roles.
>
> What do you think of make this warning an error ?
>
>
The choice of warning is made because after the command ends the grantmin
question does not exist.  The revoke was a no-op and the final state is as
the user intended.  Historically doing this didn’t give any message at all
which was confusing so we added a warning so the semantics of not failing
were preserved but there was some indication that something was amiss.  I
don’t have a compelling argument to,change the long-standing behavior.
Client code can and probably should look for a show errors reported by the
backend.  It is indeed possibly to treat this warning more serverly than
the server chooses to.

David J.

Re: documentation structure

[David G. Johnston]

On Wed, Mar 20, 2024 at 9:43 AM Robert Haas  wrote:

> On Tue, Mar 19, 2024 at 5:39 PM Andrew Dunstan 
> wrote:
> > +many for improving the index.
>
> Here's a series of four patches.


I reviewed the most recent set of 5 patches.


> Taken together, they cut down the
> number of numbered chapters from 76 to 68. I think we could easily
> save that much again if I wrote a few more patches along similar
> lines, but I'm posting these first to see what people think.
>
> 0001 removes the "Installation from Binaries" chapter. The whole thing
> is four sentences. I moved the most important information into the
> "Installation from Source Code" chapter and retitled it
> "Installation".
>

Makes sense


> 0002 removes the "Monitoring Disk Usage" chapter by folding it into
> the immediately-preceding "Monitoring Database Activity" chapter. I
> kind of feel like the "Monitoring Disk Usage" chapter might be in need
> of a bigger rewrite or just outright removal, but there's surely not
> enough content here to justify making it a top-level chapter.
>

Just going to note that the section on the cumulative statistics views
being a single page is still a strongly bothersome issue here.  Though the
quick fix actually requires upgrading the section to chapter status...

Maybe we can stub out that section in the "Monitoring Database Activity"
chapter and move that entire section after "System Views" in the Internals
part?

I agree with subordinating Monitoring Disk Usage.


> 0003 merges all of the "Internals" chapters whose names are the names
> of built-in index access methods (Btree, Gin, etc.) into a single
> chapter called "Built-In Index Access Methods". All of these chapters
> have a very similar structure and none of them are very long, so it
> makes a lot of sense, at least in my mind, to consolidate them into
> one.
>

One of the more impactful and wanted improvements, IMO.


> 0004 merges the "Generic WAL Records" and "Custom WAL Resource
> Managers" chapter together, creating a new chapter called "Write Ahead
> Logging for Extensions".
>
>
The positioning of this and the preceding Built-in Index Access Methods
chapter seem like they should be switched.

If this sticks we should add an introductory paragraph for the chapter.

and I've added a new 0005 that does essentially the same
> thing for the PL chapters.
>

The following page needs to be reworded to take the new structure into
account:

https://www.postgresql.org/docs/current/xfunc-pl.html

Not having pl/pgsql appear on the main ToC seems like a loss but the others
make sense and a special exception for it probably isn't warranted.

Maybe "pl/pgsql and Other Procedural Languages" as the title?

David J.

Re: documentation structure

[David G. Johnston]

On Thu, Mar 21, 2024 at 11:30 AM Robert Haas  wrote:

>
> My second thought is that the stuff from "VII. Internals" that I
> categorized as reference material should move into section "VI.
> Reference". I think we should also consider moving appendix F,
> "Additional Supplied Modules and Extensions," and appendix G,
> "Additional Supplied Programs" to the reference section.
>
>
For "VI. Reference" I propose the following Chapters:

SQL Commands
PL/pgSQL
Cumulative Statistics Views
System Views
System Catalogs
Client Applications
Server Applications
Modules and Extensions

-- Remove Appendix G (Programs) altogether and just note for the two that
are listed that they are in contrib as opposed to core.

-- The PostgreSQL qualifier doesn't seem helpful and once you add the
additional chapters its unusual presence stands out even more.

-- PL/pgSQL gets its own reference chapter since we wrote it.  Stuff like
Perl and Python have entire books that the user can consult as reference
material for those languages.

David J.

Re: REVOKE FROM warning on grantor

[David G. Johnston]

On Sat, Mar 16, 2024 at 1:00 PM Étienne BERSAC 
wrote:

>
> > The choice of warning is made because after the command ends the
> > grantmin question does not exist.  The revoke was a no-op and the
> > final state is as the user intended.
>
>
> Sorry, can you explain me what's the grantmin question is ?
>
>
That should have read:  the granted permission does not exist

David J.

Re: documentation structure

[David G. Johnston]

On Fri, Mar 22, 2024 at 7:10 AM Robert Haas  wrote:

>
> That's actually what we had in chapter
> 18, "Installation from Source Code on Windows", since removed. But for
> some reason we decided that on non-Windows platforms, it needed a
> whole new chapter rather than an extra sentence in the existing one. I
> think that's massively overkill.
>
>
I agree with the premise that we should have a single chapter, in the main
documentation flow, named "Installation".  It should cover the
architectural overview and point people to where they can find the stuff
they need to install PostgreSQL in the various ways available to them.  I
agree with moving the source installation material to the appendix.  None
of the sections under Installation would then actually detail how to
install the software since that isn't something the project itself handles
but has delegated to packagers for the vast majority of cases and the
source install details are in the appendix for the one "supported"
mechanism that most people do not use.

David J.

Re: documentation structure

[David G. Johnston]

On Fri, Mar 22, 2024 at 11:19 AM Robert Haas  wrote:

> On Fri, Mar 22, 2024 at 1:35 PM Bruce Momjian  wrote:
>
> But that all seems like a separate question from why we have the
> statistic collector views in a completely different part of the
> documentation from the rest of the system views. My guess is that it's
> just kind of a historical accident, but maybe there was some other
> logic to it.
>
>
The details under-pinning the cumulative statistics subsystem are
definitely large enough to warrant their own subsection. And it isn't like
placing them into the monitoring chapter is wrong and aside from a couple
of views those under System Views don't fit into what we've defined as
monitoring.  I don't have any desire to lump them under the generic system
views; which itself could probably use a level of categorization since the
nature of pg_locks and pg_cursors is decidedly different than pg_indexes
and pg_config.  This all becomes more appealing to work on once we solve
the problem of all sect2 entries being placed on a single page.

I struggled for a long while where I'd always look for pg_stat_activity
under system views instead of monitoring.  Amending my prior suggestion in
light of this I would suggest we move the Cumulative Statistics Views into
Reference but as its own Chapter, not part of System Views, and change its
name to "Monitoring Views" (going more generalized here feels like a win to
me). I'd move pg_locks, pg_cursors, pg_backend_memory_contexts,
pg_prepared_*, pg_shmem_allocations, and pg_replication_*.  Those all have
the same general monitoring nature to them compared to the others that
basically provide details regarding schema and static or session
configuration.

The original server admin monitoring section can go into detail regarding
Cumulative Statistics versus other kinds of monitoring.  We can use
section ordering to fulfill logical grouping desires until we are able to
make section3 entries appear on their own pages.

David J.

Re: documentation structure

[David G. Johnston]

On Fri, Mar 22, 2024, 09:32 Robert Haas  wrote:

>
>
> I notice that you say that the "Installation" section should "cover
> the architectural overview and point people to where they can find the
> stuff they need to install PostgreSQL in the various ways available to
> them" so maybe you're not imagining a four-sentence chapter, either.
>

Fair point but I posit that new users are looking for a chapter named
Installation in the documentation.  At least the ones willing to read
documentation.  Having two of them isn't needed but having zero doesn't
make sense either.

The current proposal does that so I'm ok as-is but it can be further
improved by moving source install talk elsewhere and having the
installation chapter redirect the reader there for details.  I'm not
concerned with how long or short the resultant installation chapter is.

David J.

>
>

Re: Reports on obsolete Postgres versions

[David G. Johnston]

On Tue, Apr 2, 2024 at 1:47 PM Bruce Momjian  wrote:

> On Tue, Apr  2, 2024 at 11:34:46AM +0200, Magnus Hagander wrote:
>
> Okay, I changed "superseded" to "old", and changed "latest" to
> "current", patch attached.
>
>
I took a pass at this and found a few items of note.  Changes on top of
Bruce's patch.

diff --git a/templates/support/versioning.html
b/templates/support/versioning.html
index 0ed79f4f..d4762967 100644
--- a/templates/support/versioning.html
+++ b/templates/support/versioning.html
@@ -21,9 +21,9 @@ a release available outside of the minor release roadmap.

 
 The PostgreSQL Global Development Group supports a major version for 5
years
-after its initial release. After its five year anniversary, a major
version will
-have one last minor release containing any fixes and will be considered
-end-of-life (EOL) and no longer supported.
+after its initial release. After its fifth anniversary, a major version
will
+have one last minor release and will be considered
+end-of-life (EOL), meaning no new bug fixes will be written for it.
 

# "fifth anniversary "seems more correct than "five year anniversary".
# The fact it gets a minor release implies it receives fixes.
# I've always had an issue with our use of the phrasing "no longer
supported".  It seems vague when practically it just means we stop writing
patches for it.  Whether individual community members refrain from
answering questions or helping people on these older releases is not a
project decision but a personal one.  Also, since we already say it is
supported for 5 years it seems a bit redundant to then also say "after 5
years it is unsupported".


 Version Numbering
@@ -45,11 +45,12 @@ number, e.g. 9.5.3 to 9.5.4.
 Upgrading

 
-Major versions usually change the internal format of the system tables.
-These changes are often complex, so we do not maintain backward
-compatibility of all stored data.  A dump/reload of the database or use of
the
-pg_upgrade module is required
-for major upgrades. We also recommend reading the
+Major versions need the data directory to be initialized so that the
system tables
+specific to that version can be created.  There are two options to then
+migrate the user data from the old directory to the new one: a dump/reload
+of the database or using the
+pg_upgrade module.
+We also recommend reading the
 upgrading section of the major
 version you are planning to upgrade to. You can upgrade from one major
version
 to another without upgrading to intervening versions, but we recommend
reading
@@ -58,14 +59,15 @@ versions prior to doing so.
 

# This still talked about "stored data" when really that isn't the main
concern and if it was pg_upgrade wouldn't work as an option.
# The choice to say "data directory" here seems reasonable if arguable.
But it implies the location of user data and we state it also contains
version-specific system tables.  It can go unsaid that they are
version-specific because the collection as a whole and the layout of
individual tables can and almost certainly will change between versions.

 
-Minor release upgrades do not require a dump and restore;  you simply stop
+Minor releases upgrades do not impact the data directory, only the
binaries.
+You simply stop
 the database server, install the updated binaries, and restart the server.
-Such upgrades might require manual changes to complete so always read
+However, some upgrades might require manual changes to complete so always
read
 the release notes first.
 

# The fact minor releases don't require dump/reload doesn't directly
preclude them from needing pg_upgrade and writing "Such upgrades" seems
like it could lead someone to think that.
# Data Directory seems generic enough to be understood here and since I
mention it in the Major Version as to why data migration is needed,
mentioning here
# as something not directly altered and thus precluding the data migration
has a nice symmetry.  The potential need for manual changes becomes clearer
as well.


 
-Minor releases only fix frequently-encountered bugs, security issues, and data corruption
 problems, so such upgrades are very low risk.  The community
 considers performing minor upgrades to be less risky than continuing to

# Reality mostly boils down to trusting our judgement when it comes to bugs
as each one is evaluated individually.  We do not promise to leave simply
buggy behavior alone in minor releases which is the only policy that would
nearly eliminate upgrade risk.  We back-patch 5 year old bugs quite often
which by definition are not frequently encountered.  I cannot think of a
good adjective to put there nor does one seem necessary even if I agree it
reads a bit odd otherwise.  I think that has more to do with this being
just the wrong structure to get our point across.  Can we pick out some
statistics from our long history of publishing minor releases to present an
objective reality to the reader to convince them to trust our track-record
in this matter?

David J.

Re: CREATE TABLE creates a composite type corresponding to the table row, which is and is not there

[David G. Johnston]

On Thu, Mar 28, 2024 at 8:02 PM Erik Wienhold  wrote:

> Thanks, that sounds better.  I incorporated that with some minor edits
> in the attached v3.
>

Looks good.

You added my missing ( but dropped the comma after "i.e."

diff --git a/doc/src/sgml/ref/create_table.sgml
b/doc/src/sgml/ref/create_table.sgml
index dc69a3f5dc..b2e9e97b93 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -251,7 +251,7 @@ WITH ( MODULUS numeric_literal, REM
  
   Creates a typed table, which takes its
structure
   from an existing (name optionally schema-qualified) stand-alone
composite
-  type (i.e. created using ) though it
+  type (i.e., created using ) though it
   still produces a new composite type as well.  The table will have
   a dependency on the referenced type such that cascaded alter and drop
   actions on the type will propagate to the table.

David J.

Re: documentation structure

[David G. Johnston]

On Fri, Apr 5, 2024 at 9:18 AM Robert Haas  wrote:

> On Fri, Apr 5, 2024 at 12:15 PM David G. Johnston
>  wrote:
> > Here is a link to my attempt at this a couple of years ago.  It
> basically "abuses" refentry.
> >
> >
> https://www.postgresql.org/message-id/CAKFQuwaVm%3D6d_sw9Wrp4cdSm5_k%3D8ZVx0--v2v4BH4KnJtqXqg%40mail.gmail.com
> >
> > I never did dive into the man page or PDF dynamics of this particular
> change but it seemed to solve HTML pagination without negative consequences
> and with minimal risk of unintended consequences since only the markup on
> the pages we want to alter is changed, not global configuration.
>
> Hmm, but it seems like that might have generated some man page entries
> that we don't want?
>

If so (didn't check) maybe just remove them in post?

David J.

Re: documentation structure

[David G. Johnston]

On Fri, Apr 5, 2024 at 9:01 AM Robert Haas  wrote:

>
> > The rendering can be adjusted to some degree, but then we also need to
> > make sure any new chunking makes sense in other chapters.  (And it might
> > also change a bunch of externally known HTML links.)
>
> I looked into this and I'm unclear how much customization is possible.
>
>
Here is a link to my attempt at this a couple of years ago.  It basically
"abuses" refentry.

https://www.postgresql.org/message-id/CAKFQuwaVm%3D6d_sw9Wrp4cdSm5_k%3D8ZVx0--v2v4BH4KnJtqXqg%40mail.gmail.com

I never did dive into the man page or PDF dynamics of this
particular change but it seemed to solve HTML pagination without negative
consequences and with minimal risk of unintended consequences since only
the markup on the pages we want to alter is changed, not global
configuration.

David J.

Re: CREATE TABLE creates a composite type corresponding to the table row, which is and is not there

[David G. Johnston]

On Thu, Mar 7, 2024 at 9:29 PM Erik Wienhold  wrote:

> I wrote:
> > The attached v2 is a simpler patch that instead modifies the existing
> > error message.
>
> Forgot to attach v2.
>
>
For consideration for the doc portion.  The existing wording is too
imprecise for my liking and just tacking on "expects...create type" is
jarring.

"""
Creates a typed table, which takes it structure from an existing (name
optionally schema-qualified) stand-alone composite type i.e., one created
using CREATE TYPE) though it still produces a new composite type as well.
The table will have a dependency to the referenced type such cascaded alter
and drop actions on the type will propagate to the table.

A typed table always has the same column names and data types as the type
it is derived from, and you cannot specify additional columns.  But the
CREATE TABLE command can add defaults and constraints to the table, as well
as specify storage parameters.
"""

We do use the term "stand-alone composite" in create type so I'm inclined
to use it instead of "composite created with CREATE TYPE"; especially in
the error messages; I'm a bit more willing to add the cross-reference to
create type in the user docs.

David J.

Re: Possibility to disable `ALTER SYSTEM`

[David G. Johnston]

On Wed, Mar 27, 2024 at 10:12 AM Isaac Morland 
wrote:

> On Wed, 27 Mar 2024 at 13:05, Greg Sabino Mullane 
> wrote:
>
>> The purpose of the setting is to prevent accidental
>>> modifications via ALTER SYSTEM in environments where
>>
>>
>> The emphasis on 'accidental' seems a bit heavy here, and odd. Surely,
>> just "to prevent modifications via ALTER SYSTEM in environments where..."
>> is enough?
>>
>
> Not necessarily disagreeing, but it's very important nobody ever mistake
> this for a security feature. I don't know if the extra word "accidental" is
> necessary, but I think that's the motivation.
>

Prevent non-malicious modifications via ALTER SYSTEM in environments where
...

David J.

Re: Possibility to disable `ALTER SYSTEM`

[David G. Johnston]

On Wed, Mar 27, 2024 at 3:13 PM Bruce Momjian  wrote:

> On Wed, Mar 27, 2024 at 06:09:02PM -0400, Bruce Momjian wrote:
> > On Wed, Mar 27, 2024 at 11:05:55AM -0400, Robert Haas wrote:
> > > On Wed, Mar 27, 2024 at 10:43 AM Jelte Fennema-Nio 
> wrote:
> > > > Alright, changed the GUC name to "allow_alter_system" since that
> seems
> > > > to have the most "votes". One other option would be to call it simply
> > > > "alter_system", just like "jit" is not called "allow_jit" or
> > > > "enable_jit".
> > > >
> > > > But personally I feel that the "allow_alter_system" is clearer than
> > > > plain "alter_system" for the GUC name.
> > >
> > > I agree, and have committed your 0001.
> >
> > So, I email "Is this really a patch we think we can push into PG 17. I
> > am having my doubts," and the patch is applied a few hours after my
> > email.  Wow.
>
> Also odd is that I don't see the commit in git master, so now I am
> confused.
>

The main feature being discussed is in the 0002 patch while Robert pushed a
doc section rename in the 0001 patch.

David J.

Re: Possibility to disable `ALTER SYSTEM`

[David G. Johnston]

On Wed, Mar 27, 2024 at 3:18 PM David G. Johnston <
david.g.johns...@gmail.com> wrote:

> On Wed, Mar 27, 2024 at 3:13 PM Bruce Momjian  wrote:
>
>> On Wed, Mar 27, 2024 at 06:09:02PM -0400, Bruce Momjian wrote:
>> > On Wed, Mar 27, 2024 at 11:05:55AM -0400, Robert Haas wrote:
>> > > On Wed, Mar 27, 2024 at 10:43 AM Jelte Fennema-Nio <
>> postg...@jeltef.nl> wrote:
>> > > > Alright, changed the GUC name to "allow_alter_system" since that
>> seems
>> > > > to have the most "votes". One other option would be to call it
>> simply
>> > > > "alter_system", just like "jit" is not called "allow_jit" or
>> > > > "enable_jit".
>> > > >
>> > > > But personally I feel that the "allow_alter_system" is clearer than
>> > > > plain "alter_system" for the GUC name.
>> > >
>> > > I agree, and have committed your 0001.
>> >
>> > So, I email "Is this really a patch we think we can push into PG 17. I
>> > am having my doubts," and the patch is applied a few hours after my
>> > email.  Wow.
>>
>> Also odd is that I don't see the commit in git master, so now I am
>> confused.
>>
>
> The main feature being discussed is in the 0002 patch while Robert pushed
> a doc section rename in the 0001 patch.
>
>
Well, the internal category name was changed though the docs did remain
unchanged.

David J.

Re: Possibility to disable `ALTER SYSTEM`

[David G. Johnston]

On Wed, Mar 27, 2024 at 5:43 PM David G. Johnston <
david.g.johns...@gmail.com> wrote:

>
> This section is also the main entry point for users into the configuration
> subsystem and hasn't been updated to reflect this new feature.  That seems
> like an oversight that needs to be corrected.
>
>
Shouldn't the "alter system" reference page receive an update as well?

David J.

Re: Possibility to disable `ALTER SYSTEM`

[David G. Johnston]

On Wed, Mar 27, 2024 at 5:17 PM Bruce Momjian  wrote:

> On Thu, Mar 28, 2024 at 12:43:29AM +0100, Jelte Fennema-Nio wrote:
> > +  xreflabel="allow_alter_system">
> > +  allow_alter_system (boolean)
> > +  
> > +   allow_alter_system configuration
> parameter
> > +  
> > +  
> > +  
> > +   
> > +When allow_alter_system is set to
> > +off, an error is returned if the
> ALTER
> > +SYSTEM command is used. This parameter can only be
> set in
>
> "command is used." -> "command is issued." ?
>

"command is executed" seems even better.  I'd take used over issued.


> > +the postgresql.conf file or on the server
> command
> > +line. The default value is on.
> > +   
> > +
> > +   
> > +Note that this setting cannot be regarded as a security
> feature. It
>
> "setting cannot be regarded" -> "setting should not be regarded"
>

"setting must not be regarded" is the correct option here.  Stronger than
should; we are unable to control whether someone can/does regard it
differently.


> > +
> > +   
> > +Turning this setting off is intended for environments where the
> > +configuration of PostgreSQL is
> managed by
> > +some outside mechanism.
> > +In such environments, a well intenioned superuser user might
> > +mistakenly use ALTER
> SYSTEM
> > +to change the configuration instead of using the outside
> mechanism.
> > +This might even appear to update the configuration as intended,
> but
>
> "This might even appear to update" -> "This might temporarily update"
>

I strongly prefer temporarily over may/might/could.



>
> > +then might be discarded at some point in the future when that
> outside
>
> "that outside" -> "the outside"
>

Feel like "external" is a more context appropriate term here than "outside".

External also has precedent.
https://www.postgresql.org/docs/current/config-setting.html#CONFIG-INCLUDES
"External tools may also modify postgresql.auto.conf. It is not recommended
to do this while the server is running,"

That suggests using "external tools" instead of "outside mechanisms"

This section is also the main entry point for users into the configuration
subsystem and hasn't been updated to reflect this new feature.  That seems
like an oversight that needs to be corrected.

> +   
> > +
> > +   
> > +This parameter only controls the use of ALTER
> SYSTEM.
> > +The settings stored in
> postgresql.auto.conf always
>
> "always" -> "still"
>

Neither qualifier is needed, nor does one seem clearly better than the
other.  Always is true so the weaker "still" seems like the worse choice.

The following is a complete and clear sentence.

The settings stored in postgresql.auto.conf take effect even if
allow_alter_system is set to off.


> Should this paragraph be moved after or as part of the paragraph about
> modifying postgresql.auto.conf?
>
>
I like it by itself.

David J.

Re: Extension for PostgreSQL WIP

[David G. Johnston]

On Sun, Mar 24, 2024 at 5:49 AM ShadowGhost 
wrote:

> Cast_jsonb_to_hstore WIP
> v1
> This extension add function that can cast jsonb to hstore.
> That link to my github where does my extension lie
> https://github.com/antuanviolin/cast_jsonb_to_hstore
>

If you are intending to submit this to the project you need to follow the
correct procedures.

https://wiki.postgresql.org/wiki/Submitting_a_Patch

Otherwise, this is not the correct place to be promoting your extension.

David J.

p.s. I would advise that including the whole bit about jsonb and hstore in
the email subject line (if you resubmit in a proper format) instead of only
in the body of the email.  Subject lines are very important on a mailing
list such as this and should be fairly precise - not just the word
extension.

Re: Reports on obsolete Postgres versions

[David G. Johnston]

On Thu, Apr 4, 2024 at 11:23 AM Bruce Momjian  wrote:

> On Wed, Apr  3, 2024 at 06:01:41PM -0700, David G. Johnston wrote:
> >  
> >  The PostgreSQL Global Development Group supports a major version for 5
> years
> > -after its initial release. After its five year anniversary, a major
> version
> > will
> > -have one last minor release containing any fixes and will be considered
> > -end-of-life (EOL) and no longer supported.
> > +after its initial release. After its fifth anniversary, a major version
> will
> > +have one last minor release and will be considered
> > +end-of-life (EOL), meaning no new bug fixes will be written for it.
> >  
> >
> > # "fifth anniversary "seems more correct than "five year anniversary".
> > # The fact it gets a minor release implies it receives fixes.
> > # I've always had an issue with our use of the phrasing "no longer
> supported".
> > It seems vague when practically it just means we stop writing patches
> for it.
> > Whether individual community members refrain from answering questions or
> > helping people on these older releases is not a project decision but a
> personal
> > one.  Also, since we already say it is supported for 5 years it seems a
> bit
> > redundant to then also say "after 5 years it is unsupported".
>
> I went with the attached patch.  I tightned up the "unsupported" part,
> trying to
> tie it to the fact that we don't make anymore releases for it.
>
> >  Version Numbering
> > @@ -45,11 +45,12 @@ number, e.g. 9.5.3 to 9.5.4.
> >  Upgrading
> >
> >  
> > -Major versions usually change the internal format of the system tables.
> > -These changes are often complex, so we do not maintain backward
> > -compatibility of all stored data.  A dump/reload of the database or use
> of the
> > -pg_upgrade module is required
> > -for major upgrades. We also recommend reading the
> > +Major versions need the data directory to be initialized so that the
> system
> > tables
> > +specific to that version can be created.  There are two options to then
> > +migrate the user data from the old directory to the new one: a
> dump/reload
> > +of the database or using the
> > +pg_upgrade module.
> > +We also recommend reading the
> >  upgrading section of the
> major
> >  version you are planning to upgrade to. You can upgrade from one major
> version
> >  to another without upgrading to intervening versions, but we recommend
> reading
> > @@ -58,14 +59,15 @@ versions prior to doing so.
> >  
> >
> > # This still talked about "stored data" when really that isn't the main
> concern
> > and if it was pg_upgrade wouldn't work as an option.
> > # The choice to say "data directory" here seems reasonable if arguable.
> But it
> > implies the location of user data and we state it also contains
> > version-specific system tables.  It can go unsaid that they are
> > version-specific because the collection as a whole and the layout of
> individual
> > tables can and almost certainly will change between versions.
> >
> >  
> > -Minor release upgrades do not require a dump and restore;  you simply
> stop
> > +Minor releases upgrades do not impact the data directory, only the
> binaries.
> > +You simply stop
> >  the database server, install the updated binaries, and restart the
> server.
> > -Such upgrades might require manual changes to complete so always read
> > +However, some upgrades might require manual changes to complete so
> always read
> >  the release notes first.
> >  
> >
> > # The fact minor releases don't require dump/reload doesn't directly
> preclude
> > them from needing pg_upgrade and writing "Such upgrades" seems like it
> could
>
> Minor upgrades never have required pg_upgrade.
>
>
How about this:
"""
Major versions make complex changes, so the contents of the data directory
cannot be maintained in a backward compatible way.  A dump and restore of
the databases is required, either done manually or as part of running the
pg_upgrade server application.
"""

My main change here is to mirror "dump and restore" in both paragraphs and
make it clear that this action is required and that the unnamed
pg_dump/pg_restore tools or pg_upgrade are used in order to perform this
task.  Since minor version upgrades do not require "dump and restore" they
need not use these tools.

Also, calling pg_upgrade a module doesn't seem correct.  It is found under
server applications in our docs and consists solely of that program (and a
bunch of manual steps) from the user's perspective.

David J.

Re: Better error messages for %TYPE and %ROWTYPE in plpgsql

[David G. Johnston]

On Mon, Feb 26, 2024 at 6:54 PM Andy Fan  wrote:

>
> "David G. Johnston"  writes:
>
> > On Mon, Feb 26, 2024 at 5:46 PM Andy Fan  wrote:
> >
> >  > Per recent discussion[1], plpgsql returns fairly unhelpful "syntax
> >  > error" messages when a %TYPE or %ROWTYPE construct references a
> >  > nonexistent object.  Here's a quick little finger exercise to try
> >  > to improve that.
> >
> >  Looks this modify the error message, I want to know how ould we treat
> >  error-message-compatible issue during minor / major upgrade.
> >
> > There is no bug here so no back-patch; and we are not yet past feature
> freeze for v17.
>
> Acutally I didn't asked about back-patch.


What else should I be understanding when you write the words "minor
upgrade"?


> So if the error message is changed, the above code may be broken.
>
>
A fair point to bring up, and is change-specific.  User-facing error
messages should be informative and where they are not changing them is
reasonable.  Runtime errors probably need more restraint since they are
more likely to be in a production monitoring alerting system but anything
that is reporting what amounts to a syntax error should be reasonable to
change and not expect people to be writing production code looking for
them.  This seems to fall firmly into the "badly written code"/syntax
category.

David J.

Re: Better error messages for %TYPE and %ROWTYPE in plpgsql

[David G. Johnston]

On Mon, Feb 26, 2024 at 5:46 PM Andy Fan  wrote:

> > Per recent discussion[1], plpgsql returns fairly unhelpful "syntax
> > error" messages when a %TYPE or %ROWTYPE construct references a
> > nonexistent object.  Here's a quick little finger exercise to try
> > to improve that.
>
> Looks this modify the error message, I want to know how ould we treat
> error-message-compatible issue during minor / major upgrade.
>

There is no bug here so no back-patch; and we are not yet past feature
freeze for v17.

David J.

Re: PG catalog

[David G. Johnston]

On Thursday, May 23, 2024, Karki, Sanjay  wrote:
>
> I need to grant select on privilege in pg_catalog to user so I can connect
> via Toad Data point ,
>
> Users can already select from the tables in pg_catalog, grant able
privileges not required or allowed.  Of course, some specific data is
restricted from non-superusers.

David J.

Re: pgsql: Add more SQL/JSON constructor functions

[David G. Johnston]

On Monday, May 27, 2024, Alvaro Herrera  wrote:

> On 2024-May-27, Alvaro Herrera wrote:
>
> > > JSON_SERIALIZE()
>
> I just noticed this behavior, which looks like a bug to me:
>
> select json_serialize('{"a":1, "a":2}' returning varchar(5));
>  json_serialize
> 
>  {"a":
>
> I think this function should throw an error if the destination type
> doesn't have room for the output json.  Otherwise, what good is the
> serialization function?
>
>
It’s not a self-evident bug given that this is exactly how casting data to
varchar(n) behaves as directed by the SQL Standard.

I'd probably leave the internal consistency and take the opportunity to
educate the reader that text is the preferred type in PostgreSQL and,
especially here, there is little good reason to use anything else.

David J.

Re: commitfest.postgresql.org is no longer fit for purpose

[David G. Johnston]

On Friday, May 17, 2024, Joe Conway  wrote:

>
> I wrote:
>
>> Namely, the week before commitfest I don't actually know if I will have
>> the time during that month, but I will make sure my patch is in the
>> commitfest just in case I get a few clear days to work on it. Because if it
>> isn't there, I can't take advantage of those "found" hours.
>>
>
> A solution to both of these issues (yours and mine) would be to allow
> things to be added *during* the CF month. What is the point of having a
> "freeze" before every CF anyway? Especially if they start out clean. If
> something is ready for review on day 8 of the CF, why not let it be added
> for review?
>

In conjunction with WIP removing this limitation on the bimonthlies makes
sense to me.

David J.

Re: CREATE TABLE creates a composite type corresponding to the table row, which is and is not there

[David G. Johnston]

On Fri, May 17, 2024 at 4:57 PM Erik Wienhold  wrote:

> On 2024-05-16 17:47 +0200, David G. Johnston wrote:
> > On Wed, May 15, 2024 at 8:46 AM Robert Haas 
> wrote:
> >
> > > On Thu, Apr 4, 2024 at 12:41 AM Erik Wienhold  wrote:
> > > > Thanks, fixed in v4.  Looks like American English prefers that comma
> and
> > > > it's also more common in our docs.
> > >
> > > Reviewing this patch:
> > >
> > > -  Creates a typed table, which takes its
> > > -  structure from the specified composite type (name optionally
> > > -  schema-qualified).  A typed table is tied to its type; for
> > > -  example the table will be dropped if the type is dropped
> > > -  (with DROP TYPE ... CASCADE).
> > > +  Creates a typed table, which takes its
> > > structure
> > > +  from an existing (name optionally schema-qualified) stand-alone
> > > composite
> > > +  type (i.e., created using )
> though
> > > it
> > > +  still produces a new composite type as well.  The table will
> have
> > > +  a dependency on the referenced type such that cascaded alter and
> > > drop
> > > +  actions on the type will propagate to the table.
> > >
> > > It would be better if this diff didn't reflow the unchanged portions
> > > of the paragraph.
>
> Right.  I now reformatted it so that first line remains unchanged.  But
> the rest of the para is still a complete rewrite.
>
> > > I agree that it's a good idea to mention that the table must have been
> > > created using CREATE TYPE .. AS here, but I disagree with the rest of
> > > the rewording in this hunk. I think we could just add "creating using
> > > CREATE TYPE" to the end of the first sentence, with an xref, and leave
> > > the rest as it is.
> >
> >
> >
> > > I don't see a reason to mention that the typed
> > > table also spawns a rowtype; that's just standard CREATE TABLE
> > > behavior and not really relevant here.
> >
> >
> > I figured it wouldn't be immediately obvious that the system would
> create a
> > second type with identical structure.  Of course, in order for SELECT tbl
> > FROM tbl; to work it must indeed do so.  I'm not married to pointing out
> > this dynamic explicitly though.
> >
> >
> > > And I don't understand what the
> > > rest of the rewording does for us.
> > >
> >
> > It calls out the explicit behavior that the table's columns can change
> due
> > to actions on the underlying type.  Mentioning this unique behavior seems
> > worth a sentence.
> >
> >
> > >   
> > > -  When a typed table is created, then the data types of the
> > > -  columns are determined by the underlying composite type and are
> > > -  not specified by the CREATE TABLE command.
> > > +  A typed table always has the same column names and data types
> as the
> > > +  type it is derived from, and you cannot specify additional
> columns.
> > >But the CREATE TABLE command can add defaults
> > > -  and constraints to the table and can specify storage parameters.
> > > +  and constraints to the table, as well as specify storage
> parameters.
> > >   
> > >
> > > I don't see how this is better.
> > >
> >
> > I'll agree this is more of a stylistic change, but mainly because the
> talk
> > about data types reasonably implies the other items the patch explicitly
> > mentions - names and additional columns.
>
> I prefer David's changes to both paras because right now the details of
> typed tables are spread over the respective CREATE and ALTER commands
> for types and tables.  Or maybe we should add those details to the
> existing "Typed Tables" section at the very end of CREATE TABLE?
>
> > > - errmsg("type %s is not a composite type",
> > > + errmsg("type %s is not a stand-alone composite type",
> > >
> > > I agree with Peter's complaint that people aren't going to understand
> > > what a stand-alone composite type means when they see the revised
> > > error message; to really help people, we're going to need to do better
> > > than this, I think.
> > >
> > >
> > We have a glossary.
>

If sticking with stand-alone composite type as a formal term we should
document it in the glossary.  It's unclear whether this will survive review
though.  With the wording provided in

Re: doc regexp_replace replacement string \n does not explained properly

[David G. Johnston]

On Monday, May 20, 2024, jian he  wrote:

> hi.
>
> https://www.postgresql.org/docs/current/functions-
> matching.html#FUNCTIONS-POSIX-REGEXP
>
>
>  If there is a match,
> the source string is returned with the replacement string substituted
> for the matching substring.


>
This happens regardless of the presence of parentheses.


>
>  The replacement string can contain \n,
> where n is 1 through 9, to indicate that the source substring matching
> the n'th parenthesized subexpression of the pattern should be
> inserted, and it can contain \& to indicate that the substring
> matching the entire pattern should be inserted.


 Then if the replacement text contains “\n” expressions those are replaced
with text captured from the corresponding parentheses group.


> <<
> i think it explained example like:
> SELECT regexp_replace('foobarbaz', 'b(..)', 'X\1Y', 'g');


global - find two matches to process.

foobarbaz
fooX\1YX\1Y
fooXarYXazY


>
> but it does not seem to explain cases like:
> SELECT regexp_replace('foobarbaz', 'b(..)', 'X\2Y', 'g');
>
>
foobarbaz
fooX\2YX\2Y
fooX{empty string, no second capture group}YX{empty}Y
fooXYXY

The docs are correct, though I suppose being explicit that a missing
capture group results in an empty string substitution instead of an error
is probably warranted.

David J.

Re: Patch bug: Fix jsonpath .* on Arrays

[David G. Johnston]

On Tuesday, June 4, 2024, David E. Wheeler  wrote:

> Hackers,
>
> The behavior of the .* jpiAnyKey jsonpath selector seems incorrect.
>
> ```
> select jsonb_path_query('[1,2,3]', '$.*');
> jsonb_path_query
> --
> (0 rows)
>
> select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', '$.*');
> jsonb_path_query
> --
> [3, 4, 5]
> ```
>
> The first example might be expected, since .* is intended for object keys,
> but the handing of `jpiAnyKey` has a branch for unwrapping arrays. The
> second example, however, just seems weird: this is .*, not .**.
>

This seems to be working correctly. Lax mode causes the first array level
to unwrap and produce new context item values.  Then the wildcard member
accessor is applied to each.  Numbers don’t have members so no matches
exist in the first example.  The object in the second indeed has a single
member and so matches the wildcard and its value, the array, is returned.

David J.

Re: Explicit specification of index ensuring uniqueness of foreign columns

[David G. Johnston]

On Friday, May 31, 2024, Tom Lane  wrote:

> Kaiting Chen  writes:
> > I'd like to resurrect a subset of my proposal in [1], specifically that:
> >   The FOREIGN KEY constraint syntax gains a [ USING INDEX index_name ]
> clause
> >   optionally following the referenced column list.
> > ...
> > While, in this minimal reproduction, the two indexes are
> interchangeable, there
> > are situations that may reasonably occur in the course of ordinary use
> in which
> > they aren't. For example, a redundant unique index with different storage
> > parameters may exist during the migration of an application schema.
>
> I agree that there's a hazard there, but I question if the case is
> sufficiently real-world to justify the costs of introducing a
> non-SQL-standard clause in foreign key constraints.
>
> One such cost is that pg_dump output would become less able to be
> loaded into other DBMSes, or even into older PG versions.
>
> I also wonder if this wouldn't just trade one fragility for another.
> Specifically, I am not sure that we guarantee that the names of
> indexes underlying constraints remain the same across dump/reload.
> If they don't, the USING INDEX clause might fail unnecessarily.
>
> As against that, I'm not sure I've ever seen a real-world case with
> intentionally-duplicate unique indexes.
>
> So on the whole I'm unconvinced that this is worth changing.


Seems like most of those issues could be avoided if we only supply “alter
table” syntax (or a function…).  i.e., give the dba a tool to modify their
system when our default choices fail them.  But continue on with the
defaults as they exist today.

David J.

Re: improve predefined roles documentation

[David G. Johnston]

On Thu, Jun 13, 2024 at 12:48 PM Nathan Bossart 
wrote:

> I think we could improve matters by abandoning the table and instead
> documenting these roles more like we document GUCs, i.e., each one has a
> section below it where we can document it in as much detail as we want.
>
>
One of the main attributes for the GUCs is their category.  If we want to
improve organization we'd need to assign categories first.  We already
implicitly do so in the description section where we do group them together
and explain why - but it is all informal.  But getting rid of those
groupings and descriptions and isolating each role so it can be linked to
more easily seems like a net loss in usability.

I'm against getting rid of the table.  If we do add authoritative
subsection anchors we should just do like we do in System Catalogs and make
the existing table name values hyperlinks to those newly added anchors.
Breaking the one table up into multiple tables along category lines is
something to consider.

David J.

Re: jsonpath: Missing Binary Execution Path?

[David G. Johnston]

On Thursday, June 13, 2024, Chapman Flack  wrote:

> On 06/13/24 21:24, David G. Johnston wrote:
> > I'm content that the operators in the 'filter operators' table need to be
> > within filter but then I cannot reconcile why this example worked:
> >
> > david=# select jsonb_path_query('1', '$ >= 1');
>
> Good point. I can't either. No way I can see to parse that as
> a .
>


Whether we note it as non-standard or not is an open question then, but it
does work and opens up a documentation question.  It seems like it needs to
appear in table T9.50.  Whether it also should appear in T9.51 is the
question.  It seems like anything in T9.50 is allowed in a filter while the
stuff in T9.51 should be limited to those things only allowed in a filter.
Which suggests moving it from T9.51 to T9.50

David J.

Re: jsonpath: Missing Binary Execution Path?

[David G. Johnston]

On Thursday, June 13, 2024, Chapman Flack  wrote:

> On 06/13/24 21:46, David G. Johnston wrote:
> >>> david=# select jsonb_path_query('1', '$ >= 1');
> >>
> >> Good point. I can't either. No way I can see to parse that as
> >> a .
> >
> > Whether we note it as non-standard or not is an open question then, but
> it
> > does work and opens up a documentation question.
>
> Does the fact that it does work raise any potential concern that our
> grammar is nonconformant in some way that could present a headache
> somewhere else, or down the road with a later standard edition?
>

This isn’t new in v17 nor, to my knowledge, has the behavior changed, so I
think we just need to live with whatever, likely minimal, chance of
headache there is.

I don’t get why the outcome of a boolean producing operation isn’t just
generally allowed to be produced, and would hope the standard would move
toward allowing that across the board, and in doing so end up matching what
we already have implemented.

David J.

Re: jsonpath: Missing Binary Execution Path?

[David G. Johnston]

On Thu, Jun 13, 2024 at 6:10 PM Chapman Flack  wrote:

> On 06/13/24 16:43, David E. Wheeler wrote:
> > Paging Mr. Eisentraut!
>
> I'm not Mr. Eisentraut, but I have at last talked my way into some
> access to the standard, so ...
>
> Note 487 emphasizes that JSON path predicates "are not expressions;
> instead they form a separate language that can only be invoked within
> a ".
>
> The only operators usable in a general expression (that is, a
>  are binary + - and binary * / % and unary + -
> over a .
>
> Inside a filter, you get to use a . That's where
> you can use ! and && and ||. But ! can only be applied to a
> : either a ,
> or any other  wrapped in parentheses.
>
> On 06/13/24 11:32, David E. Wheeler wrote:
> > david=# select jsonb_path_query('true', '$ && $');
> > david=# select jsonb_path_query('true', '$.boolean() && $.boolean()');
>
> Those don't work because, as you recognized, they're not inside filters.
>

I'm content that the operators in the 'filter operators' table need to be
within filter but then I cannot reconcile why this example worked:

david=# select jsonb_path_query('1', '$ >= 1');
 jsonb_path_query
--
 true
(1 row)

David J.

Re: Document NULL

[David G. Johnston]

On Sat, May 11, 2024 at 11:00 AM David G. Johnston <
david.g.johns...@gmail.com> wrote:

> Though I haven’t settled on a phrasing I really like.  But I’m trying to
> avoid a parenthetical.
>
>
Settled on this:

The cardinal rule, a null value is neither
   equal nor unequal
   to any value, including other null values.

I've been tempted to just say, "to any value.", but cannot quite bring
myself to do it...

David J.

Re: create role manual

[David G. Johnston]

On Sat, Jun 15, 2024 at 7:25 PM Tatsuo Ishii  wrote:

>The rules for which initial
>role membership options are enabled described below in the
>IN ROLE, ROLE, and
>ADMIN clauses.
>
> Maybe we need "are" in front of "described"?
>
>
Agreed.

David J.

Re: ON ERROR in json_query and the like

[David G. Johnston]

On Wednesday, June 12, 2024, Markus Winand  wrote:
>
>
> 10.14 SR 1: The declared type of the  simply contained
> in the  immediately contained in the  item> shall be a string type or a JSON type.


> It might be best to think of it as two separate functions, overloaded:
>
> JSON_VALUE(context_item JSONB, path_expression …)
> JSON_VALUE(context_item TEXT, path_expression …)


Yes, we need to document that we deviate from (fail to fully implement) the
standard here in that we only provide jsonb parameter functions, not text
ones.

David J.

Re: ON ERROR in json_query and the like

[David G. Johnston]

On Tuesday, May 28, 2024, Markus Winand  wrote:

>
> 2. EMPTY [ARRAY|OBJECT] ON ERROR implies ERROR ON EMPTY
>
>17beta1=# SELECT JSON_QUERY('[]', '$[*]' EMPTY ARRAY ON ERROR) a;
> a
>
> []
>(1 row)
>
>As NULL ON EMPTY is implied, it should give the same result as
>explicitly adding NULL ON EMPTY:
>
>17beta1=# SELECT JSON_QUERY('[]', '$[*]' NULL ON EMPTY EMPTY ARRAY ON
> ERROR) a;
> a
>---
>
>(1 row)
>
>Interestingly, Oracle DB gives the same (wrong) results. Db2 (LUW)
>on the other hand returns NULL for both queries.
>
>I don’t think that PostgreSQL should follow Oracle DB's suit here
>but again, in case this is intentional it should be made explicit
>in the docs.
>

The docs here don’t seem to cover the on empty clause at all nor fully
cover all options.

Where do you find the claim that the one implies the other?  Is it a typo
that your examples says “implies null on empty” but the subject line says
“implies error on empty”?

Without those clauses a result is either empty or an error - they are
mutually exclusive (ignoring matches).  I would not expect one clause to
imply or affect the behavior of the other.  There is no chaining.  The
original result is transformed to the new result specified by the clause.

I’d need to figure out whether the example you show is actually producing
empty or error; but it seems correct if the result is empty.  The first
query ignores the error clause - the empty array row seems to be the
representation of empty here; the second one matches the empty clause and
outputs null instead of the empty array.

David J.

Re: Shouldn't jsonpath .string() Unwrap?

[David G. Johnston]

On Sat, Jun 8, 2024 at 3:50 PM David E. Wheeler 
wrote:

> Hackers,
>
> Most of the jsonpath methods auto-unwrap in lax mode:
>
> david=# select jsonb_path_query('[-2,5]', '$.abs()');
>  jsonb_path_query
> --
>  2
>  5
> (2 rows)
>
> The obvious exceptions are size() and type(), which apply directly to
> arrays, so no need to unwrap:
>
> david=# select jsonb_path_query('[-2,5]', '$.size()');
>  jsonb_path_query
> --
>  2
> (1 row)
>
> david=# select jsonb_path_query('[-2,5]', '$.type()');
>  jsonb_path_query
> --
>  "array"
>
> But what about string()? Is there some reason it doesn’t unwrap?
>
> david=# select jsonb_path_query('[-2,5]', '$.string()');
> ERROR:  jsonpath item method .string() can only be applied to a bool,
> string, numeric, or datetime value
>
> What I expect:
>
> david=# select jsonb_path_query('[-2,5]', '$.string()');
>  jsonb_path_query
> —
>  "2"
>  "5"
> (2 rows)
>
> However, I do see a test[1] for this behavior, so maybe there’s a reason
> for it?
>
>
Adding Andrew.

I'm willing to call this an open item against this feature as I don't see
any documentation explaining that string() behaves differently than the
others.

David J.

Re: Document NULL

[David G. Johnston]

On Tue, Jun 18, 2024 at 8:34 PM Yugo NAGATA  wrote:

>
> It may be a trivial thing but I am not sure we need to mention case
> insensitivity
> here, because all keywords and unquoted identifiers are case-insensitive in
> PostgreSQL and it is not specific to NULL.
>

But it is neither a keyword nor an identifier.  It behaves more like:
SELECT 1 as one;  A constant, which have no implied rules - mainly because
numbers don't have case.  Which suggests adding some specific mention there
- and also probably need to bring up it and its "untyped" nature in the
syntax chapter, probably here:

https://www.postgresql.org/docs/current/sql-syntax-lexical.html#SQL-SYNTAX-CONSTANTS-GENERIC


> Also, I found the other parts of the documentation use "case-insensitive"
> in which
> words are joined with hyphen, so I wonder it is better to use the same
> form if we
> leave the description.
>
>
Typo on my part, fixed.

I'm not totally against just letting this content be assumed to be learned
from elsewhere in the documentation but it also seems reasonable to
include.  I'm going to leave it for now.

David J.

Re: Document NULL

[David G. Johnston]

On Tuesday, June 18, 2024, Tom Lane  wrote:

> Yugo NAGATA  writes:
> > On Tue, 18 Jun 2024 20:56:58 -0700
> > "David G. Johnston"  wrote:
> >> But it is neither a keyword nor an identifier.
>
> The lexer would be quite surprised by your claim that NULL isn't
> a keyword.  Per src/include/parser/kwlist.h, NULL is a keyword,
> and a fully reserved one at that.
>
>
>

Can’t it be both a value and a keyword?  I figured the not null constraint
and is null predicates are why it’s a keyword but the existence of those
doesn’t cover its usage as a literal value that can be stuck anywhere you
have an expression.

David J.

Re: Extension security improvement: Add support for extensions with an owned schema

[David G. Johnston]

On Wed, Jun 19, 2024 at 8:19 AM Jelte Fennema-Nio  wrote:


> Because part of it would
> only be relevant once we support upgrading from PG18. So for now the
> upgrade_code I haven't actually run.
>

Does it apply against v16?  If so, branch off there, apply it, then upgrade
from the v16 branch to master.

David J.

Re: SQL/JSON query functions context_item doc entry and type requirement

[David G. Johnston]

On Wed, Jun 19, 2024 at 8:29 AM jian he  wrote:

> On Mon, Jun 17, 2024 at 9:05 PM Chapman Flack  wrote:
> >
> > Hi,
> >
> > On 06/17/24 02:43, Amit Langote wrote:
> > > context_item expression can be a value of
> > > any type that can be cast to jsonb. This includes types
> > > such as char,  text, bpchar,
> > > character varying, and bytea (with
> > > ENCODING UTF8), as well as any domains over these types.
> >
> > Reading this message in conjunction with [0] makes me think that we are
> > really talking about a function that takes a first parameter of type
> jsonb,
> > and behaves exactly that way (so any cast required is applied by the
> system
> > ahead of the call). Under those conditions, this seems like an unusual
> > sentence to add in the docs, at least until we have also documented that
> > tan's argument can be of any type that can be cast to double precision.
> >
>
> I guess it would be fine to add an unusual sentence to the docs.
>
> imagine a function: array_avg(anyarray) returns anyelement.
> array_avg calculate an array's elements's avg. like
> array('{1,2,3}'::int[]) returns 2.
> but array_avg won't make sense if the input argument is a date array.
> so mentioning in the doc: array_avg can accept anyarray, but anyarray
> cannot date array.
> seems ok.
>

There is existing wording for this:

"The expression can be of any JSON type, any character string type, or
bytea in UTF8 encoding."

If you add this sentence to the paragraph the link that already exists,
which simply points the reader to this sentence, becomes redundant and
should be removed.

As for table 9.16.3 - it is unwieldy already.  Lets try and make the core
syntax shorter, not longer.  We already have precedence in the subsequent
json_table section - give each major clause item a name then below the
table define the syntax and meaning for those names.  Unlike in that
section - which probably should be modified too - context_item should have
its own description line.

David J.

Re: Wrong security context for deferred triggers?

[David G. Johnston]

On Sat, Jun 22, 2024 at 7:21 PM Joseph Koshakow  wrote:

> On Sat, Jun 22, 2024 at 6:23 PM David G. Johnston <
> david.g.johns...@gmail.com> wrote:
>
> > except invoker and triggerer are the same entity
>
> Maybe "executor" would have been a better term than 'invoker". In this
> specific example they are not the same entity. The trigger is
> triggered and queued by one role and executed by a different role,
> hence the confusion.
>

No matter what we label the keyword it would be represent the default and
existing behavior whereby the environment at trigger resolution time, not
trigger enqueue time, is used.

I suppose there is an argument for capturing and reapplying the trigger
enqueue time environment and giving that a keyword as well.  But fewer
options has value and security definer seems like the strictly superior
option.


> Though I agree with Laurenz, special SQL syntax
> for this exotic corner case is a little too much.
>

It doesn't seem like a corner case if we want to institute a new
recommended practice that all triggers should be created with security
definer.  We seem to be discussing that without giving the dba a choice in
the matter - but IMO we do want to provide the choice and leave the default.


> > Security definer on the function would take precedence as would its set
> clause.
>
> These trigger options seem a bit redundant with the equivalent options
> on the function that is executed by the trigger. What would be the
> advantages or differences of setting these options on the trigger
> versus the function?
>
>
At least security definer needs to take precedence as the function owner is
fully expecting their role to be the one executing the function, not
whomever the trigger owner might be.

If putting a set clause on the trigger is a thing then the same thing goes
- the function author, if they also did that, expects their settings to be
in place.  Whether it really makes sense to have trigger owner set
configuration when they attach the function is arguable but also the most
flexible option.

David J.

Re: Wrong security context for deferred triggers?

[David G. Johnston]

On Sat, Jun 8, 2024 at 2:37 PM Joseph Koshakow  wrote:

>
>
Something like
> `SECURITY INVOKER | SECURITY TRIGGERER` (modeled after the modifiers in
> `CREATE FUNCTION`) that control which role is used.
>

I'm inclined toward this option (except invoker and triggerer are the same
entity, we need owner|definer).  I'm having trouble accepting changing the
existing behavior here but agree that having a mode whereby the owner of
the trigger/table executes the trigger function in an initially clean
environment (server/database defaults; the owner role isn't considered as
having logged in so their personalized configurations do not take effect)
(maybe add a SET clause to create trigger too).  Security invoker would be
the default, retaining current behavior for upgrade/dump+restore.

Security definer on the function would take precedence as would its set
clause.

David J.

Re: Unable parse a comment in gram.y

[David G. Johnston]

On Sat, Jun 22, 2024 at 9:02 PM Tatsuo Ishii  wrote:

> I was unable to parse a comment in src/backend/parser/gram.y around line
> 11364:
>
> /*
>  * As func_expr but does not accept WINDOW functions directly (they
>  * can still be contained in arguments for functions etc.)
>  * Use this when window expressions are not allowed, so to disambiguate
>  * the grammar. (e.g. in CREATE INDEX)
>  */
>
> Maybe "but" is unnecessary in the first sentence in the comment?
>
>
The "but" is required, add a comma before it.  It could also be written a
bit more verbosely:

func_expr_windowless is the same as func_expr aside from not accepting
window functions directly ...

David J.

Re: ON ERROR in json_query and the like

[David G. Johnston]

On Thu, Jun 20, 2024 at 2:19 AM Amit Langote 
wrote:

> Hi,
>
> On Mon, Jun 17, 2024 at 10:07 PM Markus Winand 
> wrote:
> > > On 17.06.2024, at 08:20, Amit Langote  wrote:
> > > Agree that the documentation needs to be clear about this. I'll update
> > > my patch at [1] to add a note next to table 9.16.3. SQL/JSON Query
> > > Functions.
> >
> > Considering another branch of this thread [1] I think the
> > "Supported Features” appendix of the docs should mention that as well.
> >
> > The way I see it is that the standards defines two overloaded
> > JSON_QUERY functions, of which PostgreSQL will support only one.
> > In case of valid JSON, the implied CAST makes it look as though
> > the second variant of these function was supported as well but that
> > illusion totally falls apart once the JSON is not valid anymore.
> >
> > I think it affects the following feature IDs:
> >
> >   - T821, Basic SQL/JSON query operators
> >  For JSON_VALUE, JSON_TABLE and JSON_EXISTS
> >   - T828, JSON_QUERY
> >
> > Also, how hard would it be to add the functions that accept
> > character strings? Is there, besides the effort, any thing else
> > against it? I’m asking because I believe once released it might
> > never be changed — for backward compatibility.
>
> Hmm, I'm starting to think that adding the implied cast to json wasn't
> such a great idea after all, because it might mislead the users to
> think that JSON parsing is transparent (respects ON ERROR), which is
> what you are saying, IIUC.
>
>
Actually, the implied cast is exactly the correct thing to do here - the
issue is that we aren't using the newly added soft errors infrastructure
[1] to catch the result of that cast and instead produce whatever output on
error tells us to produce.  This seems to be in the realm of doability so
we should try in the interest of being standard conforming.  I'd even argue
to make this an open item unless and until the attempt is agreed upon to
have failed (or it succeeds of course).

David J.

[1] d9f7f5d32f201bec61fef8104aafcb77cecb4dcb

Re: minor doc issue in 9.16.2.1.1. Boolean Predicate Check Expressions

[David G. Johnston]

On Thu, Jun 20, 2024 at 7:30 PM jian he  wrote:

> "predicate check expressions return the single three-valued result of
>
the predicate: true, false, or unknown."
> "unknown" is wrong, because `select 'unknown'::jsonb;` will fail.
> here "unknown" should be "null"? see jsonb_path_query doc entry also.
>
>
The syntax for json_exists belies this claim (assuming our docs are
accurate there).  Its "on error" options are true/false/unknown.
Additionally, the predicate test operator is named "is unknown" not "is
null".

The result of the predicate test, which is never produced as a value, only
a concept, is indeed "unknown" - which then devolves to false when it is
practically applied to determining whether to output the path item being
tested.  As it does also when used in a parth expression.

postgres=# select json_value('[null]','$[0] < 1');
 json_value

 f

postgres=# select json_value('[null]','$[0] == null');
 json_value

 t

Not sure how to peek inside the jsonpath system here though...

postgres=# select json_value('[null]','($[0] < 1) == null');
ERROR:  syntax error at or near "==" of jsonpath input
LINE 1: select json_value('[null]','($[0] < 1) == null');

I am curious if that produces true (the unknown is left as null) or false
(the unknown becomes false immediately).

David J.

Re: ON ERROR in json_query and the like

[David G. Johnston]

On Thursday, June 20, 2024, Markus Winand  wrote:

>
>
> > On 21.06.2024, at 06:46, David G. Johnston 
> wrote:
> >>
>
> >
> > 2 also has the benefit of being standard conforming while 1 does not.
>
> Why do you think so? Do you have any references or is this just based on
> previous statements in this discussion?
>
>
Hearsay.


https://www.postgresql.org/message-id/CAFj8pRCnzO2cnHi5ebXciV%3DtuGVvAQOW9uPU%2BDQV1GkL31R%3D-g%40mail.gmail.com

> 4) If ALREADY PARSED is False, then it is implementation-defined whether
the
> following rules are applied:
> a) The General Rules of Subclause 9.36, "Parsing JSON text", are applied
with
> JT as JSON TEXT, an implementation-defined 
> as UNIQUENESS CONSTRAINT, and FO as FORMAT OPTION; let ST be the STATUS
and
> let CISJI be the SQL/JSON ITEM returned from the application of those
> General Rules.
> b) If ST is not successful completion, then ST is returned as the STATUS
of
> this application of these General Rules, and no further General Rules of
> this Subclause are applied.

But maybe I’m mis-interpreting that snippet and Nikita’s related commentary
regarding have chosen between options for this implementation-defined
feature.

David j.

Re: SQL/JSON query functions context_item doc entry and type requirement

[David G. Johnston]

On Thu, Jun 20, 2024 at 9:01 AM jian he  wrote:

> On Thu, Jun 20, 2024 at 5:46 PM Amit Langote 
> wrote:
> >
> > On Thu, Jun 20, 2024 at 1:03 AM David G. Johnston
> >  wrote:
> > > On Wed, Jun 19, 2024 at 8:29 AM jian he 
> wrote:
> > >>
> > >> On Mon, Jun 17, 2024 at 9:05 PM Chapman Flack 
> wrote:
> > >> >
> > >> > Hi,
> > >> >
> > >> > On 06/17/24 02:43, Amit Langote wrote:
> > >> > > context_item expression can be a value
> of
> > >> > > any type that can be cast to jsonb. This includes
> types
> > >> > > such as char,  text,
> bpchar,
> > >> > > character varying, and bytea (with
> > >> > > ENCODING UTF8), as well as any domains over these
> types.
> > >> >
> > >> > Reading this message in conjunction with [0] makes me think that we
> are
> > >> > really talking about a function that takes a first parameter of
> type jsonb,
> > >> > and behaves exactly that way (so any cast required is applied by
> the system
> > >> > ahead of the call). Under those conditions, this seems like an
> unusual
> > >> > sentence to add in the docs, at least until we have also documented
> that
> > >> > tan's argument can be of any type that can be cast to double
> precision.
> > >> >
> > >>
> > >> I guess it would be fine to add an unusual sentence to the docs.
> > >>
> > >> imagine a function: array_avg(anyarray) returns anyelement.
> > >> array_avg calculate an array's elements's avg. like
> > >> array('{1,2,3}'::int[]) returns 2.
> > >> but array_avg won't make sense if the input argument is a date array.
> > >> so mentioning in the doc: array_avg can accept anyarray, but anyarray
> > >> cannot date array.
> > >> seems ok.
> > >
> > >
> > > There is existing wording for this:
> > >
> > > "The expression can be of any JSON type, any character string type, or
> bytea in UTF8 encoding."
> > >
> > > If you add this sentence to the paragraph the link that already
> exists, which simply points the reader to this sentence, becomes redundant
> and should be removed.
> >
> > I've just posted a patch in the other thread [1] to restrict
> > context_item to be of jsonb type, which users would need to ensure by
> > adding an explicit cast if needed.  I think that makes this
> > clarification unnecessary.
> >
> > > As for table 9.16.3 - it is unwieldy already.  Lets try and make the
> core syntax shorter, not longer.  We already have precedence in the
> subsequent json_table section - give each major clause item a name then
> below the table define the syntax and meaning for those names.  Unlike in
> that section - which probably should be modified too - context_item should
> have its own description line.
> >
> > I had posted a patch a little while ago at [1] to render the syntax a
> > bit differently with each function getting its own syntax synopsis.
> > Resending it here; have addressed Jian He's comments.
> >
> > --
>

I was thinking more like:

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index c324906b22..b9d157663a 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -18692,8 +18692,10 @@ $.* ? (@ like_regex "^\\d+$")
   
 json_exists
 json_exists (
-context_item,
path_expression 
PASSING { value
AS varname } ,
...
- { TRUE | FALSE
| UNKNOWN | ERROR } ON
ERROR )
+context_item,
+path_expression
+variable_definitions
+on_error_boolean)


 Returns true if the SQL/JSON
path_expression
@@ -18732,12 +18734,14 @@ ERROR:  jsonpath array subscript is out of bounds
   
 json_query
 json_query (
-context_item,
path_expression 
PASSING { value
AS varname } ,
...
- RETURNING
data_type  FORMAT
JSON  ENCODING UTF8 
 
- { WITHOUT | WITH
{ CONDITIONAL |
UNCONDITIONAL } } 
ARRAY  WRAPPER 
- { KEEP | OMIT }
QUOTES  ON SCALAR STRING
 
- { ERROR | NULL |
EMPTY {  ARRAY 
| OBJECT } | DEFAULT
expression } ON EMPTY

- { ERROR | NULL |
EMPTY {  ARRAY 
| OBJECT } | DEFAULT
expression } ON ERROR
)
+context_item,
+path_expression
+variable_definitions
+return_clause
+wrapping_clause
+quoting_clause
+on_empty_set
+on_error_set)
   

 Returns the result of applying the SQL/JSON
@@ -18809,11 +18813,12 @@ DETAIL:  Missi

Re: ON ERROR in json_query and the like

[David G. Johnston]

On Thu, Jun 20, 2024 at 5:22 PM Amit Langote 
wrote:

>
> Soft error handling *was* used for catching cast errors in the very
> early versions of this patch (long before I got involved and the
> infrastructure you mention got added).  It was taken out after Pavel
> said [1] that he didn't like producing NULL instead of throwing an
> error.  Not sure if Pavel's around but it would be good to know why he
> didn't like it at the time.
>
>
I'm personally in the "make it error" camp but "make it conform to the
standard" is a stronger membership (in general).

I see this note in your linked thread:

> By the standard, it is implementation-defined whether JSON parsing errors
> should be caught by ON ERROR clause.

Absent someone contradicting that claim I retract my position here and am
fine with failing if these "functions" are supplied with something that
cannot be cast to json.  I'd document them like functions that accept json
with the implications that any casting to json happens before the function
is called and thus its arguments do not apply to that step.

Given that we have "expression IS JSON" the ability to hack together a case
expression to get non-erroring behavior exists.

David J.

Re: improve predefined roles documentation

[David G. Johnston]

On Tue, Jun 18, 2024 at 9:52 AM Nathan Bossart 
wrote:

> On Mon, Jun 17, 2024 at 02:10:22PM -0400, Robert Haas wrote:
> > On Thu, Jun 13, 2024 at 3:48 PM Nathan Bossart 
> wrote:
> >> I think we could improve matters by abandoning the table and instead
> >> documenting these roles more like we document GUCs, i.e., each one has a
> >> section below it where we can document it in as much detail as we want.
> >> Some of these roles should probably be documented together (e.g.,
> >> pg_read_all_data and pg_write_all_data), so the ordering is unlikely to
> be
> >> perfect, but I'm hoping it would still be a net improvement.
> >
> > +1. I'm not sure about all of the details, but I like the general idea.
>
> Here is a first try.  I did pretty much exactly what I proposed in the
> quoted text, so I don't have much else to say about it.  I didn't see an
> easy way to specify multiple ids and xreflabels for a given entry, so the
> entries that describe multiple roles just use the name of the first role
> listed.  In practice, I think this just means you need to do a little extra
> work when linking to one of the other roles from elsewhere in the docs,
> which doesn't seem too terrible.
>
>
I like this.  Losing the table turned out to be ok.  Thank you.

I would probably put pg_monitor first in the list.

+ A user granted this role cannot however send signals to a backend owned
by a superuser.

Remove "however", or put commas around it.  I prefer the first option.

Do we really need to repeat "even without having it explicitly" everywhere?

+ This role does not have the role attribute BYPASSRLS set.

Even if it did, that attribute isn't inherited anyway...

"This role is still governed by any row level security policies that may be
in force.  Consider setting the BYPASSRLS attribute on member roles."

(assuming they intend it to be ALL data then doing the bypassrls even if
they are not today using it doesn't hurt)

pg_stat_scan_tables - This explanation leaves me wanting more.  Maybe give
an example of such a function?  I think the bar is set a bit too high just
talking about a specific lock level.

"As these roles are able to access any file on the server file system,"

We forbid running under root so this isn't really true.  They do have
operating system level access logged in as the database process owner.
They are able to access all PostgreSQL files on the server file system and
usually can run a wide-variety of commands on the server.

"access, therefore great care should be taken"

I would go with:

"access.  Great care should be taken"

Seems more impactful as its own sentence then at the end of a long
multi-part sentence.

"server with COPY any other file-access functions." - s/with/using/

David J.

Re: ON ERROR in json_query and the like

[David G. Johnston]

On Thursday, June 20, 2024, Pavel Stehule  wrote:

>
>
> pá 21. 6. 2024 v 6:01 odesílatel Amit Langote 
> napsal:
>
>> On Fri, Jun 21, 2024 at 10:01 AM David G. Johnston
>>  wrote:
>>
>> > > By the standard, it is implementation-defined whether JSON parsing
>> errors
>> > > should be caught by ON ERROR clause.
>> >
>> > Absent someone contradicting that claim I retract my position here and
>> am fine with failing if these "functions" are supplied with something that
>> cannot be cast to json.  I'd document them like functions that accept json
>> with the implications that any casting to json happens before the function
>> is called and thus its arguments do not apply to that step.
>>
>> Thanks for that clarification.
>>
>> So, there are the following options:
>>
>> 1. Disallow anything but jsonb for context_item (the patch I posted
>> yesterday)
>>
>> 2. Continue allowing context_item to be non-json character or utf-8
>> encoded bytea strings, but document that any parsing errors do not
>> respect the ON ERROR clause.
>>
>> 3. Go ahead and fix implicit casts to jsonb so that any parsing errors
>> respect ON ERROR (no patch written yet).
>>
>> David's vote seems to be 2, which is my inclination too.  Markus' vote
>> seems to be either 1 or 3.  Anyone else?
>>
>
> @3 can be possibly messy (although be near Oracle or standard). I don't
> think it is safe - one example '{a:10}' is valid for Oracle, but not for
> Postgres, and using @3 impacts different results (better to raise an
> exception).
>
> The effect of @1 and @2 is similar - @1 is better so the user needs to
> explicitly cast, so maybe it is cleaner, so the cast should not be handled,
> @2 is more user friendly, because it accepts unknown string literal. From a
> developer perspective I prefer @1, from a user perspective I prefer @2.
> Maybe @2 is a good compromise.
>

2 also has the benefit of being standard conforming while 1 does not.

3 is also conforming and I wouldn’t object to it had we already done it
that way.

But since 2 is conforming too and implemented, and we are in beta, I'm
thinking we need to go with this option.

David J.

Re: Proposal: Division operator for (interval / interval => double precision)

[David G. Johnston]

On Sun, Jun 23, 2024 at 5:57 PM Gurjeet Singh  wrote:

> Is there a desire to have a division operator / that takes dividend
> and divisor of types interval, and results in a quotient of type
> double precision.

[...]
> ('365 days'::interval / '3 days'::interval) => 121
> ('365 days'::interval % '3 days'::interval) => 2
>
>
Is it double or biginteger that your operation is producing?

How about making the % operator output an interval?  What is the answer to:

'1 day 12 hours 59 min 10 sec' / '3 hours 22 min 6 sec'?

Though I'd rather add functions to produce numbers from intervals then let
the existing math operations be used on those.  These seem independently
useful though like this feature I've not really seen demand for them from
others.

in_years(interval) -> numeric
in_days(interval) -> numeric
in_hours(interval) -> numeric
in_microseconds(interval) -> numeric
etc...

That said, implementing the inverse of the existing
interval/double->interval operator has a nice symmetry.  Though the 4
examples are trivial, single unit, single scale, divisions, so exactly how
that translates into support for a possibly messy example like above I'm
uncertain.

There is no precedence, but why not add a new composite type, (whole
bigint, remainder bigint) that, for you example #2, would be
(121,2*24*60*60*1000*1000), the second field being 2 days in microseconds?
Possibly under a different operator so those who just want integer division
can have it more cheaply and easily.

David J.

Re: [PATCH] Add ACL (Access Control List) acronym

[David G. Johnston]

On Mon, Jun 24, 2024 at 12:46 PM Joel Jacobson  wrote:

> On Mon, Jun 24, 2024, at 18:02, David G. Johnston wrote:
>
> > The page we link to uses "permissions" while we consistently use
> > "privileges" to describe the contents of the list.  This seems like an
> > obvious synonym, but as the point of these is to formally define
> > things, pointing this equivalence is worth considering.
>
> I like this idea. How could this be implemented in the docs? Maybe a
> ... for ACL in acronyms.sgml?
>
>
Add a second  under the one holding the link?

David J.

Re: [PATCH] Add ACL (Access Control List) acronym

[David G. Johnston]

On Mon, Jun 24, 2024 at 8:44 AM Nathan Bossart 
wrote:

> On Mon, Jun 24, 2024 at 02:32:27PM +0200, Joel Jacobson wrote:
> > This patch is based on a suggestion from a separate thread [1]:
> >
> > On Mon, Jun 24, 2024, at 01:46, Michael Paquier wrote:
> >> Rather unrelated to this patch, still this patch makes the situation
> >> more complicated in the docs, but wouldn't it be better to add ACL as
> >> a term in acronyms.sql, and reuse it here?  It would be a doc-only
> >> patch that applies on top of the rest (could be on a new thread of its
> >> own), with some  markups added where needed.
>
> Sounds reasonable to me.
>

+1


> +  https://en.wikipedia.org/wiki/Access_Control_List;>Access
> Control List, i.e. privileges list
>
> I think we could omit "i.e. privileges list."
>
>
Agreed.  Between the docs and code we say "privileges list" once and that
refers to the dumputIls description of the arguments to grant.  As the
acronym page now defines the term using fundamentals, introducing another
term not used elsewhere seems undesirable.

Observations:
We are referencing a disambiguation page.  We never actually spell out ACL
anywhere so we might as well just reference what Wikipedia believes is the
expected spelling.

The page we link to uses "permissions" while we consistently use
"privileges" to describe the contents of the list.  This seems like an
obvious synonym, but as the point of these is to formally define things,
pointing this equivalence is worth considering.

David J.

Re: [PATCH] Add ACL (Access Control List) acronym

[David G. Johnston]

On Mon, Jun 24, 2024 at 1:49 PM Joel Jacobson  wrote:

> How about?
>
> + 
> +  The linked page uses "permissions" while we consistently use the
> synonym
> +  "privileges", to describe the contents of the list. For avoidance of
> +  doubt and clarity, these two terms are equivalent in the
> +  PostgreSQL documentation.
> + 
>
> /Joel


I really dislike "For avoidance of doubt and clarity" - and in terms of
being equivalent the following seems like a more accurate description of
reality.

The PostgreSQL documentation, and code, refers to the specifications within
the ACL as "privileges".  This has the same meaning as "permissions" on the
linked page.  Generally if we say "permissions" we are referring to
something that is not covered by the ACL.  In routine communication the two
words are often used interchangeably.

David J.

Re: improve predefined roles documentation

[David G. Johnston]

On Mon, Jun 24, 2024 at 2:53 PM Nathan Bossart 
wrote:

> On Mon, Jun 24, 2024 at 02:44:33PM -0400, Robert Haas wrote:
>
> > I don't know what to do about pg_database_owner. I almost wonder if
> > that should be moved out of the table and documented as a special
> > case. Or maybe some more wordsmithing would add clarity. Or maybe it's
> > fine as-is.
>
> I've left it alone for now.  I thought about adding something like
> "pg_database_owner does not provide any special capabilities or access
> out-of-the-box" to the beginning of the entry, but I don't have time at the
> moment to properly wordsmith the rest.  If anyone else wants to give it a
> try before I get to it (probably tomorrow), please be my guest.
>

This feels like a case where why is more important than what, so here's my
first draft suggestion.

pg_database_owner owns the initially created public schema and has an
implicit membership list of one - the role owning the connected-to database.
It exists to encourage and facilitate best practices regarding database
administration.  The primary rule being to avoid using superuser to own or
do things.  The bootstrap superuser thus should connect to the postgres
database and create a login role, with the createdb attribute, and then use
that role to create and administer additional databases.  In that context,
this feature allows the creator of the new database to log into it and
immediately begin working in the public schema.

As a result, in version 14, PostgreSQL no longer initially grants create
and usage privileges, on the public schema, to the public pseudo-role.

For technical reasons, pg_database_owner may not participate in explicitly
granted role memberships.  This is an easily mitigated limitation since the
role that owns the database may be a group and any inheriting members of
that group will be considered owners as well.

David J.

Re: Should we document how column DEFAULT expressions work?

[David G. Johnston]

On Tuesday, June 25, 2024, James Coleman  wrote:

> Hello,
>
> It's possible I'm the only one who's been in this situation, but I've
> multiple times found myself explaining to a user how column DEFAULT
> expressions work: namely how the quoting on an expression following
> the keyword DEFAULT controls whether or not the expression is
> evaluated at the time of the DDL statement or at the time of an
> insertion.
>

I don’t know if it’s worth documenting but the following sentence is
implied by the syntax:

“Do not single quote the expression as a whole.  Write the expression as
you would in a select query.”

David J.

Re: Unable parse a comment in gram.y

[David G. Johnston]

On Saturday, June 22, 2024, Tatsuo Ishii  wrote:

> >>> * As func_expr but does not accept WINDOW functions directly (they
> >>> * can still be contained in arguments for functions etc.)
> >
> >> The "but" is required, add a comma before it.  It could also be written
> a
> >> bit more verbosely:
> >
> > Perhaps s/As func_expr/Like func_expr/ would be less confusing?
>
> +1. It's easier to understand at least for me.
>
>
+1

David J.

Doc Rework: Section 9.16.13 SQL/JSON Query Functions

[David G. Johnston]

Hey!

Lots of SQL/JSON threads going about.  This one is less about technical
correctness and more about usability of the documentation.  Though in
writing this I am finding some things that aren't quite clear.  I'm going
to come back with those on a follow-on post once I get a chance to make my
second pass on this.  But for the moment just opening it up to a content
and structure review.

Please focus on the text changes.  It passes "check-docs" but I still need
to work on layout and stuff in html (markup, some more links).

Thanks!

David J.

p.s. v1 exists here (is just the idea of using basically variable names in
the function signature and minimizing direct syntax in the table);

https://www.postgresql.org/message-id/CAKFQuwbYBvUZasGj_ZnfXhC2kk4AT%3DepwGkNd2%3DRMMVXkfTNMQ%40mail.gmail.com


v2-0001-doc-json-query_9-16-3.patch
Description: Binary data

Re: Should we document how column DEFAULT expressions work?

[David G. Johnston]

On Sun, Jun 30, 2024 at 7:52 PM David Rowley  wrote:

> If that's the case, maybe a tiny step towards what Peter proposed is
> just to make trailing punctuation fail for timestamp special values in
> v18.
>
>
I'm game. If anyone is using the ambiguous spelling it is probably to their
benefit to have it break and realize they wanted a function expression, not
a constant expression.

David J.

Re: Should we document how column DEFAULT expressions work?

[David G. Johnston]

On Sun, Jun 30, 2024 at 5:47 PM David Rowley  wrote:

> On Mon, 1 Jul 2024 at 12:16, David G. Johnston
>  wrote:
> >
> > On Sun, Jun 30, 2024 at 4:55 PM David Rowley 
> wrote:
> >>
> >>
> >> I'd like to know what led someone down the path of doing something
> >> like DEFAULT 'now()'::timestamp in a CREATE TABLE. Could it be a
> >> faulty migration tool that created these and people copy them thinking
> >> it's a legitimate syntax?
> >>
> >
> > My thought process on this used to be:  Provide a text string of the
> expression that is then stored within the catalog and eval'd during
> runtime.  If the only thing you are providing is a single literal and not
> some compound expression it isn't that obvious that you are supposed to
> provide an unquoted expression - which feels like it should be immediately
> evaluated - versus something that is a constant.  Kinda like dynamic SQL.
>
> Thanks for sharing that.  Any idea where that thinking came from?
>
> Maybe it was born from the fact that nothing complains when you do:
> 'now()'::timestamp? A quick test evaluation of that with a SELECT
> statement might trick someone into thinking it'll work.


> I wonder if there's anything else like this that might help fool
> people into thinking this is some valid way of getting delayed
> evaluation.
>
>
I presume the relatively new atomic SQL functions pose a similar hazard.

It probably boils down, for me, that I learned about, though never used,
eval functions from javascript, and figured this is probably implemented
something like that and I should thus supply a string.  Internalizing that
DDL can treat the unquoted content of expression in "DEFAULT expression" as
basically text hadn't happened; nor that the actual difference between just
treating it as text and the parsing to a standard form that really happens,
is quite important.  Namely that, in reverse of expectations, quoted
things, which are literals, are transformed to their typed values during
parse while functions, which are not quoted, don't have a meaningfully
different parsed form and are indeed executed at runtime.

The fact that 'now()'::timestamp fails to fail doesn't help...

Consider this phrasing for default:

The DEFAULT clause assigns a default data value for the column whose column
definition it appears within.  The expression is parsed according to
Section X.X.X, with the limitation that it may neither include references
to other columns nor subqueries, and then stored for later evaluation of
any functions it contains.  The data type of the default expression must
match the data type of the column.

Then in Section X.X.X we note, in part:
During parsing, all constants are immediately converted to their internal
representation.  In particular, the time-related literals noted in Section
8.5.1.4 get set to their date/time values.

Then, in 8.5.1.4 we should call out:
Caution:
'now' is a special time value, evaluated during parsing.
now() is a function, evaluated during execution.
'now()' is a special time value due to the quoting, PostgreSQL ignored the
parentheses.


The above doesn't make the special constants particularly special in how
they behave within parse-bind-execute while still noting that what they do
during parsing is a bit unique since a timestamp has not representation of
'tomorrow' that is can hold but instead is a short-hand for writing the
constant representing "whatever tomorrow is" at that moment.

I hope the reason for the additional caution in this framing is intuitive
for everyone.

There is probably a good paragraph or two that could be added under the new
Section X.X.X to centralize this for views, atomic sql, defaults, etc... to
refer to and give the reader the needed framing.

David J.

Re: Should we document how column DEFAULT expressions work?

[David G. Johnston]

On Sun, Jun 30, 2024 at 4:55 PM David Rowley  wrote:

>
> I'd like to know what led someone down the path of doing something
> like DEFAULT 'now()'::timestamp in a CREATE TABLE. Could it be a
> faulty migration tool that created these and people copy them thinking
> it's a legitimate syntax?
>
>
My thought process on this used to be:  Provide a text string of the
expression that is then stored within the catalog and eval'd during
runtime.  If the only thing you are providing is a single literal and not
some compound expression it isn't that obvious that you are supposed to
provide an unquoted expression - which feels like it should be immediately
evaluated - versus something that is a constant.  Kinda like dynamic SQL.

David J.

Re: Should we document how column DEFAULT expressions work?

[David G. Johnston]

On Sun, Jun 30, 2024 at 7:52 PM David Rowley  wrote:

> On Mon, 1 Jul 2024 at 13:41, David G. Johnston
>  wrote:
> > I presume the relatively new atomic SQL functions pose a similar hazard.
>
> Do you have an example of this?
>
>
create function testnow() returns timestamptz language sql
return 'now'::timestamptz;

select testnow();
select pg_sleep(5);
select testnow(); -- same time as the first call

Which conforms with the documentation and expression parsing rules for
literals:

"This form is parsed at function definition time, the string constant form
is parsed at execution time;..."

David J.

Re: Is missing LOGIN Event on Trigger Firing Matrix ?

[David G. Johnston]

On Thu, Jun 27, 2024 at 12:39 PM Andrew Dunstan  wrote:

> On 2024-06-27 Th 2:40 PM, Marcos Pegoraro wrote:
>
> create event trigger ... on login is now available but it is not shown on
> DOCs or is it in another page ?
>
> https://www.postgresql.org/docs/devel/event-trigger-matrix.html
> doesn't have it, should be there ?
>
>
> It's not triggered by a statement. But see here:
>
>
>
> https://www.postgresql.org/docs/devel/event-trigger-database-login-example.html
>
>
>
I suggest adding a sentence and link from the main description [1] to that
page.

We already do that for the ones that have commands:
"For a complete list of commands supported by the event trigger mechanism,
see Section 38.2."

and login deserves something similar on that page.

David J.

[1] https://www.postgresql.org/docs/devel/event-trigger-definition.html

Re: Custom type's modifiers

[David G. Johnston]

On Thu, Jun 27, 2024 at 8:49 AM Marthin Laubscher 
wrote:

>
> I suppose when a cast is involved it goes via the external format as well,
> right?
>

A cast between two types is going to accept a concrete instance of the
input type, in memory, as its argument and then produces a concrete
instance of the output type, in memory, as output.  If the input data is
serialized the constructor for the input type will handle deserialization.

See: create cast

https://www.postgresql.org/docs/current/sql-createcast.html

In particular the phrasing: identical to or binary-coercible to

David J.

Re: Should we document how column DEFAULT expressions work?

[David G. Johnston]

On Tue, Jun 25, 2024 at 4:11 PM Tom Lane  wrote:

> James Coleman  writes:
> > On Tue, Jun 25, 2024 at 4:59 PM Tom Lane  wrote:
> >> Uh ... what?  I recall something about that with respect to certain
> >> features such as nextval(), but you're making it sound like there
> >> is something generic going on with DEFAULT.
>
> > Hmm, I guess I'd never considered anything besides cases like
> > nextval() and now(), but I see now that now() must also be special
> > cased (when quoted) since 'date_trunc(day, now())'::timestamp doesn't
> > work but 'now()'::timestamp does.
>
> Hmm, both of those behaviors are documented, but not in the same place
> and possibly not anywhere near where you looked for info about
> DEFAULT.  For instance, the Tip at the bottom of section 9.9.5
>
>
> https://www.postgresql.org/docs/current/functions-datetime.html#FUNCTIONS-DATETIME-CURRENT
>
> explains about how 'now'::timestamp isn't what to use in DEFAULT.
>
>
I'd suggest adding to:

DEFAULT default_expr
The DEFAULT clause assigns a default data value for the column whose column
definition it appears within. The value is any variable-free expression (in
particular, cross-references to other columns in the current table are not
allowed). Subqueries are not allowed either. The data type of the default
expression must match the data type of the column.

The default expression will be used in any insert operation that does not
specify a value for the column. If there is no default for a column, then
the default is null.

+ Be aware that the [special timestamp values 1] are resolved immediately,
not upon insert.  Use the [date/time constructor functions 2] to produce a
time relative to the future insertion.

[1]
https://www.postgresql.org/docs/current/datatype-datetime.html#DATATYPE-DATETIME-SPECIAL-VALUES
[2]
https://www.postgresql.org/docs/current/functions-datetime.html#FUNCTIONS-DATETIME-CURRENT

David J.

Re: Should we document how column DEFAULT expressions work?

[David G. Johnston]

On Tue, Jun 25, 2024 at 10:12 PM Tom Lane  wrote:

> David Rowley  writes:
> > If people don't properly understand these special timestamp input
> > values, then maybe the documentation in [1] needs to be improved.  At
> > the moment the details are within parentheses. Namely "(In particular,
> > now and related strings are converted to a specific time value as soon
> > as they are read.)".  Maybe it would be better to be more explicit
> > there and mention that these are special values that the input
> > function understands which are translated to actual timestamp values
> > when the type's input function is called.  That could maybe be tied
> > into the DEFAULT clause documentation to mention that the input
> > function for constant values is called at DML time rather than DDL
> > time.  That way, we're not adding these (unsustainable) special cases
> > to the documentation.
>
> This sounds like a reasonable approach to me for the
> magic-input-values issue.  Do we want to do anything about
> nextval()?  I guess if you hold your head at the correct
> angle, that's also a magic-input-value issue, in the sense
> that the question is when does regclass input get resolved.
>
>
>From observations we transform constants into the: " 'value'::type " syntax
which then makes it an operator resolved at execution time.  For every type
except time types the transformation leaves the constant as-is.  The
special time values are the exception whereby they get evaluated to a
specific time during the transformation.

postgres=# create table tser3 (id integer not null default nextval(regclass
'tser2_id_seq'));
CREATE TABLE
postgres=# \d tser3
Table "public.tser3"
 Column |  Type   | Collation | Nullable |  Default

+-+---+--+---
 id | integer |   | not null | nextval('tser2_id_seq'::regclass)

I cannot figure out how to get "early binding" into the default. I.e.,
nextval(9000)

Since early binding is similar to the special timestamp behavior I'd say
nextval is behaving just as expected - literal transform, no evaluation.
We need only document the transforms that also evaluate.

David J.

Re: [PATCH] Add ACL (Access Control List) acronym

[David G. Johnston]

On Tue, Jun 25, 2024 at 5:30 PM Michael Paquier  wrote:

> On Tue, Jun 25, 2024 at 11:55:03AM -0500, Nathan Bossart wrote:
> > On Tue, Jun 25, 2024 at 08:10:24AM +0200, Joel Jacobson wrote:
> > > On Tue, Jun 25, 2024, at 07:11, Michael Paquier wrote:
> > >> v1 is fine without the "privileges list" part mentioned by Nathan in
> > >> the first reply.
> > >
> > > v2 is exactly that, but renamed and attached, so we have an entry this
> > > was the last version.
> >
> > LGTM
>
> Fine by me as well.  I guess I'll just apply that once v18 opens up.
>
> Fine by me.  We aren't consistent enough about all this to try and be
authoritative.

Though there was no comment on the fact we should be linking to:

https://en.wikipedia.org/wiki/Access-control_list

not:

https://en.wikipedia.org/wiki/Access_Control_List

to avoid the dis-ambiguation redirect.

If we are making wikipedia our authority we might as well use their
standard for naming.

David J.

Re: improve predefined roles documentation

[David G. Johnston]

On Tue, Jun 25, 2024 at 1:19 PM Nathan Bossart 
wrote:

> On Tue, Jun 25, 2024 at 04:04:03PM -0400, Robert Haas wrote:
> > Looking at this again, how happy are you with the way you've got
> > several roles per  instead of one for each? I realize
> > that was probably part of the intent of the change, to move the data
> > from below the table into the table, and I see the merit of that. But
> > one of your other complaints was the entries in the table were
> > unordered, and it's hard for them to really be ordered if you have
> > groups like this, since you can't alphabetize, for example, unless you
> > have just a single entry per .
>
> Yeah, my options were to either separate the roles or to weaken the
> ordering, and I guess I felt like the weaker ordering was slightly less
> bad.  The extra context in some of the groups seemed worth keeping, and
> this probably isn't the only page of our docs that might require ctrl+f.
> But I'll yield to the majority opinion here.
>
>
There are few enough that logical grouping instead of strict alphabetical
makes sense.

v4 WFM

David J.