Thread

  1. IntArray in c.h

    Hitoshi Harada <umi.tanuki@gmail.com> — 2009-12-29T09:50:30Z

    I found the struct IntArray defined in c.h is actually used only in
    execQual.c. ISTM the definition should be at least moved to the right
    place.
    
    Attached is a trivial fix. Addition to the explanation above,  I
    replaced IntArray by simple int array bounded with MAXDIM and remove
    local variable lIndex in ExecEvalArrayRef  because the usage of the
    variable doesn't seem good to me.
    
    Regression passed and various manual tests like "UPDATE t SET
    a[1:2][1] = 1" didn't fail.
    
    
    Regards,
    
    -- 
    Hitoshi Harada
    
  2. Re: IntArray in c.h

    Tom Lane <tgl@sss.pgh.pa.us> — 2009-12-29T15:38:44Z

    Hitoshi Harada <umi.tanuki@gmail.com> writes:
    > I found the struct IntArray defined in c.h is actually used only in
    > execQual.c. ISTM the definition should be at least moved to the right
    > place.
    
    It's a general-purpose datatype that might be used anywhere that array
    indexing happens.  I think the fact that it's currently used only in
    execQual is mere happenstance, and should not be "enforced" by moving
    or removing the declaration.
    
    			regards, tom lane
    
    
  3. Re: IntArray in c.h

    Hitoshi Harada <umi.tanuki@gmail.com> — 2009-12-30T02:46:40Z

    2009/12/30 Tom Lane <tgl@sss.pgh.pa.us>:
    > Hitoshi Harada <umi.tanuki@gmail.com> writes:
    >> I found the struct IntArray defined in c.h is actually used only in
    >> execQual.c. ISTM the definition should be at least moved to the right
    >> place.
    >
    > It's a general-purpose datatype that might be used anywhere that array
    > indexing happens.  I think the fact that it's currently used only in
    > execQual is mere happenstance, and should not be "enforced" by moving
    > or removing the declaration.
    
    I would be convinced if the struct or the logic was complex, but
    actually it is so simple that it can be replaced by primitive int
    array. Also, it seems to me that c.h is too general place to declare
    it for such purpose. Does nobody else think so?
    
    Regards,
    
    -- 
    Hitoshi Harada
    
    
  4. Re: IntArray in c.h

    Peter Eisentraut <peter_e@gmx.net> — 2009-12-31T13:07:10Z

    On ons, 2009-12-30 at 11:46 +0900, Hitoshi Harada wrote:
    > 2009/12/30 Tom Lane <tgl@sss.pgh.pa.us>:
    > > Hitoshi Harada <umi.tanuki@gmail.com> writes:
    > >> I found the struct IntArray defined in c.h is actually used only in
    > >> execQual.c. ISTM the definition should be at least moved to the right
    > >> place.
    > >
    > > It's a general-purpose datatype that might be used anywhere that array
    > > indexing happens.  I think the fact that it's currently used only in
    > > execQual is mere happenstance, and should not be "enforced" by moving
    > > or removing the declaration.
    > 
    > I would be convinced if the struct or the logic was complex, but
    > actually it is so simple that it can be replaced by primitive int
    > array. Also, it seems to me that c.h is too general place to declare
    > it for such purpose. Does nobody else think so?
    
    The definition of c.h is bogus anyway.  You might think it contains
    includes and defines to set up a portable C environment, which is what
    the first half indeed does.
    
    But then things like regproc, transaction ID types, IntArray, varlena,
    bytea, oidvector, NameData, etc. do not belong there and should be moved
    to postgres.h.
    
    
    
  5. Re: IntArray in c.h

    Tom Lane <tgl@sss.pgh.pa.us> — 2009-12-31T16:28:02Z

    Peter Eisentraut <peter_e@gmx.net> writes:
    > The definition of c.h is bogus anyway.  You might think it contains
    > includes and defines to set up a portable C environment, which is what
    > the first half indeed does.
    
    > But then things like regproc, transaction ID types, IntArray, varlena,
    > bytea, oidvector, NameData, etc. do not belong there and should be moved
    > to postgres.h.
    
    Actually, what c.h does is to provide definitions that are needed in
    both frontend and backend code.  And we do NOT want to start including
    postgres.h in frontend code.  It might be that some of the declarations
    there are useless to frontend code and could be moved, but trying to be
    as strict as you suggest is only going to create problems.
    
    			regards, tom lane
    
    
  6. Re: IntArray in c.h

    Peter Eisentraut <peter_e@gmx.net> — 2010-01-01T19:54:16Z

    On tor, 2009-12-31 at 11:28 -0500, Tom Lane wrote:
    > Peter Eisentraut <peter_e@gmx.net> writes:
    > > The definition of c.h is bogus anyway.  You might think it contains
    > > includes and defines to set up a portable C environment, which is what
    > > the first half indeed does.
    > 
    > > But then things like regproc, transaction ID types, IntArray, varlena,
    > > bytea, oidvector, NameData, etc. do not belong there and should be moved
    > > to postgres.h.
    > 
    > Actually, what c.h does is to provide definitions that are needed in
    > both frontend and backend code.  And we do NOT want to start including
    > postgres.h in frontend code.  It might be that some of the declarations
    > there are useless to frontend code and could be moved, but trying to be
    > as strict as you suggest is only going to create problems.
    
    I think the list above is a pretty good list of things that client code
    doesn't need, plus or minus a few things maybe.
    
    
    
    
  7. Re: IntArray in c.h

    Hitoshi Harada <umi.tanuki@gmail.com> — 2010-01-03T03:40:54Z

    2010/1/2 Peter Eisentraut <peter_e@gmx.net>:
    > On tor, 2009-12-31 at 11:28 -0500, Tom Lane wrote:
    >> Peter Eisentraut <peter_e@gmx.net> writes:
    >> > The definition of c.h is bogus anyway.  You might think it contains
    >> > includes and defines to set up a portable C environment, which is what
    >> > the first half indeed does.
    >>
    >> > But then things like regproc, transaction ID types, IntArray, varlena,
    >> > bytea, oidvector, NameData, etc. do not belong there and should be moved
    >> > to postgres.h.
    >>
    >> Actually, what c.h does is to provide definitions that are needed in
    >> both frontend and backend code.  And we do NOT want to start including
    >> postgres.h in frontend code.  It might be that some of the declarations
    >> there are useless to frontend code and could be moved, but trying to be
    >> as strict as you suggest is only going to create problems.
    >
    > I think the list above is a pretty good list of things that client code
    > doesn't need, plus or minus a few things maybe.
    >
    Looking closer in c.h, there are several things to move or remove (and
    it gets slightly more efficient if we do), but it seems we don't have
    such motivation...
    
    Regards,
    
    
    -- 
    Hitoshi Harada