pgsql-server: Use canonicalize_path for -D, GUC paths, and paths coming

momjian@svr1.postgresql.org (Bruce Momjian) writes:

pgsql-server/src/backend/postmaster:
postmaster.c (r1.407 -> r1.408)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/backend/postmaster/postmaster.c.diff?r1=1.407&r2=1.408)

You can't do that. In the first place it will dump core if PGDATA isn't
set, and in the second place it is not kosher to scribble on environment
values.

This is the wrong place to do it anyway. It is necessary, sufficient,
and already done to do it in SetDataDir.

pgsql-server/src/backend/utils/misc:
guc.c (r1.214 -> r1.215)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/backend/utils/misc/guc.c.diff?r1=1.214&r2=1.215)

Could we not have FATAL here, please?

pgsql-server/src/bin/psql:
command.c (r1.119 -> r1.120)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/bin/psql/command.c.diff?r1=1.119&r2=1.120)

None of these are correct. canonicalize_path is only intended for
directory names not file names. (I think the same problem applies
to several of your GUC variable changes, too.)

copy.c (r1.49 -> r1.50)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/bin/psql/copy.c.diff?r1=1.49&r2=1.50)

As above.

regards, tom lane

Tom Lane wrote:

momjian@svr1.postgresql.org (Bruce Momjian) writes:

pgsql-server/src/backend/postmaster:
postmaster.c (r1.407 -> r1.408)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/backend/postmaster/postmaster.c.diff?r1=1.407&r2=1.408)

You can't do that. In the first place it will dump core if PGDATA isn't
set, and in the second place it is not kosher to scribble on environment
values.

OK, I added a strdup to make a copy of the environment variable. I also
moved the canonicialize_path down so it will process both PGDATA and -D.
Patch attached and applied.

This is the wrong place to do it anyway. It is necessary, sufficient,
and already done to do it in SetDataDir.

The problem is that checkDataDir() (called before SetDataDir()) does a
stat() on that value, and this is the failure Magnus was seeing. This
requirement is new because onlyConfigSpecified() does a stat() to test
to see if we are are pointing to a config file/dir or the data directory
before calling SetDataDir(). However, we can't remove the
canonicalize_path() from SetDataDir() because postgres and bootstrap
call that directly.

pgsql-server/src/backend/utils/misc:
guc.c (r1.214 -> r1.215)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/backend/utils/misc/guc.c.diff?r1=1.214&r2=1.215)

Could we not have FATAL here, please?

OK, changed to ERROR, but there are alot of similar calls which used
FATAL for that call in that file.

pgsql-server/src/bin/psql:
command.c (r1.119 -> r1.120)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/bin/psql/command.c.diff?r1=1.119&r2=1.120)

None of these are correct. canonicalize_path is only intended for
directory names not file names. (I think the same problem applies
to several of your GUC variable changes, too.)

canonicalize_path changes \ to /, and trims the trailing slash.
Filenames do not need trimming, but they might need the slash change. I
think we found that a filename with mixed / and \ will not work.

copy.c (r1.49 -> r1.50)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/bin/psql/copy.c.diff?r1=1.49&r2=1.50)

As above.

regards, tom lane

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman@candle.pha.pa.us> writes:

None of these are correct. canonicalize_path is only intended for
directory names not file names. (I think the same problem applies
to several of your GUC variable changes, too.)

canonicalize_path changes \ to /, and trims the trailing slash.

... and probably breaks the GUC variables that represent search paths,
rather than single file/directory names. Certainly canonicalizing those
values in toto will not have the desired effects; you'd need to
canonicalize the path elements after they are extracted.

regards, tom lane

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

None of these are correct. canonicalize_path is only intended for
directory names not file names. (I think the same problem applies
to several of your GUC variable changes, too.)

canonicalize_path changes \ to /, and trims the trailing slash.

... and probably breaks the GUC variables that represent search paths,
rather than single file/directory names. Certainly canonicalizing those
values in toto will not have the desired effects; you'd need to
canonicalize the path elements after they are extracted.

OK, the only GUC I saw that was a list was preload_libraries, and this
patch fixes that.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Tom Lane wrote:

... and probably breaks the GUC variables that represent search paths,

OK, the only GUC I saw that was a list was preload_libraries, and this
patch fixes that.

Nope, Dynamic_library_path is broken too.

regards, tom lane

OK, got it. THanks.

---------------------------------------------------------------------------

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Tom Lane wrote:

... and probably breaks the GUC variables that represent search paths,

OK, the only GUC I saw that was a list was preload_libraries, and this
patch fixes that.

Nope, Dynamic_library_path is broken too.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 9: the planner will ignore your desire to choose an index scan if your
joining column's datatypes do not match

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073