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

Hi Kristian,

On Aug 31, 2009, at 1:27 PM, Kristian Nielsen wrote:

> Paul McCullagh <email address hidden> writes:
>
>>> mysqltest: At line 3: query 'alter table t1 rename mysqltest.t1'
>>> failed: 1025: Error on rename of './test/t1' to './mysqltest/
>>> t1' (errno: 16)
>>
>>
>> Occurs because the PBMS code is compiled in.
>>
>> This can be disabled by commenting out:
>>
>> #define PBMS_ENABLED
>>
>> in xt_defs.h
>
> [xt_config.h actually]

Oops, sorry. It has been moved to xt_defs.h (where it belongs), in a
later release.

>> This is, in fact, a bug in PBMS. I have already reported the bug
>> here: https://bugs.launchpad.net/pbms/+bug/416969
>>
>> So, if the PBMS_ENABLED code is disable (which should probably be the
>> default at the moment), then that test should run through.
>
> Ok, thanks, that solved 3 of the 4 remaining failures.
>
> And the last one is just a missing result file update for test
> pbxt.ps_1general. You have this diff in the last commit for
> ha_pbxt.cc:
>
> - stats.data_file_length = ot->ot_table->tab_rec_eof_id;
> + stats.data_file_length = xt_rec_id_to_rec_offset(ot->ot_table,
> ot->ot_table->tab_rec_eof_id);
>
> So the data file length output of show table status was fixed from
> units of
> records to units of bytes. I just updated the result file
> accordingly, from 1
> to 1024.

Yup. That is correct.

>> The other problems seem to have to do with case-sensitivity. I will
>> see if I can repeat those errors on one of my machines.
>
> Yes, they are case sensitivity issues (except udf maybe). As I said,
> they all
> pass with the patch I gave, which just adds missing *-master.opt
> files that
> are in the main test suite, but where not copied into the pbxt suite
> along
> with the *.test files.
>
> So I will merge this into MariaDB with attached patch to disable
> PBMS and fix
> test failures.

OK, Great! Thanks.

In general, we will always take care that all patches you (or anyone
else) make to PBXT in MariaDB will find there way back into the PBXT
trunk.

> I really want to also enable the PBXT test suite in our
> Buildbot. Unfortunately I am really pressed for time do do this now.
> For one,
> I see a number of Valgrind errors when running the suite that I need
> to
> investigate. But one of these days,

Yup. Valgrind will be great to have.

There is still work to done on our side. Current status is that when
we run it we get warnings but we have not understood why there is a
problem.

Of course, when you have time, any help there would be greatly
appreciated. But of course, there are higher priorities at the moment.

Best regards,

Paul

--
Paul McCullagh
PrimeBase Technologies
www.primebase.org
www.blobstreaming.org
pbxt.blogspot.com