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 2010/09/22 21:25:43 UTC
Re: svn commit: r999837 - /subversion/trunk/subversion/libsvn_wc/wc-queries.sql
On Wed, Sep 22, 2010 at 05:39, <ph...@apache.org> wrote:
>...
> +++ subversion/trunk/subversion/libsvn_wc/wc-queries.sql Wed Sep 22 09:39:45 2010
> @@ -215,7 +215,7 @@ update nodes set properties = ?3
> where wc_id = ?1 and local_relpath = ?2
> and op_depth in
> (select op_depth from nodes
> - where wc_id = ?1 and local_relpath = ?2
> + where wc_id = ?1 and local_relpath = ?2 and op_depth > 0
> order by op_depth desc
> limit 1);
Wouldn't it be better to do:
where wc_id = ?1 and local_relpath = ?2
and op_depth = (select max(op_depth) from nodes
where wc_id=?1 and local_relpath=?2 and op_depth > 0);
It seems that eliminating the "order by" and "limit", in favor of
max() will tell sqlite what we're really searching for: the maximal
value.
Also note that the above query uses "op_depth in (...)"
yet:
>
> @@ -312,7 +312,7 @@ WHERE wc_id = ?1 AND local_relpath = ?2;
> update nodes set translated_size = ?3, last_mod_time = ?4
> where wc_id = ?1 and local_relpath = ?2
> and op_depth = (select op_depth from nodes
> - where wc_id = ?1 and local_relpath = ?2
> + where wc_id = ?1 and local_relpath = ?2 and op_depth > 0
> order by op_depth desc
> limit 1);
This one does not. The rest of the statements you converted all use
the "in" variant.
>...
Cheers,
-g
Re: svn commit: r999837 - /subversion/trunk/subversion/libsvn_wc/wc-queries.sql
Posted by Philip Martin <ph...@wandisco.com>.
Greg Stein <gs...@gmail.com> writes:
> On Wed, Sep 22, 2010 at 05:39, <ph...@apache.org> wrote:
>>...
>> +++ subversion/trunk/subversion/libsvn_wc/wc-queries.sql Wed Sep 22 09:39:45 2010
>> @@ -215,7 +215,7 @@ update nodes set properties = ?3
>> where wc_id = ?1 and local_relpath = ?2
>> and op_depth in
>> (select op_depth from nodes
>> - where wc_id = ?1 and local_relpath = ?2
>> + where wc_id = ?1 and local_relpath = ?2 and op_depth > 0
>> order by op_depth desc
>> limit 1);
>
> Wouldn't it be better to do:
>
> where wc_id = ?1 and local_relpath = ?2
> and op_depth = (select max(op_depth) from nodes
> where wc_id=?1 and local_relpath=?2 and op_depth > 0);
>
> It seems that eliminating the "order by" and "limit", in favor of
> max() will tell sqlite what we're really searching for: the maximal
> value.
That does look better, I'll try it tomorrow.
> Also note that the above query uses "op_depth in (...)"
That's probably a mistake.
--
Philip
Re: svn commit: r999837 - /subversion/trunk/subversion/libsvn_wc/wc-queries.sql
Posted by Greg Stein <gs...@gmail.com>.
On Thu, Sep 23, 2010 at 12:53, Erik Huelsmann <eh...@gmail.com> wrote:
> On Wed, Sep 22, 2010 at 11:25 PM, Greg Stein <gs...@gmail.com> wrote:
>...
>> Wouldn't it be better to do:
>>
>> where wc_id = ?1 and local_relpath = ?2
>> and op_depth = (select max(op_depth) from nodes
>> where wc_id=?1 and local_relpath=?2 and op_depth > 0);
>>
>> It seems that eliminating the "order by" and "limit", in favor of
>> max() will tell sqlite what we're really searching for: the maximal
>> value.
>
> I wrote those queries like that because Bert said it would introduce an
> aggregation function - at the time he said it, that sounded like it was
> something negative.
I don't think we should be second-guessing the sqlite query optimizer
unless and until we need to. The 'select max(op_depth)' query can be
optimized. If sqlite does not, then that is not our problem until some
performance data shows these queries are killing us.
>...
>> > @@ -312,7 +312,7 @@ WHERE wc_id = ?1 AND local_relpath = ?2;
>> > update nodes set translated_size = ?3, last_mod_time = ?4
>> > where wc_id = ?1 and local_relpath = ?2
>> > and op_depth = (select op_depth from nodes
>> > - where wc_id = ?1 and local_relpath = ?2
>> > + where wc_id = ?1 and local_relpath = ?2 and op_depth
>> > > 0
>> > order by op_depth desc
>> > limit 1);
>>
>> This one does not. The rest of the statements you converted all use
>> the "in" variant.
>
> The "in" variant is probably better, because - especially with the op_depth
>> 0 restriction - the result set can probably be empty.
Excellent point! Thanks.
Cheers,
-g
Re: svn commit: r999837 - /subversion/trunk/subversion/libsvn_wc/wc-queries.sql
Posted by Philip Martin <ph...@wandisco.com>.
Erik Huelsmann <eh...@gmail.com> writes:
>> > @@ -312,7 +312,7 @@ WHERE wc_id = ?1 AND local_relpath = ?2;
>> > update nodes set translated_size = ?3, last_mod_time = ?4
>> > where wc_id = ?1 and local_relpath = ?2
>> > and op_depth = (select op_depth from nodes
>> > - where wc_id = ?1 and local_relpath = ?2
>> > + where wc_id = ?1 and local_relpath = ?2 and op_depth >
>> 0
>> > order by op_depth desc
>> > limit 1);
>>
>> This one does not. The rest of the statements you converted all use
>> the "in" variant.
>>
>
> The "in" variant is probably better, because - especially with the op_depth
>> 0 restriction - the result set can probably be empty.
Hmm. I switched them all to "=", should they be switched back to
"in"? What would the effect of an empty result set be? Would it
match a null op_depth (prohibited by the schema)?
--
Philip
Re: svn commit: r999837 - /subversion/trunk/subversion/libsvn_wc/wc-queries.sql
Posted by Erik Huelsmann <eh...@gmail.com>.
On Wed, Sep 22, 2010 at 11:25 PM, Greg Stein <gs...@gmail.com> wrote:
> On Wed, Sep 22, 2010 at 05:39, <ph...@apache.org> wrote:
> >...
> > +++ subversion/trunk/subversion/libsvn_wc/wc-queries.sql Wed Sep 22
> 09:39:45 2010
> > @@ -215,7 +215,7 @@ update nodes set properties = ?3
> > where wc_id = ?1 and local_relpath = ?2
> > and op_depth in
> > (select op_depth from nodes
> > - where wc_id = ?1 and local_relpath = ?2
> > + where wc_id = ?1 and local_relpath = ?2 and op_depth > 0
> > order by op_depth desc
> > limit 1);
>
> Wouldn't it be better to do:
>
> where wc_id = ?1 and local_relpath = ?2
> and op_depth = (select max(op_depth) from nodes
> where wc_id=?1 and local_relpath=?2 and op_depth > 0);
>
> It seems that eliminating the "order by" and "limit", in favor of
> max() will tell sqlite what we're really searching for: the maximal
> value.
>
>
I wrote those queries like that because Bert said it would introduce an
aggregation function - at the time he said it, that sounded like it was
something negative.
> Also note that the above query uses "op_depth in (...)"
>
> yet:
>
> >
> > @@ -312,7 +312,7 @@ WHERE wc_id = ?1 AND local_relpath = ?2;
> > update nodes set translated_size = ?3, last_mod_time = ?4
> > where wc_id = ?1 and local_relpath = ?2
> > and op_depth = (select op_depth from nodes
> > - where wc_id = ?1 and local_relpath = ?2
> > + where wc_id = ?1 and local_relpath = ?2 and op_depth >
> 0
> > order by op_depth desc
> > limit 1);
>
> This one does not. The rest of the statements you converted all use
> the "in" variant.
>
The "in" variant is probably better, because - especially with the op_depth
> 0 restriction - the result set can probably be empty.
Bye,
Erik.