|From:||Joe Conway <mail(at)joeconway(dot)com>|
|To:||Michael Paquier <michael(dot)paquier(at)gmail(dot)com>|
|Cc:||Andres Freund <andres(at)anarazel(dot)de>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Peter Eisentraut <peter_e(at)gmx(dot)net>|
|Subject:||Re: exposing pg_controldata and pg_config as functions|
|Views:||Raw Message | Whole Thread | Download mbox|
On 02/15/2016 11:20 PM, Michael Paquier wrote:
> Here are just a couple of cosmetic comments.
> Missing markup <productname></> around PostgreSQL.
Added, but I'll note that there are tons of locations in the docs where
we do not do that. Maybe they should all be made consistent.
> + Application. There is a System Information Function
> Why is using upper-case characters necessary here? This could just say
> system function.
It is capitalized because it refers to a section in the docs. I followed
it with <xref linkend="functions-info"> as well, so it ends up reading:
"There is a System Information Function (Section 9.25) called pg_config
which underlies this view."
I guess I could be convinced to lower case it, but I thought this looked
> The paragraph in func.sgml is a copy-paste of the one in
> catalogs.sgml. We may want to remove the duplication.
The paragraphs are mostly copy-paste, but slightly different (toward the
end). There is both a function and a system view -- why would we not
want a description in both places?
> + /* let the caller know we're sending back a tuplestore */
> + rsinfo->returnMode = SFRM_Materialize;
> I guess one can recognize your style here for SRF functions :)
Old habits die hard ;-)
> @@ -61,7 +74,7 @@ libpgcommon_srv.a: $(OBJS_SRV)
> # a hack that might fail someday if there is a *_srv.o without a
> # corresponding *.o, but it works for now.
> %_srv.o: %.c %.o
> - $(CC) $(CFLAGS) $(subst -DFRONTEND,, $(CPPFLAGS)) -c $< -o $@
> + $(CC) $(CFLAGS) $(subst -DFRONTEND ,, $(CPPFLAGS)) -c $< -o $@
> Diff noise?
No, intentional. The original version leaves a white space at the
beginning of CPPFLAGS after removing -DFRONTEND.
> --- /dev/null
> +++ b/src/common/config_info.c
> + * IDENTIFICATION
> + * src/common/controldata_utils.c
> This is incorrect.
Right -- found and fixed several of these with Alvaro's help after
posting. Also fixed Copyright dates (s/2015/2016/) on the new files.
> + * IDENTIFICATION
> + * src/backend/utils/misc/pg_config.c
> + *
> + */
> I am nitpicking here but this header block should have a long
> "----------------" at its bottom.
Fair enough -- fixed.
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development
|Next Message||Bruce Momjian||2016-02-16 14:38:20||Re: pglogical - logical replication contrib module|
|Previous Message||Fabien COELHO||2016-02-16 14:12:18||Re: extend pgbench expressions with functions|