Re: Fortify float4 and float8 regression tests by ordering test results - Mailing list pgsql-hackers

On Tue, Apr 22, 2025 at 5:57 PM Pavel Borisov <pashkin.elfe@gmail.com> wrote:
> On Tue, 22 Apr 2025 at 18:40, Pavel Borisov <pashkin.elfe@gmail.com> wrote:
> > On Tue, 22 Apr 2025 at 18:23, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > >
> > > Pavel Borisov <pashkin.elfe@gmail.com> writes:
> > > > It's not a big problem, but propose a simple fix for the tests. It
> > > > just adds ORDER BY 1 to all relevant float4 and floa8 queries.
> > >
> > > You do realize that this problem is hardly confined to the
> > > float4 and float8 tests?  To a first approximation, a table AM
> > > that fails to preserve insertion order would break every single
> > > one of our regression tests.  (I speak from experience, as
> > > Salesforce had to deal with this when I was there...)
> > >
> > > The reason why we don't simply add ORDER BY to everything is
> > > explained at [1]:
> > >
> > >     You might wonder why we don't order all the regression test
> > >     queries explicitly to get rid of this issue once and for all.  The
> > >     reason is that that would make the regression tests less useful,
> > >     not more, since they'd tend to exercise query plan types that
> > >     produce ordered results to the exclusion of those that don't.
> > >
> > > At some point we will probably have to think through what we
> > > want to do about running the regression tests with table AMs that
> > > don't wish to preserve ordering as much as heapam does.  It's going
> > > to need careful balancing of multiple concerns, and I don't think
> > > "blindly slap ORDER BY everywhere" is going to be an acceptable
> > > answer.
> > >
> >
> > I agree we might need to elaborate different AM support in regression tests.
> > Also, I agree that these are not the only tests to be fixed.
> >
> > What I hardly agree with, is that we suppose it's right for regression
> > tests to require some fixed results ordering for so simple cases.
> > Maybe for more complicated plans it's useful, but for the float tests
> > mentioned in the patch it's this requirement is a total loss IMO.
> >
> > I understand your sentiment against blindly slapping order by's, but I
> > don't see a useful alternative for this time. Also a large number of
> > tests in PG were already fortified with ORDER BY 1.
>
> I forgot to mention that it's not only a question of INSERTs ordering.
> Extension AM can have some internal ordering e.g. index-organized
> tables. And besides SELECT query results following internal AM
> ordering just being allowed, more importantly they are good
> performance-wise and justify table AM extensibility.

+1,
I'd like to add that float4.out not only assumes that insert-ordering is preserved (this could be more-or-less portable between table AMs).  It also assumes the way UPDATE moves updated rows.  That seems quite heap-specific.  You can see in the following fragment, updated rows jump to the bottom.

-- test the unary float4abs operator
SELECT f.f1, @f.f1 AS abs_f1 FROM FLOAT4_TBL f;
      f1       |    abs_f1    
---------------+---------------
             0 |             0
        1004.3 |        1004.3
        -34.84 |         34.84
 1.2345679e+20 | 1.2345679e+20
 1.2345679e-20 | 1.2345679e-20
(5 rows)

UPDATE FLOAT4_TBL
   SET f1 = FLOAT4_TBL.f1 * '-1'
   WHERE FLOAT4_TBL.f1 > '0.0';
SELECT * FROM FLOAT4_TBL;
       f1      
----------------
              0
         -34.84
        -1004.3
 -1.2345679e+20
 -1.2345679e-20
(5 rows)

------
Regards,
Alexander Korotkov
Supabase