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

First seen: 2026-05-07 07:37:13+00:00 · Messages: 2 · Participants: 2

Latest Update

2026-05-08 · opus 4.7

Core Problem

transformLockingClause() in src/backend/parser/analyze.c is the parser-analyze stage routine responsible for validating FOR UPDATE/FOR NO KEY UPDATE/FOR SHARE/FOR KEY SHARE row-locking clauses and dispatching behavior based on the kind of Range Table Entry (RTE) they target. The function contains a switch over RangeTblEntry.rtekind with user-friendly error messages for each non-lockable RTE kind (subquery, join, function, values, CTE, tablefunc, result, namedtuplestore, etc.), ending with a catch-all elog(ERROR, "unrecognized RTE type: %d", ...).

PostgreSQL's SQL/PGQ support (property graphs, introduced via the CREATE PROPERTY GRAPH / GRAPH_TABLE(...) machinery) added a new RTE kind, RTE_GRAPH_TABLE (numeric value 8 in the enum at the time of this report). The locking-clause analyzer was never taught about this new kind. As a result, any attempt to apply a row-locking clause to a GRAPH_TABLE alias falls through the switch and hits the internal elog path, producing:

ERROR:  unrecognized RTE type: 8

This is a classic internal-error-leak bug: an elog (as opposed to ereport with a proper SQLSTATE) is meant to signal a "can't happen" condition. Here it is reachable from SQL, which means:

  1. Users see a confusing, implementation-detail message with no cursor position.
  2. The SQLSTATE is XX000 (internal error) rather than 0A000 (ERRCODE_FEATURE_NOT_SUPPORTED), which misleads driver/tool error handling and monitoring.
  3. It hints that the feature's integration with the rest of the parser was incomplete — a canary for other missed switch sites.

Architectural Significance

Whenever a new RTEKind is introduced, every exhaustive switch over rtekind across the backend must be audited. Known sites include transformLockingClause, markRTEForSelectPriv, expandRTE, get_rte_attribute_*, addRangeTableEntry*, ruleutils printing, and planner RTE handling. Missing one of these sites is a common class of defect when extending the range-table abstraction. This patch is an instance of that recurring maintenance burden — SQL/PGQ is a fairly large feature surface, and its introduction added an RTE kind that must be handled consistently everywhere.

The locking-clause case is particularly user-visible because FOR UPDATE etc. is commonly written reflexively by ORMs and applications, and GRAPH_TABLE is inherently a derived/computed relation (like FUNCTION or VALUES) against which row locking is semantically meaningless — there is no base heap tuple with an xmin/xmax to lock. So the correct behavior is clearly to reject with a FEATURE_NOT_SUPPORTED diagnostic, matching the precedent already set for RTE_FUNCTION, RTE_TABLEFUNC, RTE_VALUES, etc.

Proposed Fix

Satya's patch adds a new case RTE_GRAPH_TABLE: arm to the switch in transformLockingClause() that issues:

ereport(ERROR,
        (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
         errmsg("FOR %s cannot be applied to a GRAPH_TABLE",
                LCS_asString(lc->strength)),
         parser_errposition(pstate, thisrel->location)));

This mirrors exactly the pattern used for sibling derived-relation RTE kinds: proper SQLSTATE, human-readable message parameterized by the locking strength via LCS_asString(), and a cursor position derived from the RangeVar/alias location so psql can underline the offending token.

The patch also adds regression tests covering all four lock strengths (FOR UPDATE, FOR NO KEY UPDATE, FOR SHARE, FOR KEY SHARE) against a GRAPH_TABLE alias.

Tradeoff: test granularity

Satya explicitly raises the question of whether to keep all four test cases or trim to one, observing that "the code path looks simple." The technical consideration:

Zhenwei Shang's response favors keeping all four since they are "light" — a reasonable position given Postgres regression-test convention already exhaustively covers the strengths for analogous RTE kinds (see src/test/regress/expected/rowsecurity.out and the for-update tests in select_for_update).

Participant Weight

Neither author here is a committer; Satya is contributing a bug-fix patch and Zhenwei Shang offers a reviewer +1. The patch is mechanically minimal and follows established local convention exactly, so it is the kind of change a committer would typically push with little debate after confirming no other rtekind switches are missing the case. The thread as captured contains no committer response yet.

Implications and Follow-ups

The real question this bug raises (not explicitly discussed in the two messages but implied) is whether other rtekind switches in the backend similarly lack RTE_GRAPH_TABLE handling. A thorough fix would grep for every switch on rtekind and audit each — the pattern of "internal elog reachable from SQL" often travels in packs when a new RTE kind is added. A committer reviewing this patch would likely want that audit done (or confirmed already done when RTE_GRAPH_TABLE was added) before commit.

A secondary consideration is message wording consistency. The existing messages use forms like "SELECT FOR UPDATE/SHARE cannot be applied to a function" — Satya's chosen wording "FOR %s cannot be applied to a GRAPH_TABLE" using LCS_asString matches the more modern convention used for RTE_TABLEFUNC and is appropriate.

Summary

A straightforward but legitimate bug fix closing a user-visible elog leak introduced when SQL/PGQ's RTE_GRAPH_TABLE was added without updating transformLockingClause(). The patch is idiomatic, follows existing precedent for derived-relation RTE kinds, and correctly reports ERRCODE_FEATURE_NOT_SUPPORTED with a cursor position. The only minor design discussion is test coverage breadth, with consensus forming around keeping all four lock-strength cases.