schema: review-report-v2
source: postgresql-master
slug: commitfest-7057-review
target: CommitFest 7057 — doc: clarify AS requirement when VALUES used in a FROM clause
base: 2a2dab67dc40a41a003545308423c1d3def02dea
head: 2a2dab67dc40a41a003545308423c1d3def02dea
diff_sha256: 73dd56113caa057d2c5f71ffdb0b32ae4944b754deb73ceab78843dcba35f43e
verdict: ready-for-committer
blocker_counts:
  verified: 0
  unverified: 0
coverage_requirement: not-applicable
model: glm-5.2:cloud
seats:
  - pg-patch-reviewer (glm-5.2:cloud)
  - pg-security-reviewer (glm-5.2:cloud)
  - pg-finding-verifier (glm-5.2:cloud)
handoff_reports:
  - commitfest-7057-review-handoff-patch-reviewer
  - commitfest-7057-review-handoff-security-reviewer
  - commitfest-7057-review-handoff-verifier
thread_ledger_slug: commitfest-7057-review-thread-ledger

CommitFest 7057 Review — doc: clarify AS requirement when VALUES used in a FROM clause

• Patch: [PATCH] doc: clarify AS requirement when VALUES used in a FROM clause — Ian Barwick (barwick@gmail.com)
• Thread: pgsql-hackers thread-id 652493 (2026-07-24)
• Scope: working-tree-diff@HEAD 2a2dab67dc40a41a003545308423c1d3def02dea, diff sha256 73dd56113caa057d2c5f71ffdb0b32ae4944b754deb73ceab78843dcba35f43e
• Changed file: doc/src/sgml/ref/values.sgml (+6/-7), documentation-only
• Base = HEAD: 2a2dab67dc40a41a003545308423c1d3def02dea (uncommitted staged diff; merge-base equals HEAD)
• Verdict: ready-for-committer — 0 blockers, 0 majors; 3 verified NITs (optional wording/style)

Overview

The patch rewrites a single paragraph in doc/src/sgml/ref/values.sgml (lines 201–207) that follows the FROM (VALUES ...) AS t(...) example block. The OLD text stated "an AS clause is required when VALUES is used in a FROM clause" — this was inaccurate. The NEW text states "it's not necessary to have an AS clause specifying names for each column, but it's good practice to do so."

The technical correction is accurate: in src/backend/parser/gram.y:14437 the production table_ref: select_with_parens opt_alias_clause makes the alias optional for any subquery in FROM (including VALUES), and src/backend/parser/parse_relation.c:2264–2308 (addRangeTableEntryForValues) defaults refname to *VALUES* and fills column names as column1, column2, … when no alias is supplied. The contributor notes this has not been the case since PostgreSQL 16 (commit bcedd8f).

Findings

All three findings are optional documentation wording/style NITs. None block the patch.

diff --git a/doc/src/sgml/ref/values.sgml b/doc/src/sgml/ref/values.sgml
--- a/doc/src/sgml/ref/values.sgml
+++ b/doc/src/sgml/ref/values.sgml
@@ -198,11 +198,10 @@
       WHERE employees.depno = v.depno AND employees.sales >= v.target;
     </programlisting>

-   Note that an <literal>AS</literal> clause is required when <command>VALUES</command>
-   is used in a <literal>FROM</literal> clause, just as is true for
-   <command>SELECT</command>.  It is not required that the <literal>AS</literal> clause
-   specify names for all the columns, but it's good practice to do so.
-   (The default column names for <command>VALUES</command> are <literal>column1</literal>,
+   If <command>VALUES</command> is used in a <literal>FROM</literal> clause, it is not
+   necessary to have an <literal>AS</literal> clause specifying names for each column,
+   but it's good practice to do so.  (The default column names for
+   <command>VALUES</command> are <literal>column1</literal>,
      <literal>column2</literal>, etc. in <productname>PostgreSQL</productname>, but
      these names might be different in other database systems.)
     </para>

diff --git a/doc/src/sgml/ref/values.sgml b/doc/src/sgml/ref/values.sgml
--- a/doc/src/sgml/ref/values.sgml
+++ b/doc/src/sgml/ref/values.sgml
@@ -198,13 +198,13 @@
       WHERE employees.depno = v.depno AND employees.sales >= v.target;
     </programlisting>

-   Note that an <literal>AS</literal> clause is required when <command>VALUES</command>
-   is used in a <literal>FROM</literal> clause, just as is true for
-   <command>SELECT</command>.  It is not required that the <literal>AS</literal> clause
-   specify names for all the columns, but it's good practice to do so.
-   (The default column names for <command>VALUES</command> are <literal>column1</literal>,
-   <literal>column2</literal>, etc. in <productname>PostgreSQL</productname>, but
-   these names might be different in other database systems.)
+   If <command>VALUES</command> is used in a <literal>FROM</literal> clause, it is not
+   necessary to have an <literal>AS</literal> clause, nor is it necessary that the
+   <literal>AS</literal> clause specify names for all the columns, but it's good
+   practice to do so.  (The default column names for <command>VALUES</command> are
+   <literal>column1</literal>, <literal>column2</literal>, etc. in
+   <productname>PostgreSQL</productname>, but these names might be different in
+   other database systems.)
     </para>

     <para>

diff --git a/doc/src/sgml/ref/values.sgml b/doc/src/sgml/ref/values.sgml
--- a/doc/src/sgml/ref/values.sgml
+++ b/doc/src/sgml/ref/values.sgml
@@ -201,7 +201,7 @@
       Note that an <literal>AS</literal> clause is required when <command>VALUES</command>
       is used in a <literal>FROM</literal> clause, just as is true for
       <command>SELECT</command>.  It is not required that the <literal>AS</literal> clause
-     specify names for all the columns, but it's good practice to do so.
+     specify names for all the columns, but it is good practice to do so.
      (The default column names for <command>VALUES</command> are <literal>column1</literal>,
      <literal>column2</literal>, etc. in <productname>PostgreSQL</productname>, but
      these names might be different in other database systems.)

Verification Summary

All three findings are documentation wording/style NITs verified by reading the actual SGML file and the HEAD original. No code defect exists; no behavior changes. The parser evidence cited for F2 was confirmed at src/backend/parser/parse_relation.c:2295-2303 (the while (numaliases < numcolumns) loop fills unspecified alias columns, proving partial alias lists are accepted). The SELECT parallel cited for F1 is covered at doc/src/sgml/queries.sgml:663-664.

No BLOCKER or MAJOR findings. No refuted findings.

Repair Validation Summary

All three candidate patches are HEAD-bound unified diffs against doc/src/sgml/ref/values.sgml. Each was exercised in a disposable git worktree copy at HEAD (2a2dab67dc4):

1. git apply --check succeeded.
2. git apply succeeded.
3. git diff --stat matched expected counts (F1: 4 ins/5 del; F2: 7 ins/7 del; F3: 1 ins/1 del).
4. The postimage region was read back and matched the code_example exactly.
5. xmllint --noout doc/src/sgml/ref/values.sgml reported the patched file well-formed (no parse errors).
6. The worktree was reverted with git checkout between patches and removed afterward; no edits were made to the reviewed (read-only) target worktree.

A full make docs / openjade build was NOT run because the SGML DTD/toolchain is not resolvable inside the disposable worktree and these are optional NITs; the SGML suffix rule (single-file well-formedness) was satisfied via xmllint --noout. Per verifier guidance for optional NITs, repair.validation.status is recorded as not-run with an explicit reason and the actually-exercised methods listed, rather than claiming a full doc-build passed.

Security

Verified clean. Documentation-only change with no security-relevant impact. No C code, no DDL, no backend behavior, no privilege/role/auth changes, no search_path/SECURITY DEFINER, no catalog writes, no crypto, no extension SQL. The new text retains the recommendation that naming columns is good practice and retains the warning that default names (column1, column2) are PostgreSQL-specific. It does not encourage relying on implicit positional column names for security-relevant comparisons. No pathway that would mislead users into an insecure pattern.

Test Adequacy

Documentation-only change; there are no executable regression tests to add or run. The validation vehicle is a documentation build (make / make html / make check in doc/src/sgml). SGML well-formedness was checked by inspection of the changed paragraph and by xmllint --noout on the patched file in a disposable worktree. No pg_regress or pgindent concerns apply to a prose-only SGML edit. No test-gap findings (static test-adequacy rubric: no changed branches/error paths/boundaries in C code).

pgsql-hackers Thread Ledger

Status: reconciled. Ledger report slug: commitfest-7057-review-thread-ledger.

One pgsql-hackers thread (thread-id 652493) discusses this patch:

1. Ian Barwick (barwick@gmail.com), 2026-07-24T09:12:42Z, [PATCH] doc: clarify AS requirement when VALUES used in a FROM clause — patch submission citing a prior postgresql.org comment that the VALUES docs still claim an AS clause is required in a FROM clause; notes this has not been true since PostgreSQL 16 (commit bcedd8f).
2. Laurenz Albe (laurenz.albe@cybertec.at), 2026-07-24T09:51:33Z, Re: [PATCH] ... — agrees the text should be changed but suggests improved wording ("Note that if VALUES is used in a FROM clause, it is not necessary to specify an AS clause, but it is good practice to do so.") to avoid the misreading that one must specify an AS clause but can omit column names.

Finding links:

• F1 (SELECT parallel): new — not raised on the thread.
• F2 (partial column lists): already-raised — directly addressed by Laurenz Albe's reply.
• F3 (style): new — not raised on the thread.

No committer reply; no blocking objection. Laurenz Albe's suggestion is a wording-improvement review comment, not a live objection; the actual patch wording substantially addresses the concern.

Changed-Code Coverage

Coverage requirement: not-applicable.

Reason: Documentation-only change to doc/src/sgml/ref/values.sgml (SGML prose). No C/compiled implementation code, no executable branches, no changed functions or symbols. There are no changed instrumentable implementation paths; coverage measurement is not applicable.

Non-instrumented changes:

Metrics: Not applicable (no changed executable lines, branches, or functions).

Verified-Clean

• SGML well-formedness: All inline tags in the new paragraph are balanced (<command>VALUES</command> ×2, <literal>FROM</literal>, <literal>AS</literal>, <literal>column1</literal>, <literal>column2</literal>, <productname>PostgreSQL</productname>). The surrounding <para> (line 187) closes at line 207; the preceding <programlisting> (line 191) closes at line 199. No tag imbalance introduced.
• Parser contract verified: gram.y:14437 (select_with_parens opt_alias_clause) and gram.y:14615 (opt_alias_clause: alias_clause) confirm alias optionality; parse_relation.c:2274,2290,2295-2303 confirm default refname *VALUES* and default column names column1..columnN.
• Empirical run: SELECT * FROM (VALUES (1,2), (3,4)) LIMIT 2; → 2 rows, columns column1 | column2. Confirms the NEW claim and refutes the OLD.
• Consistency with queries.sgml: queries.sgml:610, 642-643, 658-664, 668-673 are all consistent with the NEW wording. No contradiction.
• No catversion / OID / pg_dump / catalog impact: documentation-only; none applicable.
• Security: verified clean (see Security section).

Aggregated Test Plan

No executable tests required (docs-only). Validation vehicle: documentation build + SGML well-formedness inspection. Optional NIT repair patches (F1/F2/F3) validated via git apply --check + xmllint --noout in a disposable worktree at HEAD 2a2dab67dc4.

Residual Risk

None. The patch corrects an inaccurate statement and aligns the reference docs with long-standing parser behavior and the tutorial/from-clause documentation in queries.sgml. The three NITs are optional wording/style improvements a committer may fold in at their discretion.

Provenance / Model Routes

• Coordinator: pg-review-harness (glm-5.2:cloud)
• pg-patch-reviewer: glm-5.2:cloud — handoff commitfest-7057-review-handoff-patch-reviewer
• pg-security-reviewer: glm-5.2:cloud — handoff commitfest-7057-review-handoff-security-reviewer
• pg-finding-verifier: glm-5.2:cloud — handoff commitfest-7057-review-handoff-verifier
• Coverage: not-applicable (docs-only); no pg-change-coverage-reviewer seat required
• Thread ledger: commitfest-7057-review-thread-ledger

Deterministic Verdict

• Verdict: ready-for-committer
• Blocker counts: verified 0 / unverified 0
• Rationale: No BLOCKER findings. No correctness/security MAJOR findings. No test-only MAJORs. No live thread objection (Laurenz Albe's reply is a wording-improvement suggestion, not a blocking objection). All three findings are NITs → ready-for-committer.

UI

• Report: http://127.0.0.1:8848/ui/reports/postgresql-master/commitfest-7057-review

Note

This report is immutable for the frozen target identity (working-tree-diff@HEAD 2a2dab67dc40a41a003545308423c1d3def02dea, diff sha256 73dd56113caa057d2c5f71ffdb0b32ae4944b754deb73ceab78843dcba35f43e). Post-review repairs or a new commit require a new HEAD/content-keyed report.