Re: IMPORT FOREIGN SCHEMA statement

Le mardi 1 juillet 2014 21:09:55 Michael Paquier a écrit :
> On Tue, Jul 1, 2014 at 4:23 PM, Ronan Dunklau <ronan.dunklau@dalibo.com>
>
> wrote:
> > The remote_schema parameter can be used for SQL injection. Either we
> > should go
> > back to using parameters, or be extra careful. Since the remote schema is
> > parsed as a name, it is limited to 64 characters which is not that useful
> > for
> > an SQL injection, but still.
>
> Yes, right, I completely forgot that. IMPORT SCHEMA could be used by a
> non-superuser so controlling only the size of relation name is not enough.
>

I reintroduced PQExecParams, trying to keep it simple enough.

> The new query as you wrote it returns the typname (was cast to regtype
>
> > before)
> > This is not schema qualified, and will fail when importing tables with
> > columns
> > from a type not in search_path.
>
> Hm. The current solution with simply LookupTypeNameOid is more elegant than
> relying on InputFunctionCall to fetch a Datum, that is then converted back
> into TypeName... In all cases I am wondering about the use of regtype where
> the same type name is used across multiple schemas. We should perhaps
> document correctly that search_path influences the custom type chosen when
> rebuilding foreign tables and that postgres_fdw does its best but that it
> may not be compatible with type on foreign server.

I think that it would be better to qualify the type name. The attached patch
does that by fetching the type schema name in another column, and using
LookupTypeNameOid.

>
> > The regression tests don't pass: a user name is hard-coded in the result
> > of
> > DROP USER MAPPING. Should we expect the tests to be run as postgres?
>
> I think that we need a cleaner solution for this test case, I've let it for
> the time being but a make installcheck would let an extra database as it
> cannot be removed in postgres_fdw.sql (an extra test case just to clean up
> would work but I don't think that it is worth the complication). We could
> abuse of search_path not tracking types located on certain schemas to
> trigger this error :)
>
> Want to give a shot on the following things? I wouldn't mind changing back
> the query construction part :)

Also attached in this patch:
 - more robust way for cleaning things up in regression tests
 - small documentation change for options in the fdwhandler API



> --
> Michael

--
Ronan Dunklau
http://dalibo.com - http://dalibo.org