monotone-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Monotone-devel] Descriptors do make a difference 8-O


From: Nathaniel Smith
Subject: Re: [Monotone-devel] Descriptors do make a difference 8-O
Date: Sun, 22 May 2005 02:41:47 -0700
User-agent: Mutt/1.5.9i

On Wed, Apr 27, 2005 at 06:35:37PM +0200, Christof Petig wrote:
> All tests pass with the current candidate
> 463b4f748c4ab4ee9b3a8f1a37ccab1ea84e63cc.
[...]
> If you feel confident about this revision (I did not check _every_ line
> of the merge ...) I propose to put it into mainline (so that the trees
> do not diverge again and future BLOB patches are much less intrusive).

Okay, finally had a chance to look at this.  Sorry for the delay.

Not sure if these comments are properly directed at you or Derek,
since it looks like he wrote the original patch.  But anyway :-):

>    // "%s" construction prevents interpretation of %-signs in the query string
>    // as formatting commands.
> -  fetch(res, any_cols, any_rows, "%s", sql.c_str());
> +  fetch(res, any_cols, any_rows, sql.c_str());

I'm not quite sure what's going on here, but clearly this change makes
the code and comment inconsistent.  (I guess %-signs are now
automatically okay because we no longer go via printf?  What if the
user does "db execute <something with a ?-sign in it>"?  Are we
automatically okay because the only place you can put a ?-sign in
normal sql is inside ''-quotes, and sqlite won't try to interpret one
there?)

> static int 
> dump_table_cb(void *data, int n, char **vals, char **cols)
> {
> [...]
>       sqlite3_exec_printf(dump->sql, "SELECT * FROM '%q'", 
>                          dump_row_cb, data, NULL, vals[0]);
> [...]

Why does this code still call sqlite3_exec_printf?  If it were
changed, then sqlite3_exec_printf and sqlite3_exec_vprintf could both
be removed.

> +  oss << "sqlite errcode=" << errcode << " " << errmsg;

This doesn't seem like a very friendly error message :-).  Surely we
can log the number and say something like "sqlite error: " instead for
the user-visible part?

> PS: Nathaniel: How would you tag a column to contain BLOBs in a way that
> makes schema migration possible?

Not sure what you mean here?

-- Nathaniel

-- 
When the flush of a new-born sun fell first on Eden's green and gold,
Our father Adam sat under the Tree and scratched with a stick in the mould;
And the first rude sketch that the world had seen was joy to his mighty heart,
Till the Devil whispered behind the leaves, "It's pretty, but is it Art?"
  -- The Conundrum of the Workshops, Rudyard Kipling




reply via email to

[Prev in Thread] Current Thread [Next in Thread]