Making PostGIS not schema relocateable to fix materialized view, restore issues, foreign table issues

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Making PostGIS not schema relocateable to fix materialized view, restore issues, foreign table issues

Regina Obe
I've been giving this a lot of thought and have my proposition in PostGIS
2.3

https://trac.osgeo.org/postgis/ticket/3496

I want to know if people have issue with this before I push forward.


Right now for PostGIS 2.2 I had included a conditional script that adds
search_path to many functions to fix the infamous raster restore issues and
some issues people were having with materialized views.

In PostGIS 2.3, I made this script included as part of extension install.
Eventually I will probably take some of this out and go more with
selectively schema qualifying function calls.

In order to schema qualify postgis calls in our functions, we need to make
PostGIS non-relocateable.

The reason why doing the search_path thing was not good enough is

1) In certain situations it changes the planner's behavior. E.g. it prevents
inlining from happening which meant I couldn't put in in functions like
ST_DWithin, ST_Intersects that are commonly used -
http://www.postgresql.org/message-id/9914.1457628521@...


2) There was also some weird slowness in putting it in functions used in
casting, so limited it to just plpgsql and STRICT sql functions.

3) It however would still be needed on some C functions such as
ST_Transform.  That one I hard-coded.  Reason for that is any C function
that makes calls to spatial_ref_sys  or any other postgis function
Either needs a search_path or schema qualify
And that is later used e.g. in a materialized view etc, would suffer the
same load issues.  Since we can't have our C code be changed at runtime, the
only way to fix it is to put it in the

CREATE FUNCTION  ... ALTER search_path

I think we may have added some other transform like thingys we may need to
do this for.

--- NOW for the more invasive plan, which Paul already gave his frowny face
about.

WE NEED TO AGREE ON A SCHEMA THAT postgis lives in and first encourage and
then force everyone to install there.  
This is admittedly a big breaking change so should wait for PostGIS 2.4 (or
see if PostgreSQL upstream comes up with a better solution).
I have complained to them extensively about this issue (which I'll get to in
a minute).

Here is why I feel we need to move in that direction.

We have some extensions that rely on PostGIS already -- postgis_topology,
pgRouting, postgis_tiger_geocoder, postgis_sfcgal.

If anybody uses any of the functions in these extensions in a materialized
view or table index or constraint that happens to call a PostGIS function
they are screwed.
They are screwed because we are preventing these extensions from schema
qualifying their PostGIS calls and they can't schema qualify them if there
is no agreed location where PostGIS is installed.


I'm hoping PostgreSQL dev group  will do something about this
http://www.postgresql.org/message-id/56E36445.3050303@...  so we
don't have to.

because it's not just us, but given our extension size, we are probably most
affected and I fear as use of materialized views, foreign tables, more
postgis related extensions etc grows, this is going to be more of a problem

We have various tickets about this - raster restore (which we can consider
already fixed), materialized views, foreign tables (Haven't figured how this
happens I suspect it affects foreign tables if the foreign table is a link
to a view that has PostGIS functions,
And it has a function that calls another postgis function - foreign table
schema path is isolated evidentally so will not use the default search_path)


Thanks,
Regina







_______________________________________________
postgis-devel mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/postgis-devel
Reply | Threaded
Open this post in threaded view
|

Re: Making PostGIS not schema relocateable to fix materialized view, restore issues, foreign table issues

Stephen Frost
Regina,

* Regina Obe ([hidden email]) wrote:
> In order to schema qualify postgis calls in our functions, we need to make
> PostGIS non-relocateable.

Making PostGIS non-relocatable (that is, you can't change the schema it
was installed into after it's installed) doesn't bother me too much,
personally.

> 3) It however would still be needed on some C functions such as
> ST_Transform.  That one I hard-coded.  Reason for that is any C function
> that makes calls to spatial_ref_sys  or any other postgis function
> Either needs a search_path or schema qualify
> And that is later used e.g. in a materialized view etc, would suffer the
> same load issues.  Since we can't have our C code be changed at runtime, the
> only way to fix it is to put it in the
>
> CREATE FUNCTION  ... ALTER search_path
>
> I think we may have added some other transform like thingys we may need to
> do this for.
That's an interesting issue, certainly..  That said, if it's C code,
couldn't you simply look up the schema that the extension is installed
into and build that into the query you're sending through SPI (apologies
if I'm not fully understanding the issue here).

> --- NOW for the more invasive plan, which Paul already gave his frowny face
> about.

Yeah, I tend to share that frowny face, to be honest. :)

> WE NEED TO AGREE ON A SCHEMA THAT postgis lives in and first encourage and
> then force everyone to install there.  
> This is admittedly a big breaking change so should wait for PostGIS 2.4 (or
> see if PostgreSQL upstream comes up with a better solution).
> I have complained to them extensively about this issue (which I'll get to in
> a minute).
>
> Here is why I feel we need to move in that direction.
>
> We have some extensions that rely on PostGIS already -- postgis_topology,
> pgRouting, postgis_tiger_geocoder, postgis_sfcgal.
>
> If anybody uses any of the functions in these extensions in a materialized
> view or table index or constraint that happens to call a PostGIS function
> they are screwed.
> They are screwed because we are preventing these extensions from schema
> qualifying their PostGIS calls and they can't schema qualify them if there
> is no agreed location where PostGIS is installed.
Ok, I understand that the issue here is that there's no way inside of
the script run at CREATE EXTENSION time to refer to the schema of
*another* extension.  You can refer to the extension that your extension
is being installed into using '@extschema@', but that doesn't help you
if you depend on objects from another extension.

That's a pretty rough lack-of-capability on the PG side.  Looking at the
code which handles '@extschema@', it might not be too hard to add in
something along the lines of:

@extschema{'postgis'}@

and have that be replaced with the location of the postgis extension.
My thinking here for how the code would work is that we'd simple run
that 'replace_text' function for all extensions which are in the
'requires' list of the control file (substituting the name of each
extension, of course).

Thoughts?

Note that we've only got a week before feature free for 9.6, so we
really need to think about this hard but also quickly if we're going to
get such a change in.  If it sounds like it might work to you then we
should bring it to the -hackers list ASAP.

Thanks!

Stephen

_______________________________________________
postgis-devel mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/postgis-devel

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Making PostGIS not schema relocateable to fix materialized view, restore issues, foreign table issues

Stephen Woodbridge
On 4/1/2016 9:11 AM, Stephen Frost wrote:

> Regina,
>
> * Regina Obe ([hidden email]) wrote:
>> In order to schema qualify postgis calls in our functions, we need to make
>> PostGIS non-relocateable.
>
> Making PostGIS non-relocatable (that is, you can't change the schema it
> was installed into after it's installed) doesn't bother me too much,
> personally.
>
>> 3) It however would still be needed on some C functions such as
>> ST_Transform.  That one I hard-coded.  Reason for that is any C function
>> that makes calls to spatial_ref_sys  or any other postgis function
>> Either needs a search_path or schema qualify
>> And that is later used e.g. in a materialized view etc, would suffer the
>> same load issues.  Since we can't have our C code be changed at runtime, the
>> only way to fix it is to put it in the
>>
>> CREATE FUNCTION  ... ALTER search_path
>>
>> I think we may have added some other transform like thingys we may need to
>> do this for.
>
> That's an interesting issue, certainly..  That said, if it's C code,
> couldn't you simply look up the schema that the extension is installed
> into and build that into the query you're sending through SPI (apologies
> if I'm not fully understanding the issue here).
>
>> --- NOW for the more invasive plan, which Paul already gave his frowny face
>> about.
>
> Yeah, I tend to share that frowny face, to be honest. :)
>
>> WE NEED TO AGREE ON A SCHEMA THAT postgis lives in and first encourage and
>> then force everyone to install there.
>> This is admittedly a big breaking change so should wait for PostGIS 2.4 (or
>> see if PostgreSQL upstream comes up with a better solution).
>> I have complained to them extensively about this issue (which I'll get to in
>> a minute).
>>
>> Here is why I feel we need to move in that direction.
>>
>> We have some extensions that rely on PostGIS already -- postgis_topology,
>> pgRouting, postgis_tiger_geocoder, postgis_sfcgal.
>>
>> If anybody uses any of the functions in these extensions in a materialized
>> view or table index or constraint that happens to call a PostGIS function
>> they are screwed.
>> They are screwed because we are preventing these extensions from schema
>> qualifying their PostGIS calls and they can't schema qualify them if there
>> is no agreed location where PostGIS is installed.
>
> Ok, I understand that the issue here is that there's no way inside of
> the script run at CREATE EXTENSION time to refer to the schema of
> *another* extension.  You can refer to the extension that your extension
> is being installed into using '@extschema@', but that doesn't help you
> if you depend on objects from another extension.
>
> That's a pretty rough lack-of-capability on the PG side.  Looking at the
> code which handles '@extschema@', it might not be too hard to add in
> something along the lines of:
>
> @extschema{'postgis'}@
>
> and have that be replaced with the location of the postgis extension.
> My thinking here for how the code would work is that we'd simple run
> that 'replace_text' function for all extensions which are in the
> 'requires' list of the control file (substituting the name of each
> extension, of course).
>
> Thoughts?

What happens if the user does something like:

alter extension set schema to blah;

on a dependent schema? Can you even do that?

-Steve

> Note that we've only got a week before feature free for 9.6, so we
> really need to think about this hard but also quickly if we're going to
> get such a change in.  If it sounds like it might work to you then we
> should bring it to the -hackers list ASAP.
>
> Thanks!
>
> Stephen
>
>
>
> _______________________________________________
> postgis-devel mailing list
> [hidden email]
> http://lists.osgeo.org/mailman/listinfo/postgis-devel
>


---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

_______________________________________________
postgis-devel mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/postgis-devel
Reply | Threaded
Open this post in threaded view
|

Re: Making PostGIS not schema relocateable to fix materialized view, restore issues, foreign table issues

Stephen Frost
* Stephen Woodbridge ([hidden email]) wrote:
> On 4/1/2016 9:11 AM, Stephen Frost wrote:
> >Thoughts?
>
> What happens if the user does something like:
>
> alter extension set schema to blah;
>
> on a dependent schema? Can you even do that?

The first part of Regina's proposal, which I agreed with, was to make
PostGIS not relocatable, which means that

alter extension postgis set schema xxx;

would return an error saying that the postgis extension is not
relocatable.

Thanks!

Stephen

_______________________________________________
postgis-devel mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/postgis-devel

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Making PostGIS not schema relocateable to fix materialized view, restore issues, foreign table issues

Regina Obe
In reply to this post by Stephen Frost

>> 3) It however would still be needed on some C functions such as
>> ST_Transform.  That one I hard-coded.  Reason for that is any C
>> function that makes calls to spatial_ref_sys  or any other postgis
>> function Either needs a search_path or schema qualify And that is
>> later used e.g. in a materialized view etc, would suffer the same load
>> issues.  Since we can't have our C code be changed at runtime, the
>> only way to fix it is to put it in the
>>
>> CREATE FUNCTION  ... ALTER search_path
>>
>> I think we may have added some other transform like thingys we may
>> need to do this for.

>> That's an interesting issue, certainly..  That said, if it's C code,
couldn't you simply look up the schema that the extension is installed into
and build that into the query you're sending through SPI (apologies if I'm
not fully understanding the issue here).
That would work.  Only  issue with that is not everyone installs with
extensions, so it might not show up.  I guess for those cases we just assume
it's in search path as we do now and screw you if it isn't and you aren't
using postgis extension approach :)
 

>> --- NOW for the more invasive plan, which Paul already gave his frowny
>> face about.

> Yeah, I tend to share that frowny face, to be honest. :)
Okay I admit I frown too so I'm pushing that way down the priority as it may
cause more problems than it solves.
I think making postgis not relocateable and us schema qualifying our calls
would deal with 95% of the issue.  The other 5% I fear will grow as more and
more extensions rely
On us.
IN case of postgis_sfcgal we can deal with that by schema qualifying all
those and since that is part of postgis, people just need to understand they
have to install both in the same schema.
Postgis_topology luckily doesn't have any functions YET that are used for
index building etc that use PostGIS where restore would become an issue.
Still it could happen and it's still possible
For some crazy person to use postgis_topology in a materialized view.
That's more of a theoretical problem at moment than a real problem.

> Ok, I understand that the issue here is that there's no way inside of the
script run at CREATE EXTENSION time to refer to the schema of
> *another* extension.  You can refer to the extension that your extension
is being installed into using '@extschema@', but that doesn't help you if
you depend on objects from another extension.

> That's a pretty rough lack-of-capability on the PG side.  Looking at the
code which handles '@extschema@', it might not be too hard to add in
something along the lines of:

 It's a mess in postgis_tiger_geocoder because of reliance of fuzzystrmatch.
As you said this is really a PostgreSQL issue
That PostgreSQL group is going to have to deal with sooner or later as more
extensions rely on each other.  So I'd rather us not have to deal with it.


> @extschema{'postgis'}@

> and have that be replaced with the location of the postgis extension.
> My thinking here for how the code would work is that we'd simple run that
'replace_text' function for all extensions which are in the 'requires' list
of the control file (substituting the name of each extension, of course).

> Thoughts?

> Note that we've only got a week before feature free for 9.6, so we really
need to think about this hard but also quickly if we're going to get such a
change in.  If it sounds like it might work to you then we should bring it
to the -hackers list ASAP.

> Thanks!

> Stephen

On surface sounds fine to me.  I wasn't expecting a fix in 9.6 for this
though would be nice if there was one.  Given we have to back support a
couple of versions of PostgreSQL (and so does pgRouting where I fear this
will become more of a problem)
We wouldn't be able to use this for a while anyway.

Also since most of these issues are theoretical issues at this time, I'm not
too worried about waiting.  But like I said as more extensions are added to
PostgreSQL and more rely on each other,
this will become more of a serious issue that PostgreSQL dev is going to be
forced to deal with. So you should get prepared.  This is just a
foreshadowing warning.

Thanks,
Regina




_______________________________________________
postgis-devel mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/postgis-devel