Over-optimization in ExecEvalWholeRowVar - Mailing list pgsql-hackers

From Tom Lane
Subject Over-optimization in ExecEvalWholeRowVar
Date
Msg-id 18336.1405107800@sss.pgh.pa.us
Whole thread Raw
Responses Re: Over-optimization in ExecEvalWholeRowVar  (Merlin Moncure <mmoncure@gmail.com>)
List pgsql-hackers
This example in the regression database is a simplified version of a bug
I was shown off-list:

regression=# select (
select q from
( select 1,2,3 where f1>0 union all select 4,5,6.0 where f1<=0
) q
)
from int4_tbl;
ERROR:  record type has not been registered

The reason for the problem is that ExecEvalWholeRowVar is designed
to bless the Var's record type (via assign_record_type_typmod) just
once per query.  That works fine, as long as the source tuple slot
is the exact same TupleTableSlot throughout the query.  But in the
above example, we have an Append plan node for the UNION ALL, which
will pass back its children's result slots directly --- so at the
level where "q" is being evaluated, we see first the first leaf SELECT's
output slot (which gets blessed) and then the second leaf SELECT's
output slot (which doesn't).  That leads to generating a composite
Datum with typmod -1 ... oops.

I can reproduce this bug in all active branches.  Probably the reason
it hasn't been identified long since is that in most scenarios the
planner won't use a whole-row Var at all in cases like this, but will
prefer to build RowExprs.  (In particular, the inconsistent data types
of the last sub-select output columns are necessary to trigger the bug
in this example.)

The easy fix is to move the blessing code from ExecEvalWholeRowVar
into ExecEvalWholeRowFast.  (ExecEvalWholeRowSlow doesn't need it
since it doesn't deal with RECORD cases.)  That'll add a usually-useless
comparison or two per row cycle, which is unlikely to be measurable.

A bigger-picture problem is that we have other once-per-query tests in
ExecEvalWholeRowVar, and ExecEvalScalarVar too, whose validity should be
questioned given this bug.  I think they are all right, though, because
those tests are concerned with validating the source rowtype, which should
not change across Append children.  The bug here is because we're assuming
not just that the input rowtype is abstractly the same across all rows,
but that the exact same TupleTableSlot is used to return every source row.
        regards, tom lane



pgsql-hackers by date:

Previous
From: Claudio Freire
Date:
Subject: Re: Minmax indexes
Next
From: Martijn van Oosterhout
Date:
Subject: Re: better atomics - v0.5