List:Commits« Previous MessageNext Message »
From:Guilhem Bichot Date:May 20 2011 1:17pm
Subject:Re: bzr commit into mysql-trunk branch (tor.didriksen:3309) WL#4800
View as plain text  
Hello,

Jorgen Loland a écrit, Le 19.05.2011 13:10:
> Guilhem,
> 
> You have my blessing on the security patch. The security checks look 
> good, but I'm trusting Dmitri's judgment on the correctness. A few notes:
> 
> === modified file 'sql/sql_prepare.cc'
> --- sql/sql_prepare.cc    2011-03-21 17:55:41 +0000
> +++ sql/sql_prepare.cc    2011-05-05 11:28:31 +0000
> @@ -2021,8 +2018,7 @@ static bool check_prepared_statement(Pre
>      if (res == 2)
>      {
>        /* Statement and field info has already been sent */
> -      res= FALSE;
> -      goto end;
> +      DBUG_RETURN(FALSE);
> 
> jl: false

In fact, I had changed this code in the past in this WL, but then 
noticed I didn't need the change, so reverted back to how it is in 
trunk. Which is why there is this old-looking FALSE.
This way, the diff between wl4800 and trunk is only:

[T35 14:28 /m/bzrrepos_new/mysql-next-mr-opt-backporting-wl4800 $]
bzr diff -r3182.15.136 sql/sql_prepare.cc
=== modified file 'sql/sql_prepare.cc'
--- sql/sql_prepare.cc	2011-04-15 12:14:35 +0000
+++ sql/sql_prepare.cc	2011-05-06 12:05:20 +0000
@@ -113,6 +113,7 @@
  #include <mysql_com.h>
  #endif
  #include "lock.h"                               // 
MYSQL_OPEN_FORCE_SHARED_MDL
+#include "opt_trace.h"                          // Opt_trace_object

  /**
    A result class used to send cursor rows using the binary protocol.
@@ -1964,6 +1965,17 @@
    if (tables)
      thd->warning_info->opt_clear_warning_info(thd->query_id);

+  /*
+    For the optimizer trace, this is the symmetric, for statement 
preparation,
+    of what is done at statement execution (in mysql_execute_command()).
+  */
+  Opt_trace_start ots(thd, tables, sql_command,
+                      thd->query(), thd->query_length(), NULL,
+                      thd->variables.character_set_client);
+
+  Opt_trace_object trace_command(&thd->opt_trace);
+  Opt_trace_array trace_command_steps(&thd->opt_trace, "steps");
+
    switch (sql_command) {
    case SQLCOM_REPLACE:
    case SQLCOM_INSERT:

which is really what wl4800 needs to work.
Can I leave it as it is? I'd prefer the wl4800 final patch to not have 
"style changes just for pleasure".

> @@ -2107,19 +2102,15 @@ static bool check_prepared_statement(Pre
> +    DBUG_RETURN(stmt->is_sql_prepare() ?
> +                FALSE : (send_prep_stmt(stmt, 0) || 
> thd->protocol->flush()));
> +error:
> +  DBUG_RETURN(TRUE);
> 
> jl: false, true

same here

> === modified file 'sql/opt_trace.h'
>  @param  support_dbug_or_support_missing_priv 'true' if this statement
>                           should have its trace in the dbug log (--debug),
>                           or if missing_privilege() may be called on this
>                           trace
> 
> jl: support_dbug_or_support_missing_priv is an awfully long name. Can 
> you make it shorter, at least by removing the second "support"?

Now using support_dbug_or_missing_priv.
Pushed.
Thread
bzr commit into mysql-trunk branch (tor.didriksen:3309) WL#4800Tor Didriksen19 May
  • Re: bzr commit into mysql-trunk branch (tor.didriksen:3309) WL#4800Jorgen Loland19 May
    • Re: bzr commit into mysql-trunk branch (tor.didriksen:3309) WL#4800Jorgen Loland19 May
    • Re: bzr commit into mysql-trunk branch (tor.didriksen:3309) WL#4800Guilhem Bichot20 May
      • Re: bzr commit into mysql-trunk branch (tor.didriksen:3309) WL#4800Jorgen Loland20 May
  • Re: bzr commit into mysql-trunk branch (tor.didriksen:3309) WL#4800Guilhem Bichot20 May
    • Re: bzr commit into mysql-trunk branch (tor.didriksen:3309) WL#4800Tor Didriksen20 May
      • Re: bzr commit into mysql-trunk branch (tor.didriksen:3309) WL#4800Guilhem Bichot21 May