- developers - lists.mariadb.org

[Maria-developers] Dynamic column for review
by Oleksandr Byelkin 29 Apr '11

29 Apr '11

Hi! Here is 2 patches: one cut version - to review, other one full version.

1 0

Re: [Maria-developers] [Commits] Rev 2908: Fixed LP bugs #717577, #724942. in file:///home/igor/maria/maria-5.3-bug717577/
by Sergey Petrunya 25 Apr '11

25 Apr '11

Hello Igor, First, an overall comment: there are lots of typos/coding style violations in the patch. To reduce amount of effort spent on such things, I was just fixing them as I saw them and I'm attaching the patch with all the fixes (i.e. this patch should be applied on top of the patch that I was reviewing). More important comments are bellow, marked with 'psergey:' On Fri, Mar 25, 2011 at 10:31:19PM -0700, Igor Babaev wrote: > At file:///home/igor/maria/maria-5.3-bug717577/ > > ------------------------------------------------------------ > revno: 2908 > revision-id: igor(a)askmonty.org-20110326053117-x50cah9krd4f1345 > parent: wlad(a)montyprogram.com-20110212174322-f7ptnc0u32vasdwy > committer: Igor Babaev <igor(a)askmonty.org> > branch nick: maria-5.3-bug717577 > timestamp: Fri 2011-03-25 22:31:17 -0700 > message: > Fixed LP bugs #717577, #724942. > > Both these two bugs happened due to the following problem. > When a view column is referenced in the query an Item_direct_view_ref > object is created that is refers to the Item_field for the column. > All references to the same view column refer to the same Item_field. > Different references can belong to different AND/OR levels and, > as a result, can be included in different Item_equal object. > These Item_equal objects may include different constant objects. > If these constant objects are substituted for the Item_field created > for a view column we have a conflict situation when the second > substitution annuls the first substitution. This leads to > wrong result sets returned by the query. Bug #724942 demonstrates > such an erroneous behaviour. > Test case of the bug #717577 produces wrong result sets because best > equal fields of the multiple equalities built for different OR levels > of the WHERE condition differs. The subsitution for the best equal field > in the second OR branch overwrites the the substitution made for the > first branch. > > To avoid such conflicts we have to substitute for the references > to the view columns rather than for the underlying field items. > To make such substitutions possible we have to include into > multiple equalities references to view columns rather than > field items created for such columns. > > This patch modifies the Item_equal class to include references > to view columns into multiple equality objects. It also performs > a clean up of the class methods and adds more comments. The methods > of the Item_direct_view_ref class that assist substitutions for > references to view columns has been also added by this patch. > === modified file 'sql/item.cc' > --- a/sql/item.cc 2011-02-01 03:33:32 +0000 > +++ b/sql/item.cc 2011-03-26 05:31:17 +0000 > @@ -7226,6 +7233,129 @@ > return FALSE; > } > > + > +Item_equal *Item_direct_view_ref::find_item_equal(COND_EQUAL *cond_equal) > +{ > + Item* field_item= real_item(); > + if (field_item->type() != FIELD_ITEM) > + return NULL; > + return ((Item_field *) field_item)->find_item_equal(cond_equal); > +} > + > + > +/** > + Check whether a reference to field item can be substituted b an equal item > + > + @details > + The function checks whether a substitution of a reference to field item for > + an equal item is valid. > + > + @param arg *arg != NULL && **arg <-> the reference is in the context > + where substitution for an equal item is valid > + > + @note > + See also the note for Item_field::subst_argument_checker > + > + @retval > + TRUE substitution is valid > + @retval > + FALSE otherwise > +*/ > +bool Item_direct_view_ref::subst_argument_checker(uchar **arg) > +{ > + bool res= (!(*arg) && (result_type() != STRING_RESULT)) || > + ((*arg) && (**arg)); > + if (*arg) > + **arg= (uchar) 0; psergey: What is the above for? Do I understand it correctly that this is needed so that Item_field that this item is wrapping is not substituted? Please add a comment. > + return res; > +} > + > + > +/** > + Set a pointer to the multiple equality the view field reference belongs to > + (if any). > + > + @details > + The function looks for a multiple equality containing this item of the type > + Item_direct_view_ref among those referenced by arg. > + In the case such equality exists the function does the following. > + If the found multiple equality contains a constant, then the item > + is substituted for this constant, otherwise the function sets a pointer > + to the multiple equality in the item. > + > + @param arg reference to list of multiple equalities where > + the item (this object) is to be looked for > + > + @note > + This function is supposed to be called as a callback parameter in calls > + of the compile method. > + > + @note > + The function calls Item_field::equal_fields_propagator for the field item > + this->real_item() to do the job. Then it takes the pointer to equal_item > + from this field item and assigns it to this->item_equal. > + > + @return > + - pointer to the replacing constant item, if the field item was substituted > + - pointer to the field item, otherwise. > +*/ > + > +Item *Item_direct_view_ref::equal_fields_propagator(uchar *arg) > +{ > + Item *field_item= real_item(); > + if (field_item->type() != FIELD_ITEM) > + return this; > + Item *item= field_item->equal_fields_propagator(arg); > + set_item_equal(field_item->get_item_equal()); > + field_item->set_item_equal(0); psergey: Please use NULL here, we use that for pointers. > + if (item != field_item) > + return item; > + return this; > +} > + ... > === modified file 'sql/item_cmpfunc.cc' > --- a/sql/item_cmpfunc.cc 2011-02-06 04:57:03 +0000 > +++ b/sql/item_cmpfunc.cc 2011-03-26 05:31:17 +0000 > @@ -5516,43 +5516,93 @@ > return 0; > } > > -Item_equal::Item_equal(Item_field *f1, Item_field *f2) > - : Item_bool_func(), const_item(0), eval_item(0), cond_false(0), > - compare_as_dates(FALSE) > -{ > - const_item_cache= 0; > - fields.push_back(f1); > - fields.push_back(f2); > -} > - > -Item_equal::Item_equal(Item *c, Item_field *f) > + > +/** > + Construct a minimal multiple equality item > + > + @param f1 the first equal item > + @param f2 the second equal item > + @param with_const_item TRUE if the first item is constant > + > + @details > + The constructor builds a new item equal object for the equality f1=f2. > + One if the equal items can be constant. If this is the case it is passed > + always as the first parameter and the parameter with_const_item serves > + as an indicator of this case. > + Currently any non-constant parameter items must refer to an item of the > + of the type FIELD_ITEM. psergey: The above is still true for the case where passed item is an Item_direct_view_ref which refers to an Item_field. The wording may be confusing for the new readers, though. Please change to explicitly indicate that f1 and f2 may point to Item_field or Item_direct_view_ref(Item_field) > +*/ > + > +Item_equal::Item_equal(Item *f1, Item *f2, bool with_const_item) > : Item_bool_func(), eval_item(0), cond_false(0) > { > const_item_cache= 0; > - fields.push_back(f); > - const_item= c; > - compare_as_dates= f->is_datetime(); > + with_const= with_const_item; > + compare_as_dates= with_const_item && f2->is_datetime(); > + equal_items.push_back(f1); > + equal_items.push_back(f2); > } > > > +/** > + Copy constructor for a multiple equality > + > + @param item_equal source item for the constructor > + > + @details > + The function creates a copy of an Item_equal object. > + This constructor is used when an item belongs to a multiple equality > + of an upper level (an upper AND/OR level or an upper level of a nested > + outer join). > +*/ > + > Item_equal::Item_equal(Item_equal *item_equal) > : Item_bool_func(), eval_item(0), cond_false(0) > { > const_item_cache= 0; > - List_iterator_fast<Item_field> li(item_equal->fields); > - Item_field *item; > + List_iterator_fast<Item> li(item_equal->equal_items); > + Item *item; > while ((item= li++)) > { > - fields.push_back(item); > + equal_items.push_back(item); > } > - const_item= item_equal->const_item; > + with_const= item_equal->with_const; > compare_as_dates= item_equal->compare_as_dates; > cond_false= item_equal->cond_false; > } > > > -void Item_equal::compare_const(Item *c) > +/* > + @brief > + Add a constant item to the Item_equal object > + > + @param[in] c the constant to add > + @param[in] f item from the list equal_items the item c is equal to > + (this parameter is optional) > + > + @details > + The method adds the constant item c to the list equal_items. If the list > + hasn't not contained any constant item yet the item c is just added > + to the very beginning of the list. Otherwise the value of c is compared > + with the value of the constant item from equal_items. If they are not > + equal cond_false is set to TRUE. This serves as an indicator that this > + Item_equal is always FALSE. psergey: Does function comment really need to duplicate the function body? In my opinion, the above should be split into multiple comments inside the function body. > + The optional parameter f is used to adjust the flag compare_as_dates. > +*/ > + > +void Item_equal::add_const(Item *c, Item *f) > { > + if (cond_false) > + return; > + if (!with_const) > + { > + with_const= TRUE; > + if (f) > + compare_as_dates= f->is_datetime(); > + equal_items.push_front(c); > + return; > + } > + Item *const_item= get_const(); > if (compare_as_dates) > { > cmp.set_datetime_cmp_func(this, &c, &const_item); ... > @@ -5811,19 +5911,15 @@ > return Item_func::transform(transformer, arg); > } > > + > void Item_equal::print(String *str, enum_query_type query_type) > { > str->append(func_name()); > str->append('('); > - List_iterator_fast<Item_field> it(fields); > + List_iterator_fast<Item> it(equal_items); > Item *item; > - if (const_item) > - const_item->print(str, query_type); > - else > - { > - item= it++; > - item->print(str, query_type); > - } > + item= it++; > + item->print(str, query_type); > while ((item= it++)) > { > str->append(','); > @@ -5834,6 +5930,14 @@ > } > > > +CHARSET_INFO *Item_equal::compare_collation() psergey: One expects compare_xxx() functions to compare something, in particular this function looks like it's going to compare some collation. Could you please rename it to e.g. comparison_collation()? > +{ > + Item_equal_fields_iterator it(*this); > + Item *item= it++; > + return item->collation.collation; > +} > + > + > /* > @brief Get the first equal field of multiple equality. > @param[in] field the field to get equal field to ... > === modified file 'sql/item_cmpfunc.h' > --- a/sql/item_cmpfunc.h 2010-10-10 14:18:11 +0000 > +++ b/sql/item_cmpfunc.h 2011-03-26 05:31:17 +0000 > @@ -1672,23 +1707,47 @@ > }; > > psergey: Please add a one-line comment describing what this class does. > -class Item_equal_iterator : public List_iterator_fast<Item_field> > +class Item_equal_fields_iterator : public List_iterator_fast<Item> > { > + Item_equal *item_equal; > + Item *curr_item; > public: > - inline Item_equal_iterator(Item_equal &item_equal) > - :List_iterator_fast<Item_field> (item_equal.fields) > - {} > - inline Item_field* operator++(int) > - { > - Item_field *item= (*(List_iterator_fast<Item_field> *) this)++; > - return item; > - } > - inline void rewind(void) > - { > - List_iterator_fast<Item_field>::rewind(); > - } > + Item_equal_fields_iterator(Item_equal &item_eq) > + :List_iterator_fast<Item> (item_eq.equal_items) > + { > + curr_item= NULL; > + item_equal= &item_eq; > + if (item_eq.with_const) > + { > + List_iterator_fast<Item> *list_it= (List_iterator_fast<Item> *) this; psergey: Why do redundant explicit typecasting? > + curr_item= (*list_it)++; > + } > + } > + Item* operator++(int) > + { > + List_iterator_fast<Item> *list_it= (List_iterator_fast<Item> *) this; psergey: Why do redundant explicit typecasting? > + curr_item= (*list_it)++; > + return curr_item; > + } > + Item ** ref() > + { > + return List_iterator_fast<Item>::ref(); > + } > + void rewind(void) > + { > + List_iterator_fast<Item> *list_it= (List_iterator_fast<Item> *) this; > + list_it->rewind(); > + if (item_equal->with_const) > + curr_item= (*list_it)++; > + } > + Field *get_curr_field() > + { > + Item_field *item= (Item_field *) (curr_item->real_item()); > + return item->field; > + } > }; > > + > class Item_cond_and :public Item_cond > { > public: > > === modified file 'sql/sql_select.cc' > --- a/sql/sql_select.cc 2011-02-11 10:27:35 +0000 > +++ b/sql/sql_select.cc 2011-03-26 05:31:17 +0000 > @@ -9623,7 +9630,8 @@ > args->concat((List<Item> *)&cond_equal.current_level); > } > } > - else if (cond->type() == Item::FUNC_ITEM) > + else if (cond->type() == Item::FUNC_ITEM || > + cond->real_item()->type() == Item::FIELD_ITEM) psergey: As far as I understand the last part of the condition is there so that direct references like ... AND view.column AND ... are processed with subst_argument_checker/equal_fields_propagator. Is it intentional that direct references like ... AND base_table.column AND ... are now processed with subst_argument_checker/equal_fields_propagator as well? > { > List<Item> eq_list; > /* -- BR Sergey -- Sergey Petrunia, Software Developer Monty Program AB, http://askmonty.org Blog: http://s.petrunia.net/blog

2 3

Re: [Maria-developers] A serious merge bug in 5.3.
by Sergei Golubchik 24 Apr '11

24 Apr '11

Hi, Igor! On Apr 24, Igor Babaev wrote: > > When debugging I discovered the following bad code in sql_select.cc of > the current 5.3 tree: > tmp= table->file->read_time(key, 1, > min(tmp,s->worst_seeks)-1); <= !!!!! ? > tmp*= record_count; > > gannotate says that it appeared in the code after your merge with 5.2. > > The fact is that 5.2 contains the correct expression here > min(tmp,s->worst_seeks) > not > min(tmp,s->worst_seeks)-1 > > As a result if the number of records is 1 then min(tmp,s->worst_seeks)-1 > is equal to 0 and we pass 0 as the last parameter to > table->file->read_time. Naturally it returns 0. > > Will you fix this problem or you'd rather want me to do it? In 5.2 it is tmp= record_count*min(tmp,s->worst_seeks); In 5.3, record_count* is moved to a separate line, so it should just be tmp= min(tmp,s->worst_seeks); But it uses table->file->read_time(). Which is defined as virtual double read_time(uint index, uint ranges, ha_rows rows) { return rows2double(ranges+rows); } so, when ranges=1 and we pass rows=min(tmp,s->worst_seeks)-1, we get the same value of min(tmp,s->worst_seeks) as the result. This was the change of introducing handler::keyread_time(), and at the same time using handler::read_time() where appropriate, to have all values comparable. Regards, Sergei

2 2

[Maria-developers] MariaDB/MySQL error code issue
by Kristian Nielsen 19 Apr '11

19 Apr '11

Hi, For merging MySQL into MariaDB, we need to work out what to do about error code. The problem occurs as (different) new error messages are added in both MySQL and MariaDB. When we then merge from MySQL, we get conflicts between the error numbers. If we do not handle this somehow, it means that error numbers for a given error message will be different, either between MySQL and MariaDB, or alternatively between one version of MariaDB and another (depending on how conflicts are resolved). The problem with error code changing is that this makes it much harder for applications to check error codes and handle errors as it want in a portable way between MySQL and MariaDB. I think we have these options: 1. Cooperate with MySQL@Oracle, so that whenever we add an error code in MariaDB, the corresponding code is also reserved in MySQL (even if not used), and vice versa. 2. Create different "namespaces" for error codes added to MariaDB and MySQL, eg. new MariaDB codes are assigned with some offset to MySQL codes, so we avoid conflicts. 3. Keep MariaDB error code numbers stable, re-assign MySQL error code numbers when merging from MySQL (I rather dislike this as well because it introduces incompatibility with MySQL). 4. Keep MySQL error code numbers when merging from MySQL, so that MariaDB-only errors will fluctuate between releases (I really dislike this option). It seems option (4) is the best option. Is someone from MySQL/Oracle willing to work on getting such a procedure running? Failing that, it seems (3) is the best option. In this case, we need (at least) to assign a suitable error code range to new MariaDB errors and change the source code to be able to handle this (eg. maybe we need a sql/share/errmsg-mariadb.txt or something). We will also need to handle the error codes already assigned to virtual columns, which I think already conflict with new MySQL error codes. I would suggest doing a one-time incompatible change to map them into the MariaDB range. So, any hope for getting solution (4) working with MySQL@Oracle? And failing that, any opinions on how to proceed? Thanks, - Kristian.

4 6

[Maria-developers] WL#198 New (by Sergei): Roles
by worklog-noreply＠askmonty.org 19 Apr '11

19 Apr '11

----------------------------------------------------------------------- WORKLOG TASK -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- TASK...........: Roles CREATION DATE..: Tue, 19 Apr 2011, 10:01 SUPERVISOR.....: IMPLEMENTOR....: COPIES TO......: CATEGORY.......: Server-RawIdeaBin TASK ID........: 198 (http://askmonty.org/worklog/?tid=198) VERSION........: WorkLog-4.0 STATUS.........: Un-Assigned PRIORITY.......: 60 WORKED HOURS...: 0 ESTIMATE.......: 250 (hours remain) ORIG. ESTIMATE.: 250 PROGRESS NOTES: DESCRIPTION: Roles, as defined by the SQL standard. See also http://forge.mysql.com/worklog/task.php?id=988 ESTIMATED WORK TIME ESTIMATED COMPLETION DATE ----------------------------------------------------------------------- WorkLog (v4.0.0)

1 0

[Maria-developers] Testing group commit with XtraBackup
by Kristian Nielsen 19 Apr '11

19 Apr '11

Hi Philip, I thought of another test of group commit that ought to be done (not sure why I didn't think to mention this earlier, sorry about that). It's not urgent, just something for the list. The test is basically the same you already did for non-blocking `mysqldump --master-data --single-transaction` used to provision a slave, but using XtraBackup instead of mysqldump. >From what I could learn, the original reason that InnoDB added the mutex that kills group commit when binlog is enabled is the following: To ensure that commit order in the InnoDB transaction log is the same as in the binlog. This is needed when XtraBackup (innobackup really, but they work the same and XtraBackup is Free Software) takes a non-blocking backup of the InnoDB data (only), and this is used to provision a new slave. If commit order was different, there might not be any binlog position on the master corresponding to the backup snapshot used to setup the new slave. Now, my group commit work also guarantees consistent commit order, but using a completely different method that does not kill group commit. So we ought to test this. So the test would be similar to your existing mysqldump test. But while the master is running, we would use non-blocking XtraBackup to take a snapshot. This snapshot we would then restore onto a new slave. The slave would then be configured with CHANGE MASTER TO, I believe one gets the correct binlog position to start from in the log file of XtraDB and/or slave mysqld (I need to check up on details). Then the test proceeds as the mysqldump case, letting the slave catch up and checking that replication is ok etc. What do you think? Now, to do this we would need to be able to use XtraBackup. I am wondering if it wouldn't make sense to at the same time include XtraBackup into MariaDB? XtraBackup seems to me quite mature already, and a very good product besides. It is arguably long overdue for us to include it. So if we include XtraBackup in mariadb-5.2-rpl (/5.3), I think this should be possible to setup without too much trouble (I can provide specific details/scripts for how to actually run XtraBackup and setup slave start position, I just need to research that a bit). Or we could also do the test first with XtraBackup as external, shouldn't be much harder, and would give us some experience with it. What do you think? Maybe we can look at it in more detail in Lisbon. - Kristian.

2 3

[Maria-developers] WL#197 New (by Mdcallag): table lock wait timeouts
by worklog-noreply＠askmonty.org 13 Apr '11

13 Apr '11

----------------------------------------------------------------------- WORKLOG TASK -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- TASK...........: table lock wait timeouts CREATION DATE..: Wed, 13 Apr 2011, 17:09 SUPERVISOR.....: IMPLEMENTOR....: COPIES TO......: CATEGORY.......: Server-RawIdeaBin TASK ID........: 197 (http://askmonty.org/worklog/?tid=197) VERSION........: WorkLog-4.0 STATUS.........: Un-Assigned PRIORITY.......: 60 WORKED HOURS...: 0 ESTIMATE.......: 0 (hours remain) ORIG. ESTIMATE.: 0 PROGRESS NOTES: DESCRIPTION: MySQL has table_lock_wait_timeout but that doesn't do anything. For some apps it would help to timeout sessions blocked on a table lock. For example, when ALTER TABLE runs it implicitly gets an X lock on the table cache entry. Queries that follow are blocked until it finishes. The client-side read timeout might expire and disconnect the client but the server-side thread will continue blocking and the server can fill with blocked sessions. Right now the stack when the client blocks is open_tables -> open_table -> wait_for_condition. I implemented a demo for this by: 1) use pthread_cond_timedwait in wait_for_condition 2) check for timeout expiration in open_tables and error out Without the change in 2) the session will loop within open_tables on calls to open_table. ESTIMATED WORK TIME ESTIMATED COMPLETION DATE ----------------------------------------------------------------------- WorkLog (v4.0.0)

1 0

Re: [Maria-developers] Windows installer MWL#55 finished.
by Vladislav Vaintroub 13 Apr '11

13 Apr '11

> -----Original Message----- > From: kahzoo(a)gmail.com [mailto:kahzoo@gmail.com] On Behalf Of Kazuhisa > Ichikawa > Sent: Sonntag, 6. Februar 2011 09:32 > To: Vladislav Vaintroub > Cc: Philip Stoev; maria-developers(a)lists.launchpad.net > Subject: Re: [Maria-developers] Windows installer MWL#55 finished. Hello, Kazuhisa > No :-). My experience with MySQL installers is limited to fresh > install. This is the first time I tried 'upgrade' and my expectation > was purely imaginary. Right, there is no upgrade functionality in MySQLs installers up-to-now (nor I heard or any planning for it recently). But I noticed that both you and Philip had quite similar requests for the changed UI in case of "upgrade possible". Both of you recommended changes at the "Database Properties" dialog, I think it is wrong place, by the time you enter this dialog it is too late, you should not have entered this dialog, as the intention was not "create new", but "upgrade existing" anyway.. So since both of my users have pointed to usability issues, I added a new dialog that comes after license agreement that and tells user that upgradable instances were found and please make a choice between installing a new instance and not installing a new instance(optional upgrade one or more existing ones). This sounds almost like your first suggestion, but in own dialog. This dialog comes right before feature selection, and only thing it might do is to unselect "Database Instance" feature in the following dialog . Technically, it duplicates functionality of the feature selection tree. So what... I hope it adds some clarity for the user - at least tells the user that his existing instances *can* be upgraded. Or maybe it won't help that much, since user never looks at the UI anyway :) (nice article about that is here http://www.codinghorror.com/blog/2009/10/treating-user-myopia.html ) Anyway, I uploaded the newest msis to the usual place, if somebody wants to play with it. Thanks, Vladislav.

2 4

[Maria-developers] MariaDB 5.6
by Peter Laursen 12 Apr '11

12 Apr '11

I noticed this on planet.mysql: http://kb.askmonty.org/v/plans-for-56 I *again* strongly want to discourage a major version number identical with a MySQL/Oracle release. MySQL plans a 5.6 too and I believe that there is already a source-tree available on launchpad. I think I understand that MariaDB 5.6 is planned to use MySQL 5.5 codebase. Am I correct? The problem arises with *generic clients* (ie clients that are supposed to work with not only one specific server version but a broad range of server versions) - including all GUI clients. Such clients will execute a "SELECT VERSION()" after connection to decide what features in the client should be enabled or not (necessary as "SHOW VARIABLES LIKE 'have_***"" and "SELECT ... FROM I_S" cannot always be used due to incomplete instrumentation with those) I have now posted this complaint 2 or 3 times (including the times I complained about the use of 5.2 for a mariaDB based on MySQL 5.1 code as there is alose a (now abandoned) MySQL 5.2 tree. I never had a reply from a MariaDB 'decisionmaker'. Could I at least request a reply this time? Please! :-) I repeat: It is really causing problems for *generic clients* if there are too different 5.6's with different feature sets. What solution would you suggest for a client that shall be able to connect to both MySQL5.6 and MariaDB 5.6 without confusing the features of one server with the other? Peter Webyog

3 5

[Maria-developers] PBXT test failures after your patch for lp:732124
by Kristian Nielsen 12 Apr '11

12 Apr '11

Hi Monty, After you pushed the patch for lp:732124, test failures appeared in Buildbot for test case pbxt.union. Most were just that the pbxt union.result file needed same update as for main.union. But there was one more issue: create table t1 (a int); insert into t1 values (1),(2),(3); create table t2 (a int); insert into t2 values (3),(4),(5); # ... SELECT SQL_CALC_FOUND_ROWS * FROM t1 UNION SELECT * FROM t2 LIMIT 2; select found_rows(); In pbxt.union, this now (after your patch) return 5. However, the result file for main.union has a return value of 6, and the test succeeds with MyISAM. The thing is ... the value that 5 PBXT returns is correct, and what MyISAM does and what is in the result file is wrong. At least according to the docs. The full query would return 5 rows (as it is union distinct), not 6... What I will do is that I will fix the PBXT union.result to have the correct value "5" to get the test suite to pass in Buildbot. However, there seems to be some deeper problem here, given that MyISAM does something different (and wrong?). There are also other cases that are wrong: SELECT SQL_CALC_FOUND_ROWS * FROM t1 LIMIT 1 UNION SELECT * FROM t2; a 1 3 4 5 select found_rows(); found_rows() 6 SELECT SQL_CALC_FOUND_ROWS * FROM t1 LIMIT 1 UNION SELECT * FROM t2 LIMIT 2; a 1 3 select found_rows(); found_rows() 6 (the result should have been 5) So maybe there are still bugs in the code? Or if this is a limitation of SQL_CALC_FOUND_ROWS with UNION (there is a comment in the test case "# The following examples will not be exact" that might hint at this), then we should document this at least ... - Kristian.

1 0