From: | Richard Guo <guofenglinux(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | tlhquynh(at)gmail(dot)com, pgsql-bugs(at)lists(dot)postgresql(dot)org |
Subject: | Re: BUG #18589: pg_get_viewdef returns wrong query |
Date: | 2024-08-27 08:46:17 |
Message-ID: | CAMbWs4_D7CVSxJVmQQA+5KWz9XxqOpEKgQvE9Pfoc8pXPwJNTw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-bugs |
On Tue, Aug 27, 2024 at 1:11 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> The second patch attached is what I think we should really do.
> What it does is to prefix Vars only in the actually-troublesome
> case where there is a conflicting SELECT-list entry, so that you
> get the clutter only when doing something that's fairly ill-advised.
I also think this is a better way.
> To do that, get_variable needs access to the SELECT list. We already
> have that available in deparse_context, but the field name is
> "windowTList" which seems rather misleading if it's to be used for
> other purposes too. So I renamed and relocated that field to make
> things less confusing.
+1 to this part.
> The other debatable point in the patch is
> how to tell get_variable that we're considering an ORDER BY item.
> I chose to re-use the special_exprkind field that's already there,
> but I'm not totally convinced that that's a better solution than
> adding another bool field. The problem is that we also use
> get_rule_sortgroupclause for GROUP BY, so that it's temporarily hiding
> the EXPR_KIND_GROUP_BY setting.
It seems to me that this might result in different behavior from
previous when deparsing Vars in a GROUP BY list, due to overriding
special_exprkind from EXPR_KIND_GROUP_BY to EXPR_KIND_ORDER_BY.
For example, with this patch:
create table t (x1 int, x2 int);
create view v as select x1 as x2, x2 as x1 from t group by x1, x2;
select pg_get_viewdef('v', true);
pg_get_viewdef
------------------------
SELECT x1 AS x2, +
x2 AS x1 +
FROM t +
GROUP BY t.x1, t.x2;
(1 row)
It seems that the prefixes in the GROUP BY Vars are unnecessary.
> (On the whole, I don't think special_exprkind is all that
> well thought out: aside from having no useful documentation, this
> experience shows that it's not nearly as extensible as the author
> probably believed. I wonder if we should replace it with
> "bool in_group_by" or the like.)
From my brief 2-minute look at how special_exprkind is used, it seems
that special_exprkind is either EXPR_KIND_GROUP_BY or EXPR_KIND_NONE.
Since it doesn't seem to be extensible as expected, I agree that maybe
a bool field is more straightforward. Then we can have another bool
field for ORDER BY, avoiding the need to override the in-group-by
setting.
Thanks
Richard
From | Date | Subject | |
---|---|---|---|
Next Message | Richard Guo | 2024-08-27 10:03:44 | Re: BUG #18589: pg_get_viewdef returns wrong query |
Previous Message | Ram Pratap Maurya | 2024-08-27 08:44:35 | Bug in PostgreSQL 15 : Facing error in PG15 |