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

[Christof Petig]

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