[PATCH] Fix search_path default value separator.

Started by Christoph Martinalmost 12 years ago6 messageshackers
Jump to latest
#1Christoph Martin
christoph.r.martin@gmail.com

Hi

I noticed a minor inconsistency with the search_path separator used in the
default configuration.

The schemas of any search_path set using `SET search_path TO...` are
separated by ", " (comma, space), while the default value is only separated
by "," (comma).

The attached patch against master changes the separator of the default
value to be consistent with the usual comma-space separators, and updates
the documentation of `SHOW search_path;` accordingly.

This massive three-byte change passes all 144 tests of make check.

Regards,

Christoph

Attachments:

0001-Fix-search_path-default-value-separator.patchtext/x-patch; charset=US-ASCII; name=0001-Fix-search_path-default-value-separator.patchDownload+3-4
#2Robert Haas
robertmhaas@gmail.com
In reply to: Christoph Martin (#1)
Re: [PATCH] Fix search_path default value separator.

On Fri, Jul 11, 2014 at 6:09 AM, Christoph Martin
<christoph.r.martin@gmail.com> wrote:

I noticed a minor inconsistency with the search_path separator used in the
default configuration.

The schemas of any search_path set using `SET search_path TO...` are
separated by ", " (comma, space), while the default value is only separated
by "," (comma).

The attached patch against master changes the separator of the default value
to be consistent with the usual comma-space separators, and updates the
documentation of `SHOW search_path;` accordingly.

This massive three-byte change passes all 144 tests of make check.

Heh. I'm not particularly averse to changing this, but I guess I
don't see any particular benefit of changing it either. Either comma
or comma-space is a legal separator, so why worry about it?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Christoph Martin
christoph.r.martin@gmail.com
In reply to: Robert Haas (#2)
Re: [PATCH] Fix search_path default value separator.

True. Both variants are legal, and most people won't ever notice. I
stumbled across this while writing a test case for a transaction helper
that sets/restores search_path before committing. The test was to simply
compare the string values of `SHOW search_path;` before `BEGIN
TRANSACTION;` and after `COMMIT;`.

It's a non-issue, really, but since there's a patch and I cannot come up
with a more common use case that would depend on the use of just-comma
separators in the default value, I'd say it's more of a question of "why
not" instead of "why", isn't it?

On 14 July 2014 16:58, Robert Haas <robertmhaas@gmail.com> wrote:

Show quoted text

On Fri, Jul 11, 2014 at 6:09 AM, Christoph Martin
<christoph.r.martin@gmail.com> wrote:

I noticed a minor inconsistency with the search_path separator used in

the

default configuration.

The schemas of any search_path set using `SET search_path TO...` are
separated by ", " (comma, space), while the default value is only

separated

by "," (comma).

The attached patch against master changes the separator of the default

value

to be consistent with the usual comma-space separators, and updates the
documentation of `SHOW search_path;` accordingly.

This massive three-byte change passes all 144 tests of make check.

Heh. I'm not particularly averse to changing this, but I guess I
don't see any particular benefit of changing it either. Either comma
or comma-space is a legal separator, so why worry about it?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#4Fujii Masao
masao.fujii@gmail.com
In reply to: Christoph Martin (#3)
Re: [PATCH] Fix search_path default value separator.

On Tue, Jul 15, 2014 at 12:20 AM, Christoph Martin
<christoph.r.martin@gmail.com> wrote:

True. Both variants are legal, and most people won't ever notice. I stumbled
across this while writing a test case for a transaction helper that
sets/restores search_path before committing. The test was to simply compare
the string values of `SHOW search_path;` before `BEGIN TRANSACTION;` and
after `COMMIT;`.

It's a non-issue, really, but since there's a patch and I cannot come up
with a more common use case that would depend on the use of just-comma
separators in the default value, I'd say it's more of a question of "why
not" instead of "why", isn't it?

On 14 July 2014 16:58, Robert Haas <robertmhaas@gmail.com> wrote:

On Fri, Jul 11, 2014 at 6:09 AM, Christoph Martin
<christoph.r.martin@gmail.com> wrote:

I noticed a minor inconsistency with the search_path separator used in
the
default configuration.

The schemas of any search_path set using `SET search_path TO...` are
separated by ", " (comma, space), while the default value is only
separated
by "," (comma).

The attached patch against master changes the separator of the default
value
to be consistent with the usual comma-space separators, and updates the
documentation of `SHOW search_path;` accordingly.

This massive three-byte change passes all 144 tests of make check.

Heh. I'm not particularly averse to changing this, but I guess I
don't see any particular benefit of changing it either. Either comma
or comma-space is a legal separator, so why worry about it?

This change might cause me to update the existing documents (which
I need to maintain in my company) including the output example of
default search_path. If the change is for the improvement, I'd be
happy to do that, but it seems not.

Also there might be some PostgreSQL extensions which their regression test
shows the default search_path. This patch would make their developers
spend the time to update the test. I'm sure that they are fine with that if
it's for an improvement. But not.

Regards,

--
Fujii Masao

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Bruce Momjian
bruce@momjian.us
In reply to: Fujii Masao (#4)
Re: [PATCH] Fix search_path default value separator.

On Fri, Aug 15, 2014 at 10:40:59PM +0900, Fujii Masao wrote:

Heh. I'm not particularly averse to changing this, but I guess I
don't see any particular benefit of changing it either. Either comma
or comma-space is a legal separator, so why worry about it?

This change might cause me to update the existing documents (which
I need to maintain in my company) including the output example of
default search_path. If the change is for the improvement, I'd be
happy to do that, but it seems not.

Also there might be some PostgreSQL extensions which their regression test
shows the default search_path. This patch would make their developers
spend the time to update the test. I'm sure that they are fine with that if
it's for an improvement. But not.

Well, rename GUC often too for clearity, so I don't see adjusting
white-space as something to avoid either. It is always about short-term
adjustments vs. long-term clarity.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Heikki Linnakangas
heikki.linnakangas@enterprisedb.com
In reply to: Bruce Momjian (#5)
Re: [PATCH] Fix search_path default value separator.

On 08/15/2014 04:58 PM, Bruce Momjian wrote:

On Fri, Aug 15, 2014 at 10:40:59PM +0900, Fujii Masao wrote:

Heh. I'm not particularly averse to changing this, but I guess I
don't see any particular benefit of changing it either. Either comma
or comma-space is a legal separator, so why worry about it?

This change might cause me to update the existing documents (which
I need to maintain in my company) including the output example of
default search_path. If the change is for the improvement, I'd be
happy to do that, but it seems not.

Also there might be some PostgreSQL extensions which their regression test
shows the default search_path. This patch would make their developers
spend the time to update the test. I'm sure that they are fine with that if
it's for an improvement. But not.

Well, rename GUC often too for clearity, so I don't see adjusting
white-space as something to avoid either. It is always about short-term
adjustments vs. long-term clarity.

I think this is an improvement, although a really minor one. Although
Robert & Fujii questioned if this is worth it, I didn't hear anyone
objecting strongly, so committed.

- Heikki

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers