Parsing of aggregate ORDER BY clauses
in [PgSql]
|
Prev: Earn more for a good life. adipoma absinth albizias
Next: [COMMITTERS] pgsql: pgindent run for 9.0, second run
From: Hitoshi Harada on 19 Jul 2010 19:08 2010/7/19 Tom Lane <tgl(a)sss.pgh.pa.us>: > I looked into the problem reported here: > http://archives.postgresql.org/pgsql-bugs/2010-07/msg00119.php > > 1. Postpone coercion of the function inputs till after processing of > the ORDER BY/DISTINCT decoration. �This isn't too good because then > we'll be using the "wrong" data type for deciding the semantics of > ORDER BY/DISTINCT. �That could lead to bizarre behavior or even > crashes, eg if we try to use numeric sort operators on a value that > actually has been coerced to float8. �We could possibly go back and > re-do the decisions about data types but it'd be a mess. > > 2. Split the processing of aggregates with ORDER BY/DISTINCT so that the > sorting/uniqueifying is done in a separate expression node that can work > with the "native" types of the given columns, and only after that do we > perform coercion to the aggregate function's input types. �This would be > logically the cleanest thing, perhaps, but it'd represent a very major > rework of the patch, with really no hope of getting it done for 9.0. > > 3. Do something so that we can still match "t::text" to "t". �This > seems pretty awful on first glance but it's not actually that bad, > because in the case we care about the cast will be marked as having > been applied implicitly. �Basically, instead of just equal() comparisons > in findTargetlistEntrySQL99(), we'd strip off any implicit cast at the > top of either expression, and only then do equal(). �Since the implicit > casts are, by definition, things the user didn't write, this would still > have the expected behavior of matching expressions that were identical > when the user wrote them. > > #3 seems the sanest fix, but I wonder if anyone has an objection or > better idea. I didn't look at the code yet, #2 sounds like the way to go. But I see the breakage is unacceptable for 9.0, so #3 is the choice for 9.0 and will we fix it as #2 for 9.1 or later? Regards, -- Hitoshi Harada -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Daniel Grace on 20 Jul 2010 14:00 On Mon, Jul 19, 2010 at 4:08 PM, Hitoshi Harada <umi.tanuki(a)gmail.com> wrote: > > 2010/7/19 Tom Lane <tgl(a)sss.pgh.pa.us>: > > I looked into the problem reported here: > > http://archives.postgresql.org/pgsql-bugs/2010-07/msg00119.php > > [...] > > > > > 2. Split the processing of aggregates with ORDER BY/DISTINCT so that the > > sorting/uniqueifying is done in a separate expression node that can work > > with the "native" types of the given columns, and only after that do we > > perform coercion to the aggregate function's input types. �This would be > > logically the cleanest thing, perhaps, but it'd represent a very major > > rework of the patch, with really no hope of getting it done for 9.0. [...] > > > #3 seems the sanest fix, but I wonder if anyone has an objection or > > better idea. > > I didn't look at the code yet, #2 sounds like the way to go. But I see > the breakage is unacceptable for 9.0, so #3 is the choice for 9.0 and > will we fix it as #2 for 9.1 or later? I'm the original reporter of the mentioned bug. One possible concern might be typecasts that aren't a 1:1 representation. While no two VARCHARs are going to produce the same TEXT, this is not true in other cases (1.1::float::integer and 1.2::float::integer both produce 1, for instance). Off the top of my head, I can't think of a good example where this would cause a problem -- it'd be easy enough to manufacture a possible test case, but it'd be so contrived and I don't know if it's something that would be seen in production code. But if we SELECT SOME_INTEGER_AGGREGATE(DISTINCT floatcol ORDER BY floatcol), should the DISTINCT operate on floatcol (i.e. 1.1 and 1.2 are distinct, even if it means the function is called with '1' twice) or floatcol::integer (1.1 and 1.2 are not distinct)? I'm guessing the former, even if it means the function is called multiple times with the same final (after typecasting) input value. The latter would only be correct if the user specifically wrote it as DISTINCT floatval::INTEGER. -- Daniel Grace -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tom Lane on 27 Jul 2010 19:16 Daniel Grace <dgrace(a)wingsnw.com> writes: > One possible concern might be typecasts that aren't a 1:1 > representation. While no two VARCHARs are going to produce the same > TEXT, this is not true in other cases (1.1::float::integer and > 1.2::float::integer both produce 1, for instance). > Off the top of my head, I can't think of a good example where this > would cause a problem -- it'd be easy enough to manufacture a possible > test case, but it'd be so contrived and I don't know if it's something > that would be seen in production code. But if we SELECT > SOME_INTEGER_AGGREGATE(DISTINCT floatcol ORDER BY floatcol), should > the DISTINCT operate on floatcol (i.e. 1.1 and 1.2 are distinct, even > if it means the function is called with '1' twice) or > floatcol::integer (1.1 and 1.2 are not distinct)? Yes. The current implementation has the advantage that any unique-ifying step is guaranteed to produce outputs that are distinct from the point of view of the aggregate function, whereas if we try to keep the two operations at arms-length, then either we lose that property or we sort-and-unique twice :-(. If memory serves, this type of consideration is also why DISTINCT and GROUP BY are made to follow ORDER BY's choice of semantics in an ordinary SELECT query --- you might find that surprising, but if they weren't on the same page it could be even more surprising. So on reflection I think that the current fix is the best one and we don't want to reconsider it later. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Robert Haas on 27 Jul 2010 19:38 On Tue, Jul 27, 2010 at 7:16 PM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote: > Daniel Grace <dgrace(a)wingsnw.com> writes: >> �But if we SELECT >> SOME_INTEGER_AGGREGATE(DISTINCT floatcol ORDER BY floatcol), should >> the DISTINCT operate on floatcol (i.e. 1.1 and 1.2 are distinct, even >> if it means the function is called with '1' twice) or >> floatcol::integer (1.1 and 1.2 are not distinct)? > > Yes. �The current implementation has the advantage that any > unique-ifying step is guaranteed to produce outputs that are distinct > from the point of view of the aggregate function, whereas if we try to > keep the two operations at arms-length, then either we lose that > property or we sort-and-unique twice :-(. Am I misreading this, or did you just answer an "either-or" question with "yes"? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tom Lane on 27 Jul 2010 19:47
Robert Haas <robertmhaas(a)gmail.com> writes: > Am I misreading this, or did you just answer an "either-or" question with "yes"? I meant "Yes, that's an issue." regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |