[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Monotone-devel] Descriptors do make a difference 8-O
From: |
Christof Petig |
Subject: |
Re: [Monotone-devel] Descriptors do make a difference 8-O |
Date: |
Mon, 23 May 2005 09:19:44 +0200 |
User-agent: |
Mozilla Thunderbird 1.0.2 (X11/20050404) |
Nathaniel Smith schrieb:
> 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());
You are right. The %s and the comment were added after Derek's changes
and I migrated the command (again) but did not look at the comment.
> 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?)
db execute <something with a ?-sign in it>
should give the expected ;-) result : too few arguments for query.
>>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.
Derek did not change this (and I did not think about it that deeply
since I wanted to maintain the changes as unintrusive as possible).
Incremental refinement is always possible once the patch is in mainline.
>>+ 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?
Agreed.
>>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?
If I had enough time to try making the certs BLOBS, how would you
represent this change in the schema (so that db migration is possible).
Currently we have
value not null, -- opaque blob
signature not null, -- RSA/SHA1 signature of "address@hidden:val]"
how would you change this to reflect the fact that value and signature
are no longer base64 encoded.
Christof
I'll (try to) prepare a new and corrected merge during this day.
signature.asc
Description: OpenPGP digital signature