Обсуждение: pg_regress: paths in largeobject test

Hi,

the largeobject test does this:

137 SELECT lo_export(loid, '@abs_builddir@/results/lotest.txt') <snip>
138
139 \lo_import 'results/lotest.txt'
140
141 \set newloid :LASTOID
142
143 -- just make sure \lo_export does not barf
144 \lo_export :newloid 'results/lotest2.txt'

I believe the results paths in line 139 and 144 are missing the
@abs_builddir@ qualifier.

The attached patch has been tested with "make check" and by running
pg_regress outside the PostgreSQL source tree, both on Solaris 11, x86.

-J
--

Jørgen Austvik, Software Engineering - QA
Sun Microsystems Database Technology Group
Index: src/test/regress/input/largeobject.source
===================================================================
RCS file: /projects/cvsroot/pgsql/src/test/regress/input/largeobject.source,v
retrieving revision 1.4
diff -c -r1.4 largeobject.source
*** src/test/regress/input/largeobject.source    3 Mar 2007 22:57:03 -0000    1.4
--- src/test/regress/input/largeobject.source    29 Nov 2007 11:55:07 -0000
***************
*** 136,147 ****

  SELECT lo_export(loid, '@abs_builddir@/results/lotest.txt') FROM lotest_stash_values;

! \lo_import 'results/lotest.txt'

  \set newloid :LASTOID

  -- just make sure \lo_export does not barf
! \lo_export :newloid 'results/lotest2.txt'

  -- This is a hack to test that export/import are reversible
  -- This uses knowledge about the inner workings of large object mechanism
--- 136,147 ----

  SELECT lo_export(loid, '@abs_builddir@/results/lotest.txt') FROM lotest_stash_values;

! \lo_import '@abs_builddir@/results/lotest.txt'

  \set newloid :LASTOID

  -- just make sure \lo_export does not barf
! \lo_export :newloid '@abs_builddir@/results/lotest2.txt'

  -- This is a hack to test that export/import are reversible
  -- This uses knowledge about the inner workings of large object mechanism
Index: src/test/regress/output/largeobject.source
===================================================================
RCS file: /projects/cvsroot/pgsql/src/test/regress/output/largeobject.source,v
retrieving revision 1.4
diff -c -r1.4 largeobject.source
*** src/test/regress/output/largeobject.source    3 Mar 2007 22:57:04 -0000    1.4
--- src/test/regress/output/largeobject.source    29 Nov 2007 11:55:07 -0000
***************
*** 251,260 ****
           1
  (1 row)

! \lo_import 'results/lotest.txt'
  \set newloid :LASTOID
  -- just make sure \lo_export does not barf
! \lo_export :newloid 'results/lotest2.txt'
  -- This is a hack to test that export/import are reversible
  -- This uses knowledge about the inner workings of large object mechanism
  -- which should not be used outside it.  This makes it a HACK
--- 251,260 ----
           1
  (1 row)

! \lo_import '@abs_builddir@/results/lotest.txt'
  \set newloid :LASTOID
  -- just make sure \lo_export does not barf
! \lo_export :newloid '@abs_builddir@/results/lotest2.txt'
  -- This is a hack to test that export/import are reversible
  -- This uses knowledge about the inner workings of large object mechanism
  -- which should not be used outside it.  This makes it a HACK
Index: src/test/regress/output/largeobject_1.source
===================================================================
RCS file: /projects/cvsroot/pgsql/src/test/regress/output/largeobject_1.source,v
retrieving revision 1.1
diff -c -r1.1 largeobject_1.source
*** src/test/regress/output/largeobject_1.source    10 Mar 2007 03:42:19 -0000    1.1
--- src/test/regress/output/largeobject_1.source    29 Nov 2007 11:55:07 -0000
***************
*** 251,260 ****
           1
  (1 row)

! \lo_import 'results/lotest.txt'
  \set newloid :LASTOID
  -- just make sure \lo_export does not barf
! \lo_export :newloid 'results/lotest2.txt'
  -- This is a hack to test that export/import are reversible
  -- This uses knowledge about the inner workings of large object mechanism
  -- which should not be used outside it.  This makes it a HACK
--- 251,260 ----
           1
  (1 row)

! \lo_import '@abs_builddir@/results/lotest.txt'
  \set newloid :LASTOID
  -- just make sure \lo_export does not barf
! \lo_export :newloid '@abs_builddir@/results/lotest2.txt'
  -- This is a hack to test that export/import are reversible
  -- This uses knowledge about the inner workings of large object mechanism
  -- which should not be used outside it.  This makes it a HACK

Jorgen Austvik - Sun Norway <Jorgen.Austvik@Sun.COM> writes:
> I believe the results paths in line 139 and 144 are missing the
> @abs_builddir@ qualifier.

I'd put it the other way around: likely we should get rid of the
one use of @abs_builddir@.

            regards, tom lane

Tom Lane wrote:
> Jorgen Austvik - Sun Norway <Jorgen.Austvik@Sun.COM> writes:
>> I believe the results paths in line 139 and 144 are missing the
>> @abs_builddir@ qualifier.
>
> I'd put it the other way around: likely we should get rid of the
> one use of @abs_builddir@.

He, he.

Generally I prefer explicit over implicit (having the full paths make
troubleshooting easier), but in this case you have the additional aspect
of the lo_import operating relative to the client, while lo_export
operates relative to the server. If you remove @abs_builddir@ on the
first one, you might e.g. get problems like this:

   SELECT lo_export(loid, 'results/lotest.txt') FROM lotest_stash_values;
   ERROR:  could not create server file "results/lotest.txt": No such
   file or directory

-J
--

Jørgen Austvik, Software Engineering - QA
Sun Microsystems Database Technology Group

Jorgen Austvik - Sun Norway wrote:
> Tom Lane wrote:
>> Jorgen Austvik - Sun Norway <Jorgen.Austvik@Sun.COM> writes:
>>> I believe the results paths in line 139 and 144 are missing the
>>> @abs_builddir@ qualifier.
>> I'd put it the other way around: likely we should get rid of the
>> one use of @abs_builddir@.
>
> He, he.
>
> Generally I prefer explicit over implicit (having the full paths make
> troubleshooting easier), but in this case you have the additional aspect of
> the lo_import operating relative to the client, while lo_export operates
> relative to the server.

I submit that the test is OK as it currently is.  The lo_export() call
is expanded by the server, which can be running anywhere -- hence the
need to use an absolute path.

Then we have \lo_import and \lo_export calls which are relative to the
client.  The client is already running in the regress builddir, so using
relative paths works fine.

If I try to run the client from another directory, it fails completely.
Exactly what is the problem you are trying to fix?

$ cd ..
$ pwd
/pgsql/build/00head/src/test
$ regress/pg_regress largeobject
(using postmaster on Unix socket, port 55432)
============== dropping database "regression"         ==============
DROP DATABASE
============== creating database "regression"         ==============
CREATE DATABASE
ALTER DATABASE
============== running regression test queries        ==============
test largeobject          ... /bin/sh: cannot open ./sql/largeobject.sql: No such file
diff: ./expected/largeobject.out: No such file or directory
diff: ./results/largeobject.out: No such file or directory
diff command failed with status 512: diff -w "./expected/largeobject.out" "./results/largeobject.out" >
"./results/largeobject.out.diff"


--
Alvaro Herrera                        http://www.advogato.org/person/alvherre
"The Postgresql hackers have what I call a "NASA space shot" mentality.
 Quite refreshing in a world of "weekend drag racer" developers."
(Scott Marlowe)

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> Jorgen Austvik - Sun Norway wrote:
>> Tom Lane wrote:
>>> I'd put it the other way around: likely we should get rid of the
>>> one use of @abs_builddir@.
>>
>> Generally I prefer explicit over implicit (having the full paths make
>> troubleshooting easier), but in this case you have the additional aspect of
>> the lo_import operating relative to the client, while lo_export operates
>> relative to the server.

> I submit that the test is OK as it currently is.

Yeah, I hadn't thought about the different-paths aspect at the time of
making the above comment; but given that, it is correct as-is.

            regards, tom lane

Tom Lane wrote:
> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
>> I submit that the test is OK as it currently is.
>
> Yeah, I hadn't thought about the different-paths aspect at the time of
> making the above comment; but given that, it is correct as-is.

OK, I still think it is easier to debug with the paths there explicitly,
and I think the test will run just as well with them as without them,
but it is no biggie.

-J
--

Jørgen Austvik, Software Engineering - QA
Sun Microsystems Database Technology Group