You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2011/05/14 21:29:53 UTC

Re: svn commit: r1103177 - in /subversion/trunk/subversion/libsvn_wc: wc-queries.sql wc_db.c

On Sat, May 14, 2011 at 14:19,  <hw...@apache.org> wrote:
>...
> +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Sat May 14 18:19:11 2011
> @@ -2466,10 +2466,18 @@ run_final_query(void *baton,
>                 svn_wc__db_wcroot_t *wcroot,
>                 apr_pool_t *scratch_pool)
>  {
> -  int *finalize_idx = baton;
> +  int *final_queries = baton;
> +  int *query_idx = final_queries;

Seems that 'final_queries' is not needed.

>...
> +  struct set_changelist_baton_t scb = { new_changelist, changelist_filter,
> +                                        depth };
> +  int final_queries[] = { STMT_DROP_CHANGELIST_LIST, STMT_DROP_TARGETS_LIST,
> +                          -1 };

It seems that it would be cleaner to just put the 'DROP TABLE
targets_list' into the STMT_DROP_CHANGELIST_LIST (maybe rename the
latter to something like STMT_FINALIZE_CHANGELIST). Then you could rip
out all this multiple statement stuff. I think one STMT would be
cleaner than all this code to support multiple.

>...

Cheers,
-g

Re: svn commit: r1103177 - in /subversion/trunk/subversion/libsvn_wc: wc-queries.sql wc_db.c

Posted by Greg Stein <gs...@gmail.com>.
On Sat, May 14, 2011 at 16:26, Hyrum K Wright <hy...@hyrumwright.org> wrote:
> On Sat, May 14, 2011 at 7:29 PM, Greg Stein <gs...@gmail.com> wrote:
>...
>>>...
>>> +  struct set_changelist_baton_t scb = { new_changelist, changelist_filter,
>>> +                                        depth };
>>> +  int final_queries[] = { STMT_DROP_CHANGELIST_LIST, STMT_DROP_TARGETS_LIST,
>>> +                          -1 };
>>
>> It seems that it would be cleaner to just put the 'DROP TABLE
>> targets_list' into the STMT_DROP_CHANGELIST_LIST (maybe rename the
>> latter to something like STMT_FINALIZE_CHANGELIST). Then you could rip
>> out all this multiple statement stuff. I think one STMT would be
>> cleaner than all this code to support multiple.
>
> The goal is to use the target_list stuff for multiple operations, such
> as propset (and potentially things like info, too).  As a result, the
> 'DROP TABLE targets_list' will be used in combination with a number of
> other finalize statements.  I suppose we could duplicate that single
> drop statement wherever we need to finalize an operation (and maybe
> that's acceptable duplication), but I'd like to leave it separate, at
> least for now.

That was my thought: just put that DROP into the other finalize
statements. We already drop a bunch of triggers and the change_list
table. I see no reason not to throw a drop of the targets_list in
there, too.

It just seems easier to have multiple SQL operations in one STMT,
rather than simulate the same in C code.

Cheers,
-g

Re: svn commit: r1103177 - in /subversion/trunk/subversion/libsvn_wc: wc-queries.sql wc_db.c

Posted by Hyrum K Wright <hy...@hyrumwright.org>.
On Sat, May 14, 2011 at 7:29 PM, Greg Stein <gs...@gmail.com> wrote:
> On Sat, May 14, 2011 at 14:19,  <hw...@apache.org> wrote:
>>...
>> +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Sat May 14 18:19:11 2011
>> @@ -2466,10 +2466,18 @@ run_final_query(void *baton,
>>                 svn_wc__db_wcroot_t *wcroot,
>>                 apr_pool_t *scratch_pool)
>>  {
>> -  int *finalize_idx = baton;
>> +  int *final_queries = baton;
>> +  int *query_idx = final_queries;
>
> Seems that 'final_queries' is not needed.

Ah, true.  r1103207.

>>...
>> +  struct set_changelist_baton_t scb = { new_changelist, changelist_filter,
>> +                                        depth };
>> +  int final_queries[] = { STMT_DROP_CHANGELIST_LIST, STMT_DROP_TARGETS_LIST,
>> +                          -1 };
>
> It seems that it would be cleaner to just put the 'DROP TABLE
> targets_list' into the STMT_DROP_CHANGELIST_LIST (maybe rename the
> latter to something like STMT_FINALIZE_CHANGELIST). Then you could rip
> out all this multiple statement stuff. I think one STMT would be
> cleaner than all this code to support multiple.

The goal is to use the target_list stuff for multiple operations, such
as propset (and potentially things like info, too).  As a result, the
'DROP TABLE targets_list' will be used in combination with a number of
other finalize statements.  I suppose we could duplicate that single
drop statement wherever we need to finalize an operation (and maybe
that's acceptable duplication), but I'd like to leave it separate, at
least for now.

-Hyrum