Re: MD5 aggregate

On Mon, Jun 17, 2013 at 11:34:52AM +0100, Dean Rasheed wrote:
> I've been playing around with the idea of an aggregate that computes
> the sum of the md5 hashes of each of its inputs, which I've called
> md5_total() for now, although I'm not particularly wedded to that
> name. Comparing it with md5_agg() on a 100M row table (see attached
> test script) produces interesting results:
> 
> SELECT md5_agg(foo.*::text)
>   FROM (SELECT * FROM foo ORDER BY id) foo;
> 
>  50bc42127fb9b028c9708248f835ed8f
> 
> Time: 92960.021 ms
> 
> SELECT md5_total(foo.*::text) FROM foo;
> 
>  02faea7fafee4d253fc94cfae031afc43c03479c
> 
> Time: 96190.343 ms
> 
> Unlike md5_agg(), it is no longer a true MD5 sum (for one thing, its
> result is longer) but it seems like it would be very useful for
> quickly comparing data in SQL, since its value is not dependent on the
> row-order making it easier to use and better performing if there is no
> usable index for ordering.

I took a look at this patch.  First, as Peter suggested upthread, it should
have been two patches: one to optimize calculateDigestFromBuffer() and
callees, another atop the first adding new SQL functions and adjusting the
infrastructure to meet their needs.

> +         <primary>md5_total</primary>
> +        </indexterm>
> +        <function>md5_total(<replaceable class="parameter">expression</replaceable>)</function>
> +       </entry>
> +       <entry><type>text</type> or <type>bytea</type></entry>
> +       <entry><type>text</type></entry>
> +       <entry>
> +        sum of the MD5 hash values of the input values, independent of their
> +        order

This is not specific enough.  We would need to provide either an algorithm
specification or a literature reference.  However, that just leads to the
problem that we should not put a literature-unrecognized cryptographic
algorithm into core PostgreSQL.  I realize that the use case inspiring this
patch wasn't concerned with cryptographic properties, but the association with
md5 immediately makes it a cryptography offering.

md5_agg() is well-defined and not cryptographically novel, and your use case
is credible.  However, not every useful-sometimes function belongs in core; we
mostly stick to functions with ubiquitous value and functions that would be
onerous to implement externally.  (We do carry legacy stuff that wouldn't make
the cut today.)  In my estimation, md5_agg() does not make that cut.  The
variety of intuitive md5_agg() definitions attested upthread doesn't bode well
for its broad applicability.  It could just as well live in an extension
published on PGXN.  Mine is just one opinion, though; I won't be horrified if
the community wants an md5_agg() in core.

> *** a/src/backend/libpq/md5.c
> --- b/src/backend/libpq/md5.c
> ***************
> *** 1,14 ****
>   /*
>    *    md5.c
>    *
> !  *    Implements    the  MD5 Message-Digest Algorithm as specified in
> !  *    RFC  1321.    This  implementation  is a simple one, in that it
> !  *    needs  every  input  byte  to  be  buffered  before doing any
> !  *    calculations.  I  do  not  expect  this  file  to be used for
> !  *    general  purpose  MD5'ing  of large amounts of data, only for
> !  *    generating hashed passwords from limited input.

In other words, performance wasn't a design criterion.  I wonder if we would
be wasting our time with a narrow performance change like removing the data
copy.  What's your opinion of copying pgcrypto's md5 implementation, which
already avoids the data copy?

Thanks,
nm

-- 
Noah Misch
EnterpriseDB                                 http://www.enterprisedb.com