Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> Here's a rebased version; there were some merge conflicts with master.
Thanks!
> I also fixed some compiler warnings. I haven't reviewed the patch in
> any detail yet. One thing that jump at me from the code style
> perspective is the strange way it deals with "isnull" from heap_getattr.
> (I think most of these should just elog(ERROR) if a null attr is found).
I think you're right here, yes.
> Another thing is that I don't find the name "uptmpl" very clear.
Any suggestion is welcome, I don't like it very much either.
> Keeping the "template.c" file name seems wrong -- exttemplate.c maybe?
> (I renamed the parse nodes to ExtTemplate)
I've been wondering about that. My thinking is that we're providing a
new set of TEMPLATE objects and commands and the commands are designed
so that it's easy to add more objects in there. The implementation too
is quite open to that, with "routing" functions in template.c.
Now I've choosen to implement the Extension templates in the same file
because currently those are the only kind of "Templates" that we manage,
and this project seems to prefer big files rather than too many files.
That said, I'm not wedded to this choice.
> There was a strange bug in pg_dump; it used "qto" where I thought
> qversion was appropriate. I changed it (I looked at this hunk more
> closely than most others because there was a compiler warning here, but
> I didn't verify that it works.)
It's quite hard for me to spot the hunk you're talking about without
setting up a whole new branch to extract the work you did in the new
diff, but maybe I'm just missing a diff-between-diffs tool?
> You seem to love using Capitalized Letters for some things in error
> messages; I don't find these very pretty, and anyway they violate our
> style guidelines. (I think these are in elog() not ereport() calls, but
> still)
I'll make sure to remember about that, thanks.
Regards,
--
Dimitri Fontaine 06 63 07 10 78
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
