You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2010/09/15 20:17:18 UTC

Re: svn commit: r997403 - /subversion/trunk/subversion/libsvn_wc/wc-metadata.sql

On Wed, Sep 15, 2010 at 13:19,  <ju...@apache.org> wrote:
>...
> -    - ### A comment on 'repos_id' and 'repos_relpath' says they may be null;
> -      is this true and wanted?

Yes. That meant "use the parent's information, and extend it to
compute this node's value".

> +    - The Node-Rev group points to the corresponding repository node-rev.
>
> -    - ### A comment on 'revnum' says, "this could be NULL for non-present
> -      nodes -- no info"; is this true and wanted?
> +    - If not used (as specified by the 'presence' table above), the values
> +      are undefined.
> +      ### Perhaps we should set them to null to make it clearer.

Yes, we should.

>
>   Content columns: (kind, properties, depth, target, checksum)
>                     ----  ----------  -----  ------  --------
> @@ -165,6 +177,7 @@ Overview of BASE_NODE columns:
>     - Includes a copy of the corresponding date and author rev-props.
>
>     - If not used (as specified by the 'presence' table above), all null.
> +      ### Not checked; in practice these columns may have undefined values.

They should never be undefined. They should be null.

We should never have undefined information in our schema. That leads
to madness...

Cheers,
-g

RE: svn commit: r997403 - /subversion/trunk/subversion/libsvn_wc/wc-metadata.sql

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Julian Foad [mailto:julian.foad@wandisco.com]
> Sent: donderdag 16 september 2010 19:47
> To: Greg Stein
> Cc: dev@subversion.apache.org
> Subject: Re: svn commit: r997403 -
> /subversion/trunk/subversion/libsvn_wc/wc-metadata.sql
> 
> On Wed, 2010-09-15 at 16:17 -0400, Greg Stein wrote:
> > On Wed, Sep 15, 2010 at 13:19,  <ju...@apache.org> wrote:
> > >...
> > > -    - ### A comment on 'repos_id' and 'repos_relpath' says they
> may be null;
> > > -      is this true and wanted?
> >
> > Yes. That meant "use the parent's information, and extend it to
> > compute this node's value".
> >
> > > +    - The Node-Rev group points to the corresponding repository
> node-rev.
> > >
> > > -    - ### A comment on 'revnum' says, "this could be NULL for non-
> present
> > > -      nodes -- no info"; is this true and wanted?
> > > +    - If not used (as specified by the 'presence' table above),
> the values
> > > +      are undefined.
> > > +      ### Perhaps we should set them to null to make it clearer.
> >
> > Yes, we should.
> 
> OK, I'm trying this - enforcing repos_id/relpath/rev being null for
> not-present nodes).

The rev value on not-present nodes is used during 'svn update' to determine if the not-present marker should be deleted at the end of the update during the revision bump. (It's value is never relevant in other places). If I remember correctly, if the revision is the same revision as the target of the update, the not-present node will be removed during directory close.
(We could probably use a hashtable for this garbage collection handling, instead of the revision. But we had the revision in svn_wc_entry_t.)

> I'm concerned about 'file externals':
> svn_wc__db_temp_op_set_file_external() creates a row with status
> 'not-present' and claims that it is 'switched', and inserts a repos-id
> and repos_relpath.
> 
> I'm not yet clear on what this really means.  Sure it's a 'hack' but I
> guess this is the way we do it for the time being, so, the statement
> that not-present implies repos-id/relpath/revnum are all null is false.

I don't think they are really NULL, but that doesn't tell that the values are relevant.

	Bert 


Re: svn commit: r997403 - /subversion/trunk/subversion/libsvn_wc/wc-metadata.sql

Posted by Julian Foad <ju...@wandisco.com>.
On Wed, 2010-09-15 at 16:17 -0400, Greg Stein wrote:
> On Wed, Sep 15, 2010 at 13:19,  <ju...@apache.org> wrote:
> >...
> > -    - ### A comment on 'repos_id' and 'repos_relpath' says they may be null;
> > -      is this true and wanted?
> 
> Yes. That meant "use the parent's information, and extend it to
> compute this node's value".
> 
> > +    - The Node-Rev group points to the corresponding repository node-rev.
> >
> > -    - ### A comment on 'revnum' says, "this could be NULL for non-present
> > -      nodes -- no info"; is this true and wanted?
> > +    - If not used (as specified by the 'presence' table above), the values
> > +      are undefined.
> > +      ### Perhaps we should set them to null to make it clearer.
> 
> Yes, we should.

OK, I'm trying this - enforcing repos_id/relpath/rev being null for
not-present nodes).

I'm concerned about 'file externals':
svn_wc__db_temp_op_set_file_external() creates a row with status
'not-present' and claims that it is 'switched', and inserts a repos-id
and repos_relpath.  

I'm not yet clear on what this really means.  Sure it's a 'hack' but I
guess this is the way we do it for the time being, so, the statement
that not-present implies repos-id/relpath/revnum are all null is false.


Ignoring that for a moment, I'll start with splitting
svn_wc__db_base_add_absent_node() into two functions: itself for absent
and excluded, and new function '..._add_not_present_node()' for
'not-present'.  In the current code it is handling all three cases, and
treats them all identically.  Splitting the cases up is more sensible.

- Julian


> >   Content columns: (kind, properties, depth, target, checksum)
> >                     ----  ----------  -----  ------  --------
> > @@ -165,6 +177,7 @@ Overview of BASE_NODE columns:
> >     - Includes a copy of the corresponding date and author rev-props.
> >
> >     - If not used (as specified by the 'presence' table above), all null.
> > +      ### Not checked; in practice these columns may have undefined values.
> 
> They should never be undefined. They should be null.
> 
> We should never have undefined information in our schema. That leads
> to madness...
> 
> Cheers,
> -g