Greg Stein wrote:
> That sounds like a great idea! +1
>
> It shouldn't be hard to tweak transform_sql.py. If you have a specific
> prefix for the "token" symbols (eg TOKEN_BASE_DELETED), then we can
> have the transform script simply look for lines like you suggest in
> (say) wc_db.h. If the search regex is strong enough, then it will have
> no problem finding them. And if transform_sql.py sees TOKEN_FOO that
> it does NOT recognize, then it should throw an error. Either somebody
> forgot the mapping, or the syntax was not conformant -- both are
> "errors".
>
> I would be near -1 on putting short numeric values into the db; I
> chose the tokens for debuggability, and think that is quite important.
> I like your improvement on the strengthening the db/token system.
>
> Cheers,
> -g
>
>
> On Fri, Dec 7, 2012 at 12:54 PM, Philip Martin <phi...@codematters.co.uk>
> wrote:
>> Columns such as nodes.kind, nodes.presence, etc. have strings that
>> should be one of a discrete set of values. When we bind these columns
>> in C code we use something like:
>>
>> svn_sqlite__bindf("t", presence_map, svn_wc__db_status_normal);
>>
>> This means we only use known values (svn_wc__db_status_normal) and the
>> map converts it to the correct discrete string. This checking happens
>> at build time.
>>
>> We also have queries where the strings are defined as literals in
>> wc-queries.sql like:
>>
>> DELETE FROM nodes
>> WHERE wc_id = ?1
>> AND IS_STRICT_DESCENDANT_OF(local_relpath, ?2)
>> AND (op_depth < ?3
>> OR (op_depth = ?3 AND presence = 'base-deleted'))
>>
>> There is no checking of these literals to catch errors such as
>> 'base-delete'.
>>
>> I've been thinking that transform_sql.py should do some checking.
>> Perhaps we could move the maps into a know header, annotate them:
>>
>> { "base-deleted", svn_wc__db_status_base_deleted }, /* MAP_DELETED */
>>
>> and then have transform_sql.py parse the header and convert:
>>
>> OR (op_depth = ?3 AND presence = MAP_DELETED))
>>
>> into
>>
>> OR (op_depth = ?3 AND presence = 'base-deleted'))
If we're doing string matching anyway, can we just make it parse the plain
{ "base-deleted", svn_wc__db_status_base_deleted },
lines out of the header file (recognizing that it's a line in a token map, by
whatever means is convenient) and then match the literal
... presence = 'base-deleted'
by the pattern:
... <recognized_column_name> = <valid_token_for_that_column>
Even if we have to append "/* TOKEN_MAP(presence) */" to every line, but
hopefully without that, that's better than "/* MAP_DELETED */".
- Julian
