Re: Add CREATE support to event triggers - Mailing list pgsql-hackers

<div dir="ltr"><br /><div class="gmail_extra"><br /><div class="gmail_quote">On Thu, Oct 9, 2014 at 6:26 AM, Alvaro
Herrera<span dir="ltr"><<a href="mailto:alvherre@2ndquadrant.com"
target="_blank">alvherre@2ndquadrant.com</a>></span>wrote:<br /><blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px#ccc solid;padding-left:1ex">About patch 1, which I just pushed, there were two reviews:<br /><span
class=""><br/> Andres Freund wrote:<br /> > On 2014-09-25 18:59:31 -0300, Alvaro Herrera wrote:<br /></span><span
class="">>> diff --git a/src/include/utils/ruleutils.h b/src/include/utils/ruleutils.h<br /> > > new file
mode100644<br /> > > index 0000000..520b066<br /> > > --- /dev/null<br /> > > +++
b/src/include/utils/ruleutils.h<br/><br /></span><span class="">> I wondered for a minute whether any of these are
likelyto cause<br /> > problems for code just including builtins.h - but I think there will be<br /> >
sufficientlyfew callers for them to make that not much of a concern.<br /><br /></span>Great.<br /><span class=""><br
/>Michael Paquier wrote:<br /><br /> > Patch 1: I still like this patch as it gives a clear separation of the<br />
>built-in functions and the sub-functions of ruleutils.c that are<br /> > completely independent. Have you
consideredadding the external<br /> > declaration of quote_all_identifiers as well? It is true that this<br /> >
impactsextensions (some of my stuff as well), but my point is to bite<br /> > the bullet and make the separation
cleanerbetween builtins.h and<br /> > ruleutils.h. Attached is a patch that can be applied on top of patch 1<br />
>doing so... Feel free to discard for the potential breakage this would<br /> > create though.<br /><br
/></span>Yes,I did consider moving the quoting function definitions and the<br /> global out of builtins.h.  It is much
moredisruptive, however, because<br /> it affects a lot of external code not only our own; and given the looks<br /> I
gotafter people had to mess with adding the htup_details.h header to<br /> external projects due to the refactoring I
didto htup.h in an earlier<br /> release, I'm not sure about it.<br /><br /> However, if I were to do it, I would
insteadcreate a quote.h file and<br /> would also add the quote_literal_cstr() prototype to it, perhaps even<br /> move
theimplementations from their current ruleutils.c location to<br /> quote.c.  (Why is half the stuff in ruleutils.c
ratherthan quote.c is<br /> beyond me.)<br /></blockquote>Yes, an extra header file would be cleaner. Now if we are
worriedabout creating much incompatibilities with existing extension (IMO that's not something core should worry much
about),then it is better not to do it. Now, if I can vote on it, I think it is worth doing in order to have builtins.h
onlycontain only Datum foo(PG_FUNCTION_ARGS), and move all the quote-related things in quote.c.<br /></div>-- <br
/>Michael<br/></div></div>