Hello Sergei, Thanks for the review. An updated patch is attached. See comments below: On 10/11/21 8:44 PM, Sergei Golubchik wrote:
Hi, Alexander!
See comments below
diff --git a/plugin/type_inet/mysql-test/type_inet/type_inet6_innodb.test b/plugin/type_inet/mysql-test/type_inet/type_inet6_innodb.test index dd6049abbf3..3f3a6fdac2c 100644 --- a/plugin/type_inet/mysql-test/type_inet/type_inet6_innodb.test +++ b/plugin/type_inet/mysql-test/type_inet/type_inet6_innodb.test @@ -12,6 +12,30 @@ SET default_storage_engine=InnoDB; --source type_inet6_engines.inc
+--echo # +--echo # MDEV-26742 Assertion `field->type_handler() == this' failed in FixedBinTypeBundle<NATIVE_LEN, MAX_CHAR_LEN>::Type_handler_fbt::stored_field_cmp_to_item +--echo # + +CREATE TABLE t1 (pk inet6, c text) engine=myisam; +INSERT INTO t1 VALUES ('::',1); +CREATE TABLE t2 (d text, KEY (d)) engine=innodb ; +INSERT INTO t2 VALUES (2); +--error ER_TRUNCATED_WRONG_VALUE +UPDATE t2 JOIN t1 ON ( t1.pk > t2.d) SET t1.c = 1; +SET sql_mode=''; +UPDATE t2 JOIN t1 ON ( t1.pk > t2.d) SET t1.c = 1; +SET sql_mode=DEFAULT; +SELECT * FROM t1; +SELECT * FROM t2; +DROP TABLE t1, t2; + +CREATE TABLE t1 (pk inet6, c text) engine=myisam; +INSERT INTO t1 VALUES ('::',1); +CREATE TABLE t2 ( d text, KEY (d)) engine=innodb ;
I don't get it, why do you recreate tables with exactly the same definition and exactly the same content?
I joined these two tests inside a single CREATE..DROP.
+INSERT INTO t2 VALUES (2); +SELECT * FROM t2 JOIN t1 ON ( t1.pk > t2.d); +DROP TABLE t1, t2; +
--echo # --echo # End of 10.5 tests diff --git a/plugin/type_inet/mysql-test/type_inet/type_inet6_myisam.test b/plugin/type_inet/mysql-test/type_inet/type_inet6_myisam.test index c5183f01cf0..1ccd654181c 100644 --- a/plugin/type_inet/mysql-test/type_inet/type_inet6_myisam.test +++ b/plugin/type_inet/mysql-test/type_inet/type_inet6_myisam.test @@ -10,6 +10,21 @@ SET default_storage_engine=MyISAM; --source type_inet6_engines.inc
+--echo # +--echo # MDEV-26742 Assertion `field->type_handler() == this' failed in FixedBinTypeBundle<NATIVE_LEN, MAX_CHAR_LEN>::Type_handler_fbt::stored_field_cmp_to_item +--echo # + +CREATE TABLE t1 (c varchar(64), key(c)) engine=myisam; +INSERT INTO t1 VALUES ('::1'),('::2'); +SELECT * FROM t1 WHERE c>CAST('::1' AS INET6); +DROP TABLE t1; + +CREATE TABLE t1 (c varchar(64), key(c)) engine=myisam; +INSERT INTO t1 VALUES ('::1'),('0::1');
also don't see much point in recreating
Joined.
+SELECT * FROM t1 WHERE c=CAST('::1' AS INET6); +EXPLAIN SELECT * FROM t1 WHERE c=CAST('::1' AS INET6); +DROP TABLE t1; +
--echo # --echo # End of 10.5 tests diff --git a/plugin/type_uuid/mysql-test/type_uuid/type_uuid_innodb.test b/plugin/type_uuid/mysql-test/type_uuid/type_uuid_innodb.test index 062e25d366b..27e3214af15 100644 --- a/plugin/type_uuid/mysql-test/type_uuid/type_uuid_innodb.test +++ b/plugin/type_uuid/mysql-test/type_uuid/type_uuid_innodb.test @@ -12,5 +12,27 @@ SET default_storage_engine=InnoDB; --source type_uuid_engines.inc
--echo # ---echo # End of 10.5 tests +--echo # MDEV-26742 Assertion `field->type_handler() == this' failed in FixedBinTypeBundle<NATIVE_LEN, MAX_CHAR_LEN>::Type_handler_fbt::stored_field_cmp_to_item +--echo # + +CREATE TABLE t1 ( pk uuid, c text) engine=myisam; +INSERT INTO t1 VALUES ('00000000-0000-0000-0000-000000000000',1); +CREATE TABLE t2 ( d text, KEY (d)) engine=innodb ; +INSERT INTO t2 VALUES (2); +--error ER_TRUNCATED_WRONG_VALUE +UPDATE t2 JOIN t1 ON ( t1.pk > t2.d) SET t1.c = 1; +SET sql_mode=''; +UPDATE t2 JOIN t1 ON ( t1.pk > t2.d) SET t1.c = 1; +SET sql_mode=DEFAULT; +DROP TABLE t1, t2; + +CREATE TABLE t1 (pk uuid, c text) engine=myisam; +INSERT INTO t1 VALUES ('00000000-0000-0000-0000-000000000000',1); +CREATE TABLE t2 ( d text, KEY (d)) engine=innodb ; +INSERT INTO t2 VALUES (2);
same here
Joined.
+SELECT * FROM t2 JOIN t1 ON ( t1.pk > t2.d); +DROP TABLE t1, t2; + +--echo # +--echo # End of 10.7 tests --echo # diff --git a/sql/field.cc b/sql/field.cc index 46a3a1deea3..fc3456e2ccb 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -7368,7 +7368,9 @@ bool Field_longstr::cmp_to_string_with_same_collation(const Item_bool_func *cond, const Item *item) const { + Type_handler_hybrid_field_type cmp(type_handler_for_comparison());
Can this be anything else but type_handler_long_blob ? if not, then I'd suggest
Type_handler_hybrid_field_type cmp(type_handler_long_blob); DBUG_ASSERT(type_handler_for_comparison() == &type_handler_long_blob);
Generally we don't know. It's now for every single plugin to decide. One can make a plugin using Field_longstr as a base for its Field, but with a different comparison type handler. I wrote it in a generic way which should work for all cases.
+ return !cmp.aggregate_for_comparison(item->type_handler_for_comparison()) && + cmp.type_handler() == type_handler_for_comparison() && - return item->cmp_type() == STRING_RESULT && charset() == cond->compare_collation();
This, basically, means, that cmp_type() is now meaningless, one cannot use it anymore? There's no INET6_RESULT and UUID_RESULT for comparison, so what's the point of cmp_type() now?
cmp_type() is getting gradually replaced to operations on Type_handler pointers and to Type_handler virtual method calls. There are still some less urgent places where Item_result, cmp_type(), result_type() are used. They should be eventually rewritten to use Type_handler, e.g.: - Item_func_field::val_int() - Item_func_benchmark::val_int() - many DBUG_ASSERTs - these can be rewritten using dynamic_cast. I don't know yet if Item_result and cmp_type() will totally disappear at the end, or will be turned into a protected enum inside Type_collection_std. cmp_type() looks quite natural inside these methods: - Type_collection_std::aggregate_for_comparison() - Type_collection_std::aggregate_for_min_max() - Type_collection_std::aggregate_for_num_op()
One more question. What type do we have where you need to aggregate here and simple
item->type_handler_for_comparison() != &type_handler_long_blob
won't do?
We don't know. It's for a plugin to decide if its data type is stronger of weaker than CHAR/VARCHAR/TEXT. For INET6 and UUID we decided they are stronger. But eventually we may want to add some weaker data type.
}
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org