From: Øystein Grøvlen
Date: January 20 2011 10:41am
Subject: Re: bzr commit into mysql-trunk branch (roy.lyseng:3287) Bug#57525
List-Archive: http://lists.mysql.com/commits/129266
Message-Id: <4D381158.6040301@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: quoted-printable

Hi Roy,

Thanks for sorting this out.  Approved with the following suggestions:

On 11/18/2010 10:47 AM, Roy Lyseng wrote:
=2E..

 >        sql/item.cc
 >          A common implementation of fix_after_pullout() for all Item_r=
ef
 >          classes has been made. The exception is Item_aggregate_ref wh=
ich
 >          must have its own implementation and Item_outer_ref which we =

still
 >          have no test case for.


It would be good it the code could contain a comment as to why
Item_aggregate_ref needs its owmn implementation.  I.e., why
Item_ident::fix_after_pullout should not be called.

=2E..

 >   void Item_ref::cleanup()
 >   {
 >     DBUG_ENTER("Item_ref::cleanup");
 > +  /*
 > +    Save depended_from so that it can be restored after=20
Item_ident::cleanup().
 > +    Item_ref::depended_from is not recalculated in later=20
fix_fields() calls,
 > +    so this hack is needed to have correct dependency information in =

prepared
 > +    statement execution. Dependencies should not change unless an=20
involved
 > +    view is updated, and this currently has much more severe=20
implications.
 > +  */
 > +  st_select_lex *save_depended_from=3D depended_from;
 >     Item_ident::cleanup();
 > +  depended_from=3D save_depended_from;
 >     result_field=3D 0;
 >     DBUG_VOID_RETURN;
 >   }

Instead, why not just remove the setting of depended_from from
Item_iden::cleanup?  I tried, and got no test failures for main suite,
not even with --ps-protocol.

--
=D8ystein