record identical operator
Attached is a patch for a bit of infrastructure I believe to be
necessary for correct behavior of REFRESH MATERIALIZED VIEW
CONCURRENTLY as well as incremental maintenance of matviews.
The idea is that after RMVC or incremental maintenance, the matview
should not be visibly different that it would have been at creation
or on a non-concurrent REFRESH. The issue is easy to demonstrate
with citext, but anywhere that the = operator allows user-visible
differences between "equal" values it can be an issue.
test=# CREATE TABLE citext_table (
test(# id serial primary key,
test(# name citext
test(# );
CREATE TABLE
test=# INSERT INTO citext_table (name)
test-# VALUES ('one'), ('two'), ('three'), (NULL), (NULL);
INSERT 0 5
test=# CREATE MATERIALIZED VIEW citext_matview AS
test-# SELECT * FROM citext_table;
SELECT 5
test=# CREATE UNIQUE INDEX citext_matview_id
test-# ON citext_matview (id);
CREATE INDEX
test=# UPDATE citext_table SET name = 'Two' WHERE name = 'TWO';
UPDATE 1
At this point, the table and the matview have visibly different
values, yet without the patch the query used to find differences
for RMVC would be essentially like this (slightly simplified for
readability):
test=# SELECT *
FROM citext_matview m
FULL JOIN citext_table t ON (t.id = m.id AND t = m)
WHERE t IS NULL OR m IS NULL;
id | name | id | name
----+------+----+------
(0 rows)
No differences were found, so without this patch, the matview would
remain visibly different from the results generated by a run of its
defining query.
The patch adds an "identical" operator (===) for the record type:
test=# SELECT *
FROM citext_matview m
FULL JOIN citext_table t ON (t.id = m.id AND t === m)
WHERE t IS NULL OR m IS NULL;
id | name | id | name
----+------+----+------
| | 2 | Two
2 | two | |
(2 rows)
The difference is now found, so RMVC makes the appropriate change.
test=# REFRESH MATERIALIZED VIEW CONCURRENTLY citext_matview;
REFRESH MATERIALIZED VIEW
test=# SELECT * FROM citext_matview ORDER BY id;
id | name
----+-------
1 | one
2 | Two
3 | three
4 |
5 |
(5 rows)
The patch adds all of the functions, operators, and catalog
information to support merge joins using the "identical" operator.
The new operator is logically similar to IS NOT DISTINCT FROM for a
record, although its implementation is very different. For one
thing, it doesn't replace the operation with column level operators
in the parser. For another thing, it doesn't look up operators for
each type, so the "identical" operator does not need to be
implemented for each type to use it as shown above. It compares
values byte-for-byte, after detoasting. The test for identical
records can avoid the detoasting altogether for any values with
different lengths, and it stops when it finds the first column with
a difference.
I toyed with the idea of supporting hashing of records using this
operator, but could not see how that would be a performance win.
The identical (===) and not identical (!==) operator names were
chosen because of a vague similarity to the "exactly equals"
concepts in JavaScript and PHP, which use that name. The semantics
aren't quite the same, but it seemed close enough not to be too
surprising. The additional operator names seemed natural to me
based on the first two, but I'm not really that attached to these
names for the operators if someone has a better idea.
Since the comparison of record values is not documented (only
comparisons involving row value constructors), it doesn't seem like
we should document this special case. It is intended primarily for
support of matview refresh and maintenance, and it seems likely
that record comparison was not documented on the basis that it is
intended primarily for support of such things as indexing and merge
joins -- so leaving the new operators undocumented seems consistent
with existing policy. I'm open to arguments that the policy should
change.
--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachments:
record-identical-v1.patchtext/x-diff; name=record-identical-v1.patchDownload+647-11
On Thu, Sep 12, 2013 at 11:27 PM, Kevin Grittner <kgrittn@ymail.com> wrote:
Attached is a patch for a bit of infrastructure I believe to be
necessary for correct behavior of REFRESH MATERIALIZED VIEW
CONCURRENTLY as well as incremental maintenance of matviews.
[...]
The patch adds an "identical" operator (===) for the record type:
[...]
The new operator is logically similar to IS NOT DISTINCT FROM for a
record, although its implementation is very different. For one
thing, it doesn't replace the operation with column level operators
in the parser. For another thing, it doesn't look up operators for
each type, so the "identical" operator does not need to be
implemented for each type to use it as shown above. It compares
values byte-for-byte, after detoasting. The test for identical
records can avoid the detoasting altogether for any values with
different lengths, and it stops when it finds the first column with
a difference.I toyed with the idea of supporting hashing of records using this
operator, but could not see how that would be a performance win.The identical (===) and not identical (!==) operator names were
chosen because of a vague similarity to the "exactly equals"
concepts in JavaScript and PHP, which use that name. The semantics
aren't quite the same, but it seemed close enough not to be too
surprising. The additional operator names seemed natural to me
based on the first two, but I'm not really that attached to these
names for the operators if someone has a better idea.Since the comparison of record values is not documented (only
comparisons involving row value constructors), it doesn't seem like
we should document this special case. It is intended primarily for
support of matview refresh and maintenance, and it seems likely
that record comparison was not documented on the basis that it is
intended primarily for support of such things as indexing and merge
joins -- so leaving the new operators undocumented seems consistent
with existing policy. I'm open to arguments that the policy should
change.-
Wouldn't it be slightly less surprising / magical to not declare new
operators
but just new primitive functions? In the least invasive version they could
even
be called matview_is_record_identical or similar.
cheers,
Bene
Benedikt Grundmann <bgrundmann@janestreet.com> wrote:
Kevin Grittner <kgrittn@ymail.com> wrote:
Attached is a patch for a bit of infrastructure I believe to be
necessary for correct behavior of REFRESH MATERIALIZED VIEW
CONCURRENTLY as well as incremental maintenance of matviews.
[...]
The patch adds an "identical" operator (===) for the record
type:
Wouldn't it be slightly less surprising / magical to not declare
new operators but just new primitive functions? In the least
invasive version they could even be called
matview_is_record_identical or similar.
I'm not sure what is particularly surprising or magical about new
operators, but it is true that the patch could be smaller if
operators were not added. The SQL functions added by the current
patch to support the operator approach are:
extern Datum record_image_eq(PG_FUNCTION_ARGS);
extern Datum record_image_ne(PG_FUNCTION_ARGS);
extern Datum record_image_lt(PG_FUNCTION_ARGS);
extern Datum record_image_gt(PG_FUNCTION_ARGS);
extern Datum record_image_le(PG_FUNCTION_ARGS);
extern Datum record_image_ge(PG_FUNCTION_ARGS);
extern Datum btrecordimagecmp(PG_FUNCTION_ARGS);
All take two record arguments and all but the last return a
boolean. The last one returns -1, 0, or 1 depending on how the
values compare. All comparisons are based on memcmp() of the data
in its storage format (after detoasting, where applicable).
As currently written, the patch still requires a unique index on
the matview in order to allow RMVC, but this patch was intended to
support removal of that restriction, which is something some people
were saying they wanted. It just seemed best to do that with a
separate patch once we had the mechanism to support it.
RMVC currently generates its "diff" data with a query similar to
this:
test=# explain analyze
test-# SELECT *
test-# FROM citext_matview m
test-# FULL JOIN citext_table t ON (t === m)
test-# WHERE t.id IS NULL OR m.id IS NULL;
test=# \pset pager off
Pager usage is off.
test=# explain analyze
SELECT *
FROM citext_matview m
FULL JOIN citext_table t ON (t.id = m.id AND t === m)
WHERE t.id IS NULL OR m.id IS NULL;
QUERY PLAN
---------------------------------------------------------------------------------------------------------------------
Hash Full Join (cost=1.11..2.24 rows=1 width=16) (actual time=0.056..0.056 rows=0 loops=1)
Hash Cond: (m.id = t.id)
Join Filter: (t.* === m.*)
Filter: ((t.id IS NULL) OR (m.id IS NULL))
Rows Removed by Filter: 5
-> Seq Scan on citext_matview m (cost=0.00..1.05 rows=5 width=40) (actual time=0.002..0.006 rows=5 loops=1)
-> Hash (cost=1.05..1.05 rows=5 width=40) (actual time=0.023..0.023 rows=5 loops=1)
Buckets: 1024 Batches: 1 Memory Usage: 1kB
-> Seq Scan on citext_table t (cost=0.00..1.05 rows=5 width=40) (actual time=0.010..0.016 rows=5 loops=1)
Total runtime: 0.102 ms
(10 rows)
With the operator support, we can remove the primary key columns
from the join conditions, and it still works, albeit with a slower
plan:
test=# explain analyze
SELECT *
FROM citext_matview m
FULL JOIN citext_table t ON (t === m)
WHERE t.id IS NULL OR m.id IS NULL;
QUERY PLAN
-----------------------------------------------------------------------------------------------------------------------
Merge Full Join (cost=2.22..2.32 rows=1 width=16) (actual time=0.072..0.072 rows=0 loops=1)
Merge Cond: (m.* === t.*)
Filter: ((t.id IS NULL) OR (m.id IS NULL))
Rows Removed by Filter: 5
-> Sort (cost=1.11..1.12 rows=5 width=40) (actual time=0.035..0.035 rows=5 loops=1)
Sort Key: m.*
Sort Method: quicksort Memory: 25kB
-> Seq Scan on citext_matview m (cost=0.00..1.05 rows=5 width=40) (actual time=0.012..0.016 rows=5 loops=1)
-> Sort (cost=1.11..1.12 rows=5 width=40) (actual time=0.014..0.014 rows=5 loops=1)
Sort Key: t.*
Sort Method: quicksort Memory: 25kB
-> Seq Scan on citext_table t (cost=0.00..1.05 rows=5 width=40) (actual time=0.003..0.003 rows=5 loops=1)
Total runtime: 0.128 ms
(13 rows)
So, if the operators are included it will be a very small and
simple patch to relax the requirement for a unique index.
Currently we generate an error if no index columns were found, but
with the new operators we could leave them out and the query would
still work. Adding indexes to matviews would then be just a matter
of optimization, not a requirement to be able to use the RMVC
feature.
Using the functions instead of the operators things work just as
well as long as we use columns in the join conditions, which is
currently based on indexed columns:
test=# explain analyze
test-# SELECT *
test-# FROM citext_matview m
test-# FULL JOIN citext_table t ON (t.id = m.id AND record_image_eq(t, m))
test-# WHERE t.id IS NULL OR m.id IS NULL;
QUERY PLAN
---------------------------------------------------------------------------------------------------------------------
Hash Full Join (cost=1.11..2.24 rows=1 width=16) (actual time=0.056..0.056 rows=0 loops=1)
Hash Cond: (m.id = t.id)
Join Filter: record_image_eq(t.*, m.*)
Filter: ((t.id IS NULL) OR (m.id IS NULL))
Rows Removed by Filter: 5
-> Seq Scan on citext_matview m (cost=0.00..1.05 rows=5 width=40) (actual time=0.003..0.006 rows=5 loops=1)
-> Hash (cost=1.05..1.05 rows=5 width=40) (actual time=0.023..0.023 rows=5 loops=1)
Buckets: 1024 Batches: 1 Memory Usage: 1kB
-> Seq Scan on citext_table t (cost=0.00..1.05 rows=5 width=40) (actual time=0.011..0.015 rows=5 loops=1)
Total runtime: 0.101 ms
(10 rows)
Without columns in the FULL JOIN conditions, the function fails
entirely, because it is the operator information which lets the
planner know that a FULL JOIN is even possible:
test=# explain analyze
test-# SELECT *
test-# FROM citext_matview m
test-# FULL JOIN citext_table t ON (record_image_eq(t, m))
test-# WHERE t.id IS NULL OR m.id IS NULL;
ERROR: FULL JOIN is only supported with merge-joinable or hash-joinable join conditions
So, either the indexes would continue to be required, or we would
need some other criteria for deciding which columns to use for the
additional FULL JOIN criteria.
--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi Kevin,
On 2013-09-12 15:27:27 -0700, Kevin Grittner wrote:
The new operator is logically similar to IS NOT DISTINCT FROM for a
record, although its implementation is very different.� For one
thing, it doesn't replace the operation with column level operators
in the parser.� For another thing, it doesn't look up operators for
each type, so the "identical" operator does not need to be
implemented for each type to use it as shown above.� It compares
values byte-for-byte, after detoasting.� The test for identical
records can avoid the detoasting altogether for any values with
different lengths, and it stops when it finds the first column with
a difference.
In the general case, that operator sounds dangerous to me. We don't
guarantee that a Datum containing the same data always has the same
binary representation. E.g. array can have a null bitmap or may not have
one, depending on how they were created.
I am not actually sure whether that's a problem for your usecase, but I
get headaches when we try circumventing the type abstraction that way.
Yes, we do such tricks in other places already, but afaik in all those
places errorneously believing two Datums are distinct is not error, just
a missed optimization. Allowing a general operator with such a murky
definition to creep into something SQL exposed... Hm. Not sure.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andres Freund <andres@2ndquadrant.com> wrote:
On 2013-09-12 15:27:27 -0700, Kevin Grittner wrote:
The new operator is logically similar to IS NOT DISTINCT FROM for a
record, although its implementation is very different. For one
thing, it doesn't replace the operation with column level operators
in the parser. For another thing, it doesn't look up operators for
each type, so the "identical" operator does not need to be
implemented for each type to use it as shown above. It compares
values byte-for-byte, after detoasting. The test for identical
records can avoid the detoasting altogether for any values with
different lengths, and it stops when it finds the first column with
a difference.In the general case, that operator sounds dangerous to me. We don't
guarantee that a Datum containing the same data always has the same
binary representation. E.g. array can have a null bitmap or may not have
one, depending on how they were created.I am not actually sure whether that's a problem for your usecase, but I
get headaches when we try circumventing the type abstraction that way.Yes, we do such tricks in other places already, but afaik in all those
places errorneously believing two Datums are distinct is not error, just
a missed optimization. Allowing a general operator with such a murky
definition to creep into something SQL exposed... Hm. Not sure.
Well, the only two alternatives I could see were to allow
user-visible differences not to be carried to the matview if they
old and new values were considered "equal", or to implement an
"identical" operator or function in every type that was to be
allowed in a matview. Given those options, what's in this patch
seemed to me to be the least evil.
It might be worth noting that this scheme doesn't have a problem
with correctness if there are multiple equal values which are not
identical, as long as any two identical values are equal. If the
query which generates contents for a matview generates
non-identical but equal values from one run to the next without any
particular reason, that might cause performance problems.
To mangle Orwell: "Among pairs of equal values, some pairs are more
equal than others."
--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2013-09-13 14:36:27 -0700, Kevin Grittner wrote:
Andres Freund <andres@2ndquadrant.com> wrote:
On 2013-09-12 15:27:27 -0700, Kevin Grittner wrote:
The new operator is logically similar to IS NOT DISTINCT FROM for a
record, although its implementation is very different. For one
thing, it doesn't replace the operation with column level operators
in the parser. For another thing, it doesn't look up operators for
each type, so the "identical" operator does not need to be
implemented for each type to use it as shown above. It compares
values byte-for-byte, after detoasting. The test for identical
records can avoid the detoasting altogether for any values with
different lengths, and it stops when it finds the first column with
a difference.In the general case, that operator sounds dangerous to me. We don't
guarantee that a Datum containing the same data always has the same
binary representation. E.g. array can have a null bitmap or may not have
one, depending on how they were created.I am not actually sure whether that's a problem for your usecase, but I
get headaches when we try circumventing the type abstraction that way.Yes, we do such tricks in other places already, but afaik in all those
places errorneously believing two Datums are distinct is not error, just
a missed optimization. Allowing a general operator with such a murky
definition to creep into something SQL exposed... Hm. Not sure.Well, the only two alternatives I could see were to allow
user-visible differences not to be carried to the matview if they
old and new values were considered "equal", or to implement an
"identical" operator or function in every type that was to be
allowed in a matview. Given those options, what's in this patch
seemed to me to be the least evil.It might be worth noting that this scheme doesn't have a problem
with correctness if there are multiple equal values which are not
identical, as long as any two identical values are equal. If the
query which generates contents for a matview generates
non-identical but equal values from one run to the next without any
particular reason, that might cause performance problems.
I am not actually that concerned with MVCs using this, you're quite
capable of analyzing the dangers. What I am wary of is exposing an
operator that's basically broken from the get go to SQL.
Now, the obvious issue there is that matviews use SQL to refresh :(
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andres Freund <andres@2ndquadrant.com> wrote:
I am not actually that concerned with MVCs using this, you're quite
capable of analyzing the dangers. What I am wary of is exposing an
operator that's basically broken from the get go to SQL.
Now, the obvious issue there is that matviews use SQL to refresh :(
I'm not sure why these operators are more broken or dangerous than
those which already exist to support the text_pattern_ops and
bpchar_pattern_ops operator families. I could overload those
operator names as much as possible if that seems better. As I said
at the start of the thread, I have no particular attachment to
these operator names. For example, that would mean using ~>=~ as
the operator for record_image_ge() instead of using >==. It would
be trivial to make that adjustment to the patch.
--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2013-09-13 15:13:20 -0700, Kevin Grittner wrote:
Andres Freund <andres@2ndquadrant.com> wrote:
I am not actually that concerned with MVCs using this, you're quite
capable of analyzing the dangers. What I am wary of is exposing an
operator that's basically broken from the get go to SQL.
Now, the obvious issue there is that matviews use SQL to refresh :(I'm not sure why these operators are more broken or dangerous than
those which already exist to support the text_pattern_ops and
bpchar_pattern_ops operator families. I could overload those
operator names as much as possible if that seems better. As I said
at the start of the thread, I have no particular attachment to
these operator names. For example, that would mean using ~>=~ as
the operator for record_image_ge() instead of using >==. It would
be trivial to make that adjustment to the patch.
Hm. I don't see the similarity. Those have pretty clearly defined
behaviour. Not one that's dependendant on padding bytes, null bitmaps
that can or cannot be present and such.
I guess we need input from others here.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andres Freund <andres@2ndquadrant.com> wrote:
Not one that's dependendant on padding bytes, null bitmaps that
can or cannot be present and such.
Can you provide an example of where that's an issue with this
patch?
--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2013-09-13 19:20:11 -0700, Kevin Grittner wrote:
Andres Freund <andres@2ndquadrant.com> wrote:
Not one that's dependendant on padding bytes, null bitmaps that
can or cannot be present and such.Can you provide an example of where that's an issue with this
patch?
I haven't yet tested your patch, but what I am talking about is that
e.g.: SELECT (ARRAY[1,2,3,NULL])[1:3] = ARRAY[1,2,3];
obviously should be true. But both arrays don't have the same binary
representation since the former has a null bitmap, the latter not.
So, if you had a composite type like (int4[]) and would compare that
without invoking operators you'd return something false in some cases
because of the null bitmaps.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andres Freund <andres@2ndquadrant.com> wrote:
what I am talking about is that
e.g.: SELECT (ARRAY[1,2,3,NULL])[1:3] = ARRAY[1,2,3];
obviously should be true.
The patch does not change the behavior of the = operator for any
type under any circumstances.
But both arrays don't have the same binary representation since
the former has a null bitmap, the latter not. So, if you had a
composite type like (int4[]) and would compare that without
invoking operators you'd return something false in some cases
because of the null bitmaps.
Not for the = operator. The new "identical" operator would find
them to not be identical, though.
Since the new operator is only for the record type, I need to wrap
the values in your example:
test=# SELECT row ((ARRAY[1,2,3,NULL])[1:3])::record
test-# = row (ARRAY[1,2,3])::record;
?column?
----------
t
(1 row)
test=# SELECT row ((ARRAY[1,2,3,NULL])[1:3])::record
test-# === row (ARRAY[1,2,3])::record;
?column?
----------
f
(1 row)
Or, to borrow from the citext example using arrays:
test=# CREATE TABLE array_table (
test(# id serial primary key,
test(# nums int4[]
test(# );
CREATE TABLE
test=# INSERT INTO array_table (nums)
test-# VALUES ((ARRAY[1,2,3,NULL])[1:3]), (ARRAY[1,2,3]),
test-# (ARRAY[1,2,3]), (NULL), (NULL);
INSERT 0 5
test=# CREATE MATERIALIZED VIEW array_matview AS
test-# SELECT * FROM array_table;
SELECT 5
test=# CREATE UNIQUE INDEX array_matview_id
test-# ON array_matview (id);
CREATE INDEX
test=# select * from array_matview;
id | nums
----+---------
1 | {1,2,3}
2 | {1,2,3}
3 | {1,2,3}
4 |
5 |
(5 rows)
Note that the on-disk representation of the row where id = 1
differs from the on-disk representation where id in (2,3), both in
the table and the matview.
test=# SELECT *
test-# FROM array_matview m
test-# FULL JOIN array_table t ON (t.id = m.id AND t === m)
test-# WHERE t.id IS NULL OR m.id IS NULL;
id | nums | id | nums
----+------+----+------
(0 rows)
... so the query looking for work for the RMVC statement finds
nothing to do.
test=# UPDATE array_table SET nums = (ARRAY[1,2,3,NULL])[1:3]
test-# WHERE id between 1 and 2;
UPDATE 2
Now we have added an unnecessary bitmap to the on-disk storage of
the value where id = 2.
test=# SELECT *
test-# FROM array_matview m
test-# FULL JOIN array_table t ON (t.id = m.id AND t === m)
test-# WHERE t.id IS NULL OR m.id IS NULL;
id | nums | id | nums
----+---------+----+---------
| | 2 | {1,2,3}
2 | {1,2,3} | |
(2 rows)
... and the query sees that they differ.
test=# REFRESH MATERIALIZED VIEW CONCURRENTLY array_matview;
REFRESH MATERIALIZED VIEW
test=# SELECT *
test-# FROM array_matview m
test-# FULL JOIN array_table t ON (t.id = m.id AND t === m)
test-# WHERE t.id IS NULL OR m.id IS NULL;
id | nums | id | nums
----+------+----+------
(0 rows)
The REFRESH causes them to match again, and later REFRESH runs
won't see a need to do any work there unless the on-disk
representation changes again.
As far as I can see, we have four choices:
(1) Never update values that are "equal", even if they appear
different to the users, as was demonstrated with the citext
example.
(2) Require every data type which can be used in a matview to
implement some new operator or function for "identical". Perhaps
that could be mitigated to only implementat it if equal values can
have user-visible differences.
(3) Embed special cases into record identical tests for types
known to allow multiple on-disk representations which have no
user-visible differences.
(4) Base the need to update a matview column on whether its
on-disk representation is identical to what a new run of the
defining query would generate. If this causes performance problems
for use of a given type in a matview, one possible solution would
be to modify that particular type to use a canonical format when
storing a value into a record. For example, storing an array which
has a bitmap of null values even though there are no nulls in the
array could strip the bitmap as it is stored to the record.
Currently we are using (4). I only included (3) for completeness;
even just typing it as a hypothetical made me want to take a
shower. (1) seems pretty horrid, too. (2) isn't evil, exactly,
but any types which allowed user-visible differences in equal
values would not exhibit correct behavior in matviews until and
unless an "identical" operator was added. It might also perform
noticeably worse than (4).
Option (4) has the advantage of showing correct logical behavior in
all types immediately, and restricting any performance fixes to the
types with the inconsistent storage formats.
--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2013-09-14 11:25:52 -0700, Kevin Grittner wrote:
Andres Freund <andres@2ndquadrant.com> wrote:
what I am talking about is that
e.g.: SELECT (ARRAY[1,2,3,NULL])[1:3] = ARRAY[1,2,3];
obviously should be true.The patch does not change the behavior of the = operator for any
type under any circumstances.
Yes, sure. I wasn't thinking you would.
But both arrays don't have the same binary representation since
the former has a null bitmap, the latter not. So, if you had a
composite type like (int4[]) and would compare that without
invoking operators you'd return something false in some cases
because of the null bitmaps.Not for the = operator. The new "identical" operator would find
them to not be identical, though.
Yep. And I think that's a problem if exposed to SQL. People won't
understand the hazards and end up using it because its faster or
somesuch.
Since the new operator is only for the record type, I need to wrap
the values in your example:
Yes.
The REFRESH causes them to match again, and later REFRESH runs
won't see a need to do any work there unless the on-disk
representation changes again.
Yes, I understand that the matview code itself will just perform
superflous work. We use such comparisons in other parts of the code
similarly.
As far as I can see, we have four choices:
(1) Never update values that are "equal", even if they appear
different to the users, as was demonstrated with the citext
example.
I think, introducing a noticeable amount of infrastructure for this just
because of citext is a bad idea.
At some point we need to replace citext with proper case-insensitive
collation support - then it really might become necessary.
(2) Require every data type which can be used in a matview to
implement some new operator or function for "identical". Perhaps
that could be mitigated to only implementat it if equal values can
have user-visible differences.
That basically would require adding a new member to btree opclasses that
btrees don't need themselves... Hm.
(3) Embed special cases into record identical tests for types
known to allow multiple on-disk representations which have no
user-visible differences.
I think this is a complete nogo. a) I don't forsee we know of all these
cases b) it wouldn't be extensible.
Oh. Now that I've read further, I see you feel the same. Good ;)
(4) Base the need to update a matview column on whether its
on-disk representation is identical to what a new run of the
defining query would generate. If this causes performance problems
for use of a given type in a matview, one possible solution would
be to modify that particular type to use a canonical format when
storing a value into a record. For example, storing an array which
has a bitmap of null values even though there are no nulls in the
array could strip the bitmap as it is stored to the record.
If matview refreshs weren't using plain SQL and thus wouldn't require
exposing that operator to SQL I wouldn't have a problem with this...
There's the ungodly ugly choice of having an matview_equal function (or
operator) that checks if we're doing a refresh atm...
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andres Freund <andres@2ndquadrant.com> wrote:
If matview refreshs weren't using plain SQL and thus wouldn't
require exposing that operator to SQL I wouldn't have a problem
with this...
If RMVC were the end of the story, it might be worth building up a
mass of execution nodes directly, although it would be hard to see
how we could make the right planning choices (e.g., MergeJoin
versus HashJoin) that way. But the whole incremental maintenance
area, to have any chance of working accurately and without an
endless stream of bugs, needs to be based on relational algebra.
There needs to be a way to express that in a much higher level
language than execution node creation. If it doesn't use SQL we
would need to invent a relational language very much like it, which
would be silly when we have a perfectly good language we can
already use. The sky is blue; let's move on.
The test for identical records will be needed in SQL if we want to
have these matview features. We could limit use of that to
contexts where MatViewIncrementalMaintenanceIsEnabled(), but I
don't see the point. If someone uses an undocumented operator, and
uses it inappropriately, they may get a surprising result. We
already have undocumented operators to support record comparison
and pattern opclasses, and if people use those they may get
surprising results. I don't recall any reports of problems from
people actually trying to do so.
--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, Sep 14, 2013 at 08:58:32PM +0200, Andres Freund wrote:
On 2013-09-14 11:25:52 -0700, Kevin Grittner wrote:
Andres Freund <andres@2ndquadrant.com> wrote:
But both arrays don't have the same binary representation since
the former has a null bitmap, the latter not. So, if you had a
composite type like (int4[]) and would compare that without
invoking operators you'd return something false in some cases
because of the null bitmaps.Not for the = operator. The new "identical" operator would find
them to not be identical, though.Yep. And I think that's a problem if exposed to SQL. People won't
understand the hazards and end up using it because its faster or
somesuch.
The important question is whether to document the new operator and/or provide
it under a guessable name. If we give the operator a weird name, don't
document it, and put an "internal use only" comment in the catalogs, that is
essentially as good as hiding this feature at the SQL level.
I'm of two minds on that question. On the one hand, MV maintenance is hardly
the first use case for an identity operator. Any replication system or user
space materialized view implementation might want this. On the other hand,
offering it for the record type exclusively is surprising. It's also
surprising how records with different numbers of dropped columns can be found
identical, even though a record column within the top-level record is not
permitted to vary that way.
Supposing a decision to document the operator, a second question is whether
"===" is the right name:
On Thu, Sep 12, 2013 at 03:27:27PM -0700, Kevin Grittner wrote:
The identical (===) and not identical (!==) operator names were
chosen because of a vague similarity to the "exactly equals"
concepts in JavaScript and PHP, which use that name.� The semantics
aren't quite the same, but it seemed close enough not to be too
surprising.
Maybe. If we were mimicking the JavaScript/PHP operator of the same name,
'1.0'::numeric === '1.00'::numeric would return true, and '1'::int4 ===
'1'::int2 would return false. The patch submitted returns false for the first
example and raises a "cannot compare dissimilar column types" error when
comparing an int4 record column to an int2 record column.
I think, introducing a noticeable amount of infrastructure for this just
because of citext is a bad idea.
At some point we need to replace citext with proper case-insensitive
collation support - then it really might become necessary.
citext is just one example. Others include '1.0'::numeric = '1.00'::numeric
and '30 day'::interval = '1 mon'::interval.
(2) Require every data type which can be used in a matview to
implement some new operator or function for "identical". Perhaps
that could be mitigated to only implementat it if equal values can
have user-visible differences.That basically would require adding a new member to btree opclasses that
btrees don't need themselves... Hm.
That's the wrong way to model it. Identity is a particular kind of equality,
which is to say a particular hash opclass. It so happens that memcmp() is the
logical way to implement most/all identity operators, so we can get a btree
opclass as well.
Type-specific identity operators seem like overkill, anyway. If we find that
meaningless variations in a particular data type are causing too many false
non-matches for the generic identity operator, the answer is to make the
functions generating datums of that type settle on a canonical form. That
would be the solution for your example involving array null bitmaps.
--
Noah Misch
EnterpriseDB http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 09/15/2013 01:35 PM, Kevin Grittner wrote:
Andres Freund <andres@2ndquadrant.com> wrote:
If matview refreshs weren't using plain SQL and thus wouldn't
require exposing that operator to SQL I wouldn't have a problem
with this...If RMVC were the end of the story, it might be worth building up a
mass of execution nodes directly, although it would be hard to see
how we could make the right planning choices (e.g., MergeJoin
versus HashJoin) that way. But the whole incremental maintenance
area, to have any chance of working accurately and without an
endless stream of bugs, needs to be based on relational algebra.
There needs to be a way to express that in a much higher level
language than execution node creation. If it doesn't use SQL we
would need to invent a relational language very much like it, which
would be silly when we have a perfectly good language we can
already use. The sky is blue; let's move on.The test for identical records will be needed in SQL if we want to
have these matview features. We could limit use of that to
contexts where MatViewIncrementalMaintenanceIsEnabled(), but I
don't see the point. If someone uses an undocumented operator, and
uses it inappropriately, they may get a surprising result.
Just remember to document it as "undocumented" so
people will know not to use them ;)
Lots of people were bitten when (undocumented) hash
functions were changed thus breaking hash-based partitioning.
Cheers
--
Hannu Krosing
PostgreSQL Consultant
Performance, Scalability and High Availability
2ndQuadrant Nordic O�
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hannu Krosing <hannu@2ndQuadrant.com> wrote:
Lots of people were bitten when (undocumented) hash
functions were changed thus breaking hash-based partitioning.
Nobody can be affected by the new operators in this patch unless
they choose to use them to compare two records. They don't work
for any other type and they don't come into play unless you
specifically request them.
--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2013-09-15 19:49:26 -0400, Noah Misch wrote:
On Sat, Sep 14, 2013 at 08:58:32PM +0200, Andres Freund wrote:
On 2013-09-14 11:25:52 -0700, Kevin Grittner wrote:
Andres Freund <andres@2ndquadrant.com> wrote:
But both arrays don't have the same binary representation since
the former has a null bitmap, the latter not. So, if you had a
composite type like (int4[]) and would compare that without
invoking operators you'd return something false in some cases
because of the null bitmaps.Not for the = operator. The new "identical" operator would find
them to not be identical, though.Yep. And I think that's a problem if exposed to SQL. People won't
understand the hazards and end up using it because its faster or
somesuch.The important question is whether to document the new operator and/or provide
it under a guessable name. If we give the operator a weird name, don't
document it, and put an "internal use only" comment in the catalogs, that is
essentially as good as hiding this feature at the SQL level.
Doesn't match my experience.
Type-specific identity operators seem like overkill, anyway. If we find that
meaningless variations in a particular data type are causing too many false
non-matches for the generic identity operator, the answer is to make the
functions generating datums of that type settle on a canonical form. That
would be the solution for your example involving array null bitmaps.
I think that's pretty much unrealistic. I am pretty sure that if either
of us starts looking we will find at about a dozen of such cases and
miss the other dozen. Not to speak about external code which is damn
likely to contain such cases.
And I think that efficiency will often make such normalization expensive
(consider postgis where Datums afaik can exist with an internal bounding
box or without).
I think it's far more realistic to implement an identity operator that
will fall back to a type specific operator iff equals has "strange"
properties.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andres Freund <andres@2ndquadrant.com> wrote:
I think it's far more realistic to implement an identity operator
that will fall back to a type specific operator iff equals has
"strange" properties.
My biggest problem with that approach is that it defaults to
incorrect behavior for types for which user-visible differences
among "equal" values are possible, and requires per-type fixes to
get correct behavior. The approach in this patch provides correct
behavior as the default, with possible per-type changes for
optimization, as people find that desirable.
--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sun, Sep 15, 2013 at 6:49 PM, Noah Misch <noah@leadboat.com> wrote:
On Sat, Sep 14, 2013 at 08:58:32PM +0200, Andres Freund wrote:
On 2013-09-14 11:25:52 -0700, Kevin Grittner wrote:
Andres Freund <andres@2ndquadrant.com> wrote:
But both arrays don't have the same binary representation since
the former has a null bitmap, the latter not. So, if you had a
composite type like (int4[]) and would compare that without
invoking operators you'd return something false in some cases
because of the null bitmaps.Not for the = operator. The new "identical" operator would find
them to not be identical, though.Yep. And I think that's a problem if exposed to SQL. People won't
understand the hazards and end up using it because its faster or
somesuch.The important question is whether to document the new operator and/or provide
it under a guessable name. If we give the operator a weird name, don't
document it, and put an "internal use only" comment in the catalogs, that is
essentially as good as hiding this feature at the SQL level.I'm of two minds on that question. On the one hand, MV maintenance is hardly
the first use case for an identity operator. Any replication system or user
space materialized view implementation might want this. On the other hand,
offering it for the record type exclusively is surprising. It's also
surprising how records with different numbers of dropped columns can be found
identical, even though a record column within the top-level record is not
permitted to vary that way.Supposing a decision to document the operator, a second question is whether
"===" is the right name:
I vote to reserve '===' as shorthand for 'IS NOT DISTINCT FROM' and
give the binary equality operator a funky name. I would document the
operator though.
merlin
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 09/16/2013 04:01 PM, Kevin Grittner wrote:
Hannu Krosing <hannu@2ndQuadrant.com> wrote:
Lots of people were bitten when (undocumented) hash
functions were changed thus breaking hash-based partitioning.Nobody can be affected by the new operators in this patch unless
they choose to use them to compare two records. They don't work
for any other type and they don't come into play unless you
specifically request them.
What I meant is that rather than leave it really undocumented,
document it as "system function for specific usage, has caveats
and may change in future versions. use at your own risk and
make sure you know what you are doing"
PostgreSQL has good enough introspection features that people
tend to find functions and operators using psql-s \d ...
--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Hannu Krosing
PostgreSQL Consultant
Performance, Scalability and High Availability
2ndQuadrant Nordic O�
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
