Fault in GenTest::Transform::ExecuteAsUpdateDelete

[Roel Van de Paar]

The following code in ExecuteAsUpdateDelete.pm:

=====
( $original_result->rows() == 1 ?
        "UPDATE $table_name SET `$col_name` = ( $original_query ) WHERE `$col_name` IN ( $original_query ) " :
        "UPDATE $table_name SET `$col_name` = $col_name WHERE `$col_name` IN ( $original_query ) "
),

# The queries above should have updated all rows
"SELECT IF(ROW_COUNT() = ".$original_result->rows()." OR ROW_COUNT() = -1, 1, 0) /* TRANSFORM_OUTCOME_SINGLE_INTEGER_ONE */",
=====

Is incorrect. Example:

=====
mysql> show create table id4\G
*************************** 1. row ***************************
       Table: id4
Create Table: CREATE TABLE `id4` (
  `id` int(11) DEFAULT NULL
) ENGINE=InnoDB DEFAULT CHARSET=latin1
1 row in set (0.00 sec)

mysql> select * from id4\G
*************************** 1. row ***************************
id: 1
1 row in set (0.01 sec)

mysql> update id4 set id=id where id=1\G
Query OK, 0 rows affected (0.00 sec)
Rows matched: 1 Changed: 0 Warnings: 0

mysql> select row_count();
+-------------+
| row_count() |
+-------------+
| 0 |
+-------------+
1 row in set (0.00 sec)
=====

Reason (** highlight added):

ROW_COUNT()
[...]
DML statements other than SELECT: The number of **affected rows**. This applies to statements such as UPDATE, INSERT, or DELETE (as before), but now also to statements such as ALTER TABLE and LOAD DATA INFILE.
Source: http://dev.mysql.com/doc/refman/5.5/en/information-functions.html#function_row-count

So the IF line should be:

"SELECT IF(ROW_COUNT() = 0 OR ROW_COUNT() = -1, 1, 0) /* TRANSFORM_OUTCOME_SINGLE_INTEGER_ONE */",

Instead.

[Roel Van de Paar (roel11)]

Did some more testing. 5.1 and 5.5 both react like this:

mysql> update id4 set id=id where id=1\G
Query OK, 0 rows affected (0.06 sec)
Rows matched: 1 Changed: 0 Warnings: 0

mysql> select row_count();
+-------------+
| row_count() |
+-------------+
| 0 |
+-------------+
1 row in set (0.03 sec)

mysql> update id4 set id=10 where id=1\G
Query OK, 1 row affected (0.00 sec)
Rows matched: 1 Changed: 1 Warnings: 0

mysql> select row_count();
+-------------+
| row_count() |
+-------------+
| 1 |
+-------------+
1 row in set (0.00 sec)

Now, what worries me a bit before changing this is that in the orginal two queries I am not exactly sure if the query will always be in the form of id=id instead of id=value, as per the examples above.

I'd like a second opinion here.

Is there maybe another way around this? Why do we need the == 1 check? Maybe we can just always use SET `$col_name` = $col_name? Why no quotes around the second $col_name?

[Matthias Leich (matthias-leich)]

Hi Roel,

Am 28.06.2011 02:35, schrieb Roel Van de Paar:
> Did some more testing. 5.1 and 5.5 both react like this:
>
> mysql> update id4 set id=id where id=1\G
> Query OK, 0 rows affected (0.06 sec)
> Rows matched: 1 Changed: 0 Warnings: 0

we called this in my non MySQL history a pseudo update.

>
> mysql> select row_count();
> +-------------+
> | row_count() |
> +-------------+
> | 0 |
> +-------------+
> 1 row in set (0.03 sec)
>
> mysql> update id4 set id=10 where id=1\G
> Query OK, 1 row affected (0.00 sec)
> Rows matched: 1 Changed: 1 Warnings: 0
>
> mysql> select row_count();
> +-------------+
> | row_count() |
> +-------------+
> | 1 |
> +-------------+
> 1 row in set (0.00 sec)
>
> Now, what worries me a bit before changing this is that in the orginal
> two queries I am not exactly sure if the query will always be in the
> form of id=id instead of id=value, as per the examples above.

Are you sure that you want a discussion with me and not with John?
You want to change this to what?
The grammar "decides" if you get id=<column> or id=<value> generated.
And some grammars generate both variants.

>
> I'd like a second opinion here.
>
> Is there maybe another way around this? Why do we need the == 1 check?
> Maybe we can just always use SET `$col_name` = $col_name? Why no quotes
> around the second $col_name?
>

Don't know what you are referring to but yes it looks inconsistent.
BTW: I hate the MySQL capability to have optional backticks around identifier.
      It follows the infamous Microsoft and unfortunately also MySQL tradition
      to do things different but in most cases not better than the
      competition or the existing de facto standard.
      Double quotes please. :)

Regards

Matthias

--
Matthias Leich
Senior QA Developer
Office: [phone]

ORACLE Deutschland B.V. & Co. KG
Hauptverwaltung: Riesstr. 25, D-80992 München
Registergericht: Amtsgericht München, HRA 95603

Komplementärin: ORACLE Deutschland Verwaltung B.V.
Rijnzathe 6, 3454PV De Meern, Niederlande
Handelsregister der Handelskammer Midden-Niederlande, Nr. 30143697
Geschäftsführer: Jürgen Kunz, Marcel van de Molen, Alexander van der Ven

[Bernt M Johnsen (bernt-johnsen)]

Matthias Leich wrote:

> Double quotes please. :)

+1

[John H. Embretsen (johnemb)]

You cannot "verify" RQG behavior with respect to ROW_COUNT() by using a different mysql client than what the RQG is using (DBD::mysql). This is becacuse DBD::mysql sets the client flag mysql_client_found_rows (CLIENT_FOUND_ROWS) by default (it has been doing so since 2003, version 2.9002). This flag tells MySQL to report "matched rows" as ROW_COUNT(), whereas the default without this flag is to report "changed rows" as ROW_COUNT().

http://dev.mysql.com/doc/refman/5.6/en/information-functions.html#function_row-count

As far as I can tell the logic in ExecuteAsUpdateDelete.pm is correct when considering the Perl driver's settings, so this is likely not a bug. However, sometimes the transformer does seem to report issues that seem related to this when running the generated test cases, so there may still be something fishy going on in this area.

[Bernt M Johnsen (bernt-johnsen)]

The current behaviour of DBD::mysql is the most reasonable one (compared with other databases). If needed, one could add (yet another [sigh]) option to set mysql_client_found_rows to FALSE. But that's another issue.

[Roel Van de Paar (roel11)]

How about if we just remove the (rather confusing) IF((ROW_COUNT()... queries? We should see a much more clearer (and already sufficient) output from the immediately following 2x TRANSFORM_OUTCOME_UNORDERED_MATCH and 1x TRANSFORM_OUTCOME_EMPTY_RESULT compares. This way the analysis of issues would be much clearer/simpler, and we would not loose any functionality.