Hi, Alexander! On Jan 17, Alexander Barkov wrote:
diff --git a/sql/item_jsonfunc.cc b/sql/item_jsonfunc.cc index ddf5fc32ea4..81003be4656 100644 --- a/sql/item_jsonfunc.cc +++ b/sql/item_jsonfunc.cc @@ -1441,6 +1441,32 @@ longlong Item_func_json_contains_path::val_int() ... +static bool is_json_type(const Item *item) +{ + for ( ; ; ) + { + if (Type_handler_json_common::is_json_type_handler(item->type_handler())) + return true; + const Item_func_conv_charset *func; + if (!(func= dynamic_cast
(item))) + return false; + item= func->arguments()[0]; can you have nested CONVERT()'s ?
We cannot have auto-generated nested CONVERT().
The new code just reproduces the old behavior: Item_func_conv_charset::is_json_type() traversed through all Item_func_conv_charset's recursively.
As the comment says, this is probably not correct. Let's fix it under terms of a separate MDEV. I can file an MDEV about this.
please, do
diff --git a/sql/sql_type.cc b/sql/sql_type.cc index c1801c1ae3e..3b2753d80a6 100644 --- a/sql/sql_type.cc +++ b/sql/sql_type.cc @@ -1755,19 +1755,110 @@ const Type_handler *Type_handler_typelib::cast_to_int_type_handler() const
/***************************************************************************/
+class Recursive_type_pair_iterator +{ + const Type_handler *m_a; + const Type_handler *m_b; + uint m_switched_to_base_count; +public: + Recursive_type_pair_iterator(const Type_handler *a, + const Type_handler *b, + uint switched_to_base_count= 0) + :m_a(a), m_b(b), m_switched_to_base_count(switched_to_base_count) + { } + const Type_handler *a() const { return m_a; } + const Type_handler *b() const { return m_b; } + Recursive_type_pair_iterator base() const + { + Recursive_type_pair_iterator res(m_a->type_handler_base(), + m_b->type_handler_base()); + res.m_switched_to_base_count= (res.m_a != NULL) + (res.m_b != NULL); + if (res.m_a == NULL) + res.m_a= m_a; + if (res.m_b == NULL) + res.m_b= m_b; + return res;
that's an unusual semantics, not what I would expect from an iterator. It'd expect it to iterare with it++; or it.next(); but not it = it.base();
I agree it did not look nice.
I replaced Recursive_type_pair_iterator to a more generic new class:
class Type_handler_pair { const Type_handler *m_a; const Type_handler *m_b; ... };
why did you do it this way? You create an object. Then in the loop you create a new object, copying the old one. Then you conditionally assign m_a->type_handler_base(). Then you diff then to see if any condition from the previous step has succeeded. Why not, like class Type_handler_pair { bool up() { bool r=false; n_a=m_a->type_handler_base(); n_b=m_b->type_handler_base(); if (n_a) { m_a=n_a; r=true; } if (n_b) { m_a=n_b; r=true; } return r; } and later do { ... } while (tp.up()); also don't forget to update the commit comment, it still says "Recursive_type_pair_iterator".
+ }
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org