Woah!
Still catching up here, but I don't think you should be using LIKE in
*any* SQL statement. That is a yellow flag right there, for more
investigation. For the most part, it says "do a full table scan". Bad
bad bad.
Looking at the code, I think what you really want is to examine
parent_relpath, NOT do a LIKE clause.
Right?
Cheers,
-g
On Thu, Jun 25, 2009 at 21:02, Hyrum K. Wright <hyrum_at_hyrumwright.org> wrote:
>
> Author: hwright
> Date: Thu Jun 25 12:02:05 2009
> New Revision: 38202
>
> Log:
> Simplify a little bit of the relocate sql updating logic. Also, properly
> escape URLs being passed into a 'like' clause.
>
> * subversion/libsvn_wc/wc_db.c
> (statement_keys, statements): Remove and refresh a few statements.
> (LIKE_ESCAPE_CHAR): New.
> (escape_sqlite_like): New.
> (relocate_txn): Use the improved SQL statements, and be sure to escape
> URLs used in like clauses.
>
> Modified:
> trunk/subversion/libsvn_wc/wc_db.c
>
> Modified: trunk/subversion/libsvn_wc/wc_db.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/wc_db.c?pathrev=38202&r1=38201&r2=38202
> ==============================================================================
> --- trunk/subversion/libsvn_wc/wc_db.c Thu Jun 25 08:17:12 2009 (r38201)
> +++ trunk/subversion/libsvn_wc/wc_db.c Thu Jun 25 12:02:05 2009 (r38202)
> @@ -190,13 +190,15 @@ enum statement_keys {
> STMT_SELECT_DELETION_INFO,
> STMT_SELECT_PARENT_STUB_INFO,
> STMT_DELETE_LOCK,
> - STMT_UPDATE_BASE_REPO,
> - STMT_UPDATE_BASE_CHILDREN_REPO,
> - STMT_UPDATE_WORKING_COPYFROM_REPO,
> - STMT_UPDATE_WORKING_CHILDREN_COPYFROM_REPO,
> + STMT_UPDATE_BASE_RECURSIVE_REPO,
> + STMT_UPDATE_WORKING_RECURSIVE_COPYFROM_REPO,
> STMT_UPDATE_LOCK_REPOS_ID
> };
>
> +/* This is a character used to escape itself and the globbing character in
> + globbing sql expressions below. See escape_sqlite_glob(). */
> +#define LIKE_ESCAPE_CHAR "#"
> +
> static const char * const statements[] = {
> /* STMT_SELECT_BASE_NODE */
> "select wc_id, local_relpath, repos_id, repos_relpath, "
> @@ -332,26 +334,23 @@ static const char * const statements[] =
> "delete from lock "
> "where repos_id = ?1 and repos_relpath = ?2;",
>
> - /* STMT_UPDATE_BASE_REPO */
> - "update base_node set repos_id = ?3 "
> - "where wc_id = ?1 and local_relpath = ?2;",
> -
> - /* STMT_UPDATE_BASE_CHILDREN_REPO */
> - "update base_node set repos_id = ?3 "
> - "where repos_id is not null and wc_id = ?1 and parent_relpath like ?2;",
> -
> - /* STMT_UPDATE_WORKING_COPYFROM_REPO */
> - "update working_node set copyfrom_repos_id = ?3 "
> - "where copyfrom_repos_id is not null and wc_id = ?1 and local_relpath = ?2;",
> -
> - /* STMT_UPDATE_WORKING_CHILDREN_COPYFROM_REPO */
> - "update working_node set copyfrom_repos_id = ?3 "
> - "where copyfrom_repos_id is not null and wc_id = ?1 "
> - " and parent_relpath like ?2;",
> + /* STMT_UPDATE_BASE_RECURSIVE_REPO */
> + "update base_node set repos_id = ?4 "
> + "where repos_id is not null and wc_id = ?1 and "
> + " (local_relpath = ?2 or "
> + " local_relpath like ?3 escape '" LIKE_ESCAPE_CHAR "');",
> +
> + /* STMT_UPDATE_WORKING_RECURSIVE_COPYFROM_REPO */
> + "update working_node set copyfrom_repos_id = ?4 "
> + "where copyfrom_repos_id is not null and wc_id = ?1 and "
> + " (local_relpath = ?2 or "
> + " local_relpath like ?3 escape '" LIKE_ESCAPE_CHAR "');",
>
> /* STMT_UPDATE_LOCK_REPOS_ID */
> - "update lock set repos_id = ?3 "
> - "where repos_id = ?1 and repos_relpath like ?2;",
> + "update lock set repos_id = ?4 "
> + "where repos_id = ?1 and "
> + " (repos_relpath = ?2 or "
> + " repos_relpath like ?3 escape '" LIKE_ESCAPE_CHAR "');",
>
> NULL
> };
> @@ -485,6 +484,42 @@ get_translated_size(svn_sqlite__stmt_t *
> return svn_sqlite__column_int64(stmt, slot);
> }
>
> +static const char *
> +escape_sqlite_like(const char * const str, apr_pool_t *result_pool)
> +{
> + char *result;
> + const char *old_ptr;
> + char *new_ptr;
> + int len = 0;
> +
> + /* Count the number of extra characters we'll need in the escaped string.
> + We could just use the worst case (double) value, but we'd still need to
> + iterate over the string to get it's length. So why not do something
> + useful why iterating over it, and save some memory at the same time? */
> + for (old_ptr = str; *old_ptr; ++old_ptr)
> + {
> + len++;
> + if (*old_ptr == '%'
> + || *old_ptr == '_'
> + || *old_ptr == LIKE_ESCAPE_CHAR[0])
> + len++;
> + }
> +
> + result = apr_pcalloc(result_pool, len + 1);
> +
> + /* Now do the escaping. */
> + for (old_ptr = str, new_ptr = result; *old_ptr; ++old_ptr, ++new_ptr)
> + {
> + if (*old_ptr == '%'
> + || *old_ptr == '_'
> + || *old_ptr == LIKE_ESCAPE_CHAR[0])
> + *(new_ptr++) = LIKE_ESCAPE_CHAR[0];
> + *new_ptr = *old_ptr;
> + }
> +
> + return result;
> +}
> +
>
> /* Construct a new wcroot_t. The WCROOT_ABSPATH and SDB parameters must
> have lifetime of at least RESULT_POOL. */
> @@ -3083,6 +3118,7 @@ static svn_error_t *
> relocate_txn(void *baton, svn_sqlite__db_t *sdb)
> {
> struct relocate_baton *rb = baton;
> + const char *like_arg;
> apr_pool_t *scratch_pool = rb->scratch_pool;
> svn_sqlite__stmt_t *stmt;
> apr_int64_t new_repos_id;
> @@ -3098,51 +3134,41 @@ relocate_txn(void *baton, svn_sqlite__db
> SVN_ERR(create_repos_id(&new_repos_id, rb->repos_root_url, rb->repos_uuid,
> sdb, scratch_pool));
>
> - if (rb->have_base_node)
> - {
> - /* Update the BASE_NODE.repos_id. */
> - SVN_ERR(svn_sqlite__get_statement(&stmt, sdb, STMT_UPDATE_BASE_REPO));
> - SVN_ERR(svn_sqlite__bindf(stmt, "isi", rb->wc_id, rb->local_relpath,
> - new_repos_id));
> -
> - SVN_ERR(svn_sqlite__step_done(stmt));
> - }
> -
> - /* Update a non-NULL WORKING_NODE.copyfrom_repos_id. */
> - SVN_ERR(svn_sqlite__get_statement(&stmt, sdb,
> - STMT_UPDATE_WORKING_COPYFROM_REPO));
> - SVN_ERR(svn_sqlite__bindf(stmt, "isi", rb->wc_id, rb->local_relpath,
> - new_repos_id));
> - SVN_ERR(svn_sqlite__step_done(stmt));
> + if (rb->local_relpath[0] == 0)
> + like_arg = "%";
> + else
> + like_arg = apr_psprintf(scratch_pool, "%s/%%",
> + escape_sqlite_like(rb->local_relpath, scratch_pool));
>
> - /* Update and child working nodes. */
> + /* Update non-NULL WORKING_NODE.copyfrom_repos_id. */
> SVN_ERR(svn_sqlite__get_statement(&stmt, sdb,
> - STMT_UPDATE_WORKING_CHILDREN_COPYFROM_REPO));
> - SVN_ERR(svn_sqlite__bindf(stmt, "isi", rb->wc_id,
> - apr_psprintf(scratch_pool, "%s/%%",
> - rb->local_relpath),
> - new_repos_id));
> + STMT_UPDATE_WORKING_RECURSIVE_COPYFROM_REPO));
> + SVN_ERR(svn_sqlite__bindf(stmt, "issi", rb->wc_id, rb->local_relpath,
> + like_arg, new_repos_id));
> SVN_ERR(svn_sqlite__step_done(stmt));
>
> /* Do a bunch of stuff which is conditional on us actually having a
> base_node in the first place. */
> if (rb->have_base_node)
> {
> - /* Update any children which have non-NULL repos_id's */
> + /* Update any BASE which have non-NULL repos_id's */
> SVN_ERR(svn_sqlite__get_statement(&stmt, sdb,
> - STMT_UPDATE_BASE_CHILDREN_REPO));
> - SVN_ERR(svn_sqlite__bindf(stmt, "isi", rb->wc_id, rb->local_relpath,
> - new_repos_id));
> + STMT_UPDATE_BASE_RECURSIVE_REPO));
> + SVN_ERR(svn_sqlite__bindf(stmt, "issi", rb->wc_id, rb->local_relpath,
> + like_arg, new_repos_id));
> SVN_ERR(svn_sqlite__step_done(stmt));
>
> /* Update any locks for the root or its children. */
> + if (rb->repos_relpath[0] == 0)
> + like_arg = "%";
> + else
> + like_arg = apr_psprintf(scratch_pool, "%s/%%",
> + escape_sqlite_like(rb->repos_relpath, scratch_pool));
> +
> SVN_ERR(svn_sqlite__get_statement(&stmt, sdb,
> STMT_UPDATE_LOCK_REPOS_ID));
> - SVN_ERR(svn_sqlite__bindf(stmt, "isi",
> - rb->old_repos_id,
> - apr_psprintf(scratch_pool, "%s/%%",
> - rb->repos_relpath),
> - new_repos_id));
> + SVN_ERR(svn_sqlite__bindf(stmt, "issi", rb->old_repos_id,
> + rb->repos_relpath, like_arg, new_repos_id));
> SVN_ERR(svn_sqlite__step_done(stmt));
> }
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=2365439
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2365537
Received on 2009-06-26 01:55:16 CEST