Re: Ooops ... seems we need a re-release pronto - Mailing list pgsql-hackers

Peter Eisentraut <peter_e@gmx.net> writes:
> Shouldn't we at least add the one or two exemplary statements that 
> failed so we have some sort of coverage of the problem?

We could, but I'm unexcited about it.  The known failures are an
extremely narrow case: we're trying to evaluate expressions (either
CHECK constraints or index expressions) over a tuple proposed to be
inserted into a relation.  But the TupleDesc that's been supplied to
the evaluator is not the tuple descriptor of the target relation,
it's a descriptor generated on-the-fly from the target list of the
plan tree (by ExecTypeFromTL).  And the target list includes some
constants, and our nodetree representation for constants fails to
preserve typmod knowledge, and so ExecTypeFromTL produces atttypmod
-1 for this column, and the security check didn't like that because
the Var it was checking had a nondefault typmod.

When the first reports came in, I thought seriously about fixing it
by forcing the target relation's real tupdesc (from its relcache entry)
to be used in this context instead of a generated tupdesc.  I concluded
that it was too likely that there were other cases where we were
evaluating expressions against generated tuples, and we had to back off
the strength of the security check instead.  I do not actually have any
specific examples, but I think it's fairly pointless to add a regression
test that covers this one narrow scenario when there are probably lots
of others.

I'm not especially a fan of the testing philosophy that says you
memorialize each individual past mistake as a permanent regression test
--- I think that just bloats the tests, and test bloat is a bad thing
because it discourages people from running the tests.  (MySQL's
regression tests currently require about an hour on a fast machine.
Somehow this has not helped them to achieve a low bug rate...)  I do
agree with adding a test when you think it is likely to be able to catch
a whole class of errors, or even a specific error if it seems especially
likely to recur, but right now I'm not seeing how we do that here.

BTW, I think a good case could be made that the core of the problem
is exactly that struct Const doesn't carry typmod, and thus that we
lose information about constructs like 'foo'::char(7).  We should fix
that, and also anywhere else in the expression tree structure where
we are discarding knowledge about the typmod of a result.  This has
got some urgency because of Teodor's recent work on allowing user
defined types to have typmods --- we can expect massive growth in the
number of scenarios where it matters.
        regards, tom lane