From: Mike Bayer Date: Mon, 18 Aug 2014 16:50:29 +0000 (-0400) Subject: - major simplification of _collect_update_commands. in particular, X-Git-Tag: rel_1_0_0b1~216^2~4 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=d39927ec20dd0b66f4ab3aab3e4e67b3814186ce;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - major simplification of _collect_update_commands. in particular, we only call upon the history API fully for primary key columns. We also now skip the whole step of looking at PK columns and using any history at all if no net changes are detected on the object. --- diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index 1e12918570..14dc5d7f87 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -1893,6 +1893,19 @@ class Mapper(InspectionAttr): """ + @_memoized_configured_property + def _propkey_to_col(self): + return dict( + ( + table, + dict( + (self._columntoproperty[col].key, col) + for col in columns + ) + ) + for table, columns in self._cols_by_table.items() + ) + @_memoized_configured_property def _col_to_propkey(self): return dict( diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py index d511c0816c..228cfef3aa 100644 --- a/lib/sqlalchemy/orm/persistence.py +++ b/lib/sqlalchemy/orm/persistence.py @@ -304,96 +304,70 @@ def _collect_update_commands(base_mapper, uowtransaction, params = {} value_params = {} - hasdata = hasnull = False + propkey_to_col = mapper._propkey_to_col[table] - for col in mapper._cols_by_table[table]: - if col is mapper.version_id_col: - params[col._label] = \ - mapper._get_committed_state_attr_by_column( - row_switch or state, - row_switch and row_switch.dict - or state_dict, - col) + for propkey in set(propkey_to_col).intersection(state.committed_state): + value = state_dict[propkey] + col = propkey_to_col[propkey] - prop = mapper._columntoproperty[col] - history = state.manager[prop.key].impl.get_history( - state, state_dict, attributes.PASSIVE_NO_INITIALIZE - ) - if history.added: - params[col.key] = history.added[0] - hasdata = True + if not state.manager[propkey].impl.is_equal( + value, state.committed_state[propkey]): + if isinstance(value, sql.ClauseElement): + value_params[col] = value else: - if mapper.version_id_generator is not False: - val = mapper.version_id_generator(params[col._label]) - params[col.key] = val - - # HACK: check for history, in case the - # history is only - # in a different table than the one - # where the version_id_col is. - for prop in mapper._columntoproperty.values(): - history = ( - state.manager[prop.key].impl.get_history( - state, state_dict, - attributes.PASSIVE_NO_INITIALIZE)) - if history.added: - hasdata = True + params[col.key] = value + + if mapper.version_id_col is not None: + col = mapper.version_id_col + params[col._label] = \ + mapper._get_committed_state_attr_by_column( + row_switch if row_switch else state, + row_switch.dict if row_switch else state_dict, + col) + + if col.key not in params and \ + mapper.version_id_generator is not False: + val = mapper.version_id_generator(params[col._label]) + params[col.key] = val + + if not (params or value_params): + continue + + pk_params = {} + for col in pks: + propkey = mapper._columntoproperty[col].key + history = state.manager[propkey].impl.get_history( + state, state_dict, attributes.PASSIVE_OFF) + + if row_switch and not history.deleted and history.added: + # row switch present. convert a row that thought + # it would be an INSERT into an UPDATE, by removing + # the PK value from the SET clause and instead putting + # it in the WHERE clause. + del params[col.key] + pk_params[col._label] = history.added[0] + elif history.added: + # we're updating the PK value. + assert history.deleted, ( + "New PK value without an old one not " + "possible for an UPDATE") + # check if an UPDATE of the PK value + # has already occurred as a result of ON UPDATE CASCADE. + # If so, use the new value to locate the row. + if ("pk_cascaded", state, col) in uowtransaction.attributes: + pk_params[col._label] = history.added[0] + else: + # else, use the old value to locate the row + pk_params[col._label] = history.deleted[0] else: - prop = mapper._columntoproperty[col] - history = state.manager[prop.key].impl.get_history( - state, state_dict, - attributes.PASSIVE_OFF if col in pks else - attributes.PASSIVE_NO_INITIALIZE) - if history.added: - if isinstance(history.added[0], - sql.ClauseElement): - value_params[col] = history.added[0] - else: - value = history.added[0] - params[col.key] = value - - if col in pks: - if history.deleted and \ - not row_switch: - # if passive_updates and sync detected - # this was a pk->pk sync, use the new - # value to locate the row, since the - # DB would already have set this - if ("pk_cascaded", state, col) in \ - uowtransaction.attributes: - value = history.added[0] - params[col._label] = value - else: - # use the old value to - # locate the row - value = history.deleted[0] - params[col._label] = value - hasdata = True - else: - # row switch logic can reach us here - # remove the pk from the update params - # so the update doesn't - # attempt to include the pk in the - # update statement - del params[col.key] - value = history.added[0] - params[col._label] = value - if value is None: - hasnull = True - else: - hasdata = True - elif col in pks: - value = history.unchanged[0] - if value is None: - hasnull = True - params[col._label] = value + pk_params[col._label] = history.unchanged[0] - if hasdata: - if hasnull: + if params or value_params: + if None in pk_params.values(): raise orm_exc.FlushError( - "Can't update table " - "using NULL for primary " + "Can't update table using NULL for primary " "key value") + params.update(pk_params) update.append((state, state_dict, params, mapper, connection, value_params)) return update diff --git a/test/orm/test_dynamic.py b/test/orm/test_dynamic.py index bc47ba3f3c..950ff19539 100644 --- a/test/orm/test_dynamic.py +++ b/test/orm/test_dynamic.py @@ -509,10 +509,6 @@ class UOWTest( self.assert_sql_execution( testing.db, sess.flush, - CompiledSQL( - "SELECT users.id AS users_id, users.name AS users_name " - "FROM users WHERE users.id = :param_1", - lambda ctx: [{"param_1": u1_id}]), CompiledSQL( "SELECT addresses.id AS addresses_id, addresses.email_address " "AS addresses_email_address FROM addresses " @@ -523,7 +519,11 @@ class UOWTest( "UPDATE addresses SET user_id=:user_id WHERE addresses.id = " ":addresses_id", lambda ctx: [{'addresses_id': a2_id, 'user_id': None}] - ) + ), + CompiledSQL( + "SELECT users.id AS users_id, users.name AS users_name " + "FROM users WHERE users.id = :param_1", + lambda ctx: [{"param_1": u1_id}]), ) def test_rollback(self): diff --git a/test/orm/test_unitofworkv2.py b/test/orm/test_unitofworkv2.py index 122fe2514b..374a77237c 100644 --- a/test/orm/test_unitofworkv2.py +++ b/test/orm/test_unitofworkv2.py @@ -1277,6 +1277,8 @@ class RowswitchAccountingTest(fixtures.MappedTest): old = attributes.get_history(p3, 'child')[2][0] assert old in sess + # essentially no SQL should emit here, + # because we've replaced the row with another identical one sess.flush() assert p3.child._sa_instance_state.session_id == sess.hash_key