GUC names in messages - Mailing list pgsql-hackers

Hi hackers,

While reviewing another patch I noticed how the GUCs are
inconsistently named within the GUC_check_errdetail messages:

======

below, the GUC name is embedded but not quoted:

src/backend/access/transam/xlogprefetcher.c:
GUC_check_errdetail("recovery_prefetch is not supported on platforms
that lack posix_fadvise().");
src/backend/access/transam/xlogrecovery.c:
GUC_check_errdetail("recovery_target_timeline is not a valid
number.");
src/backend/commands/variable.c:
GUC_check_errdetail("effective_io_concurrency must be set to 0 on
platforms that lack posix_fadvise().");
src/backend/commands/variable.c:
GUC_check_errdetail("maintenance_io_concurrency must be set to 0 on
platforms that lack posix_fadvise().");
src/backend/port/sysv_shmem.c:
GUC_check_errdetail("huge_page_size must be 0 on this platform.");
src/backend/port/win32_shmem.c:
GUC_check_errdetail("huge_page_size must be 0 on this platform.");
src/backend/replication/syncrep.c:
GUC_check_errdetail("synchronous_standby_names parser failed");
src/backend/storage/file/fd.c:
GUC_check_errdetail("debug_io_direct is not supported on this
platform.");
src/backend/storage/file/fd.c:
GUC_check_errdetail("debug_io_direct is not supported for WAL because
XLOG_BLCKSZ is too small");
src/backend/storage/file/fd.c:
GUC_check_errdetail("debug_io_direct is not supported for data because
BLCKSZ is too small");
src/backend/tcop/postgres.c:
GUC_check_errdetail("client_connection_check_interval must be set to 0
on this platform.");

~~~

below, the GUC name is embedded and double-quoted:

src/backend/commands/vacuum.c:
GUC_check_errdetail("\"vacuum_buffer_usage_limit\" must be 0 or
between %d kB and %d kB",
src/backend/commands/variable.c:
GUC_check_errdetail("Conflicting \"datestyle\" specifications.");
src/backend/storage/buffer/localbuf.c:
GUC_check_errdetail("\"temp_buffers\" cannot be changed after any
temporary tables have been accessed in the session.");
src/backend/tcop/postgres.c:
GUC_check_errdetail("\"max_stack_depth\" must not exceed %ldkB.",
src/backend/tcop/postgres.c:        GUC_check_errdetail("Cannot enable
parameter when \"log_statement_stats\" is true.");
src/backend/tcop/postgres.c:        GUC_check_errdetail("Cannot enable
\"log_statement_stats\" when "

~~~

below, the GUC name is substituted but not quoted:

src/backend/access/table/tableamapi.c:      GUC_check_errdetail("%s
cannot be empty.",
src/backend/access/table/tableamapi.c:      GUC_check_errdetail("%s is
too long (maximum %d characters).",

~~~

I had intended to make a patch to address the inconsistency, but
couldn't decide which of those styles was the preferred one.

Then I worried this could be the tip of the iceberg -- GUC names occur
in many other error messages where they are sometimes quoted and
sometimes not quoted:
e.g. Not quoted -- errhint("You might need to run fewer transactions
at a time or increase max_connections.")));
e.g. Quoted -- errmsg("\"max_wal_size\" must be at least twice
\"wal_segment_size\"")));

Ideally, they should all look the same everywhere, shouldn't they?

======
Kind Regards,
Peter Smith.
Fujitsu Australia