On Wed, 19 Feb 2025 at 01:42, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
>
> One thing I don't like about this is that it's introducing more code
> duplication between pullup_replace_vars() and
> ReplaceVarsFromTargetList(). Those already had a lot of code in common
> before RETURNING OLD/NEW was added, and this is duplicating even more
> code. I think it'd be better to refactor so that they share common
> code, since it has become quite complex, and it would be better to
> have just one place to maintain. Attached is an updated patch doing
> that.
>
I've been doing some more testing of this, and attached is another
update, improving a few comments and adding regression tests based on
the cases discussed so far here.
One of the new regression tests fails, which actually appears to be a
pre-existing grouping sets bug, due to the fact that only non-Vars are
wrapped in PHVs. This bug can be triggered without virtual generated
columns:
CREATE TABLE t (a int, b int);
INSERT INTO t VALUES (1, 1);
SELECT * FROM (SELECT a, a AS b FROM t) AS vt
GROUP BY GROUPING SETS (a, b)
HAVING b = 1;
a | b
---+---
1 |
(1 row)
whereas the result should be
a | b
---+---
| 1
(1 row)
For reference, this code dates back to 90947674fc.
Regards,
Dean
