On Sat, Oct 15, 2022 at 1:47 AM Amit Langote <amitlangot...@gmail.com> wrote: > I have merged your incremental patch into 0003.
Note that if someone goes to commit 0003, they would have no idea that I contributed to the effort. You should probably try to keep a running list of co-authors, reviewers, or other people that need to be acknowledged in your draft commit messages. On that note, I think that the commit messages for 0001 and to some extent 0002 need some more work. In particular, it seems like the commit message for 0001 is entirely concerned with what the patch does and says nothing about why it's a good idea. In my opinion, a good commit message needs to do both, ideally but not always in less space than this patch takes to do only one of those things. 0002 has the same problem to a lesser degree, since it is perhaps not so hard to infer that the reason for avoiding the SQL query is performance. I am wondering if the ordering for this patch series needs to be rethought. The commit message for 0004 reads as if it is fixing a bug introduced by earlier patches in the series. If that is not correct, maybe it can be made clearer. If it is correct, then that's not good, because we don't want to commit buggy patches and then make follow-up commits to remove the bugs. If a planned commit needs new infrastructure to avoid being buggy, the commits adding that infrastructure should happen first. But I think the bigger problem for this patch set is that the design-level feedback from https://www.postgresql.org/message-id/CA%2BTgmoaiTNj4DgQy42OT9JmTTP1NWcMV%2Bke0i%3D%2Ba7%3DVgnzqGXw%40mail.gmail.com hasn't really been addressed, AFAICS. ri_LookupKeyInPkRelPlanIsValid is still trivial in v7, and that still seems wrong to me. And I still don't know how we're going to avoid changing the semantics in ways that are undesirable, or even knowing precisely what we did change. If we don't have answers to those questions, then I suspect that this patch set isn't going anywhere. -- Robert Haas EDB: http://www.enterprisedb.com