Обсуждение: BUG #6715: 9.2b2 psql \ir does not understand leading ../

The following bug has been logged on the website:

Bug reference:      6715
Logged by:          Greg Davidson
Email address:      jgd@well.com
PostgreSQL version: Unsupported/Unknown
Operating system:   OpenSuSe 11.3, PostgreSQL 9.wbeta2
Description:=20=20=20=20=20=20=20=20

When I do
\ir ../bar.sql
I get the bar.sql file in the CWD.

$ psql
psql (9.2beta2)
Type "help" for help.
\q

$ psql -aX greg -f foo.sql
\! ls ../bar.sql
ls: cannot access ../bar.sql: No such file or directory
\! ls bar.sql
bar.sql
\! cat bar.sql
\echo BAR
\ir ../bar.sql
\echo BAR
BAR

Please also note these infelicities:
(1) There's no 9.2beta2 choice in the bug report tool.
(2) On the beta announcement web page
http://www.postgresql.org/about/news/1395/
it says that information on how to test and report bugs with the beta are on
the web page
http://www.postgresql.org/developer/beta/
but no such information is on that page!

_Greg

jgd@well.com writes:
> When I do
> \ir ../bar.sql
> I get the bar.sql file in the CWD.

AFAICS, it works fine when \ir is used interactively.  However,
you seem to be using it from a script:

> $ psql -aX greg -f foo.sql

and then indeed it does not work so well.  psql is trying to evaluate
the \ir relative to the script file's location, using this code:

            snprintf(relpath, MAXPGPATH, "%s", pset.inputfile);
            get_parent_directory(relpath);
            join_path_components(relpath, relpath, filename);
            canonicalize_path(relpath);

The get_parent_directory() call reduces "foo.sql" to an empty string,
which seems a tad bogus --- wouldn't "." be better?  But in any case,
join_path_components() thinks it can process a "../" prefix of the
tail argument by stripping a directory name from the head argument,
and there's nothing there to strip.  So IMO join_path_components()
is flat-out broken when applied to a non-absolute head path.
Not sure about what a useful solution would be.

            regards, tom lane

On Tue, Jul 3, 2012 at 11:41 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> jgd@well.com writes:
>> When I do
>> \ir ../bar.sql
>> I get the bar.sql file in the CWD.
>
> AFAICS, it works fine when \ir is used interactively. =A0However,
> you seem to be using it from a script:
>
>> $ psql -aX greg -f foo.sql
>
> and then indeed it does not work so well. =A0psql is trying to evaluate
> the \ir relative to the script file's location, using this code:
>
> =A0 =A0 =A0 =A0 =A0 =A0 snprintf(relpath, MAXPGPATH, "%s", pset.inputfile=
);
> =A0 =A0 =A0 =A0 =A0 =A0 get_parent_directory(relpath);
> =A0 =A0 =A0 =A0 =A0 =A0 join_path_components(relpath, relpath, filename);
> =A0 =A0 =A0 =A0 =A0 =A0 canonicalize_path(relpath);
>
> The get_parent_directory() call reduces "foo.sql" to an empty string,
> which seems a tad bogus --- wouldn't "." be better? =A0But in any case,
> join_path_components() thinks it can process a "../" prefix of the
> tail argument by stripping a directory name from the head argument,
> and there's nothing there to strip. =A0So IMO join_path_components()
> is flat-out broken when applied to a non-absolute head path.
> Not sure about what a useful solution would be.

I may not have time to look at this today, but I think this behavior
worked fine in the last version[1] of Gurjeet's \ir patch which I
reviewed, which had different behavior for pathname normalization than
what got committed.

Josh

[1] http://archives.postgresql.org/message-id/BANLkTi=3DeW_nUH9195=3D9uPqF7=
Treg4UH7-g@mail.gmail.com

Josh Kupershmidt <schmiddy@gmail.com> writes:
> On Tue, Jul 3, 2012 at 11:41 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> The get_parent_directory() call reduces "foo.sql" to an empty string,
>> which seems a tad bogus --- wouldn't "." be better?  But in any case,
>> join_path_components() thinks it can process a "../" prefix of the
>> tail argument by stripping a directory name from the head argument,
>> and there's nothing there to strip.  So IMO join_path_components()
>> is flat-out broken when applied to a non-absolute head path.
>> Not sure about what a useful solution would be.

> I may not have time to look at this today, but I think this behavior
> worked fine in the last version[1] of Gurjeet's \ir patch which I
> reviewed, which had different behavior for pathname normalization than
> what got committed.
> [1] http://archives.postgresql.org/message-id/BANLkTi=eW_nUH9195=9uPqF7Treg4UH7-g@mail.gmail.com

[ looks at that... ]  Well, I can certainly see why Robert got rid of
that kluge in favor of using the path-manipulation functions we already
have.  It looks like most of the existing uses of join_path_components
are using absolute paths (eg, the result of getcwd), which probably
explains why we've not previously noticed that it's not working
correctly in such cases.

On reflection I think that what we need to do is fix it so that it only
strips "../" from the tail string when there is a directory name
available to be stripped from the head string.  Otherwise just stop
trimming.

            regards, tom lane

I wrote:
> On reflection I think that what we need to do is fix it so that it only
> strips "../" from the tail string when there is a directory name
> available to be stripped from the head string.  Otherwise just stop
> trimming.

Actually, there is a much simpler fix: don't let it strip ".." at all.
There is no correctness reason to do that, and as for cosmetics, callers
who care can use canonicalize_path() to remove ".." correctly.  (AFAICS,
all existing callers of join_path_components already do so anyway.)

            regards, tom lane