On Mon, Apr 18, 2011 at 15:41,  <rhuij...@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

Reply via email to