Hi Ole John,
Thank you for the patch. I have only one real concern as can be seen inline.
> -void QUICK_RANGE_SELECT::need_sorted_output()
> +void QUICK_RANGE_SELECT::need_sorted_output(bool sort)
> {
> - if (!(mrr_flags& HA_MRR_SORTED))
> + if (sort)
> {
> - /*
> - Native implementation can't produce sorted output. We'll have to
> - switch to default
> - */
> - mrr_flags |= HA_MRR_USE_DEFAULT_IMPL;
> + if (!(mrr_flags& HA_MRR_SORTED))
> + {
> + /*
> + Native implementation can't produce sorted output. We'll have to
> + switch to default
> + */
> + mrr_flags |= HA_MRR_USE_DEFAULT_IMPL;
> + }
> + mrr_flags |= HA_MRR_SORTED;
> + }
> + else
> + {
> + mrr_flags&= ~HA_MRR_SORTED;
> }
> - mrr_flags |= HA_MRR_SORTED;
> }
If need_sorted_output(false) is called after need_sorted_output(true), mrr_flags
is incorrectly tagged with HA_MRR_USE_DEFAULT_IMPL.
> @@ -10976,7 +10994,11 @@ int QUICK_GROUP_MIN_MAX_SELECT::reset(vo
> DBUG_ENTER("QUICK_GROUP_MIN_MAX_SELECT::reset");
>
> head->set_keyread(TRUE); /* We need only the key attributes */
> - if ((result= file->ha_index_init(index,1)))
> +
> + // Request ordered index access as usage of ::index_last(), ::index_first()
> + // within QUICK_GROUP_MIN_MAX_SELECT depends on it.
Multi-line comments should have the format
/*
comment
*/
> + bool sorted= true;
> + if ((result= file->ha_index_init(index,sorted)))
It's an improvement to add the comment and change from 1 to true, but IMO it's a
bit overkill to create a variable since it's only used once.
--
Jørgen Løland | Senior Software Engineer | +47 73842138
Oracle MySQL
Trondheim, Norway
