Alexander Barkov added a comment - 2023-10-03 02:36 - edited psergei , thanks, good catch about MTR. The new method was needed to override the DATETIME-based code in Type_handler_temporal_result::Item_const_eq(), which compares two MYSQL_TIME values, and which is not relevant for TIMESTAMP any more because Item_timestamp_literal does not have a MYSQL_TIME value inside. Without the change, Type_handler_temporal_result::Item_const_eq() would crash on elimination of equal conditions having Item_timestamp_literal inside. But it appeared that trivial condition elimination haven't been covered in MTR at all so far, this is why you did not get crashes. Please find a new patch: https://github.com/MariaDB/server/commit/c44b546c0067bdf7aa66706af438b4b024a4578b Now instead of "return false", it implements detecting equal Item_timestamp_literal's inside this method, and adds MTR tests.

Sergei Petrunia added a comment - 2023-12-20 12:55 - edited An observation: IN is not covered: MariaDB [test]> EXPLAIN EXTENDED SELECT * FROM t1 WHERE a in ('2020-03-03 01:01:01', '2020-03-04 01:01:01'); +------+-------------+-------+-------+---------------+------+---------+------+------+----------+--------------------------+ | id | select_type | table | type | possible_keys | key | key_len | ref | rows | filtered | Extra | +------+-------------+-------+-------+---------------+------+---------+------+------+----------+--------------------------+ | 1 | SIMPLE | t1 | range | a | a | 4 | NULL | 2 | 100.00 | Using where; Using index | +------+-------------+-------+-------+---------------+------+---------+------+------+----------+--------------------------+ 1 row in set, 1 warning (0.002 sec)   MariaDB [test]> show warnings; +-------+------+-----------------------------------------------------------------------------------------------------------------------+ | Level | Code | Message | +-------+------+-----------------------------------------------------------------------------------------------------------------------+ | Note | 1003 | select `test`.`t1`.`a` AS `a` from `test`.`t1` where `test`.`t1`.`a` in ('2020-03-03 01:01:01','2020-03-04 01:01:01') | +-------+------+-----------------------------------------------------------------------------------------------------------------------+ Not sure if it can/should be done as part of this patch, yet, continuing with review. BETWEEN is not handled, either: explain format=json select * from t1 where a >= '2021-01-01 00:00:00' and a<='2021-02-02 00:00:00'; "attached_condition": "t1.a >= TIMESTAMP/*WITH LOCAL TIME ZONE*/'2021-01-01 00:00:00' and t1.a <= TIMESTAMP/*WITH LOCAL TIME ZONE*/'2021-02-02 00:00:00'"   explain format=json select * from t1 where a between '2021-01-01 00:00:00' and '2021-02-02 00:00:00'; "attached_condition": "t1.a between <cache>('2021-01-01 00:00:00') and <cache>('2021-02-02 00:00:00')" The commit comment for the patch: MDEV-32148 Inefficient WHERE timestamp_column=datetime_const_expr Changing the way how the following condition is evaluated: WHERE timestamp_column=datetime_const_expr Before the change it was always performed as DATETIME. ... this should be changed to indicate that non-equality comparisons are also handled. For --let $replace_regex_tsltz6= ... ... --replace_regex $replace_regex_tsltz6 I would ask to introduce something with less cryptic name and use it in only one location: --source include/replace_timestamp_w_local_tz_value.inc

Sergei Petrunia added a comment - 2023-12-22 10:27 A suggestion how to get rid of the extra warnings. At the cost of not providing the optimization for the queries that produce such warnings: --- item_cmpfunc.cc +++ item_cmpfunc.cc @@ -460,9 +460,28 @@ get_timestamp_item_for_comparison !expr1->type_handler()->can_return_date()) return NULL; + struct Count_handler : public Internal_error_handler + { + bool hit=false; + bool handle_condition(THD *thd, + uint sql_errno, + const char *sqlstate, + Sql_condition::enum_warning_level *level, + const char *msg, + Sql_condition **cond_hdl) + { + hit=true; + return *level == Sql_condition::WARN_LEVEL_WARN; + } + } cnt_handler; + + //Suppress_warnings_error_handler warning_handler; + thd->push_internal_handler(&cnt_handler); + Datetime dt(thd, expr1, Timestamp::DatetimeOptions(thd)); - if (!dt.is_valid_datetime()) + thd->pop_internal_handler(); + if (cnt_handler.hit || !dt.is_valid_datetime()) return NULL; // SQL NULL DATETIME, or a DATETIME with zeros in YYYYMMDD // '0000-00-00 00:00:00' is a special valid MariaDB TIMESTAMP value

Sergei Petrunia added a comment - 2023-12-30 12:45 Note to self: The patch has this call: expr1->can_eval_in_optimize() and it will evaluate ##expr1## if that call returns TRUE. This will happen at Name Resolution (fix_fields()) phase. However, this will not attempt to evaluate "cheap" subqueries: subqueries do not yet have query plans at Name Resolution stage, so they are not considered "cheap".

Sergei Petrunia added a comment - 2024-01-08 12:21 Ok, the patch needs the following: Address the input in https://jira.mariadb.org/browse/MDEV-32148?focusedCommentId=277190&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-277190 Consider applying comment/formatting fixes from here: https://gist.github.com/spetrunia/89531b74dfb7ba8aeaf6c293e31aa909 Change the target version to be 11.3 (Discussed this with Monty) After that, it's ok to push into 11.3.