Skip to content

Commit

Permalink
MDEV-33827 UUID() returns a NULL-able result
Browse files Browse the repository at this point in the history
It was wrong to derive Item_func_uuid from Item_func_sys_guid,
because the former is a function returning the UUID data type,
while the latter is a string function returning VARCHAR.

As a result of the wrong hierarchy, Item_func_uuid erroneously derived
Item_str_func::fix_fields(), which contains this code:

  /*
    In Item_str_func::check_well_formed_result() we may set null_value
    flag on the same condition as in test() below.
  */
  if (thd->is_strict_mode())
    set_maybe_null();

This code is not relevant to UUID() at all.
A simple fix would be to set_maybe_null(false) in
Item_func_uuid::fix_length_and_dec(). However,
it'd fix only exactly this single consequence of the wrong
class hierarchy, and similar bugs could appear again in
the future. Moreover, we're going to add functions UUIDv4()
and UUIDv7() soon (in 11.6). So it's better to fix the class hierarchy
in the right way before adding these new functions.

Fix:

- Adding a new abstract class Item_fbt_func in the template
  in sql_type_fixedbin.h
- Deriving Item_typecast_fbt from Item_fbt_func
- Deriving Item_func_uuid from Item_fbt_func
- Adding a new helper class UUIDv1. It derives from UUID, and additionally
  initializes the value to "UUID version 1" right in the constructor.
  Note, the new coming soon SQL functions UUIDv4() and UUIDv7()
  will also have corresponding classes UUIDv4 and UUIDv7.

So now UUID() is a pure "returning UUID" function,
like CAST(expr AS UUID) used to be, without any unintentional
artifacts of functions returning VARCHAR/TEXT.

Cleanup:
- Removing the member Item_func_sys_guid::with_dashes,
  as it's not needed any more:
  * Item_func_sys_guid now does not have any descendants any more
  * Item_func_sys_guid::val_str() itself always displays without dashes
  • Loading branch information
abarkov committed Apr 5, 2024
1 parent 0c0db46 commit 9b02b7c
Show file tree
Hide file tree
Showing 8 changed files with 109 additions and 63 deletions.
4 changes: 2 additions & 2 deletions mysql-test/main/func_misc.result
Expand Up @@ -118,8 +118,8 @@ create table t1 as select uuid(), length(uuid());
show create table t1;
Table Create Table
t1 CREATE TABLE `t1` (
`uuid()` uuid DEFAULT NULL,
`length(uuid())` int(10) DEFAULT NULL
`uuid()` uuid NOT NULL,
`length(uuid())` int(10) NOT NULL
) ENGINE=MyISAM DEFAULT CHARSET=latin1 COLLATE=latin1_swedish_ci
select length(`uuid()`) from t1;
length(`uuid()`)
Expand Down
2 changes: 1 addition & 1 deletion mysql-test/suite/vcol/r/binlog.result
Expand Up @@ -88,7 +88,7 @@ CREATE TEMPORARY TABLE t1 SELECT UUID();
show create table t1;
Table Create Table
t1 CREATE TEMPORARY TABLE `t1` (
`UUID()` uuid DEFAULT NULL
`UUID()` uuid NOT NULL
) ENGINE=MyISAM DEFAULT CHARSET=latin1 COLLATE=latin1_swedish_ci
CREATE TABLE t2 (a INT PRIMARY KEY, b TEXT, c INT GENERATED ALWAYS AS(b));
INSERT INTO t2 (a,b) VALUES (1,1);
Expand Down
17 changes: 1 addition & 16 deletions plugin/type_uuid/item_uuidfunc.cc
Expand Up @@ -16,7 +16,6 @@
#define MYSQL_SERVER
#include "mariadb.h"
#include "item_uuidfunc.h"
#include "sql_type_uuid.h"

String *Item_func_sys_guid::val_str(String *str)
{
Expand All @@ -27,20 +26,6 @@ String *Item_func_sys_guid::val_str(String *str)

uchar buf[MY_UUID_SIZE];
my_uuid(buf);
my_uuid2str(buf, const_cast<char*>(str->ptr()), with_dashes);
my_uuid2str(buf, const_cast<char*>(str->ptr()), 0);
return str;
}

const Type_handler *Item_func_uuid::type_handler() const
{
return Type_handler_uuid_new::singleton();
}

bool Item_func_uuid::val_native(THD *, Native *to)
{
DBUG_ASSERT(fixed());
to->alloc(MY_UUID_SIZE);
to->length(MY_UUID_SIZE);
my_uuid((uchar*)to->ptr());
return 0;
}
30 changes: 22 additions & 8 deletions plugin/type_uuid/item_uuidfunc.h
Expand Up @@ -18,15 +18,15 @@


#include "item.h"
#include "sql_type_uuid.h"

class Item_func_sys_guid: public Item_str_func
{
protected:
bool with_dashes;
size_t uuid_len() const
{ return MY_UUID_BARE_STRING_LENGTH + with_dashes*MY_UUID_SEPARATORS; }
static size_t uuid_len()
{ return MY_UUID_BARE_STRING_LENGTH; }
public:
Item_func_sys_guid(THD *thd): Item_str_func(thd), with_dashes(false) {}
Item_func_sys_guid(THD *thd): Item_str_func(thd) {}
bool fix_length_and_dec(THD *thd) override
{
collation.set(DTCollation_numeric());
Expand All @@ -49,17 +49,31 @@ class Item_func_sys_guid: public Item_str_func
{ return get_item_copy<Item_func_sys_guid>(thd, this); }
};

class Item_func_uuid: public Item_func_sys_guid
class Item_func_uuid: public Type_handler_uuid_new::Item_fbt_func
{
public:
Item_func_uuid(THD *thd): Item_func_sys_guid(thd) { with_dashes= true; }
const Type_handler *type_handler() const override;
Item_func_uuid(THD *thd): Item_fbt_func(thd) { }
bool const_item() const override { return false; }
table_map used_tables() const override { return RAND_TABLE_BIT; }
bool check_vcol_func_processor(void *arg) override
{
return mark_unsupported_function(func_name(), "()", arg, VCOL_NON_DETERMINISTIC);
}
LEX_CSTRING func_name_cstring() const override
{
static LEX_CSTRING name= {STRING_WITH_LEN("uuid") };
return name;
}
bool val_native(THD *thd, Native *to) override;
String *val_str(String *str) override
{
DBUG_ASSERT(fixed());
return UUIDv1().to_string(str) ? NULL : str;
}
bool val_native(THD *thd, Native *to) override
{
DBUG_ASSERT(fixed());
return UUIDv1::construct_native(to);
}
Item *get_copy(THD *thd) override
{ return get_item_copy<Item_func_uuid>(thd, this); }
};
Expand Down
10 changes: 10 additions & 0 deletions plugin/type_uuid/mysql-test/type_uuid/func_uuid.result
@@ -0,0 +1,10 @@
#
# MDEV-33827 UUID() returns a NULL-able result
#
CREATE TABLE t1 AS SELECT UUID();
SHOW CREATE TABLE t1;
Table Create Table
t1 CREATE TABLE `t1` (
`UUID()` uuid NOT NULL
) ENGINE=MyISAM DEFAULT CHARSET=latin1 COLLATE=latin1_swedish_ci
DROP TABLE t1;
7 changes: 7 additions & 0 deletions plugin/type_uuid/mysql-test/type_uuid/func_uuid.test
@@ -0,0 +1,7 @@
--echo #
--echo # MDEV-33827 UUID() returns a NULL-able result
--echo #

CREATE TABLE t1 AS SELECT UUID();
SHOW CREATE TABLE t1;
DROP TABLE t1;
18 changes: 18 additions & 0 deletions plugin/type_uuid/sql_type_uuid.h
Expand Up @@ -327,4 +327,22 @@ class Type_collection_uuid: public Type_collection
typedef Type_handler_fbt<UUID<1>, Type_collection_uuid> Type_handler_uuid_old;
typedef Type_handler_fbt<UUID<0>, Type_collection_uuid> Type_handler_uuid_new;


class UUIDv1: public Type_handler_uuid_new::Fbt
{
public:
UUIDv1()
{
my_uuid((uchar*) m_buffer);
}
static bool construct_native(Native *to)
{
to->alloc(MY_UUID_SIZE);
to->length(MY_UUID_SIZE);
my_uuid((uchar*)to->ptr());
return 0;
}
};


#endif // SQL_TYPE_UUID_INCLUDED
84 changes: 48 additions & 36 deletions sql/sql_type_fixedbin.h
Expand Up @@ -896,26 +896,60 @@ class Type_handler_fbt: public Type_handler
}
};

class Item_typecast_fbt: public Item_func
class Item_fbt_func: public Item_func
{
public:
Item_typecast_fbt(THD *thd, Item *a) :Item_func(thd, a) {}

using Item_func::Item_func;
const Type_handler *type_handler() const override
{ return singleton(); }
bool fix_length_and_dec(THD *thd) override
{
Type_std_attributes::operator=(Type_std_attributes_fbt());
return false;
}
String *val_str(String *to) override
{
Fbt_null tmp(args[0]);
return (null_value= tmp.is_null() || tmp.to_string(to)) ? NULL : to;
}
longlong val_int() override
{
return 0;
}
double val_real() override
{
return 0;
}
my_decimal *val_decimal(my_decimal *to) override
{
my_decimal_set_zero(to);
return to;
}
bool get_date(THD *thd, MYSQL_TIME *ltime, date_mode_t fuzzydate) override
{
set_zero_time(ltime, MYSQL_TIMESTAMP_TIME);
return false;
}
};

class Item_typecast_fbt: public Item_fbt_func
{
public:
Item_typecast_fbt(THD *thd, Item *a) :Item_fbt_func(thd, a) {}

enum Functype functype() const override { return CHAR_TYPECAST_FUNC; }
Item_func::Functype functype() const override
{ return Item_func::CHAR_TYPECAST_FUNC; }
bool eq(const Item *item, bool binary_cmp) const override
{
if (this == item)
return true;
if (item->type() != FUNC_ITEM ||
if (item->type() != Item_fbt_func::FUNC_ITEM ||
functype() != ((Item_func*)item)->functype())
return false;
if (type_handler() != item->type_handler())
if (Item_fbt_func::type_handler() != item->type_handler())
return false;
Item_typecast_fbt *cast= (Item_typecast_fbt*) item;
return args[0]->eq(cast->args[0], binary_cmp);
return Item_fbt_func::args[0]->eq(cast->args[0], binary_cmp);
}
LEX_CSTRING func_name_cstring() const override
{
Expand All @@ -928,45 +962,23 @@ class Type_handler_fbt: public Type_handler
void print(String *str, enum_query_type query_type) override
{
str->append(STRING_WITH_LEN("cast("));
args[0]->print(str, query_type);
Item_fbt_func::args[0]->print(str, query_type);
str->append(STRING_WITH_LEN(" as "));
str->append(singleton()->name().lex_cstring());
str->append(')');
}
bool fix_length_and_dec(THD *thd) override
{
Type_std_attributes::operator=(Type_std_attributes_fbt());
if (Fbt::fix_fields_maybe_null_on_conversion_to_fbt(args[0]))
set_maybe_null();
return false;
}
String *val_str(String *to) override
{
Fbt_null tmp(args[0]);
return (null_value= tmp.is_null() || tmp.to_string(to)) ? NULL : to;
}
longlong val_int() override
{
return 0;
}
double val_real() override
{
return 0;
}
my_decimal *val_decimal(my_decimal *to) override
{
my_decimal_set_zero(to);
return to;
}
bool get_date(THD *thd, MYSQL_TIME *ltime, date_mode_t fuzzydate) override
{
set_zero_time(ltime, MYSQL_TIMESTAMP_TIME);
Item_fbt_func::fix_length_and_dec(thd);
if (Fbt::fix_fields_maybe_null_on_conversion_to_fbt(
Item_fbt_func::args[0]))
Item_fbt_func::set_maybe_null();
return false;
}
bool val_native(THD *thd, Native *to) override
{
Fbt_null tmp(args[0]);
return null_value= tmp.is_null() || tmp.to_native(to);
Fbt_null tmp(Item_fbt_func::args[0]);
return Item_fbt_func::null_value= tmp.is_null() || tmp.to_native(to);
}
Item *get_copy(THD *thd) override
{ return get_item_copy<Item_typecast_fbt>(thd, this); }
Expand Down

0 comments on commit 9b02b7c

Please sign in to comment.