public inbox for [email protected]  
help / color / mirror / Atom feed
Re: [PATCH] Add pg_current_vxact_id() function to expose virtual transaction IDs
3+ messages / 3 participants
[nested] [flat]

* Re: [PATCH] Add pg_current_vxact_id() function to expose virtual transaction IDs
@ 2026-01-09 03:15 Henson Choi <[email protected]>
  2026-02-05 13:13 ` Re: [PATCH] Add pg_current_vxact_id() function to expose virtual transaction IDs Pavlo Golub <[email protected]>
  0 siblings, 1 reply; 3+ messages in thread

From: Henson Choi @ 2026-01-09 03:15 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: Pavlo Golub <[email protected]>; [email protected]

2026년 1월 9일 (금) AM 9:25, Michael Paquier <[email protected]>님이 작성:

> On Mon, Dec 08, 2025 at 12:09:58PM +0000, Pavlo Golub wrote:
> > Virtual transaction IDs are fundamental to PostgreSQL's transaction
> > tracking,
> > appearing in pg_locks.virtualtransaction, log output via %v placeholder,
> and
> > internal transaction management. However, there's currently no direct SQL
> > function to retrieve the current VXID, forcing applications to query
> > pg_locks
> > or parse log files to obtain this information.
>
> This is replacing one SQL in a given session by another, as a session
> currently running a transaction can query itself pg_locks and match an
> entry with its own pg_backend_pid().  Hence I don't see the need for
> this function, except simplicity in retrieving a session's state with
> less characters typed at the end?
>

I see this as a tradeoff between minor convenience and negligible
addition cost.

The community should decide whether this tradeoff is worth it.


> Thoughts and opinions from others are welcome.  I'm always OK to be
> outvoted.
> --
> Michael
>

Best regards,
Henson Choi


^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: [PATCH] Add pg_current_vxact_id() function to expose virtual transaction IDs
  2026-01-09 03:15 Re: [PATCH] Add pg_current_vxact_id() function to expose virtual transaction IDs Henson Choi <[email protected]>
@ 2026-02-05 13:13 ` Pavlo Golub <[email protected]>
  0 siblings, 0 replies; 3+ messages in thread

From: Pavlo Golub @ 2026-02-05 13:13 UTC (permalink / raw)
  To: [email protected]; +Cc: Michael Paquier <[email protected]>; [email protected]

Hi hackers,

Thank you a lot for the feedback on this patch. And sorry for the
delay with the answer.

I'd like to address Michael's concern that pg_current_vxact_id() is
merely a convenience feature when pg_locks can provide the same
information. I believe there are stronger technical and design reasons
for this function beyond reducing typing.

pg_current_vxact_id() complexity is O(1) with direct memory reads and
no locks, while querying pg_locks requires O(n) where n is the number
of locks, among these operations are constructing tuples then
filtering. For me it's obviously more work. The proposed function is
essentially free compared to pg_locks querying.

As for the usage pattern this is how Postgres codebase is built from
the very beginning. PostgreSQL has many established patterns where
both SQL commands and functions exist for the same purpose, providing
programmatic access even when an alternative exists, e.g. NOTIFY vs
pg_notify(), SHOW vs current_setting(), pg_stat_activity vs
pg_current_xact_id() vs pg_current_xact_id_if_assigned(),
pg_stat_activity vs pg_backend_pid(), etc.

I see a clear pattern where Postgres provides function APIs fir
frequently needed info, even if it's theoretically possible to get
that info through other means.

My main concern though is about semantic clarity. I see a huge problem
that one needs to query pg_locks to get VXID. Why would I want to
query the lock subsystem to get transaction ID? That's very confusing.

pg_current_vxact_id() is semantically precise: "What is my
transaction's VXID?" We immediately have the direct answer from MyProc
without indirection through unrelated subsystems, which is pretty
obvious to anyone reading the code.

Having pg_current_vxact_id() increases API consistency. VXIDs deserve
the same treatment because they're fundamental to Postgres transaction
tracking and they uniquely identify every transaction (unlike XIDs). I
could day VXIDs are universal and that's why I think the current
asymmetry is odd:
- get XID: SELECT pg_current_xact_id()
- get PID: SELECT pg_backend_pid()
- get VXID: SELECT ... FROM pg_locks WHERE ... LIMIT 1  (why?)

Plus taking into account the minimal implementation and support costs
we should apply it. 20 lines of code without new infrastructure or
data structures changes sounds like a solid and maintainable piece. No
runtime overhead, no maintenance burden, simplest function without
locks or side effects.

I've prepared v3 of the patch addrressing Henson's code review:
- Added #define VXID_FMT "%d/%u" to lock.h
- Updated lockfuncs.c, elog.c, and xid8funcs.c to use it
- Use "localXID" (not "localTransactionId") in user docs

I'd appreciate thoughts from others in the community on whether this
reasoning is compelling.

Best regards,
Pavlo Golub

On Fri, Jan 9, 2026 at 4:15 AM Henson Choi <[email protected]> wrote:
>
>
>
> 2026년 1월 9일 (금) AM 9:25, Michael Paquier <[email protected]>님이 작성:
>>
>> On Mon, Dec 08, 2025 at 12:09:58PM +0000, Pavlo Golub wrote:
>> > Virtual transaction IDs are fundamental to PostgreSQL's transaction
>> > tracking,
>> > appearing in pg_locks.virtualtransaction, log output via %v placeholder, and
>> > internal transaction management. However, there's currently no direct SQL
>> > function to retrieve the current VXID, forcing applications to query
>> > pg_locks
>> > or parse log files to obtain this information.
>>
>> This is replacing one SQL in a given session by another, as a session
>> currently running a transaction can query itself pg_locks and match an
>> entry with its own pg_backend_pid().  Hence I don't see the need for
>> this function, except simplicity in retrieving a session's state with
>> less characters typed at the end?
>
>
> I see this as a tradeoff between minor convenience and negligible
> addition cost.
>
> The community should decide whether this tradeoff is worth it.
>
>>
>> Thoughts and opinions from others are welcome.  I'm always OK to be
>> outvoted.
>> --
>> Michael
>
>
> Best regards,
> Henson Choi
>


Attachments:

  [application/x-patch] v3-0001-add-pg_current_vxact_id-function.patch (9.5K, ../../CAK7ymcKXT__KoY1DHswEP0M1MUGoh9KmQ8c2oayP4W+62L9pCw@mail.gmail.com/2-v3-0001-add-pg_current_vxact_id-function.patch)
  download | inline diff:
From f3c86d977629244b4de45eb0c611bdbee0368505 Mon Sep 17 00:00:00 2001
From: Pavlo Golub <[email protected]>
Date: Thu, 5 Feb 2026 12:15:40 +0000
Subject: [PATCH] Add pg_current_vxact_id() function

This patch introduces pg_current_vxact_id() to expose the current
backend's virtual transaction ID as text in the format 'procNumber/lxid'
(e.g., '3/42').

Virtual transaction IDs are always assigned to every transaction, unlike
regular XIDs which are only assigned when a transaction modifies data.
This makes VXIDs useful for tracking and correlating all transactions,
including read-only ones.

The VXID format matches what's used in:
- elog %v placeholder for logging
- pg_locks.virtualtransaction column
- Internal PostgreSQL transaction tracking

The function returns NULL during recovery or when no valid VXID exists.

Changes in v3:
- Add VXID_FMT macro to lock.h to eliminate format string duplication
- Update lockfuncs.c, elog.c, and xid8funcs.c to use VXID_FMT
- Fix documentation to use 'localXID' terminology for consistency
- Add comprehensive regression tests in xid.sql
- Add documentation in func-info.sgml and xact.sgml
- Use OID 9538 (in development range 8000-9999)

Addresses review comments from Henson Choi.

Discussion: https://www.postgresql.org/message-id/CAAAe_zBUd3epVqcDAMVmLDt4-dhxVY5W09%2Bgp5ND_P--b90eeA%40mail.gmail.com
---
 doc/src/sgml/func/func-info.sgml  | 22 +++++++++++++++++++++
 doc/src/sgml/xact.sgml            |  4 +++-
 src/backend/utils/adt/lockfuncs.c |  2 +-
 src/backend/utils/adt/xid8funcs.c | 26 +++++++++++++++++++++++++
 src/backend/utils/error/elog.c    |  4 ++--
 src/include/catalog/pg_proc.dat   |  3 +++
 src/include/storage/lock.h        |  7 +++++++
 src/test/regress/expected/xid.out | 32 +++++++++++++++++++++++++++++++
 src/test/regress/sql/xid.sql      | 13 +++++++++++++
 9 files changed, 109 insertions(+), 4 deletions(-)

diff --git a/doc/src/sgml/func/func-info.sgml b/doc/src/sgml/func/func-info.sgml
index 294f45e82a3..4efe01d63b9 100644
--- a/doc/src/sgml/func/func-info.sgml
+++ b/doc/src/sgml/func/func-info.sgml
@@ -2864,6 +2864,28 @@ acl      | {postgres=arwdDxtm/postgres,foo=r/postgres}
        </para></entry>
       </row>
 
+      <row>
+       <entry role="func_table_entry"><para role="func_signature">
+        <indexterm>
+         <primary>pg_current_vxact_id</primary>
+        </indexterm>
+        <function>pg_current_vxact_id</function> ()
+        <returnvalue>text</returnvalue>
+       </para>
+       <para>
+        Returns the current virtual transaction ID (VXID) in the
+        format <literal>procNumber/localXID</literal>
+        (for example, <literal>3/42</literal>).
+        Virtual transaction ID is always assigned when a transaction starts,
+        unlike regular transaction IDs which are only assigned when the
+        transaction performs a database write.  VXIDs are session-scoped and
+        do not persist across server restarts.  They are primarily useful for
+        correlating transactions with log entries that use the <literal>%v</literal> placeholder
+        in <xref linkend="guc-log-line-prefix"/>.
+        See <xref linkend="transaction-id"/> for details.
+       </para></entry>
+      </row>
+
       <row>
        <entry role="func_table_entry"><para role="func_signature">
         <indexterm>
diff --git a/doc/src/sgml/xact.sgml b/doc/src/sgml/xact.sgml
index 3aa7ee1383e..4a5753178e0 100644
--- a/doc/src/sgml/xact.sgml
+++ b/doc/src/sgml/xact.sgml
@@ -31,7 +31,9 @@
    <literal>localXID</literal>.  For example, the virtual transaction
    ID <literal>4/12532</literal> has a <literal>procNumber</literal>
    of <literal>4</literal> and a <literal>localXID</literal> of
-   <literal>12532</literal>.
+   <literal>12532</literal>.  The function
+   <function>pg_current_vxact_id</function> returns the current
+   transaction's VXID.
   </para>
 
   <para>
diff --git a/src/backend/utils/adt/lockfuncs.c b/src/backend/utils/adt/lockfuncs.c
index 9dadd6da672..36772ffd3f1 100644
--- a/src/backend/utils/adt/lockfuncs.c
+++ b/src/backend/utils/adt/lockfuncs.c
@@ -80,7 +80,7 @@ VXIDGetDatum(ProcNumber procNumber, LocalTransactionId lxid)
 	 */
 	char		vxidstr[32];
 
-	snprintf(vxidstr, sizeof(vxidstr), "%d/%u", procNumber, lxid);
+	snprintf(vxidstr, sizeof(vxidstr), VXID_FMT, procNumber, lxid);
 
 	return CStringGetTextDatum(vxidstr);
 }
diff --git a/src/backend/utils/adt/xid8funcs.c b/src/backend/utils/adt/xid8funcs.c
index c607e78d9ac..bc9e1076cc9 100644
--- a/src/backend/utils/adt/xid8funcs.c
+++ b/src/backend/utils/adt/xid8funcs.c
@@ -33,6 +33,7 @@
 #include "libpq/pqformat.h"
 #include "miscadmin.h"
 #include "storage/lwlock.h"
+#include "storage/proc.h"
 #include "storage/procarray.h"
 #include "storage/procnumber.h"
 #include "utils/builtins.h"
@@ -360,6 +361,31 @@ pg_current_xact_id_if_assigned(PG_FUNCTION_ARGS)
 	PG_RETURN_FULLTRANSACTIONID(topfxid);
 }
 
+/*
+ * pg_current_vxact_id() returns text
+ *
+ *	Return the current virtual transaction ID (vxid).
+ *	vxid is always assigned and available, unlike regular transaction IDs.
+ *	Returns NULL if no valid vxid exists (e.g., during startup/recovery).
+ */
+Datum
+pg_current_vxact_id(PG_FUNCTION_ARGS)
+{
+	char		vxidstr[32];
+
+	/*
+	 * Check if we have a valid vxid.  The vxid format matches what's used
+	 * in elog.c for the %v placeholder and in pg_locks.virtualtransaction.
+	 */
+	if (MyProc == NULL || MyProc->vxid.procNumber == INVALID_PROC_NUMBER)
+		PG_RETURN_NULL();
+
+	snprintf(vxidstr, sizeof(vxidstr), VXID_FMT,
+			 MyProc->vxid.procNumber, MyProc->vxid.lxid);
+
+	PG_RETURN_TEXT_P(cstring_to_text(vxidstr));
+}
+
 /*
  * pg_current_snapshot() returns pg_snapshot
  *
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index e6a4ef99059..fd64a4847ef 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -3181,12 +3181,12 @@ log_status_format(StringInfo buf, const char *format, ErrorData *edata)
 					{
 						char		strfbuf[128];
 
-						snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u",
+						snprintf(strfbuf, sizeof(strfbuf) - 1, VXID_FMT,
 								 MyProc->vxid.procNumber, MyProc->vxid.lxid);
 						appendStringInfo(buf, "%*s", padding, strfbuf);
 					}
 					else
-						appendStringInfo(buf, "%d/%u", MyProc->vxid.procNumber, MyProc->vxid.lxid);
+						appendStringInfo(buf, VXID_FMT, MyProc->vxid.procNumber, MyProc->vxid.lxid);
 				}
 				else if (padding != 0)
 					appendStringInfoSpaces(buf,
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 83f6501df38..fb2f9324b02 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -10692,6 +10692,9 @@
 { oid => '5066', descr => 'commit status of transaction',
   proname => 'pg_xact_status', provolatile => 'v', prorettype => 'text',
   proargtypes => 'xid8', prosrc => 'pg_xact_status' },
+{ oid => '9538', descr => 'get current virtual transaction ID',
+  proname => 'pg_current_vxact_id', provolatile => 's', proparallel => 'u',
+  prorettype => 'text', proargtypes => '', prosrc => 'pg_current_vxact_id' },
 
 # record comparison using normal comparison rules
 { oid => '2981',
diff --git a/src/include/storage/lock.h b/src/include/storage/lock.h
index 55ffaa5e4a5..962009bde08 100644
--- a/src/include/storage/lock.h
+++ b/src/include/storage/lock.h
@@ -64,6 +64,13 @@ typedef struct
 	LocalTransactionId localTransactionId;	/* lxid from PGPROC */
 } VirtualTransactionId;
 
+/*
+ * Format string for VXID display.
+ * Used by elog.c (%v placeholder), lockfuncs.c (VXIDGetDatum),
+ * and xid8funcs.c (pg_current_vxact_id).
+ */
+#define VXID_FMT "%d/%u"
+
 #define InvalidLocalTransactionId		0
 #define LocalTransactionIdIsValid(lxid) ((lxid) != InvalidLocalTransactionId)
 #define VirtualTransactionIdIsValid(vxid) \
diff --git a/src/test/regress/expected/xid.out b/src/test/regress/expected/xid.out
index 1ce7826cf90..55649cc4358 100644
--- a/src/test/regress/expected/xid.out
+++ b/src/test/regress/expected/xid.out
@@ -463,6 +463,38 @@ SELECT pg_current_xact_id_if_assigned() IS NOT DISTINCT FROM xid8 :'pg_current_x
  t
 (1 row)
 
+COMMIT;
+-- test pg_current_vxact_id
+BEGIN;
+SELECT pg_current_vxact_id() IS NOT NULL AS vxid_assigned;
+ vxid_assigned 
+---------------
+ t
+(1 row)
+
+SELECT pg_current_vxact_id() ~ '^\d+/\d+$' AS vxid_format_ok;
+ vxid_format_ok 
+----------------
+ t
+(1 row)
+
+SELECT pg_current_vxact_id() AS vxid1 \gset
+SELECT pg_current_vxact_id() = :'vxid1' AS vxid_stable;
+ vxid_stable 
+-------------
+ t
+(1 row)
+
+COMMIT;
+-- start new transaction, vxid should change
+BEGIN;
+SELECT pg_current_vxact_id() AS vxid2 \gset
+SELECT :'vxid2' <> :'vxid1' AS vxid_changed;
+ vxid_changed 
+--------------
+ t
+(1 row)
+
 COMMIT;
 -- test xid status functions
 BEGIN;
diff --git a/src/test/regress/sql/xid.sql b/src/test/regress/sql/xid.sql
index 9f716b3653a..5a48f914a1f 100644
--- a/src/test/regress/sql/xid.sql
+++ b/src/test/regress/sql/xid.sql
@@ -132,6 +132,19 @@ SELECT pg_current_xact_id() \gset
 SELECT pg_current_xact_id_if_assigned() IS NOT DISTINCT FROM xid8 :'pg_current_xact_id';
 COMMIT;
 
+-- test pg_current_vxact_id
+BEGIN;
+SELECT pg_current_vxact_id() IS NOT NULL AS vxid_assigned;
+SELECT pg_current_vxact_id() ~ '^\d+/\d+$' AS vxid_format_ok;
+SELECT pg_current_vxact_id() AS vxid1 \gset
+SELECT pg_current_vxact_id() = :'vxid1' AS vxid_stable;
+COMMIT;
+-- start new transaction, vxid should change
+BEGIN;
+SELECT pg_current_vxact_id() AS vxid2 \gset
+SELECT :'vxid2' <> :'vxid1' AS vxid_changed;
+COMMIT;
+
 -- test xid status functions
 BEGIN;
 SELECT pg_current_xact_id() AS committed \gset
-- 
2.52.0



^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: Fix duplicate detection for null-treatment window functions
@ 2026-07-10 02:44 Ewan Young <[email protected]>
  0 siblings, 0 replies; 3+ messages in thread

From: Ewan Young @ 2026-07-10 02:44 UTC (permalink / raw)
  To: Chao Li <[email protected]>; +Cc: PostgreSQL-development <[email protected]>; Tatsuo Ishii <[email protected]>

Hi Chao,


On Thu, Jul 9, 2026 at 8:35 PM Chao Li <[email protected]> wrote:
>
>
>
> > On Jul 7, 2026, at 08:14, Chao Li <[email protected]> wrote:
> >
> > Hi,
> >
> > I just spotted an oversight from “[25a30bbd4] Add IGNORE NULLS/RESPECT NULLS option to Window functions”.
> >
> > In ExecInitWindowAgg(), there is logic to detect duplicate functions:
> > ```
> > if (i <= wfuncno && wfunc->ignore_nulls == perfunc[i].ignore_nulls)
> > {
> > /* Found a match to an existing entry, so just mark it */
> > wfuncstate->wfuncno = i;
> > continue;
> > }
> > ```
> >
> > However, when appending function info to perfunc, ignore_nulls is not copied:
> > ```
> > /* Fill in the perfuncstate data */
> > perfuncstate->wfuncstate = wfuncstate;
> > perfuncstate->wfunc = wfunc;
> > perfuncstate->numArguments = list_length(wfuncstate->args);
> > perfuncstate->winCollation = wfunc->inputcollid;
> > ```
> >
> > As a result, wfunc->ignore_nulls == perfunc[i].ignore_nulls can never be true for duplicate IGNORE NULLS or explicit RESPECT NULLS calls. This means duplicate detection doesn't work for those calls. This bug is easy to prove by adding temporary logs, and the fix is straightforward: copy ignore_nulls when filling in the perfuncstate data.
> >
> > This is a simple repro:
> >
> > Without the fix:
> > ```
> > evantest=# WITH t(x) AS (VALUES (NULL::int), (1), (2))
> > evantest-# SELECT first_value(x) IGNORE NULLS OVER w AS a,
> > evantest-#        first_value(x) IGNORE NULLS OVER w AS b
> > evantest-# FROM t
> > evantest-# WINDOW w AS (ORDER BY x NULLS FIRST
> > evantest(#              ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING);
> > INFO:  WindowAgg duplicate cache miss: no previous match, ignore_nulls 1
> > INFO:  WindowAgg duplicate cache miss: matched wfuncno 0, ignore_nulls 1, cached ignore_nulls 0
> > a | b
> > ---+---
> > 1 | 1
> > 1 | 1
> > 1 | 1
> > (3 rows)
> > ```
> >
> > With the fix:
> > ```
> > evantest=# WITH t(x) AS (VALUES (NULL::int), (1), (2))
> > evantest-# SELECT first_value(x) IGNORE NULLS OVER w AS a,
> > evantest-#        first_value(x) IGNORE NULLS OVER w AS b
> > evantest-# FROM t
> > evantest-# WINDOW w AS (ORDER BY x NULLS FIRST
> > evantest(#              ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING);
> > INFO:  WindowAgg duplicate cache miss: no previous match, ignore_nulls 1
> > INFO:  WindowAgg duplicate cache hit: existing wfuncno 0, ignore_nulls 1
> > a | b
> > ---+---
> > 1 | 1
> > 1 | 1
> > 1 | 1
> > (3 rows)
> > ```
> >
> > See the attached patch for details. The actual fix is only one line. The INFO logs above were produced with temporary debug logs added around the duplicate-function lookup, those logs are not part of the proposed fix. I left those logs in the patch with TODO comments only so reviewers can see the behavior before and after the fix. They should be removed before pushing.
> >
> > Best regards,
> > --
> > Chao Li (Evan)
> > HighGo Software Co., Ltd.
> > https://www.highgo.com/
> >
> >
> >
> >
> > <v1-0001-Fix-duplicate-detection-for-null-treatment-window.patch>
>
> I realized that leaving the temp log code in the patch is not friendly to the CF test. So, splitting the temp log part into a diff file.

Nice catch, and v2 does restore the intended de-duplication.

While reviewing it, though, I think the check the patch repairs is actually
redundant, and the real issue is that the code (both the original commit and
the patch) keeps a shadow copy of ignore_nulls that has to be maintained by
hand.

WindowFunc.ignore_nulls is a plain scalar field with no pg_node_attr, so
equal() already compares it -- the generated _equalWindowFunc() has:

    COMPARE_SCALAR_FIELD(winagg);
    COMPARE_SCALAR_FIELD(ignore_nulls);
    COMPARE_LOCATION_FIELD(location);

That means the equal() call in the dedup loop already distinguishes two
WindowFuncs that differ only in null treatment:

    for (i = 0; i <= wfuncno; i++)
    {
        if (equal(wfunc, perfunc[i].wfunc) &&
            !contain_volatile_functions((Node *) wfunc))
            break;
    }
    if (i <= wfuncno && wfunc->ignore_nulls == perfunc[i].ignore_nulls)

If ignore_nulls differs, equal() returns false, the loop never breaks on
that entry, and we never reach the extra term. If equal() matches, then
ignore_nulls necessarily matched too. So "&& wfunc->ignore_nulls ==
perfunc[i].ignore_nulls" can never change the outcome, and
WindowStatePerFuncData.ignore_nulls exists only to feed it -- it is read
only there and, before this patch, written nowhere (which is exactly why it
was always 0).

So rather than populating the field, I'd suggest dropping the redundant term
and the field, and letting equal() do the work:

and the field, and letting equal() do the work:

-   if (i <= wfuncno && wfunc->ignore_nulls == perfunc[i].ignore_nulls)
+   if (i <= wfuncno)
    {
        /* Found a match to an existing entry, so just mark it */
        wfuncstate->wfuncno = i;
        continue;
    }

plus removing the ignore_nulls member from WindowStatePerFuncData and
trimming the now-stale "which needs the same ignore_nulls value" comment.
That fixes the same bug while removing the duplicated state that caused it,
so it can't silently drift again.

The one argument for an explicit check is defensiveness: if someone later
tags ignore_nulls with a pg_node_attr that excludes it from equal(), the
loop would start collapsing functions with different null treatment. If
that's a worry, the robust form compares the stored node directly instead of
a shadow copy, and still needs no separate field:

    if (i <= wfuncno &&
        wfunc->ignore_nulls == perfunc[i].wfunc->ignore_nulls)

Given ignore_nulls has to stay significant to equal() anyway (two calls with
different null treatment really are different functions), I'd lean toward
just removing the check.

Happy to send a patch along these lines if you agree.

>
> Best regards,
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/
>
>
>
>

-- 
Regards,
Ewan Young






^ permalink  raw  reply  [nested|flat] 3+ messages in thread


end of thread, other threads:[~2026-07-10 02:44 UTC | newest]

Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-01-09 03:15 Re: [PATCH] Add pg_current_vxact_id() function to expose virtual transaction IDs Henson Choi <[email protected]>
2026-02-05 13:13 ` Pavlo Golub <[email protected]>
2026-07-10 02:44 Re: Fix duplicate detection for null-treatment window functions Ewan Young <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox