Opened 3 years ago

Closed 3 years ago

#3496 closed defect (fixed)

Make postgis non-relocateable and schema qualify at least core functions (extension installs)

Reported by: robe Owned by: robe
Priority: high Milestone: PostGIS 2.3.0
Component: build/upgrade/install Version: trunk
Keywords: history Cc:

Description (last modified by robe)

I misunderstood the docs a bit. I blame pgAdmin for my stupidity because it always greyed out the schema feature in extension gui if an extension is marked non-relocatable.

I did some experiments, and it does seem that we can schema qualify our functions, without forcing users to install postgis in postgis schema. Though I still think that is where we want to move for the sake of other extensions that may rely on postgis.

Sooo if we have our control file look like this:

# postgis extension
comment = 'PostGIS geometry, geography, and raster spatial types and functions'
default_version = '2.3.0dev'
module_pathname = '$libdir/postgis-2.3'
relocatable = false

Then we can use the variable @extschema@ in lieu of the actual schema name. This will still allow users to do:

CREATE EXTENSION postgis SCHEMA whereever_I_damn_want_you_to_be;

And if I then have ST_Intersects rewritten as:

CREATE OR REPLACE FUNCTION ST_Intersects(geom1 geometry, geom2 geometry)
	RETURNS boolean
	AS 'SELECT $1 && $2 AND @extschema@._ST_Intersects($1,$2)'
	LANGUAGE 'sql' IMMUTABLE;

Once installed, in the desired user location, it will then become:

CREATE OR REPLACE FUNCTION ST_Intersects(geom1 geometry, geom2 geometry)
	RETURNS boolean
	AS 'SELECT $1 && $2 AND whereever_I_damn_want_you_to_be._ST_Intersects($1,$2)'
	LANGUAGE 'sql' IMMUTABLE;

upon installation. The downside being, the user can't then do:

ALTER EXTENSION postgis SET schema public;

They'll get a

ERROR:  extension "postgis" does not support SET SCHEMA

Now how to make this work nicely with non-extension piece.

I have some thoughts.

1) Have all our schema qualified calls have @extschema@ to follow extension convention.

2) Have a build-time extension schema value that defaults to nothing

3) Instead of having the perl build script run once and get used for both extension and non-extension, it would be changed to run twice.

4) If no extension schema is provided (and it's a regular non-extension build run), the perl scripts will just wipe out @extschema@. If there is a build time specified it will replace @extschema@ with the build time version.

5) the the control files, if there is a build-time schema specified (it will put this in the generated control file) -- this is more for future.

Attachments (1)

schema_lock_raster.patch (27.8 KB) - added by robe 3 years ago.
just portion to start patching raster

Download all attachments as: .zip

Change History (15)

comment:1 Changed 3 years ago by robe

Component: postgisbuild/upgrade/install
Owner: changed from pramsey to strk

comment:2 Changed 3 years ago by robe

Owner: changed from strk to robe

comment:3 Changed 3 years ago by robe

Description: modified (diff)

comment:4 Changed 3 years ago by robe

strk here is patch I created from my gogs repo. I don't know what I'm doing wrong why gogs refuses to accept my commits.

Changed 3 years ago by robe

Attachment: schema_lock_raster.patch added

just portion to start patching raster

comment:5 Changed 3 years ago by robe

patch committed at r15025.

I'll go ahead and complete the raster logic and then move on to key postgis functions.

strk, pramsey, dbaston, or dustymugs feel free to cleanup my horrible horrible perl code.

comment:6 Changed 3 years ago by strk

I've noticed you simply stripped away the @extschema@. Would it make more sense to replace it with a configure-time decided schema instead ? That way you could still have a secure installation of PostGIS in your schema of choice.

comment:7 Changed 3 years ago by strk

Another question: postgis.control.in now says "relocatable = false" but doesn't say in which schema to install it, how does it know ?

comment:8 Changed 3 years ago by strk

Sorry, figured (it's all explained in the summary). So basically via extension-install you decide the installation schema at "CREATE EXTENSION" time, and the same basically happens on non-extension-install (things are installed in the first schema of the search_path).

The difference between extension and non-extension installs is that extension-install has internal calls all fully-qualified to the schema decided at installation time, while non-extension install do not.

In turn, the above means that upgrade may still be broken for non-extension installs, right ?

I'm a bit afraid of this difference between extension and non-extension install. Moving to hard-coded destination schema (decided at build time) should remove the difference, and we'd have all internal linking schema-qualified.

Or, we should provide a postgis enabler-script to replace the manual load of the .sql files and provide the ability to specify target schema at that time. Would that be a too big of a change for non-extension installs ?

comment:9 Changed 3 years ago by robe

I feel most people are using extension installs, so I didn't want to spend too much time thinking about it. If enough people complain we can revisit at 2.4.

So the way I have it now non-extension installs are never schema qualified -- so yes they could break, but I left that open so we could instead of stripping replace with an at install time solution. That requires me fussing with configure which I really didn't want to spend time on.

I suspect most users not using extensions are compiling their own in which case install time is fine for them. If we wanted to go down the enabling script route though, then I think we'd end up with scripts much like the one I marked 'for_extension' and then the enabler would just replace the @extschema@ with whatever the user decides.

That actually if you do it now then we wouldn't need two sets of scripts, but it would mean teaching non-extension folks yet another new way to install. :)

comment:10 Changed 3 years ago by robe

Just about done schema qualifying all calls in raster. I probably missed some spots. I took out the add search_path to raster function from postgis extension include since not needed anymore.

However I had to change some tests (the ones that are supposed to error, errored in different location if installed via extension because of the extra characters introduced by the schema. So get something like error in Character 68 instead of Character 62. Very annoying.

comment:11 Changed 3 years ago by robe

In 15032:

schema qualify raster function calls
references #3496

comment:12 Changed 3 years ago by robe

Keywords: history added

comment:13 Changed 3 years ago by robe

Summary: Make postgis non-relocateable and schema qualify at least core functionsMake postgis non-relocateable and schema qualify at least core functions (extension installs)

comment:14 Changed 3 years ago by robe

Resolution: fixed
Status: newclosed

In 15041:

schema qualify function and operator calls in geometry and geography functions

Closes #3496
Closes #3494
Closes #3486
Closes #3076

Note: See TracTickets for help on using tickets.