MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:marc.alff Date:February 12 2007 8:59pm
Subject:bk commit into 5.0 tree (malff:1.2394) BUG#24532
View as plain text  
Below is the list of changes that have just been committed into a local
5.0 repository of marcsql. When marcsql does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2007-02-12 13:59:29-07:00, malff@weblab.(none) +7 -0
  Bug#24532 (The return data type of IS TRUE is different from similar
    operations)
  
  Before this change, the boolean predicates:
  - X IS TRUE,
  - X IS NOT TRUE,
  - X IS FALSE,
  - X IS NOT FALSE
  were implemented by expanding the Item tree in the parser, by using a
  construct like:
  Item_func_if(Item_func_ifnull(X, <value>), <value>, <value>)
  
  Each <value> was a constant integer, either 0 or 1.
  
  A bug in the implementation of the function IF(a, b, c), in
  Item_func_if::fix_length_and_dec(), would cause the following :
  
  When the arguments b and c are both unsigned, the result type of the
  function was signed, instead of unsigned.
  
  When the result of the if function is signed, space for the sign could be
  counted twice (in the max() expression for a signed argument, and in the
  total), causing the member max_length to be too high.
  
  An effect of this is that the final type of IF(x, int(1), int(1)) would be
  int(2) instead of int(1).
  
  With this fix, the problems found in Item_func_if::fix_length_and_dec()
  have been fixed.
  
  While it's semantically correct to represent 'X IS TRUE' with
  Item_func_if(Item_func_ifnull(X, <value>), <value>, <value>),
  there are however more problems with this construct.
  
  a)
  Building the parse tree involves :
  - creating 5 Item instances (3 ints, 1 ifnull, 1 if),
  - creating each Item calls my_pthread_getspecific_ptr() once in the operator
    new(size), and a second time in the Item::Item() constructor, resulting
    in a total of 10 calls to get the current thread.
  Evaluating the expression involves evaluating up to 4 nodes at runtime.
  This representation could be greatly simplified and improved.
  
  b)
  Transforming the parse tree internally with if(ifnull(...)) is fine as long
  as this transformation is internal to the server implementation.
  With views however, the result of the parse tree is later exposed by the
  ::print() functions, and stored as part of the view definition.
  Doing this has long term consequences:
  
  1)
  The original semantic 'X IS TRUE' is lost, and replaced by the
  if(ifnull(...)) expression. As a result, SHOW CREATE VIEW does not restore
  the original code.
  
  2)
  Should a future version of MySQL implement the SQL BOOLEAN data type for
  example, views created today using 'X IS NULL' can be exported using
  mysqldump, and imported again. Such views would be converted correctly and
  automatically to use a BOOLEAN column in the future version.
  With 'X IS TRUE' and the current implementations, views using these
  "boolean" predicates would not be converted during the export/import, and
  would use integer columns instead.
  The difference traces back to how SHOW CREATE VIEW preserves 'X IS NULL' but
  does not preserve the 'X IS TRUE' semantic.
  
  With this fix, internal representation of 'X IS TRUE' booleans predicates
  has changed, so that:
  - dedicated Item classes are created for each predicate,
  - only 1 Item is created to represent 1 predicate
  - my_pthread_getspecific_ptr() is invoked 1 time instead of 10
  - SHOW CREATE VIEW preserves the original semantic, and prints 'X IS TRUE'.
  
  Note that, because of the fix in Item_func_if, views created before this fix
  will:
  - correctly use a int(1) type instead of int(2) for boolean predicates,
  - incorrectly print the if(ifnull(...), ...) expression in SHOW CREATE VIEW,
  since the original semantic (X IS TRUE) has been lost.
  - except for the syntax used in SHOW CREATE VIEW, these views will operate
  properly, no action is needed.
  
  Views created after this fix will operate correctly, and will preserve the
  original code semantic in SHOW CREATE VIEW.

  mysql-test/r/func_if.result@stripped, 2007-02-12 13:59:26-07:00, malff@weblab.(none) +3 -0
    IF(x, unsigned, unsigned) should be unsigned.

  mysql-test/r/view.result@stripped, 2007-02-12 13:59:26-07:00, malff@weblab.(none) +125 -0
    Preserve the semantic of 'X IS [NOT] (TRUE|FALSE)' boolean predicates.

  mysql-test/t/func_if.test@stripped, 2007-02-12 13:59:26-07:00, malff@weblab.(none) +11 -0
    IF(x, unsigned, unsigned) should be unsigned.

  mysql-test/t/view.test@stripped, 2007-02-12 13:59:26-07:00, malff@weblab.(none) +86 -0
    Preserve the semantic of 'X IS [NOT] (TRUE|FALSE)' boolean predicates.

  sql/item_cmpfunc.cc@stripped, 2007-02-12 13:59:26-07:00, malff@weblab.(none) +68 -6
    Preserve the semantic of 'X IS [NOT] (TRUE|FALSE)' boolean predicates.
    IF(x, unsigned, unsigned) should be unsigned.

  sql/item_cmpfunc.h@stripped, 2007-02-12 13:59:26-07:00, malff@weblab.(none) +86 -0
    Preserve the semantic of 'X IS [NOT] (TRUE|FALSE)' boolean predicates.

  sql/sql_yacc.yy@stripped, 2007-02-12 13:59:26-07:00, malff@weblab.(none) +12 -16
    Preserve the semantic of 'X IS [NOT] (TRUE|FALSE)' boolean predicates.

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	malff
# Host:	weblab.(none)
# Root:	/home/marcsql/TREE/mysql-5.0-24532

--- 1.228/sql/item_cmpfunc.cc	2007-02-12 13:59:34 -07:00
+++ 1.229/sql/item_cmpfunc.cc	2007-02-12 13:59:34 -07:00
@@ -721,6 +721,59 @@ int Arg_comparator::compare_e_row()
 }
 
 
+void Item_func_truth::fix_length_and_dec()
+{
+  maybe_null= 0;
+  null_value= 0;
+  decimals= 0;
+  max_length= 1;
+}
+
+
+void Item_func_truth::print(String *str)
+{
+  str->append('(');
+  args[0]->print(str);
+  str->append(STRING_WITH_LEN(" is "));
+  if (! affirmative)
+    str->append(STRING_WITH_LEN("not "));
+  if (value)
+    str->append(STRING_WITH_LEN("true"));
+  else
+    str->append(STRING_WITH_LEN("false"));
+  str->append(')');
+}
+
+
+bool Item_func_truth::val_bool()
+{
+  bool val= args[0]->val_bool();
+  if (args[0]->null_value)
+  {
+    /*
+      NULL val IS {TRUE, FALSE} --> FALSE
+      NULL val IS NOT {TRUE, FALSE} --> TRUE
+    */
+    return (! affirmative);
+  }
+
+  if (affirmative)
+  {
+    /* {TRUE, FALSE} val IS {TRUE, FALSE} value */
+    return (val == value);
+  }
+
+  /* {TRUE, FALSE} val IS NOT {TRUE, FALSE} value */
+  return (val != value);
+}
+
+
+longlong Item_func_truth::val_int()
+{
+  return (val_bool() ? 1 : 0);
+}
+
+
 bool Item_in_optimizer::fix_left(THD *thd, Item **ref)
 {
   if (!args[0]->fixed && args[0]->fix_fields(thd, args) ||
@@ -1432,6 +1485,7 @@ Item_func_if::fix_length_and_dec()
 {
   maybe_null=args[1]->maybe_null || args[2]->maybe_null;
   decimals= max(args[1]->decimals, args[2]->decimals);
+  unsigned_flag=args[1]->unsigned_flag && args[2]->unsigned_flag;
 
   enum Item_result arg1_type=args[1]->result_type();
   enum Item_result arg2_type=args[2]->result_type();
@@ -1461,12 +1515,20 @@ Item_func_if::fix_length_and_dec()
       collation.set(&my_charset_bin);	// Number
     }
   }
-  max_length=
-    (cached_result_type == DECIMAL_RESULT || cached_result_type == INT_RESULT) ?
-    (max(args[1]->max_length - args[1]->decimals,
-         args[2]->max_length - args[2]->decimals) + decimals +
-         (unsigned_flag ? 0 : 1) ) :
-    max(args[1]->max_length, args[2]->max_length);
+
+  if ((cached_result_type == DECIMAL_RESULT )
+      || (cached_result_type == INT_RESULT))
+  {
+    int len1= args[1]->max_length - args[1]->decimals
+      - (args[1]->unsigned_flag ? 0 : 1);
+
+    int len2= args[2]->max_length - args[2]->decimals
+      - (args[2]->unsigned_flag ? 0 : 1);
+
+    max_length=max(len1, len2) + decimals + (unsigned_flag ? 0 : 1);
+  }
+  else
+    max_length= max(args[1]->max_length, args[2]->max_length);
 }
 
 

--- 1.136/sql/item_cmpfunc.h	2007-02-12 13:59:34 -07:00
+++ 1.137/sql/item_cmpfunc.h	2007-02-12 13:59:34 -07:00
@@ -98,6 +98,92 @@ public:
   uint decimal_precision() const { return 1; }
 };
 
+
+/**
+  Abstract Item class, to represent <code>X IS [NOT] (TRUE | FALSE)</code>
+  boolean predicates.
+*/
+
+class Item_func_truth : public Item_bool_func
+{
+public:
+  virtual bool val_bool();
+  virtual longlong val_int();
+  virtual void fix_length_and_dec();
+  virtual void print(String *str);
+
+protected:
+  Item_func_truth(Item *a, bool a_value, bool a_affirmative)
+  : Item_bool_func(a), value(a_value), affirmative(a_affirmative)
+  {}
+
+  ~Item_func_truth()
+  {}
+private:
+  /**
+    True for <code>X IS [NOT] TRUE</code>,
+    false for <code>X IS [NOT] FALSE</code> predicates.
+  */
+  const bool value;
+  /**
+    True for <code>X IS Y</code>, false for <code>X IS NOT Y</code> predicates.
+  */
+  const bool affirmative;
+};
+
+
+/**
+  This Item represents a <code>X IS TRUE</code> boolean predicate.
+*/
+
+class Item_func_istrue : public Item_func_truth
+{
+public:
+  Item_func_istrue(Item *a) : Item_func_truth(a, true, true) {}
+  ~Item_func_istrue() {}
+  virtual const char* func_name() const { return "istrue"; }
+};
+
+
+/**
+  This Item represents a <code>X IS NOT TRUE</code> boolean predicate.
+*/
+
+class Item_func_isnottrue : public Item_func_truth
+{
+public:
+  Item_func_isnottrue(Item *a) : Item_func_truth(a, true, false) {}
+  ~Item_func_isnottrue() {}
+  virtual const char* func_name() const { return "isnottrue"; }
+};
+
+
+/**
+  This Item represents a <code>X IS FALSE</code> boolean predicate.
+*/
+
+class Item_func_isfalse : public Item_func_truth
+{
+public:
+  Item_func_isfalse(Item *a) : Item_func_truth(a, false, true) {}
+  ~Item_func_isfalse() {}
+  virtual const char* func_name() const { return "isfalse"; }
+};
+
+
+/**
+  This Item represents a <code>X IS NOT FALSE</code> boolean predicate.
+*/
+
+class Item_func_isnotfalse : public Item_func_truth
+{
+public:
+  Item_func_isnotfalse(Item *a) : Item_func_truth(a, false, false) {}
+  ~Item_func_isnotfalse() {}
+  virtual const char* func_name() const { return "isnotfalse"; }
+};
+
+
 class Item_cache;
 #define UNKNOWN ((my_bool)-1)
 

--- 1.504/sql/sql_yacc.yy	2007-02-12 13:59:34 -07:00
+++ 1.505/sql/sql_yacc.yy	2007-02-12 13:59:34 -07:00
@@ -58,15 +58,6 @@ const LEX_STRING null_lex_str={0,0};
     YYABORT;				\
   }
 
-/* Helper for parsing "IS [NOT] truth_value" */
-inline Item *is_truth_value(Item *A, bool v1, bool v2)
-{
-  return new Item_func_if(create_func_ifnull(A,
-	new Item_int((char *) (v2 ? "TRUE" : "FALSE"), v2, 1)),
-	new Item_int((char *) (v1 ? "TRUE" : "FALSE"), v1, 1),
-	new Item_int((char *) (v1 ? "FALSE" : "TRUE"),!v1, 1));
-}
-
 #ifndef DBUG_OFF
 #define YYDEBUG 1
 #else
@@ -4457,13 +4448,18 @@ bool_factor:
 	| bool_test ;
 
 bool_test:
-	bool_pri IS TRUE_SYM	{ $$= is_truth_value($1,1,0); }
-	| bool_pri IS not TRUE_SYM { $$= is_truth_value($1,0,0); }
-	| bool_pri IS FALSE_SYM	{ $$= is_truth_value($1,0,1); }
-	| bool_pri IS not FALSE_SYM { $$= is_truth_value($1,1,1); }
-	| bool_pri IS UNKNOWN_SYM { $$= new Item_func_isnull($1); }
-	| bool_pri IS not UNKNOWN_SYM { $$= new Item_func_isnotnull($1); }
-	| bool_pri ;
+          bool_pri IS TRUE_SYM
+          { $$= new (YYTHD->mem_root) Item_func_istrue($1); }
+        | bool_pri IS not TRUE_SYM
+          { $$= new (YYTHD->mem_root) Item_func_isnottrue($1); }
+        | bool_pri IS FALSE_SYM
+          { $$= new (YYTHD->mem_root) Item_func_isfalse($1); }
+        | bool_pri IS not FALSE_SYM
+          { $$= new (YYTHD->mem_root) Item_func_isnotfalse($1); }
+        | bool_pri IS UNKNOWN_SYM { $$= new Item_func_isnull($1); }
+        | bool_pri IS not UNKNOWN_SYM { $$= new Item_func_isnotnull($1); }
+        | bool_pri
+        ;
 
 bool_pri:
 	bool_pri IS NULL_SYM	{ $$= new Item_func_isnull($1); }

--- 1.187/mysql-test/r/view.result	2007-02-12 13:59:34 -07:00
+++ 1.188/mysql-test/r/view.result	2007-02-12 13:59:34 -07:00
@@ -3033,4 +3033,129 @@ ERROR HY000: View's SELECT contains a su
 SELECT * FROM (SELECT 1) AS t;
 1
 1
+drop view if exists view_24532_a;
+drop view if exists view_24532_b;
+drop table if exists table_24532;
+create table table_24532 (
+a int,
+b bigint,
+c int(4),
+d bigint(48)
+);
+create view view_24532_a as
+select
+a IS TRUE,
+a IS NOT TRUE,
+a IS FALSE,
+a IS NOT FALSE,
+a IS UNKNOWN,
+a IS NOT UNKNOWN,
+a is NULL,
+a IS NOT NULL,
+ISNULL(a),
+b IS TRUE,
+b IS NOT TRUE,
+b IS FALSE,
+b IS NOT FALSE,
+b IS UNKNOWN,
+b IS NOT UNKNOWN,
+b is NULL,
+b IS NOT NULL,
+ISNULL(b),
+c IS TRUE,
+c IS NOT TRUE,
+c IS FALSE,
+c IS NOT FALSE,
+c IS UNKNOWN,
+c IS NOT UNKNOWN,
+c is NULL,
+c IS NOT NULL,
+ISNULL(c),
+d IS TRUE,
+d IS NOT TRUE,
+d IS FALSE,
+d IS NOT FALSE,
+d IS UNKNOWN,
+d IS NOT UNKNOWN,
+d is NULL,
+d IS NOT NULL,
+ISNULL(d)
+from table_24532;
+describe view_24532_a;
+Field	Type	Null	Key	Default	Extra
+a IS TRUE	int(1)	NO		0	
+a IS NOT TRUE	int(1)	NO		0	
+a IS FALSE	int(1)	NO		0	
+a IS NOT FALSE	int(1)	NO		0	
+a IS UNKNOWN	int(1)	NO		0	
+a IS NOT UNKNOWN	int(1)	NO		0	
+a is NULL	int(1)	NO		0	
+a IS NOT NULL	int(1)	NO		0	
+ISNULL(a)	int(1)	NO		0	
+b IS TRUE	int(1)	NO		0	
+b IS NOT TRUE	int(1)	NO		0	
+b IS FALSE	int(1)	NO		0	
+b IS NOT FALSE	int(1)	NO		0	
+b IS UNKNOWN	int(1)	NO		0	
+b IS NOT UNKNOWN	int(1)	NO		0	
+b is NULL	int(1)	NO		0	
+b IS NOT NULL	int(1)	NO		0	
+ISNULL(b)	int(1)	NO		0	
+c IS TRUE	int(1)	NO		0	
+c IS NOT TRUE	int(1)	NO		0	
+c IS FALSE	int(1)	NO		0	
+c IS NOT FALSE	int(1)	NO		0	
+c IS UNKNOWN	int(1)	NO		0	
+c IS NOT UNKNOWN	int(1)	NO		0	
+c is NULL	int(1)	NO		0	
+c IS NOT NULL	int(1)	NO		0	
+ISNULL(c)	int(1)	NO		0	
+d IS TRUE	int(1)	NO		0	
+d IS NOT TRUE	int(1)	NO		0	
+d IS FALSE	int(1)	NO		0	
+d IS NOT FALSE	int(1)	NO		0	
+d IS UNKNOWN	int(1)	NO		0	
+d IS NOT UNKNOWN	int(1)	NO		0	
+d is NULL	int(1)	NO		0	
+d IS NOT NULL	int(1)	NO		0	
+ISNULL(d)	int(1)	NO		0	
+create view view_24532_b as
+select
+a IS TRUE,
+if(ifnull(a, 0), 1, 0) as old_istrue,
+a IS NOT TRUE,
+if(ifnull(a, 0), 0, 1) as old_isnottrue,
+a IS FALSE,
+if(ifnull(a, 1), 0, 1) as old_isfalse,
+a IS NOT FALSE,
+if(ifnull(a, 1), 1, 0) as old_isnotfalse
+from table_24532;
+describe view_24532_b;
+Field	Type	Null	Key	Default	Extra
+a IS TRUE	int(1)	NO		0	
+old_istrue	int(1)	NO		0	
+a IS NOT TRUE	int(1)	NO		0	
+old_isnottrue	int(1)	NO		0	
+a IS FALSE	int(1)	NO		0	
+old_isfalse	int(1)	NO		0	
+a IS NOT FALSE	int(1)	NO		0	
+old_isnotfalse	int(1)	NO		0	
+show create view view_24532_b;
+View	Create View
+view_24532_b	CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `view_24532_b` AS select (`table_24532`.`a` is true) AS `a IS TRUE`,if(ifnull(`table_24532`.`a`,0),1,0) AS `old_istrue`,(`table_24532`.`a` is not true) AS `a IS NOT TRUE`,if(ifnull(`table_24532`.`a`,0),0,1) AS `old_isnottrue`,(`table_24532`.`a` is false) AS `a IS FALSE`,if(ifnull(`table_24532`.`a`,1),0,1) AS `old_isfalse`,(`table_24532`.`a` is not false) AS `a IS NOT FALSE`,if(ifnull(`table_24532`.`a`,1),1,0) AS `old_isnotfalse` from `table_24532`
+insert into table_24532 values (0, 0, 0, 0);
+select * from view_24532_b;
+a IS TRUE	old_istrue	a IS NOT TRUE	old_isnottrue	a IS FALSE	old_isfalse	a IS NOT FALSE	old_isnotfalse
+0	0	1	1	1	1	0	0
+update table_24532 set a=1;
+select * from view_24532_b;
+a IS TRUE	old_istrue	a IS NOT TRUE	old_isnottrue	a IS FALSE	old_isfalse	a IS NOT FALSE	old_isnotfalse
+1	1	0	0	0	0	1	1
+update table_24532 set a=NULL;
+select * from view_24532_b;
+a IS TRUE	old_istrue	a IS NOT TRUE	old_isnottrue	a IS FALSE	old_isfalse	a IS NOT FALSE	old_isnotfalse
+0	0	1	1	0	0	1	1
+drop view view_24532_a;
+drop view view_24532_b;
+drop table table_24532;
 End of 5.0 tests.

--- 1.172/mysql-test/t/view.test	2007-02-12 13:59:34 -07:00
+++ 1.173/mysql-test/t/view.test	2007-02-12 13:59:34 -07:00
@@ -2992,5 +2992,91 @@ eval CREATE VIEW v1 AS $query;
 --echo # Previously the following would fail.
 eval $query;
 
+#
+# Bug#24532: The return data type of IS TRUE is different from similar
+# operations
+#
+
+--disable_warnings
+drop view if exists view_24532_a;
+drop view if exists view_24532_b;
+drop table if exists table_24532;
+--enable_warnings
+
+create table table_24532 (
+  a int,
+  b bigint,
+  c int(4),
+  d bigint(48)
+);
+
+create view view_24532_a as
+select
+  a IS TRUE,
+  a IS NOT TRUE,
+  a IS FALSE,
+  a IS NOT FALSE,
+  a IS UNKNOWN,
+  a IS NOT UNKNOWN,
+  a is NULL,
+  a IS NOT NULL,
+  ISNULL(a),
+  b IS TRUE,
+  b IS NOT TRUE,
+  b IS FALSE,
+  b IS NOT FALSE,
+  b IS UNKNOWN,
+  b IS NOT UNKNOWN,
+  b is NULL,
+  b IS NOT NULL,
+  ISNULL(b),
+  c IS TRUE,
+  c IS NOT TRUE,
+  c IS FALSE,
+  c IS NOT FALSE,
+  c IS UNKNOWN,
+  c IS NOT UNKNOWN,
+  c is NULL,
+  c IS NOT NULL,
+  ISNULL(c),
+  d IS TRUE,
+  d IS NOT TRUE,
+  d IS FALSE,
+  d IS NOT FALSE,
+  d IS UNKNOWN,
+  d IS NOT UNKNOWN,
+  d is NULL,
+  d IS NOT NULL,
+  ISNULL(d)
+from table_24532;
+
+describe view_24532_a;
+
+create view view_24532_b as
+select
+  a IS TRUE,
+  if(ifnull(a, 0), 1, 0) as old_istrue,
+  a IS NOT TRUE,
+  if(ifnull(a, 0), 0, 1) as old_isnottrue,
+  a IS FALSE,
+  if(ifnull(a, 1), 0, 1) as old_isfalse,
+  a IS NOT FALSE,
+  if(ifnull(a, 1), 1, 0) as old_isnotfalse
+from table_24532;
+
+describe view_24532_b;
+
+show create view view_24532_b;
+
+insert into table_24532 values (0, 0, 0, 0);
+select * from view_24532_b;
+update table_24532 set a=1;
+select * from view_24532_b;
+update table_24532 set a=NULL;
+select * from view_24532_b;
+
+drop view view_24532_a;
+drop view view_24532_b;
+drop table table_24532;
 
 --echo End of 5.0 tests.

--- 1.23/mysql-test/r/func_if.result	2007-02-12 13:59:34 -07:00
+++ 1.24/mysql-test/r/func_if.result	2007-02-12 13:59:34 -07:00
@@ -128,3 +128,6 @@ f1	f2	if(f1, 40.0, 5.00)
 0	0	5.00
 1	1	40.00
 drop table t1;
+select if(0, 18446744073709551610, 18446744073709551610);
+if(0, 18446744073709551610, 18446744073709551610)
+18446744073709551610

--- 1.21/mysql-test/t/func_if.test	2007-02-12 13:59:34 -07:00
+++ 1.22/mysql-test/t/func_if.test	2007-02-12 13:59:34 -07:00
@@ -97,3 +97,14 @@ create table t1 (f1 int, f2 int);
 insert into t1 values(1,1),(0,0);
 select f1, f2, if(f1, 40.0, 5.00) from t1 group by f1 order by f2;
 drop table t1;
+
+#
+# Bug#24532 (The return data type of IS TRUE is different from similar
+# operations)
+#
+# IF(x, unsigned, unsigned) should be unsigned.
+#
+
+select if(0, 18446744073709551610, 18446744073709551610);
+
+
Thread
bk commit into 5.0 tree (malff:1.2394) BUG#24532marc.alff12 Feb