I wrote:
> I think the issue basically arises from this concern in exec_move_row:

>      * NOTE: this code used to demand row->nfields ==
>      * HeapTupleHeaderGetNatts(tup->t_data), but that's wrong.  The tuple
>      * might have more fields than we expected if it's from an
>      * inheritance-child table of the current table, or it might have fewer if
>      * the table has had columns added by ALTER TABLE.

BTW, this comment is very old, dating to commit 8bb3c8fe5, which was
in response to

Looking at it again now, I wonder whether I didn't make the wrong choice
about how to fix the problem.  The above concerns seem valid so far as the
physical number of fields in the tuple go, but they do not apply to the
tupdesc that comes with it.  So arguably the real bug was using the
physical tuple rather than the tupdesc to drive the conversion, which is
something we got away from later anyway when we added dropped-column
support.  Perhaps it'd be reasonable to throw an error if the number of
non-dropped columns in the source and destination tupdescs don't match,
even for the composite-type case.

The specific issue mentioned in the above bug report is gone anyway: if I
try that test case now, I don't see any tuples with extra columns arriving
at exec_move_row, because we insert a ConvertRowtypeExpr node to convert
the rows of the child table to the parent rowtype.  But I'm not quite
prepared to assume that such cases can't arise through other examples.

Now, I do not think we can remove the facility for doing data type
conversion on the fields; undoubtedly there are people relying on being
able to SELECT INTO a numeric variable from an integer source column, for
example.  So maybe being lax about the number of columns is a sensible
thing to go along with that.  But it sure seems like a foot-gun.

                        regards, tom lane

Reply via email to