You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Rall <dl...@collab.net> on 2007/05/31 17:55:17 UTC
Re: svn commit: r25231 - trunk/subversion/libsvn_fs_util
On Thu, 31 May 2007, hwright@tigris.org wrote:
...
> When searching for child mergeinfo, look for proper children of the given path,
> not just paths with a common prefix. Followup to r25138.
...
> * subversion/libsvn_fs_util/mergeinfo-sqlite-index.c
> (get_mergeinfo_for_children): Select only paths which are proper children of
> the requested path.
...
> --- trunk/subversion/libsvn_fs_util/mergeinfo-sqlite-index.c (original)
> +++ trunk/subversion/libsvn_fs_util/mergeinfo-sqlite-index.c Thu May 31 07:50:12 2007
> @@ -619,7 +619,7 @@
> "WHERE path LIKE ? AND revision <= ?;",
> -1, &stmt, NULL), db);
>
> - like_path = apr_psprintf(pool, "%s%%", path);
> + like_path = apr_psprintf(pool, "%s/%%", path);
>
> SQLITE_ERR(sqlite3_bind_text(stmt, 1, like_path, -1, SQLITE_TRANSIENT), db);
> SQLITE_ERR(sqlite3_bind_int64(stmt, 2, rev), db);
...
Looks good, Hyrum. Is our test case exercising this yet? (I think it
should, to prevent regression.)
Re: svn commit: r25231 - trunk/subversion/libsvn_fs_util
Posted by Daniel Rall <dl...@collab.net>.
On Thu, 31 May 2007, Hyrum K. Wright wrote:
> Daniel Rall wrote:
> > On Thu, 31 May 2007, hwright@tigris.org wrote:
> > ...
> >> When searching for child mergeinfo, look for proper children of the given path,
> >> not just paths with a common prefix. Followup to r25138.
> > ...
> >> * subversion/libsvn_fs_util/mergeinfo-sqlite-index.c
> >> (get_mergeinfo_for_children): Select only paths which are proper children of
> >> the requested path.
> > ...
> >> --- trunk/subversion/libsvn_fs_util/mergeinfo-sqlite-index.c (original)
> >> +++ trunk/subversion/libsvn_fs_util/mergeinfo-sqlite-index.c Thu May 31 07:50:12 2007
> >> @@ -619,7 +619,7 @@
> >> "WHERE path LIKE ? AND revision <= ?;",
> >> -1, &stmt, NULL), db);
> >>
> >> - like_path = apr_psprintf(pool, "%s%%", path);
> >> + like_path = apr_psprintf(pool, "%s/%%", path);
> >>
> >> SQLITE_ERR(sqlite3_bind_text(stmt, 1, like_path, -1, SQLITE_TRANSIENT), db);
> >> SQLITE_ERR(sqlite3_bind_int64(stmt, 2, rev), db);
> > ...
> >
> > Looks good, Hyrum. Is our test case exercising this yet? (I think it
> > should, to prevent regression.)
>
> Not directly, but the merge sensitive log functionality depends on this
> function, so the test(s) are hitting this function indirectly. Do we
> have a simple way to directly test this function?
I should've been more clear -- I intended to refer to filtering of
"/tags/1.1.0.1" when the path we're trying to get merge info for is
"/tags/1.1.0" (which you fixed with the addition of the "/" character
in value inserted into the "path LIKE ?" clause).
Re: svn commit: r25231 - trunk/subversion/libsvn_fs_util
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
Daniel Rall wrote:
> On Thu, 31 May 2007, hwright@tigris.org wrote:
> ...
>> When searching for child mergeinfo, look for proper children of the given path,
>> not just paths with a common prefix. Followup to r25138.
> ...
>> * subversion/libsvn_fs_util/mergeinfo-sqlite-index.c
>> (get_mergeinfo_for_children): Select only paths which are proper children of
>> the requested path.
> ...
>> --- trunk/subversion/libsvn_fs_util/mergeinfo-sqlite-index.c (original)
>> +++ trunk/subversion/libsvn_fs_util/mergeinfo-sqlite-index.c Thu May 31 07:50:12 2007
>> @@ -619,7 +619,7 @@
>> "WHERE path LIKE ? AND revision <= ?;",
>> -1, &stmt, NULL), db);
>>
>> - like_path = apr_psprintf(pool, "%s%%", path);
>> + like_path = apr_psprintf(pool, "%s/%%", path);
>>
>> SQLITE_ERR(sqlite3_bind_text(stmt, 1, like_path, -1, SQLITE_TRANSIENT), db);
>> SQLITE_ERR(sqlite3_bind_int64(stmt, 2, rev), db);
> ...
>
> Looks good, Hyrum. Is our test case exercising this yet? (I think it
> should, to prevent regression.)
Not directly, but the merge sensitive log functionality depends on this
function, so the test(s) are hitting this function indirectly. Do we
have a simple way to directly test this function?
-Hyrum