List:Commits« Previous MessageNext Message »
From:Tor Didriksen Date:May 19 2011 10:55am
Subject:bzr commit into mysql-trunk branch (tor.didriksen:3309) WL#4800
View as plain text  
#At file:///export/home/didrik/repo/next-mr-opt-backporting-wl4800/ based on revid:jorgen.loland@stripped

 3309 Tor Didriksen	2011-05-19
      WL#4800 Review comments.

    modified:
      WL4800_TODO.txt
      sql/opt_trace.cc
      sql/opt_trace.h
      sql/opt_trace2server.cc
      sql/sql_array.h
      sql/sql_select.cc
=== modified file 'WL4800_TODO.txt'
--- a/WL4800_TODO.txt	2011-05-04 20:53:28 +0000
+++ b/WL4800_TODO.txt	2011-05-19 10:55:07 +0000
@@ -85,6 +85,8 @@ included everywhere. Of opt_trace.h, sql
 Opt_trace_context. Should declaration of Opt_trace_context move to a
 separate smaller include file, to be included in sql_class.h?
 Guilhem suggests: yes.
+Tor: +1
 
 C10) Guilhem to Tor: can we delete opt_notrace-t.cc now? I think I
 replied in some old mail about this file.
+Tor: yes.

=== modified file 'sql/opt_trace.cc'
--- a/sql/opt_trace.cc	2011-05-05 15:47:46 +0000
+++ b/sql/opt_trace.cc	2011-05-19 10:55:07 +0000
@@ -211,6 +211,7 @@ public:
   void assert_current_struct(const Opt_trace_struct *s) const
   { DBUG_ASSERT(current_struct == s); }
 
+  /// ?????????????????????????????????
   void missing_privilege();
 
   bool support_I_S() const { return I_S_disabled == 0; }
@@ -944,7 +945,7 @@ bool Opt_trace_context::start(bool suppo
 
   /*
     Decide on optimizations possible to realize the requested support.
-    If I_S or debug output is requested, need to create an Opt_trace_stmt.
+    If I_S or debug output is requested, we need to create an Opt_trace_stmt.
     Same if we should support calls to Opt_trace_context::missing_privilege(),
     because that function requires an Opt_trace_stmt.
   */

=== modified file 'sql/opt_trace.h'
--- a/sql/opt_trace.h	2011-05-04 20:53:28 +0000
+++ b/sql/opt_trace.h	2011-05-19 10:55:07 +0000
@@ -25,6 +25,7 @@
 struct st_schema_table;
 struct TABLE_LIST;
 struct TABLE;
+class sp_head;
 class sp_instr;
 
 /**
@@ -433,7 +434,7 @@ public:
                               member function has undefined effects.
   */
   bool start(bool support_I_S,
-             bool support_dbug_or_support_t_missing_priv,
+             bool support_dbug_or_support_missing_priv,
              bool end_marker, bool one_line,
              long offset, long limit, ulong max_mem_size,
              ulonglong features);
@@ -684,6 +685,10 @@ private:
     long since_offset_0;
   };
 
+  // An XXX_impl class is usually hidden away in the .cc file
+  //   but OK, I accept your request for inlining.
+
+  // The pointer to the impl is traditionally named 'pimpl'
   Opt_trace_context_impl *impl;
 
   /**
@@ -1308,12 +1313,14 @@ class st_select_lex;
 void opt_trace_print_expanded_query(THD *thd,
                                     st_select_lex *select_lex);
 
+// ???????????????????
 void opt_trace_disable_if_no_view_access(THD *thd, TABLE_LIST *view,
-                                 TABLE_LIST *underlying_tables);
+                                         TABLE_LIST *underlying_tables);
 
-class sp_head;
+// ???????????????????
 void opt_trace_disable_if_no_stored_proc_func_access(THD *thd, sp_head *sp);
 
+// ???????????????????
 void opt_trace_disable_if_no_security_context_access(THD *thd);
 
 /**
@@ -1435,7 +1442,8 @@ public:
 };
 
 #define opt_trace_print_expanded_query(thd, select_lex) do {} while (0)
-#define opt_trace_disable_if_no_view_access(thd, view, underlying_tables) do {} while (0)
+#define opt_trace_disable_if_no_view_access(thd, view, underlying_tables) \
+        do {} while (0)
 #define opt_trace_disable_if_no_stored_proc_func_access(thd, sp) do {} while (0)
 #define opt_trace_disable_if_no_security_context_access(thd) do {} while (0)
 

=== modified file 'sql/opt_trace2server.cc'
--- a/sql/opt_trace2server.cc	2011-05-05 15:47:46 +0000
+++ b/sql/opt_trace2server.cc	2011-05-19 10:55:07 +0000
@@ -239,8 +239,8 @@ void opt_trace_print_expanded_query(THD 
    @name Description of trace-induced additional security checks.
    A trace exposes information. For example if one does SELECT on a view, the
    trace contains the view's body. So, the user should be allowed to see the
-   trace only if she/he has privilege to see the body, i.e. privilege to do SHOW
-   CREATE VIEW.
+   trace only if she/he has privilege to see the body, i.e. privilege to do
+   SHOW CREATE VIEW.
    There are similar issues with stored procedures, functions, triggers.
 
    We implement this by doing additional checks on SQL objects when tracing is
@@ -286,7 +286,7 @@ void opt_trace_print_expanded_query(THD 
    - from files (reading their content), with LOAD_FILE()
    - from the list of connections (reading their queries...), with
    I_S.PROCESSLIST.
-   - from etc, maybe.
+   - from etc, maybe.  ???
    If the connected user has EXECUTE privilege on a routine which does a
    security context change, the routine can retrieve information internally
    (if allowed by the SUID context's privileges), and present only a portion
@@ -314,7 +314,7 @@ void opt_trace_disable_if_no_security_co
   DBUG_ENTER("opt_trace_check_disable_if_no_security_context_access");
   if (likely(!(thd->variables.optimizer_trace &
                Opt_trace_context::FLAG_ENABLED)) || // (1)
-      thd->system_thread)                        // (2)
+      thd->system_thread)                           // (2)
   {
     /*
       (1) We know that the routine's execution starts with "enabled=off".
@@ -361,10 +361,13 @@ void opt_trace_disable_if_no_security_co
   */
   if (!(test_all_bits(thd->main_security_ctx.master_access,
                       (GLOBAL_ACLS & ~GRANT_ACL))) &&
-      (strcmp(thd->main_security_ctx.priv_user,
-              thd->security_ctx->priv_user) ||
-       my_strcasecmp(system_charset_info, thd->main_security_ctx.priv_host,
-                     thd->security_ctx->priv_host)))
+      ( 0 != strcmp(thd->main_security_ctx.priv_user,
+                    thd->security_ctx->priv_user)
+        ||
+        0 != my_strcasecmp(system_charset_info,
+                           thd->main_security_ctx.priv_host,
+                           thd->security_ctx->priv_host)
+       ))
     trace->missing_privilege();
   DBUG_VOID_RETURN;
 #endif
@@ -449,12 +452,15 @@ void opt_trace_disable_if_no_view_access
   Security_context * const backup_table_sctx= view->security_ctx;
   Security_context * const backup_thd_sctx= thd->security_ctx;
   const GRANT_INFO backup_grant_info= view->grant;
+
   view->security_ctx= NULL;                   // no SUID context for view
   thd->security_ctx= &thd->main_security_ctx; // no SUID context for THD
   const int rc= check_table_access(thd, SHOW_VIEW_ACL, view, false, 1, true);
+
   view->security_ctx= backup_table_sctx;
   thd->security_ctx= backup_thd_sctx;
   view->grant= backup_grant_info;
+
   if (rc)
   {
     trace->missing_privilege();
@@ -583,10 +589,13 @@ int fill_optimizer_trace_info(THD *thd, 
   */
   if (!test_all_bits(thd->security_ctx->master_access,
                      (GLOBAL_ACLS & ~GRANT_ACL)) &&
-      (strcmp(thd->main_security_ctx.priv_user,
-              thd->security_ctx->priv_user) ||
-       my_strcasecmp(system_charset_info, thd->main_security_ctx.priv_host,
-                     thd->security_ctx->priv_host)))
+      ( 0 != strcmp(thd->main_security_ctx.priv_user,
+                    thd->security_ctx->priv_user)
+        ||
+        0 != my_strcasecmp(system_charset_info,
+                           thd->main_security_ctx.priv_host,
+                           thd->security_ctx->priv_host)
+        ))
     return 0;
   /*
     The list must not change during the iterator's life time. This is ok as

=== modified file 'sql/sql_array.h'
--- a/sql/sql_array.h	2011-04-05 07:34:13 +0000
+++ b/sql/sql_array.h	2011-05-19 10:55:07 +0000
@@ -73,6 +73,7 @@ public:
      Pops the last element. The returned element must not be used after the
      next update to the array.
   */
+  // This interface is still wrong!!
   const Elem& pop()
   {
     return *static_cast<Elem *>(pop_dynamic(&array));

=== modified file 'sql/sql_select.cc'
--- a/sql/sql_select.cc	2011-05-13 14:02:31 +0000
+++ b/sql/sql_select.cc	2011-05-19 10:55:07 +0000
@@ -10269,6 +10269,7 @@ static bool make_join_select(JOIN *join,
               sel->cond=orig_cond;
               if (!*tab->on_expr_ref)
 		DBUG_RETURN(1);			// Impossible WHERE
+// untabify above
               Opt_trace_object trace_without_on(trace, "without_ON_clause");
               if (sel->test_quick_select(thd, tab->keys,
                                          used_tables & ~ current_map,


Attachment: [text/bzr-bundle] bzr/tor.didriksen@oracle.com-20110519105507-axasc7wx7m394lv3.bundle
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