Hi Yuchen, On Thu, Nov 09, 2023 at 11:57:35AM +1100, Yuchen Pei wrote:
Hi Sergey,
[... 14 lines elided]
Generally the patch looks good. I have only a few cosmetic comments.
Thanks for the comments. I've updated my patch and the commit is in my latest comment in the ticket. I understand this task has fix version 11.5 so no rush.
Please rename the test file from mdev_27576.test to something more descriptive like desc_index_min_max.test (following desc_index_range.test)
Done.
Also please rename the table in the test to t1.
Done, but why?
It's a convention to name tables t1,t2 ... One may argue if it has a lot of merit. At least one of them is that one known which table names would not conflict with tables used in the test.
diff --git a/sql/sql_select.h b/sql/sql_select.h index e0bbadadc69..45d7527280a 100644 --- a/sql/sql_select.h +++ b/sql/sql_select.h @@ -126,6 +126,7 @@ typedef struct st_table_ref uint key_parts; ///< num of ... uint key_length; ///< length of key_buff int key; ///< key no + bool reverse; ///< key is reverse uchar *key_buff; ///< value to look for with key uchar *key_buff2; ///< key_buff+key_length store_key **key_copy; //
It's actually not "key is reverse" but "last used key part is reverse"... also this member is never set for the most common scenario of TABLE_REF use, which is ref access.
The member is set in a rather trivial logic in find_key_for_maxmin(), and then it's used in get_index_min_value() and get_index_max_value().
I suggest that we do without this member. get_index_min_value() and get_index_max_value() can check:
bool reverse= table->key_info[ref->key].key_part[ref->key_part]->key_part_flag & HA_REVERSE_SORT
I tried this, but it did not work. For example it will not work on ...
I still think it could work, I've just made a typo, it should be table->key_info[ref->key].key_part[ref->key_parts-1].key_part_flag but it's fine if a local variable is passed around. Please apply the attached diff fixing coding style. After that let's create a preview tree and submit the feature for testing. BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Blog: http://petrunia.net