| From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
|---|---|
| To: | surya poondla <suryapoondla4(at)gmail(dot)com> |
| Cc: | Rafia Sabih <rafia(dot)pghackers(at)gmail(dot)com>, cca5507 <cca5507(at)qq(dot)com>, Giuliano Gagliardi <gogi(at)gogi(dot)tv>, pgsql-bugs <pgsql-bugs(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Two issues with REFRESH MATERIALIZED VIEW CONCURRENTLY |
| Date: | 2026-07-27 20:28:06 |
| Message-ID: | CA+TgmoaryPLAMNWhF8cP4L+f+wQKkwS5OGCC4rUCwfpm+u9-fg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-bugs |
On Tue, Jul 21, 2026 at 2:55 PM surya poondla <suryapoondla4(at)gmail(dot)com> wrote:
> For issue 1 (v7-0001) the behaviour is correct:
> 1) A duplicate row containing NULLs is now detected and reported:
> REFRESH MATERIALIZED VIEW CONCURRENTLY m;
> ERROR: new data for materialized view "m" contains duplicate rows
I agree that there's a bug here, but I'm not sure that we've got a
sufficiently clear-eyed view of where the bug is. The proposed fix
doesn't touch the function header comment, which lays out the
algorithm that Kevin intended to use and the reasoning behind it. I
think there's a bug in that algorithm, which means that the function
should probably be updating it. But before we get to that point, what
exactly is the bug?
* This is called after a new version of the data has been created in a
* temporary table. It performs a full outer join against the old version of
* the data, producing "diff" results. This join cannot work if there are any
* duplicated rows in either the old or new versions, in the sense that every
* column would compare as equal between the two rows. It does work correctly
* in the face of rows which have at least one NULL value, with all non-NULL
* columns equal.
I think the bug is in this last sentence. If that statement were true,
then the delta table would be constructed correctly in the test case
from the original email, because in the example, all the rows involved
contain at least one NULL value. But in fact, the delta table ends up
being wrong in that test case. As far as it can see, every row in the
new query output is present in the materialized view and every row in
the materialized view is present in the new query output, and so it
misses the fact that there is a new row to insert.
But ... how exactly is that sentence wrong? What the delta query
actually does is look for rows that are image-identical and SQL-equal
on all key columns. Now, that's really strange. If the rows are
image-identical, then you might think that they would *also* be
SQL-equal on all key columns, but that is not the case, because in SQL
equality, NULL = NULL evaluates to NULL, not true (or false). So what
that means is that the query considers two rows to be identical if
they actually are identical and if, in addition, every column covered
by a usable unique index is non-NULL. Flipping that statement around,
the query considers rows that actually are identical to be
non-identical if any column covered by a usable unique index contains
a NULL, which is pretty crazy IMHO.
But on the upside, the consequence of that craziness is that a unique
index that ignores nulls (which is the default) is still good enough
to make this code work. What will happen is that we'll see those
null-containing rows in the current version of the MV as all being
different from the null-containing rows in the new data we want to put
into the MV, so we'll delete them all and reinsert them on every
refresh. That's going to be terrible for performance if there are many
such rows, but that might not be the case, in which case it will work
fine.
However, all of the above logic is only correct when the null occurs
*in a column covered by a usable unique index*. If the null is
elsewhere in the tuple, as in case #1 from the original email, then it
doesn't force the delta query to treat those rows as unequal, and we
don't get pushed into the delete-and-insert path.
So the last sentence I quote above is sort of doubly wrong. First, the
behavior is only correct in the face of duplicated rows which have at
least one NULL value *in a column covered by a unique index*. Second,
"correct" only means that we manage to get the right answer, not that
the code works in an understandable way or is efficient. :-(
It might be a good idea to think about redesigning this whole
mechanism to work in a less-surprising way. But I think in the
back-branches, that's out of the question. What we can do, though, is
tighten the null precheck so it looks specifically for nulls in
columns covered by unique indexes and errors out if none are found.
With that change, the "new data for materialized view \"%s\" contains
duplicate rows without any null columns" error would trigger in case
#1 from the original email, which I think is the expected behavior.
--
Robert Haas
EDB: http://www.enterprisedb.com
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Zsolt Parragi | 2026-07-27 20:50:16 | Re: Race between datachecksum enablement and create table with the file_copy strategy |
| Previous Message | Tom Lane | 2026-07-27 19:25:02 | Re: BUG #19582: Query fails on mixed IPv4 IPv6 data when index added |