Author: rhuijben
Date: Thu May 12 14:25:36 2011
New Revision: 1102325

URL: http://svn.apache.org/viewvc?rev=1102325&view=rev
Log:
Answer the question "Does this node have switched descendants?" completely in
SQL instead of in a C function that tried to do things with the working copy
root and then has to fix up on common cases, while it should really only
operate on the selected subtree.

The resulting SQLite code is not pretty, but it does the job...
(The different cases could use a few more testcases though).

* subversion/libsvn_wc/revision_status.c
  (svn_wc_revision_status2): Use pcalloc. The struct is documented to be
    extendable later.

* subversion/libsvn_wc/wc-queries.sql
  (STMT_SELECT_SWITCHED_NODES): Remove statement.
  (STMT_SELECT_UNSWITCHED_NODES): Remove statement.

  (STMT_HAS_SWITCHED,
   STMT_HAS_SWITCHED_REPOS_ROOT,
   STMT_HAS_SWITCHED_WCROOT,
   STMT_HAS_SWITCHED_WCROOT_REPOS_ROOT): New statements.

* subversion/libsvn_wc/wc_db.c
  (has_switched_subtree): Do the cheap check first as we have to obtain that
    information anyway. Then call the right switch check function depending on
    whether the origin node is the root of the working copy, the repository or
    both.

Modified:
    subversion/trunk/subversion/libsvn_wc/revision_status.c
    subversion/trunk/subversion/libsvn_wc/wc-queries.sql
    subversion/trunk/subversion/libsvn_wc/wc_db.c

Modified: subversion/trunk/subversion/libsvn_wc/revision_status.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/revision_status.c?rev=1102325&r1=1102324&r2=1102325&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/revision_status.c (original)
+++ subversion/trunk/subversion/libsvn_wc/revision_status.c Thu May 12 14:25:36 
2011
@@ -42,7 +42,7 @@ svn_wc_revision_status2(svn_wc_revision_
                         apr_pool_t *result_pool,
                         apr_pool_t *scratch_pool)
 {
-  svn_wc_revision_status_t *result = apr_palloc(result_pool, sizeof(*result));
+  svn_wc_revision_status_t *result = apr_pcalloc(result_pool, sizeof(*result));
 
   *result_p = result;
 

Modified: subversion/trunk/subversion/libsvn_wc/wc-queries.sql
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc-queries.sql?rev=1102325&r1=1102324&r2=1102325&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc-queries.sql (original)
+++ subversion/trunk/subversion/libsvn_wc/wc-queries.sql Thu May 12 14:25:36 
2011
@@ -1137,28 +1137,73 @@ WHERE wc_id = ?1 AND (local_relpath = ?2
   AND properties IS NOT NULL
 LIMIT 1
 
-/* Find nodes that are switched relative to the wc root.
-   This query expects repos_path(wcroot)/% as arg 4,
-   and repos_path(wcroot), with a slash appended unless the path is empty,
-   as arg 5. */
--- STMT_SELECT_SWITCHED_NODES
-SELECT local_relpath, repos_path FROM nodes_base
-WHERE wc_id = ?1 AND local_relpath != ""
-  AND op_depth = 0
-  AND (local_relpath = ?2 OR local_relpath LIKE ?3 ESCAPE '#')
-  AND ((repos_path NOT LIKE ?4 ESCAPE '#' AND repos_path != local_relpath)
-       OR (repos_path != ?5 || local_relpath))
-  AND file_external IS NULL
+/* Determine if there is some switched subtree in just SQL. This looks easy,
+   but it really isn't, because we don't have a simple (and optimizable)
+   path join operation in SQL.
+
+   To work around that we have 4 different cases:
+      * Check on a node that is neither wcroot nor repos root
+      * Check on a node that is repos_root, but not wcroot.
+      * Check on a node that is wcroot, but not repos root.
+      * Check on a node that is both wcroot and repo sroot.
+
+   To make things easier, our testsuite is usually in that last category,
+   while normal working copies are almost always in one of the others.
+*/
+-- STMT_HAS_SWITCHED
+SELECT o.repos_path || '/' || SUBSTR(s.local_relpath, LENGTH(?2)+2) AS expected
+       /*,s.local_relpath, s.repos_path, o.local_relpath, o.repos_path*/
+FROM nodes AS o
+LEFT JOIN nodes AS s
+ON o.wc_id = s.wc_id
+   AND s.local_relpath > ?2 || '/' AND s.local_relpath < ?2 || '0'
+   AND s.op_depth = 0
+   AND s.repos_id = o.repos_id
+   AND s.file_external IS NULL
+WHERE o.wc_id = ?1 AND o.local_relpath=?2 AND o.op_depth=0
+  AND s.repos_path != expected
+LIMIT 1
 
-/* Find nodes that are unswitched relative to the wc root.
-   This query expects repos_path(wcroot), with a slash appended unless the
-   path is empty, as arg 4. */
--- STMT_SELECT_UNSWITCHED_NODES
-SELECT local_relpath FROM nodes_base
-WHERE wc_id = ?1 AND local_relpath != ""
-  AND (local_relpath = ?2 OR local_relpath LIKE ?3 ESCAPE '#')
-  AND (repos_path == ?4 || local_relpath)
-  AND file_external IS NULL
+-- STMT_HAS_SWITCHED_REPOS_ROOT
+SELECT SUBSTR(s.local_relpath, LENGTH(?2)+2) AS expected
+       /*,s.local_relpath, s.repos_path, o.local_relpath, o.repos_path*/
+FROM nodes AS o
+LEFT JOIN nodes AS s
+ON o.wc_id = s.wc_id
+   AND s.local_relpath > ?2 || '/' AND s.local_relpath < ?2 || '0'
+   AND s.op_depth = 0
+   AND s.repos_id = o.repos_id
+   AND s.file_external IS NULL
+WHERE o.wc_id = ?1 AND o.local_relpath=?2 AND o.op_depth=0
+  AND s.repos_path != expected
+LIMIT 1
+
+-- STMT_HAS_SWITCHED_WCROOT
+SELECT o.repos_path || '/' || s.local_relpath AS expected
+       /*,s.local_relpath, s.repos_path, o.local_relpath, o.repos_path*/
+FROM nodes AS o
+LEFT JOIN nodes AS s
+ON o.wc_id = s.wc_id
+   AND s.local_relpath != ''
+   AND s.op_depth = 0
+   AND s.repos_id = o.repos_id
+   AND s.file_external IS NULL
+WHERE o.wc_id = ?1 AND o.local_relpath=?2 AND o.op_depth=0
+  AND s.repos_path != expected
+LIMIT 1
+
+-- STMT_HAS_SWITCHED_WCROOT_REPOS_ROOT
+SELECT s.local_relpath AS expected
+       /*,s.local_relpath, s.repos_path, o.local_relpath, o.repos_path*/
+FROM nodes AS o
+LEFT JOIN nodes AS s
+ON o.wc_id = s.wc_id
+   AND s.local_relpath != ''
+   AND s.op_depth = 0
+   AND s.repos_id = o.repos_id
+   AND s.file_external IS NULL
+WHERE o.wc_id = ?1 AND o.local_relpath=?2 AND o.op_depth=0
+  AND s.repos_path != expected
 LIMIT 1
 
 -- STMT_SELECT_BASE_FILES_RECURSIVE

Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1102325&r1=1102324&r2=1102325&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Thu May 12 14:25:36 2011
@@ -11730,136 +11730,80 @@ has_switched_subtrees(svn_boolean_t *is_
                       apr_pool_t *scratch_pool)
 {
   svn_sqlite__stmt_t *stmt;
-  const char *wcroot_repos_relpath;
-  const char *local_relpath_repos_relpath;
-  svn_boolean_t target_is_switched_subtree;
   svn_boolean_t have_row;
+  apr_int64_t repos_id;
+  const char *repos_relpath;
+
+  /* Optional argument handling for caller */
+  if (!is_switched)
+    return SVN_NO_ERROR;
 
   *is_switched = FALSE;
 
-  SVN_ERR(read_info(NULL, NULL, NULL, &wcroot_repos_relpath, NULL,
-                    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
-                    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
-                    NULL, NULL, NULL,
-                    wcroot, "", scratch_pool, scratch_pool));
-
-  SVN_ERR(read_info(NULL, NULL, NULL, &local_relpath_repos_relpath, NULL,
-                    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
-                    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
-                    NULL, NULL, NULL,
-                    wcroot, local_relpath, scratch_pool, scratch_pool));
+  SVN_ERR(base_get_info(NULL, NULL, NULL, &repos_relpath, &repos_id, NULL,
+                        NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+                        wcroot, local_relpath,
+                        scratch_pool, scratch_pool));
 
-  if (local_relpath[0] == '\0' || !local_relpath_repos_relpath)
-    target_is_switched_subtree = FALSE;
-  else
-    target_is_switched_subtree = (strcmp(local_relpath_repos_relpath,
-                                  svn_relpath_join(wcroot_repos_relpath,
-                                                   local_relpath,
-                                                   scratch_pool)) != 0);
+  /* First do the cheap check where we only need info on the origin itself */
+  if (trail_url != NULL)
+    {
+      const char *repos_root_url;
+      const char *url;
+      apr_size_t len1, len2;
 
-  SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
-                                    STMT_SELECT_SWITCHED_NODES));
-  SVN_ERR(svn_sqlite__bindf(stmt, "issss", wcroot->wc_id, local_relpath,
-                            construct_like_arg(local_relpath, scratch_pool),
-                            construct_like_arg(wcroot_repos_relpath,
-                                               scratch_pool),
-                            wcroot_repos_relpath[0] == '\0' ?
-                              "" : apr_pstrcat(scratch_pool,
-                                               wcroot_repos_relpath, "/",
-                                               (char *)NULL)));
-  /* If this query returns a row, some part of the working copy is switched. */
-  SVN_ERR(svn_sqlite__step(&have_row, stmt));
+      /* If the trailing part of the URL of the working copy directory
+         does not match the given trailing URL then the whole working
+         copy is switched. */
 
-  if (have_row)
-    {
-      if (!target_is_switched_subtree)
+      SVN_ERR(fetch_repos_info(&repos_root_url, NULL, wcroot->sdb, repos_id,
+                           scratch_pool));
+      url = svn_path_url_add_component2(repos_root_url, repos_relpath,
+                                        scratch_pool);
+
+      len1 = strlen(trail_url);
+      len2 = strlen(url);
+      if ((len1 > len2) || strcmp(url + len2 - len1, trail_url))
         {
-          /* If LOCAL_RELPATH isn't switched against the wc root but even
-             one of its children is, then we have our answer. */
           *is_switched = TRUE;
-        }
-      else
-        {
-          /* LOCAL_RELPATH is within, or is the root of, a switched subtree.
-             Check if it has any children switched relative to it. */
-          apr_pool_t *iterpool = svn_pool_create(scratch_pool);
-
-          while (have_row)
-            {
-              const char *relpath;
-
-              svn_pool_clear(iterpool);
-              relpath = svn_sqlite__column_text(stmt, 0, iterpool);
-
-              if (strcmp(relpath, local_relpath) != 0)
-                {
-                  const char *child_path = svn_relpath_is_child(local_relpath,
-                                                                relpath,
-                                                                iterpool);
-                  const char *repos_path = svn_sqlite__column_text(stmt, 1,
-                                                                   iterpool);
-
-                  relpath = svn_relpath_join(local_relpath_repos_relpath,
-                                             child_path, iterpool);
-                  if (strcmp(relpath, repos_path) != 0)
-                    {
-                      *is_switched = TRUE;
-                      break;
-                    }
-                }
-              SVN_ERR(svn_sqlite__step(&have_row, stmt));
-            }
-          svn_pool_destroy(iterpool);
+          return SVN_NO_ERROR;
         }
     }
 
-  SVN_ERR(svn_sqlite__reset(stmt));
-
-  /* If LOCAL_RELPATH is itself a switched subtree, any of its subtrees that
-     are switched relative to LOCAL_RELPATH, but not switched relative to the
-     WC root will be missed by STMT_SELECT_SWITCHED_NODES. */
-  if (! *is_switched
-      && target_is_switched_subtree)
+  /* Select the right query based on whether the node is the wcroot, repos root
+     or neither. */
+  if (*local_relpath == '\0')
     {
-        SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
-                                          STMT_SELECT_UNSWITCHED_NODES));
-        SVN_ERR(svn_sqlite__bindf(stmt, "isss",
-                                  wcroot->wc_id,
-                                  local_relpath,
-                                  construct_like_arg(local_relpath,
-                                                     scratch_pool),
-                                  wcroot_repos_relpath[0] == '\0' ?
-                                    "" : apr_pstrcat(scratch_pool,
-                                                     wcroot_repos_relpath,
-                                                     "/",
-                                                     (char *)NULL)));
-        SVN_ERR(svn_sqlite__step(&have_row, stmt));
-        *is_switched = have_row;
-        SVN_ERR(svn_sqlite__reset(stmt));
+      SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
+                        (*repos_relpath == '\0')
+                            ? STMT_HAS_SWITCHED_WCROOT_REPOS_ROOT
+                            : STMT_HAS_SWITCHED_WCROOT));
     }
-
-  if (! *is_switched && trail_url != NULL)
+  else
     {
-      const char *url;
-
-      /* If the trailing part of the URL of the working copy directory
-         does not match the given trailing URL then the whole working
-         copy is switched. */
-      SVN_ERR(read_url(&url, wcroot, local_relpath, scratch_pool,
-                       scratch_pool));
-      if (! url)
-        {
-          *is_switched = TRUE;
-        }
-      else
-        {
-          apr_size_t len1 = strlen(trail_url);
-          apr_size_t len2 = strlen(url);
-          if ((len1 > len2) || strcmp(url + len2 - len1, trail_url))
-            *is_switched = TRUE;
-        }
+      SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
+                        (*repos_relpath == '\0')
+                            ? STMT_HAS_SWITCHED_REPOS_ROOT
+                            : STMT_HAS_SWITCHED));
     }
 
+  SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
+
+  SVN_ERR(svn_sqlite__step(&have_row, stmt));
+  /* ### Please keep this code for a little while or until the code has enough
+         test coverage. These columns are only available in the 4 queries
+         after their selection is uncommented. */
+/*if (have_row)
+    SVN_DBG(("Expected %s for %s, but got %s. Origin=%s with %s\n",
+             svn_sqlite__column_text(stmt, 0, scratch_pool),
+             svn_sqlite__column_text(stmt, 1, scratch_pool),
+             svn_sqlite__column_text(stmt, 2, scratch_pool),
+             svn_sqlite__column_text(stmt, 3, scratch_pool),
+             svn_sqlite__column_text(stmt, 4, scratch_pool)));*/
+  if (have_row)
+    *is_switched = TRUE;
+  SVN_ERR(svn_sqlite__reset(stmt));
+
   return SVN_NO_ERROR;
 }
 
@@ -12098,6 +12042,18 @@ revision_status_txn(void *baton,
                     apr_pool_t *scratch_pool)
 {
   struct revision_status_baton_t *rsb = baton;
+  svn_error_t *err;
+  svn_boolean_t exists;
+
+  SVN_ERR(does_node_exist(&exists, wcroot, local_relpath));
+
+  if (!exists)
+    {
+      return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
+                               _("The node '%s' was not found."),
+                               path_for_error_message(wcroot, local_relpath,
+                                                      scratch_pool));
+    }
 
   /* Determine mixed-revisionness. */
   SVN_ERR(get_min_max_revisions(rsb->min_revision, rsb->max_revision, wcroot,
@@ -12114,8 +12070,19 @@ revision_status_txn(void *baton,
     SVN_ERR(rsb->cancel_func(rsb->cancel_baton));
 
   /* Check for switched nodes. */
-  SVN_ERR(has_switched_subtrees(rsb->is_switched, wcroot, local_relpath,
-                                rsb->trail_url, scratch_pool));
+  {
+    err = has_switched_subtrees(rsb->is_switched, wcroot, local_relpath,
+                                rsb->trail_url, scratch_pool);
+
+    if (err)
+      {
+        if (err->apr_err != SVN_ERR_WC_PATH_NOT_FOUND)
+          return svn_error_return(err);
+
+        svn_error_clear(err); /* No Base node, but no fatal error */
+        *rsb->is_switched = FALSE;
+      }
+  }
 
   if (rsb->cancel_func)
     SVN_ERR(rsb->cancel_func(rsb->cancel_baton));


Reply via email to