#324 HeidiSQL updates too many rows

[Anonymous]

Originally created by: ansgar.b...@wolterskluwer.de
Originally owned by: a...@anse.de

Reported as bug #1916578 by nobody@sf on 2008-03-17 04:02

#in the query editor:
SELECT * FROM `tablename` WHERE id=12345;

#in the result box i changed some data, so the query
#looks fine and is okay
UPDATE `tablename` SET `fieldname1`='anything1'
`fieldname2`='anything2' WHERE id=12345

#....but...here it goes wrong:
#in the query editor:
SELECT `fieldname` FROM `tablename` WHERE id=12345;

#in the result box i changed some data,but
#where´s the records ID -> 12345???
UPDATE `tablename` SET `fieldname`=0 WHERE `fieldname`=1

That query affects to all records where the fieldname is '1', not only the
record with ID 12345

Please inform me if you solve the bug, thank.
schmalle@gmx.net

[Anonymous]

Originally posted by: rosenfie...@gmail.com

(No comment was entered for this change.)

Labels: -priority-medium Severity-Broken

[Anonymous]

Originally posted by: a...@anse.de

(No comment was entered for this change.)

Labels: -migrated

[Anonymous]

Originally posted by: rosenfie...@gmail.com

Confirmed.

=== Test case ===

a)
CREATE TABLE test.test (
  Id tinyint NOT NULL,
  Data varchar(255),
  PRIMARY KEY (Id)
);

b)
INSERT INTO test.test VALUES (1, null), (2, null), (3, null);

c)
SELECT Data FROM test.test;

d)
Change one of the field values in the grid.

e)
The following update is generated by HeidiSQL:
UPDATE `test` SET `Data`='whatever' WHERE `Data` IS NULL

f)
Three rows are updated, which is two too many.  Also, the grid does not reflect
this, and no popup warns of the error.

Summary: HeidiSQL updates too many rows
Labels: -Severity-Broken Severity-Corrupts

[Anonymous]

Originally posted by: a...@anse.de

An emergency workaround for this would be to disable editing in the query grid but
that would unfortunately remove a really nice feature...

[Anonymous]

Originally posted by: rosenfie...@gmail.com

In other database servers, this would be implemented using cursors: Select the data,
seek to a specific row number using cursors, and tell the server to update a field
to a new value, and the server figures out where the original data is and updates
only that.

There's no such feature in MySQL.

Zeos knows which original table and column an altered field came from, though.  In
theory, Zeos could do a SHOW KEYS on the original table of each altered field, and
decide whether there are enough primary key parts (read: all of them) from that
table in the current grid data to do a proper update.  Problem solved?

There is another problem, which is: what if fields from two different tables are
modified at the same time?  Zeos would need to do the above check twice, once for
each table.  Plus it would need to generate two update statements; currently it can
make only one statement.  That would need solving too.

Then a third problem crops up; what does Zeos do when the user is adding a row, and
the new row successfully inserts in table 1, but cannot be inserted in table 2?  The
grid knows only two results from a Post() action, success or failure.  So Zeos has
to delete the already inserted data and then abort.  Fair enough..

But we're thinking way ahead down that code path, far from the original problem
described here.  Let's back up a little, and go back to the part where Zeos (in
theory) looks at the 'original table' and 'original column' information.  Because
there is another issue too.  I have a feeling that people would complain if Zeos did
a "SHOW KEYS FROM <..>" for each SELECT they did using the regular data grid.  So
Zeos would need to be selective, and in the case where a "SELECT * FROM <..>" is
performed, just assume that the asterisk means it has got all PK part columns from
that table (those that MySQL has kindly marked with the 'is part of primary key'
flag) and avoid doing the SHOW KEYS.

That should just about cover all the changes needed to make this work properly, I
guess.

[Anonymous]

Originally posted by: rosenfie...@gmail.com

Relevant code in ZDbcGenericResolver.pas:

PostUpdates():
Generates and executes one SQL query, where it in some situations needs to either
abort early, or do something much more advanced.

FormUpdateStatement():
(Takes part in generating that one SQL query, so may need modifications too.)

DefineWhereKeyColumns():
Called by FormUpdateStatement and friends.  Supposedly figures out the relevant
primary key columns for a given table.  As seen in this issue and the test case,
this is severely broken.

Assuming that an abort has already happened if multiple original tables are
involved, this function would need to find the original table for all updated
fields, find all key columns from those tables, find out if field values are
available in the current row for all of those, and abort if not.

[Anonymous]

Originally posted by: rosenfie...@gmail.com

Perhaps a few more test cases should be added before someone digs into the code. 
For example:

- test updates to two tables at once, all key fields present
- test updates to two tables at once, key fields from one table fully present, only
some key fields from other table present
- test updates to two tables at once, no key fields present
- test updates to grid, data from SELECT * query.

[Anonymous]

Originally posted by: a...@anse.de

Test results:

*** table 1:
CREATE TABLE `test1` (
  `id` int(10) unsigned NOT NULL,
  `title` varchar(255) COLLATE latin1_german1_ci DEFAULT NULL,
  PRIMARY KEY (`id`)
) ENGINE=MyISAM DEFAULT CHARSET=latin1 COLLATE=latin1_german1_ci

*** table 2:
CREATE TABLE `test2` (
  `refid` int(11) DEFAULT NULL,
  `somefield` varchar(50) DEFAULT NULL
) ENGINE=MyISAM DEFAULT CHARSET=latin1

Switching to Query tab and executing different joins:

*** 1) Updates to two tables at once, PK and one other field of table 1 present
SELECT id, title FROM test1, test2 WHERE test2.refid = test1.id
=> All updates work as they should. Doesn't matter if the selected fields come from
table 1 or 2 unless they're not mixed.

*** 2) Same as 1), now using table aliases:
SELECT id, title FROM test1 t1, test2 t2 WHERE t2.refid = t1.id
=> Updates not working. Zeos fires a "SHOW KEYS FROM `t1`" which results in "SQL
error: Table t1 doesn't exist".
=> Severity: Silly but doesn't corrupt data.

*** 3) Same as 1), plus selecting a field from table 2:
SELECT id, title, refid FROM test1, test2 WHERE test2.refid = test1.id
=> Updates are blocked by Zeos before anything is posted to the server: "Cannot
update a complex query with more than one table". Doesn't matter if the selected
fields are key fields or not.
=> Severity: Looks ok so far, some safety blocker,

*** 4) Same as 1) but without PK of table 1 present:
SELECT title FROM test1, test2 WHERE test2.refid = test1.id
or with only 1 table and same effect:
SELECT title FROM test1
=> Updates are broken. WHERE clause ignores PK, only uses the present field: "UPDATE
`test1` SET `title`='foobar' WHERE `title`='foo'" This is the case which was
initially reported here.
=> Severity: Highest, can corrupt data silently.

*** 5) Simple query selecting "virtual" data:
SELECT 'foo'
=> OK. Fields block modification their content.

Hope I didn't miss an important case.

[Anonymous]

Originally posted by: a...@anse.de

Testcase 4) also applies to the Data tab grid: If you deselect the primary key
column of a table via column selection drop down, UPDATES only include present
fields in the WHERE clause. Will be fixed along with fixing issue 618.

[Anonymous]

Originally posted by: a...@anse.de

Fixing this issue will unfortunately include making the query grid readonly. Would
need some extensive parser routine. However, the data tab grid will get some nice
new editors for date fields, memos and maybe more.

[Anonymous]

Originally posted by: a...@anse.de

The data tab grid should block editing if there is no primary or unique key present,
including some meaningful alert box. Discussed some time ago with a poll. Anyway,
teaches users to enhance their database design.

[Anonymous]

Originally posted by: a...@anse.de

[r1604] should fix this. However, I see the need for further testing before closing
this issue.

[Anonymous]

Originally posted by: a...@anse.de

Closing. Looks stable now.

Status: Fixed

[Anonymous]

Originally posted by: djony.rsl@gmail.com

(No comment was entered for this change.)