Re: [Monotone-devel] Typesafe VA_ARGS replacement for database::execute/fetch

[Vinzenz 'evilissimo' Feenstra]

Here is the diff but I wasn't abled to test it since I can't get the 
actual revision to built maybe anyone could test it.

BR
Vinzenz

Nathaniel Smith schrieb:
> On Fri, Jan 20, 2006 at 04:12:35PM +0100, Christof Petig wrote:
>   
> > I strongly feel that varargs is a bad choice for C++ programs, using
> > your proposed % syntax is much cleaner (as with L() and boost::format).
> >     
>
> He actually told me the same thing when I suggested going back to this
> :-).  Maybe I agree with you guys... but I should say why I suggested
> it.  The thing is that for an API like:
>   query("blah") % foo % bar % baz
> I feel rather uncomfortable unless we copy the foo/bar/baz strings
> into the query object; there's no convention in general that the
> string data should stay in scope longer than the query object, so we
> need to make a local copy.  With arguments, there's more of a
> convention that I can pass stuff by reference or by pointer and that
> reference/pointer will stay alive until the call finishes.
>
> Since we shovel hundreds of megabytes through this interface, reducing
> copying seems worthwhile.  I guess we do always copy now, though, so
> actually I guess I have no data on whether the win is valuable or not.
> We spend a lot of time in sqlite, but I don't know where exactly.
>
> -- Nathaniel
>
>   

# 
# old_revision [df5476eddd4e7b00482c8306acc4ba06e5af4ee6]
# 
# patch "database.cc"
#  from [0f90adb5a765051661760d886a200529e8fd4b42]
#    to [4d858febac5d15c6c7c9be81aa13113c0d717f9a]
# 
============================================================
--- database.cc 0f90adb5a765051661760d886a200529e8fd4b42
+++ database.cc 4d858febac5d15c6c7c9be81aa13113c0d717f9a
@@ -1,9 +1,10 @@
 // -*- mode: C++; c-file-style: "gnu"; indent-tabs-mode: nil -*-
 // copyright (C) 2002, 2003 graydon hoare <address@hidden>
 // all rights reserved.
 // licensed to the public under the terms of the GNU GPL (>= 2)
 // see the file COPYING for details
 
+
 #include <algorithm>
 #include <deque>
 #include <fstream>
@@ -56,10 +57,51 @@
 int const any_rows = -1;
 int const any_cols = -1;
 
-namespace
+namespace 
 {
-  // track all open databases for close_all_databases() handler
-  set<sqlite3*> sql_contexts;
+    struct query_args
+    {
+        enum arg_type{ text,blob };
+        arg_type type;
+        char const * data;
+        size_t size;
+    };
+
+    query_args
+    text( std::string const txt )
+    {
+        query_args q = { query_args::text , txt.c_str() , txt.size() };
+        return q;
+    }
+
+    query_args
+    text( char const * txt)
+    {
+        query_args q = { query_args::text , txt , strlen(txt) };
+        return q;
+    }
+
+    query_args
+    blob( void const * data , 
+          size_t const size )
+    {
+        query_args q = { query_args::blob, 
+            reinterpret_cast<char const*>(data),
+            size
+        };
+        return q;
+    }
+
+    query_args
+    blob( char const * data , 
+          size_t const size )
+    {
+        query_args q = {query_args::blob,data,size};
+        return q;
+    }
+
+    // track all open databases for close_all_databases() handler
+    set<sqlite3*> sql_contexts;
 }
 
 extern "C" {
@@ -101,7 +143,7 @@
   string revisions_query = "SELECT 1 FROM revisions LIMIT 1";
   string rosters_query = "SELECT 1 FROM rosters LIMIT 1";
 
-  fetch(res_revisions, one_col, any_rows, revisions_query.c_str());
+  fetch(res_revisions, one_col, any_rows, revisions_query.c_str()));
 
   if (res_revisions.size() > 0)
     {
@@ -569,7 +611,7 @@
   results res;
   va_list args;
   va_start(args, query);
-  fetch(res, 0, 0, query, args);
+  fetch(res, 0, 0, query, text(args));
   va_end(args);
 }
 
@@ -731,7 +773,7 @@
 {
   results res;
   string query = "SELECT id FROM " + table + " WHERE id = ?";
-  fetch(res, one_col, any_rows, query.c_str(), ident().c_str());
+  fetch(res, one_col, any_rows, query.c_str(), text(ident()));
   I((res.size() == 1) || (res.size() == 0));
   return res.size() == 1;
 }
@@ -743,7 +785,7 @@
 {
   results res;
   string query = "SELECT id FROM " + table + " WHERE id = ?";
-  fetch(res, one_col, any_rows, query.c_str(), ident().c_str());
+  fetch(res, one_col, any_rows, query.c_str(), text(ident()));
   return res.size() > 0;
 }
 
@@ -752,7 +794,7 @@
 {
   results res;
   string query = "SELECT COUNT(*) FROM " + table;
-  fetch(res, one_col, one_row, query.c_str());
+  fetch(res, one_col, one_row, text(query));
   return lexical_cast<unsigned long>(res[0][0]);  
 }
 
@@ -763,7 +805,7 @@
   // COALESCE is required since SUM({empty set}) is NULL.
   // the sqlite docs for SUM suggest this as a workaround
   string query = "SELECT COALESCE(SUM(LENGTH(" + concatenated_columns + ")), 
0) FROM " + table;
-  fetch(res, one_col, one_row, query.c_str());
+  fetch(res, one_col, one_row, text(query));
   return lexical_cast<unsigned long>(res[0][0]);
 }
 
@@ -772,7 +814,7 @@
 {
   results res;
   string query = "SELECT id FROM " + table;
-  fetch(res, one_col, any_rows, query.c_str());
+  fetch(res, one_col, any_rows, text(query));
 
   for (size_t i = 0; i < res.size(); ++i)
     {
@@ -787,7 +829,7 @@
 {
   results res;
   string query = "SELECT data FROM " + table + " WHERE id = ?";
-  fetch(res, one_col, one_row, query.c_str(), ident().c_str());
+  fetch(res, one_col, one_row, query.c_str(), text(ident()));
 
   // consistency check
   base64<gzip<data> > rdata(res[0][0]);
@@ -812,7 +854,7 @@
   results res;
   string query = "SELECT delta FROM " + table + " WHERE id = ? AND base = ?";
   fetch(res, one_col, one_row, query.c_str(), 
-        ident().c_str(), base().c_str());
+        text(ident()), text(base()));
 
   base64<gzip<delta> > del_packed = res[0][0];
   unpack(del_packed, del);
@@ -1024,7 +1066,7 @@
                   // This tip is not a root, so extend the path.
                   results res;                  
                   fetch(res, one_col, any_rows, 
-                        delta_query.c_str(), tip().c_str());
+                        delta_query.c_str(), text(tip()));
 
                   I(res.size() != 0);
 
@@ -1165,7 +1207,7 @@
     results res;
     string query = "SELECT id FROM " + delta_table + " WHERE base = ?";
     fetch(res, one_col, any_rows, 
-          query.c_str(), target_id().c_str());
+          query.c_str(), text(target_id()));
     for (size_t i = 0; i < res.size(); ++i)
       {
         hexenc<id> old_id(res[i][0]);
@@ -1188,7 +1230,7 @@
           results res;
           string query = "SELECT base FROM " + delta_table + " WHERE id = ?";
           fetch(res, one_col, any_rows, 
-                query.c_str(), target_id().c_str());
+                query.c_str(), text(target_id()));
           I(res.size() > 0);
           newer_id = hexenc<id>(res[0][0]);
           get_version(newer_id, newer_data, data_table, delta_table);
@@ -1250,7 +1292,7 @@
   results res;
   string query = ("SELECT roster_id FROM revision_roster WHERE rev_id = ? ");
   fetch(res, one_col, any_rows, query.c_str(), 
-        rev_id.inner()().c_str());
+        text(rev_id.inner()()));
   I((res.size() == 1) || (res.size() == 0));
   return res.size() == 1;
 }
@@ -1261,7 +1303,7 @@
   results res;
   string query = ("SELECT roster_id FROM revision_roster WHERE rev_id = ? ");
   fetch(res, one_col, any_rows, query.c_str(), 
-        rev_id.inner()().c_str());
+        text(rev_id.inner()()));
   I((res.size() == 1) || (res.size() == 0));
   return (res.size() == 1) && roster_version_exists(hexenc<id>(res[0][0]));
 }
@@ -1364,7 +1406,7 @@
   parents.clear();
   fetch(res, one_col, any_rows, 
         "SELECT parent FROM revision_ancestry WHERE child = ?",
-        id.inner()().c_str());
+        text(id.inner()()));
   for (size_t i = 0; i < res.size(); ++i)
     parents.insert(revision_id(res[i][0]));
 }
@@ -1377,7 +1419,7 @@
   children.clear();
   fetch(res, one_col, any_rows, 
         "SELECT child FROM revision_ancestry WHERE parent = ?",
-        id.inner()().c_str());
+        text(id.inner()()));
   for (size_t i = 0; i < res.size(); ++i)
     children.insert(revision_id(res[i][0]));
 }
@@ -1408,7 +1450,7 @@
   results res;
   fetch(res, one_col, one_row, 
         "SELECT data FROM revisions WHERE id = ?",
-        id.inner()().c_str());
+        text(id.inner()()));
 
   base64<gzip<data> > rdat_packed;
   rdat_packed = base64<gzip<data> >(res[0][0]);
@@ -1581,7 +1623,7 @@
     results res;
     string query = ("SELECT rev_id, roster_id FROM revision_roster "
                     "WHERE roster_id = ?");
-    fetch(res, 2, any_rows, query.c_str(), roster_id().c_str());
+    fetch(res, 2, any_rows, query.c_str(), text(roster_id()));
     I(res.size() > 0);
     link_count = res.size();
   }
@@ -1632,7 +1674,7 @@
   if (pattern != "")
     fetch(res, one_col, any_rows, 
           "SELECT id FROM public_keys WHERE id GLOB ?",
-          pattern.c_str());
+          text(pattern));
   else
     fetch(res, one_col, any_rows, 
           "SELECT id FROM public_keys");
@@ -1664,7 +1706,7 @@
   results res;
   fetch(res, one_col, any_rows, 
         "SELECT id FROM public_keys WHERE hash = ?",
-        hash().c_str());
+        text(hash()));
   I((res.size() == 1) || (res.size() == 0));
   if (res.size() == 1) 
     return true;
@@ -1677,7 +1719,7 @@
   results res;
   fetch(res, one_col, any_rows, 
         "SELECT id FROM public_keys WHERE id = ?",
-        id().c_str());
+        text(id()));
   I((res.size() == 1) || (res.size() == 0));
   if (res.size() == 1) 
     return true;
@@ -1692,7 +1734,7 @@
   results res;
   fetch(res, 2, one_row, 
         "SELECT id, keydata FROM public_keys WHERE hash = ?", 
-        hash().c_str());
+        text(hash()));
   id = res[0][0];
   pub_encoded = res[0][1];
 }
@@ -1704,7 +1746,7 @@
   results res;
   fetch(res, one_col, one_row, 
         "SELECT keydata FROM public_keys WHERE id = ?", 
-        pub_id().c_str());
+        text(pub_id()));
   pub_encoded = res[0][0];
 }
 
@@ -1743,11 +1785,11 @@
     "AND signature = ?";
     
   fetch(res, 1, any_rows, query.c_str(),
-        t.ident().c_str(),
-        t.name().c_str(),
-        t.value().c_str(),
-        t.key().c_str(),
-        t.sig().c_str());
+        text(t.ident()),
+        text(t.name()),
+        text(t.value()),
+        text(t.key()),
+        text(t.sig()));
   I(res.size() == 0 || res.size() == 1);
   return res.size() == 1;
 }
@@ -1844,7 +1886,7 @@
     "SELECT id, name, value, keypair, signature FROM " + table + 
     " WHERE id = ?";
 
-  fetch(res, 5, any_rows, query.c_str(), ident().c_str());
+  fetch(res, 5, any_rows, query.c_str(), text(ident()));
   results_to_certs(res, certs);
 }
 
@@ -1858,7 +1900,7 @@
   string query = 
     "SELECT id, name, value, keypair, signature FROM " + table + 
     " WHERE name = ?";
-  fetch(res, 5, any_rows, query.c_str(), name().c_str());
+  fetch(res, 5, any_rows, query.c_str(), text(name()));
   results_to_certs(res, certs);
 }
 
@@ -1875,7 +1917,7 @@
     " WHERE id = ? AND name = ?";
 
   fetch(res, 5, any_rows, query.c_str(), 
-        ident().c_str(), name().c_str());
+        text(ident()), text(name()));
   results_to_certs(res, certs);
 }
 
@@ -1891,7 +1933,7 @@
     " WHERE name = ? AND value = ?";
 
   fetch(res, 5, any_rows, query.c_str(), 
-        name().c_str(), val().c_str());
+        text(name()), text(val()));
   results_to_certs(res, certs);
 }
 
@@ -1909,9 +1951,9 @@
     " WHERE id = ? AND name = ? AND value = ?";
 
   fetch(res, 5, any_rows, query.c_str(),
-        ident().c_str(),
-        name().c_str(),
-        value().c_str());
+        text(ident()),
+        text(name()),
+        text(value()));
   results_to_certs(res, certs);
 }
 
@@ -2020,7 +2062,7 @@
         "SELECT hash "
         "FROM revision_certs "
         "WHERE id = ?", 
-        ident.inner()().c_str());
+        text(ident.inner()()));
   ts.clear();
   for (size_t i = 0; i < res.size(); ++i)
     ts.push_back(hexenc<id>(res[i][0]));
@@ -2036,7 +2078,7 @@
         "SELECT id, name, value, keypair, signature "
         "FROM revision_certs "
         "WHERE hash = ?", 
-        hash().c_str());
+        text(hash()));
   results_to_certs(res, certs);
   I(certs.size() == 1);
   c = revision<cert>(certs[0]);
@@ -2051,7 +2093,7 @@
         "SELECT id "
         "FROM revision_certs "
         "WHERE hash = ?", 
-        hash().c_str());
+        text(hash()));
   I(res.size() == 0 || res.size() == 1);
   return (res.size() == 1);
 }
@@ -2090,7 +2132,7 @@
 
   fetch(res, 1, any_rows,
         "SELECT id FROM revisions WHERE id GLOB ?",
-        pattern.c_str());
+        text(pattern));
   
   for (size_t i = 0; i < res.size(); ++i)
     completions.insert(revision_id(res[i][0]));  
@@ -2108,7 +2150,7 @@
 
   fetch(res, 1, any_rows,
         "SELECT id FROM files WHERE id GLOB ?",
-        pattern.c_str());
+        text(pattern));
 
   for (size_t i = 0; i < res.size(); ++i)
     completions.insert(file_id(res[i][0]));  
@@ -2117,7 +2159,7 @@
 
   fetch(res, 1, any_rows,
         "SELECT id FROM file_deltas WHERE id GLOB ?",
-        pattern.c_str());
+        text(pattern));
 
   for (size_t i = 0; i < res.size(); ++i)
     completions.insert(file_id(res[i][0]));  
@@ -2134,7 +2176,7 @@
 
   fetch(res, 2, any_rows,
         "SELECT hash, id FROM public_keys WHERE hash GLOB ?",
-        pattern.c_str());
+        text(pattern));
 
   for (size_t i = 0; i < res.size(); ++i)
     completions.insert(make_pair(key_id(res[i][0]), utf8(res[i][1])));
@@ -2416,7 +2458,7 @@
   fetch(res, 2, any_rows,
         "SELECT branch, epoch FROM branch_epochs"
         " WHERE hash = ?",
-        eid.inner()().c_str());
+        text(eid.inner()()));
   I(res.size() == 1);
   base64<cert_value> encoded(idx(idx(res, 0), 0));
   decode_base64(encoded, branch);
@@ -2429,7 +2471,7 @@
   results res;
   fetch(res, one_col, any_rows,
         "SELECT hash FROM branch_epochs WHERE hash = ?",
-        eid.inner()().c_str());
+        text(eid.inner()()));
   I(res.size() == 1 || res.size() == 0);
   return res.size() == 1;
 }
@@ -2527,7 +2569,7 @@
     results res;
     string query="SELECT DISTINCT value FROM revision_certs WHERE name= ?";
     string cert_name="branch";
-    fetch(res, one_col, any_rows, query.c_str(), cert_name.c_str());
+    fetch(res, one_col, any_rows, query.c_str(), text(cert_name));
     for (size_t i = 0; i < res.size(); ++i)
       {
         base64<data> row_encoded(res[i][0]);
@@ -2550,7 +2592,7 @@
   results res;
   string query = ("SELECT roster_id FROM revision_roster WHERE rev_id = ? ");  
   fetch(res, one_col, any_rows, query.c_str(),
-        rev_id.inner()().c_str());
+        text(rev_id.inner()()));
   I(res.size() == 1);
   roster_id = hexenc<id>(res[0][0]);
 }