Re: [HACKERS] Function transform optimizations versus reality
On Wed, Jan 18, 2017 at 12:10 PM, Tom Lanewrote: > More generally, this is the second serious bug we've found in the last > month in the "transform" optimizations (see also bug #14479 and commit > f0774abde). I'm starting to get the feeling that that idea was an > attractive nuisance --- at the very least, it would behoove us to go > through all the transform functions with a gimlet eye. So, two things for perspective: 1. Transform functions have been around for just over 5 years now; if the two bugs we have had in the last month are the only ones, that's a better track record than most features. If we've had one a month for 5 years, that's terrible, of course, but I don't think that's so. 2. The value of these transform functions is principally in that table rewrites can be avoided when executing ALTER TABLE. And that is a big value. People are still looking for ways to further reduce table rewrites, as evidenced for example by Serge Rielau's patch for implicit defaults. Of course, it is entirely possible that Noah and I missed some important things when we added that feature, and an audit of the code is very welcome. But I would discourage you from disabling any more of the optimization than necessary, because I think people really care about whether ALTER TABLE has to rewrite the table or just lock it long enough to change the metadata. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Function transform optimizations versus reality
I looked into bug #14504, https://www.postgresql.org/message-id/20170118144828.1432.52...@wrigleys.postgresql.org The problem is that timestamp_zone_transform() has the cute idea that it can simplify timezone('UTC', timestamptzvalue) into a RelabelType that claims the timestamptzvalue is just type timestamp. Well, that's just too cute, because it confuses the index machinery completely. What _bt_first() sees is an equality comparison between an index column that it knows is timestamptz, and a RHS constant that it knows is timestamp, so it selects timestamptz_cmp_timestamp() as the comparison function to use. And that function will apply a timezone rotation, resulting in the wrong answers. You could blame this on the fact that the planner will ignore the RelabelType when trying to match the qual to indexes. But that hack has fifteen years' seniority on this one, and removing it would break (at least) use of indexes on varchar columns, so I'm not planning on fixing this that way. I think the only realistic fix is to disable this optimization in timestamp_zone_transform; certainly that's the only way I'd be comfortable with back-patching. (Another reason for being unhappy about this optimization is that EXPLAIN will print the resulting expression tree as timestamptzvalue::timestamp, which is a completely misleading description; an actual cast like that would not behave this way.) More generally, this is the second serious bug we've found in the last month in the "transform" optimizations (see also bug #14479 and commit f0774abde). I'm starting to get the feeling that that idea was an attractive nuisance --- at the very least, it would behoove us to go through all the transform functions with a gimlet eye. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers