Re: IMPORT FOREIGN SCHEMA statement

On Sun, May 25, 2014 at 11:23:41PM +0200, Ronan Dunklau wrote:
> Le dimanche 25 mai 2014 12:41:18 David Fetter a écrit :
> > On Fri, May 23, 2014 at 10:08:06PM +0200, Ronan Dunklau wrote:
> > > Hello,
> > > 
> > > Since my last proposal didn't get any strong rebuttal, please find
> > > attached a more complete version of the IMPORT FOREIGN SCHEMA statement.
> > 
> > Thanks!
> > 
> > Please to send future patches to this thread so people can track them
> > in their mail.
> 
> I'll do.
> 
> I didn't for the previous one because it was a few months ago, and no patch 
> had been added to the commit fest.

Thanks for adding this one :)

> > > The import_foreign_schema_postgres_fdw patch proposes an implementation of
> > > this API for postgres_fdw. It will import a foreign schema using the right
> > > types as well as nullable information.
> > 
> > In the case of PostgreSQL, "the right types" are obvious until there's
> > a user-defined one.  What do you plan to do in that case ?
> 
> The current implementation fetches the types as regtype, and when receiving a 
> custom type, two things can happen:
> 
>  - the type is defined locally: everything will work as expected
>  - the type is not defined locally: the conversion function will fail, and 
> raise an error of the form: ERROR:  type "schema.typname" does not exist
> 
> Should I add that to the regression test suite ?

Yes.

In the "easy" case of PostgreSQL, you might also be able to establish
whether the UDT in the "already defined locally" case above is
identical to the one defined remotely, but I don't think it's possible
even in principle for non-PostgreSQL remote systems, possibly not even
for ones with non-identical architecture, PostgreSQL major version,
etc.

> > > Regarding documentation, I don't really know where it should have been
> > > put. If I missed something, let me know and I'll try to correct it.
> > 
> > It's not exactly something you missed, but I need to bring it up
> > anyway before we go too far.  The standard missed two crucial concepts
> > when this part of it was written:
> > 
> > 1.  No single per-database-type universal type mapping can be correct.
> > 
> > [snip]
> > 
> > To address these problems, I propose the following:
> > 
> > - We make type mappings settable at the level of:
> >     - FDW
> >     - Instance (a.k.a. cluster)
> >     - Database
> >     - Schema
> >     - Table
> >     - Column
> > 
> >     using the existing ALTER command and some way of spelling out how

Oops.  Forgot to include CREATE in the above.

> >     a remote type maps to a local type.
> > 
> >     This would consist of:
> >     - The remote type
> >     - The local type to which it maps
> >     - The inbound transformation (default identity)
> >     - The outbound transformation (default identity)
> > 
> >     At any given level, the remote type would need to be unique.  To
> >     communicate this to the system, we either invent new syntax, with
> >     all the hazards attendant thereto, or we could use JSON or similar
> >     serialization.
> > 
> >     ALTER FOREIGN TABLE foo
> >     ADD TYPE MAPPING
> >         FROM "datetime"
> >         TO TEXT
> >         WITH (
> >             INBOUND TRANSFORMATION IDENTITY,
> >             OUTBOUND TRANSFORMATION IDENTITY
> >         ) /* Ugh!!! */

"Ugh!!!" means I don't think we should do it this way.

> >     vs.
> > 
> >     ALTER FOREIGN TABLE foo ADD (mapping '{
> >         "datetime": "text",
> >         "inbound": "IDENTITY",
> >         outbound: "IDENTITY"
> >     }')
> > 
> > Each FDW would have some set of default mappings and some way to
> > override them as above.
> 
> I understand your points, but I'm not really comfortable with the
> concept, unless there is something that I missed.

My poor communication ability might have a lot to do with it.  I
assure you my explanation would have been even worse if I had tried it
in French, though. :P

> We can already support this use case through specific-fdw options. Should I 
> add that to postgres_fdw ?

I believe the capability belongs in our FDW API with the decision of
whether to implement it up to FDW authors.  They know (or should know)
how to throw ERRCODE_FEATURE_NOT_SUPPORTED.

> Additionally, I don't really see how that would be useful in a general case. 
> With an "in-core" defined meaning of type transformation, any FDW that doesn't 
> fit exactly into the model would have a hard time. For example, what happens 
> if an FDW is only ever capable of returning text ?

That's actually the case where it's most important to have the feature
all the way down to the column level.

> Or if a mapping can only be set at the server or FDW model because
> it depends on some connection parameter ?

ERRCODE_FEATURE_NOT_SUPPORTED.

> The bulk of the code for managing type mappings would be
> FDW-specific anyway.

The part that actually does the transformations would necessarily be
part of each FDW.  I omitted opining on whether such transformations
should be assumed to be local or remote because I can imagine cases
where only local (or only remote) could be correct.

> What you're proposing looks like a "universal option", with a
> specific syntax, that should therefore be supported by all fdws,
> with well-defined semantics.

At the DDL level, yes.

> Moreover, extending the spec seems a bit dangerous to me, since if the spec 
> decides to address this specific point in the future, there is a risk that our 
> behavior will not be compatible.

That's why I suggested doing it via CREATE/ALTER with JSONB or similar
containing the details rather than inventing new SQL grammar, an
option I included only to dismiss it.

Cheers,
David.
-- 
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter      XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate