MDEV-10426 ANY_VALUE function#5151
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements the ANY_VALUE() function by adding it to the client commands, defining the Item_func_any_value class (inheriting from Item_func_coalesce), and registering it in the function registry. Feedback on the changes points out a critical issue: because Item_func_any_value inherits from Item_func_coalesce, it will still trigger ONLY_FULL_GROUP_BY errors unless check_cols_in_group_by() is overridden to return false directly.
c6f6e73 to
9525ad3
Compare
Per Daniel Black's reference to PR MariaDB#5151 (copy_or_same() fix for ANY_VALUE), investigated whether the same gap exists for our BIT_AND/BIT_OR/BIT_XOR binary mode aggregates. Finding: no bug exists. Item_sum_or/Item_sum_and/Item_sum_xor already have correct copy_or_same() implementations that clone via the Item_sum_bit copy constructor, which properly deep-copies m_str_value and m_binary_bit_counters for each rollup level's independent accumulator. Verified WITH ROLLUP produces mathematically correct results for binary mode BIT_OR/BIT_AND/BIT_XOR (manually cross-checked byte by byte against expected values). Added Section 16 as permanent regression coverage for this previously-untested combination.
| SELECT charset(ANY_VALUE(a)), collation(ANY_VALUE(a)) FROM t; | ||
| SELECT coercibility(ANY_VALUE(a)), coercibility(ANY_VALUE('x')) FROM t; | ||
|
|
||
| DROP TABLE t; |
There was a problem hiding this comment.
This all seems fine so far.
Bonus points, consider this
create table t1 (a int, b int, index(a));
insert into t1 values (1, 1), (1, 2), (2, 3);
select a, any_value(b) from t1 group by a;
does the row t1(a,b) = (1,2) need to read by the engine/handler?
Is it?
There is already similar code in the server (see loosescan here)
https://mariadb.com/docs/server/ha-and-performance/optimization-and-tuning/query-optimizer/minmax-optimization
Note: i don't actually expect you to implement this, but do write up something.
There was a problem hiding this comment.
Also, as you've added a token to the parser, you've changed the 'digest hashes' in a perfschema test. You'll need to record/update those too.
There was a problem hiding this comment.
Also also, there is a single line in a commit with whitespace before a newline, remove that whitespace.
Add some tests without ONLY_FULL_GROUP_BY sql_mode, and demonstrate how to use it as a window function.
There was a problem hiding this comment.
Updated the digest hashes, fixed whitespace, and added the additional test cases. I will report back with short circuit possibilities for any_value
86dfc73 to
7660557
Compare
gkodinov
left a comment
There was a problem hiding this comment.
Thank you for working on this. This is a preliminary review.
Please:
- squash your commits into one per feature. I guess one in your case?
- I'd also consider resolving the conflicts with the upstream branch sooner than later.
Is it okay if I save this step until the very end? In the case that I need to make further modifications during the review process, it would be helpful for me to retain the commit history |
8cc1af6 to
7a00a0c
Compare
… to non_agg_fields only if BOTH in_sum_func and in_any_value are false
…related subquery as long as it is aggregated
…g Item_sum_min_max
- Tests without ONLY_FULL_GROUP_BY - More NULL tests - Window function tests
7a00a0c to
e87743f
Compare
MDEV-10426: Add support for the MySQL
ANY_VALUE()function (SQL:2023 feature T626).