On 15.03.21 18:35, Tom Lane wrote:
Anyway, taking a quick look at the v4 patch, the only complaint
I have is that it seems a bit bulky and brute-force to duplicate
so much code. Is it feasible to share most of the implementation
between old and new functions, returning (say) an int64 that can
then be converted to either numeric or float8 by a wrapper? That
would also reduce the pressure to duplicate all the test cases.
Yeah, it's not straightforward to do this, because you'd also need to
carry around scale and infinity information, so you might end up
creating a mini-numeric implementation just for this.
An easy way to reduce duplication would be to convert the existing
date_part() into a wrapper around the new extract(), with a cast. But
then you'd pay the performance penalty of the numeric version.
Which leads me to: After retesting this now, with a new machine, the
performance of the numeric implementation is brutal compared to the
float implementation, for cases where we need numeric division, which is
milliseconds, seconds, and epoch. In the first two cases, I imagine we
could rewrite this a bit to avoid a lot of the numeric work, but for the
epoch case (which is what started this thread), there isn't enough space
in int64 to make this work. Perhaps int128 could be pressed into
service, optionally. I think it would also help if we cracked open the
numeric APIs a bit to avoid all the repeated unpacking and packing for
each step.
So I think we need to do a bit more thinking and work here, meaning it
will have to be postponed.
Here are the kinds of tests I ran:
=> select date_part('epoch', localtime + generate_series(0, 10000000) *
interval '1 second') \g /dev/null
Time: 2537.482 ms (00:02.537)
=> select extract(epoch from localtime + generate_series(0, 10000000) *
interval '1 second') \g /dev/null
Time: 6106.586 ms (00:06.107)
