List:Commits« Previous MessageNext Message »
From:Guilhem Bichot Date:February 7 2011 8:52pm
Subject:Re: bzr commit into mysql-next-mr-bugfixing branch (jorgen.loland:3219)
WL#5594
View as plain text  
Hello Jorgen,

Jorgen Loland a écrit, Le 22.10.2010 09:04:
>  >>    
>
> seq->param->max_key_part=max(seq->param->max_key_part,key_tree->part);
>  >> +
>  >> +  if (key_range_trace.length())
>  >> +  {
>  >> +    DBUG_ASSERT(seq->param->thd->opt_trace);
>  >> +    DBUG_ASSERT(seq->param->thd->opt_trace->is_started());
>  >> +    Opt_trace_array *trace_range=
>  >> +      
> (Opt_trace_array*)seq->param->thd->opt_trace->get_current_struct();
>  >
> 
>  >GB Do we have a way to pass this array down from the callers, for more
>  >safety? Maybe by adding it as a member of "SEL_ARG_RANGE_SEQ seq" (I'm
>  >looking for a way which doesn't affect the storage engine API).
> 
> Yes, that's possible, but I'm not sure I like that better than getting
> it like here.
> 
>  >If impossible, it would be better to not cast, and declare trace_range
>  >of type Opt_trace_struct*. Indeed, with casting, if the struct happens
>  >to be Opt_trace_object, a wrong cast may give a crash including in
>  >release binaries. Without casting, the worse which can happen is a
>  >JSON syntax error, which asserts in debug builds but is not dangerous
>  >in release binaries (see Opt_trace_context::syntax_error()).
> 
> Good point! Cast removed.
> 
>  >I have the goal to make get_current_struct() private soon if
>  >possible. So, nobody could access "the current struct"; it could just
>  >add objects/arrays (which implicitely adds them as children of the
>  >current struct), or pass down an object/array if a function wants to
>  >add to it.
> 
> Ok, but can I please get this patch in first?

Now that the range opt patch has been pushed, it's time to think about 
this get_current_struct().
I suggest that the current structure (Opt_trace_array), which is created 
by handler::multi_range_read_info_const(), is passed by that creator to 
seq->next(), as a third parameter named "trace_array".
So the signature of RANGE_SEQ_IF::next() would get a third parameter 
(minor change to handler API, quite probably ok in Fuchsia).
If that *Opt_trace_array parameter is not NULL, next() would be expected 
to add to it (and it's not deadly if the engine implementing next() 
forgets to do that: the trace will just miss information). Only the 
default implementation of next() (sel_arg_range_seq_next()) would add to 
the array (as it is now: only sel_arg_range_seq_next() adds to the 
array; quick_range_seq_next(), bka_range_seq_next(), 
bka_unique_range_seq_next() don't).
Then get_current_struct() would be deleted from Opt_trace_context, which 
is better for isolation.

What do you think of this?
Thread
bzr commit into mysql-next-mr-bugfixing branch (jorgen.loland:3219) WL#5594Jorgen Loland7 Oct
  • Re: bzr commit into mysql-next-mr-bugfixing branch (jorgen.loland:3219)WL#5594Guilhem Bichot16 Oct
    • Re: bzr commit into mysql-next-mr-bugfixing branch (jorgen.loland:3219)WL#5594Jorgen Loland22 Oct
      • Re: bzr commit into mysql-next-mr-bugfixing branch (jorgen.loland:3219)WL#5594Guilhem Bichot10 Nov
        • Re: bzr commit into mysql-next-mr-bugfixing branch (jorgen.loland:3219)WL#5594Jorgen Loland12 Nov
          • Re: bzr commit into mysql-next-mr-bugfixing branch (jorgen.loland:3219)WL#5594Guilhem Bichot15 Nov
      • Re: bzr commit into mysql-next-mr-bugfixing branch (jorgen.loland:3219)WL#5594Guilhem Bichot7 Feb
        • Re: bzr commit into mysql-next-mr-bugfixing branch (jorgen.loland:3219)WL#5594Jorgen Loland8 Feb