public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v4 3/3] Fix timeline-tracking failure while sending a historic timeline
5+ messages / 4 participants
[nested] [flat]
* [PATCH v4 3/3] Fix timeline-tracking failure while sending a historic timeline
@ 2020-12-09 08:22 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: Kyotaro Horiguchi @ 2020-12-09 08:22 UTC (permalink / raw)
Walsender should track timeline switches while sending a historic
timeline. Regain that behavior, which was broken in PG13, by a thinko
of 709d003fbd.
---
src/backend/replication/walsender.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index fe0d368a35..8545c6c423 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -2491,7 +2491,7 @@ WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo,
XLogSegNo endSegNo;
XLByteToSeg(sendTimeLineValidUpto, endSegNo, state->segcxt.ws_segsize);
- if (state->seg.ws_segno == endSegNo)
+ if (nextSegNo == endSegNo)
*tli_p = sendTimeLineNextTLI;
}
--
2.27.0
----Next_Part(Thu_Jan__7_16_32_36_2021_122)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="0001-Fix-timeline-tracking-failure-while-sending-a-PG13.patch.txt"
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: [Bug] Add the missing RTE_GRAPH_TABLE case to transformLockingClause()
@ 2026-06-09 19:33 Ayush Tiwari <[email protected]>
2026-06-10 12:49 ` Re: [Bug] Add the missing RTE_GRAPH_TABLE case to transformLockingClause() Ashutosh Bapat <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Ayush Tiwari @ 2026-06-09 19:33 UTC (permalink / raw)
To: Ashutosh Bapat <[email protected]>; Peter Eisentraut <[email protected]>; +Cc: SATYANARAYANA NARLAPURAM <[email protected]>; PostgreSQL Hackers <[email protected]>
Hi,
On Wed, 13 May 2026 at 10:57, Ashutosh Bapat <[email protected]>
wrote:
> > Repro:
> >
> > CREATE TABLE v(id int PRIMARY KEY, vname text);
> > CREATE PROPERTY GRAPH g VERTEX TABLES (v);
> > SELECT * FROM GRAPH_TABLE(g MATCH (a) COLUMNS (a.vname)) gt
> > FOR UPDATE OF gt;
> > -- ERROR: unrecognized RTE type: 8
> >
> > Attached a patch that returns ERRCODE_FEATURE_NOT_SUPPORTED "FOR ...
> cannot be
> > applied to a GRAPH_TABLE" with a position pointer, matching the
> convention used by
> > the function/tablefunc etc.
>
> In a fully matured SQL/PGQ support, a row mark on GRAPH_TABLE should
> be pushed down to the resulting subqueries in rewriteGraphTable(). I
> see this similar to how the row mark is pushed down into views during
> the rewrite phase. I don't think the code changes will be massive, it
> will be a matter of invoking markQueryForLocking() on the query that
> replaces the graph pattern in rewriteGraphTable(). I am not sure
> whether we want to do that now when stabilizing the feature. Peter,
> what do you think?
>
I wanted to implement this by trying to push FOR UPDATE/SHARE
down into the relational tables underlying the GRAPH_TABLE (the
"fully matured SQL/PGQ" direction mentioned). But had
some doubts around the implementation.
I'd like input on a few design questions from you/Peter/hackers:
Q1. Is the right place to do the pushdown rewriteGraphTable(),
mirroring what ApplyRetrieveRule() does for views (i.e. call
markQueryForLocking() on the freshly built subquery)? That
would require exposing markQueryForLocking() outside
rewriteHandler.c.
Q2. How should FOR UPDATE behave when the MATCH expands to more
than one path and rewriteGraphTable() produces a UNION?
markQueryForLocking() refuses to descend into set-operations,
same as it does for views. Options:
(a) reject FOR UPDATE on multi-path MATCH with a clear
"cannot lock rows from a set-operation result" error,
matching the view behaviour;
(b) silently fall back to the current
"raise relation lock only" behaviour and document it.
I think it should be (a).
Q3. SQL/PGQ does not really define UPDATE/DELETE semantics for
graph paths. Is the intent here that FOR UPDATE/SHARE on
GRAPH_TABLE is purely a concurrency hint rather than a
precursor to UPDATE WHERE CURRENT OF? If so, that should
probably be documented next to the GRAPH_TABLE syntax in
the SQL reference.
> If we decide to not to support it, the code changes look on the right
> path. We need to update the following comment which appears earlier in
> the function to mention GRAPH_TABLE, /* ignore JOIN, SPECIAL,
> FUNCTION, VALUES, CTE RTEs */. I think it will be better to specify
> that this restriction is temporary in the comments at both the places;
> but that's arguable.
>
> > Patch includes tests for all four locking strengths.
> > Since the code path looks simple we can just keep one of them as well
> and trim other
> > tests. Thoughts?
>
> I don't think we need all the four tests, but we need a test for
> locking clause without relation since that takes a different code
> path.
>
Attached is v2-0001, rebased on master and addressing
the comments you added in case we plan on parking the feature
work for once v20 opens.
Regards,
Ayush
Attachments:
[application/octet-stream] v2-0001-Add-the-missing-RTE_GRAPH_TABLE-case-to-transform.patch (4.5K, ../../CAJTYsWX8J2cUTCnVRwks0gJw4wiMCv_qVSxshFC9f8oh7Ch6JA@mail.gmail.com/3-v2-0001-Add-the-missing-RTE_GRAPH_TABLE-case-to-transform.patch)
download | inline diff:
From 8c75b0d94e507a60616d304e85153461cf96c2c5 Mon Sep 17 00:00:00 2001
From: Ayush Tiwari <[email protected]>
Date: Tue, 9 Jun 2026 20:14:13 +0530
Subject: [PATCH v2] Add the missing RTE_GRAPH_TABLE case to
transformLockingClause()
A FOR UPDATE/SHARE clause that names a GRAPH_TABLE alias hit the
internal "unrecognized RTE type" error rather than a user-friendly
"FOR ... cannot be applied to a GRAPH_TABLE" message:
CREATE TABLE v(id int PRIMARY KEY, vname text);
CREATE PROPERTY GRAPH g VERTEX TABLES (v);
SELECT * FROM GRAPH_TABLE(g MATCH (a) COLUMNS (a.vname)) gt
FOR UPDATE OF gt;
-- ERROR: unrecognized RTE type: 8
Add an explicit case for RTE_GRAPH_TABLE that raises
ERRCODE_FEATURE_NOT_SUPPORTED, matching the convention already used
for RTE_FUNCTION, RTE_TABLEFUNC, RTE_VALUES, RTE_CTE and
RTE_NAMEDTUPLESTORE. Tests cover both the named-rel and the
all-rels code paths.
Author: SATYANARAYANA NARLAPURAM <[email protected]>
Reported-by: SATYANARAYANA NARLAPURAM <[email protected]>
Reviewed-by: Zhenwei Shang <[email protected]>
Reviewed-by: Ashutosh Bapat <[email protected]>
Discussion: https://postgr.es/m/CAHg%2BQDcE9wp6nOEC3SCRQ90nrCO%3DQF%2BOZq1MG8Qc6hnusmogqw%40mail.gmail.com
---
src/backend/parser/analyze.c | 16 +++++++++++++++-
src/test/regress/expected/graph_table.out | 16 ++++++++++++++++
src/test/regress/sql/graph_table.sql | 7 +++++++
3 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 93fa66ae57c..74294581ef9 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -3866,7 +3866,7 @@ transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc,
allrels, true);
break;
default:
- /* ignore JOIN, SPECIAL, FUNCTION, VALUES, CTE RTEs */
+ /* ignore JOIN, SPECIAL, FUNCTION, VALUES, CTE, GRAPH_TABLE */
break;
}
}
@@ -4001,6 +4001,20 @@ transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc,
LCS_asString(lc->strength)),
parser_errposition(pstate, thisrel->location)));
break;
+ case RTE_GRAPH_TABLE:
+ /*
+ * XXX: push the row mark down to the
+ * relations the GRAPH_TABLE expands to
+ * during rewriting.
+ */
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ /*------
+ translator: %s is a SQL row locking clause such as FOR UPDATE */
+ errmsg("%s cannot be applied to a GRAPH_TABLE",
+ LCS_asString(lc->strength)),
+ parser_errposition(pstate, thisrel->location)));
+ break;
/* Shouldn't be possible to see RTE_RESULT here */
diff --git a/src/test/regress/expected/graph_table.out b/src/test/regress/expected/graph_table.out
index 70d986e8ab0..d95752cf92c 100644
--- a/src/test/regress/expected/graph_table.out
+++ b/src/test/regress/expected/graph_table.out
@@ -1077,4 +1077,20 @@ SELECT src.vname, count(*) FROM v1 AS src
v13 | 1
(3 rows)
+-- FOR UPDATE/SHARE on GRAPH_TABLE: not supported when the alias is named
+-- in the locking clause; silently ignored when no relations are named.
+SELECT * FROM GRAPH_TABLE (g1 MATCH (src IS vl1) COLUMNS (src.vname)) gt
+ FOR UPDATE OF gt; -- error
+ERROR: FOR UPDATE cannot be applied to a GRAPH_TABLE
+LINE 2: FOR UPDATE OF gt;
+ ^
+SELECT * FROM GRAPH_TABLE (g1 MATCH (src IS vl1) COLUMNS (src.vname)) gt
+ FOR UPDATE; -- ok, lock silently skipped for GRAPH_TABLE
+ vname
+-------
+ v11
+ v12
+ v13
+(3 rows)
+
-- leave the objects behind for pg_upgrade/pg_dump tests
diff --git a/src/test/regress/sql/graph_table.sql b/src/test/regress/sql/graph_table.sql
index 0b44f70d7e7..3976db4e231 100644
--- a/src/test/regress/sql/graph_table.sql
+++ b/src/test/regress/sql/graph_table.sql
@@ -612,4 +612,11 @@ SELECT src.vname, count(*) FROM v1 AS src
HAVING count(*) >= (SELECT count(*) FROM GRAPH_TABLE (g1 MATCH (a IS vl1 | vl2) COLUMNS (a.vname AS n)) WHERE n = src.vname)
ORDER BY vname;
+-- FOR UPDATE/SHARE on GRAPH_TABLE: not supported when the alias is named
+-- in the locking clause; silently ignored when no relations are named.
+SELECT * FROM GRAPH_TABLE (g1 MATCH (src IS vl1) COLUMNS (src.vname)) gt
+ FOR UPDATE OF gt; -- error
+SELECT * FROM GRAPH_TABLE (g1 MATCH (src IS vl1) COLUMNS (src.vname)) gt
+ FOR UPDATE; -- ok, lock silently skipped for GRAPH_TABLE
+
-- leave the objects behind for pg_upgrade/pg_dump tests
--
2.34.1
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: [Bug] Add the missing RTE_GRAPH_TABLE case to transformLockingClause()
2026-06-09 19:33 Re: [Bug] Add the missing RTE_GRAPH_TABLE case to transformLockingClause() Ayush Tiwari <[email protected]>
@ 2026-06-10 12:49 ` Ashutosh Bapat <[email protected]>
2026-07-09 08:18 ` Re: [Bug] Add the missing RTE_GRAPH_TABLE case to transformLockingClause() Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Ashutosh Bapat @ 2026-06-10 12:49 UTC (permalink / raw)
To: Ayush Tiwari <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; SATYANARAYANA NARLAPURAM <[email protected]>; PostgreSQL Hackers <[email protected]>
On Wed, Jun 10, 2026 at 1:03 AM Ayush Tiwari
<[email protected]> wrote:
>
> Hi,
>
> On Wed, 13 May 2026 at 10:57, Ashutosh Bapat <[email protected]> wrote:
>>
>> > Repro:
>> >
>> > CREATE TABLE v(id int PRIMARY KEY, vname text);
>> > CREATE PROPERTY GRAPH g VERTEX TABLES (v);
>> > SELECT * FROM GRAPH_TABLE(g MATCH (a) COLUMNS (a.vname)) gt
>> > FOR UPDATE OF gt;
>> > -- ERROR: unrecognized RTE type: 8
>> >
>> > Attached a patch that returns ERRCODE_FEATURE_NOT_SUPPORTED "FOR ... cannot be
>> > applied to a GRAPH_TABLE" with a position pointer, matching the convention used by
>> > the function/tablefunc etc.
>>
>> In a fully matured SQL/PGQ support, a row mark on GRAPH_TABLE should
>> be pushed down to the resulting subqueries in rewriteGraphTable(). I
>> see this similar to how the row mark is pushed down into views during
>> the rewrite phase. I don't think the code changes will be massive, it
>> will be a matter of invoking markQueryForLocking() on the query that
>> replaces the graph pattern in rewriteGraphTable(). I am not sure
>> whether we want to do that now when stabilizing the feature. Peter,
>> what do you think?
>
>
> I wanted to implement this by trying to push FOR UPDATE/SHARE
> down into the relational tables underlying the GRAPH_TABLE (the
> "fully matured SQL/PGQ" direction mentioned). But had
> some doubts around the implementation.
>
> I'd like input on a few design questions from you/Peter/hackers:
>
> Q1. Is the right place to do the pushdown rewriteGraphTable(),
> mirroring what ApplyRetrieveRule() does for views (i.e. call
> markQueryForLocking() on the freshly built subquery)? That
> would require exposing markQueryForLocking() outside
> rewriteHandler.c.
That's right. I think making markQueryForLocking non-static is fine
since we are using it in rewriter code itself.
>
> Q2. How should FOR UPDATE behave when the MATCH expands to more
> than one path and rewriteGraphTable() produces a UNION?
> markQueryForLocking() refuses to descend into set-operations,
> same as it does for views. Options:
> (a) reject FOR UPDATE on multi-path MATCH with a clear
> "cannot lock rows from a set-operation result" error,
> matching the view behaviour;
> (b) silently fall back to the current
> "raise relation lock only" behaviour and document it.
> I think it should be (a).
>
Building a UNION query is an implementation artifact of GRAPH_TABLE,
locking clause should not depend upon it. I think we should build the
subqueries within the setop with a locking clause of their own. That's
supported.
> Q3. SQL/PGQ does not really define UPDATE/DELETE semantics for
> graph paths. Is the intent here that FOR UPDATE/SHARE on
> GRAPH_TABLE is purely a concurrency hint rather than a
> precursor to UPDATE WHERE CURRENT OF? If so, that should
> probably be documented next to the GRAPH_TABLE syntax in
> the SQL reference.
>
I don't understand the WHERE CURRENT OF reference here. FOR
UPDATE/SHARE is generally used with an intent of locking the read rows
so that they don't get modified while the transaction is in progress.
One can use just UPDATE to update the FOR UPDATE locked rows as well.
>>
>> If we decide to not to support it, the code changes look on the right
>> path. We need to update the following comment which appears earlier in
>> the function to mention GRAPH_TABLE, /* ignore JOIN, SPECIAL,
>> FUNCTION, VALUES, CTE RTEs */. I think it will be better to specify
>> that this restriction is temporary in the comments at both the places;
>> but that's arguable.
>>
>> > Patch includes tests for all four locking strengths.
>> > Since the code path looks simple we can just keep one of them as well and trim other
>> > tests. Thoughts?
>>
>> I don't think we need all the four tests, but we need a test for
>> locking clause without relation since that takes a different code
>> path.
>
>
> Attached is v2-0001, rebased on master and addressing
> the comments you added in case we plan on parking the feature
> work for once v20 opens.
Thanks. I revised it a bit. Reworded the commit message to describe
user facing behaviour instead of the internals, which are quite
obvious from the code changes. Other cases in transformLockingClause()
don't mention XXX, they just have ereport(). Removed that comment.
Also massaged comments in the test - we usually don't describe the
expected behaviour or error when it is obvious from the output.
Attached revised patch.
--
Best Wishes,
Ashutosh Bapat
Attachments:
[text/x-patch] v20260610-0009-Prohibit-Locking-Clauses-on-GRAPH_TABLE.patch (3.8K, ../../CAExHW5tGQCjWEdyhOHGMj32NAMObH--eZDGeoxKRS9O3Wv_7wQ@mail.gmail.com/2-v20260610-0009-Prohibit-Locking-Clauses-on-GRAPH_TABLE.patch)
download | inline diff:
From a2f2bf2dadd670b3cdf452abced1cbbd4c99b373 Mon Sep 17 00:00:00 2001
From: Ayush Tiwari <[email protected]>
Date: Tue, 9 Jun 2026 20:14:13 +0530
Subject: [PATCH v20260610 9/9] Prohibit Locking Clauses on GRAPH_TABLE
Specifying a locking clause (FOR UPDATE/SHARE) that names a GRAPH_TABLE
alias currently results in an unhelpful "unrecognized RTE type: 8"
error. This commit explicitly prohibits specifying a locking clause on a
GRAPH_TABLE alias, raising a more user-friendly "FOR ... cannot be
applied to a GRAPH_TABLE" error instead.
Author: SATYANARAYANA NARLAPURAM <[email protected]>
Author: Ayush Tiwari <[email protected]>
Reported-by: SATYANARAYANA NARLAPURAM <[email protected]>
Reviewed-by: Ashutosh Bapat <[email protected]>
Discussion: https://postgr.es/m/CAHg%2BQDcE9wp6nOEC3SCRQ90nrCO%3DQF%2BOZq1MG8Qc6hnusmogqw%40mail.gmail.com
---
src/backend/parser/analyze.c | 15 ++++++++++++++-
src/test/regress/expected/graph_table.out | 13 +++++++++++++
src/test/regress/sql/graph_table.sql | 4 ++++
3 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 93fa66ae57c..f762b653e77 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -3866,7 +3866,11 @@ transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc,
allrels, true);
break;
default:
- /* ignore JOIN, SPECIAL, FUNCTION, VALUES, CTE RTEs */
+
+ /*
+ * ignore JOIN, SPECIAL, FUNCTION, VALUES, CTE,
+ * GRAPH_TABLE
+ */
break;
}
}
@@ -4001,6 +4005,15 @@ transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc,
LCS_asString(lc->strength)),
parser_errposition(pstate, thisrel->location)));
break;
+ case RTE_GRAPH_TABLE:
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ /*------
+ translator: %s is a SQL row locking clause such as FOR UPDATE */
+ errmsg("%s cannot be applied to a GRAPH_TABLE",
+ LCS_asString(lc->strength)),
+ parser_errposition(pstate, thisrel->location)));
+ break;
/* Shouldn't be possible to see RTE_RESULT here */
diff --git a/src/test/regress/expected/graph_table.out b/src/test/regress/expected/graph_table.out
index c229212cb51..c0622ab7234 100644
--- a/src/test/regress/expected/graph_table.out
+++ b/src/test/regress/expected/graph_table.out
@@ -1129,4 +1129,17 @@ SELECT src.vname, count(*) FROM v1 AS src
v13 | 1
(3 rows)
+-- Locking clause on GRAPH_TABLE
+SELECT * FROM GRAPH_TABLE (g1 MATCH (src IS vl1) COLUMNS (src.vname)) gt FOR UPDATE OF gt; -- not supported
+ERROR: FOR UPDATE cannot be applied to a GRAPH_TABLE
+LINE 1: ...MATCH (src IS vl1) COLUMNS (src.vname)) gt FOR UPDATE OF gt;
+ ^
+SELECT * FROM GRAPH_TABLE (g1 MATCH (src IS vl1) COLUMNS (src.vname)) gt FOR UPDATE; -- ignored
+ vname
+-------
+ v11
+ v12
+ v13
+(3 rows)
+
-- leave the objects behind for pg_upgrade/pg_dump tests
diff --git a/src/test/regress/sql/graph_table.sql b/src/test/regress/sql/graph_table.sql
index e30c908dccc..437fb3b01f2 100644
--- a/src/test/regress/sql/graph_table.sql
+++ b/src/test/regress/sql/graph_table.sql
@@ -637,4 +637,8 @@ SELECT src.vname, count(*) FROM v1 AS src
HAVING count(*) >= (SELECT count(*) FROM GRAPH_TABLE (g1 MATCH (a IS vl1 | vl2) COLUMNS (a.vname AS n)) WHERE n = src.vname)
ORDER BY vname;
+-- Locking clause on GRAPH_TABLE
+SELECT * FROM GRAPH_TABLE (g1 MATCH (src IS vl1) COLUMNS (src.vname)) gt FOR UPDATE OF gt; -- not supported
+SELECT * FROM GRAPH_TABLE (g1 MATCH (src IS vl1) COLUMNS (src.vname)) gt FOR UPDATE; -- ignored
+
-- leave the objects behind for pg_upgrade/pg_dump tests
--
2.34.1
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: [Bug] Add the missing RTE_GRAPH_TABLE case to transformLockingClause()
2026-06-09 19:33 Re: [Bug] Add the missing RTE_GRAPH_TABLE case to transformLockingClause() Ayush Tiwari <[email protected]>
2026-06-10 12:49 ` Re: [Bug] Add the missing RTE_GRAPH_TABLE case to transformLockingClause() Ashutosh Bapat <[email protected]>
@ 2026-07-09 08:18 ` Peter Eisentraut <[email protected]>
2026-07-09 09:40 ` Re: [Bug] Add the missing RTE_GRAPH_TABLE case to transformLockingClause() Ashutosh Bapat <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Peter Eisentraut @ 2026-07-09 08:18 UTC (permalink / raw)
To: Ashutosh Bapat <[email protected]>; Ayush Tiwari <[email protected]>; +Cc: SATYANARAYANA NARLAPURAM <[email protected]>; PostgreSQL Hackers <[email protected]>
On 10.06.26 14:49, Ashutosh Bapat wrote:
>> Attached is v2-0001, rebased on master and addressing
>> the comments you added in case we plan on parking the feature
>> work for once v20 opens.
> Thanks. I revised it a bit. Reworded the commit message to describe
> user facing behaviour instead of the internals, which are quite
> obvious from the code changes. Other cases in transformLockingClause()
> don't mention XXX, they just have ereport(). Removed that comment.
> Also massaged comments in the test - we usually don't describe the
> expected behaviour or error when it is obvious from the output.
> Attached revised patch.
Committed.
I also modified the comment that said "ignore JOIN, SPECIAL, ..." to
just say "ignore all other RTE kinds", because keeping that kind of list
up to date is silly. (RTE_SPECIAL hasn't existed in a long time.)
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: [Bug] Add the missing RTE_GRAPH_TABLE case to transformLockingClause()
2026-06-09 19:33 Re: [Bug] Add the missing RTE_GRAPH_TABLE case to transformLockingClause() Ayush Tiwari <[email protected]>
2026-06-10 12:49 ` Re: [Bug] Add the missing RTE_GRAPH_TABLE case to transformLockingClause() Ashutosh Bapat <[email protected]>
2026-07-09 08:18 ` Re: [Bug] Add the missing RTE_GRAPH_TABLE case to transformLockingClause() Peter Eisentraut <[email protected]>
@ 2026-07-09 09:40 ` Ashutosh Bapat <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: Ashutosh Bapat @ 2026-07-09 09:40 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Ayush Tiwari <[email protected]>; SATYANARAYANA NARLAPURAM <[email protected]>; PostgreSQL Hackers <[email protected]>
On Thu, Jul 9, 2026 at 1:48 PM Peter Eisentraut <[email protected]> wrote:
>
> On 10.06.26 14:49, Ashutosh Bapat wrote:
> >> Attached is v2-0001, rebased on master and addressing
> >> the comments you added in case we plan on parking the feature
> >> work for once v20 opens.
> > Thanks. I revised it a bit. Reworded the commit message to describe
> > user facing behaviour instead of the internals, which are quite
> > obvious from the code changes. Other cases in transformLockingClause()
> > don't mention XXX, they just have ereport(). Removed that comment.
> > Also massaged comments in the test - we usually don't describe the
> > expected behaviour or error when it is obvious from the output.
> > Attached revised patch.
>
> Committed.
>
Thanks.
> I also modified the comment that said "ignore JOIN, SPECIAL, ..." to
> just say "ignore all other RTE kinds", because keeping that kind of list
> up to date is silly. (RTE_SPECIAL hasn't existed in a long time.)
That comment is useful since it confirms that the omission is
intentional; however I agree that keeping it up to date is cumbersome.
Maybe we need some way to tie the two switch cases so that
unintentional omissions can be easily caught.
--
Best Wishes,
Ashutosh Bapat
^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2026-07-09 09:40 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-12-09 08:22 [PATCH v4 3/3] Fix timeline-tracking failure while sending a historic timeline Kyotaro Horiguchi <[email protected]>
2026-06-09 19:33 Re: [Bug] Add the missing RTE_GRAPH_TABLE case to transformLockingClause() Ayush Tiwari <[email protected]>
2026-06-10 12:49 ` Re: [Bug] Add the missing RTE_GRAPH_TABLE case to transformLockingClause() Ashutosh Bapat <[email protected]>
2026-07-09 08:18 ` Re: [Bug] Add the missing RTE_GRAPH_TABLE case to transformLockingClause() Peter Eisentraut <[email protected]>
2026-07-09 09:40 ` Re: [Bug] Add the missing RTE_GRAPH_TABLE case to transformLockingClause() Ashutosh Bapat <[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