Hi All,

Also, for issue 1, additional test case should be added.

Sure, I will add test cases for issue 1.

For issue 2, it would be helpful if you may share some performance numbers to confirm if this solution is only improving the performance and not causing any regressions.

I ran check, check-world and didn't see any regressions.

Here is the output and performance improvement:

postgres=# \timing on

Timing is on.

postgres=# DROP MATERIALIZED VIEW IF EXISTS s CASCADE;

NOTICE: materialized view "s" does not exist, skipping

DROP MATERIALIZED VIEW

Time: 0.858 ms

postgres=#

postgres=# CREATE MATERIALIZED VIEW s AS SELECT generate_series as x, null as y FROM generate_series(1, 1000000);

SELECT 1000000

Time: 1076.254 ms (00:01.076)

postgres=#

postgres=# CREATE UNIQUE INDEX ON s(x);

CREATE INDEX

Time: 375.026 ms

postgres=# REFRESH MATERIALIZED VIEW CONCURRENTLY s;

REFRESH MATERIALIZED VIEW

Time: 3807.143 ms (00:03.807)

postgres=# CREATE UNIQUE INDEX ON s(y);

CREATE INDEX

Time: 331.382 ms

postgres=# REFRESH MATERIALIZED VIEW CONCURRENTLY s;

REFRESH MATERIALIZED VIEW

Time: 3636.049 ms (00:03.636)
postgres=#

As we can see the REFRESH MATERIALIZED VIEW CONCURRENTLY now takes 3636.049 ms

Regrading the performance, (quoting the output from my previous message) with unique index having NULL values we see that both "REFRESH MATERIALIZED VIEW CONCURRENTLY s;" operations (operation 1 was after CREATE UNIQUE INDEX ON s(x); and operation 2 was after CREATE UNIQUE INDEX ON s(x);) take about the same time. Without the patch, operation 2 was taking around ~11000 ms, due to NULL = NULL comparison checks and this was causing the degradation.

Regarding different commits to each issue, I don't have any particular opinion but since both the issues are related to the same function and NULL comparison, I feel we can have a single commit, but open to create 2 commits too.

Regards,
Surya Poondla