From: | Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> |
---|---|
To: | Jim Mlodgenski <jimmy76(at)gmail(dot)com> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PgHacker <pgsql-hackers(at)postgresql(dot)org>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Shigeru Hanada <shigeru(dot)hanada(at)gmail(dot)com> |
Subject: | Re: Custom Scan APIs (Re: Custom Plan node) |
Date: | 2013-11-19 14:41:58 |
Message-ID: | CADyhKSV4+u4OA9Vp5sr_aLpuCmGkj3rPi3xq7hNYBsUW6fSoCA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Thanks for your review.
2013/11/19 Jim Mlodgenski <jimmy76(at)gmail(dot)com>:
> 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.
> 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.
Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>
Attachment | Content-Type | Size |
---|---|---|
pgsql-v9.4-custom-scan-part3.v4.patch | application/octet-stream | 62.0 KB |
pgsql-v9.4-custom-scan-part2.v4.patch | application/octet-stream | 51.2 KB |
pgsql-v9.4-custom-scan-part1.v4.patch | application/octet-stream | 61.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Fujii Masao | 2013-11-19 14:47:57 | Re: Review: pset autocomplete add missing options |
Previous Message | Tom Lane | 2013-11-19 14:37:02 | Re: New option for pg_basebackup, to specify a different directory for pg_xlog |