List:Commits« Previous MessageNext Message »
From:Tor Didriksen Date:February 8 2011 11:53am
Subject:Re: bzr commit into mysql-trunk branch (jorgen.loland:3605) Bug#59793
View as plain text  
> === modified file 'mysql-test/t/negation_elimination.test'
> --- mysql-test/t/negation_elimination.test    2005-07-28 00:22:47 +0000
> +++ mysql-test/t/negation_elimination.test    2011-02-07 15:27:02 +0000
> @@ -65,10 +65,40 @@
>  explain select * from t1 where ((a between 5 and 15) and (not(a like
10)));
>  select * from t1 where ((a between 5 and 15) and (not(a like 10)));
>
> +--echo
> +--echo # XOR [ NOT (a XOR b) == (NOT a) XOR b == a XOR (NOT b) ]
> +

That comment was hard to parse!!!!


> +--echo # Should return 6,7
> +SELECT * FROM t1 WHERE ((a > 5) XOR (a > 7));
> +
> +--echo # Should return 0..5,8..19
> +SELECT * FROM t1 WHERE ((NOT (a > 5)) XOR (a > 7));
> +SELECT * FROM t1 WHERE ((a > 5) XOR (NOT (a > 7)));
> +SELECT * FROM t1 WHERE NOT ((a > 5) XOR (a > 7));
> +
> +--echo # Should return 6,7
> +SELECT * FROM t1 WHERE NOT ((NOT (a > 5)) XOR (a > 7));
> +SELECT * FROM t1 WHERE NOT ((a > 5) XOR (NOT (a > 7)));
> +
> +--echo # Should return 0..5,8..19
> +SELECT * FROM t1 WHERE NOT (NOT (a > 5) XOR (NOT (a > 7)));
> +
> +SELECT * FROM t1 WHERE (NULL XOR (a > 7));
> +SELECT * FROM t1 WHERE NOT (NULL XOR (a > 7));
> +
> +--echo # Should be "...WHERE (a XOR a)
> +explain extended SELECT * FROM t1 WHERE NOT ((NOT a) XOR (a));
> +

Maybe the comment should say
Should be simplified to ...

EXPLAIN EXTENDED

> +--echo # Should be "...WHERE (a XOR a)
> +explain extended SELECT * FROM t1 WHERE NOT (a XOR (NOT a));
> +
> +--echo # XOR End
> +--echo
> +
>  delete from t1 where a > 3;
>  select a, not(not(a)) from t1;
>  explain extended select a, not(not(a)), not(a <= 2 and not(a)), not(a not
like "1"), not (a not in (1,2)), not(a != 2) from t1 where not(not(a))
having not(not(a));
>
>  drop table t1;
>
> -# End of 4.1 tests
> +
>
> === modified file 'sql/item_cmpfunc.cc'
> --- sql/item_cmpfunc.cc    2011-01-19 14:39:13 +0000
> +++ sql/item_cmpfunc.cc    2011-02-07 15:27:02 +0000
> @@ -5330,17 +5330,15 @@
>      very fast to use.
>  */
>
> -longlong Item_cond_xor::val_int()
> +longlong Item_func_xor::val_int()
>  {
>    DBUG_ASSERT(fixed == 1);
> -  List_iterator<Item> li(list);
> -  Item *item;
> -  int result=0;
> +  int result=0;
>    null_value=0;
> -  while ((item=li++))
> +  for (uint i= 0; i < arg_count; i++)
>    {
> -    result^= (item->val_int() != 0);
> -    if (item->null_value)
> +    result^= (args[i]->val_int() != 0);
> +    if (args[i]->null_value)
>      {
>        null_value=1;
>        return 0;
> @@ -5387,6 +5385,27 @@
>    return item;
>  }
>
> +/*
> +  NOT (a XOR b) == (NOT a) XOR b == a XOR (NOT b)
> + */

  Again: hard to parse that comment.


> +Item *Item_func_xor::neg_transformer(THD *thd)
> +{
> +  Item *neg_argument;
> +  Item_func_xor *new_item;
> +  if ((neg_argument= args[0]->neg_transformer(thd)))
> +    // args[0] has neg_tranformer
> +    new_item= new Item_func_xor(neg_argument, args[1]);
> +  else if ((neg_argument= args[1]->neg_transformer(thd)))
> +    // args[0] has neg_tranformer
> +    new_item= new Item_func_xor(args[0], neg_argument);
> +  else
> +  {
> +    neg_argument= new Item_func_not(args[0]);
> +    new_item= new Item_func_xor(neg_argument, args[1]);

If you use placement new here, you will save my_pthread_getspecific_ptr,
and since you have the thd here anyways:

new_item= new (thd->mem_root) Item_func_xor(neg_argument, args[1]);

> +  }
> +  return new_item;
> +}
> +
>
>  /**
>    a IS NULL  ->  a IS NOT NULL.
>
> === modified file 'sql/item_cmpfunc.h'
> --- sql/item_cmpfunc.h    2010-12-29 00:38:59 +0000
> +++ sql/item_cmpfunc.h    2011-02-07 15:27:02 +0000
> @@ -1,7 +1,7 @@
>  #ifndef ITEM_CMPFUNC_INCLUDED
>  #define ITEM_CMPFUNC_INCLUDED
>
> -/* Copyright (c) 2000, 2010, Oracle and/or its affiliates. All rights
reserved.
> +/* Copyright (c) 2000, 2011, Oracle and/or its affiliates. All rights
reserved.
>
>     This program is free software; you can redistribute it and/or modify
>     it under the terms of the GNU General Public License as published by
> @@ -411,6 +411,22 @@
>    bool subst_argument_checker(uchar **arg) { return TRUE; }
>  };
>
> +/*
> +  XOR inherits from Item_bool_func2 because it is not optimized yet.
> +  Later, when (a XOR b) is optimized, it needs to inherit from
> +  Item_cond instead. It's low prio, though */

Comments should be of the form

/**
  blah blah blah blah
 */

> +class Item_func_xor :public Item_bool_func2
> +{
> +public:
> +  Item_func_xor(Item *i1,Item *i2) :Item_bool_func2(i1,i2) {}

When a function has multiple arguments separated by commas (','),
put one space after each comma

> +  enum Functype functype() const { return XOR_FUNC; }
> +  /* longlong val_int(); */

Why this comment?

> +  const char *func_name() const { return "xor"; }
> +  longlong val_int();
> +  void top_level_item() {}

Could you explain why you override this virtual function?

> +  Item *neg_transformer(THD *thd);
> +};
> +
>  class Item_func_not :public Item_bool_func
>  {
>  public:
> @@ -1759,45 +1775,6 @@
>    return (cond_item->functype() == Item_func::COND_OR_FUNC);
>  }
>
> === modified file 'unittest/gunit/item-t.cc'
> --- unittest/gunit/item-t.cc    2011-02-07 13:03:47 +0000
> +++ unittest/gunit/item-t.cc    2011-02-07 15:27:02 +0000
> @@ -183,4 +183,31 @@
>    EXPECT_LE(item_decrypt->max_length, length);
>  }

With unit test, YAY!!!


Separate each TEST_F with two blank lines.


> +TEST_F(ItemTest, ItemFuncXor)
> +{
> +  const uint length= 1U;
> +  Item_int *item_one= new Item_int(0, length);
> +  Item_int *item_two= new Item_int(1, length);
> +
> +  Item_func_xor *item_xor=
> +    new Item_func_xor(item_two, item_one);
> +
> +  item_xor->fix_fields(m_thd, NULL);


EXPECT_FALSE(item_xor->fix_fields(m_thd, NULL));


> +  EXPECT_EQ(1, item_xor->val_int());

How about an extra test of func_name()

  String print_buffer;
  item_xor->print(&print_buffer, QT_ORDINARY);
  EXPECT_STREQ("(1 xor 0)", print_buffer.c_ptr());



> +
> +  Item *neg_xor= item_xor->neg_transformer(m_thd);
> +  neg_xor->fix_fields(m_thd, NULL);

EXPECT_FALSE

> +  EXPECT_EQ(0, neg_xor->val_int());
> +  EXPECT_EQ(0, neg_xor->val_real());

EXPECT_DOUBLE_EQ(0.0, neg_xor->val_real());

> +  EXPECT_EQ(0, neg_xor->val_bool());

EXPECT_FALSE

> +  EXPECT_EQ(0, neg_xor->is_null());
> +
> +  Item_func_xor *item_xor_null=
> +    new Item_func_xor(item_one, new Item_null());
> +  item_xor_null->fix_fields(m_thd, NULL);
> +
> +  EXPECT_EQ(0, item_xor_null->val_int());
> +  EXPECT_EQ(1, item_xor_null->is_null());

EXPECT_TRUE

> +}
> +
>  }





> --
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe:
> http://lists.mysql.com/commits?unsub=1
>

Thread
bzr commit into mysql-trunk branch (jorgen.loland:3605) Bug#59793Jorgen Loland7 Feb
  • Re: bzr commit into mysql-trunk branch (jorgen.loland:3605) Bug#59793Tor Didriksen8 Feb
    • Re: bzr commit into mysql-trunk branch (jorgen.loland:3605) Bug#59793Jorgen Loland9 Feb
  • Re: bzr commit into mysql-trunk branch (jorgen.loland:3605) Bug#59793Roy Lyseng8 Feb
    • Re: bzr commit into mysql-trunk branch (jorgen.loland:3605) Bug#59793Øystein Grøvlen8 Feb
      • Re: bzr commit into mysql-trunk branch (jorgen.loland:3605) Bug#59793Roy Lyseng8 Feb
      • Re: bzr commit into mysql-trunk branch (jorgen.loland:3605) Bug#59793Jorgen Loland9 Feb
    • Re: bzr commit into mysql-trunk branch (jorgen.loland:3605) Bug#59793Jorgen Loland9 Feb