On Tue, 18 Jul 2006, Madan S. wrote:
> On Tue, 18 Jul 2006 01:35:56 +0530, Daniel Rall <dlr@collab.net> wrote:
>
> [snip]
> >
> >This is clearly redundant. The following snippet was all added in 20091:
> >
> >...
> >+ }
> >+ apr_hash_set(*result, mergedfrom, APR_HASH_KEY_STRING,
> >pathranges);
> >+
> >+ if (sqlite_result != SQLITE_DONE)
> >+ return svn_error_create(SVN_ERR_FS_SQLITE_ERROR, NULL,
> >+ sqlite3_errmsg(db));
> >+
> >+ apr_hash_set(*result, mergedfrom, APR_HASH_KEY_STRING,
> >+ pathranges);.
> >
> >However, why remove the second occurrence instead of the first? If we
> >detect an error, wouldn't we want to fail-fast instead of setting a
> >possibly bogus value in the hash?
>
> Because, the while loop keeps building valid apr_array_header_t of
> svn_merge_range_t s, which never get set into the hash unless control
> comes out of the while or when control enters either the following if:
> if (lastmergedfrom && strcmp(mergedfrom, lastmergedfrom) != 0)
>
> So, it is possible that while the last row has not returned SQLITE_DONE,
> but we already have some data that we have collected from the db, that we
> might as well put into the hash, before erroring out. Am not very
> confident of this, but I think this might help us debug which exact row
> was the last before the SQLITE error.
r20727
- application/pgp-signature attachment: stored
Received on Tue Jul 18 21:40:15 2006