Opened 8 years ago

Closed 3 years ago

Last modified 2 years ago

#2503 closed defect (fixed)

ERROR: table "topology" is not a member of the extension being created

Reported by: strk Owned by: robe
Priority: blocker Milestone: PostGIS 2.1.1
Component: build/upgrade/install Version: master
Keywords: extension Cc: robe

Description

Steps to reproduce:

  1. Install postgis-2.0.4 (full)
  2. Enable postgis, raster, topology into a "pg20" database using scripts (no extension)
  3. Install postgis-2.1.0 (full)
  4. CREATE EXTENSION postgis FROM '2.0.4'
  5. CREATE EXTENSION postgis_topology FROM '2.0.4'

The last step fails with:

ERROR: table "topology" is not a member of the extension being created

Change History (30)

comment:1 Changed 8 years ago by strk

Cc: robe added

comment:2 Changed 8 years ago by strk

Keywords: extension added

The statement triggering the error is this one:

SELECT pg_catalog.pg_extension_config_dump('topology', '');

Which is found in the postgis_topology--2.0.4--2.1.1dev.sql (and all the others). Same kind of error is triggered by this other call:

SELECT pg_catalog.pg_extension_config_dump('layer', '');

comment:3 Changed 8 years ago by strk

I think one problem is with misuse of CREATE EXTENSION command. Step 4 works by luck, both steps 4 and 5 are bogus because I'm pretending to have extension installed (rather than "unpackaged"). Not 100% sure this is bogus, and that's a second problem: we're not being friendly enough with the user to tell what's inappropriate call, if so.

Now, is it correct for use to put the pg_extension_config_dump in the upgrade scripts, rather than only in the unpackaged-to-whatever ?

Also, should we have multiple unpackaged--* scripts, one for each unpackaged minor version ?

comment:4 Changed 8 years ago by strk

forget the versioned unpackaged scripts, we already have that (unpackaged--<version>.sql). can be accessed with something like:

CREATE EXTENSION postgis VERSION '2.0.4' FROM unpackaged;

The usability problem still exist, and is that I can do:

CREATE EXTENSION postgis VERSION '2.1.1' FROM unpakcaged;

Even if the unpackaged version is 2.0.4. We should forbid that. Besides, I think we had a check preventing that when not using EXTENSION, but can't see it anymore (was it removed ?)

comment:5 Changed 8 years ago by strk

Forget the version check observation, it's still there but only prevents upgrades between different major (not minor) versions. I guess it would be an improvement to add checks for minors, to ensure upgrade paths are not misused.

comment:6 Changed 8 years ago by pramsey

Milestone: PostGIS 2.1.1

comment:7 Changed 8 years ago by robe

I think packaging the prior unpackaged with newer is out of the question since what is in packaged changes too much even across micros with hidden functions. Because the unpackaged script has to include every function/type/etc/ that was packaged with that version.

The correct way to upgrade to packaged is to 1) Upgrade without extension (to bring you up to 2.1.1 or whatever) 2) then run CREATE EXTENSION ... FROM unpackaged.

Is topology schema in your search path? I'm thinking those commands since (topology is always going to be in topology schema) should be prefixed with topology.

Though that brings up the question of if you ran with just CREATE EXTENSION .. FROM unpackaged if it would run those scripts. I don't think I have those in the unpackaged script, but probably should.

SELECT pg_catalog.pg_extension_config_dump('topology.topology', '');
SELECT pg_catalog.pg_extension_config_dump('topology.layer', '');

They are for backup so should be in run when you are going from no extension to extension.

the reason is because the table structures are never backed up and the data is only backed up if you have those commands.

comment:8 Changed 8 years ago by robe

Owner: changed from strk to robe

comment:9 Changed 8 years ago by robe

Resolution: invalid
Status: newclosed

strk,

I'm going to dismiss this as wontfix. I just realized now the reason you got "topology" is not a member of extension is valid and expected.

In the above as you said you are not installing from unpackaged at all, you are trying to install postgis anew and postgis_topology anew and there is no way for the extension model to know you already have postgis installed.

So the error you get

"ERROR: table "topology" is not a member of the extension being created "

means you are trying to install topology and it is failing at packaging it already exists as not part of package.

The correct steps are

CREATE EXTENSION postgis FROM unpackaged version '2.0.4';
CREATE EXTENSION postgis_topology FROM unpackaged version '2.0.4';
ALTER EXTENSION postgis UPDATE TO "2.1.0";
ALTER EXTENSION postgis_topology UPDATE TO "2.1.0";

and that works for me. I will say that we could have documented the behavior better and there is an issue with me dropping the postgis extension without doing a drop cascade. I think that issue though is because of wholes in my unpackaged script which should be fixed by your updated one.

comment:10 Changed 8 years ago by strk

So you're saying that what I tried to do with:

CREATE EXTENSION postgis FROM '2.0.4' 

Is instead done with:

CREATE EXTENSION postgis FROM unpackaged VERSION '2.0.4';

Is that correct ?

This is important info for #2535.

In this case we'd want the first attempt to fail with a nice error message telling the user about the problem. I guess the problem here was "PostGIS EXTENSION '2.0.4' is not installed", is that correct ? Shouldn't PostgreSQL itself error out in that case ?

comment:11 Changed 8 years ago by strk

I would add that since there could be different versions of unpackaged, it would have more sense to use:

CREATE EXTENSION postgis FROM '2.0.4-unpackaged';

or something like that, expecting PostgreSQL to bring it up to whatever current version is installed...

comment:12 Changed 8 years ago by robe

That doesn't make much sense to me. The unpackaged scripts are already versioned.

CREATE EXTENSION postgis FROM unpackaged VERSION '2.2.0dev';

means run postgis--unpackaged--2.2.0dev.sql

What you are proposing would create a version that is not upgradeable without adding a lot more dupes.

The best way to upgrade which I think which I outlined in the docs in section

http://postgis.net/docs/manual-2.1/postgis_installation.html#make_install_postgis_extensions

Is if you started off without installing using extensions, then you

  1. Upgrade without extensions
  1. then you run the unpackaged step -- which always defaults to the latest installed postgis version.

The reason for doing it that way as opposed to first install from unpackaged version ... is that the unpackaged version ... has to exist, and if your distro package is super anal, they would have uninstalled the older version, before giving you the newer version, thus making it impossible to upgrade that way.

comment:13 Changed 3 years ago by strk

Resolution: invalid
Status: closedreopened
Version: 2.1.xtrunk

I'm reopening this because I'm seeing this error again now as part of postgis_extensions_upgrade call, while I'm adding re-packaging feature in it (see #4197).

The workflow in this case is:

 createdb x
 psql -f postgis/postgis.sql x
 psql -f spatial_ref_sys.sql x
 psql -f topology/topology.sql x
 psql -c 'create extension postgis from unpackaged' x
 psql -c 'create extension postgis_topology from unpackaged' x

Again, I fail to see why the above should not work...

comment:14 Changed 3 years ago by strk

I shall note that postgis--unpackaged--3.0.0dev.sql does NOT contain any call to pg_extension_config_dump, which is the function triggering this error. Is it a bug for postgis_topology--unpackaged--3.0.0dev.sql to contain it ? Or is it a bug for postgis--unpackaged--3.0.0dev.sql to NOT contain it ?

Regina: extension is your land, please help me fight this beast

comment:15 Changed 3 years ago by robe

strk you should probably schema qualify the topology layer and topology table in backup.

Not sure if that is what is causing your issue though. Are you sure you are adding the topology schema, topology.layer and topology.topology table to the extension using an

ALTER EXTENSION postgis_topology ADD ....;

If you are not doing that before those calls are made, that would cause this issue.

comment:16 Changed 3 years ago by strk

The postgis_topology--unpackaged--3.0.0dev.sql script contains these lines:

DO $$
BEGIN
 ALTER EXTENSION postgis_topology ADD TABLE topology.layer;
 RAISE NOTICE 'newly registered TABLE topology.layer';
EXCEPTION WHEN object_not_in_prerequisite_state THEN
  IF SQLERRM ~ '\mpostgis_topology\M'
  THEN
    RAISE NOTICE 'already registered TABLE topology.layer';
  ELSE
    RAISE EXCEPTION '%', SQLERRM;
  END IF;
END;
$$ LANGUAGE 'plpgsql';
DO $$
BEGIN
 ALTER EXTENSION postgis_topology ADD TABLE topology.topology;
 RAISE NOTICE 'newly registered TABLE topology.topology';
EXCEPTION WHEN object_not_in_prerequisite_state THEN
  IF SQLERRM ~ '\mpostgis_topology\M'
  THEN
    RAISE NOTICE 'already registered TABLE topology.topology';
  ELSE
    RAISE EXCEPTION '%', SQLERRM;
  END IF;
END;
$$ LANGUAGE 'plpgsql';

But they are at the end of the file. If I move them at the beginning, right after the lines with

postgis_extension_remove_objects

Then CREATE .. FROM unpackaged works great !

Regina, cannot you reproduce on your system ? This is with PostgreSQL 9.6.9.

I guess the parser is complaining about code in function body writing into topology.topology _BEFORE_ that table is registered with the extension (not sure)

comment:17 Changed 3 years ago by strk

I just noticed that our utils/check_all_upgrades script NEVER checks upgrades of postgis_topology (or now postgis_raster) in that it only runs make check from under the regress/ directory. That would explain why this problem was not raised by our bots before. I'll ticket fixing this one.

comment:18 Changed 3 years ago by strk

Testing upgrades of topology (and raster) was ticketed as #4199

comment:19 Changed 3 years ago by strk

Priority: mediumblocker

comment:20 Changed 3 years ago by strk

Another easier way in which you can reproduce:

regress/run_test.pl -v --topology --extension \
  --upgrade-path unpackaged--3.0.0dev \
  topology/test/regress/gml.sql 

comment:21 Changed 3 years ago by strk

Testing of upgrades was committed and revealed this issue does not affect upgrades from previous topology releases. But it does affect upgrades from unpackaged (on my machine) which is still untested. Having bots check upgrades from unpackaged is hanlded in #4204 (although I'm thinking we should check that from the normal "make installcheck" rather than reaching out to utils/check_all_upgrades.sh in that upgrades from unpackage don't need any previous version installed on the system)

comment:22 Changed 3 years ago by strk

There, Dronie now also confirms: https://drone.osgeo.org/strk/postgis/81/3 This is after enabling extension packaging testing (#4204)

comment:23 Changed 3 years ago by strk

So, the other fix is moving the pg_catalog.pg_extension_config_dump dumps to _AFTER_ the corresponding tables were added to the extension. Right now those calls are part of the extension upgrade scripts (--ANY--3.0.0dev.sql) so they happen _before_.

I don't know why we use the extension upgrade scripts rather than the normal upgrade scripts for the pre-packaging upgrade, I think it is wrong. Will try using the normal (script-based) upgrade script instead

comment:24 Changed 3 years ago by strk

Regina: when is the _config_dump call needed ? Is it only when packaging or also on upgrade ?

comment:25 Changed 3 years ago by strk

And, how can we test that _config_dump was called ? Do we have/need dump/restore tests ?

comment:26 Changed 3 years ago by robe

I thought we have a dump restore in run_test though maybe we turned it off or don't have it for topology

comment:27 Changed 3 years ago by strk

Yes, run_test.pl accepts a --dumprestore switch.

Winnie, Debbie and GitLab-CI seem to be passing that switch via RUNTESTFLAGS on "make check" so they must be also including topology

comment:28 Changed 3 years ago by strk

But what we need is probably a specific test to check if custom records written in those "config tables" are retained across dump/reload operations, I guess. This is most likely _not_ done by run_test.pl (how could it know...)

comment:29 Changed 3 years ago by strk

Resolution: fixed
Status: reopenedclosed

In 16925:

Do not mark extention objects during pre-extension upgrades

Fixes CREATE EXTENSION postgis_topology FROM unpackaged
Closes #2503

comment:30 Changed 2 years ago by strk

In 17757:

Have postgis_extensions_upgrade repackage topology as well

Closes #4486
References #2503

See failure here:
https://gitlab.com/postgis/postgis/-/jobs/277847404

Note: See TracTickets for help on using tickets.