[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Adding CHECK to NODES?

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Mon, 11 Mar 2013 12:50:34 +0000 (GMT)


--
Certified & Supported Apache Subversion Downloads: http://www.wandisco.com/subversion/download
----- Original Message -----
> From: Philip Martin <philip.martin_at_wandisco.com>
> To: dev_at_subversion.apache.org
> Cc: 
> Sent: Monday, 4 March 2013, 5:43
> Subject: Re: Adding CHECK to NODES?
> 
> Philip Martin <philip.martin_at_wandisco.com> writes:
> 
>>  philip_at_apache.org writes:
>> 
>>>  Author: philip
>>>  Date: Fri Mar 1 10:02:20 2013
>>>  New Revision: 1451549
>>> 
>>>  URL: http://svn.apache.org/r1451549
>>>  Log:
>>>  Store NULL rather than 0 when not setting NODES.moved_here TRUE.
>>> 
>>>  * subversion/libsvn_wc/wc_db.c
>>>   (insert_working_node, db_op_copy_shadowed_layer): Only bind
>>>   moved_here if set.
>> 
>>  NODES.moved_here is an SQLite INTEGER used as a boolean. Most of the
>>  code uses 1 to indicate moved here and NULL to indicate not moved here.
>>  There were a few places that were setting moved_here to 0, I have now
>>  removed those.
>> 
>>  I tracked down the places that need to be changed by temporarily adding:
>> 
>>   CHECK (moved_here IS NULL OR moved_here = 1)
>> 
>>  to the NODES table definition in wc-metadata.sql. I imagine there is a
>>  cost associated which such a check but it is probably small. Should we
>>  add checks like this permanently? Should we attempt to benchmark the
>>  cost? I guess the cost is only on INSERT/REPLACE/UPDATE and not on
>>  SELECT? Are there other columns that should have CHECKs?
> 
> A CHECK query would be added to the CREATE TABLE and so would always be
> present. We could add it to upgraded working copies using ALTER TABLE.
> Bert suggested using triggers as an alternative. We could have a
> trigger like:
> 
> CREATE TEMPORARY TRIGGER moved_here_update BEFORE UPDATE ON nodes
> WHEN new.moved_here IS NOT NULL AND new.moved_here != 1
> BEGIN
>  RAISE(abort, 'invalid moved_here');
> END
> 
> with a similar trigger on INSERT. Such temporary triggers only exist
> for the duration of the connection which would allow checking only when
> SVN_DEBUG is defined.
+1 on adding this check (and others in the same vein) temporarily when running debug builds. I think this kind of debugging technique can be very valuable.
Also we should consider making an equivalent set of checks be part of some WC validation procedure that can be run from time to time -- for example during "svn cleanup" -- but for existing rows rather then while adding or modifying rows.
- Julian
Received on 2013-03-11 13:54:02 CET

This is an archived mail posted to the Subversion Dev mailing list.