Hi, On Wed, Nov 17, 2021 at 11:28:43PM +0100, Tomas Vondra wrote: > On 11/17/21 15:45, Thomas wrote: > > Thank you for the feedback and sorry for the oversight. I fixed the bug > > and attached a new version of the patch. > > > > Kind Regards, Thomas > > > > Am Mi., 17. Nov. 2021 um 15:03 Uhr schrieb Daniel Gustafsson > > <dan...@yesql.se <mailto:dan...@yesql.se>>: > > > > This patch fails to compile due to an incorrect function name in an > > assertion: > > > > nodeMergejoin.c:297:9: warning: implicit declaration of function > > 'list_legth' is invalid in C99 [-Wimplicit-function-declaration] > > Assert(list_legth(node->rangeclause) < 3); > > > > That still doesn't compile with asserts, because MJCreateRangeData has > > Assert(list_length(node->rangeclause) < 3); > > but there's no 'node' variable :-/ > > > I took a brief look at the patch, and I think there are two main issues > preventing it from moving forward. > > 1) no tests > > 2) lack of comments > > 3) I'm not quite sure I like "Range Merge Join" to be honest. It's still a > "Merge Join" pretty much. What about ditching the "Range"? There'll still be > "Range Cond" key, which should be good enough I think. > > 4) Some minor whitespace issues (tabs vs. spaces). See 0002.
It's been 2 months since Tomas posted that review. Thomas, do you plan to work on that patch during this commitfest?
