MySQL Bugs: #95233: check constraint doesn't consider IF function that returns boolean a boolean fun

Bug #95233	check constraint doesn't consider IF function that returns boolean a boolean fun
Submitted:	3 May 2019 1:46	Modified:	23 Mar 4:54
Reporter:	Daniel Black	Email Updates:
Status:	Verified	Impact on me:	None
Category:	MySQL Server: DDL	Severity:	S3 (Non-critical)
Version:	8.0.16	OS:	Any
Assigned to:		CPU Architecture:	Any

Description:
An IF() function in a check constraint isn't considered a boolean type.

How to repeat:
create table t1 (source enum('comment','post') NOT NULL, comment_id int unsigned, post_id int unsigned);

alter table t1 add check(IF(source = 'comment', comment_id IS NOT NULL AND post_id IS NULL, post_id IS NOT NULL AND comment_id IS NULL))

error:
An expression of non-boolean type specified to a check constraint 't1_chk_1'.

note:

Can be worked around, just becomes verbose

alter table t1 add check((source = 'comment' AND comment_id IS NOT NULL AND post_id IS NULL) OR (source = 'post' AND post_id IS NOT NULL AND comment_id IS NULL))

Suggested fix:
For IF/(CASE?) functions, evaluate the type of the expression.

Hello Daniel,

Thank you for the report and test case.

Thanks,
Umesh

The main problem here is that MySQL does not implement a boolean type internally.

A simpler workaround may be this:

  alter table t1 add check(IF(source = 'comment', comment_id IS NOT NULL AND
  post_id IS NULL, post_id IS NOT NULL AND comment_id IS NULL) = 1);

which utilizes that a TRUE value is represented as 1 internally.

The lack of boolean wasn't the problem, just the lack of a is_bool_func implementation. Quickly rectified and tested as below:

$ git diff
diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc
index 8c32859a389..c0da3b55069 100644
--- a/sql/item_cmpfunc.cc
+++ b/sql/item_cmpfunc.cc
@@ -3078,6 +3078,10 @@ longlong Item_func_if::val_int() {
   return value;
 }
 
+bool Item_func_if::is_bool_func() const {
+  return args[1]->is_bool_func() && args[2]->is_bool_func();
+}
+
 String *Item_func_if::val_str(String *str) {
   DBUG_ASSERT(fixed == 1);
 
diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h
index 12a98912e04..3afa17b723c 100644
--- a/sql/item_cmpfunc.h
+++ b/sql/item_cmpfunc.h
@@ -1116,6 +1116,7 @@ class Item_func_if final : public Item_func {
   uint decimal_precision() const override;
   const char *func_name() const override { return "if"; }
   enum Functype functype() const override { return IF_FUNC; }
+  bool is_bool_func() const override;
 };
 
 class Item_func_nullif final : public Item_bool_func2 {

----------- MYSQLTEST OUTPUT START -----------

create table t1 (source enum('comment','post') NOT NULL, comment_id int unsigned, post_id int unsigned);
alter table t1 add check(IF(source = 'comment', comment_id IS NOT NULL AND post_id IS NULL, post_id IS NOT NULL AND comment_id IS NULL));
show create table t1;
Table	Create Table
t1	CREATE TABLE `t1` (
  `source` enum('comment','post') NOT NULL,
  `comment_id` int(10) unsigned DEFAULT NULL,
  `post_id` int(10) unsigned DEFAULT NULL,
  CONSTRAINT `t1_chk_1` CHECK (if((`source` = _utf8mb4'comment'),((`comment_id` is not null) and isnull(`post_id`)),((`post_id` is not null) and isnull(`comment_id`))))
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci

------------ MYSQLTEST OUTPUT END -----------

Hi Daniel,

while this will fix your problem, I'm not so sure overall. is_bool_func() is supposed to be a property of the SQL function, regardless of the data types of its arguments. But we will consider the suggestion.

Thank you, please ensure to re-send the patch via "contribution" tab. Otherwise we would not be able to accept it.

regards,
Umesh

Contribution submitted via Github - bug #95233: check constraints now support case/if functions (*) Contribution by Daniel Black (Github grooverdan, mysql-server/pull/263#issuecomment-492470224): I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.

Contribution: git_patch_275645126.txt (text/plain), 10.84 KiB.

Hey Roy,

Now that it's been a while since the original patch, have you reconsidered? As the function is dynamic, its boolean status may as well be dynamic too. It's a pretty low overhead for the check to add this functionality.