On 07.02.2011 02:52, Branko Čibej wrote:
> Note that I didn't run the tests with this patch, so I'm not claiming it
> to be bug-free. In fact there's one bug in the recursive proplist of the
> WC root, where the root props show up twice -- there's an issue with the
> filter in the query there. But it's proof-of-concept after all.

Well, I fixed that bug and the new patch is attached. I ran
prop_tests.py, and did get one failure (see below); could be a
difference in the output (the results of a recursive proplist are
ordered now), but I don't plan to investigate that; whoever takes this
patch and runs with it is welcome to do so. :)

-- Brane

Error: expected keywords:  ['new-add', 'new-keep', 'p', 'p', 'Properties on ', 
'Properties on ']
       actual full output: ['    new-add\n', '    new-keep\n', '    
old-keep\n', '  p\n', '  p\n', '  p\n', "Properties on 
'svn-test-work/working_copies/prop_tests-15/A/added':\n", "Properties on 
'svn-test-work/working_copies/prop_tests-15/iota':\n", "Properties on 
'svn-test-work/working_copies/prop_tests-15/iota':\n"]
Traceback (most recent call last):
  File 
"/Users/brane/src/svn/repos/trunk/subversion/tests/cmdline/svntest/main.py", 
line 1222, in run
    rc = self.pred.run(sandbox)
  File 
"/Users/brane/src/svn/repos/trunk/subversion/tests/cmdline/svntest/testcase.py",
 line 182, in run
    return self.func(sandbox)
  File "./prop_tests.py", line 1093, in recursive_base_wc_ops
    output, errput)
  File "./prop_tests.py", line 1053, in verify_output
    raise svntest.Failure
Failure
FAIL:  prop_tests.py 15: recursive property operations in BASE and WC


Index: subversion/libsvn_wc/props.c
===================================================================
--- subversion/libsvn_wc/props.c        (revision 1067878)
+++ subversion/libsvn_wc/props.c        (working copy)
@@ -1711,6 +1711,7 @@
   SVN_ERR(svn_wc__db_read_props_of_immediates(b->db, local_abspath,
                                               b->receiver_func,
                                               b->receiver_baton,
+                                              NULL, NULL,
                                               scratch_pool));
   return SVN_NO_ERROR;
 }
@@ -1725,9 +1726,12 @@
                             void *cancel_baton,
                             apr_pool_t *scratch_pool)
 {
-  struct read_dir_props_baton read_dir_baton;
+  svn_boolean_t is_root;
 
-  if (depth <= svn_depth_immediates)
+  SVN_ERR(svn_wc__db_is_wcroot(&is_root, wc_ctx->db, local_abspath,
+                               scratch_pool));
+
+  if (depth <= svn_depth_immediates || !is_root)
     {
       apr_hash_t *props;
 
@@ -1744,28 +1748,28 @@
     {
       SVN_ERR(svn_wc__db_read_props_of_files(wc_ctx->db, local_abspath,
                                              receiver_func, receiver_baton,
+                                             cancel_func, cancel_baton,
                                              scratch_pool));
-      return SVN_NO_ERROR;
     }
-
-  if (depth == svn_depth_immediates)
+  else if (depth == svn_depth_immediates)
     {
       SVN_ERR(svn_wc__db_read_props_of_immediates(wc_ctx->db, local_abspath,
-                                                  receiver_func,
-                                                  receiver_baton,
+                                                  receiver_func, 
receiver_baton,
+                                                  cancel_func, cancel_baton,
                                                   scratch_pool));
-      return SVN_NO_ERROR;
     }
+  else if (depth == svn_depth_infinity)
+    {
+      SVN_ERR(svn_wc__db_read_props_recursive(wc_ctx->db, local_abspath,
+                                              receiver_func, receiver_baton,
+                                              cancel_func, cancel_baton,
+                                              scratch_pool));
+    }
+  else
+    {
+      SVN_ERR_MALFUNCTION();
+    }
 
-  read_dir_baton.db = wc_ctx->db;
-  read_dir_baton.root_abspath = local_abspath;
-  read_dir_baton.receiver_func = receiver_func;
-  read_dir_baton.receiver_baton = receiver_baton;
-
-  SVN_ERR(svn_wc__internal_walk_children(wc_ctx->db, local_abspath, FALSE,
-                                         read_dir_props, &read_dir_baton,
-                                         depth, cancel_func, cancel_baton,
-                                         scratch_pool));
   return SVN_NO_ERROR;
 }
 
Index: subversion/libsvn_wc/wc-queries.sql
===================================================================
--- subversion/libsvn_wc/wc-queries.sql (revision 1067878)
+++ subversion/libsvn_wc/wc-queries.sql (working copy)
@@ -726,7 +726,69 @@
 UPDATE nodes SET checksum = ?4
 WHERE wc_id = ?1 AND local_relpath = ?2 AND op_depth = ?3;
 
+/* ------------------------------------------------------------------------- */
+/* PROOF OF CONCEPT: Complex queries for callback walks, using file-backed
+ *                   temporary database. */
 
+-- STMT_ATTACH_TEMPORARY_DATABASE
+ATTACH DATABASE '' AS temp_query_cache;
+
+-- STMT_DETACH_TEMPORARY_DATABASE
+DETACH DATABASE temp_query_cache;
+
+-- STMT_CLEAR_NODE_PROPS_CACHE
+DROP TABLE IF EXISTS temp_query_cache.node_props_cache;
+
+-- STMT_QUERY_NODE_PROPS_OF_CHILDREN
+CREATE TABLE temp_query_cache.node_props_cache AS
+  SELECT N.local_relpath, N.properties, N.kind, ?1 AS wc_id
+    FROM nodes AS N
+    JOIN (
+      SELECT local_relpath, MAX(op_depth) AS op_depth FROM nodes
+      WHERE wc_id = ?1 AND parent_relpath = ?2
+      GROUP BY local_relpath
+    ) AS V
+      ON N.local_relpath = V.local_relpath
+        AND N.op_depth = V.op_depth
+  WHERE N.wc_id = ?1
+    AND (N.presence = 'normal' OR N.presence = 'incomplete');
+CREATE UNIQUE INDEX temp_query_cache.node_props_cache_unique
+  ON node_props_cache (local_relpath);
+
+-- STMT_QUERY_NODE_PROPS_RECURSIVE
+CREATE TABLE temp_query_cache.node_props_cache AS
+  SELECT N.local_relpath, N.properties, '' AS kind, ?1 AS wc_id
+    FROM nodes AS N
+    JOIN (
+      SELECT local_relpath, MAX(op_depth) AS op_depth FROM nodes
+      WHERE wc_id = ?1
+        AND local_relpath LIKE
+          CASE ?2 WHEN '' THEN '%' ELSE ?2 || '/%' END
+      GROUP BY local_relpath
+    ) AS V
+      ON N.local_relpath = V.local_relpath
+        AND N.op_depth = V.op_depth
+  WHERE N.wc_id = ?1
+    AND (N.presence = 'normal' OR N.presence = 'incomplete');
+CREATE UNIQUE INDEX temp_query_cache.node_props_cache_unique
+  ON node_props_cache (local_relpath);
+
+-- STMT_REPLACE_ACTUAL_PROPS_IN_CACHE
+INSERT OR REPLACE INTO temp_query_cache.node_props_cache
+  (local_relpath, properties)
+  SELECT N.local_relpath, N.properties
+    FROM actual_node AS N JOIN temp_query_cache.node_props_cache AS C
+      ON N.local_relpath = C.local_relpath
+        AND  N.wc_id = C.wc_id;
+/*COMMIT TRANSACTION;*/
+
+-- STMT_SELECT_RELEVANT_PROPS_FROM_CACHE
+SELECT local_relpath, properties, kind
+  FROM temp_query_cache.node_props_cache
+WHERE properties IS NOT NULL
+ORDER BY local_relpath;
+
+
 /* ------------------------------------------------------------------------- */
 
 /* Grab all the statements related to the schema.  */
Index: subversion/libsvn_wc/wc_db.c
===================================================================
--- subversion/libsvn_wc/wc_db.c        (revision 1067878)
+++ subversion/libsvn_wc/wc_db.c        (working copy)
@@ -5056,187 +5056,150 @@
   return SVN_NO_ERROR;
 }
 
-/* Parse a node's PROP_DATA (which is PROP_DATA_LEN bytes long)
- * into a hash table keyed by property names and containing property values.
- *
- * If parsing succeeds, and the set of properties is not empty,
- * add the hash table to PROPS_PER_CHILD, keyed by the absolute path
- * of the node CHILD_RELPATH within the working copy at WCROOT_ABSPATH.
- *
- * If the set of properties is empty, and PROPS_PER_CHILD already contains
- * an entry for the node, clear the entry. This facilitates overriding
- * properties retrieved from the NODES table with empty sets of properties
- * stored in the ACTUAL_NODE table. */
+/** POC **/
+
 static svn_error_t *
-maybe_add_child_props(apr_hash_t *props_per_child,
-                      const char *prop_data,
-                      apr_size_t prop_data_len,
-                      const char *child_relpath,
-                      const char *wcroot_abspath,
-                      apr_pool_t *result_pool,
-                      apr_pool_t *scratch_pool)
+attach_cache_db(svn_wc__db_pdh_t *pdh)
 {
-  const char *child_abspath;
-  apr_hash_t *props;
-  svn_skel_t *prop_skel;
+  return svn_error_return
+    (svn_sqlite__exec_statements(pdh->wcroot->sdb,
+                                 STMT_ATTACH_TEMPORARY_DATABASE));
+}
 
-  prop_skel = svn_skel__parse(prop_data, prop_data_len, scratch_pool);
-  if (svn_skel__list_length(prop_skel) == 0)
-    return SVN_NO_ERROR;
+static svn_error_t *
+detach_cache_db(svn_wc__db_pdh_t *pdh)
+{
+  return svn_error_return
+    (svn_sqlite__exec_statements(pdh->wcroot->sdb,
+                                 STMT_DETACH_TEMPORARY_DATABASE));
+}
 
-  child_abspath = svn_dirent_join(wcroot_abspath, child_relpath, result_pool);
-  SVN_ERR(svn_skel__parse_proplist(&props, prop_skel, result_pool));
-  if (apr_hash_count(props))
-    apr_hash_set(props_per_child, child_abspath, APR_HASH_KEY_STRING, props);
-  else
-    apr_hash_set(props_per_child, child_abspath, APR_HASH_KEY_STRING, NULL);
+typedef struct cache_props_of_children_baton_t {
+  int stmt_idx;
+  apr_int64_t wc_id;
+  const char *local_relpath;
+} cache_props_of_children_baton_t;
 
+static svn_error_t *
+cache_props_of_children(void *cb_baton,
+                        svn_sqlite__db_t *db,
+                        apr_pool_t *scratch_pool)
+{
+  cache_props_of_children_baton_t *baton = cb_baton;
+  svn_sqlite__stmt_t *stmt;
+
+  SVN_ERR(svn_sqlite__get_statement(&stmt, db, baton->stmt_idx));
+  SVN_ERR(svn_sqlite__bindf(stmt, "is", baton->wc_id, baton->local_relpath));
+  SVN_ERR(svn_sqlite__step_done(stmt));
+  SVN_ERR(svn_sqlite__exec_statements(db, STMT_REPLACE_ACTUAL_PROPS_IN_CACHE));
+
   return SVN_NO_ERROR;
 }
 
 /* Call RECEIVER_FUNC, passing RECEIVER_BATON, an absolute path, and
  * a hash table mapping <tt>char *</tt> names onto svn_string_t *
- * values for any properties of immediate child nodes of LOCAL_ABSPATH.
+ * values for any properties of immediate or recursive child nodes of
+ * LOCAL_ABSPATH, the actual query being determined by STMT_IDX.
  * If FILES_ONLY is true, only report properties for file child nodes.
+ * Check for cancellation between calls of RECEIVER_FUNC.
  */
+
 static svn_error_t *
 read_props_of_children(svn_wc__db_t *db,
+                       int stmt_idx,
                        const char *local_abspath,
                        svn_boolean_t files_only,
                        svn_wc__proplist_receiver_t receiver_func,
                        void *receiver_baton,
+                       svn_cancel_func_t cancel_func,
+                       void *cancel_baton,
                        apr_pool_t *scratch_pool)
 {
   svn_wc__db_pdh_t *pdh;
-  const char *local_relpath;
-  const char *prev_child_relpath;
+  cache_props_of_children_baton_t txn_baton;
   svn_sqlite__stmt_t *stmt;
   svn_boolean_t have_row;
-  apr_hash_t *props_per_child;
-  apr_hash_t *files;
-  apr_hash_t *not_present;
-  apr_hash_index_t *hi;
   apr_pool_t *iterpool;
 
   SVN_ERR_ASSERT(svn_dirent_is_absolute(local_abspath));
   SVN_ERR_ASSERT(receiver_func);
 
-  SVN_ERR(svn_wc__db_pdh_parse_local_abspath(&pdh, &local_relpath, db,
-                                             local_abspath,
+  SVN_ERR(svn_wc__db_pdh_parse_local_abspath(&pdh, &txn_baton.local_relpath,
+                                             db, local_abspath,
                                              svn_sqlite__mode_readwrite,
                                              scratch_pool, scratch_pool));
   VERIFY_USABLE_PDH(pdh);
 
-  props_per_child = apr_hash_make(scratch_pool);
-  not_present = apr_hash_make(scratch_pool);
-  if (files_only)
-    files = apr_hash_make(scratch_pool);
-  else
-    files = NULL;
+  SVN_ERR(attach_cache_db(pdh));
+  SVN_ERR(svn_sqlite__exec_statements(pdh->wcroot->sdb,
+                                      STMT_CLEAR_NODE_PROPS_CACHE));
 
+  txn_baton.stmt_idx = stmt_idx;
+  txn_baton.wc_id = pdh->wcroot->wc_id;
+  SVN_ERR(svn_sqlite__with_transaction(pdh->wcroot->sdb,
+                                       cache_props_of_children,
+                                       &txn_baton,
+                                       scratch_pool));
+
+  iterpool = svn_pool_create(scratch_pool);
+
   SVN_ERR(svn_sqlite__get_statement(&stmt, pdh->wcroot->sdb,
-                                    STMT_SELECT_NODE_PROPS_OF_CHILDREN));
-  SVN_ERR(svn_sqlite__bindf(stmt, "is", pdh->wcroot->wc_id, local_relpath));
+                                    STMT_SELECT_RELEVANT_PROPS_FROM_CACHE));
   SVN_ERR(svn_sqlite__step(&have_row, stmt));
-  prev_child_relpath = NULL;
   while (have_row)
     {
-      svn_wc__db_status_t child_presence;
-      const char *child_relpath;
       const char *prop_data;
       apr_size_t len;
 
-      child_relpath = svn_sqlite__column_text(stmt, 2, scratch_pool);
-
-      if (prev_child_relpath && strcmp(child_relpath, prev_child_relpath) == 0)
+      if (files_only)
         {
-          /* Same child, but lower op_depth -- skip this row. */
-          SVN_ERR(svn_sqlite__step(&have_row, stmt));
-          continue;
-        }
-      prev_child_relpath = child_relpath;
+          svn_wc__db_kind_t child_kind;
 
-      child_presence = svn_sqlite__column_token(stmt, 1, presence_map);
-      if (child_presence != svn_wc__db_status_normal)
-        {
-          apr_hash_set(not_present, child_relpath, APR_HASH_KEY_STRING, "");
-          SVN_ERR(svn_sqlite__step(&have_row, stmt));
-          continue;
-        }
-
-      prop_data = svn_sqlite__column_blob(stmt, 0, &len, NULL);
-      if (prop_data)
-        {
-          if (files_only)
+          child_kind = svn_sqlite__column_token(stmt, 2, kind_map);
+          if (child_kind != svn_wc__db_kind_file &&
+              child_kind != svn_wc__db_kind_symlink)
             {
-              svn_wc__db_kind_t child_kind;
-
-              child_kind = svn_sqlite__column_token(stmt, 3, kind_map);
-              if (child_kind != svn_wc__db_kind_file &&
-                  child_kind != svn_wc__db_kind_symlink)
-                {
-                  SVN_ERR(svn_sqlite__step(&have_row, stmt));
-                  continue;
-                }
-              apr_hash_set(files, child_relpath, APR_HASH_KEY_STRING, NULL);
+              SVN_ERR(svn_sqlite__step(&have_row, stmt));
+              continue;
             }
-
-          SVN_ERR(maybe_add_child_props(props_per_child, prop_data, len,
-                                        child_relpath, pdh->wcroot->abspath,
-                                        scratch_pool, scratch_pool));
         }
 
-      SVN_ERR(svn_sqlite__step(&have_row, stmt));
-    }
+      svn_pool_clear(iterpool);
 
-  SVN_ERR(svn_sqlite__reset(stmt));
+      /* See if someone wants to cancel this operation. */
+      if (cancel_func)
+        SVN_ERR(cancel_func(cancel_baton));
 
-  SVN_ERR(svn_sqlite__get_statement(&stmt, pdh->wcroot->sdb,
-                                    STMT_SELECT_ACTUAL_PROPS_OF_CHILDREN));
-  SVN_ERR(svn_sqlite__bindf(stmt, "is", pdh->wcroot->wc_id, local_relpath));
-  SVN_ERR(svn_sqlite__step(&have_row, stmt));
-  while (have_row)
-    {
-      const char *child_relpath;
-      const char *prop_data;
-      apr_size_t len;
-
-      prop_data = svn_sqlite__column_blob(stmt, 0, &len, NULL);
+      prop_data = svn_sqlite__column_blob(stmt, 1, &len, NULL);
       if (prop_data)
         {
-          child_relpath = svn_sqlite__column_text(stmt, 1, scratch_pool);
+          svn_skel_t *prop_skel;
 
-          if (apr_hash_get(not_present, child_relpath, APR_HASH_KEY_STRING) ||
-              (files_only &&
-               apr_hash_get(files, child_relpath, APR_HASH_KEY_STRING) == 
NULL))
+          prop_skel = svn_skel__parse(prop_data, len, iterpool);
+          if (svn_skel__list_length(prop_skel) != 0)
             {
-                SVN_ERR(svn_sqlite__step(&have_row, stmt));
-                continue;
+              const char *child_relpath;
+              const char *child_abspath;
+              apr_hash_t *props;
+
+              child_relpath = svn_sqlite__column_text(stmt, 0, NULL);
+              child_abspath = svn_dirent_join(pdh->wcroot->abspath,
+                                              child_relpath, iterpool);
+              SVN_ERR(svn_skel__parse_proplist(&props, prop_skel, iterpool));
+              if (receiver_func && apr_hash_count(props) > 0)
+                {
+                  SVN_ERR((*receiver_func)(receiver_baton,
+                                           child_abspath, props,
+                                           iterpool));
+                }
             }
-          SVN_ERR(maybe_add_child_props(props_per_child, prop_data, len,
-                                        child_relpath, pdh->wcroot->abspath,
-                                        scratch_pool, scratch_pool));
         }
 
       SVN_ERR(svn_sqlite__step(&have_row, stmt));
     }
 
   SVN_ERR(svn_sqlite__reset(stmt));
-
-  iterpool = svn_pool_create(scratch_pool);
-  for (hi = apr_hash_first(scratch_pool, props_per_child);
-       hi;
-       hi = apr_hash_next(hi))
-    {
-      const char *child_abspath = svn__apr_hash_index_key(hi);
-      apr_hash_t *child_props = svn__apr_hash_index_val(hi);
-
-      svn_pool_clear(iterpool);
-
-      if (child_props)
-        SVN_ERR((*receiver_func)(receiver_baton, child_abspath, child_props,
-                                 iterpool));
-    }
+  SVN_ERR(detach_cache_db(pdh));
   svn_pool_destroy(iterpool);
 
   return SVN_NO_ERROR;
@@ -5247,11 +5210,17 @@
                                const char *local_abspath,
                                svn_wc__proplist_receiver_t receiver_func,
                                void *receiver_baton,
+                               svn_cancel_func_t cancel_func,
+                               void *cancel_baton,
                                apr_pool_t *scratch_pool)
 {
-  return svn_error_return(read_props_of_children(db, local_abspath, TRUE,
-                                                 receiver_func, receiver_baton,
-                                                 scratch_pool));
+  SVN_ERR(read_props_of_children(db,
+                                 STMT_QUERY_NODE_PROPS_OF_CHILDREN,
+                                 local_abspath, TRUE,
+                                 receiver_func, receiver_baton,
+                                 cancel_func, cancel_baton,
+                                 scratch_pool));
+  return SVN_NO_ERROR;
 }
 
 svn_error_t *
@@ -5259,13 +5228,36 @@
                                     const char *local_abspath,
                                     svn_wc__proplist_receiver_t receiver_func,
                                     void *receiver_baton,
+                                    svn_cancel_func_t cancel_func,
+                                    void *cancel_baton,
                                     apr_pool_t *scratch_pool)
 {
-  return svn_error_return(read_props_of_children(db, local_abspath, FALSE,
-                                                 receiver_func, receiver_baton,
-                                                 scratch_pool));
+  SVN_ERR(read_props_of_children(db,
+                                 STMT_QUERY_NODE_PROPS_OF_CHILDREN,
+                                 local_abspath, FALSE,
+                                 receiver_func, receiver_baton,
+                                 cancel_func, cancel_baton,
+                                 scratch_pool));
+  return SVN_NO_ERROR;
 }
 
+svn_error_t *
+svn_wc__db_read_props_recursive(svn_wc__db_t *db,
+                                const char *local_abspath,
+                                svn_wc__proplist_receiver_t receiver_func,
+                                void *receiver_baton,
+                                svn_cancel_func_t cancel_func,
+                                void *cancel_baton,
+                                apr_pool_t *scratch_pool)
+{
+  SVN_ERR(read_props_of_children(db,
+                                 STMT_QUERY_NODE_PROPS_RECURSIVE,
+                                 local_abspath, FALSE,
+                                 receiver_func, receiver_baton,
+                                 cancel_func, cancel_baton,
+                                 scratch_pool));
+  return SVN_NO_ERROR;
+}
 
 static svn_error_t *
 db_read_pristine_props(apr_hash_t **props,
Index: subversion/libsvn_wc/wc_db.h
===================================================================
--- subversion/libsvn_wc/wc_db.h        (revision 1067878)
+++ subversion/libsvn_wc/wc_db.h        (working copy)
@@ -1580,9 +1580,10 @@
 svn_error_t *
 svn_wc__db_read_props_of_files(svn_wc__db_t *db,
                                const char *local_abspath,
-                               svn_wc__proplist_receiver_t
-                                 receiver_func,
+                               svn_wc__proplist_receiver_t receiver_func,
                                void *receiver_baton,
+                               svn_cancel_func_t cancel_func,
+                               void *cancel_baton,
                                apr_pool_t *scratch_pool);
 
 /* Call RECEIVER_FUNC, passing RECEIVER_BATON, an absolute path, and
@@ -1592,11 +1593,25 @@
 svn_error_t *
 svn_wc__db_read_props_of_immediates(svn_wc__db_t *db,
                                     const char *local_abspath,
-                                    svn_wc__proplist_receiver_t
-                                      receiver_func,
+                                    svn_wc__proplist_receiver_t receiver_func,
                                     void *receiver_baton,
+                                    svn_cancel_func_t cancel_func,
+                                    void *cancel_baton,
                                     apr_pool_t *scratch_pool);
 
+/* Call RECEIVER_FUNC, passing RECEIVER_BATON, an absolute path, and
+ * a hash table mapping <tt>char *</tt> names onto svn_string_t *
+ * values for any properties of all (recursive) child nodes of LOCAL_ABSPATH.
+ */
+svn_error_t *
+svn_wc__db_read_props_recursive(svn_wc__db_t *db,
+                                const char *local_abspath,
+                                svn_wc__proplist_receiver_t receiver_func,
+                                void *receiver_baton,
+                                svn_cancel_func_t cancel_func,
+                                  void *cancel_baton,
+                                apr_pool_t *scratch_pool);
+
 /* Set *PROPS to the properties of the node LOCAL_ABSPATH in the WORKING
    tree (looking through to the BASE tree as required).
 

Reply via email to