Login | Register
My pages Projects Community openCollabNet

Discussions > dev [DISABLED] > Re: svn commit: r35823 - branches/explore-wc/subversion/libsvn_wc

subversion
Discussion topic

There will be a brief maintenance window every Friday at 17:00 Pacific.
For further details, see CollabNet's maintenance and upgrade policy.

Back to topic list

Re: svn commit: r35823 - branches/explore-wc/subversion/libsvn_wc

Author "Hyrum K dot Wright" <hyrum_wright at mail dot utexas dot edu>
Full name "Hyrum K dot Wright" <hyrum_wright at mail dot utexas dot edu>
Date 2009-02-13 21:04:43 PST
Message On Feb 13, 2009, at 8:38 PM, Greg Stein wrote:

> On Wed, Feb 11, 2009 at 22:57, Hyrum K. Wright
> <hyrum at hyrumwright dot org> wrote:
>> ...
>> +++ branches/explore-wc/​subversion/libsvn_wc​/entries.c Wed Feb 11
>> 13:57:45 2009 (r35823)
>> ...
>> @@ -1138,6 +1147,20 @@ fetch_actual_nodes(apr_hash_t **nodes,
>> }
>>
>> static svn_error_t *
>> +fetch_wc_id(apr_int64_t *wc_id, svn_sqlite__db_t *wc_db)
>> +{
>> + svn_sqlite__stmt_t *stmt;
>> + svn_boolean_t have_row;
>> +
>> + SVN_ERR(svn_sqlite__​get_statement(&s​tmt, wc_db,
>> STMT_SELECT_WCROOT_NULL));
>> + SVN_ERR(svn_sqlite__​step(&have_row, stmt));
>> + if (!have_row)
>> + return svn_error_create(SVN​_ERR_WC_DB_ERROR, NULL, _("No WC
>> table entry"));
>
> Simplify: use svn_sqlite__step_row(). And in lots of other places!
>
> And its companion svn_sqlite__step_done().

Maybe I'm obtuse, or maybe it's just late, but the use of those APIs
isn't readily intuitive. It appears that I need to know a priori how
many rows there are to fetch, which isn't the case here.

Oh, and svn_sqlite__step_done() finalizes the statement, and we'd
rather have it reset instead.

>
>> + *wc_id = svn_sqlite__column_int(stmt, 0);
>
> I just looked at that SQL statement, and it tests local_abspath for
> being NULL. But local_abspath is declared as NOT NULL.
>
> Further, looking at the INSERT for that row, the SQL has a value to
> insert, but we bind nothing to it.
>
> I think the right answer is to change the schema, and add a comment
> that NULL means "implied by the location this database in the
> filesystem. e.g. for /some/path/.svn/wc.db, the local_abspath is
> implied as '/some/path'"

I did this on the branch, and merged the change to wc-metadata.sql to
trunk in r35873.

>> ...
>> + /* Also remove from the sqlite database. */
>> + /* Open the wc.db sqlite database. */
>> + SVN_ERR(svn_sqlite__​open(&wc_db, db_path(parent_dir,
>> scratch_pool),
>> + svn_sqlite__mode_readwrite, statements,
>> + SVN_WC__VERSION, upgrade_sql,
>> + scratch_pool, scratch_pool));
>
> Note that we "could" cache open databases in the adm_access stuff. But
> I believe that is going away entirely (for API consistency, we'll have
> a placebo adm_access structure). Further, all sqlite db handles will
> be managed (and cached/reused/etc) under the wc_db.h covers.

Agreed. Let's worry about caching sqlite handles once all this moves
behind the wc_db interface.

-Hyrum

« Previous message in topic | 2 of 5 | Next message in topic »

Messages

Show all messages in topic

Re: svn commit: r35823 - branches/explore-wc/subversion/libsvn_wc gstein Greg Stein 2009-02-13 18:38:15 PST
     Re: svn commit: r35823 - branches/explore-wc/subversion/libsvn_wc "Hyrum K dot Wright" <hyrum_wright at mail dot utexas dot edu> "Hyrum K dot Wright" <hyrum_wright at mail dot utexas dot edu> 2009-02-13 21:04:43 PST
         Re: svn commit: r35823 - branches/explore-wc/subversion/libsvn_wc gstein Greg Stein 2009-02-14 02:56:26 PST
             Re: svn commit: r35823 - branches/explore-wc/subversion/libsvn_wc "Hyrum K dot Wright" <hyrum_wright at mail dot utexas dot edu> "Hyrum K dot Wright" <hyrum_wright at mail dot utexas dot edu> 2009-02-17 13:34:48 PST
                 Re: svn commit: r35823 - branches/explore-wc/subversion/libsvn_wc gstein Greg Stein 2009-02-17 13:55:18 PST
Messages per page: