Re: preliminary: logical column order

Tom Lane <tgl@sss.pgh.pa.us> writes:
> Bruce and I were chatting about that on the phone today.  I think it
> might be useful for TupleDescs to doubly index their contained
> attribute rows

Ok, I implemented this. I made it so that the properly sorted
attribute array is constructed lazily -- my reasoning was that (a)
relatively few locations in the code actually use the sorted attribute
array, so there is no point allocating and initializing it every time
a TupleDesc is used (b) the array could potentially be 1,500 pointers
long -- not huge, but not tiny either, so it is worth a little effort
to avoid unnecessarily alloc'ing and sorting it. Access to the sorted
array is (only) done via a new function, GetAttrByLogicalPosition()

> Easy to fix given above proposal ... although actually I am not sure
> why this would occur.  printtup and friends should always get a
> constructed TupDesc that has no notion of deleted or renumbered
> columns.

It seems to happen because:

   - SendRowDescription() in printtup.c notes that "the TupleDesc has
     been manufactured by ExecTypeFromTL() or some similar function;
     it does not contain a full set of fields."

   - ExecTypeFromTL() allocates an empty TupleDesc, and then fills in
     its individual entries with data from a TargetList -- the
     Form_pg_attribute for the attributes involved is never consulted,
     so the default attlogpos inserted by TupleDescInitEntry() is
     used: attlogpos == attnum

I'm unsure of the best way to fix this so that the TupleDesc that is
handed to printtup & friends contains the information we require. Any
suggestions?

A new version of the patch is attached. Changes:

   - replace sorting code with a lazily-constructed sorted array of
     pointers to attribute data in TupleDesc
   - include Manfred's suggested fix for the alter table regression
     test
   - (unrelated) add a comment to rewrite/rewriteDefine.c noting the
     intent of the code and known problems
   - (unrelated) merge my other patch for refactoring
     CreateTupleDescCopy() into this patch: I needed to modify this
     function in this patch, so I didn't want to deal with patching
     complexities.

-Neil