preserve order of SET clauses based on update values ordering

Issue #3541 resolved
Gorka Eguileor created an issue

In some SQL DBs the UPDATE operation is order dependent, so the operation behaves differently depending on the order of the values.

As an example, a volumes table with columns 'status' and 'previous_status' and we want to update a volume that has 'available' in the status column.

If the SQL query is generated as:

UPDATE volumes SET previous_status=status, status='new' WHERE id=1;

This will result in a volume with 'new' status and 'available' previous_status both on SQLite and MariaDB, but if columns get reversed and the query is generated as:

UPDATE volumes SET status='new', previous_status=status WHERE id=1;

We will get the same result in SQLite but will result in a volume with status and previous_status set to 'new' in MariaDB, which is not what we want.

And this cannot be selected when calling update or values on update().

Comments (5)

  1. Mike Bayer repo owner

    Normally I'd not be too enthused about this kind of thing, as this is the most vanishingly small number of use cases and imposes a not quite trivial amount of new complexity on some of the most essential bits of the Core. If a program really needs a certain ordering to a SET clause in an UPDATE, that's usually going to be just one UPDATE in the whole app doing this special thing and they're usually going to write it as a string; after all this is already a super-database-specific behavior.

    But, you've provided this amazing pull request, where you got tests and everything going, so that is enormously helpful. I am concerned about the performance overhead of OrderedDict inside of CRUD complication so I'm going to propose that we make the use of that data structure conditional on the keep_order flag as well; I'm surprised that you didn't see failures in test/aaa_profiling/test_compiler.py with these changes? Also may I propose that we move "keep_ordering" up to UpdateClause itself and make it such that it only sets if the user provides an ordered sequence of values? That is, don't assume any dictionaries are ordered.

  2. Mike Bayer repo owner

    Reviewing the functionality here, the SET clause right now orders in terms of the Table->Column ordering; the ordering is deterministic. So this feature is to provide for an alternate ordering that derives from a sequence of tuples passed to values() instead of a Python mapping (e.g. dictionary).

  3. Mike Bayer repo owner

    Preserve order in update method

    In some DBs the UPDATE operation is order dependent, so the operation behaves differently depending on the order of the values.

    As an example, imagine a volumes table with columns 'status' and 'previous_status' and we want to update a volume that has 'available' in the status column.

    If the SQL query is performed as:

    UPDATE volumes SET previous_status=status, status='new' WHERE id=1;

    This will result in a volume with 'new' status and 'available' previous_status both on SQLite and MariaDB, but if we reverse the columns:

    UPDATE volumes SET status='new', previous_status=status WHERE id=1;

    We will get the same result in SQLite but will result in a volume with status and previous_status set to 'new' in MariaDB, which is not what we want.

    So order must be taken into consideration in some cases and it should be allowed to ve specified via the Query update method or the values method of an update.

    This patch fixes this issue by preserving the order of parameters in updates and allowing to receive not only dictionaries in update and values but also ordered dictionaries and list/tuples of value pairs (like dict and OrderedDict do).

    fixes #3541

    → <<cset fc73036865a0>>

  4. Mike Bayer repo owner

    Preserve order in update method

    In some DBs the UPDATE operation is order dependent, so the operation behaves differently depending on the order of the values.

    As an example, imagine a volumes table with columns 'status' and 'previous_status' and we want to update a volume that has 'available' in the status column.

    If the SQL query is performed as:

    UPDATE volumes SET previous_status=status, status='new' WHERE id=1;

    This will result in a volume with 'new' status and 'available' previous_status both on SQLite and MariaDB, but if we reverse the columns:

    UPDATE volumes SET status='new', previous_status=status WHERE id=1;

    We will get the same result in SQLite but will result in a volume with status and previous_status set to 'new' in MariaDB, which is not what we want.

    So order must be taken into consideration in some cases and it should be allowed to ve specified via the Query update method or the values method of an update.

    This patch fixes this issue by preserving the order of parameters in updates and allowing to receive not only dictionaries in update and values but also ordered dictionaries and list/tuples of value pairs (like dict and OrderedDict do).

    fixes #3541

    → <<cset fc73036865a0>>

  5. Log in to comment