Thread

1. [Bug] Add the missing RTE_GRAPH_TABLE case to transformLockingClause()

   SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com> — 2026-05-07T07:37:13Z

   Hi Hackers,
   
   Add the missing RTE_GRAPH_TABLE case to transformLockingClause(). Without
   this four
   row-locking strengths applied to a GRAPH_TABLE alias triggers not give a
   user friendly
   error message.
   
   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. 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?
   
   Thanks,
   Satya
   

2. Re: [Bug] Add the missing RTE_GRAPH_TABLE case to transformLockingClause()

   Zhenwei Shang <a934172442@gmail.com> — 2026-05-07T08:40:43Z

   在 2026/5/7 15:37, SATYANARAYANA NARLAPURAM 写道:
   > Hi Hackers,
   >
   > Add the missing RTE_GRAPH_TABLE case to transformLockingClause().
   > Without this four
   > row-locking strengths applied to a GRAPH_TABLE alias triggers not give
   > a user friendly
   > error message.
   >
   > 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. 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?
   >
   > Thanks,
   > Satya
   
   The change is straightforward, clearly an improvement to me. I think you
   may keep all 4 cases as they are light.
   
   Regards,
   Zhenwei Shang
   
   
   
   

3. Re: [Bug] Add the missing RTE_GRAPH_TABLE case to transformLockingClause()

   Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2026-05-13T05:27:13Z

   On Thu, May 7, 2026 at 1:07 PM SATYANARAYANA NARLAPURAM
   <satyanarlapuram@gmail.com> wrote:
   >
   > Hi Hackers,
   >
   > Add the missing RTE_GRAPH_TABLE case to transformLockingClause(). Without this four
   > row-locking strengths applied to a GRAPH_TABLE alias triggers not give a user friendly
   > error message.
   >
   > 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?
   
   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.
   
   -- 
   Best Wishes,
   Ashutosh Bapat