January 2015 - developers - lists.mariadb.org

[Maria-developers] Memory barrier problem in InnoDB/XtraDB mutex implementation causing hangs
by Kristian Nielsen 27 May '15

27 May '15

I have been debugging some test failures seen in Buildbot: https://mariadb.atlassian.net/browse/MDEV-7026 After some debugging, this is what I think is going on. I will refer to MariaDB 5.5 sources, though the problem is also in 10.0. InnoDB/XtraDB mutexes are taken in mutex_enter_func() and unlocked in mutex_exit_func(). Simplifying for clarity, here are the relevant parts of the logic: ENTER: # First spin wait, hoping to get the mutex without yielding. os_compare_and_swap_ulint(&mutex->waiters, 0, 1); if (os_atomic_test_and_set_byte(&mutex->lock_word, 1)) sync_array_wait_event() RELEASE: os_atomic_lock_release_byte(&mutex->lock_word); os_rmb; waiters = *(volatile *)&mutex->waiters; if (waiters) mutex_signal_object(mutex); The interesting point here is how to ensure that we do not lose the wakeup of a waiting thread when the holding thread releases the mutex. The idea is that the waiting thread first sets mutex->waiters=1. Then it checks if the mutex is free - if not, it goes to sleep. The thread that releases the lock first marks the mutex free. Then it checks if there are any waiters, and if so wakes them up. For this to work, we need to ensure that the setting of waiters=1 has completed before the load of lock_word can start - a full memory barrier. Likewise, we need the setting of lock_word to 0 to complete before the load of mutex->waiters - again a full memory barrier. Otherwise we can get something like this: ENTER RELEASE Start store to mutex->waiters Start store to lock_word Load mutex->lock_word Store to lock_word becomes visible Load mutex->waiters Store to waiters becomes visible Check lock word==1 Check waiters==0, skip wakeup Sleep (forever) Or this: ENTER RELEASE Start store to lock_word Start store to mutex->waiters Load mutex->waiters Store to waiters becomes visible Load mutex->lock_word Store to lock_word becomes visible Check lock word==1 Sleep (forever) Check waiters==0, skip wakeup (So both full barriers are needed, the one in ENTER to prevent the first example, the one in RELEASE to prevent the second). It looks like for x86, the full barriers were broken in MariaDB 5.5.40 and 10.0.13. os_compare_and_swap_ulint() is defined as either __sync_bool_compare_and_swap() or atomic_cas_ulong(), both of which implies a full barrier. So ENTER looks ok. os_atomic_lock_release_byte() however is now defined as __sync_lock_release(). This is _not_ a full barrier, just a release barrier. So the RELEASE case seems to be incorrect, allowing the second deadlock example to occur. Previously, RELEASE used os_atomic_test_and_set_byte(), which is __sync_lock_test_and_set(). This is defined in GCC docs as an "aquire" barrier only, which is also wrong, obviously. But it happens to generate an XCHG instruction on x86, which is a full barrier. This is probably why it works on x86 (and probably why it did not work on PowerPC). Here is a small test program, and the assembly from GCC 5.7 on amd64: void test(unsigned *p, unsigned *q) { while (__sync_lock_test_and_set(q, 1)) { } *p++; __sync_lock_release(q); } test: movl $1, %edx .L2: movl %edx, %eax xchgl (%rsi), %eax testl %eax, %eax jne .L2 movl $0, (%rsi) ret As can be seen, the __sync_lock_test_and_set() generates a full barrier, but the __sync_lock_release() generates nothing (as this is sufficient for release semantics for the rather strong memory order on x86). So the InnoDB mutexes look completely broken from MariaDB 5.5.40 and 10.0.13 :-( This is consistent with the observations in MDEV-7026. A thread is waiting for wakeup on a mutex that is already marked free. Apparently this is not a new problem. This comment in mutex_exit_func() makes me want to cry: /* A problem: we assume that mutex_reset_lock word is a memory barrier, that is when we read the waiters field next, the read must be serialized in memory after the reset. A speculative processor might perform the read first, which could leave a waiting thread hanging indefinitely. Our current solution call every second sync_arr_wake_threads_if_sema_free() to wake up possible hanging threads if they are missed in mutex_signal_object. */ So someone found the mutex implementation broken, but instead of fixing it, they implemented a horrible workaround to try and break any deadlocks that might occur. (The reason that the deadlocks are not broken in the MDEV-7026 seems to be that it occurs during startup, before starting the srv_error_monitor_thread() that does the sync_arr_wake_threads_if_sema_free()). Now, what is also interesting is that this seems to explain very nicely the many server lockups we have seen for users that upgrade to recent MariaDB versions. I was doing a very detailed investigation of one such case where the user was able to supply unusually detailed information. I found a thread that was stuck waiting on a mutex, but no other threads seemed to be holding that mutex. This is exactly what would be seen in case of a lost wakeup due to this issue. Furthermore, the sync_arr_wake_threads_if_sema_free() mechanism, as well as the normal InnoDB "long semaphore wait", was itself blocked on that same mutex wait, which explains why they were not effective for the user in that particular case. So it looks like we need to revert the incorrect patch and release new 5.5.40a and 10.0.15 releases ASAP, to solve these hangs for users. And do a proper patch for PowerPC with proper full memory barriers. (Incidentally, the os_rmb in RELEASE seems to also be part of some PowerPC patch, it also looks completely wrong). However, more importantly, this issue clearly shows some fundamental problems in our development procedures that we need to fix ASAP. Just look at it. We have users (and customers!) that suffer from strange hangs. This is their production systems that are down. Support and developers are unable to help them due to the difficulty of debugging rare race conditions in production systems. And all the time the bug was staring us in the face, easily reproducable in our own Buildbot! But we deliberately chose to ignore those failures. The only thing that matters is "when can you push", and getting more releases out. Never mind what kind of crap is then pushed out to users. I am deeply ashamed of treating our users like this. And extremely unhappy. I even wrote about this (the importance of not ignoring Buildbot failures) just a few weeks ago. This was completely ignored, I got zero replies, and the weekly calls continued with mindless juggling of Jira numbers and buggy releases. As a result, our users suffer. We need to stop this. Please? - Kristian.

4 16

Re: [Maria-developers] e91559c: MDEV-7006 MDEV-7007: SET STATEMENT and slow log
by Sergei Golubchik 16 Feb '15

16 Feb '15

Hi, Oleksandr! On Jan 28, Oleksandr Byelkin wrote: > revision-id: e91559cf7f2b19378e9e5d3f72c3a6794a09d1e0 > parent(s): cbc318fcf3680c01dca1320b87f625124a497528 > committer: Oleksandr Byelkin > branch nick: work-maria-10.1-MDEV-6997 > timestamp: 2014-11-26 11:54:29 +0100 > message: > > MDEV-7006 MDEV-7007: SET STATEMENT and slow log > fixed embedded server tests > > old values of SET STATENENT variables now saved in its own Query_arena and restored later Basically, I have two comments: 1. I think it might be kind of expensive to create MEM_ROOT for every SET STATEMENT. And perhaps a user will never need to set-statement these log variables. I don't see a good solution for that. Either we can drop the support for log related variables in SET STATEMENT, basically not using your patch. I wouldn't like that, from a user point of view this is a pretty artificial limitation, he doesn't care about memroots. Or we can only create memroot for log related variables and put other variables in the statement memroot, which also means restoring values in two steps. Seems like too complex for a solution. A compromise solution would be to do almost like in your patch, but not to free memroot after every statement. That is, a memroot is created once, for the first SET STATEMENT. At the end of the statement you only MY_MARK_BLOCKS_FREE or MY_KEEP_PREALLOC. Then the next SET STATEMENT wouldn't need to allocate that much. Not perfect, but might do as a tradeoff. Or, perhaps you can suggest a better solution. 2. I didn't quite like that you've added restore_set_statement_var, reset_arena_for_set_stmt, free_arena_for_set_stmt, etc everywhere. I'd prefer you to add them to one function which is called from all these places. Like, there must be some cleanup-after-statement function that you should be able to use. > diff --git a/mysql-test/t/set_statement.test b/mysql-test/t/set_statement.test > index a658071..108c1a3 100644 > --- a/mysql-test/t/set_statement.test > +++ b/mysql-test/t/set_statement.test > @@ -995,6 +985,57 @@ set global general_log=0; > set statement lock_wait_timeout=1 for select @@lock_wait_timeout; > set global general_log=@save_general_log; > > +--echo # MDEV-7006 MDEV-7007: SET statement and slow log > + > +set @save_long_query_time= @@long_query_time; > +set @save_slow_query_log= @@slow_query_log; > +set @save_log_output= @@log_output; > + > +set statement long_query_time=default for select @@long_query_time; > +set statement log_slow_filter=default for select @@log_slow_filter; > +set statement log_slow_verbosity=default for select @@log_slow_verbosity; > +set statement log_slow_rate_limit=default for select @@log_slow_rate_limit; > +set statement slow_query_log=default for select @@slow_query_log; > + > +truncate table mysql.slow_log; > +set slow_query_log= 1; > +set global log_output='TABLE'; > + > +select sql_text from mysql.slow_log; > +set @@long_query_time=0.01; > +--echo #should be written > +select sleep(0.1); > +set @@long_query_time=@save_long_query_time; > +select sql_text from mysql.slow_log; > +--echo #--- > +--echo #should be written > +set statement long_query_time=0.01 for select sleep(0.1); > +select sql_text from mysql.slow_log; Okay, I hope it'll work, but please do try it in a separate branch in buildbot. all these timing-related tests have proven to be very fragile, so I've learned to be very cautious about these stuff. even in test that looks safe to me :) > +--echo #--- > +set @@long_query_time=0.01; > +--echo #should NOT be written > +set statement slow_query_log=0 for select sleep(0.1); > +set @@long_query_time=@save_long_query_time; > +select sql_text from mysql.slow_log; > +--echo #--- > +--echo #should be NOT written > +set statement long_query_time=0.01,log_slow_filter='full_scan' for select sleep(0.1); > +select sql_text from mysql.slow_log; > +--echo #--- > +--echo #should be NOT written > +set statement long_query_time=0.01,log_slow_rate_limit=9999 for select sleep(0.1); > +select sql_text from mysql.slow_log; > +--echo #--- > +# > +# log_slow_verbosity is impossible to check because results are not written > +# in TABLE mode > +# > + > +set global log_output= @save_log_output; > +set @@slow_query_log= @save_slow_query_log; > +set @@long_query_time= @save_long_query_time; > + > + > # > # Prohibited Variables > # > diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc > index 6432e19..08094ce 100644 > --- a/sql/sql_lex.cc > +++ b/sql/sql_lex.cc > @@ -4235,6 +4236,53 @@ int LEX::print_explain(select_result_sink *output, uint8 explain_flags, > return res; > } > > +void LEX::set_arena_for_set_stmt(Query_arena *backup) > +{ > + DBUG_ENTER("LEX::set_arena_for_set_stmt"); > + DBUG_ASSERT(arena_for_set_stmt== 0); > + MEM_ROOT *mem_root= new MEM_ROOT(); > + if (!(mem_root)) > + DBUG_VOID_RETURN; > + init_sql_alloc(mem_root, ALLOC_ROOT_MIN_BLOCK_SIZE, 0, > + MYF(MY_THREAD_SPECIFIC)); > + if (!(arena_for_set_stmt= new Query_arena(mem_root, > + Query_arena::STMT_INITIALIZED))) > + DBUG_VOID_RETURN; 1. allocate a new memroot for every SET STATEMENT? 2. no error reporting? > + DBUG_PRINT("info", ("mem_root: 0x%lx arena: 0x%lx", > + (ulong) mem_root, (ulong) arena_for_set_stmt)); > + thd->set_n_backup_active_arena(arena_for_set_stmt, backup); > + DBUG_VOID_RETURN; > +} > + > + > +void LEX::reset_arena_for_set_stmt(Query_arena *backup) > +{ > + DBUG_ENTER("LEX::reset_arena_for_set_stmt"); > + DBUG_ASSERT(arena_for_set_stmt); > + thd->restore_active_arena(arena_for_set_stmt, backup); > + DBUG_PRINT("info", ("mem_root: 0x%lx arena: 0x%lx", > + (ulong) arena_for_set_stmt->mem_root, > + (ulong) arena_for_set_stmt)); > + DBUG_VOID_RETURN; > +} > + > + > +void LEX::free_arena_for_set_stmt() > +{ > + DBUG_ENTER("LEX::free_arena_for_set_stmt"); > + if (!arena_for_set_stmt) > + return; > + DBUG_PRINT("info", ("mem_root: 0x%lx arena: 0x%lx", > + (ulong) arena_for_set_stmt->mem_root, > + (ulong) arena_for_set_stmt)); > + arena_for_set_stmt->free_items(); > + free_root(arena_for_set_stmt->mem_root, MYF(0)); > + delete(arena_for_set_stmt->mem_root); > + delete(arena_for_set_stmt); Eh... The arena is allocated in the memroot. I don't think you can free the memroot before the arena. Probably something like this: MEM_ROOT *memroot= arena_for_set_stmt->mem_root; delete arena_for_set_stmt; free_root(memroot, MYF(0)); delete memroot; > + arena_for_set_stmt= 0; > + DBUG_VOID_RETURN; > +} > + > void LEX::restore_set_statement_var() > { > DBUG_ENTER("LEX::restore_set_statement_var"); > @@ -4243,7 +4291,9 @@ void LEX::restore_set_statement_var() > DBUG_PRINT("info", ("vars: %d", old_var_list.elements)); > sql_set_variables(thd, &old_var_list, false); > old_var_list.empty(); > + free_arena_for_set_stmt(); > } > + DBUG_ASSERT(!is_arena_for_set_stmt()); the indentation is broken like this in many parts of the patch. May be that's because you've forwarded it with your MUA... > DBUG_VOID_RETURN; > } > > diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc > index 5af700b..ae831d0 100644 > --- a/sql/sql_parse.cc > +++ b/sql/sql_parse.cc > @@ -1956,6 +1958,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd, > > /* Check that some variables are reset properly */ > DBUG_ASSERT(thd->abort_on_warning == 0); > + thd->lex->restore_set_statement_var(); too bad you had to put these restore_set_statement_var, reset_arena_for_set_stmt, free_arena_for_set_stmt, etc everywhere explicitly. > DBUG_RETURN(error); > } > Regards, Sergei

2 2

Re: [Maria-developers] Row-based replication of old DECIMAL to new DECIMAL
by Sergei Golubchik 13 Feb '15

13 Feb '15

Hi, Alexander! On Dec 22, Alexander Barkov wrote: > Hi Sergei, Kristian, > > I'm looking at: > > MDEV-7268 Column of table cannot be converted from type 'decimal(0,?)' > to type ' 'decimal(10,7)' > > This bug cannot be fixed in general case, because the old DECIMAL data > type does not write its metadata into the binary log. > > I suggest we don't fix replication of old DECIMAL > (neither binlogging, nor slave-side). I think that's reasonable. But then CHECK TABLE ... FOR UPGRADE should issue a warning for old DECIMAL columns. > Instead, we fix "ALTER TABLE ... FORCE" (and thus mysql_upgrade) > to force changing old DECIMAL to new DECIMAL. > Currently, "ALTER TABLE...FORCE" preserve old DECIMAL. > > Not sure which version is best to fix in. > The complainer had problems with 5.5. I don't know, I'd say 10.0. For the complainer there's a workaround - I'm sure you can come up with an ALTER TABLE that changes columns from old to new DECIMAL, may be something like ALTER TABLE xxx MODIFY COLUMN yyy DECIMAL(....) Regards, Sergei

7 15

[Maria-developers] Interest in Contributing to MariaDB and participation in GSoC 2015
by Tekang Check 11 Feb '15

11 Feb '15

Hello, I am Tekang Mengwe, a third year Accounting student at the University of Buea. I am interested in contributing code to MariaDB and participating in the 2015 round of the Google Summer of Code. While looking at the projects. I'm interested in Database replication and would love to do some work in this respect. I am proficient in C/C++ and code as a hobby. I will greatly appreciate any assistance in this respect. Regards, Tekang

6 7

[Maria-developers] custom storage backend rnd_next() implementation and caching questions
by Andras Szabo 03 Feb '15

03 Feb '15

Hi Everyone, I am developing a custom mariadb storage backend which is intended to give filesystem related information via mariadb tables. For example one could tell: CREATE TEMPORARY TABLE IF NOT EXISTS `/var/log` ENGINE=fsview; which would create a table (using the assisted discovery method) with the following schema: MariaDB [test]> describe `/var`; +-----------+--------------+------+-----+---------+-------+ | Field | Type | Null | Key | Default | Extra | +-----------+--------------+------+-----+---------+-------+ | file_name | varchar(255) | YES | | NULL | | | contents | longblob | YES | | NULL | | +-----------+--------------+------+-----+---------+———+ However i have some implementation issues: 1. I am having trouble implementing rnd_next() in the backend: int ha_fsview::rnd_next(uchar *buf) { DBUG_ENTER(__PRETTY_FUNCTION__); if (!dirp) { DBUG_RETURN(HA_ERR_INTERNAL_ERROR); } if (struct dirent* entry = readdir(dirp)) { uchar *pos = buf; *pos = 0; ++pos; // file name (1bytes size + str) *pos = strlen(entry->d_name); ++pos; size_t bytes_to_copy = strlen(entry->d_name); memcpy(pos, entry->d_name, bytes_to_copy); pos+=bytes_to_copy; DBUG_PRINT("info", ("fsview filename: %s", entry->d_name)); // content (4 bytes size + content ptr) static const std::string fake_content = "hello world"; int s = fake_content.size(); memcpy(pos, &s, sizeof(s)); pos+=sizeof(s); const char* c = fake_content.c_str(); memcpy(pos, &c, sizeof(c)); pos+=sizeof(c); DBUG_RETURN(0); } else { DBUG_RETURN(HA_ERR_END_OF_FILE); } } My problem is that filenames (first field) is ok, but i cannot really see the expected content “hello world” in the second field. I am assuming that i am doing something wrong, but i cannot really find the reason. Could somebody help me, please? 2. If i am issuing: SELECT * FROM `/var`; twice, then the server is not initiating a table scan again on my backend (most likely results from the first run are stored in some cache). So if filesystem contents are changed between the two statements, then it won’t be reflected in the resultset. Of course if i do a: SELECT SQL_NO_CACHE * FROM `/var`; then everything is fine, however i would not like to put the burden on the user to specify the option to not cache the results. My question is whether it’s possible to specify in the storage backend that the results from the tables should not be cached. Thank for the help in advance -- Andras Szabo Sent with Airmail

3 7

Re: [Maria-developers] [Commits] Rev 4577: MDEV-5719: Wrong result with GROUP BY and LEFT OUTER JOIN in lp:~maria-captains/maria/10.0
by Sergey Petrunia 29 Jan '15

29 Jan '15

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(a)mariadb.com-20150123162551-h88uxn780ivj5k2n > parent: sergii(a)pisem.net-20150119153032-fqrmxjr13z0p1ly0 > committer: Nirbhay Choubey <nirbhay(a)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(a)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(a)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

2 2

[Maria-developers] Dynamic columns support in libmaria client library?
by Peter Laursen 28 Jan '15

28 Jan '15

This page https://mariadb.com/kb/en/mariadb/documentation/nosql/dynamic-columns-api/ does not mention anything about it. it just says *The API is a part of libmysql C client library. In order to use it, one needs to include this header file* *#include <mysql/ma_dyncol.h>* *and link against libmysql.* Now can ma_dyncol.h be linked against libmaria as well? There are 2 questions actually 1) technical: will it work with libmaria? 2) legal: can a client program using libmaria under LGPL license include ma_dyncol.h? -- Peter -- Webyog

3 5

Re: [Maria-developers] [Commits] 51fa027: Improve performance for calculating memory allocation
by Sergei Golubchik 25 Jan '15

25 Jan '15

Hi, Monty! On Jan 23, Michael Widenius wrote: > revision-id: 51fa0272febd782674b3d44828718e805c44c1a8 > parent(s): ab440b0fb7302d707ba0ba41382bf911404db1cb > committer: Michael Widenius > branch nick: maria-10.1 > timestamp: 2015-01-23 14:32:53 +0200 > message: > > Improve performance for calculating memory allocation See my comments below: > diff --git a/include/mysql/plugin.h b/include/mysql/plugin.h > index 16b0c6e..9ae0fe8 100644 > --- a/include/mysql/plugin.h > +++ b/include/mysql/plugin.h > @@ -190,6 +196,7 @@ struct st_mysql_show_var { > > #define SHOW_VAR_FUNC_BUFF_SIZE (256 * sizeof(void*)) > typedef int (*mysql_show_var_func)(MYSQL_THD, struct st_mysql_show_var*, char *); > +typedef int (*mysql_show_var_extended_func)(MYSQL_THD, struct st_mysql_show_var*, enum enum_var_type, char *); I'd put scope the last making mysql_show_var_extended_func backward compatible with mysql_show_var_func. this makes some code simpler, see below. > > /* > diff --git a/sql/sql_show.cc b/sql/sql_show.cc > index 044b60d..8eed7b9 100644 > --- a/sql/sql_show.cc > +++ b/sql/sql_show.cc > @@ -3115,9 +3115,16 @@ static bool show_status_array(THD *thd, const char *wild, > if var->type is SHOW_FUNC or SHOW_SIMPLE_FUNC, call the function. > Repeat as necessary, if new var is again one of the above > */ > - for (var=variables; var->type == SHOW_FUNC || > - var->type == SHOW_SIMPLE_FUNC; var= &tmp) > - ((mysql_show_var_func)(var->value))(thd, &tmp, buff); > + for (var=variables; > + var->type == SHOW_FUNC || var->type == SHOW_SIMPLE_FUNC || > + var->type == SHOW_EXTENDED_FUNC; > + var= &tmp) > + { > + if (var->type == SHOW_EXTENDED_FUNC) > + ((mysql_show_var_extended_func)(var->value))(thd, &tmp, scope, buff); > + else > + ((mysql_show_var_func)(var->value))(thd, &tmp, buff); You won't need this if() when scope is the last argument. In fact I'm still thinking whether it would be a good idea to add scope to mysql_show_var_func and SHOW_FUNC, without introducing SHOW_EXTENDED_FUNC at all. This wouldn't break any existing plugin, as it's perfectly binary compatible extension. Also, see below. > + } > > SHOW_TYPE show_type=var->type; > if (show_type == SHOW_ARRAY) > diff --git a/sql/sql_class.cc b/sql/sql_class.cc > index 0d49407..d200af6 100644 > --- a/sql/sql_class.cc > +++ b/sql/sql_class.cc > @@ -1747,6 +1749,16 @@ void add_to_status(STATUS_VAR *to_var, STATUS_VAR *from_var) > to_var->binlog_bytes_written+= from_var->binlog_bytes_written; > to_var->cpu_time+= from_var->cpu_time; > to_var->busy_time+= from_var->busy_time; > + to_var->local_memory_used+= from_var->local_memory_used; why? it's not additive, right? and global_status_var.local_memory_used is never shown anyway. > + > + /* > + Update global_memory_used. We have to do this with atomic_add as the > + global value can change outside of LOCK_status. > + */ > + // workaround for gcc 4.2.4-1ubuntu4 -fPIE (from DEB_BUILD_HARDENING=1) > + int64 volatile * volatile ptr= &to_var->global_memory_used; I'd rather remove this workaround. 4.2.4 is ancient, and quite probably this bug doesn't happen with MY_MEMORY_ORDER_RELAXED at all > + my_atomic_add64_explicit(ptr, from_var->global_memory_used, > + MY_MEMORY_ORDER_RELAXED); > } > > /* > @@ -1784,6 +1796,11 @@ void add_diff_to_status(STATUS_VAR *to_var, STATUS_VAR *from_var, > dec_var->binlog_bytes_written; > to_var->cpu_time+= from_var->cpu_time - dec_var->cpu_time; > to_var->busy_time+= from_var->busy_time - dec_var->busy_time; > + > + /* > + We don't need to accumulate memory_used as these are not after > + this function is called. Sorry, I cannot parse that :( > + */ > } > > #define SECONDS_TO_WAIT_FOR_KILL 2 > diff --git a/sql/mysqld.cc b/sql/mysqld.cc > index ea53e47..c4e1dc3 100644 > --- a/sql/mysqld.cc > +++ b/sql/mysqld.cc > @@ -3972,19 +3972,24 @@ static void my_malloc_size_cb_func(long long size, my_bool is_thread_specific) > if (thd) > { > DBUG_PRINT("info", ("memory_used: %lld size: %lld", > - (longlong) thd->status_var.memory_used, size)); > + (longlong) thd->status_var.local_memory_used, > + size)); > - thd->status_var.memory_used+= size; > + thd->status_var.local_memory_used+= size; > - DBUG_ASSERT((longlong) thd->status_var.memory_used >= 0); > + DBUG_ASSERT((longlong) thd->status_var.local_memory_used >= 0); > } > } > } > + else if (likely(thd)) > + thd->status_var.global_memory_used+= size; Hmm, ok. So, you think that current_thd is cheaper than updating a global variable? > + else > + { > // workaround for gcc 4.2.4-1ubuntu4 -fPIE (from DEB_BUILD_HARDENING=1) > - int64 volatile * volatile ptr=&global_status_var.memory_used; > + int64 volatile * volatile ptr=&global_status_var.global_memory_used; I would've removed this workaround too > my_atomic_add64_explicit(ptr, size, MY_MEMORY_ORDER_RELAXED); > + } > } > } > > - > static int init_common_variables() > { > umask(((~my_umask) & 0666)); > @@ -8028,6 +8033,21 @@ static int show_default_keycache(THD *thd, SHOW_VAR *var, char *buff) > return 0; > } > > + > +static int show_memory_used(THD *thd, SHOW_VAR *var, enum enum_var_type scope, > + char *buff) > +{ > + var->type= SHOW_LONGLONG; > + var->value= buff; > + if (scope == OPT_GLOBAL) > + *(longlong*) buff= (global_status_var.local_memory_used + > + global_status_var.global_memory_used); > + else > + *(longlong*) buff= thd->status_var.local_memory_used; > + return 0; I'm surprised you haven't added SHOW_EXTENDED_SIMPLE_FUNC :) That would've make a very confusing name though. May be we should indeed add scope as the last argument to mysql_show_var_func and reuse both SHOW_SIMPLE_FUNC and SHOW_FUNC? I'm starting to think it's a good thing to do. Makes the API simpler and the number of choices to understand - smaller. If you do that - please don't forget to increment MARIA_PLUGIN_INTERFACE_VERSION. > +} > + > + > #ifndef DBUG_OFF > static int debug_status_func(THD *thd, SHOW_VAR *var, char *buff) > { Regards, Sergei

1 0

[Maria-developers] Additions to GIS in 10.1.
by Alexey Botchkov 23 Jan '15

23 Jan '15

Hi all. What's new about GIS in 10.1. Related tasks in JIRA are: MDEV-4045 Missing OGC Spatial functions. MDEV-60 Support for Spatial Reference systems for the GIS data. MDEV-12 OpenGIS: create required tables: GeometryColumns, related views. Speaking shortly, the MariaDB GIS part is now OpenGIS compliant, and passes all the OpenGIS conformance tests To get that the next things were done: Missing functions were added: IsRing(ln) The return type is Integer, with a return value of 1 for TRUE, 0 for FALSE, and –1 for UNKNOWN corresponding to a function invocation on NULL arguments; return TRUE if c is a ring, i.e., if c is closed and simple. A simple Curve does not pass through the same Point more than once. PointOnSurface(s) The return type is POINT, returns a Point guaranteed to lie on the Surface. arguments: POLYGON or MYLTIPOLYGON. NumInteriorRing(p) The return type is Integer, returns the number of interiorRings arguments: POLYGON. Relate(g1 Geometry, g2 Geometry, patternMatrix String) The return type is Integer, with a return value of 1 for TRUE, 0 for FALSE, and –1 for UNKNOWN corresponding to a function invocation on NULL arguments; returns TRUE if the spatial relationship specified by the pattern Matrix holds. ConvexHull(g1 Geometry) The return is Geometry, returna a geometric object that is the convex hull of g1 arguments: GEOMETRY. The information about the GEOMETRY column's reference system (SRID) is now stored in the field's metadata. Syntax of the CREATE and ALTER was modified to specify the SRID for the geometry columns. With this information some views required by OpenGIS, were added: GEOMETRY_COLUMNS, SPATIAL_REF_SYS: These are links to the INFORMATION_SCHEMA.GEOMETRY_COLUMNS and INFORMATION_SCHEMA.SPATIAL_REF_SYS respectively. INFORMATION_SCHEMA.GEOMETRY_COLUMNS: describes the available feature tables and their Geometry properties. Fields: F_TABLE_CATALOG CHARACTER VARYING(256) NOT NULL, F_TABLE_SCHEMA CHARACTER VARYING(256) NOT NULL, F_TABLE_NAME CHARACTER VARYING(256) NOT NULL, the above 3 fields are the fully qualified name of the feature table containing the geometry column; F_GEOMETRY_COLUMN CHARACTER VARYING(256) NOT NULL, the name of the column in the feature table that is the Geometry Column. G_TABLE_CATALOG CHARACTER VARYING(256) NOT NULL, G_TABLE_SCHEMA CHARACTER VARYING(256) NOT NULL, G_TABLE_NAME CHARACTER VARYING(256) NOT NULL, the above 3 fields are the name of the geometry table and its schema and catalog. The geometry table implements the geometry column; STORAGE_TYPE INTEGER, always 1 in MariaDB (means binary geometry implementation) GEOMETRY_TYPE INTEGER, the type of geometry values stored in this column. 0 = GEOMETRY 1 = POINT 3 = LINESTRING 5 = POLYGON 7 = MULTIPOINT 9 = MULTILINESTRING 11 = MULTIPOLYGON COORD_DIMENSION INTEGER, the number of dimensions in the spatial reference system. Always 2 in MariaDB. MAX_PPR INTEGER, Always 0 in MariaDB. SRID INTEGER the ID of the Spatial Reference System used for the coordinate geometry in this table. It is a foreign key reference to the SPATIAL_REF_SYS table. INFORMATION_SCHEMA.SPATIAL_REF_SYS: stores information on each spatial reference system used in the database. Fields: SRID INTEGER NOT NULL PRIMARY KEY, an integer value that uniquely identifies each Spatial Reference System within a database. AUTH_NAME CHARACTER VARYING(256), the name of the standard or standards body that is being cited for this reference system. In most cases the value is "EPSG". AUTH_SRID INTEGER TEXT the Well-known Text Representation of the Spatial Reference System. Required stored procedures added: AddGeometryColumn(FEATURE_TABLE_CATALOG, FEATURE_TABLE_SCHEMA, FEATURE_TABLE_NAME, GEOMETRY_COLUMN_NAME, SRID) adds new column of spatial type to the specified table DropGeometryColumn(FEATURE_TABLE_CATALOG, FEATURE_TABLE_SCHEMA, FEATURE_TABLE_NAME, GEOMETRY_COLUMN_NAME) removes the spatial column from the table. That's it for now. Best regards. HF

1 0

[Maria-developers] Renaming Virtual Columns to Computed Columns
by Ian Gilfillan 19 Jan '15

19 Jan '15

There was a suggestion on the docs mailing list that we rename Virtual Columns to Computed Columns. I think it makes sense, but before we implement I want to make sure everyone is happy with the change, so am repeating it here as not everyone follows docs. Currently, we use the term Virtual Columns (see https://mariadb.com/kb/en/mariadb/documentation/sql-structure-and-commands/…) Oracle uses this term, while MS SQL and Firebird use the term Computed Columns to describe similar functionality. MariaDB's virtual column implementation has both persistent and virtual attributes. Talking of "virtual virtual" and "persistent virtual" can be confusing. Oracle's implementation, to my knowledge, only has virtual virtual, while MS SQL's Computed Columns offers both persistent ("persisted" in their case) and virtual. I think "Computed" is a slightly more descriptive term, and avoids the ambiguity of "persistent virtual" and "virtual virtual". The MariaDB error messages already use the term computed, for example "Constant expression in computed column function is not allowed". Unless anyone has a good reason to prefer the term virtual, we're going to rather use the term "computed column" in the docs. The article will still mention virtual column as an alternative etc, but computed will be the preferred term. ian

3 3