Thread: View with duplicate GROUP BY entries
Hi All,
While reviewing patch for similar problem in postgres_fdw [1], I
noticed that we don't use positional notation while creating the view.
This might introduced anomalies when GROUP BY entries are
non-immutable.
E.g.
postgres=# create view aggv as select c2 c21, c2 c22 from t1 group by 1, 2;
postgres=# \d+ aggv View "public.aggv"Column | Type | Collation | Nullable | Default |
Storage| Description
--------+---------+-----------+----------+---------+---------+-------------c21 | integer | | |
| plain |c22 | integer | | | | plain |
View definition:SELECT t1.c2 AS c21, t1.c2 AS c22 FROM t1 GROUP BY t1.c2, t1.c2;
That's not a problematic case, but following is
create view aggv_rand as select random() c21, random() c22 from t1
group by 1, 2;
CREATE VIEW
postgres=# \d+ aggv_rand View "public.aggv_rand"Column | Type | Collation |
Nullable| Default | Storage
| Description
--------+------------------+-----------+----------+---------+---------+-------------c21 | double precision |
| | | plain |c22 | double precision | | | | plain |
View definition:SELECT random() AS c21, random() AS c22 FROM t1 GROUP BY (random()), (random());
Notice four instances of random() instead of two in the original definition.
What is printed in \d+ output also goes into dump file, so when such a
view is restored, it will have a different behaviour that the intended
one.
[1] http://postgr.es/m/10660.1510093781@sss.pgh.pa.us
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> writes:
> While reviewing patch for similar problem in postgres_fdw [1], I
> noticed that we don't use positional notation while creating the view.
> This might introduced anomalies when GROUP BY entries are
> non-immutable.
Yeah, we probably ought to make more of an effort to regenerate the
original query wording. I do not think that forcing positional notation
is a suitable answer in this case, because it would result in converting
SQL-standard queries to nonstandard ones. We might have to extend the
parsetree representation so that we can tell whether positional notation
was used to begin with.
regards, tom lane
On Tue, Nov 21, 2017 at 12:05 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> writes: >> While reviewing patch for similar problem in postgres_fdw [1], I >> noticed that we don't use positional notation while creating the view. >> This might introduced anomalies when GROUP BY entries are >> non-immutable. > > Yeah, we probably ought to make more of an effort to regenerate the > original query wording. I do not think that forcing positional notation > is a suitable answer in this case, because it would result in converting > SQL-standard queries to nonstandard ones. Who cares? The other end is presumptively PostgresSQL, because this is postgres_fdw. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Nov 21, 2017 at 12:05 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Yeah, we probably ought to make more of an effort to regenerate the
>> original query wording. I do not think that forcing positional notation
>> is a suitable answer in this case, because it would result in converting
>> SQL-standard queries to nonstandard ones.
> Who cares? The other end is presumptively PostgresSQL, because this
> is postgres_fdw.
No, you missed the context. Yes, the original problem is in postgres_fdw,
and there indeed it seems fine to emit GROUP BY 1,2. What Ashutosh is
pointing out is that ruleutils.c can emit a representation of a view
that fails to preserve its original semantics, thus causing dump/reload
problems that have nothing at all to do with FDWs. And what I'm pointing
out is that we don't like pg_dump to emit nonstandard representations
of objects that were created with perfectly standard-compliant queries;
therefore emitting GROUP BY 1,2 isn't good if the query wasn't spelled
like that to begin with.
regards, tom lane
On Tue, Nov 21, 2017 at 1:11 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Tue, Nov 21, 2017 at 12:05 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> Yeah, we probably ought to make more of an effort to regenerate the >>> original query wording. I do not think that forcing positional notation >>> is a suitable answer in this case, because it would result in converting >>> SQL-standard queries to nonstandard ones. > >> Who cares? The other end is presumptively PostgresSQL, because this >> is postgres_fdw. > > No, you missed the context. Yes, the original problem is in postgres_fdw, > and there indeed it seems fine to emit GROUP BY 1,2. What Ashutosh is > pointing out is that ruleutils.c can emit a representation of a view > that fails to preserve its original semantics, thus causing dump/reload > problems that have nothing at all to do with FDWs. Oh, woops. Sorry. > And what I'm pointing > out is that we don't like pg_dump to emit nonstandard representations > of objects that were created with perfectly standard-compliant queries; > therefore emitting GROUP BY 1,2 isn't good if the query wasn't spelled > like that to begin with. That's not an unreasonable goal, but I'm not sure how much effort I'd personally be willing to expend on it. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company