← Back to team overview

maria-developers team mailing list archive

Re: Review request: SHOW EXPLAIN

 

Hi, Sergei!

A couple of replies:

On Jun 26, Sergei Petrunia wrote:
> > > +  /*
> > > +    Todo: we should remove this check for thd->lex->describe on the next
> > > +    line. With SHOW EXPLAIN code, EXPLAIN printout code no longer depends
> > > +    on it. However, removing the check caused change in lots of query
> > > +    plans! Does the optimizer depend on the contents of
> > > +    table_ref->key_copy ? If yes, do we produce incorrect EXPLAINs? 
> > > +  */
> >  
> > Are you going to do something about it, or just leave the comment around?
> 
> I'll just leave a comment, for now. I don't want SHOW EXPLAIN patch to include
> a rewrite of parts of the optimizer.  I think, eventually we will figure out
> the reason for the difference and remove dependency on thd->lex->describe.

That's ok. But then I'd suggest to create a task for it in Jira (and
keep the comment too).

> > > === modified file 'sql/protocol.h'
> > > --- sql/protocol.h	2012-01-13 14:50:02 +0000
> > > +++ sql/protocol.h	2012-06-12 19:04:11 +0000
> > > @@ -73,6 +78,20 @@ public:
> > >    virtual bool send_result_set_metadata(List<Item> *list, uint flags);
> > >    bool send_result_set_row(List<Item> *row_items);
> > >  
> > > +  void get_packet(const char **start, size_t *length) 
> > > +  {
> > > +    *start= packet->ptr();
> > > +    *length= packet->length(); 
> > > +  }
> > 
> > This is part of your select_result_explain_buffer code.
> 
> I don't understand the question? Anyway, this part is likely to be gone
> after SHOW EXPLAIN is made to use I_S tables.

Exactly. That's what I meant by my comment.

> > > +void mysqld_show_explain(THD *thd, ulong thread_id)
> > > +{
> > > +  THD *tmp;
> > > +  Protocol *protocol= thd->protocol;
> > > +  List<Item> field_list;
> > > +  DBUG_ENTER("mysqld_show_explain");
> > > +  
> > 
> > You don't seem to check the privileges here.
>  
> I check them inside mysql_execute_command(), like KILL does.

SHOW PROCESSLIST verifies that either one has PROCESS_ACL or one can see
only his own threads. I don't see where you have this check. The one in
mysql_execute_command() doesn't do it, as far as I can see.

May be I'm wrong here.
But anyway, when you add tests for privileges, you'll see what works and
what doesn't.

Regards,
Sergei


References