Re: [DOCS] Confusing Trigger Docs.

2023-11-22 Thread Laurenz Albe
On Wed, 2023-11-22 at 14:49 -0800, Peter Geoghegan wrote:
> I don't think that your proposed wording for this is an improvement.

Well, the existing wording is impenetrable even for someone with some
PostgreSQL knowledge, like me.

Yours,
Laurenz Albe




Re: [DOCS] Confusing Trigger Docs.

2023-11-22 Thread Peter Geoghegan
On Wed, Nov 22, 2023 at 1:13 PM Bruce Momjian  wrote:
> I think I found out what it trying to say by looking at the INSERT
> manual page:
>
> Note that the effects of all per-row BEFORE
> INSERT triggers are reflected in
> excluded values, since those effects may
> have contributed to the row being excluded from insertion.
>
> I modified the attached patch to explain this since it is not really the
> same as modifying the actual row.  Does that add any value?  If not,
> let's remove it.

I don't understand where the confusion lies.

EXCLUDED.* is literally the row that we tried and failed to insert,
which quite naturally carries the effects of BEFORE ROW triggers. This
seems like the only behavior that makes any sense to me, even if it
can lead to confusing outcomes at times. It comes with an obvious
problem: EXCLUDED.* might not exactly match the row originally
proposed for insertion by the INSERT statement. This kind of confusion
can happen even when the table only has INSERT-type BEFORE ROW
triggers, though -- cases involving a mix of INSERT and UPDATE row
triggers are just a more elaborate version of the same confusion.

I don't think that your proposed wording for this is an improvement.
In particular, "it is possible for both row-level BEFORE INSERT and
BEFORE UPDATE triggers to be executed on the same row" leaves me
wondering what "the same row" refers to. Is it the row as it was prior
to the BEFORE triggers ran, after they ran, or something else? Or is
it more like the execution context of a single row?

-- 
Peter Geoghegan




Re: [DOCS] Confusing Trigger Docs.

2023-11-22 Thread David G. Johnston
On Wed, Nov 22, 2023 at 2:13 PM Bruce Momjian  wrote:

> On Wed, Nov 22, 2023 at 10:31:25AM +0100, Laurenz Albe wrote:
> > I agree that the paragraph you are trying to improve needs it.
> >
> > I am not sure about that last sentence you added:
> >
> >   The modification of
> >   EXCLUDED columns has similar interactions.
> >
> > How do you modify an EXCLUDED column?  Are you talking about a BEFORE
> > INSERT trigger?  Reading the original text, I get the impression that
> > it means "the behavior is obvious if you modify a column that is used
> > with EXCLUDED in the DO UPDATE clause, but it can also happen if that
> > column is not user with EXCLUDED".
> >
> > Perhaps you should omit that sentence for clarity.
>
> I think I found out what it trying to say by looking at the INSERT
> manual page:
>
> Note that the effects of all per-row BEFORE
> INSERT triggers are reflected in
> excluded values, since those effects may
> have contributed to the row being excluded from insertion.
>
> I modified the attached patch to explain this since it is not really the
> same as modifying the actual row.  Does that add any value?  If not,
> let's remove it.
>
>
There is too much exposition drowning out the main purpose here which is to
explain how the dual trigger situation introduced with on conflict gets
handled.  The following is a more direct approach.

If an insert command contains an on conflict do update clause, before
insert row triggers will be
applied to the proposed row before conflict detection.
If the update branch is taken, before update row triggers will also be
applied.
Either an insert or an update after row trigger will fire for each row.
Before statement triggers fire for insertions first and then for updates,
while
after statement triggers fire in the reverse order, updates and then
inserts.
Statement triggers fire regardless if any rows were actually inserted or
updated.


Tangentially, having the partition table content between this and the merge
content seems odd.  There also seems to be room to integrate this and merge
a bit better but that is beyond what I want to try right now.

David J.


Re: [DOCS] Double negation in parameter description

2023-11-22 Thread Bruce Momjian
On Wed, Feb  8, 2017 at 03:27:43PM +, a...@robillo.net wrote:
> The following documentation comment has been logged on the website:
> 
> Page: 
> https://www.postgresql.org/docs/9.6/static/sql-refreshmaterializedview.html
> Description:
> 
> In the description of the CONCURRENTLY parameter (3rd line/paragraph, the
> statement employs double negation which makes it hard to read and
> understand.
> 
> This option may not be used when the materialized view is not already
> populated.
> 
> I guess the author would want to mean: 
> 
> This option may only be used on a populated materialized view.?
> 
> Your clarification is much appreciated.

I know this is six years old, but fixed in master with the attached
patch.

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

  Only you can decide what is important to you.
diff --git a/doc/src/sgml/ref/refresh_materialized_view.sgml b/doc/src/sgml/ref/refresh_materialized_view.sgml
index 675d6090f3..7a019162c3 100644
--- a/doc/src/sgml/ref/refresh_materialized_view.sgml
+++ b/doc/src/sgml/ref/refresh_materialized_view.sgml
@@ -67,7 +67,7 @@ REFRESH MATERIALIZED VIEW [ CONCURRENTLY ] nameWHERE clause.
  
  
-  This option may not be used when the materialized view is not already
+  This option can only be used when the materialized view is already
   populated.
  
  


Re: [DOCS] Confusing Trigger Docs.

2023-11-22 Thread Bruce Momjian
On Wed, Nov 22, 2023 at 10:31:25AM +0100, Laurenz Albe wrote:
> I agree that the paragraph you are trying to improve needs it.
> 
> I am not sure about that last sentence you added:
> 
>   The modification of
>   EXCLUDED columns has similar interactions.
> 
> How do you modify an EXCLUDED column?  Are you talking about a BEFORE
> INSERT trigger?  Reading the original text, I get the impression that
> it means "the behavior is obvious if you modify a column that is used
> with EXCLUDED in the DO UPDATE clause, but it can also happen if that
> column is not user with EXCLUDED".
> 
> Perhaps you should omit that sentence for clarity.

I think I found out what it trying to say by looking at the INSERT
manual page:

Note that the effects of all per-row BEFORE
INSERT triggers are reflected in
excluded values, since those effects may
have contributed to the row being excluded from insertion.

I modified the attached patch to explain this since it is not really the
same as modifying the actual row.  Does that add any value?  If not,
let's remove it.

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

  Only you can decide what is important to you.
diff --git a/doc/src/sgml/trigger.sgml b/doc/src/sgml/trigger.sgml
new file mode 100644
index 6e1f370..e3ec784
*** a/doc/src/sgml/trigger.sgml
--- b/doc/src/sgml/trigger.sgml
***
*** 140,160 
 
  
 
! If an INSERT contains an ON CONFLICT
! DO UPDATE clause, it is possible that the effects of
! row-level BEFORE INSERT triggers and
! row-level BEFORE UPDATE triggers can
! both be applied in a way that is apparent from the final state of
! the updated row, if an EXCLUDED column is referenced.
! There need not be an EXCLUDED column reference for
! both sets of row-level BEFORE triggers to execute,
! though.  The
! possibility of surprising outcomes should be considered when there
! are both BEFORE INSERT and
! BEFORE UPDATE row-level triggers
! that change a row being inserted/updated (this can be
! problematic even if the modifications are more or less equivalent, if
! they're not also idempotent).  Note that statement-level
  UPDATE triggers are executed when ON
  CONFLICT DO UPDATE is specified, regardless of whether or not
  any rows were affected by the UPDATE (and
--- 140,160 
 
  
 
! If an INSERT contains an ON
! CONFLICT DO UPDATE clause, it is possible for both
! row-level BEFORE INSERT and
! BEFORE UPDATE triggers to be
! executed on the same row.  Surprising outcomes are possible when
! they change rows being inserted/updated; this can happen even if
! the modifications are more or less equivalent, if they're not also
! idempotent.  BEFORE INSERT
! triggers can also affect the value of EXCLUDED
! columns that are seen by BEFORE
! UPDATE triggers.
!
! 
!
! Note that statement-level
  UPDATE triggers are executed when ON
  CONFLICT DO UPDATE is specified, regardless of whether or not
  any rows were affected by the UPDATE (and


Re: [DOCS] Add example about date ISO format

2023-11-22 Thread David G. Johnston
On Wed, Nov 22, 2023 at 12:26 PM Bruce Momjian  wrote:

> On Wed, Nov 22, 2023 at 06:26:45PM +0100, Álvaro Herrera wrote:
> > On 2023-Nov-22, Laurenz Albe wrote:
> >
> > > I think the example had best be at "8.5.2. Date/Time Output", in
> > > doc/src/sgml/datatype.sgml around line 2552.
> >
> > Actually, isn't that a strange location?  Chapter 8.5.2 is about the
> > datatype itself, and there's already a cross-link to Section 9.8 for
> > to_char() stuff.  Since this is to_char() that the example wants to add,
> > I think the to_char reference is a more appropriate place -- probably
> > table "9.31 to_char Examples".
>
> I originally thought it belonged in section 9.8 too, but I think the
> value of this example is ISO 8601 and I don't see how we can cleanly
> mention that in table 9.31.
>
>
Most of our tables have description columns, we could add one here.  Or
I've seen us use footnote superscripts before in a table then add the
footnote text after the end of the table.

I'm against incorporating this material into the data types in Chapter 8.

David J.


Re: [DOCS] Add example about date ISO format

2023-11-22 Thread Bruce Momjian
On Wed, Nov 22, 2023 at 06:26:45PM +0100, Álvaro Herrera wrote:
> On 2023-Nov-22, Laurenz Albe wrote:
> 
> > I think the example had best be at "8.5.2. Date/Time Output", in
> > doc/src/sgml/datatype.sgml around line 2552.
> 
> Actually, isn't that a strange location?  Chapter 8.5.2 is about the
> datatype itself, and there's already a cross-link to Section 9.8 for
> to_char() stuff.  Since this is to_char() that the example wants to add,
> I think the to_char reference is a more appropriate place -- probably
> table "9.31 to_char Examples".

I originally thought it belonged in section 9.8 too, but I think the
value of this example is ISO 8601 and I don't see how we can cleanly
mention that in table 9.31.

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

  Only you can decide what is important to you.




Re: [DOCS] Add example about date ISO format

2023-11-22 Thread Alvaro Herrera
On 2023-Nov-22, Laurenz Albe wrote:

> I think the example had best be at "8.5.2. Date/Time Output", in
> doc/src/sgml/datatype.sgml around line 2552.

Actually, isn't that a strange location?  Chapter 8.5.2 is about the
datatype itself, and there's already a cross-link to Section 9.8 for
to_char() stuff.  Since this is to_char() that the example wants to add,
I think the to_char reference is a more appropriate place -- probably
table "9.31 to_char Examples".


(While scrolling the 9.6 version of this page[1] I noticed that, in dark
mode, the  box becomes unreadable because of white text on
yellowish background.  Not sure what's an appropriate fix for that, if
any; current versions don't have that problem.  Maybe it's better to
leave it alone.)

[1] https://www.postgresql.org/docs/9.6/functions-formatting.html

-- 
Álvaro Herrera   48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"Porque francamente, si para saber manejarse a uno mismo hubiera que
rendir examen... ¿Quién es el machito que tendría carnet?"  (Mafalda)




Re: [DOCS] Add example about date ISO format

2023-11-22 Thread Erik Wienhold
On 2023-11-22 17:58 +0100, Bruce Momjian wrote:
> On Wed, Nov 22, 2023 at 02:02:02PM +0100, Erik Wienhold wrote:
> > > > + 
> > > > +  
> > > > +to_char(current_timestamp AT TIME ZONE 'UTC',
> > > > +'-MM-DD"T"HH24:MI:SSZ') outputs the current UTC
> > 
> > This might be excessive, but should we have an example with other time
> > zones?  ISO 8601 is not limited to UTC.  For example:
> > -MM-DD"T"HH24:MI:SSOF or -MM-DD"T"HH24:MI:SSTZH:TZM
> > 
> > Fractional seconds are also possible: -MM-DD"T"HH24:MI:SS,FF6
> 
> Uh, I think the goal was to show how to output ISO 8601 output with "T".
> I assume they can figure out how to customize that.

Fair point.

> > > > +date/time in ISO 8601 date/time format.
> > > > +  
> > > > + 
> > > > +
> > > >  
> > > > 
> > > >  
> > > 
> > > +1 on the idea, but from the context it looks like you added that example
> > > at the regular expression matching functions.
> > > 
> > > I think the example had best be at "8.5.2. Date/Time Output", in
> > > doc/src/sgml/datatype.sgml around line 2552.
> > 
> > +1 for moving it to section 8.5.2.
> 
> Okay, I moved it into the "Note" section that talked about ISO 8601
> output with "T", in the attached patch.
> 
> I will apply this only to master since it is not a correction.

LGTM.

-- 
Erik




Re: [DOCS] Add example about date ISO format

2023-11-22 Thread Bruce Momjian
On Wed, Nov 22, 2023 at 02:02:02PM +0100, Erik Wienhold wrote:
> > > + 
> > > +  
> > > +to_char(current_timestamp AT TIME ZONE 'UTC',
> > > +'-MM-DD"T"HH24:MI:SSZ') outputs the current UTC
> 
> This might be excessive, but should we have an example with other time
> zones?  ISO 8601 is not limited to UTC.  For example:
> -MM-DD"T"HH24:MI:SSOF or -MM-DD"T"HH24:MI:SSTZH:TZM
> 
> Fractional seconds are also possible: -MM-DD"T"HH24:MI:SS,FF6

Uh, I think the goal was to show how to output ISO 8601 output with "T".
I assume they can figure out how to customize that.

> > > +date/time in ISO 8601 date/time format.
> > > +  
> > > + 
> > > +
> > >  
> > > 
> > >  
> > 
> > +1 on the idea, but from the context it looks like you added that example
> > at the regular expression matching functions.
> > 
> > I think the example had best be at "8.5.2. Date/Time Output", in
> > doc/src/sgml/datatype.sgml around line 2552.
> 
> +1 for moving it to section 8.5.2.

Okay, I moved it into the "Note" section that talked about ISO 8601
output with "T", in the attached patch.

I will apply this only to master since it is not a correction.

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

  Only you can decide what is important to you.
diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml
index e4a7b07033..4943f63871 100644
--- a/doc/src/sgml/datatype.sgml
+++ b/doc/src/sgml/datatype.sgml
@@ -2472,7 +2472,11 @@ TIMESTAMP WITH TIME ZONE '2004-10-19 10:23:54+02'
   input, but on output it uses a space rather than T, as shown
   above.  This is for readability and for consistency with
   https://tools.ietf.org/html/rfc3339;>RFC 3339 as
-  well as some other database systems.
+  well as some other database systems. The function call
+  to_char(current_timestamp AT TIME ZONE 'UTC',
+  '-MM-DD"T"HH24:MI:SSZ') outputs the current
+  UTC date/time in ISO 8601 format with
+  T.
  
 
 


Re: [DOCS] Add example about date ISO format

2023-11-22 Thread Erik Wienhold
On 2023-11-22 10:14 +0100, Laurenz Albe wrote:
> On Tue, 2023-11-21 at 23:33 -0500, Bruce Momjian wrote:
> > On Fri, Feb 17, 2017 at 04:01:54PM +, juha.musto...@iki.fi wrote:
> > > The following documentation comment has been logged on the website:
> > > 
> > > Page: https://www.postgresql.org/docs/9.6/static/functions-formatting.html
> > > Description:
> > > 
> > > The documentation should include an example how to format datetime entry
> > > into most commonly known ISO format. This is a bit tricky as literal
> > > character needs to included with quotes:
> > > 
> > > to_char(NOW(), -MM-DDTHH24:MI:SSZ)
> > 
> > I know this is a six-year-old idea, but it is still a good one.  I have
> > developed the attached patch I would like to apply to master.
> > 
> > diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
> > index 93f068edcf..297cafb341 100644
> > --- a/doc/src/sgml/func.sgml
> > +++ b/doc/src/sgml/func.sgml
> > @@ -8489,6 +8489,14 @@ SELECT regexp_match('abc01234xyz', 
> > '(?:(.*?)(\d+)(.*)){1,1}');
> >
> >   
> >  
> > + 
> > +  
> > +to_char(current_timestamp AT TIME ZONE 'UTC',
> > +'-MM-DD"T"HH24:MI:SSZ') outputs the current UTC

This might be excessive, but should we have an example with other time
zones?  ISO 8601 is not limited to UTC.  For example:
-MM-DD"T"HH24:MI:SSOF or -MM-DD"T"HH24:MI:SSTZH:TZM

Fractional seconds are also possible: -MM-DD"T"HH24:MI:SS,FF6

> > +date/time in ISO 8601 date/time format.
> > +  
> > + 
> > +
> >  
> > 
> >  
> 
> +1 on the idea, but from the context it looks like you added that example
> at the regular expression matching functions.
> 
> I think the example had best be at "8.5.2. Date/Time Output", in
> doc/src/sgml/datatype.sgml around line 2552.

+1 for moving it to section 8.5.2.

-- 
Erik




Re: [DOCS] Confusing Trigger Docs.

2023-11-22 Thread Laurenz Albe
On Tue, 2023-11-21 at 21:01 -0500, Bruce Momjian wrote:
> On Thu, Aug 31, 2017 at 09:22:22AM -0700, Peter Geoghegan wrote:
> > On Thu, Aug 31, 2017 at 6:25 AM, Bruce Momjian  wrote:
> > > On Mon, Jul  3, 2017 at 08:07:10PM +, n...@fairwindsoft.com wrote:
> > > > The following documentation comment has been logged on the website:
> > > > 
> > > > Page: https://www.postgresql.org/docs/9.6/static/trigger-definition.html
> > > > Description:
> > > > 
> > > > https://www.postgresql.org/docs/devel/static/trigger-definition.html
> > > > 
> > > > This sentence:
> > > > 
> > > > If an INSERT contains an ON CONFLICT DO UPDATE clause, it is 
> > > > possible that
> > > > the effects of all row-level BEFORE INSERT triggers and all row-level 
> > > > BEFORE
> > > > UPDATE triggers can both be applied in a way that is apparent from the 
> > > > final
> > > > state of the updated row, if an EXCLUDED column is referenced.
> > > > 
> > > > is very hard to digest.
> > 
> > EXCLUDED.* is exactly what the name suggests -- the tuple that was not
> > inserted because of a conflict. So, naturally it has the effects of
> > any before insert trigger, and carries them forward. But you still
> > have before triggers on the update side.
> > 
> > Typically, this won't matter at all, because before triggers tend to
> > be written in an idempotent fashion -- something gets filled in. But I
> > can imagine cases where it is not idempotent, and apply a before
> > update trigger modifies the row in a way that is surprising. Just
> > because ON CONFLICT DO UPDATE was used rather than UPDATE. That's what
> > the documentation warns about.
> 
> I know this thread is six years old, but I still found it confusing, so
> the attached patch tries to simplify it.

I agree that the paragraph you are trying to improve needs it.

I am not sure about that last sentence you added:

  The modification of
  EXCLUDED columns has similar interactions.

How do you modify an EXCLUDED column?  Are you talking about a BEFORE
INSERT trigger?  Reading the original text, I get the impression that
it means "the behavior is obvious if you modify a column that is used
with EXCLUDED in the DO UPDATE clause, but it can also happen if that
column is not user with EXCLUDED".

Perhaps you should omit that sentence for clarity.

Yours,
Laurenz Albe




Re: [DOCS] intagg.sgml: example wrongly named and does not compile

2023-11-22 Thread Laurenz Albe
On Tue, 2023-11-21 at 22:27 -0500, Bruce Momjian wrote:
> I like this six year old patch so would like to apply it to master,
> attached.

+1, since it is arguably a bug fix.

Yours,
Laurenz Albe




Re: [DOCS] Add example about date ISO format

2023-11-22 Thread Laurenz Albe
On Tue, 2023-11-21 at 23:33 -0500, Bruce Momjian wrote:
> On Fri, Feb 17, 2017 at 04:01:54PM +, juha.musto...@iki.fi wrote:
> > The following documentation comment has been logged on the website:
> > 
> > Page: https://www.postgresql.org/docs/9.6/static/functions-formatting.html
> > Description:
> > 
> > The documentation should include an example how to format datetime entry
> > into most commonly known ISO format. This is a bit tricky as literal
> > character needs to included with quotes:
> > 
> > to_char(NOW(), -MM-DDTHH24:MI:SSZ)
> 
> I know this is a six-year-old idea, but it is still a good one.  I have
> developed the attached patch I would like to apply to master.

+1 on the idea, but from the context it looks like you added that example
at the regular expression matching functions.

I think the example had best be at "8.5.2. Date/Time Output", in
doc/src/sgml/datatype.sgml around line 2552.

Yours,
Laurenz Albe