On Feb 14, 2009, at 4:56 AM, Greg Stein wrote:
> On Sat, Feb 14, 2009 at 06:04, Hyrum K. Wright
> <hyrum_wright_at_mail.utexas.edu> wrote:
>> 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.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(&stmt, 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.
>
> Feel free to change those two APIs into something that makes more
> sense (just checked; nothing uses them). In the above case, you *know*
> that the step should return a row, so step_row() is Goodness. Shoot...
> if a row isn't returned, you raise an error, which is *exactly* what
> step_row() does. Just wrapped up into a simple API.
>
> For DTML statements, no rows are expected, so step_done() makes sense.
> That said... we already have an __insert() call, so maybe step_done is
> bogus, in favor of __delete() and various __create() entry points. But
> I'd say keep step_done -- we have the __insert() in order to return
> the rowid. That isn't the case for other DTML statements.
Ah gotcha. I'll take a look at where they can be used.
>> Oh, and svn_sqlite__step_done() finalizes the statement, and we'd
>> rather
>> have it reset instead.
>
> No, it doesn't.
In which case, the docs are lying:
/* Steps the given statement; raises an SVN error (and finalizes the
statement) if it doesn't return SQLITE_DONE. */
svn_error_t *
svn_sqlite__step_done(svn_sqlite__stmt_t *stmt);
-Hyrum
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1181226
Received on 2009-02-17 22:35:12 CET