Re: [PATCH] postgres_fdw extension support - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: [PATCH] postgres_fdw extension support
Date
Msg-id CAB7nPqT2hjOu0Fnh6376sNaD_B77YcbrBTf6AC_tTRA9GoTDeA@mail.gmail.com
Whole thread Raw
In response to Re: [PATCH] postgres_fdw extension support  (Paul Ramsey <pramsey@cleverelephant.ca>)
Responses Re: [PATCH] postgres_fdw extension support  (Paul Ramsey <pramsey@cleverelephant.ca>)
List pgsql-hackers
On Sat, Sep 26, 2015 at 4:29 AM, Paul Ramsey wrote:
> On Thu, Aug 20, 2015 at 6:01 PM, Michael Paquier wrote:
> src/backend/utils/adt/format_type.c
> +/*
> + * This version allows a nondefault typemod to be specified and fully
> qualified.
> + */
> +char *
> +format_type_with_typemod_qualified(Oid type_oid, int32 typemod)
> +{
> +       return format_type_internal(type_oid, typemod, true, false, true);
> +}

Patch 0001 in this email has a routine called format_type_detailed
that I think is basically what we need for this stuff:
http://www.postgresql.org/message-id/CACACo5Q_UXYwF117LBhjZ3xaMPyrgqnqE=mXvRhEfjJ51aCfwQ@mail.gmail.com
Could you look at it?

> I've put a very very small regression file in that tests the shippable
> feature, which can be fleshed out further as needed.
>
> Thanks so much!

Nice. Thanks for the addition of the regression tests. It begins to
have a nice shape.

+ * shippable.c
+ *       Non-built-in objects cache management and utilities.
+ *
+ * Is a non-built-in shippable to the remote server? Only if
+ * the object is in an extension declared by the user in the
+ * OPTIONS of the wrapper or the server.
This is rather unclear. It would be more simple to describe that as
"Facility to track database objects shippable to a foreign server".

+extern bool extractExtensionList(char *extensionString,
+                                       List **extensionOids);
What's the point of the boolean status in this new routine? The return
value of extractExtensionList is never checked, and it would be more
simple to just return the parsed list as return value, no?

-REGRESS = postgres_fdw
+REGRESS = postgres_fdw shippable
+EXTRA_INSTALL = contrib/seg
The order of the tests is important and should be mentioned,
shippable.sql using the loopback server created by postgres_fdw.

+-- ===================================================================
+-- clean up
+-- ===================================================================
Perhaps here you meant dropping the schema and the foreign tables
created previously?

+CREATE SCHEMA "SH 1";
Is there a reason to use double-quoted relations? There is no real
reason to not use them, this is just to point out that this is not a
common practice in the other regression tests.
-- 
Michael



pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: Parallel Seq Scan
Next
From: Michael Paquier
Date:
Subject: Re: Improving test coverage of extensions with pg_dump