Hi, Alexander!
- We'll probably have more FORMATs in the future.
Like what? As far as I can see, standard doesn't support FORMAT in the column definition at all.
Adding all format tests inside "normal" Field_string/Field_varchar/Field_blob is not a good idea.
+ */ + if (Type_handler_json_common::has_json_valid_constraint(this))
could you please use names that don't depend on how exactly we detect json? especially if the goal is to move to FORMAT JSON syntax
For now this name reflects what it actually does.
When we split JSON off and fix the code to have JSON put extra data type information into FRM, this test will be gone from Field_xxx.
But it will move to the code opening FRM, to be able open old tables where JSON is detected only by a CHECK constraint. And there it will reflect what it actually does again.
I propose to keep it as is.
ok
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; + } + bool done() const + { + switch (m_switched_to_base_count) + { + case 2: + /* + Expression: + COALESCE(TYPE1, TYPE2) + + where both type handlers derive from some other handlers, e.g. + VARCHAR -> TYPE1 + TEXT -> TYPE2 + + Continue aggregation as: + SELECT COALESCE(VARCHAR, TEXT) + + Note, we now have only one level of data type inheritance, + e.g. VARCHAR -> VARCHAR/JSON + + This code branch will change when we have longer inheritance chains: + A0 -> A1 -> A2 -> A3 + B0 -> B1 -> B2 + + Functions like COALESE(A2, B2) will need to do full overload resolution: + - iterate through all pairs from the below matrix + - find all existing candidates + - resolve ambiguities in case multiple candidates, etc.
and I don't see how you're going to do that. you need to try m_a+m_b.base(), m_a.base()+m_b, and m_a.base()+m_b.base(). And that's not what you're doing.
I suggest to add an assert that m_switched_to_base_count < 2 and stop trying to solve complex cases until we have at least one.
Yes, we'll need to do all these combinations eventually. But I'm not trying to solve a more complex case than it's really necessary now:
If the SQL query is:
SELECT varchar_json_expr + text_json_expr FROM t1;
it works as follows:
- It tries VARCHAR/JSON+TEXT/JSON in Type_collection_json, nothing is found
- Then it tries directly VARCHAR+TEXT, which is m_a.base()+m_b.base(), and this combination is found in Type_collection_std,
I'd expect it to find VARCHAR/JSON+TEXT/JSON right away. It would've naturally preserved the json property. Otherwise you'd need to do an extra json propagation. Here, of course, I mean not Item_func_plus, but, say, COALESCE. Or UNION.
diff --git a/sql/sql_type_json.cc b/sql/sql_type_json.cc index a804366ec03..a84f720bcc9 100644 --- a/sql/sql_type_json.cc +++ b/sql/sql_type_json.cc @@ -20,17 +20,111 @@ #include "sql_class.h"
-Type_handler_json_longtext type_handler_json_longtext; +Named_type_handler<Type_handler_string_json> + type_handler_string_json("char/json");
do you mean these names are not shown anywhere?
They are shown in error messages, e.g.:
Illegal parameter data types inet6 and longblob/json for operation '+'
eh, okay. may be we should change it later to "longblob format json". But not now. Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org