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(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)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