Prevent extension creation in temporary schemas

On Sun, Jan 6, 2019 at 10:26 PM Michael Paquier <michael@paquier.xyz> wrote:

This combination makes no actual sense, so wouldn't it be better to
restrict the case?

Hmm. What exactly doesn't make sense about it?

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

On Fri, Jan 11, 2019 at 02:22:01PM -0500, Robert Haas wrote:

On Sun, Jan 6, 2019 at 10:26 PM Michael Paquier <michael@paquier.xyz> wrote:

This combination makes no actual sense, so wouldn't it be better to
restrict the case?

Hmm. What exactly doesn't make sense about it?

In my mind, extensions are designed to be database-wide objects which
are visible to all sessions. Perhaps I am just confused by what I
think they should be, and I can see no trace on the archives about
concept of extensions + temp schema as base (adding Dimitri in CC if
he has an idea). I can see as well that there have been stuff about
using temporary objects in extension script though ("Fix bugs with
temporary or transient tables used in extension scripts" in release
notes of 9.1).

For most of extensions, this can randomly finish with strange error
messages, say that:
=# create extension file_fdw with schema pg_temp_3;
ERROR: 42883: function file_fdw_handler() does not exist
LOCATION: LookupFuncName, parse_func.c:2088

There are cases where the extension can be created:
=# create extension pgcrypto with schema pg_temp_3;
CREATE EXTENSION
Time: 36.567 ms
=# \dx pgcrypto
List of installed extensions
Name | Version | Schema | Description
----------+---------+-----------+-------------------------
pgcrypto | 1.3 | pg_temp_3 | cryptographic functions
(1 row)

Then the extension is showing up as beginning to be present for other
users. I am mainly wondering if this case has actually been thought
about in the past or discussed, and what to do about that and if we
need to do something. Temporary extensions can exist as long as the
extension script does not include for example REVOKE queries on the
functions it creates (which should actually work?), and there is a
separate thread about restraining 2PC when touching the temporary
namespace for the creation of many objects, and extensions are one
case discussed. Still the concept looks a bit wider, so I spawned a
separate thread.
--
Michael

On Sat, Jan 12, 2019 at 08:34:37AM +0900, Michael Paquier wrote:

Then the extension is showing up as beginning to be present for other
users. I am mainly wondering if this case has actually been thought
about in the past or discussed, and what to do about that and if we
need to do something.

The point here is about the visibility in \dx.
--
Michael

This could probably use a quick note in the docs.

On Sat, Jan 12, 2019 at 12:48 AM Michael Paquier <michael@paquier.xyz>
wrote:

On Sat, Jan 12, 2019 at 08:34:37AM +0900, Michael Paquier wrote:

Then the extension is showing up as beginning to be present for other
users. I am mainly wondering if this case has actually been thought
about in the past or discussed, and what to do about that and if we
need to do something.

The point here is about the visibility in \dx.

If the point is visibility in \dx it seems to me we want to fix the \dx
query.

This is actually a very interesting set of problems and behavior is not
intuitive here in PostgreSQL. I wonder how much more inconsistency we want
to add.

For example: suppose I create a type in pg_temp and create a table in
public with a column using that type.

What is the expected visibility in other sessions?

What happens to the table when I log out?

I went ahead and tested that case and I found the behavior to be, well,
unintuitive. The temporary type is visible to other sessions and the
column is implicitly dropped when the type falls out of session scope.
Whether or not we want to prevent that, I think that having special casing
here for extensions makes this behavior even more inconsistent. I guess I
would vote against accepting this patch as it is.

--
Michael

--
Best Regards,
Chris Travers
Head of Database

Tel: +49 162 9037 210 | Skype: einhverfr | www.adjust.com
Saarbrücker Straße 37a, 10405 Berlin

On Wed, Feb 13, 2019 at 12:08:50PM +0100, Chris Travers wrote:

If the point is visibility in \dx it seems to me we want to fix the \dx
query.

Yes, I got to think a bit more about that case, and there are cases
where this actually works properly as this depends on the objects
defined in the extension. Fixing \dx to not show up extensions
defined in temp schemas of other sessions is definitely a must in my
opinion, and I would rather drop the rest of the proposal for now. A
similar treatment is needed for \dx+.

For example: suppose I create a type in pg_temp and create a table in
public with a column using that type.

I am wondering if this scenario could make sense to populate data on
other, existing, relations for a schema migration, and that a two-step
process is done, with temporary tables used as intermediates. But
that sounds like the thoughts of a crazy man..

What is the expected visibility in other sessions?

What happens to the table when I log out?

Anything depending on a temporary object will be dropped per
dependency links once the session is over.

Attached is a patch to adjust \dx and \dx+. What do you think?
--
Michael

Dear Michael,

I seem this patch is enough, but could you explain the reason
you drop initial proposal more detail?
I'm not sure why extensions contained by temporary schemas are acceptable.

Anything depending on a temporary object will be dropped per
dependency links once the session is over.

Extensions locate at pg_temp_* schemas are temporary objects IMO.
How do you think? Would you implement this functionality in future?

Hayato Kuroda
Fujitsu LIMITED

On Mon, Feb 18, 2019 at 6:40 AM Kuroda, Hayato <kuroda.hayato@jp.fujitsu.com>
wrote:

Dear Michael,

I seem this patch is enough, but could you explain the reason
you drop initial proposal more detail?
I'm not sure why extensions contained by temporary schemas are acceptable.

Here's my objection.

Everything a relocatable extension can create can be created normally in a
temporary schema currently. This includes types, functions, etc.

So I can create a type in a temporary schema and then create a table in a
public schema using that type as a column. This behaves oddly (when I log
out of my session the column gets implicitly dropped) but it works
consistently. Adding special cases to extensions strikes me as adding more
funny corners to the behavior of the db in this regard.

Now there are times I could imagine using temporary schemas with
extensions. This could include testing multiple versions of an extension
so that multiple concurrent test runs don't see each other's versions.
This could be done with normal schemas but the guarantees are not as strong
regarding cleanup.

Anything depending on a temporary object will be dropped per
dependency links once the session is over.

Extensions locate at pg_temp_* schemas are temporary objects IMO.
How do you think? Would you implement this functionality in future?

That's the way things are now as far as I understand it, or do I
misunderstand your question?

Hayato Kuroda
Fujitsu LIMITED

--
Best Regards,
Chris Travers
Head of Database

Tel: +49 162 9037 210 | Skype: einhverfr | www.adjust.com
Saarbrücker Straße 37a, 10405 Berlin

On Thu, Feb 14, 2019 at 4:57 PM Michael Paquier <michael@paquier.xyz> wrote:

On Wed, Feb 13, 2019 at 12:08:50PM +0100, Chris Travers wrote:

If the point is visibility in \dx it seems to me we want to fix the \dx
query.

Yes, I got to think a bit more about that case, and there are cases
where this actually works properly as this depends on the objects
defined in the extension. Fixing \dx to not show up extensions
defined in temp schemas of other sessions is definitely a must in my
opinion, and I would rather drop the rest of the proposal for now. A
similar treatment is needed for \dx+.

I'd vote for accepting the extension creation in temporary schemas and
fixing \dx and \dx+. However the error raised by creating extensions
in temporary schema still looks strange to me. Since we don't search
functions and operators defined in temporary schemas (which is stated
by the doc) unless we use qualified function name we cannot create
extensions in temporary schema whose functions refer theirs other
functions. I'd like to fix it or to find a workaround but cannot come
up with a good idea yet.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

On Mon, Feb 18, 2019 at 05:39:09AM +0000, Kuroda, Hayato wrote:

I seem this patch is enough, but could you explain the reason
you drop initial proposal more detail?
I'm not sure why extensions contained by temporary schemas are
acceptable.

Because there are cases where they actually work. We have some of
these in core.

Anything depending on a temporary object will be dropped per
dependency links once the session is over.

Extensions locate at pg_temp_* schemas are temporary objects IMO.
How do you think? Would you implement this functionality in future?

Per the game of dependencies, extensions located in a temporary schema
would get automatically dropped at session end.
--
Michael

On Mon, Feb 18, 2019 at 08:02:54PM +0900, Masahiko Sawada wrote:

I'd vote for accepting the extension creation in temporary schemas and
fixing \dx and \dx+.

Thanks.

However the error raised by creating extensions
in temporary schema still looks strange to me. Since we don't search
functions and operators defined in temporary schemas (which is stated
by the doc) unless we use qualified function name we cannot create
extensions in temporary schema whose functions refer theirs other
functions. I'd like to fix it or to find a workaround but cannot come
up with a good idea yet.

Agreed. Getting a schema mismatch is kind of disappointing, and it
depends on the DDL used in the extension SQL script. I would suspect
that getting that addressed correctly may add quite some facility, for
little gain. But I may be wrong, that's only the feeling coming from
a shiver in my back.
--
Michael

Michael Paquier <michael@paquier.xyz> writes:

On Mon, Feb 18, 2019 at 05:39:09AM +0000, Kuroda, Hayato wrote:

I'm not sure why extensions contained by temporary schemas are
acceptable.

Because there are cases where they actually work.

More to the point, it doesn't seem that hard to think of cases
where this would be useful. PG extensions are very general
things. If you want to create a whole pile of temporary objects
and do that repeatedly, wrapping them up into an extension is
a nice way to do that, nicer really than anything else we offer.
So I'd be sad if we decided to forbid this.

Per the game of dependencies, extensions located in a temporary schema
would get automatically dropped at session end.

Yeah, it doesn't seem like there's actually any missing functionality
there, at least not any that's specific to extensions.

regards, tom lane

Dear Michael, Chris and Tom,

Adding special cases to extensions strikes me as adding more
funny corners to the behavior of the db in this regard.

I understand your arguments and its utility.

For most of extensions, this can randomly finish with strange error
messages, say that:
=# create extension file_fdw with schema pg_temp_3;
ERROR: 42883: function file_fdw_handler() does not exist
LOCATION: LookupFuncName, parse_func.c:2088

I found that this strange error appears after making
temporary tables.

test=> CREATE TEMPORARY TABLE temp (id int);
CREATE TABLE
test=> CREATE EXTENSION file_fdw WITH SCHEMA pg_temp_3;
ERROR: function file_fdw_handler() does not exist

I would try to understand this problem for community and
my experience.

Best Regards,
Hayato Kuroda
Fujitsu LIMITED

Hi

I found that this strange error appears after making
temporary tables.

test=> CREATE TEMPORARY TABLE temp (id int);
CREATE TABLE
test=> CREATE EXTENSION file_fdw WITH SCHEMA pg_temp_3;
ERROR: function file_fdw_handler() does not exist

I would try to understand this problem for community and
my experience.

This behavior seems as not related to extensions infrastructure:

postgres=# CREATE TEMPORARY TABLE temp (id int);
CREATE TABLE
postgres=# set search_path to 'pg_temp_3';
SET
postgres=# create function foo() returns int as 'select 1' language sql;
CREATE FUNCTION
postgres=# select pg_temp_3.foo();
foo
-----
1
(1 row)

postgres=# select foo();
ERROR: function foo() does not exist
LINE 1: select foo();
^
HINT: No function matches the given name and argument types. You might need to add explicit type casts.
postgres=# show search_path ;
search_path
-------------
pg_temp_3
(1 row)

regards, Sergei

Sergei Kornilov <sk@zsrv.org> writes:

test=> CREATE EXTENSION file_fdw WITH SCHEMA pg_temp_3;
ERROR: function file_fdw_handler() does not exist

This behavior seems as not related to extensions infrastructure:

Yeah, I think it's just because we won't search the pg_temp schema
for function or operator names, unless the calling SQL command
explicitly writes "pg_temp.foo(...)" or equivalent. That's an
ancient security decision, which we're unlikely to undo. It
certainly puts a crimp in the usefulness of putting extensions into
pg_temp, but I don't think it totally destroys the usefulness.
You could still use an extension to package, say, the definitions
of a bunch of temp tables and views that you need to create often.

regards, tom lane

On Thu, Feb 28, 2019 at 10:13:17AM -0500, Tom Lane wrote:

Yeah, I think it's just because we won't search the pg_temp schema
for function or operator names, unless the calling SQL command
explicitly writes "pg_temp.foo(...)" or equivalent. That's an
ancient security decision, which we're unlikely to undo. It
certainly puts a crimp in the usefulness of putting extensions into
pg_temp, but I don't think it totally destroys the usefulness.
You could still use an extension to package, say, the definitions
of a bunch of temp tables and views that you need to create often.

Even with that, it should still be possible to enforce search_path
within the extension script to allow such objects to be created
correctly, no? That would be a bit hacky, still for the purpose of
temp object handling that looks kind of enough to live with when
creating an extension.
--
Michael

Michael Paquier <michael@paquier.xyz> writes:

On Thu, Feb 28, 2019 at 10:13:17AM -0500, Tom Lane wrote:

Yeah, I think it's just because we won't search the pg_temp schema
for function or operator names, unless the calling SQL command
explicitly writes "pg_temp.foo(...)" or equivalent. That's an
ancient security decision, which we're unlikely to undo. It
certainly puts a crimp in the usefulness of putting extensions into
pg_temp, but I don't think it totally destroys the usefulness.
You could still use an extension to package, say, the definitions
of a bunch of temp tables and views that you need to create often.

Even with that, it should still be possible to enforce search_path
within the extension script to allow such objects to be created
correctly, no? That would be a bit hacky, still for the purpose of
temp object handling that looks kind of enough to live with when
creating an extension.

If you're suggesting that we disable that security restriction
during extension creation, I really can't see how that'd be a
good thing ...

regards, tom lane

On Thu, Feb 28, 2019 at 10:52:52PM -0500, Tom Lane wrote:

If you're suggesting that we disable that security restriction
during extension creation, I really can't see how that'd be a
good thing ...

No, I don't mean that. I was just wondering if someone can set
search_path within the SQL script which includes the extension
contents to bypass the restriction and the error. They can always
prefix such objects with pg_temp anyway if need be...
--
Michael

Michael Paquier <michael@paquier.xyz> writes:

On Thu, Feb 28, 2019 at 10:52:52PM -0500, Tom Lane wrote:

If you're suggesting that we disable that security restriction
during extension creation, I really can't see how that'd be a
good thing ...

No, I don't mean that. I was just wondering if someone can set
search_path within the SQL script which includes the extension
contents to bypass the restriction and the error. They can always
prefix such objects with pg_temp anyway if need be...

You'd have to look in namespace.c to be sure, but I *think* that
we don't consult the temp schema during function/operator lookup
even if it's explicitly listed in search_path.

It might be possible for an extension script to get around this with
code like, say,

CREATE TRIGGER ... EXECUTE PROCEDURE @extschema@.myfunc();

although you'd have to give up relocatability of the extension
to use @extschema@. (Maybe it was a bad idea to not provide
that symbol in relocatable extensions? A usage like this doesn't
prevent the extension from being relocated later.)

regards, tom lane