update with sub select produces invalid results from transaction_reader/transaction_log

[Joe Daly]

I added the following test to the transaction_log suite, the test generated a missing WHERE clause when updating table t2. See below.

jdaly@rx7:~/drizzle/repos/transaction_log/tests/suite/transaction_log/t> cat sub_select.inc
#
# Tests a update with a sub query
#

--disable_warnings
DROP TABLE IF EXISTS t1;
DROP TABLE IF EXISTS t2;
--enable_warnings

CREATE TABLE t1 (
  id INT NOT NULL
, padding VARCHAR(200) NOT NULL
);

INSERT INTO t1 VALUES (1, "I love testing.");
INSERT INTO t1 VALUES (2, "I hate testing.");

CREATE TABLE t2 (
  id INT NOT NULL
, padding VARCHAR(200) NOT NULL
);

INSERT INTO t2 VALUES (4, "I like to procrastinate");
UPDATE t1 SET id=(SELECT MAX(id) FROM t2);
DROP TABLE t1;
jdaly@rx7:~/drizzle/repos/transaction_log/tests/suite/transaction_log/t>

This produced the following output from the transaction_reader:

jdaly@rx7:~/drizzle/repos/transaction_log/tests/suite/transaction_log/r> cat sub_select.result
DROP TABLE IF EXISTS t1;
DROP TABLE IF EXISTS t2;
CREATE TABLE t1 (
id INT NOT NULL
, padding VARCHAR(200) NOT NULL
);
INSERT INTO t1 VALUES (1, "I love testing.");
INSERT INTO t1 VALUES (2, "I hate testing.");
CREATE TABLE t2 (
id INT NOT NULL
, padding VARCHAR(200) NOT NULL
);
INSERT INTO t2 VALUES (4, "I like to procrastinate");
UPDATE t1 SET id=(SELECT MAX(id) FROM t2);
DROP TABLE t1;
DROP TABLE IF EXISTS `t1`;
DROP TABLE IF EXISTS `t2`;
CREATE TABLE t1 ( id INT NOT NULL , padding VARCHAR(200) NOT NULL );
INSERT INTO `test`.`t1` (`id`,`padding`) VALUES (1,'I love testing.');
INSERT INTO `test`.`t1` (`id`,`padding`) VALUES (2,'I hate testing.');
CREATE TABLE t2 ( id INT NOT NULL , padding VARCHAR(200) NOT NULL );
INSERT INTO `test`.`t2` (`id`,`padding`) VALUES (4,'I like to procrastinate');
START TRANSACTION;
UPDATE `test`.`t1` SET `id`=4 WHERE ; <------------------Missing clause
UPDATE `test`.`t1` SET `id`=4 WHERE ; <------------------Missing clause
COMMIT;
DROP TABLE `t1`;
SET GLOBAL transaction_log_truncate_debug= true;
jdaly@rx7:~/drizzle/repos/transaction_log/tests/suite/transaction_log/r>

[Jay Pipes (jaypipes)]

Hi Joe!

So...you've run into an (undocumented) limitation of the transaction log and Drizzle replication. :) All tables which will be replicated must have a primary key or replication/transaction log does not work.

So, I don't think this is a bug, however we *do* need to put some sort of error message together for when the transaction log notices there is no primary key available. Either we bomb out during the call to TransactionLog::apply() or we bomb out in ReplicationServices::push(). I think I'm favoring the latter option.

Thoughts?

-jay

[Joe Daly (skinny.moey)]

ah yes, I knew that. Ill update the test case with a primary key and push it up. It does work with the primary key. Theres really not a great place to put a error message if you allow tables without primary keys. If someone has this error they would have to alter the table and restart replication basically to get the master and slave in sync again? A long term fix may be to verify the schema is suitable for replication.

[fmpwizard (diego-fmpwizard)]

Hi,

On Tue, Nov 10, 2009 at 10:19 AM, Joe Daly <email address hidden> wrote:
> ah yes, I knew that. Ill update the test case with a primary key and
> push it up. It does work with the primary key. Theres really not a great
> place to put a error message if you allow tables without primary keys.
> If someone has this error they would have to alter the table and restart
> replication basically to get the master and slave in sync again?  A long
> term fix may be to verify the schema is suitable for replication.
>

I think there is another thread on the mailing list addressing this
but, could Drizzle handle the missing primary key in a similar way
innodb does?
What I mean is, if you have an innodb table without a primary key,
innodb creates one for you internally (which I believe the user has no
access to).

IMHO, as an end user, this is cleaner than having to add a primary key
to all my tables just because Drizzle replication needs it :)

Thanks

          -Diego

--
Diego Medina
Web Developer
http://www.fmpwizard.com

[Jay Pipes (jaypipes)]

fmpwizard wrote:
> Hi,
>
> On Tue, Nov 10, 2009 at 10:19 AM, Joe Daly <email address hidden> wrote:
>> ah yes, I knew that. Ill update the test case with a primary key and
>> push it up. It does work with the primary key. Theres really not a great
>> place to put a error message if you allow tables without primary keys.
>> If someone has this error they would have to alter the table and restart
>> replication basically to get the master and slave in sync again? A long
>> term fix may be to verify the schema is suitable for replication.
>>
>
> I think there is another thread on the mailing list addressing this
> but, could Drizzle handle the missing primary key in a similar way
> innodb does?
> What I mean is, if you have an innodb table without a primary key,
> innodb creates one for you internally (which I believe the user has no
> access to).

Unfortunately, not right now :( It's just too much work to get it
working correctly for very little benefit IMHO.

> IMHO, as an end user, this is cleaner than having to add a primary key
> to all my tables just because Drizzle replication needs it :)

How many times have you had a use for a table without a primary key that
*wasn't a temporary table*? FYI, temp tables aren't replicated in
Drizzle...

-jay

[fmpwizard (diego-fmpwizard)]

On Thu, Nov 12, 2009 at 9:33 AM, Jay Pipes <email address hidden> wrote:
> fmpwizard wrote:
>>>
>>
>> I think there is another thread on the mailing list addressing this
>> but, could Drizzle handle the missing primary key in a similar way
>> innodb does?
>> What I mean is, if you have an innodb table without a primary key,
>> innodb creates one for you internally (which I believe the user has no
>> access to).
>
> Unfortunately, not right now :(  It's just too much work to get it
> working correctly for very little benefit IMHO.
>
>> IMHO, as an end user, this is cleaner than having to add a primary key
>> to all my tables just because Drizzle replication needs it :)
>
> How many times have you had a use for a table without a primary key that
> *wasn't a temporary table*?  FYI, temp tables aren't replicated in
> Drizzle...

After I sent the previous email I tried to remember, and no, I have
always had a primary key on all my tables :)

>
> -jay
>
> --
> update with sub select produces invalid results from transaction_reader/transaction_log
> https://bugs.launchpad.net/bugs/479743
> You received this bug notification because you are a member of Drizzle-
> developers, which is subscribed to Drizzle.
>
> Status in A Lightweight SQL Database for Cloud and Web: Incomplete
>
> Bug description:
> I added the following test to the transaction_log suite, the test generated a missing WHERE clause when updating table t2. See below.
>
> jdaly@rx7:~/drizzle/repos/transaction_log/tests/suite/transaction_log/t> cat sub_select.inc
> #
> # Tests a update with a sub query
> #
>
> --disable_warnings
> DROP TABLE IF EXISTS t1;
> DROP TABLE IF EXISTS t2;
> --enable_warnings
>
> CREATE TABLE t1 (
>  id INT NOT NULL
> , padding VARCHAR(200) NOT NULL
> );
>
> INSERT INTO t1 VALUES (1, "I love testing.");
> INSERT INTO t1 VALUES (2, "I hate testing.");
>
> CREATE TABLE t2 (
>  id INT NOT NULL
> , padding VARCHAR(200) NOT NULL
> );
>
> INSERT INTO t2 VALUES (4, "I like to procrastinate");
> UPDATE t1 SET id=(SELECT MAX(id) FROM t2);
> DROP TABLE t1;
> jdaly@rx7:~/drizzle/repos/transaction_log/tests/suite/transaction_log/t>
>
> This produced the following output from the transaction_reader:
>
> jdaly@rx7:~/drizzle/repos/transaction_log/tests/suite/transaction_log/r> cat sub_select.result
> DROP TABLE IF EXISTS t1;
> DROP TABLE IF EXISTS t2;
> CREATE TABLE t1 (
> id INT NOT NULL
> , padding VARCHAR(200) NOT NULL
> );
> INSERT INTO t1 VALUES (1, "I love testing.");
> INSERT INTO t1 VALUES (2, "I hate testing.");
> CREATE TABLE t2 (
> id INT NOT NULL
> , padding VARCHAR(200) NOT NULL
> );
> INSERT INTO t2 VALUES (4, "I like to procrastinate");
> UPDATE t1 SET id=(SELECT MAX(id) FROM t2);
> DROP TABLE t1;
> DROP TABLE IF EXISTS `t1`;
> DROP TABLE IF EXISTS `t2`;
> CREATE TABLE t1 ( id INT NOT NULL , padding VARCHAR(200) NOT NULL );
> INSERT INTO `test`.`t1` (`id`,`padding`) VALUES (1,'I love testing.');
> INSERT INTO `test`.`t1` (`id`,`padding`) VALUES (2,'I hate testing.');
> CREATE TABLE t2 ( id INT NOT NULL , padding VARCHAR(200) NOT NULL );
> INSERT INTO `test`.`t2` (`id`,`padding`) VALUES (4,'I like to procrastinate');
> START TRANSACTION;
> UPDATE `test`.`t1` SET `i...