Re: RFC: Allow EXPLAIN to Output Page Fault Information

2025-02-08 Thread Jelte Fennema-Nio
On Mon, 27 Jan 2025 at 10:05, torikoshia  wrote:
> Therefore, I believe it would be reasonable to report the raw values
> as-is, as they should still be useful for understanding storage I/O
> activity.

Sounds reasonable.

Below some feedback on the patch. It's all really minor. The patch
looks great. I'll play around with it a bit next week.

meta: it's confusing that this one is called v1 again, it would be
clearer if it was called v2.

nit: at line 528 the "if (es->buffers)" check can simply be merged
with the if block above (which does the exact same check)

> if (usage->inblock <= 0 && usage->outblock <= 0)
> return false;
>
> else
>return true;

nit: You can replace that if-else with:
return usage->inblock > 0 || usage->outblock > 0;

> StorageIOUsageAccumDiff(StorageIOUsage *dst, const StorageIOUsage *add, const 
> StorageIOUsage *sub)

Missing a function comment




Re: Why doesn't GiST VACUUM require a super-exclusive lock, like nbtree VACUUM?

2025-02-08 Thread Michail Nikolaev
Hello, everyone!

Just some commit messages + few cleanups.

Best regards,
Mikhail.


v8-0002-Fix-index-only-scan-race-condition-in-GiST-implem.patch
Description: Binary data


v8-0003-Improve-buffer-handling-for-killed-items-in-GiST-.patch
Description: Binary data


v8-0004-Fix-index-only-scan-race-condition-in-SP-GiST-imp.patch
Description: Binary data


v8-0001-Tests-for-index-only-scan-race-condition-with-con.patch
Description: Binary data


Re: Showing applied extended statistics in explain Part 2

2025-02-08 Thread Tomas Vondra



On 1/24/25 11:17, Andrei Lepikhov wrote:
> On 11/1/24 12:22, Tatsuro Yamada wrote:
>> Hi All,
>>
>> I apologize for not being able to continue development due to various
>> circumstances.
>> The attached file is the rebased patch.
>> I will now catch up on the discussion and try to revise the patch.
> I wonder why it’s so important to know exactly where and who has used
> extended statistics.
> 
> I often use SQL Server to compare execution plans generated by
> PostgreSQL, and I appreciate how they display the usage of extended
> statistics. They clearly identify which statistics were utilized during
> query planning and include them in the summary section of query plan.
> 
I admit not knowing what exactly SQL Server shows in the explain, but it
seems helpful to know which part of the plan used which statistics, no?
Imagine we only knew an index was used, but not which node used it and
for what keys. That would be a bit ... useless.

Or what info does the SQL server include in the plan, exactly? Can you
share an example?

> I find this method much easier to implement, as it allows us to see any
> usage points - remember that `estimate_num_groups` may be applied in
> multiple places and may not always correspond to a node clause.
> 

I may be wrong, but I don't quite see why would that be significantly
easier to implement. You still need to track the information somewhere,
and it probably needs for individual Path nodes. Because while building
the plan you don't know which paths will get selected.

Although, maybe the selectivity estimation is sufficiently independent
from the exact paths? In which case we might track it at the plan level.

Still, I don't think that really makes this much easier to implement.
The code would only move a little bit to a different place, but other
than that it would remain the same.

> It’s worth noting that some clauses may be transformed during the
> planning process, and the scan filter may not align with the estimated
> clause. It’s possible that certain clauses might not appear in the final
> estimated plan based on the extended statistics.
> 
> If necessary, we could add an `extstat_ID` to the summary to reference
> it in the plan nodes.
> 

Not sure, but I'd prefer not to add "indirection" the people would have
to follow in the explain plan. I'd much rather duplicate the same info
(which should be rare anyway, we usually don't have the same statistics
used in multiple places in one query).

The point about estimate_num_groups is good - I think there will be more
cases where linking the extended statistics to a clause will be hard.
But maybe let's not block the whole patch because of that?


regards

-- 
Tomas Vondra





Re: proposal - plpgsql - support standard syntax for named arguments for cursors

2025-02-08 Thread Tom Lane
Pavel Stehule  writes:
> I propose to enhancing to ANSI/SQL standard syntax for named arguments
> `argname => value`

Is there any reason to think that that's actually in the standard?
I poked around in SQL:2021 a little and couldn't find anything about
cursors with arguments at all.

regards, tom lane




Improve pgindent exclude handling: ignore empty lines

2025-02-08 Thread Zsolt Parragi
Hello,

We ran into an issue where pgindent stopped reformatting anything with
our custom exclude file, and after some investigation we found the
empty line accidentally inserted into the exclude file.

Pgindent currently treats empty lines as valid exclusions and creates
an empty regex from them. The empty regex matches any pattern,
resulting in ignoring everything.

As this behavior doesn't seem to be useful in practice, and it is easy
to reproduce accidentally (it works even at the end of the file), I
propose a patch that ignores empty lines in the exclude file.

If somebody really wants to ignore everything, that is still possible
with more explicit patterns like ".*"


0001-Improve-pgindent-exclude-handling-ignore-empty-lines.patch
Description: Binary data


Re: proposal - plpgsql - support standard syntax for named arguments for cursors

2025-02-08 Thread Japin Li
On Sat, 08 Feb 2025 at 16:34, Julien Rouhaud  wrote:
> Hi,
>
> On Sat, Feb 08, 2025 at 07:47:23AM +0100, Pavel Stehule wrote:
>> Hi
>>
>> when I worked on strict expr check patch I found so syntax for named
>> arguments of cursors supports only our legacy  proprietary syntax `argname
>> := value`
>>
>> https://www.postgresql.org/docs/current/plpgsql-cursors.html
>>
>> I propose to enhancing to ANSI/SQL standard syntax for named arguments
>> `argname => value`
>
> Seems sensible to me.
>
>> The patch is almost trivial
>
> Documentation and tests are updated, and the patch LGTM.

Maybe we should also update the comments?

diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index 867017d8ed9..43186c8e85e 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -3911,7 +3911,7 @@ read_cursor_args(PLpgSQL_var *cursor, int until, YYSTYPE 
*yylvalp, YYLTYPE *yyll
tok2;
int arglocation;
 
-   /* Check if it's a named parameter: "param := value" */
+   /* Check if it's a named parameter: "param := value" or "param 
=> value" */
plpgsql_peek2(&tok1, &tok2, &arglocation, NULL, yyscanner);
if (tok1 == IDENT && (tok2 == COLON_EQUALS || tok2 == 
EQUALS_GREATER))
{
@@ -3939,7 +3939,7 @@ read_cursor_args(PLpgSQL_var *cursor, int until, YYSTYPE 
*yylvalp, YYLTYPE *yyll
 parser_errposition(*yyllocp)));
 
/*
-* Eat the ":=". We already peeked, so the error should 
never
+* Eat the ":=" and "=>". We already peeked, so the 
error should never
 * happen.
 */
tok2 = yylex(yylvalp, yyllocp, yyscanner);
-- 
Regrads,
Japin Li




Re: pgbench with partitioned tables

2025-02-08 Thread Álvaro Herrera
On 2025-Feb-07, Melanie Plageman wrote:

> Okay, I've stared at this a bit, and it seems basically fine the way
> it is (I might add a bit more whitespace, clean up the commit message,
> etc). So I'm interested in committing it. I will admit that having
> never committed anything to pgbench, I'm a bit nervous. So, give me a
> couple days to work up the nerve.

Sounds good.  I only wanted to say that I would use PQexecParams()
instead of PQexec() in the new function, avoiding the psprintf and
pfree, and also that initializing relkind to RELKIND_RELATION is strange
and probably unnecessary.

-- 
Álvaro Herrera PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Cuando mañana llegue pelearemos segun lo que mañana exija" (Mowgli)




Re: proposal - plpgsql - support standard syntax for named arguments for cursors

2025-02-08 Thread Julien Rouhaud
Hi,

On Sat, Feb 08, 2025 at 07:47:23AM +0100, Pavel Stehule wrote:
> Hi
>
> when I worked on strict expr check patch I found so syntax for named
> arguments of cursors supports only our legacy  proprietary syntax `argname
> := value`
>
> https://www.postgresql.org/docs/current/plpgsql-cursors.html
>
> I propose to enhancing to ANSI/SQL standard syntax for named arguments
> `argname => value`

Seems sensible to me.

> The patch is almost trivial

Documentation and tests are updated, and the patch LGTM.




Re: access numeric data in module

2025-02-08 Thread Ed Behn
I've created a patch (attached) to implement the changes discussed below.

This change moves the definition of the NumericVar structure to numeric.h.
Function definitions for functions used to work with NumericVar are also
moved to the header as are definitions of functions used to convert
NumericVar to Numeric. (Numeric is used to store numeric and decimal types.)

All of this is so that third-party libraries can access numeric and decimal
values without having to access the opaque Numeric structure.

There is actually no new code. Code is simply moved from numeric.c to
numeric.h.

This is a patch against branch master.

This successfully compiles and is tested with regression tests.

Also attached is a code sample that uses the change.

Please provide feedback. I'm planning to submit this for the March
commitfest.

   -Ed

On Wed, Sep 18, 2024 at 9:50 AM Robert Haas  wrote:

> On Sat, Sep 14, 2024 at 2:10 PM Ed Behn  wrote:
> > Was there a resolution of this? I'm wondering if it is worth it for
> me to submit a PR for the next commitfest.
>
> Well, it seems like what you want is different than what I want, and
> what Tom wants is different from both of us. I'd like there to be a
> way forward here but at the moment I'm not quite sure what it is.
>
> --
> Robert Haas
> EDB: http://www.enterprisedb.com
>
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 40dcbc7b67..126b7dc452 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -50,60 +50,6 @@
 #define NUMERIC_DEBUG
  */
 
-
-/* --
- * Local data types
- *
- * Numeric values are represented in a base-NBASE floating point format.
- * Each "digit" ranges from 0 to NBASE-1.  The type NumericDigit is signed
- * and wide enough to store a digit.  We assume that NBASE*NBASE can fit in
- * an int.  Although the purely calculational routines could handle any even
- * NBASE that's less than sqrt(INT_MAX), in practice we are only interested
- * in NBASE a power of ten, so that I/O conversions and decimal rounding
- * are easy.  Also, it's actually more efficient if NBASE is rather less than
- * sqrt(INT_MAX), so that there is "headroom" for mul_var and div_var to
- * postpone processing carries.
- *
- * Values of NBASE other than 1 are considered of historical interest only
- * and are no longer supported in any sense; no mechanism exists for the client
- * to discover the base, so every client supporting binary mode expects the
- * base-1 format.  If you plan to change this, also note the numeric
- * abbreviation code, which assumes NBASE=1.
- * --
- */
-
-#if 0
-#define NBASE		10
-#define HALF_NBASE	5
-#define DEC_DIGITS	1			/* decimal digits per NBASE digit */
-#define MUL_GUARD_DIGITS	4	/* these are measured in NBASE digits */
-#define DIV_GUARD_DIGITS	8
-
-typedef signed char NumericDigit;
-#endif
-
-#if 0
-#define NBASE		100
-#define HALF_NBASE	50
-#define DEC_DIGITS	2			/* decimal digits per NBASE digit */
-#define MUL_GUARD_DIGITS	3	/* these are measured in NBASE digits */
-#define DIV_GUARD_DIGITS	6
-
-typedef signed char NumericDigit;
-#endif
-
-#if 1
-#define NBASE		1
-#define HALF_NBASE	5000
-#define DEC_DIGITS	4			/* decimal digits per NBASE digit */
-#define MUL_GUARD_DIGITS	2	/* these are measured in NBASE digits */
-#define DIV_GUARD_DIGITS	4
-
-typedef int16 NumericDigit;
-#endif
-
-#define NBASE_SQR	(NBASE * NBASE)
-
 /*
  * The Numeric type as stored on disk.
  *
@@ -252,75 +198,6 @@ struct NumericData
 	 | ((n)->choice.n_short.n_header & NUMERIC_SHORT_WEIGHT_MASK)) \
 	: ((n)->choice.n_long.n_weight))
 
-/*
- * Maximum weight of a stored Numeric value (based on the use of int16 for the
- * weight in NumericLong).  Note that intermediate values held in NumericVar
- * and NumericSumAccum variables may have much larger weights.
- */
-#define NUMERIC_WEIGHT_MAX			PG_INT16_MAX
-
-/* --
- * NumericVar is the format we use for arithmetic.  The digit-array part
- * is the same as the NumericData storage format, but the header is more
- * complex.
- *
- * The value represented by a NumericVar is determined by the sign, weight,
- * ndigits, and digits[] array.  If it is a "special" value (NaN or Inf)
- * then only the sign field matters; ndigits should be zero, and the weight
- * and dscale fields are ignored.
- *
- * Note: the first digit of a NumericVar's value is assumed to be multiplied
- * by NBASE ** weight.  Another way to say it is that there are weight+1
- * digits before the decimal point.  It is possible to have weight < 0.
- *
- * buf points at the physical start of the palloc'd digit buffer for the
- * NumericVar.  digits points at the first digit in actual use (the one
- * with the specified weight).  We normally leave an unused digit or two
- * (preset to zeroes) between buf and digits, so that there is room to store
- * a carry out of the top digit without reallocating space.  We just need to
- * decrement digits (and incr

Re: Final result (display) collation?

2025-02-08 Thread Jeff Davis
On Tue, 2024-12-17 at 20:36 -0500, Tom Lane wrote:
> Jeff Davis  writes:
> > Crazy idea: what if we treated the top-level ORDER BY as special? 
> 
> This seems like an amazing kluge, and it's not even clear that
> there's any field demand for it.

I had a discussion with Vik, and he suggested something less crazy:
what if we had a "COLLATE SESSION" (or some special name) that would
mean that it should get the actual collation from a GUC? (Obviously
this would only work on expressions and not column definitions.)

That would solve one fairly annoying problem: we don't guarantee that
any particular collation actually exists (aside from the memcmp-based
ones), so it's hard to configure applications to use the COLLATE clause
at all.

The Django docs[1] reference "et-x-icu", but it's hard to know which
ones are available. We have documentation about how to construct ICU
language tags, but not about which specific language/region
combinations are available, because those comes from ICU, which doesn't
seem to document them either.

Granted, if you want to use US English, someone will have to know the
name "en-US-x-icu" or "en_US" at some point. But it's more reasonable
to expect a user doing "ALTER USER ... SET" to know the available
Postgres collations on that particular server than an application
developer who might expect it to work on any Postgres instance. And
GUCs are a much more flexible solution than an application
configuration file, or worse, hard-coded in application code[2].

Aside: we'd need to differentiate this enough from the SQL standard
"SET COLLATION", which (IIUC) affects all expressions in the query.

Regards,
Jeff Davis

[1]
https://docs.djangoproject.com/en/5.1/ref/models/database-functions/

[2]
https://stackoverflow.com/questions/60318707/how-to-use-collate-as-with-sequel-and-ruby




Fix punctuation errors in PostgreSQL documentation

2025-02-08 Thread 斉藤登
Hi,

I am a member of the project translating PostgreSQL documentation into Japanese.
For the translation, I check the number of sentences by periods.
I found some issues in the original English documentation that I
believe need to be corrected.

Could you please review the attached patch file?

Best regards,

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 38244409e3c..d3309a02798 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8134,7 +8134,7 @@ COPY postgres_log FROM
'/full/path/to/logfile.csv' WITH csv;
various purposes. The cluster name appears in the process title for
all server processes in this cluster. Moreover, it is the default
application name for a standby connection (see .)
+ linkend="guc-synchronous-standby-names"/>).


diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index e04acf1c208..c49e975b082 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -5392,7 +5392,7 @@ PGresult *PQgetResult(PGconn *conn);
PGRES_PIPELINE_SYNC will be returned.
The result of the next query after the synchronization point follows
immediately (that is, no null pointer is returned after
- the synchronization point.)
+ the synchronization point).


diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index edc2470bcf9..fcb7196364a 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -1148,7 +1148,7 @@ description | Waiting for a newly initialized
WAL file to reach durable storage


Extensions can add Extension,
- InjectionPoint. and LWLock events
+ InjectionPoint, and LWLock events
to the lists shown in  and
. In some cases, the name
of an LWLock assigned by an extension will not be
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 46240e3f725..920204a7c86 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -909,7 +909,7 @@ pgbench 
options  d
Print messages about all errors and failures (errors without retrying)
including which limit for retries was exceeded and how far it was
exceeded for the serialization/deadlock failures. (Note that in this
- case the output can be significantly increased.).
+ case the output can be significantly increased.)
See  for more information.


diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 38244409e3c..d3309a02798 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8134,7 +8134,7 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
 various purposes.  The cluster name appears in the process title for
 all server processes in this cluster.  Moreover, it is the default
 application name for a standby connection (see .)
+linkend="guc-synchronous-standby-names"/>).

 

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index e04acf1c208..c49e975b082 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -5392,7 +5392,7 @@ PGresult *PQgetResult(PGconn *conn);
PGRES_PIPELINE_SYNC will be returned.
The result of the next query after the synchronization point follows
immediately (that is, no null pointer is returned after
-   the synchronization point.)
+   the synchronization point).
   
 
   
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index edc2470bcf9..fcb7196364a 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -1148,7 +1148,7 @@ description | Waiting for a newly initialized WAL file to reach durable storage

 
  Extensions can add Extension,
- InjectionPoint. and LWLock events
+ InjectionPoint, and LWLock events
  to the lists shown in  and
  . In some cases, the name
  of an LWLock assigned by an extension will not be
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 46240e3f725..920204a7c86 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -909,7 +909,7 @@ pgbench  options  d
 Print messages about all errors and failures (errors without retrying)
 including which limit for retries was exceeded and how far it was
 exceeded for the serialization/deadlock failures. (Note that in this
-case the output can be significantly increased.).
+case the output can be significantly increased.)
 See  for more information.

   


Re: proposal - plpgsql - support standard syntax for named arguments for cursors

2025-02-08 Thread Pavel Stehule
so 8. 2. 2025 v 22:25 odesílatel Tom Lane  napsal:

> Pavel Stehule  writes:
> > so 8. 2. 2025 v 20:25 odesílatel Tom Lane  napsal:
> >> Is there any reason to think that that's actually in the standard?
>
> > I think the possibility to use named arguments in OPEN statements is a
> > PostgreSQL proprietary feature.
> > And usage of cursors in PL/pgSQL is based on PL/SQL (not on SQL/PSM from
> > standard), but named
> > arguments for cursor is PostgreSQL proprietary feature and the syntax
> based
> > on usage `:=` is our
> > proprietary too.
>
> Hmm ... yeah, it's not in SQL/PSM, but looking at PL/SQL:
>
>
> https://docs.oracle.com/en/database/oracle/oracle-database/19/lnpls/OPEN-statement.html
>
> I see
>
> You can specify actual cursor parameters with either
> positional notation or named notation. For information about
> these notations, see "Positional, Named, and Mixed Notation
> for Actual Parameters".
>
> and that link blesses the use of "name => value" (and not ":=").
> So agreed, we should adjust this.
>
> Is there a reason we need a whole new test case instead of
> tweaking one of the existing ones?
>

just aesthetic reasons - it looks strange for me to mix two styles in one
code.
But in this very simple case - it is not important.
please, modify tests how you like

Regards

Pavel


>
> regards, tom lane
>


Re: Expanding HOT updates for expression and partial indexes

2025-02-08 Thread Laurenz Albe
On Thu, 2025-02-06 at 22:24 +, Burd, Greg wrote:
> Attached find a patch that expands the cases where heap-only tuple (HOT) 
> updates are possible
> without changing the basic semantics of HOT. This is accomplished by 
> examining expression
> indexes for changes to determine if indexes require updating or not. A 
> similar approach is
> taken for partial indexes, the predicate is evaluated and, in some cases, HOT 
> updates are
> allowed.
>
> [...]
>
> Third, there is overhead to this patch, it is no longer a single simple 
> bitmap test to choose
> HOT or not in heap_update(). Sometimes this patch will perform expensive 
> additional checks
> and ultimately not go down the HOT path, new overhead with no benefit. Some 
> expressions are
> more expensive than others to evaluate, there is no logic to adjust for that. 
> The Surjective
> patch/email thread had quite a bit of discussion on this without resolution. 
> I’ve chosen to
> add a GUC that optionally avoids the expression evaluation. I’m open to ideas 
> here as well,
> addition of another GUC or removal of the one I’ve added. I’ve tried to avoid 
> rechecking
> indexes for changes when possible.

I think that the goal of this patch is interesting and desirable.

The greatest concern for me is the performance impact.  I think that a switch 
is warranted,
but I am not sure if it should be a GUC.  Wouldn't it be better to have a 
reloption, so that
this can be configured per table?  I am not sure if a global switch is 
necessary, but I am
not fundamentally against it.

Yours,
Laurenz Albe




Re: proposal - plpgsql - support standard syntax for named arguments for cursors

2025-02-08 Thread Pavel Stehule
Hi

so 8. 2. 2025 v 11:27 odesílatel Japin Li  napsal:

> On Sat, 08 Feb 2025 at 16:34, Julien Rouhaud  wrote:
> > Hi,
> >
> > On Sat, Feb 08, 2025 at 07:47:23AM +0100, Pavel Stehule wrote:
> >> Hi
> >>
> >> when I worked on strict expr check patch I found so syntax for named
> >> arguments of cursors supports only our legacy  proprietary syntax
> `argname
> >> := value`
> >>
> >> https://www.postgresql.org/docs/current/plpgsql-cursors.html
> >>
> >> I propose to enhancing to ANSI/SQL standard syntax for named arguments
> >> `argname => value`
> >
> > Seems sensible to me.
> >
> >> The patch is almost trivial
> >
> > Documentation and tests are updated, and the patch LGTM.
>
> Maybe we should also update the comments?
>
> diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
> index 867017d8ed9..43186c8e85e 100644
> --- a/src/pl/plpgsql/src/pl_gram.y
> +++ b/src/pl/plpgsql/src/pl_gram.y
> @@ -3911,7 +3911,7 @@ read_cursor_args(PLpgSQL_var *cursor, int until,
> YYSTYPE *yylvalp, YYLTYPE *yyll
> tok2;
> int arglocation;
>
> -   /* Check if it's a named parameter: "param := value" */
> +   /* Check if it's a named parameter: "param := value" or
> "param => value" */
> plpgsql_peek2(&tok1, &tok2, &arglocation, NULL, yyscanner);
> if (tok1 == IDENT && (tok2 == COLON_EQUALS || tok2 ==
> EQUALS_GREATER))
> {
> @@ -3939,7 +3939,7 @@ read_cursor_args(PLpgSQL_var *cursor, int until,
> YYSTYPE *yylvalp, YYLTYPE *yyll
>
>  parser_errposition(*yyllocp)));
>
> /*
> -* Eat the ":=". We already peeked, so the error
> should never
> +* Eat the ":=" and "=>". We already peeked, so
> the error should never
>  * happen.
>  */
> tok2 = yylex(yylvalp, yyllocp, yyscanner);
>

good idea

done

Regards

Pavel


> --
> Regrads,
> Japin Li
>
From 3eb6e29eacbe2ad1dd0d2b7847ca1eccde71594f Mon Sep 17 00:00:00 2001
From: "ok...@github.com" 
Date: Sat, 8 Feb 2025 07:45:58 +0100
Subject: [PATCH] allow to use standard syntax for named arguments for plpgsql
 cursor arguments

---
 doc/src/sgml/plpgsql.sgml |  3 ++-
 src/pl/plpgsql/src/pl_gram.y  | 13 -
 src/test/regress/expected/plpgsql.out | 19 +++
 src/test/regress/sql/plpgsql.sql  | 15 +++
 4 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 78e4983139b..60af57712b7 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -3317,7 +3317,7 @@ OPEN curs1 FOR EXECUTE format('SELECT * FROM %I WHERE col1 = $1',tabname) USING
  Opening a Bound Cursor
 
 
-OPEN bound_cursorvar  (  argument_name :=  argument_value , ... ) ;
+OPEN bound_cursorvar  (  argument_name { := | => }  argument_value , ... ) ;
 
 
  
@@ -3352,6 +3352,7 @@ OPEN bound_cursorvar  (   42);
 
  
 
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index 64d2c362bf9..618a97b64f2 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -3911,9 +3911,12 @@ read_cursor_args(PLpgSQL_var *cursor, int until, YYSTYPE *yylvalp, YYLTYPE *yyll
 	tok2;
 		int			arglocation;
 
-		/* Check if it's a named parameter: "param := value" */
+		/*
+		 * Check if it's a named parameter: "param := value"
+		 * or "param => value"
+		 */
 		plpgsql_peek2(&tok1, &tok2, &arglocation, NULL, yyscanner);
-		if (tok1 == IDENT && tok2 == COLON_EQUALS)
+		if (tok1 == IDENT && (tok2 == COLON_EQUALS || tok2 == EQUALS_GREATER))
 		{
 			char	   *argname;
 			IdentifierLookup save_IdentifierLookup;
@@ -3939,11 +3942,11 @@ read_cursor_args(PLpgSQL_var *cursor, int until, YYSTYPE *yylvalp, YYLTYPE *yyll
 		 parser_errposition(*yyllocp)));
 
 			/*
-			 * Eat the ":=". We already peeked, so the error should never
-			 * happen.
+			 * Eat the ":=" and the "=>". We already peeked, so the error should
+			 * never happen.
 			 */
 			tok2 = yylex(yylvalp, yyllocp, yyscanner);
-			if (tok2 != COLON_EQUALS)
+			if (tok2 != COLON_EQUALS && tok2 != EQUALS_GREATER)
 yyerror(yyllocp, NULL, yyscanner, "syntax error");
 
 			any_named = true;
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index 0a6945581bd..4549019f103 100644
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -2429,6 +2429,25 @@ select namedparmcursor_test9(6);
  1
 (1 row)
 
+-- check standard syntax for named parameters
+create or replace function namedparmcursor_test9(p1 int) returns int4 as $$
+declare
+  c1 cursor (p1 int, p2 int, debug int) for
+select count(*) from tenk1 where thousand = p1 and tenthous = p2
+  and four = debug;
+  p2 int4 := 1006;
+  n int4;
+

Re: Get rid of WALBufMappingLock

2025-02-08 Thread Alexander Korotkov
On Fri, Feb 7, 2025 at 1:39 PM Alexander Korotkov  wrote:
> Good, thank you.  I think 0001 patch is generally good, but needs some
> further polishing, e.g. more comments explaining how does it work.

Two things comes to my mind worth rechecking about 0001.
1) Are XLogCtl->InitializeReserved, XLogCtl->InitializedUpTo and
XLogCtl->xlblocks always page-aligned?  Because algorithm seems to be
sensitive to that.  If so, I would propose to explicitly comment that
and add corresponding asserts.
2) Check if there are concurrency issues between
AdvanceXLInsertBuffer() and switching to the new WAL file.

--
Regards,
Alexander Korotkov
Supabase




Re: proposal - plpgsql - support standard syntax for named arguments for cursors

2025-02-08 Thread Pavel Stehule
Hi

so 8. 2. 2025 v 20:25 odesílatel Tom Lane  napsal:

> Pavel Stehule  writes:
> > I propose to enhancing to ANSI/SQL standard syntax for named arguments
> > `argname => value`
>
> Is there any reason to think that that's actually in the standard?
> I poked around in SQL:2021 a little and couldn't find anything about
> cursors with arguments at all.
>

I think the possibility to use named arguments in OPEN statements is a
PostgreSQL proprietary feature.
And usage of cursors in PL/pgSQL is based on PL/SQL (not on SQL/PSM from
standard), but named
arguments for cursor is PostgreSQL proprietary feature and the syntax based
on usage `:=` is our
proprietary too.

This is from patch
https://github.com/postgres/postgres/commit/4adead1d224278ff3064636063a818eba17cb211

It is from the window, when the named arguments was supported already

https://www.postgresql.org/message-id/20091008023926.1be85753...@cvs.postgresql.org
(the syntax was changed before release)
but not with ANSI syntax
https://github.com/postgres/postgres/commit/865f14a2d31af23a05bbf2df04c274629c5d5c4d

I forgot to fix this in my patch for 9.5 - probably I missed this
functionality

Regards

Pavel



>
> regards, tom lane
>


Re: Test to dump and restore objects left behind by regression

2025-02-08 Thread Michael Paquier
On Fri, Feb 07, 2025 at 07:11:25AM +0900, Michael Paquier wrote:
> Okay, thanks for the feedback.  We have been relying on diff -u for
> the parts of the tests touched by 0001 for some time now, so if there
> are no objections I would like to apply 0001 in a couple of days.

This part has been applied as 169208092f5c.
--
Michael


signature.asc
Description: PGP signature


Re: proposal - plpgsql - support standard syntax for named arguments for cursors

2025-02-08 Thread Tom Lane
Pavel Stehule  writes:
> so 8. 2. 2025 v 20:25 odesílatel Tom Lane  napsal:
>> Is there any reason to think that that's actually in the standard?

> I think the possibility to use named arguments in OPEN statements is a
> PostgreSQL proprietary feature.
> And usage of cursors in PL/pgSQL is based on PL/SQL (not on SQL/PSM from
> standard), but named
> arguments for cursor is PostgreSQL proprietary feature and the syntax based
> on usage `:=` is our
> proprietary too.

Hmm ... yeah, it's not in SQL/PSM, but looking at PL/SQL:

https://docs.oracle.com/en/database/oracle/oracle-database/19/lnpls/OPEN-statement.html

I see

You can specify actual cursor parameters with either
positional notation or named notation. For information about
these notations, see "Positional, Named, and Mixed Notation
for Actual Parameters".

and that link blesses the use of "name => value" (and not ":=").
So agreed, we should adjust this.

Is there a reason we need a whole new test case instead of
tweaking one of the existing ones?

regards, tom lane




Re: On non-Windows, hard depend on uselocale(3)

2025-02-08 Thread Peter Eisentraut

Checking the status of this thread ...

The patches that removed the configure checks for _configthreadlocale(), 
and related cleanup, have been committed.


The original patch to "Tidy up locale thread safety in ECPG library" is 
still outstanding.


Attached is a rebased version, based on the posted v6, with a couple of 
small fixups from me.


I haven't re-reviewed it yet, but from scanning the discussion, it looks 
close to done.
From 842a5837898a4a47c8c79d07a2d14f3a82514a36 Mon Sep 17 00:00:00 2001
From: Thomas Munro 
Date: Sat, 10 Aug 2024 11:01:21 +1200
Subject: [PATCH v7 1/3] Tidy up locale thread safety in ECPG library.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Remove setlocale() and _configthreadlocal() as fallback strategy on
systems that don't have uselocale(), where ECPG tries to control
LC_NUMERIC formatting on input and output of floating point numbers.  It
was probably broken on some systems (NetBSD), and the code was also
quite messy and complicated, with obsolete configure tests (Windows).
It was also arguably broken, or at least had unstated environmental
requirements, if pgtypeslib code was called directly.

Instead, introduce PG_C_LOCALE to refer to the "C" locale as a locale_t
value.  It maps to the special constant LC_C_LOCALE when defined by libc
(macOS, NetBSD), or otherwise uses a process-lifetime locale_t that is
allocated on first use, just as ECPG previously did itself.  The new
replacement might be more widely useful.  Then change the float parsing
and printing code to pass that to _l() functions where appropriate.

Unfortunately the portability of those functions is a bit complicated.
First, many obvious and useful _l() functions are missing from POSIX,
though most standard libraries define some of them anyway.  Second,
although the thread-safe save/restore technique can be used to replace
the missing ones, Windows and NetBSD refused to implement standard
uselocale().  They might have a point: "wide scope" uselocale() is hard
to combine with other code and error-prone, especially in library code.
Luckily they have the  _l() functions we want so far anyway.  So we have
to be prepared for both ways of doing things:

1.  In ECPG, use strtod_l() for parsing, and supply a port.h replacement
using uselocale() over a limited scope if missing.

2.  Inside our own snprintf.c, use three different approaches to format
floats.  For frontend code, call libc's snprintf_l(), or wrap libc's
snprintf() in uselocale() if it's missing.  For backend code, snprintf.c
can keep assuming that the global locale's LC_NUMERIC is "C" and call
libc's snprintf() without change, for now.

(It might eventually be possible to call our in-tree Ryū routines to
display floats in snprintf.c, given the C-locale-always remit of our
in-tree snprintf(), but this patch doesn't risk changing anything that
complicated.)

Reviewed-by: Peter Eisentraut 
Reviewed-by: Tristan Partin 
Discussion: https://postgr.es/m/CWZBBRR6YA8D.8EHMDRGLCKCD%40neon.tech
---
 configure|  2 +-
 configure.ac |  2 +
 meson.build  |  2 +
 src/include/pg_config.h.in   |  6 ++
 src/include/port.h   | 31 
 src/include/port/win32_port.h|  1 +
 src/interfaces/ecpg/ecpglib/connect.c| 39 +-
 src/interfaces/ecpg/ecpglib/data.c   |  2 +-
 src/interfaces/ecpg/ecpglib/descriptor.c | 37 -
 src/interfaces/ecpg/ecpglib/ecpglib_extern.h | 12 ---
 src/interfaces/ecpg/ecpglib/execute.c| 53 -
 src/interfaces/ecpg/pgtypeslib/dt_common.c   |  6 +-
 src/interfaces/ecpg/pgtypeslib/interval.c|  4 +-
 src/interfaces/ecpg/pgtypeslib/numeric.c |  2 +-
 src/port/Makefile|  1 +
 src/port/locale.c| 80 
 src/port/meson.build |  1 +
 src/port/snprintf.c  | 55 ++
 18 files changed, 188 insertions(+), 148 deletions(-)
 create mode 100644 src/port/locale.c

diff --git a/configure b/configure
index 0ffcaeb4367..017e63598c1 100755
--- a/configure
+++ b/configure
@@ -14934,7 +14934,7 @@ fi
 LIBS_including_readline="$LIBS"
 LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
 
-for ac_func in backtrace_symbols copyfile copy_file_range elf_aux_info 
getauxval getifaddrs getpeerucred inet_pton kqueue mbstowcs_l memset_s 
posix_fallocate ppoll pthread_is_threaded_np setproctitle setproctitle_fast 
strchrnul strsignal syncfs sync_file_range uselocale wcstombs_l
+for ac_func in backtrace_symbols copyfile copy_file_range elf_aux_info 
getauxval getifaddrs getpeerucred inet_pton kqueue mbstowcs_l memset_s 
posix_fallocate ppoll pthread_is_threaded_np setproctitle setproctitle_fast 
snprintf_l strtod_l strchrnul strsignal syncfs sync_file_range uselocale 
wcstombs_l
 do :