Thread: backend dies on 7.1.1 loading large datamodel.

I just checked out CVS this morning the REL7_1_STABLE branch.  I
configured it with:

  ./configure --enable-debug

and ran the regression test fine on OpenBSD 2.8  (AMD processor) (The
same problem has been reproduced by someone else on RH6.2)

I then proceed to load the OpenACS datamodel and had the backend crash.
This datamodel loads fine on 7.1.

I can send the datamodel and core file if needed.  I loaded GDB with the
core file and got the following:

$ gdb /usr/local/pgsql/bin/postmaster postgres.core
GNU gdb 4.16.1
  :: snip ::
Program terminated with signal 11, Segmentation fault.
  :: snip ::
#0  SPI_gettypeid (tupdesc=0x0, fnumber=1) at spi.c:501
501             if (tupdesc->natts < fnumber || fnumber <= 0)
(gdb) where
#0  SPI_gettypeid (tupdesc=0x0, fnumber=1) at spi.c:501
#1  0x402946bf in exec_move_row (estate=0xdfbfcddc, rec=0x0, row=0x186420,
    tup=0x0, tupdesc=0x0) at pl_exec.c:2640
#2  0x40292b71 in exec_stmt_select (estate=0xdfbfcddc, stmt=0x186600)
    at pl_exec.c:1455
#3  0x40292252 in exec_stmt (estate=0xdfbfcddc, stmt=0x186600) at pl_exec.c:978
#4  0x402920ea in exec_stmts (estate=0xdfbfcddc, stmts=0x276410)
    at pl_exec.c:920
#5  0x40292044 in exec_stmt_block (estate=0xdfbfcddc, block=0x186660)
    at pl_exec.c:876
#6  0x402914c1 in plpgsql_exec_function (func=0x27b500, fcinfo=0x22b65c)
    at pl_exec.c:381
#7  0x4028edb6 in plpgsql_call_handler (fcinfo=0x22b65c) at pl_handler.c:128
#8  0x83be1 in ExecMakeFunctionResult (fcache=0x22b648, arguments=0x22b058,
    econtext=0x22b0f8, isNull=0xdfbfcf2b "", isDone=0xdfbfcf2c)
    at execQual.c:807
#9  0x83c9a in ExecEvalFunc (funcClause=0x22b140, econtext=0x22b0f8,
    isNull=0xdfbfcf2b "", isDone=0xdfbfcf2c) at execQual.c:901
#10 0x840e9 in ExecEvalExpr (expression=0x22b140, econtext=0x22b0f8,
    isNull=0xdfbfcf2b "", isDone=0xdfbfcf2c) at execQual.c:1226
#11 0x843e1 in ExecTargetList (targetlist=0x22b1b0, nodomains=1,
 :: snip ::
#24 0x9314e in main (argc=3, argv=0xdfbfdc3c) at main.c:171
(gdb) p tupdesc
$1 = 0x0
(gdb)

On Mon, May 07, 2001 at 05:14:48PM -0500, Robert Hentosh wrote:
  :: snip ::
> I can send the datamodel and core file if needed.  I loaded GDB with the
> core file and got the following:

I just put the datamodel at http://www.io.com/~hentosh/sql.tar.gz

Robert Hentosh <hentosh@io.com> writes:
> I then proceed to load the OpenACS datamodel and had the backend crash.
> This datamodel loads fine on 7.1.

Ugh.

> I can send the datamodel and core file if needed.

Datamodel please, corefile no.

            regards, tom lane

Robert Hentosh <hentosh@io.com> writes:
> I then proceed to load the OpenACS datamodel and had the backend crash.
> This datamodel loads fine on 7.1.

Sigh.  Looks like I managed to break plpgsql's handling of SELECT with
no data found.  Mea maxima culpa (though the regression tests perhaps
deserve a share of the blame too, for not covering that case).

Patch to appear shortly.  I guess there will be a 7.1.2 sooner than
I thought, also :-(

            regards, tom lane

Robert Hentosh <hentosh@io.com> writes:
> I just put the datamodel at http://www.io.com/~hentosh/sql.tar.gz

Hm.  I notice that postgres.sql hardwires the location of the plpgsql
handler:

create function plpgsql_call_handler() RETURNS opaque
as '/usr/local/pgsql/lib/plpgsql.so' language 'c';

create trusted procedural language 'plpgsql'
HANDLER plpgsql_call_handler
LANCOMPILER 'PL/pgSQL';

If this were to suck in a wrong-version copy of plpgsql.so (and yes,
I think 7.1 vs 7.1.1 could be wrong version) then that could cause
failures.

postgres-pgtcl.sql is equally unwise about the pltcl handler.

This is *not* the source of your problem, since I was able to
reproduce the crash even with a proper "createlang plpgsql" used
instead of the bogus commands.  But you might want to pass on the
observation to the OpenACS guys.

On with debugging ...

            regards, tom lane

On Mon, May 07, 2001 at 08:28:33PM -0400, Tom Lane wrote:
> Robert Hentosh <hentosh@io.com> writes:
> > I just put the datamodel at http://www.io.com/~hentosh/sql.tar.gz
>
> Hm.  I notice that postgres.sql hardwires the location of the plpgsql
> handler:
>
> create function plpgsql_call_handler() RETURNS opaque
> as '/usr/local/pgsql/lib/plpgsql.so' language 'c';
>
> create trusted procedural language 'plpgsql'
> HANDLER plpgsql_call_handler
> LANCOMPILER 'PL/pgSQL';
>
> If this were to suck in a wrong-version copy of plpgsql.so (and yes,
> I think 7.1 vs 7.1.1 could be wrong version) then that could cause
> failures.

I played with this a little.  What would be the proper solution?
Doesn't the backend go and cd to the data directory.  I blindly
tried:
    as 'plpgsql.so' language 'c';
and
    as 'lib/plpgsql.so' language 'c';

and it can't find the file.  Is there a way to correctly reference the
lib directory associated with the execuables directory structure?

One of the examples in the docs shows the full path, too. At the bottom
of this URL:

http://postgresql.readysetnet.com/users-lounge/docs/7.0/postgres/sql-createlanguage.htm

Tom Lane wrote:
>
> Robert Hentosh <hentosh@io.com> writes:
> > I then proceed to load the OpenACS datamodel and had the backend crash.
> > This datamodel loads fine on 7.1.
>
> Ugh.
>

There was a bug report in Japan that plpgsql
crashes if select returns no row. This seems
a bug introduced by your latest change on
pl_exec.c.

regards,
Hiroshi Inoue

Hiroshi Inoue <Inoue@tpf.co.jp> writes:
> There was a bug report in Japan that plpgsql
> crashes if select returns no row. This seems
> a bug introduced by your latest change on
> pl_exec.c.

Indeed.  Too bad that report didn't arrive on Friday.
I am mightily embarrassed :-(

            regards, tom lane

Robert Hentosh <hentosh@io.com> writes:
>> If this were to suck in a wrong-version copy of plpgsql.so (and yes,
>> I think 7.1 vs 7.1.1 could be wrong version) then that could cause
>> failures.

> I played with this a little.  What would be the proper solution?

At the moment, the solution is to use the createlang script rather than
issuing the commands directly.

In the long run I think we should abandon the notion that full path
specifications are the preferred way to locate dynamic libraries.
It would be a lot better for portability if C function libraries could
be referred to like this:

create function pltcl_call_handler() returns opaque
    as 'pltcl' language 'C';

where the backend automatically assumes that a relative path is relative
to $PGLIB.  I'd like to see the backend adding the file extension too,
to avoid platform dependencies (".so" is not universal).  A function
definition like the above could be dumped and reloaded without fear,
whereas the existing approach is pretty much guaranteed to break
whenever you change machines or install directories.

            regards, tom lane

> ... (though the regression tests perhaps
> deserve a share of the blame too, for not covering that case).
> Patch to appear shortly...

Will the patch include a case for the regression test? Or could someone
(other than me??!!) volunteer to cover that?

                    - Thomas

Thomas Lockhart <lockhart@alumni.caltech.edu> writes:
> Will the patch include a case for the regression test? Or could someone
> (other than me??!!) volunteer to cover that?

Seems like a good idea.  As the embarrassee, I'm perhaps too close
to the problem to write a good addition to the regress tests; any
volunteers?

            regards, tom lane

> Hiroshi Inoue <Inoue@tpf.co.jp> writes:
> > There was a bug report in Japan that plpgsql
> > crashes if select returns no row. This seems
> > a bug introduced by your latest change on
> > pl_exec.c.
>
> Indeed.  Too bad that report didn't arrive on Friday.
> I am mightily embarrassed :-(

Humbled, I would say.  Happens to us all.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Tom Lane writes:

> In the long run I think we should abandon the notion that full path
> specifications are the preferred way to locate dynamic libraries.
> It would be a lot better for portability if C function libraries could
> be referred to like this:
>
> create function pltcl_call_handler() returns opaque
>     as 'pltcl' language 'C';
>
> where the backend automatically assumes that a relative path is relative
> to $PGLIB.  I'd like to see the backend adding the file extension too,
> to avoid platform dependencies (".so" is not universal).

We could have a run-time parameter that sets a path where to look for
modules.  Eventually, we might also want to take a look at libtool's
libltdl, the portable interface to dynamic loading, which would do this
and a number of other things for us.

--
Peter Eisentraut   peter_e@gmx.net   http://funkturm.homeip.net/~peter

Peter Eisentraut <peter_e@gmx.net> writes:
> Tom Lane writes:
>> where the backend automatically assumes that a relative path is relative
>> to $PGLIB.  I'd like to see the backend adding the file extension too,
>> to avoid platform dependencies (".so" is not universal).

> We could have a run-time parameter that sets a path where to look for
> modules.

For obvious security reasons, the library path must only be settable by
the dbadmin, and I see no good reason that it should be changeable
on-the-fly.  But we could treat it as a postmaster-start-only GUC
parameter...

            regards, tom lane

Tom Lane writes:

> For obvious security reasons, the library path must only be settable by
> the dbadmin, and I see no good reason that it should be changeable
> on-the-fly.  But we could treat it as a postmaster-start-only GUC
> parameter...

On the fly could be useful for trying alternative sets of modules.  Also,
you could include such a SET command into your function loading script.

--
Peter Eisentraut   peter_e@gmx.net   http://funkturm.homeip.net/~peter