public inbox for [email protected]
help / color / mirror / Atom feedFrom: Kwangwon Seo <[email protected]>
To: [email protected]
Subject: [PATCH] Fix quotation logic for unreserved keywords in window specifications
Date: Fri, 10 Jul 2026 18:28:23 +0900
Message-ID: <CAHJxwBWx_v=aWp7ZrGRFw2r_7MJdxYX2um3ZOmxTg4c_tL1qLA@mail.gmail.com> (raw)
Hi hackers,
I was testing views with pg_dump/pg_restore and found a case where an
unreserved keyword is not quoted correctly.
example:
-- Just a base table
CREATE TABLE t (v int);
INSERT INTO t VALUES (1), (2);
-- Create a view
CREATE VIEW v AS
SELECT count(*) OVER w2 FROM t
WINDOW "rows" AS (PARTITION BY v), w2 AS ("rows" ORDER BY v);
But the problem is, here, that quotation is gone.
-- Problem: Quotation is disappared
SELECT pg_get_viewdef('v');
pg_get_viewdef
-------------------------------------------------------------
SELECT count(*) OVER w2 AS count +
FROM t +
WINDOW rows AS (PARTITION BY v), w2 AS (rows ORDER BY v);
(1 row)
-- If you use this for recovering the view above, You'll face this error.
SELECT count(*) OVER w2 AS count
FROM t
WINDOW rows AS (PARTITION BY v), w2 AS (rows ORDER BY v);
ERROR: syntax error at or near "ORDER"
LINE 3: WINDOW rows AS (PARTITION BY v), w2 AS (rows ORDER BY v);
This breaks pg_dump/restore: the view is lost.
As a result, pg_dump/pg_restore cannot restore the view successfully.
Fortunately, I found this comment in the gram.y:
It says:
* If we see PARTITION, RANGE, ROWS or GROUPS as the first token after the
'('
* of a window_specification, we want the assumption to be that there is
* no existing_window_name; but those keywords are unreserved and so could
* be ColIds.
I think that assumption can be broken when inheritance is used
across multiple window definitions.
I mean this: w2 AS ("rows" ORDER BY v)
Some comments for patch file:
The attached patch quotes the window name when it is a keyword
in get_rule_windowspec().
Output for ordinary window names is unchanged. The window definition itself
is a ColId so no change is needed there; only the refname is quoted.
Maybe this is a known thing on the team's side, but I think, at least,
should not break
compatibility with current tools. That is the motivation of this patch...
I'd appreciate any feedback or suggestions...!!
Best regards...
Attachments:
[text/x-patch] v1-0001-Fix-quotation-logic-for-unreserved-keywords-in-wi.patch (4.5K, ../CAHJxwBWx_v=aWp7ZrGRFw2r_7MJdxYX2um3ZOmxTg4c_tL1qLA@mail.gmail.com/3-v1-0001-Fix-quotation-logic-for-unreserved-keywords-in-wi.patch)
download | inline diff:
From 72c0dd0ea78155daa348845fcd74b9102a404056 Mon Sep 17 00:00:00 2001
From: Kwangwon Seo <[email protected]>
Date: Fri, 10 Jul 2026 16:26:33 +0900
Subject: [PATCH v1] Fix quotation logic for unreserved keywords in window
specifications
---
src/backend/utils/adt/ruleutils.c | 30 +++++++++++++++++++++++++++-
src/test/regress/expected/window.out | 12 +++++++++++
src/test/regress/sql/window.sql | 7 +++++++
3 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 819631781c0..6084b48f435 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -450,6 +450,7 @@ static void get_rule_orderby(List *orderList, List *targetList,
static void get_rule_windowclause(Query *query, deparse_context *context);
static void get_rule_windowspec(WindowClause *wc, List *targetList,
deparse_context *context);
+static void appendWindowRefName(StringInfo buf, const char *refname);
static void get_window_frame_options(int frameOptions,
Node *startOffset, Node *endOffset,
deparse_context *context);
@@ -7159,7 +7160,7 @@ get_rule_windowspec(WindowClause *wc, List *targetList,
appendStringInfoChar(buf, '(');
if (wc->refname)
{
- appendStringInfoString(buf, quote_identifier(wc->refname));
+ appendWindowRefName(buf, wc->refname);
needspace = true;
}
/* partition clauses are always inherited, so only print if no refname */
@@ -7201,6 +7202,33 @@ get_rule_windowspec(WindowClause *wc, List *targetList,
appendStringInfoChar(buf, ')');
}
+/*
+ * Emit the name of the window definition.
+ *
+ * PARTITION, RANGE, ROWS, and GROUPS have the same precedence as IDENT
+ * at the start of a window specification, preventing them from being
+ * recognized as an existing_window_name (see opt_existing_window_name
+ * in gram.y). Since these are unreserved keywords, quote_identifier()
+ * does not quote them, causing the generated SQL to fail when reparsed.
+ * Therefore, quote any keyword here rather than maintaining a list.
+ */
+static void
+appendWindowRefName(StringInfo buf, const char *refname)
+{
+ const char *quoted = quote_identifier(refname);
+
+ if (quoted == refname &&
+ ScanKeywordLookup(refname, &ScanKeywords) >= 0)
+ {
+ /* quote_identifier() left it bare, so it needs no escaping */
+ appendStringInfoChar(buf, '"');
+ appendStringInfoString(buf, refname);
+ appendStringInfoChar(buf, '"');
+ }
+ else
+ appendStringInfoString(buf, quoted);
+}
+
/*
* Append the description of a window's framing options to context->buf
*/
diff --git a/src/test/regress/expected/window.out b/src/test/regress/expected/window.out
index c0bde1c5eec..5080b415e0b 100644
--- a/src/test/regress/expected/window.out
+++ b/src/test/regress/expected/window.out
@@ -1361,6 +1361,18 @@ SELECT pg_get_viewdef('v_window');
FROM generate_series(now(), (now() + '@ 100 days'::interval), '@ 1 hour'::interval) i(i);
(1 row)
+-- A window name that is an unreserved keyword cannot be an existing_window_name
+CREATE TEMP VIEW v2_window_unreserved_kw AS
+ SELECT count(*) OVER w2 FROM generate_series(1, 1) s(v)
+ WINDOW "rows" AS (PARTITION BY v), w2 AS ("rows" ORDER BY v);
+SELECT pg_get_viewdef('v2_window_unreserved_kw');
+ pg_get_viewdef
+---------------------------------------------------------------
+ SELECT count(*) OVER w2 AS count +
+ FROM generate_series(1, 1) s(v) +
+ WINDOW rows AS (PARTITION BY v), w2 AS ("rows" ORDER BY v);
+(1 row)
+
-- test overflow frame specifications
SELECT sum(unique1) over (rows between current row and 9223372036854775807 following exclude current row),
unique1, four
diff --git a/src/test/regress/sql/window.sql b/src/test/regress/sql/window.sql
index 8e6f92d94c7..396b95d0d39 100644
--- a/src/test/regress/sql/window.sql
+++ b/src/test/regress/sql/window.sql
@@ -330,6 +330,13 @@ CREATE TEMP VIEW v_window AS
SELECT pg_get_viewdef('v_window');
+-- A window name that is an unreserved keyword cannot be an existing_window_name
+CREATE TEMP VIEW v2_window_unreserved_kw AS
+ SELECT count(*) OVER w2 FROM generate_series(1, 1) s(v)
+ WINDOW "rows" AS (PARTITION BY v), w2 AS ("rows" ORDER BY v);
+
+SELECT pg_get_viewdef('v2_window_unreserved_kw');
+
-- test overflow frame specifications
SELECT sum(unique1) over (rows between current row and 9223372036854775807 following exclude current row),
unique1, four
--
2.52.0
view thread (49+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: [PATCH] Fix quotation logic for unreserved keywords in window specifications
In-Reply-To: <CAHJxwBWx_v=aWp7ZrGRFw2r_7MJdxYX2um3ZOmxTg4c_tL1qLA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox