I think that I'd like to try a SELECT/INSERT directly into revert_list, rather than using triggers. Thoughts?
On Tue, May 17, 2011 at 11:02, Greg Stein <gst...@gmail.com> wrote: > Philip: can you review my approach here? > > > On Tue, May 17, 2011 at 10:59, <gst...@apache.org> wrote: >> Author: gstein >> Date: Tue May 17 14:59:21 2011 >> New Revision: 1104308 >> >> URL: http://svn.apache.org/viewvc?rev=1104308&view=rev >> Log: >> Fix issue #3859 by rearranging the order of database operations, so that >> the triggers fire in a different order. This allows us to ensure that >> changes to NODES has precedence over changes to ACTUAL_NODE. >> >> * subversion/libsvn_wc/wc_db.c: >> (op_revert_txn, op_revert_recursive_txn): move the ACTUAL_NODE changes >> further up in the function and leave a comment about why. >> >> * subversion/tests/cmdline/revert_tests.py: >> (revert_empty_actual): remove XFail marker >> >> Modified: >> subversion/trunk/subversion/libsvn_wc/wc_db.c >> subversion/trunk/subversion/tests/cmdline/revert_tests.py >> >> Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c >> URL: >> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1104308&r1=1104307&r2=1104308&view=diff >> ============================================================================== >> --- subversion/trunk/subversion/libsvn_wc/wc_db.c (original) >> +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Tue May 17 14:59:21 2011 >> @@ -5417,6 +5417,21 @@ op_revert_txn(void *baton, >> op_depth = svn_sqlite__column_int64(stmt, 0); >> SVN_ERR(svn_sqlite__reset(stmt)); >> >> + /* Handle ACTUAL_NODE first. We want any changes to NODES, which fire >> + triggers to update 'revert_list', to take precedence over these >> + changes to ACTUAL_NODE. */ >> + SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb, >> + >> STMT_DELETE_ACTUAL_NODE_LEAVING_CHANGELIST)); >> + SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath)); >> + SVN_ERR(svn_sqlite__update(&affected_rows, stmt)); >> + if (!affected_rows) >> + { >> + SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb, >> + >> STMT_CLEAR_ACTUAL_NODE_LEAVING_CHANGELIST)); >> + SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath)); >> + SVN_ERR(svn_sqlite__step_done(stmt)); >> + } >> + >> if (op_depth > 0 && op_depth == relpath_depth(local_relpath)) >> { >> /* Can't do non-recursive revert if children exist */ >> @@ -5456,18 +5471,6 @@ op_revert_txn(void *baton, >> SVN_ERR(svn_sqlite__step_done(stmt)); >> } >> >> - SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb, >> - >> STMT_DELETE_ACTUAL_NODE_LEAVING_CHANGELIST)); >> - SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath)); >> - SVN_ERR(svn_sqlite__update(&affected_rows, stmt)); >> - if (!affected_rows) >> - { >> - SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb, >> - >> STMT_CLEAR_ACTUAL_NODE_LEAVING_CHANGELIST)); >> - SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath)); >> - SVN_ERR(svn_sqlite__update(&affected_rows, stmt)); >> - } >> - >> return SVN_NO_ERROR; >> } >> >> @@ -5526,12 +5529,9 @@ op_revert_recursive_txn(void *baton, >> if (!op_depth) >> op_depth = 1; /* Don't delete BASE nodes */ >> >> - SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb, >> - STMT_DELETE_NODES_RECURSIVE)); >> - SVN_ERR(svn_sqlite__bindf(stmt, "isi", wcroot->wc_id, >> - local_relpath, op_depth)); >> - SVN_ERR(svn_sqlite__step_done(stmt)); >> - >> + /* Handle ACTUAL_NODE first. We want any changes to NODES, which fire >> + triggers to update 'revert_list', to take precedence over these >> + changes to ACTUAL_NODE. */ >> SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb, >> >> STMT_DELETE_ACTUAL_NODE_LEAVING_CHANGELIST_RECURSIVE)); >> SVN_ERR(svn_sqlite__bindf(stmt, "iss", wcroot->wc_id, >> @@ -5544,6 +5544,12 @@ op_revert_recursive_txn(void *baton, >> local_relpath, like_arg)); >> SVN_ERR(svn_sqlite__step_done(stmt)); >> >> + SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb, >> + STMT_DELETE_NODES_RECURSIVE)); >> + SVN_ERR(svn_sqlite__bindf(stmt, "isi", wcroot->wc_id, >> + local_relpath, op_depth)); >> + SVN_ERR(svn_sqlite__step_done(stmt)); >> + >> /* ### This removes the locks, but what about the access batons? */ >> SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb, >> STMT_DELETE_WC_LOCK_ORPHAN_RECURSIVE)); >> >> Modified: subversion/trunk/subversion/tests/cmdline/revert_tests.py >> URL: >> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/revert_tests.py?rev=1104308&r1=1104307&r2=1104308&view=diff >> ============================================================================== >> --- subversion/trunk/subversion/tests/cmdline/revert_tests.py (original) >> +++ subversion/trunk/subversion/tests/cmdline/revert_tests.py Tue May 17 >> 14:59:21 2011 >> @@ -1261,7 +1261,7 @@ def revert_nested_add_depth_immediates(s >> expected_status.remove('A/X', 'A/X/Y') >> svntest.actions.run_and_verify_status(wc_dir, expected_status) >> >> -@XFail() >> + >> @Issue(3859) >> def revert_empty_actual(sbox): >> "revert with superfluous actual node" >> >> >> >
