Обсуждение: Re: [COMMITTERS] pgsql: Add ANALYZE into regression tests

Поиск
Список
Период
Сортировка

Re: [COMMITTERS] pgsql: Add ANALYZE into regression tests

От
Tom Lane
Дата:
Stephen Frost <sfrost@snowman.net> writes:
> * Greg Stark (stark@mit.edu) wrote:
>> But the original goal seems like it would be easier and better done with an
>> immutable function which lies and calls elog to leak information. That's
>> the actual attack this is supposed to protect against anyways.

> Sure, but there's a whole slew of tests that would have to change if we
> changed the explain output, not just this one.

Sure, but I think Greg's point is that this could be tested by a
black-box functional test ("does it print something it shouldn't")
rather than a white-box test that necessarily depends on a whole lot
of *other* planner choices that don't have much to do with the point
in question.  You already got bit by variances in the choice of join
type, which is not what the test is about.

I think the test is okay as-is as long as we don't see more failures
from it; but if we do see any more I'd suggest rewriting as per Greg's
suggestion rather than trying to constrain the plan choice even more
narrowly.

            regards, tom lane


Re: [COMMITTERS] pgsql: Add ANALYZE into regression tests

От
Stephen Frost
Дата:
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Sure, but I think Greg's point is that this could be tested by a
> black-box functional test ("does it print something it shouldn't")
> rather than a white-box test that necessarily depends on a whole lot
> of *other* planner choices that don't have much to do with the point
> in question.  You already got bit by variances in the choice of join
> type, which is not what the test is about.

Yes, but as I said, I'm *also* doing the black-box functional test..
Perhaps that means this extra EXPLAIN test is overkill, but, after
trying to run down whatever build animal 'hamerkop' has done to break
the tablespace regression test, I'm definitely keen to err on the side
of 'more information'.

> I think the test is okay as-is as long as we don't see more failures
> from it; but if we do see any more I'd suggest rewriting as per Greg's
> suggestion rather than trying to constrain the plan choice even more
> narrowly.

The 'rewriting', in this case, would simply be removing the 'explain'
part of the test and keeping the rest.  If you or Greg see an issue with
the test that actually does the 'raise notice' for every value seen by
the 'snoop' function or it doesn't match your 'black-box' expectation,
please let me know and I can try to refine it..

    Thanks,

        Stephen

Вложения