On Mon, May 9, 2011 at 11:43, <rhuij...@apache.org> wrote: >... > +++ subversion/trunk/subversion/libsvn_wc/wc-metadata.sql Mon May 9 15:43:56 > 2011 > @@ -530,7 +530,64 @@ BEGIN > WHERE checksum = OLD.checksum; > END; > > +-- STMT_CREATE_EXTERNALS > > +CREATE TABLE EXTERNALS ( > + /* Working copy location related fields (like NODES)*/ > + > + wc_id INTEGER NOT NULL REFERENCES WCROOT (id), > + local_relpath TEXT NOT NULL, > + > + /* The working copy root can't be recorded as an external in itself > + so this will never be NULL. */ > + parent_relpath TEXT NOT NULL, > + > + /* Repository location fields */ > + > + /* Always set for file and symlink externals. NULL for directory externals > */ > + repos_id INTEGER REFERENCES REPOSITORY (id), > + repos_path TEXT, > + revision INTEGER,
Shouldn't that be repos_relpath? > + > + /* Content fields */ > + > + /* the kind of the external. */ > + kind TEXT NOT NULL, > + > + /* Variouse information (NULL for directories; see NODES for explanation) > */ > + properties BLOB, > + checksum TEXT REFERENCES PRISTINE (checksum), > + symlink_target TEXT, > + > + /* Last-Change fields (NULL for directories; see NODES for explanation) */ > + changed_revision INTEGER, > + changed_date INTEGER, > + changed_author TEXT, > + > + /* Various cache fields (NULL for directories; see NODES for explanation) > */ > + translated_size INTEGER, > + last_mod_time INTEGER, > + dav_cache BLOB, Since this is a new table, I think we can get these columns names to better reflect their semantic (and use in the code): recorded_size and recorded_mod_time. > + > + > + /* The local relpath of the directory NODE defining this external > + (Defaults to the parent directory of the file external after upgrade) */ > + record_relpath TEXT NOT NULL, This name is confusing with the recorded_* columns. Could you use 'definition_relpath' here? It might even by nice to use something like 'def_local_relpath' to indicate the relationship to 'local_relpath'. > + > + /* The url of the external as used in the definition */ > + recorded_url TEXT NOT NULL, Is this a full URL, or a repos_relpath? Please detail in the column since we mostly use <repos_id, repos_relpath> elsewhere. > + > + /* The operational (peg) and node revision if this is a revision fixed > + external; otherwise NULL. (Usually these will both have the same value) > */ > + recorded_operational_revision TEXT NOT NULL, > + recorded_revision TEXT NOT NULL, How about 'defined_*' for these? ... or something besides 'recorded' since we've been using that to imply state from elsewhere that we simply record into the database. The comment says these can be NULL, yet you label them as NOT NULL. > + > + PRIMARY KEY (wc_id, local_relpath) > +); > + > +CREATE INDEX I_EXTERNALS_PARENT ON EXTERNALS (wc_id, parent_relpath); > +CREATE UNIQUE INDEX I_EXTERNALS_RECORDED ON EXTERNALS (wc_id, record_relpath, > + local_relpath); STMT_SELECT_EXTERNALS_DEFINED uses <wc_id, record_relpath>. There is no query that uses the triple you define above, so this index seems pretty useless (and, in fact, can slow things down since it must be maintained). I do see that you also have queries based on <wc_id, local_relpath>. Is the intent to create one index for both types of WHERE clauses? Does the one index work for both? >... Cheers, -g
