Re: [PATCH] Keeps tracking the uniqueness with UniqueKey - Mailing list pgsql-hackers
From | Andy Fan |
---|---|
Subject | Re: [PATCH] Keeps tracking the uniqueness with UniqueKey |
Date | |
Msg-id | CAKU4AWoeKTrj16p4hKgTJEYCGQ2nb-R6Bd7FcyWtLEj4yBYKNQ@mail.gmail.com Whole thread Raw |
In response to | Re: [PATCH] Keeps tracking the uniqueness with UniqueKey (Dmitry Dolgov <9erthalion6@gmail.com>) |
Responses |
RE: [PATCH] Keeps tracking the uniqueness with UniqueKey
Re: [PATCH] Keeps tracking the uniqueness with UniqueKey |
List | pgsql-hackers |
On Wed, Oct 7, 2020 at 9:55 PM Dmitry Dolgov <9erthalion6@gmail.com> wrote:
> On Wed, Sep 09, 2020 at 07:51:12AM +0800, Andy Fan wrote:
>
> Thank you Michael for checking it, I can reproduce the same locally after
> rebasing to the latest master. The attached v11 has fixed it and includes
> the fix Floris found.
>
> The status of this patch is we are still in discussion about which data
> type should
> UniqueKey->expr use. Both David [1] and I [2] shared some thinking about
> EquivalenceClasses, but neither of us have decided on it. So I still didn't
> change
> anything about that now. I can change it once we have decided on it.
>
> [1]
> https://www.postgresql.org/message-id/CAApHDvoDMyw%3DhTuW-258yqNK4bhW6CpguJU_GZBh4x%2Brnoem3w%40mail.gmail.com
>
> [2]
> https://www.postgresql.org/message-id/CAKU4AWqy3Uv67%3DPR8RXG6LVoO-cMEwfW_LMwTxHdGrnu%2Bcf%2BdA%40mail.gmail.com
Hi,
In the Index Skip Scan thread Peter mentioned couple of issues that I
believe need to be addressed here. In fact one about valgrind errors was
already fixed as far as I see (nkeycolumns instead of ncolumns), another
one was:
/code/postgresql/patch/build/../source/src/backend/optimizer/path/uniquekeys.c:
In function ‘populate_baserel_uniquekeys’:
/code/postgresql/patch/build/../source/src/backend/optimizer/path/uniquekeys.c:797:13:
warning: ‘expr’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
797 | else if (!list_member(unique_index->rel->reltarget->exprs, expr))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
I can fix this warning in the next version, thanks for reporting it. It can be
fixed like below or just adjust the if-elseif-else pattern.
+++ b/src/backend/optimizer/path/uniquekeys.c
@@ -760,6 +760,7 @@ get_exprs_from_uniqueindex(IndexOptInfo *unique_index,
{
/* Index on system column is not supported */
Assert(false);
+ expr = NULL; /* make compiler happy */
}
Other than that I wanted to ask what are the plans to proceed with this
patch? It's been a while since the question was raised in which format
to keep unique key expressions, and as far as I can see no detailed
suggestions or patch changes were proposed as a follow up. Obviously I
would love to see the first two preparation patches committed to avoid
dependencies between patches, and want to suggest an incremental
approach with simple format for start (what we have right now) with the
idea how to extend it in the future to cover more cases.
dedicated time to review which would be the hardest part for the reviewers.
I don't have a good suggestion, however.
--
Best Regards
Andy Fan
pgsql-hackers by date: