Обсуждение: Re: [HACKERS] Re: varchar() troubles (fwd)

Forwarded message:
> > The problem was that some things were copied using VARSIZE rather than
> > subtracting out VARHDRSZ first (actually, I think it might have use
> > sizeof(int) and other dangers too). I patched that near the end of the year
> > and my 980101.d tree and 980106.d tree do not exhibit the symptom:
> >
> > postgres=> create table t (v varchar(80),i int);
> > CREATE
> > postgres=> insert into t values ('hi', 1);
> > INSERT 643562 1
> > postgres=> select * from t;
> > v |i
> > --+-
> > hi|1
> > (1 row)
>
> I have found that ExecEvalVar() uses a descriptor that has the attr
> length set to the maximum, instead of -1.  The ExecTypeFromTL() comment
> says:
>
> /* ----------------------------------------------------------------
>  *      ExecTypeFromTL
>  *
>  *      Currently there are about 4 different places where we create
>  *      TupleDescriptors.  They should all be merged, or perhaps
>  *      be rewritten to call BuildDesc().
>  *
>
> Clearly stating that the tuple descriptors in the system are created in
> several places.  Some places have the length set wrong.  I am going to
> have to take a look at all those places, and make sure they have
> consistent behaviour.

Vadim, can you look at this for me.  If you set a break at ExecEvalVar
before executing the SELECT, you will see its
tupledescriptor->attrs[0].attlen is the max length, and not -1 as it
should be.

I can't figure out where that is getting set.  Can you also check the
other tupledescriptor initializations to see they have the -1 for
varchar too.  I am stumped.

--
Bruce Momjian
maillist@candle.pha.pa.us

Bruce Momjian wrote:
>
> >
> > I have found that ExecEvalVar() uses a descriptor that has the attr
> > length set to the maximum, instead of -1.  The ExecTypeFromTL() comment
...
>
> Vadim, can you look at this for me.  If you set a break at ExecEvalVar
> before executing the SELECT, you will see its
> tupledescriptor->attrs[0].attlen is the max length, and not -1 as it
> should be.
>
> I can't figure out where that is getting set.  Can you also check the
> other tupledescriptor initializations to see they have the -1 for
> varchar too.  I am stumped.

Why attlen should be -1 ?
attlen in pg_attribute for v in table t is 84, why run-time attlen
should be -1 ? How else maxlen constraint could be checked ?
IMHO, you have to change heap_getattr() to check is atttype == VARCHAROID
and use vl_len if yes. Also, other places where attlen is used must be
changed too - e.g. ExecEvalVar():

    {
        len = tuple_type->attrs[attnum - 1]->attlen;
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        byval = tuple_type->attrs[attnum - 1]->attbyval ? true : false;
    }

    execConstByVal = byval;
    execConstLen = len;
    ^^^^^^^^^^^^^^^^^^ - used in nodeHash.c

Vadim

>
> Bruce Momjian wrote:
> >
> > >
> > > I have found that ExecEvalVar() uses a descriptor that has the attr
> > > length set to the maximum, instead of -1.  The ExecTypeFromTL() comment
> ...
> >
> > Vadim, can you look at this for me.  If you set a break at ExecEvalVar
> > before executing the SELECT, you will see its
> > tupledescriptor->attrs[0].attlen is the max length, and not -1 as it
> > should be.
> >
> > I can't figure out where that is getting set.  Can you also check the
> > other tupledescriptor initializations to see they have the -1 for
> > varchar too.  I am stumped.
>
> Why attlen should be -1 ?
> attlen in pg_attribute for v in table t is 84, why run-time attlen
> should be -1 ? How else maxlen constraint could be checked ?
> IMHO, you have to change heap_getattr() to check is atttype == VARCHAROID
> and use vl_len if yes. Also, other places where attlen is used must be
> changed too - e.g. ExecEvalVar():
>
>     {
>         len = tuple_type->attrs[attnum - 1]->attlen;
>         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>         byval = tuple_type->attrs[attnum - 1]->attbyval ? true : false;
>     }
>
>     execConstByVal = byval;
>     execConstLen = len;
>     ^^^^^^^^^^^^^^^^^^ - used in nodeHash.c
>

The major problem is that TupleDesc comes from several places, and
attlen means several things.

There are some cases where TupleDesc (int numatt, Attrs[]) is created
on-the-fly (tupdesc.c), and the attlen is the length of the type.  In
other cases, we get attlen from opening the relation, heap_open(), and
in these cases it is the length as defined for the particular attribute.

Certainly a bad situation.  I am not sure about a fix.


--
Bruce Momjian
maillist@candle.pha.pa.us