Closed shane-circuithub closed 2 years ago
Fabulous, thanks! I'll have to give it a deeper look when I have more time, but it looks great to me.
I'm happy to add the LATERAL
-using version and consider a non-LATERAL
version later if people want it.
I still haven't had time to look at this properly. I'd particularly like to add some property tests for it. If the delay in getting this merged is an inconvenience then please let me know and I can prioritise it.
This looks great, thanks! Would you mind addressing the comments I've made? Then I will be happy to merge it.
I've addressed your comments :)
Now that I understand WITH
a bit better I was able to extract out some of the functionality. @shane-circuithub, I will merge my version in due course and I would welcome any comments from you if you have time to look at it.
Thanks to @shane-circuithub and @hesselink for your work on this! Great to finally have it in Opaleye.
This is based on @hesselink's original attempt in #58, but we use a
Binaryspec
instead of anUnpackspec
(to ensure that both the base and recursive cases have the same number of columns).Also, I've simplified the
PrimQuery
constructor; it's no longer specific toWITH RECURSIVE
; it could easily be used for ordinaryWITH
expressions as well.In terms of the implementation, I use a
LATERAL
product to avoid the need for theaddTable
function that the original implementation had. I don't know if there are performance implications for this in general, but for the one query I've tested this on it produces the same plan whether usingLATERAL
or theaddTable
method.