At Wed, 05 Apr 2017 15:51:10 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in <27982.1491421870@sss.pgh.pa.us>
> Hmm, this still isn't right --- testing shows that you had the comparison
> rule right the first time.
Perhaps Laplaces's deamon is continuously nudging on my head
toward wrong conclusion, sigh. Sorry for bothering you.
At Wed, 05 Apr 2017 17:06:53 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in <385.1491426413@sss.pgh.pa.us>
> I wrote:
> > Looking at what we've got here, it's already a substantial fraction of
> > what would be needed to provide a compiler-independent implementation
> > of the int128-based aggregate logic in numeric.c. With that in mind,
> > I propose that we extract the relevant stuff into a new header file
> > that is designed as general-purpose int128 support.
+1
> > Something like the
> > attached. I also attach the test program I put together to verify it.
The new patch seems cleaner and fine to me with maybe-fresh eyes.
Since we have some instances of failure cases on non-native
int128 arithmetic, I'd like to provide regression test for them
but that seems not so simple.
By the way the adt directory is, as suggested by the name,
storing files with names of SQL data types so "int128.c" among
then seems incongruous. Is "int128_test.c" acceptable? int16.c
will be placed there in case we support int16 or hugeint on SQL.
> Here's a fleshed-out patch for the original problem based on that.
> I found that direct int64-to-int128 coercions were also needed to
> handle some of the steps in timestamp.c, so I added those to int128.h.
>
> I think it would be reasonable to back-patch this, although it would
> need some adjustments for the back branches since we only recently
> got rid of the float-timestamp option. Also I'd not be inclined to
> depend on native int128 any further back than it was already in use.
Back to 9.5 seems reasonable to me.
--
Kyotaro Horiguchi
NTT Open Source Software Center
