Re: record identical operator - Mailing list pgsql-hackers

On Thu, Oct 3, 2013 at 11:12 AM, Stephen Frost <sfrost@snowman.net> wrote:
>> Binary equality has existing precedent and is used in
>> numerous places in the code for good reason.  Users might be confused
>> about the use of those semantics in those places also, but AFAICT
>> nobody is.
>
> You've stated that a few times and I've simply not had time to run down
> the validity of it- so, where does internal-to-PG binary equality end up
> being visible to our users?  Independent of that, are there places in
> the backend which could actually be refactored to use these new
> operators where it would reduce code complexity?

Well, the most obvious example is HOT.  README.HOT sayeth:

% The requirement for doing a HOT update is that none of the indexed
% columns are changed.  This is checked at execution time by comparing the
% binary representation of the old and new values.  We insist on bitwise
% equality rather than using datatype-specific equality routines.  The
% main reason to avoid the latter is that there might be multiple notions
% of equality for a datatype, and we don't know exactly which one is
% relevant for the indexes at hand.  We assume that bitwise equality
% guarantees equality for all purposes.

That bit about multiple notions of equality applies here too - because
we don't know what the user will do with the data in the materialized
view, Kevin's looking for an operator which guarantees equality for
all purposes.  Note that HOT could feed everything through the send
and recv functions, as you are proposing here, and that would allow
HOT to apply in cases where it does not currently apply.  We could,
for example, perform a HOT update when a value in a numeric column is
changed from the long format to the short format without changing the
user-perceived value.

You could argue that HOT isn't user-visible, but we certainly advise
people to think about structuring their indexing in a fashion that
does not defeat HOT, so I think to some extent it is user-visible.

Also, in xfunc.sgml, we have this note:
       The planner also sometimes relies on comparing constants via       bitwise equality, so you can get undesirable
planningresults if       logically-equivalent values aren't bitwise equal.
 

There are other places as well.  If two node trees are compared using
equal(), any Const nodes in that tree will be compared for binary
equality.  So for example MergeWithExistingConstraint() will error out
if the constraints are equal under btree equality operators but not
binary equal.  equal() is also used in various places in the planner,
which may be the reason for the above warning.

The point I want to make here is that we have an existing precedent to
use bitwise equality when we want to make sure that values are
equivalent for all purposes, regardless of what opclass or whatever is
in use.  There are not a ton of those places but there are some.

>> On the other hand, if you are *replicating* those data types, then you
>> don't want that tolerance.  If you're checking whether two boxes are
>> equal, you may indeed want the small amount of fuzziness that our
>> comparison operators allow.  But if you're copying a box or a float
>> from one table to another, or from one database to another, you want
>> the values copied exactly, including all of those low-order bits that
>> tend to foul up your comparisons.  That's why float8out() normally
>> doesn't display any extra_float_digits - because you as the user
>> shouldn't be relying on them - but pg_dump does back them up because
>> not doing so would allow errors to propagate.  Similarly here.
>
> I agree that we should be copying the values exactly- and I think we're
> already good there when it comes to doing a *copy*.  I further agree
> that updating the matview should be a copy, but the manner in which
> we're doing that is using an equality check to see if the value needs to
> be updated or not which is where things get a bit fuzzy.  If we were
> consistently copying and updating the value based on some external
> knowledge that the value has changed (similar to how slony works w/
> triggers that dump change sets into a table- it doesn't consider "has
> any value on this row changed?"; the user did an update, presumably for
> some purpose, therefore the change gets recorded and propagated), I'd be
> perfectly happy.

Sure, that'd work, but it doesn't explain what's wrong with Kevin's
proposal.  You're basically saying that memcpy(a, b, len) is OK with
you but if (memcmp(a, b, len) != 0) memcpy(a, b, len) is not OK with
you.  I don't understand how you can endorse copying the value
exactly, but not be OK with the optimization that says, well if it
already matches exactly, then we don't need to copy it.

We can certainly rip out the current implementation of REFRESH
MATERIALIZED VIEW CONCURRENTLY and replace it with something that
deletes every row in the view and reinserts them all, but it will be
far less efficient than what we have now.  All that is anybody is
asking for here is the ability to skip deleting and reinserting rows
that are absolutely identical in the old and new versions of the view.

Your send/recv proposal would let us also skip deleting and
reinserting rows that are ALMOST identical except for
not-normally-user-visible binary format differences... but since we
haven't worried about allowing such cases for e.g. HOT updates, I
don't think we need to worry about them here, either.  In practice,
such changes are rare as hen's teeth anyway.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company