Hi Sergei, == TL;DR == I have debugged that LEFT JOIN problem. The fix is not pretty, but correct. I will now push it into the 10.0 tree. == Details == So, the commit changes - return (table->null_row ? TRUE : - null_ptr ? MY_TEST(null_ptr[row_offset] & null_bit) : 0); + return real_maybe_null() ? + MY_TEST(null_ptr[row_offset] & null_bit) : table->null_row; The patch changes the behavior for NULLable fields. Suppose * real_maybe_null()= TRUE * table->null_row==TRUE // the table has a NULL-complemented row * (null_ptr[row_offset] & null_bit) == FALSE // however the value // is not null. the old code will return TRUE the new code will return FALSE The new code may seem wrong at first glance (it's a NULL-complemented row, why we return FALSE from is_null()?) However, if we look where table->null_row is set to true, we will find that it is done here: inline void mark_as_null_row(TABLE *table) { table->null_row=1; table->status|=STATUS_NULL_ROW; bfill(table->null_flags,table->s->null_bytes,255); } The function also sets all NULL-bits, so normally one can assume that (table->null_row=TRUE) => ((null_ptr[..] & null_bit) == TRUE) However, there is a case when the above does not hold. It happens when we run GROUP BY query, and GROUP BY is satisfied by scanning an index which produces rows in group order. In that case, the following can occur: consider the query from the testcase: SELECT ... FROM table_one a LEFT JOIN table_two b ON a.oid=b.oid GROUP BY a.oid * we read from a row (1,3) * we make index lookup in table b and find row (3). * row combination a(1,3)-b(3) passes the WHERE clause, GROUP BY execution code records it as current row. * there are no other matches in table b, so we get back to reading table a. * we read row a(1,4) * we look for matches in table b. there are no matches, so we produce a NULL-complemented row b(NULLs) * row combination a(1,4)-b(NULLs) is passed to the GROUP BY runtime. The runtime sees that we are now in a different group, and figures that it should pass the row combination representing the previous group into query output. * It attempts to pass the row combination a(1,3)-b(3) (*) into query output. However, the current row for table b is already the NULL-complemented row. the row combination (*) is stored in the "result_field" objects (and not in "regular field" objects which refer to table->record[0]). It seems wrong that we look at result_field (which points to a row that's different from table->record[0]), and at the same time we are looking at table->null_row, which refers to the record in table->record[0]. However, "result_field" storage has no place for table->null_row bit. The fix basically means: "for NULL-able fields, field's NULL-bit should override table->null_row". Considering the above, it is correct (albeit ugly). Now, what about GROUP BY and NOT NULL fields? I discovered the following: if a table has table->maybe_null, then Field objects that refer to such table and are pointed by Item_field::result_field, are always NULLable. (code-wise, check out setup_copy_fields(), these calls: item->result_field=field->new_field(thd->mem_root,field->table, 1); item->result_field->move_field(copy->to_ptr,copy->to_null_ptr,1); result_field object of NOT NULL column is null-able. On Fri, Jan 23, 2015 at 08:25:59AM -0800, Nirbhay Choubey wrote:
At lp:~maria-captains/maria/10.0
------------------------------------------------------------ revno: 4577 revision-id: nirbhay@mariadb.com-20150123162551-h88uxn780ivj5k2n parent: sergii@pisem.net-20150119153032-fqrmxjr13z0p1ly0 committer: Nirbhay Choubey <nirbhay@mariadb.com> branch nick: b5719-10.0 timestamp: Fri 2015-01-23 11:25:51 -0500 message: MDEV-5719: Wrong result with GROUP BY and LEFT OUTER JOIN
Merged revision 5224 from mysql-5.6 and added a test case. .. revno: 5224 committer: Sergey Glukhov <sergey.glukhov@oracle.com> branch nick: mysql-5.6 timestamp: Wed 2013-06-19 14:24:08 +0400 message: Bug#16620047 INCORRECT QUERY RESULT (AFTER SERVER UPGRADE) ..
=== modified file 'mysql-test/r/group_by_innodb.result' --- a/mysql-test/r/group_by_innodb.result 2014-01-26 20:49:11 +0000 +++ b/mysql-test/r/group_by_innodb.result 2015-01-23 16:25:51 +0000 @@ -57,3 +57,26 @@ NULL 11.1,22.2 DROP TABLE t1; End of 5.5 tests +# +# MDEV-5719: Wrong result with GROUP BY and LEFT OUTER JOIN +# +CREATE TABLE table_one (oidGroup INT, oid INT PRIMARY KEY)ENGINE=INNODB; +INSERT INTO table_one VALUES (1,1),(1,2),(1,3),(1,4); +CREATE TABLE table_two (oid INT PRIMARY KEY)ENGINE=INNODB; +INSERT INTO table_two VALUES (3); +SELECT a.oidGroup, a.oid, b.oid FROM table_one a LEFT JOIN table_two b ON +a.oid=b.oid WHERE a.oidGroup=1; +oidGroup oid oid +1 1 NULL +1 2 NULL +1 3 3 +1 4 NULL +SELECT a.oidGroup, a.oid, b.oid FROM table_one a LEFT JOIN table_two b ON +a.oid=b.oid WHERE a.oidGroup=1 GROUP BY a.oid; +oidGroup oid oid +1 1 NULL +1 2 NULL +1 3 3 +1 4 NULL +DROP TABLE table_one, table_two; +# End of tests
=== modified file 'mysql-test/t/group_by_innodb.test' --- a/mysql-test/t/group_by_innodb.test 2014-01-26 20:49:11 +0000 +++ b/mysql-test/t/group_by_innodb.test 2015-01-23 16:25:51 +0000 @@ -67,3 +67,22 @@
--echo End of 5.5 tests
+--echo # +--echo # MDEV-5719: Wrong result with GROUP BY and LEFT OUTER JOIN +--echo # +CREATE TABLE table_one (oidGroup INT, oid INT PRIMARY KEY)ENGINE=INNODB; +INSERT INTO table_one VALUES (1,1),(1,2),(1,3),(1,4); + +CREATE TABLE table_two (oid INT PRIMARY KEY)ENGINE=INNODB; +INSERT INTO table_two VALUES (3); + +# Returns a value +SELECT a.oidGroup, a.oid, b.oid FROM table_one a LEFT JOIN table_two b ON +a.oid=b.oid WHERE a.oidGroup=1; + +SELECT a.oidGroup, a.oid, b.oid FROM table_one a LEFT JOIN table_two b ON +a.oid=b.oid WHERE a.oidGroup=1 GROUP BY a.oid; + +DROP TABLE table_one, table_two; + +--echo # End of tests
=== modified file 'sql/field.h' --- a/sql/field.h 2014-10-20 12:42:00 +0000 +++ b/sql/field.h 2015-01-23 16:25:51 +0000 @@ -656,21 +656,28 @@ inline bool is_null(my_ptrdiff_t row_offset= 0) const { /* + If the field is NULLable, it returns NULLity based + on null_ptr[row_offset] value. Otherwise it returns + NULL flag depending on TABLE::null_row value. + The table may have been marked as containing only NULL values for all fields if it is a NULL-complemented row of an OUTER JOIN or if the query is an implicitly grouped query (has aggregate functions but no GROUP BY clause) with no qualifying rows. If - this is the case (in which TABLE::null_row is true), the field - is considered to be NULL. + this is the case (in which TABLE::null_row is true) and the + field is not nullable, the field is considered to be NULL. + + Do not change the order of testing. Fields may be associated + with a TABLE object without being part of the current row. + For NULL value check to work for these fields, they must + have a valid null_ptr, and this pointer must be checked before + TABLE::null_row. + Note that if a table->null_row is set then also all null_bits are set for the row. - - Otherwise, if the field is NULLable, it has a valid null_ptr - pointer, and its NULLity is recorded in the "null_bit" bit of - null_ptr[row_offset]. */ - return (table->null_row ? TRUE : - null_ptr ? MY_TEST(null_ptr[row_offset] & null_bit) : 0); + return real_maybe_null() ? + MY_TEST(null_ptr[row_offset] & null_bit) : table->null_row; } inline bool is_real_null(my_ptrdiff_t row_offset= 0) const { return null_ptr ? (null_ptr[row_offset] & null_bit ? 1 : 0) : 0; }
_______________________________________________ commits mailing list commits@mariadb.org https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits
-- BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog