> My initial review on this feature: > - The patches apply and build, but it produces a warning: > ctidscan.c: In function ‘CTidInitCustomScanPlan’: > ctidscan.c:362:9: warning: unused variable ‘scan_relid’ [-Wunused-variable] >
This variable was only used in Assert() macro, so it causes a warning if you don't put --enable-cassert on the configure script. Anyway, I adjusted the code to check relid of RelOptInfo directly.
The warning is now gone.
> I'd recommend that you split the part1 patch containing the ctidscan contrib > into its own patch. It is more than half of the patch and its certainly > stands on its own. IMO, I think ctidscan fits a very specific use case and > would be better off being an extension instead of in contrib. >
OK, I split them off. The part-1 is custom-scan API itself, the part-2 is ctidscan portion, and the part-3 is remote join on postgres_fdw.
Attached is a patch for the documentation. I think the documentation still needs a little more work, but it is pretty close. I can add some more detail to it once finish adapting the hadoop_fdw to using the custom scan api and have a better understanding of all of the calls.
