Hi Olav,
Thank you for the review. Unless commented on below, I have incorporated your
exact suggestions
On 05/06/2011 11:25 AM, Olav Sandstaa wrote:
> Hi Jørgen,
>
> Thanks for making this effort on adding systematic comments to the key_or()
> function. I like your "ascii art" comments. This is makes it much easier to
> understand what and where things are done.
>
> I have only very minor comments. OK to push this patch after you have looked at
> my comments.
>
> Olav
>
> + /*
> + Used to describe how two key values are positioned compared to
> + each other. Consider key_value_a.<cmp_func>(key_value_b):
> +
> + -2: key_value_a is smaller than key_value_b, and they are adjacent
> + -1: key_value_a is smaller than key_value_b (not adjacent)
> + 0: the key values are equal
> + 1: key_value_a is bigger than key_value_b (not adjacent)
> + -2: key_value_a is bigger than key_value_b, and they are adjacent
> +
> + Example: "cmp= tmp->cmp_max_to_min(key2)"
> +
> + key2: [-------- (10 <= x ...)
> + tmp: -----] (... x < 10) => cmp==-2
> + tmp: ----] (... x <= 9) => cmp==-1
> + tmp: ------] (... x = 10) => cmp== 0
> + tmp: --------] (... x <= 12) => cmp== 1
>
> Great explanation. For completeness maybe include also an example for cmp==2?
I would have to change to another compare function then. cmp == 2 does not make
sense for cmp_max_to_min()
> + /*
> + This is the case:
> + key2: [-------]
> + tmp: [----]
> + */
> SEL_ARG *next=tmp->next;
> - /* key1 on the left of key2 non-overlapping */
>
> Consider including this comment in the above (new comment). I found it useful
> since it included the word "key1" instead of just tmp.
I added this instead:
/*
+ key1 consists of one or more ranges. tmp is the range currently
+ being handled.
+
initialize tmp to the latest range in key1 that starts the same
place or before the range in key2 starts
key2: [------]
key1: [---] [-----] [----]
^
tmp
*/
SEL_ARG *tmp=key1->find_range(key2);
--
Jørgen Løland | Senior Software Engineer | +47 73842138
Oracle MySQL
Trondheim, Norway
