On Wed, Oct 7, 2020 at 9:55 PM Dmitry Dolgov <9erthali...@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. --- a/src/backend/optimizer/path/uniquekeys.c +++ 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. > I think the hardest part of this series is commit 2, it probably needs lots of 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
