Re: Extension upgrade, patch v0: debug help needed

Hi,

I've been working on finishing the extension's coding work for 9.1 with
adding support for extensions UPGRADE. That means both running the
upgrade script of an already installed extension and supporting the case
when a bunch of objects were not recognized as an extension before.

This is important in two respects. First, any user having been using
PostgreSQL before 9.1 and having installed a contrib or some third-party
code in his database will want to be able to benefit from 9.1
extensions. More than that, extensions will not grow out of nowhere: in
most cases "internal extensions" (unpublished code, PL collection for a
given application, etc) will be made from already existing objects in a
database.

To support that is quite simple in fact, as the following commands will
do the trick:

CREATE WRAPPER EXTENSION ...; -- don't run the script
ALTER OBJECT ... SET EXTENSION ...; -- that's in the upgrade script
ALTER EXTENSION ... UPGRADE; -- as "usual"

Here's an example:

dim=# \i ~/pgsql/exts/share/contrib/lo.sql
CREATE DOMAIN
CREATE FUNCTION
CREATE FUNCTION

dim=# create wrapper extension lo;
CREATE EXTENSION

dim=# alter extension lo upgrade;
ALTER EXTENSION

dim=# \dx lo
Objects in extension "lo"
Object Class | Object Description
--------------+----------------------
pg_extension | extension lo
pg_type | type lo
pg_proc | function lo_oid(lo)
pg_proc | function lo_manage()
(4 rows)

The WRAPPER keyword meaning is that we're only creating the catalog
entry, forcing version to NULL and not running the extension's script.
This keyword looked like the best choice from existing ones in our
grammar, please feel free to pick any other phrase here.

Now, I need help because I can't seem to be able to debug the following
problem. I have added an Assert() in my patch so that it's easy to spot
where it's failing, I had "unrecognized node type: %d" error before (and
left a printf-style debug helpful "BAR" addition in the patch too).

Note that running the prepared script (@extschema@ -> public) by hand
works, but that's avoiding the SPI call stack…

dim=# alter extension citext upgrade;

Program received signal SIGABRT, Aborted.
0x00007fff83e07616 in __kill ()
(gdb) bt
#0 0x00007fff83e07616 in __kill ()
#1 0x00007fff83ea7cca in abort ()
#2 0x00000001003b62b9 in ExceptionalCondition (conditionName=Could not find the frame base for "ExceptionalCondition".
) at assert.c:57
#3 0x00000001001eb38f in copyObject (from=0x10110a6e0) at copyfuncs.c:3723
#4 0x00000001001eb192 in _copyList (from=0x10110a7a0) at copyfuncs.c:3657
#5 0x00000001001ec723 in copyObject (from=0x10110a7a0) at copyfuncs.c:4018
#6 0x00000001001e86eb in _copyAlterObjectExtensionStmt (from=0x10110a720) at copyfuncs.c:2837
#7 0x00000001001ec9ab in copyObject (from=0x10110a720) at copyfuncs.c:4136
#8 0x00000001001c3956 in _SPI_prepare_plan (src=0x1010f4c38 "/* contrib/citext/citext.upgrade.sql */\n\n--\n--\tUpgrade the citext extension from before there was extension support\n--\n\nalter type public.citext set extension citext;\nalter function public.citextin(cs"..., plan=0x7fff5fbfe950, boundParams=0x0) at spi.c:1712
#9 0x00000001001c0adb in SPI_execute (src=0x1010f4c38 "/* contrib/citext/citext.upgrade.sql */\n\n--\n--\tUpgrade the citext extension from before there was extension support\n--\n\nalter type public.citext set extension citext;\nalter function public.citextin(cs"..., read_only=0 '\0', tcount=0) at spi.c:357
#10 0x00000001001384ec in execute_sql_string (sql=0x1010f4c38 "/* contrib/citext/citext.upgrade.sql */\n\n--\n--\tUpgrade the citext extension from before there was extension support\n--\n\nalter type public.citext set extension citext;\nalter function public.citextin(cs"...) at extension.c:429
#11 0x0000000100138931 in execute_extension_script (control=0x1010006d8, schema=0x1010006b8 "public", create=0 '\0', user_data=0 '\0') at extension.c:559
#12 0x000000010013b400 in UpgradeExtension (stmt=0x10101d4b0) at extension.c:1529
#13 0x00000001002b37da in standard_ProcessUtility (parsetree=0x10101d4b0, queryString=0x10101ca38 "alter extension citext upgrade;", params=0x0, isTopLevel=1 '\001', dest=0x10101d828, completionTag=0x7fff5fbff210 "") at utility.c:575
#14 0x0000000100856161 in pgss_ProcessUtility (parsetree=0x10101d4b0, queryString=0x10101ca38 "alter extension citext upgrade;", params=0x0, isTopLevel=1 '\001', dest=0x10101d828, completionTag=0x7fff5fbff210 "") at pg_stat_statements.c:603
#15 0x00000001002b2fb3 in ProcessUtility (parsetree=0x10101d4b0, queryString=0x10101ca38 "alter extension citext upgrade;", params=0x0, isTopLevel=1 '\001', dest=0x10101d828, completionTag=0x7fff5fbff210 "") at utility.c:328
#16 0x00000001002b1dd8 in PortalRunUtility (portal=0x10103c638, utilityStmt=0x10101d4b0, isTopLevel=1 '\001', dest=0x10101d828, completionTag=0x7fff5fbff210 "") at pquery.c:1191
#17 0x00000001002b1f5b in PortalRunMulti (portal=0x10103c638, isTopLevel=1 '\001', dest=0x10101d828, altdest=0x10101d828, completionTag=0x7fff5fbff210 "") at pquery.c:1296
#18 0x00000001002b1530 in PortalRun (portal=0x10103c638, count=9223372036854775807, isTopLevel=1 '\001', dest=0x10101d828, altdest=0x10101d828, completionTag=0x7fff5fbff210 "") at pquery.c:822
#19 0x00000001002aab21 in exec_simple_query (query_string=0x10101ca38 "alter extension citext upgrade;") at postgres.c:1059
#20 0x00000001002af55e in PostgresMain (argc=2, argv=0x10101a5c0, username=0x10101a4a0 "dim") at postgres.c:3932
#21 0x00000001002649f0 in BackendRun (port=0x100901ef0) at postmaster.c:3560
#22 0x0000000100264019 in BackendStartup (port=0x100901ef0) at postmaster.c:3245
#23 0x0000000100261154 in ServerLoop () at postmaster.c:1431
#24 0x0000000100260847 in PostmasterMain (argc=3, argv=0x100900590) at postmaster.c:1092
#25 0x00000001001d4d4e in main (argc=3, argv=0x100900590) at main.c:199
(gdb) frame 6
#6 0x00000001001e86eb in _copyAlterObjectExtensionStmt (from=0x101109f20) at copyfuncs.c:2837
2837 COPY_NODE_FIELD(objarg);
(gdb) print ((Node*)(from))->type
$4 = T_AlterObjectExtensionStmt
(gdb) frame 5
#5 0x00000001001ec723 in copyObject (from=0x101109fa0) at copyfuncs.c:4018
4018 retval = _copyList(from);
(gdb) print ((Node*)(from))->type
$5 = T_List
(gdb) frame 4
#4 0x00000001001eb192 in _copyList (from=0x101109fa0) at copyfuncs.c:3657
3657 COPY_NODE_CELL(new->head, from->head);
(gdb) print ((Node*)(from))->type
$6 = T_List
(gdb) frame 3
#3 0x00000001001eb38f in copyObject (from=0x101109ee0) at copyfuncs.c:3723
3723 Assert(nodeTag(from) <= T_InlineCodeBlock);
(gdb) print ((Node*)(from))->type
$7 = 1701999714

The problem occurs on ALTER OPERATOR FAMILY ... SET EXTENSION, that's
what dichotomy on the citext.upgrade.sql tells me.

Another try, and getting some more information:

(gdb) frame 6
#6 0x00000001001e86eb in _copyAlterObjectExtensionStmt (from=0x10110c120) at copyfuncs.c:2837
2837 COPY_NODE_FIELD(objarg);
(gdb) print list_head(from->objarg)->data.ptr_value
$1 = (void *) 0x10110c0e0
(gdb) print (char *)list_head(from->objarg)->data.ptr_value
$2 = 0x10110c0e0 "btree"
(gdb) frame 4
#4 0x00000001001eb192 in _copyList (from=0x10110c1a0) at copyfuncs.c:3657
3657 COPY_NODE_CELL(new->head, from->head);
(gdb) print ((Node*)(from))->type
$3 = T_List
(gdb) print (char *)list_head(from)->data.ptr_value
$4 = 0x10110c0e0 "btree"
(gdb) frame 3
#3 0x00000001001eb38f in copyObject (from=0x10110c0e0) at copyfuncs.c:3723
3723 Assert(nodeTag(from) <= T_InlineCodeBlock);
(gdb) print (char *)list_head(from)->data.ptr_value
$5 = 0x259 <Address 0x259 out of bounds>
(gdb) print from
$6 = (void *) 0x10110c0e0
(gdb) print list_head(from)->data
$7 = {
ptr_value = 0x259,
int_value = 601,
oid_value = 601
}
(gdb) print (char *) from
$8 = 0x10110c0e0 "btree"

How to attack and debug that? Is it something obvious that I missed?

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

PS: yes, current patch contains some cleanups that should go into the
main extension patch, that's next step.

Dimitri Fontaine <dimitri(at)2ndQuadrant(dot)fr> writes:
> The problem occurs on ALTER OPERATOR FAMILY ... SET EXTENSION, that's
> what dichotomy on the citext.upgrade.sql tells me.

The code in question was copy/pasted from the SET SCHEMA code path in
gram.y then other related files. So I just tested a clean HEAD checkout
then the following steps:

make -C contrib/citext install
psql -f .../head/share/contrib/citext.sql
psql
dim=# do $$ begin execute 'alter operator class public.citext_ops using btree set schema utils'; end; $$;
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.

Will try to debug that as soon as possible, but spare time here tends to
be sparse so I preferred sending this mail first. Preliminary
investigation leads me thinking the cause is using n->objarg rather than
n->addname to host the access_method. Working on a fix.

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

Dimitri Fontaine <dimitri(at)2ndQuadrant(dot)fr> writes:
> make -C contrib/citext install
> psql -f .../head/share/contrib/citext.sql
> psql
> dim=# do $$ begin execute 'alter operator class public.citext_ops using btree set schema utils'; end; $$;
> server closed the connection unexpectedly
> This probably means the server terminated abnormally
> before or while processing the request.
> The connection to the server was lost. Attempting reset: Failed.

The fix was ok, but I had to test with the right environment to be able
to appreciate that :)

Please find it attached.

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

On Jan 1, 2011, at 2:30 PM, Dimitri Fontaine wrote:

> To support that is quite simple in fact, as the following commands will
> do the trick:
>
> CREATE WRAPPER EXTENSION ...; -- don't run the script
> ALTER OBJECT ... SET EXTENSION ...; -- that's in the upgrade script
> ALTER EXTENSION ... UPGRADE; -- as "usual"

I rather doubt that "WRAPPER" will be accepted as a reserved word in the grammar.

> Here's an example:
>
> dim=# \i ~/pgsql/exts/share/contrib/lo.sql
> CREATE DOMAIN
> CREATE FUNCTION
> CREATE FUNCTION
>
> dim=# create wrapper extension lo;
> CREATE EXTENSION

What happened to your UPGRADE from NULL idea?

> The WRAPPER keyword meaning is that we're only creating the catalog
> entry, forcing version to NULL and not running the extension's script.
> This keyword looked like the best choice from existing ones in our
> grammar, please feel free to pick any other phrase here.

I don't see why it's necessary at all.

Best,

David

"David E. Wheeler" <david(at)kineticode(dot)com> writes:
> I rather doubt that "WRAPPER" will be accepted as a reserved word in the grammar.

It's already in the grammar, and I didn't change its "level".

>> dim=# create wrapper extension lo;
>> CREATE EXTENSION
>
> What happened to your UPGRADE from NULL idea?

You upgrade an installed extension, right?

>> The WRAPPER keyword meaning is that we're only creating the catalog
>> entry, forcing version to NULL and not running the extension's script.
>> This keyword looked like the best choice from existing ones in our
>> grammar, please feel free to pick any other phrase here.
>
> I don't see why it's necessary at all.

Care to offer an alternative or go in any level of details about what
you have in mind and how *exactly* you think it should work? I've not
been offered psychic powers this Christmas, sorry…

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

On Jan 3, 2011, at 11:49 AM, Dimitri Fontaine wrote:

> "David E. Wheeler" <david(at)kineticode(dot)com> writes:
>> I rather doubt that "WRAPPER" will be accepted as a reserved word in the grammar.
>
> It's already in the grammar, and I didn't change its "level".

Okay.

>>> dim=# create wrapper extension lo;
>>> CREATE EXTENSION
>>
>> What happened to your UPGRADE from NULL idea?
>
> You upgrade an installed extension, right?

What?

>>> The WRAPPER keyword meaning is that we're only creating the catalog
>>> entry, forcing version to NULL and not running the extension's script.
>>> This keyword looked like the best choice from existing ones in our
>>> grammar, please feel free to pick any other phrase here.
>>
>> I don't see why it's necessary at all.
>
> Care to offer an alternative or go in any level of details about what
> you have in mind and how *exactly* you think it should work? I've not
> been offered psychic powers this Christmas, sorry…

That's what I understood your original "UPGRADE from NULL" being. Did I misread you?

Best,

David

"David E. Wheeler" <david(at)kineticode(dot)com> writes:
> That's what I understood your original "UPGRADE from NULL" being. Did I misread you?

Are the docs about the feature, available handy in HTML so that you
don't have to read them in SGML at my git repository, are they *that*
bad?

http://pgsql.tapoueh.org/extensions/doc/html/extend-extension.html

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

On Jan 3, 2011, at 11:54 AM, Dimitri Fontaine wrote:

>> That's what I understood your original "UPGRADE from NULL" being. Did I misread you?
>
> Are the docs about the feature, available handy in HTML so that you
> don't have to read them in SGML at my git repository, are they *that*
> bad?
>
> http://pgsql.tapoueh.org/extensions/doc/html/extend-extension.html

I was responding to your email mentioning it, which did not reference said docs.

David

"David E. Wheeler" <david(at)kineticode(dot)com> writes:
>> http://pgsql.tapoueh.org/extensions/doc/html/extend-extension.html
>
> I was responding to your email mentioning it, which did not reference said docs.

Fair enough, I'm still interested in you telling me if I get to rewrite
them all or if it's explaining the thing well enough :)

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

On Sun, Jan 2, 2011 at 6:43 AM, Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr> wrote:
> Dimitri Fontaine <dimitri(at)2ndQuadrant(dot)fr> writes:
>>   make -C contrib/citext install
>>   psql -f .../head/share/contrib/citext.sql
>>   psql
>>   dim=# do $$ begin execute 'alter operator class public.citext_ops using btree set schema utils'; end; $$;
>> server closed the connection unexpectedly
>>       This probably means the server terminated abnormally
>>       before or while processing the request.
>> The connection to the server was lost. Attempting reset: Failed.
>
> The fix was ok, but I had to test with the right environment to be able
> to appreciate that :)

Committed.

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

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> Committed.

Thanks!

--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support