You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Julian Foad <ju...@btopenworld.com> on 2010/02/19 15:09:53 UTC

[PATCH] wc-metadata.sql - more doc strings

Bert, Greg, Hyrum, etc. - please see below several more suggested doc
updates/fixes/questions. Please comment, and please feel free to commit
the ones that are good or use them as starting points for better
updates.

(I'm not so much trying to get patches committed, I'm trying to get
folks to understand and agree on the definitions. I'm open to other
suggestions on how to go about it if this doesn't feel the best way.)

- Julian


[[[
* 
  (BASE_NODE, WORKING_NODE, ACTUAL_NODE): Tweak doc strings on
    several columns. One particular change is to say "is null when
    ..." to avoid the ambiguity of "may be null when ...".
]]]

[[[
Index: subversion/libsvn_wc/wc-metadata.sql
===================================================================
--- subversion/libsvn_wc/wc-metadata.sql	(revision 911819)
+++ subversion/libsvn_wc/wc-metadata.sql	(working copy)
@@ -83,11 +83,13 @@ CREATE TABLE BASE_NODE (
   wc_id  INTEGER NOT NULL REFERENCES WCROOT (id),
   local_relpath  TEXT NOT NULL,
 
   /* the repository this node is part of, and the relative path [to its
-     root] within that repository.  these may be NULL, implying it should
-     be derived from the parent and local_relpath.  non-NULL typically
-     indicates a switched node.
+     root] within revision "revnum" of that repository.  These may be NULL,
+     implying they should be derived from the parent and local_relpath.
+     Non-NULL typically indicates a switched node.
+     ### Better: 'These two columns and "revnum" may all be null, implying
+     ### ...'?
 
      Note: they must both be NULL, or both non-NULL. */
   repos_id  INTEGER REFERENCES REPOSITORY (id),
   repos_relpath  TEXT,
@@ -100,27 +102,36 @@ CREATE TABLE BASE_NODE (
   /* Is this node "present" or has it been excluded for some reason?
      The "base-deleted" presence value is not allowed.  */
   presence  TEXT NOT NULL,
 
-  /* what kind of node is this? may be "unknown" if the node is not present */
+  /* The node kind: "file", "dir", or "symlink", or "unknown" if the node is
+     not present. */
   kind  TEXT NOT NULL,
 
-  /* this could be NULL for non-present nodes -- no info. */
+  /* The revision number in which "repos_relpath" applies in "repos_id".
+     May be NULL, implying it should be derived from the parent.
+     (### Can't be null if repos_relpath is not null.)
+     Non-NULL typically indicates a switched node.*/
   revnum  INTEGER,
 
-  /* if this node is a file, then the checksum and its translated size
-     (given the properties on this file) are specified by the following
-     two fields. translated_size may be NULL if the size has not (yet)
-     been computed. The kind of checksum (e.g. SHA-1, MD5) is stored in the
-     value */
+  /* If this node is a file, then the SHA-1 checksum of the pristine text. */
   checksum  TEXT,
+
+  /* The size in bytes of the working file when it had no local text
+     modifications. This means the size of the text when translated from
+     repository-normal format to working copy format with EOL style
+     translated and keywords expanded according to the properties in the
+     "properties" column of this row.
+
+     NULL if this node is not a file or if the size has not (yet) been
+     computed. */
   translated_size  INTEGER,
 
   /* Information about the last change to this node. changed_rev must be
      not-null if this node has presence=="normal". changed_date and
      changed_author may be null if the corresponding revprops are missing.
 
-     All three values may be null for non-present nodes.  */
+     All three values are null for a not-present node.  */
   changed_rev  INTEGER,
   changed_date  INTEGER,  /* an APR date/time (usec since 1970) */
   changed_author  TEXT,
 
@@ -261,20 +272,27 @@ CREATE TABLE WORKING_NODE (
 
   /* the kind of the new node. may be "unknown" if the node is not present. */
   kind  TEXT NOT NULL,
 
-  /* if this node was added-with-history AND is a file, then the checksum
-     and its translated size (given the properties on this file) are
-     specified by the following two fields. translated_size may be NULL
-     if the size has not (yet) been computed. */
+  /* The SHA-1 checksum of the pristine text, if this node is a file and was
+     added-with-history, else NULL. */
   checksum  TEXT,
+
+  /* The size in bytes of the working file when it had no local text
+     modifications. This means the size of the text when translated from
+     repository-normal format to working copy format with EOL style
+     translated and keywords expanded according to the properties in the
+     "properties" column of this row.
+
+     NULL if this node is not a file or is not added-with-history or if
+     the size has not (yet) been computed. */
   translated_size  INTEGER,
 
   /* If this node was added-with-history, then the following fields may
      have information about their source node. See BASE_NODE.changed_* for
      more information.
 
-     For added or not-present nodes, these may be null.  */
+     For an added or not-present node, these are null.  */
   changed_rev  INTEGER,
   changed_date  INTEGER,  /* an APR date/time (usec since 1970) */
   changed_author  TEXT,
 
]]]

- Julian


Re: [PATCH] wc-metadata.sql - more doc strings

Posted by Philip Martin <ph...@wandisco.com>.
Philip Martin <ph...@wandisco.com> writes:

> Philip Martin <ph...@wandisco.com> writes:
>
>> Greg Stein <gs...@gmail.com> writes:
>>
>>> revnum is NOT inheritable.
>>
>> At presnt working_node.parent_relpath says:
>>
>>   /* parent's local_relpath for aggregating children of a given parent.
>>      this will be "" if the parent is the wcroot. NULL if this is the
>>      wcroot node. */
>>   parent_relpath  TEXT,
>>
>> Does it make sense for a working node to be a wcroot?
>
> Sent too early.  I meant to ask is it inheritable?

We will probably want to index on parent_relpath, can we do that if it
is inherited?  If we can't index an inherited column we probably don't
want parent_relpath to be inherited.

-- 
Philip

Re: [PATCH] wc-metadata.sql - more doc strings

Posted by Julian Foad <ju...@btopenworld.com>.
On Fri, 2010-02-19, Greg Stein wrote:
> revnum is NOT inheritable.
> 
> I prefer copied/moved-here rather than "added-with-history". The
> latter phrase is somewhat opaque to readers on what operation was
> actually performed by the user.
> 
> The rest seems fine. Thx!

Thanks. Committed with those changes in r912537.

- Julian


> On Fri, Feb 19, 2010 at 12:30, Philip Martin <ph...@wandisco.com> wrote:
> > Philip Martin <ph...@wandisco.com> writes:
> >
> >> Greg Stein <gs...@gmail.com> writes:
> >>
> >>> revnum is NOT inheritable.
> >>
> >> At presnt working_node.parent_relpath says:
> >>
> >>   /* parent's local_relpath for aggregating children of a given parent.
> >>      this will be "" if the parent is the wcroot. NULL if this is the
> >>      wcroot node. */
> >>   parent_relpath  TEXT,
> >>
> >> Does it make sense for a working node to be a wcroot?
> 
> Hmm. You're right. I'm not sure how you could possibly get a wcroot
> into such a state. You're not allowed to delete it, nor copy/move
> something over it. ... well, maybe we *could*, but it would be quite
> weird to not do that from a checkout of the parent.
> 
> > Sent too early.  I meant to ask is it inheritable?
> 
> Nope. parent_relpath is intended to be a key, so that you can easily
> find all nodes within a given directory. (which is also why we have
> I_PARENT and friends).
> 
> 
> Regarding inheriting repos_* values... I'm beginning to think that may
> be more trouble than its worth. The idea was to avoid having to
> compute them all the time, or to have to update them during a switch,
> etc. But... having to compensate for their absence is now seeming to
> be worse.
> 
> Cheers,
> -g


Re: [PATCH] wc-metadata.sql - more doc strings

Posted by Greg Stein <gs...@gmail.com>.
On Fri, Feb 19, 2010 at 12:30, Philip Martin <ph...@wandisco.com> wrote:
> Philip Martin <ph...@wandisco.com> writes:
>
>> Greg Stein <gs...@gmail.com> writes:
>>
>>> revnum is NOT inheritable.
>>
>> At presnt working_node.parent_relpath says:
>>
>>   /* parent's local_relpath for aggregating children of a given parent.
>>      this will be "" if the parent is the wcroot. NULL if this is the
>>      wcroot node. */
>>   parent_relpath  TEXT,
>>
>> Does it make sense for a working node to be a wcroot?

Hmm. You're right. I'm not sure how you could possibly get a wcroot
into such a state. You're not allowed to delete it, nor copy/move
something over it. ... well, maybe we *could*, but it would be quite
weird to not do that from a checkout of the parent.

> Sent too early.  I meant to ask is it inheritable?

Nope. parent_relpath is intended to be a key, so that you can easily
find all nodes within a given directory. (which is also why we have
I_PARENT and friends).


Regarding inheriting repos_* values... I'm beginning to think that may
be more trouble than its worth. The idea was to avoid having to
compute them all the time, or to have to update them during a switch,
etc. But... having to compensate for their absence is now seeming to
be worse.

Cheers,
-g

Re: [PATCH] wc-metadata.sql - more doc strings

Posted by Philip Martin <ph...@wandisco.com>.
Philip Martin <ph...@wandisco.com> writes:

> Greg Stein <gs...@gmail.com> writes:
>
>> revnum is NOT inheritable.
>
> At presnt working_node.parent_relpath says:
>
>   /* parent's local_relpath for aggregating children of a given parent.
>      this will be "" if the parent is the wcroot. NULL if this is the
>      wcroot node. */
>   parent_relpath  TEXT,
>
> Does it make sense for a working node to be a wcroot?

Sent too early.  I meant to ask is it inheritable?

-- 
Philip

Re: [PATCH] wc-metadata.sql - more doc strings

Posted by Philip Martin <ph...@wandisco.com>.
Greg Stein <gs...@gmail.com> writes:

> revnum is NOT inheritable.

At presnt working_node.parent_relpath says:

  /* parent's local_relpath for aggregating children of a given parent.
     this will be "" if the parent is the wcroot. NULL if this is the
     wcroot node. */
  parent_relpath  TEXT,

Does it make sense for a working node to be a wcroot?

-- 
Philip

Re: [PATCH] wc-metadata.sql - more doc strings

Posted by Greg Stein <gs...@gmail.com>.
revnum is NOT inheritable.

I prefer copied/moved-here rather than "added-with-history". The
latter phrase is somewhat opaque to readers on what operation was
actually performed by the user.

The rest seems fine. Thx!

Cheers,
-g

On Fri, Feb 19, 2010 at 10:09, Julian Foad <ju...@btopenworld.com> wrote:
> Bert, Greg, Hyrum, etc. - please see below several more suggested doc
> updates/fixes/questions. Please comment, and please feel free to commit
> the ones that are good or use them as starting points for better
> updates.
>
> (I'm not so much trying to get patches committed, I'm trying to get
> folks to understand and agree on the definitions. I'm open to other
> suggestions on how to go about it if this doesn't feel the best way.)
>
> - Julian
>
>
> [[[
> *
>  (BASE_NODE, WORKING_NODE, ACTUAL_NODE): Tweak doc strings on
>    several columns. One particular change is to say "is null when
>    ..." to avoid the ambiguity of "may be null when ...".
> ]]]
>
> [[[
> Index: subversion/libsvn_wc/wc-metadata.sql
> ===================================================================
> --- subversion/libsvn_wc/wc-metadata.sql        (revision 911819)
> +++ subversion/libsvn_wc/wc-metadata.sql        (working copy)
> @@ -83,11 +83,13 @@ CREATE TABLE BASE_NODE (
>   wc_id  INTEGER NOT NULL REFERENCES WCROOT (id),
>   local_relpath  TEXT NOT NULL,
>
>   /* the repository this node is part of, and the relative path [to its
> -     root] within that repository.  these may be NULL, implying it should
> -     be derived from the parent and local_relpath.  non-NULL typically
> -     indicates a switched node.
> +     root] within revision "revnum" of that repository.  These may be NULL,
> +     implying they should be derived from the parent and local_relpath.
> +     Non-NULL typically indicates a switched node.
> +     ### Better: 'These two columns and "revnum" may all be null, implying
> +     ### ...'?
>
>      Note: they must both be NULL, or both non-NULL. */
>   repos_id  INTEGER REFERENCES REPOSITORY (id),
>   repos_relpath  TEXT,
> @@ -100,27 +102,36 @@ CREATE TABLE BASE_NODE (
>   /* Is this node "present" or has it been excluded for some reason?
>      The "base-deleted" presence value is not allowed.  */
>   presence  TEXT NOT NULL,
>
> -  /* what kind of node is this? may be "unknown" if the node is not present */
> +  /* The node kind: "file", "dir", or "symlink", or "unknown" if the node is
> +     not present. */
>   kind  TEXT NOT NULL,
>
> -  /* this could be NULL for non-present nodes -- no info. */
> +  /* The revision number in which "repos_relpath" applies in "repos_id".
> +     May be NULL, implying it should be derived from the parent.
> +     (### Can't be null if repos_relpath is not null.)
> +     Non-NULL typically indicates a switched node.*/
>   revnum  INTEGER,
>
> -  /* if this node is a file, then the checksum and its translated size
> -     (given the properties on this file) are specified by the following
> -     two fields. translated_size may be NULL if the size has not (yet)
> -     been computed. The kind of checksum (e.g. SHA-1, MD5) is stored in the
> -     value */
> +  /* If this node is a file, then the SHA-1 checksum of the pristine text. */
>   checksum  TEXT,
> +
> +  /* The size in bytes of the working file when it had no local text
> +     modifications. This means the size of the text when translated from
> +     repository-normal format to working copy format with EOL style
> +     translated and keywords expanded according to the properties in the
> +     "properties" column of this row.
> +
> +     NULL if this node is not a file or if the size has not (yet) been
> +     computed. */
>   translated_size  INTEGER,
>
>   /* Information about the last change to this node. changed_rev must be
>      not-null if this node has presence=="normal". changed_date and
>      changed_author may be null if the corresponding revprops are missing.
>
> -     All three values may be null for non-present nodes.  */
> +     All three values are null for a not-present node.  */
>   changed_rev  INTEGER,
>   changed_date  INTEGER,  /* an APR date/time (usec since 1970) */
>   changed_author  TEXT,
>
> @@ -261,20 +272,27 @@ CREATE TABLE WORKING_NODE (
>
>   /* the kind of the new node. may be "unknown" if the node is not present. */
>   kind  TEXT NOT NULL,
>
> -  /* if this node was added-with-history AND is a file, then the checksum
> -     and its translated size (given the properties on this file) are
> -     specified by the following two fields. translated_size may be NULL
> -     if the size has not (yet) been computed. */
> +  /* The SHA-1 checksum of the pristine text, if this node is a file and was
> +     added-with-history, else NULL. */
>   checksum  TEXT,
> +
> +  /* The size in bytes of the working file when it had no local text
> +     modifications. This means the size of the text when translated from
> +     repository-normal format to working copy format with EOL style
> +     translated and keywords expanded according to the properties in the
> +     "properties" column of this row.
> +
> +     NULL if this node is not a file or is not added-with-history or if
> +     the size has not (yet) been computed. */
>   translated_size  INTEGER,
>
>   /* If this node was added-with-history, then the following fields may
>      have information about their source node. See BASE_NODE.changed_* for
>      more information.
>
> -     For added or not-present nodes, these may be null.  */
> +     For an added or not-present node, these are null.  */
>   changed_rev  INTEGER,
>   changed_date  INTEGER,  /* an APR date/time (usec since 1970) */
>   changed_author  TEXT,
>
> ]]]
>
> - Julian
>
>
>