[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1094707 - in /subversion/trunk/subversion: libsvn_wc/ tests/libsvn_wc/

From: Greg Stein <gstein_at_gmail.com>
Date: Mon, 18 Apr 2011 17:16:13 -0400

On Mon, Apr 18, 2011 at 15:41, <rhuijben_at_apache.org> wrote:
>...
> +++ subversion/trunk/subversion/libsvn_wc/wc-queries.sql Mon Apr 18 19:41:26 2011
> @@ -603,6 +603,11 @@ SELECT md5_checksum
> †FROM pristine
> †WHERE checksum = ?1
>
> +-- STMT_SELECT_PRISTINE_SIZE
> +SELECT size
> +FROM pristine
> +WHERE checksum = ?1 LIMIT 1

checksum is the PRIMARY KEY. Thus, it is unique and will select just
one row. The LIMIT 1 is superfluous, and could even be construed as
misleading ("what? more than one row could get selected?")

>...
> +++ subversion/trunk/subversion/libsvn_wc/workqueue.c Mon Apr 18 19:41:26 2011
> @@ -625,7 +625,7 @@ process_commit_file_install(svn_wc__db_t
> † † † †"repair" them if the file is unmodified */
> † † † SVN_ERR(svn_wc__internal_file_modified_p(&modified, NULL, NULL,
> † † † † † † † † † † † † † † † † † † † † † † † †db, local_abspath,
> - † † † † † † † † † † † † † † † † † † † † † † † TRUE, FALSE, scratch_pool));
> + † † † † † † † † † † † † † † † † † † † † † † † FALSE, FALSE, scratch_pool));

I find this block of the 'if' statement to be very poor form. We don't
use the MODIFIED result, so why are we calling this function? Just for
its *side effects*? That sounds rather janky. Why don't we simply grab
the values and shove them into the db. In other words... just like the
true-block of the 'if' statement. Seems that we can just strike the
'if' and unconditionally grab new values to shove into the db with a
direct call to record_fileinfo().

>...

Cheers,
-g
Received on 2011-04-18 23:16:44 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.