Code review comment for lp:~paul-mccullagh/maria/maria-pbxt-rc2

Paul McCullagh <email address hidden> writes:

> Paul McCullagh has proposed merging lp:~paul-mccullagh/maria/maria-pbxt-rc2 into lp:maria.
>
> Requested reviews:
> Maria-captains (maria-captains)
>
> Updated the PBXT engine to RC2 (lp:pbxt/rc2), version 1.0.08c. All tests in the PBXT suite pass.
>
> Note that this branch does not yet include the entire history of PBXT because attempts to do this have failed so far due to problems with bzr.

Thanks a lot Paul for preparing this.

I tried building your tree and running the test suite. I have a question, as I
got some build and test failures. I suspect that perhaps your tree is missing
a commit, or maybe has one too many.

Your tree has the following 3 commits in addition to lp:maria:

 2722 Paul McCullagh 2009-08-18
      Merged changes for bug fix update 1.0.08c RC2

 2721 Paul McCullagh 2009-08-17
      Updated all tests for RC2

 2720 Paul McCullagh 2009-08-17
      Updated PBXT to version 1.0.08 RC2

If I build revision 2721 (drop the last commit), things look good. I even have
zero failures in the test suite with a simple patch (attached), mostly some
simple fixes for case-sensitive file system.

But if I try the current tree, revision 2722, I get both build and test
failures, as detailed below.

So the basic question is, should I merge just revision 2721, or should I merge
2722 with some additional build (and test?) fixes?

-----------------------------------------------------------------------

So some more details.

First, I fixed all of the test failures in revision 2721 with a simple patch
(attached). The only problem was the *-master.opt files were not copied along
with test .test files from the main suite. This caused failures on
case-sensitive file systems (and also failure in udf.test due to wrong search
path for .so). You should of course check the patch, but I think it should be
ok.

Now, for the problems in revision 2722:

They might be related to the replacement of stream_xt with pbms_enabled. To
even build, I need this patch:

--- storage/pbxt/src/Makefile.am 2009-05-09 04:01:53 +0000
+++ storage/pbxt/src/Makefile.am 2009-08-28 10:11:41 +0000
@@ -19,7 +19,7 @@ noinst_HEADERS = bsearch_xt.h cache_xt.
       datadic_xt.h datalog_xt.h filesys_xt.h hashtab_xt.h \
       ha_pbxt.h heap_xt.h index_xt.h linklist_xt.h \
       memory_xt.h myxt_xt.h pthread_xt.h restart_xt.h \
- streaming_xt.h sortedlist_xt.h strutil_xt.h \
+ pbms_enabled.h sortedlist_xt.h strutil_xt.h \
       tabcache_xt.h table_xt.h trace_xt.h thread_xt.h \
       util_xt.h xaction_xt.h xactlog_xt.h lock_xt.h \
       systab_xt.h ha_xtsys.h discover_xt.h \
@@ -30,7 +30,7 @@ libpbxt_la_SOURCES = bsearch_xt.cc cache
       datadic_xt.cc datalog_xt.cc filesys_xt.cc hashtab_xt.cc \
       ha_pbxt.cc heap_xt.cc index_xt.cc linklist_xt.cc \
       memory_xt.cc myxt_xt.cc pthread_xt.cc restart_xt.cc \
- streaming_xt.cc sortedlist_xt.cc strutil_xt.cc \
+ pbms_enabled.cc sortedlist_xt.cc strutil_xt.cc \
       tabcache_xt.cc table_xt.cc trace_xt.cc thread_xt.cc \
       systab_xt.cc ha_xtsys.cc discover_xt.cc \
       util_xt.cc xaction_xt.cc xactlog_xt.cc lock_xt.cc locklist_xt.cc

I assume you must have something similar, maybe you forgot to push it?

And in the test suite, I get some failures. For example, the following:

    create database mysqltest;
    create table t1 (c1 int);
    alter table t1 rename mysqltest.t1;

mysqltest: At line 3: query 'alter table t1 rename mysqltest.t1' failed: 1025: Error on rename of './test/t1' to './mysqltest/t1' (errno: 16)

In the mysqld.1.err log I have this:

    090828 16:19:14 [Error] pbms_rename_table_with_blobs() Error: PBMS does not support renaming tables across databases.

Which is a bit strange since table t1 has only an int?

There are 3 other test failures, which from a quick look might be related to
blob. I attached the full mysql-test-run output, maybe you can take a look?

Please let me know if you know what the issue is, or what more I can do to
help.

 - Kristian.