| From: | Henson Choi <assam258(at)gmail(dot)com> |
|---|---|
| To: | Tatsuo Ishii <ishii(at)postgresql(dot)org> |
| Cc: | jian(dot)universality(at)gmail(dot)com, zsolt(dot)parragi(at)percona(dot)com, sjjang112233(at)gmail(dot)com, vik(at)postgresfriends(dot)org, er(at)xs4all(dot)nl, jacob(dot)champion(at)enterprisedb(dot)com, david(dot)g(dot)johnston(at)gmail(dot)com, peter(at)eisentraut(dot)org, li(dot)evan(dot)chao(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: Row pattern recognition |
| Date: | 2026-08-31 13:21:42 |
| Message-ID: | CAAAe_zCXuuCYekjpSDWipby6-Joe2LOcS59oaNQtE0kc+0oV6g@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Tatsuo, Jian,
Attached as a wip patch is one change I would like an opinion on
before it goes into the series. It is not in v51; it sits after that
posting on my branch.
The case
It is in the regression suite already, written down as a failure
rather than as a fix. The commit that made get_rule_define() print
bare column names put it there, and said the ambiguity would be closed
later in the series by the commit that pins those names. This is that
commit.
Two tables joined, a DEFINE that names a column of one of them, and
then the other side acquires a column of the same name:
CREATE TEMP TABLE sa (id int, price int);
CREATE TEMP TABLE sb (id int, qty int);
CREATE TEMP VIEW sv AS
SELECT a.id, count(*) OVER w AS cnt
FROM sa a JOIN sb b ON a.id = b.id
WINDOW w AS (ORDER BY a.id ROWS BETWEEN CURRENT ROW AND UNBOUNDED
FOLLOWING PATTERN (UP+) DEFINE UP AS price > 0);
ALTER TABLE sb ADD COLUMN price int;
pg_get_viewdef() then prints "up AS price > 0" unchanged, and that
text no longer means anything. Feeding it back gives
ERROR: column reference "price" is ambiguous
The test carried that state deliberately: the comment on sv said it
is unrestorable and that is why it had to be temporary and dropped
at the end, sv3 showed the deparsed text being rejected on a fresh
CREATE VIEW, and sv4 showed the column alias list that a person would
have to write by hand for the view to survive.
The reason the deparser has nothing better to print is that the
standard gives the qualifier slot in a DEFINE expression to pattern
variables. A qualified column is not something the parser reads back
there -- every qualified spelling is rejected -- which leaves the bare
name as the only re-parseable one, and that is why that earlier commit
sets varprefix to false for the clause. So the bare name has to
resolve exactly as printed. A column merged by USING is in the same
position, and that is where I took the implementation from.
What it prints now
The same query, under the names the patch's own test uses, with the
psql padding taken out:
SELECT j1.id,
count(*) OVER w AS cnt
FROM rpr_pin_j1 j1
JOIN rpr_pin_j2 j2(id, qty, price_1) ON j1.id = j2.id
WINDOW w AS (ORDER BY j1.id ROWS BETWEEN CURRENT ROW AND UNBOUNDED
FOLLOWING
AFTER MATCH SKIP PAST LAST ROW
INITIAL
PATTERN (a+)
DEFINE
a AS price > 0);
The alias list sv4 had to carry by hand is what the deparser emits on
its own now: the newcomer moves aside to price_1, the DEFINE reference
keeps meaning what it meant, and the view restores.
How
set_using_names() already meets the same problem for a column merged
by USING: it too is printed bare, so it too has to keep resolving. It
reserves the name -- no other relation of the query may be given it,
and the column itself is exempt from renaming -- and the rest of the
deparser reacts on its own. The alias list above is that reaction,
not new code.
set_define_names() is the same reservation made for DEFINE references.
It runs once, right after set_using_names(), walks each DEFINE clause,
and pins the name of every column the clause reads.
The collision is then settled by machinery that was already there. A
column whose name is reserved is passed over when aliases are assigned,
so it prints as written; and when the same pass reaches the relation
that acquired the colliding name, that name no longer passes the
uniqueness test, so it is treated as any other duplicate -- a digit is
appended, and a column alias list is printed for that relation so the
new name is declared where it is used.
Whether the direction taken here is the right one, whether there is a
defect in it, and whether it carries a side effect I have not seen --
I would be grateful for your opinion.
Best regards,
Henson
| Attachment | Content-Type | Size |
|---|---|---|
| wip-0001-pin-define-column-names.txt | text/plain | 33.9 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Bertrand Drouvot | 2026-08-31 13:24:06 | Re: pgstat: Flush some statistics within running transactions, take 2 |
| Previous Message | Andrei Lepikhov | 2026-08-31 13:12:45 | Re: RFC: Logging plan of the running query |