MDEV-33971 NAME_CONST in WHERE clause replaced by inner item

[DaveGosselin-MariaDB]

Improve performance of queries like
SELECT * FROM t1 WHERE field = NAME_CONST('a', 4);
by, in this example, replacing the WHERE clause with field = 4 in the case of ref access.

The rewrite is done during fix_fields and we disambiguate this case from other cases of NAME_CONST by inspecting where we are in parsing. We rely on THD::where to accomplish this. To improve performance there, we change the type of THD::where to be an enumeration, so we can avoid string comparisons during Item_name_const::fix_fields. Consequently, this patch also changes all usages of THD::where to conform likewise.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[spetrunia]

Generally, looks like a step in the right direction.

I'm wondering if the patch could be smaller if there was a const char* thd_where_is_where_cond and then one could compare thd->where strings by comparing pointers... but since we've added an enum, let's not remove it. It has an advantage of listing the possible values of thd->where ...

I think in addition to handling WHERE, we need to handle ON expressions.
Handling other expressions (like HAVING) looks more risky as they may have references to select list values...

The patch needs test cases... I used this to play:

create table t1 (a int, b int);
insert into t1 values (1,1),(2,2);

explain format=json
select * from t1 where a=name_const(1,'varname');

explain format=json
select * from t1 left join t1 as t2 on t1.a=name_const(1,'varname') and t1.b=t2.b;

drop table t1;

[spetrunia]

One other question:

What if Item_const::fix_fields() always did the following, regardless of its location:

• call value_item->set_name(...) in the same way it does for itself in Item_const::Item_const.
• substitute itself with value_item

Will this work (EDIT: quick attempt: no, rpl.rpl_name_const fails) and if not why?

[DaveGosselin-MariaDB]

Generally, looks like a step in the right direction.

Thanks! 😄

I'm wondering if the patch could be smaller if there was a const char* thd_where_is_where_cond and then one could compare thd->where strings by comparing pointers... but since we've added an enum, let's not remove it. It has an advantage of listing the possible values of thd->where ...

I had considered string pointers very briefly, but decided against that approach for a number of reasons:

• pointer values are hard to distinguish in logs (should these get logged)
• enumerations are easier to read and (sometimes) remember
• future maintainers could look at the string pointer comparisons and say "did he really mean to compare the pointer values and not the strings themselves?" which would lead to endless stumbling.
• comparing values across different builds would be impossible

Generally, enumerations are more intentional and less ambiguous.

I think in addition to handling WHERE, we need to handle ON expressions. Handling other expressions (like HAVING) looks more risky as they may have references to select list values...

Would that be appropriate to do now or as a separate MDEV?

[DaveGosselin-MariaDB]

One other question:

What if Item_const::fix_fields() always did the following, regardless of its location:

• call value_item->set_name(...) in the same way it does for itself in Item_const::Item_const.
• substitute itself with value_item

Will this work (EDIT: quick attempt: no, rpl.rpl_name_const fails) and if not why?

This is an interesting idea, we should explore this further in another MDEV.

[spetrunia]

Got a question:

https://jira.mariadb.org/browse/MDEV-33971?focusedCommentId=285058&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-285058

need to sort this out first.