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.

-- 
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download