Move postgresql_fdw_validator into dblink

I'd like to propose moving postgresql_fdw_validator into contrib/dblink
as dblink's own validator.

Main purpose of this proposal is to reserve the name "postgresql_fdw"
for concrete FDW for PostgreSQL. I used to use "pgsql_fdw" as the name,
but in previous CF I got comments that full product name is appropriate
rather than abbreviation (e.g. pgsql_fdw) for FDW's name.

In addition, this change would avoid potential problem that third-party
product might use this validator and introduce undesirable dependency
between PG core.

This change breaks backward compatibility, but AFAIK no one except
dblink seems to use this validator, so it would not be serious problem.
# Please let me know if any product uses this validator!

Changes in this patch are:

1) Move postgresql_fdw_validator from core backend to contrib/dblink
with renaming to dblink_fdw_validator. Also I modified this validator
so that it uses PQconndefault() to get libpq's valid options instead of
having its own options list.

2) For ease of use, dblink's CREATE EXTENSION provides default FDW
dblink_fdw which accepts libpq's connection options via user mapping
("user" and secret options such as "password") and foreign server (all
other options).

3) Bump dblink's version to 1.1. Of cource upgrade script is provided.

4) Update documents. (Should we mention removal of
postgresql_fdw_validator?)

5) Use simplified postgresql_fdw_validator in regression test
foreign_data. I didn't change actual test cases because they don't seem
to depend on postgresql_fdw_validator deeply.

Comments and questions are welcome.
--
Shigeru HANADA

Hanada-san,

What about your plan to upstream contrib/pgsql_fdw module on the upcoming
commit-fest?
Even though I understand the point I noticed (miss-synchronization of sub-
transaction block between local and remote side) is never easy problem to
solve, it is worth to get the patch on the table of discussion.
In my opinion, it is an idea to split-off the transaction control portion as
a limitation of current version. For example, just raise an error when
the foreign-table being referenced in sub-transaction block; with explicit
description in the document.

Anyway, let me pick up your patch for reviewing. And, I hope you to prepare
contrib/pgsql_fdw patch based on this patch.

2012/9/11 Shigeru HANADA <shigeru(dot)hanada(at)gmail(dot)com>:
> I'd like to propose moving postgresql_fdw_validator into contrib/dblink
> as dblink's own validator.
>
> Main purpose of this proposal is to reserve the name "postgresql_fdw"
> for concrete FDW for PostgreSQL. I used to use "pgsql_fdw" as the name,
> but in previous CF I got comments that full product name is appropriate
> rather than abbreviation (e.g. pgsql_fdw) for FDW's name.
>
> In addition, this change would avoid potential problem that third-party
> product might use this validator and introduce undesirable dependency
> between PG core.
>
> This change breaks backward compatibility, but AFAIK no one except
> dblink seems to use this validator, so it would not be serious problem.
> # Please let me know if any product uses this validator!
>
> Changes in this patch are:
>
> 1) Move postgresql_fdw_validator from core backend to contrib/dblink
> with renaming to dblink_fdw_validator. Also I modified this validator
> so that it uses PQconndefault() to get libpq's valid options instead of
> having its own options list.
>
> 2) For ease of use, dblink's CREATE EXTENSION provides default FDW
> dblink_fdw which accepts libpq's connection options via user mapping
> ("user" and secret options such as "password") and foreign server (all
> other options).
>
> 3) Bump dblink's version to 1.1. Of cource upgrade script is provided.
>
> 4) Update documents. (Should we mention removal of
> postgresql_fdw_validator?)
>
> 5) Use simplified postgresql_fdw_validator in regression test
> foreign_data. I didn't change actual test cases because they don't seem
> to depend on postgresql_fdw_validator deeply.
>
> Comments and questions are welcome.
> --
> Shigeru HANADA
>
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

Kaigai-san,

(2012/09/13 16:56), Kohei KaiGai wrote:
> What about your plan to upstream contrib/pgsql_fdw module on the upcoming
> commit-fest?

I will post pgsql_fdw patch (though it will be renamed to
postgresql_fdw) in opening CF (2012 Sep), as soon as I resolve a bug in
ANALYZE support, maybe on tomorrow. :-)

> Even though I understand the point I noticed (miss-synchronization of sub-
> transaction block between local and remote side) is never easy problem to
> solve, it is worth to get the patch on the table of discussion.
> In my opinion, it is an idea to split-off the transaction control portion as
> a limitation of current version. For example, just raise an error when
> the foreign-table being referenced in sub-transaction block; with explicit
> description in the document.

I agree not to support synchronize TX block between remote and local, at
least in next CF (I mean keeping remote TX open until local COMMIT or
ABORT). It would require 2PC and many issues to be solved, so I'd like
to focus fundamental part first. OTOH, using foreign tables in
sub-transaction seems essential to me.

> Anyway, let me pick up your patch for reviewing. And, I hope you to prepare
> contrib/pgsql_fdw patch based on this patch.

Thanks for your volunteer :-)

Regards,
--
Shigeru HANADA

Hanada-san,

I checked your patch. It can be applied to the latest master branch
without any conflicts, and regression tests were fine.

Unlike the original postgresql_fdw_validator(), the new
dblink_fdw_validator() has wise idea; that pulls list of connection
options from libpq, instead of self-defined static table.
I basically agree not to have multiple source of option list.

+ /*
+ * Get list of valid libpq options. It contains default values too, but we
+ * don't care the values. Obtained list is allocated with malloc, so we
+ * must free it before leaving this function.
+ */
+ options = PQconndefaults();
+ if (options == NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_FDW_OUT_OF_MEMORY),
+ errmsg("out of memory"),
+ errdetail("could not get libpq default connection options")));

I doubt whether PQconndefaults needs to be invoked for each
validator calls. At least, supported option list of libpq should be
never changed during run-time. So, I think PQconndefaults()
should be called only once at first invocation, then option list
can be stored in static variables or somewhere.
As source code comments says, it is allocated with malloc, thus,
we don't worry about unintentional release. :-)

I don't think other part of this patch is arguable.

Thanks,

2012/9/13 Shigeru HANADA <shigeru(dot)hanada(at)gmail(dot)com>:
> Kaigai-san,
>
> (2012/09/13 16:56), Kohei KaiGai wrote:
>> What about your plan to upstream contrib/pgsql_fdw module on the upcoming
>> commit-fest?
>
> I will post pgsql_fdw patch (though it will be renamed to
> postgresql_fdw) in opening CF (2012 Sep), as soon as I resolve a bug in
> ANALYZE support, maybe on tomorrow. :-)
>
>> Even though I understand the point I noticed (miss-synchronization of sub-
>> transaction block between local and remote side) is never easy problem to
>> solve, it is worth to get the patch on the table of discussion.
>> In my opinion, it is an idea to split-off the transaction control portion as
>> a limitation of current version. For example, just raise an error when
>> the foreign-table being referenced in sub-transaction block; with explicit
>> description in the document.
>
> I agree not to support synchronize TX block between remote and local, at
> least in next CF (I mean keeping remote TX open until local COMMIT or
> ABORT). It would require 2PC and many issues to be solved, so I'd like
> to focus fundamental part first. OTOH, using foreign tables in
> sub-transaction seems essential to me.
>
>> Anyway, let me pick up your patch for reviewing. And, I hope you to prepare
>> contrib/pgsql_fdw patch based on this patch.
>
> Thanks for your volunteer :-)
>
> Regards,
> --
> Shigeru HANADA

--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

Hanada-san,

The attached patch is a revised one according to my previous
suggestion. It re-defines "PQconninfoOption *options" as static
variable with NULL initial value, then, PQconndefaults() shall
be invoked at once. The default options never changed during
duration of the backend process, so here is no reason why we
allocate and free this object for each validator invocation.

If it is also OK for you, I'd like to take over this patch to comitter.
This patch is prerequisite of postgresql_fdw, so I hope this patch
getting merged soon.

Thanks,

2012/9/20 Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>:
> Hanada-san,
>
> I checked your patch. It can be applied to the latest master branch
> without any conflicts, and regression tests were fine.
>
> Unlike the original postgresql_fdw_validator(), the new
> dblink_fdw_validator() has wise idea; that pulls list of connection
> options from libpq, instead of self-defined static table.
> I basically agree not to have multiple source of option list.
>
> + /*
> + * Get list of valid libpq options. It contains default values too, but we
> + * don't care the values. Obtained list is allocated with malloc, so we
> + * must free it before leaving this function.
> + */
> + options = PQconndefaults();
> + if (options == NULL)
> + ereport(ERROR,
> + (errcode(ERRCODE_FDW_OUT_OF_MEMORY),
> + errmsg("out of memory"),
> + errdetail("could not get libpq default connection options")));
>
> I doubt whether PQconndefaults needs to be invoked for each
> validator calls. At least, supported option list of libpq should be
> never changed during run-time. So, I think PQconndefaults()
> should be called only once at first invocation, then option list
> can be stored in static variables or somewhere.
> As source code comments says, it is allocated with malloc, thus,
> we don't worry about unintentional release. :-)
>
> I don't think other part of this patch is arguable.
>
> Thanks,
>
> 2012/9/13 Shigeru HANADA <shigeru(dot)hanada(at)gmail(dot)com>:
>> Kaigai-san,
>>
>> (2012/09/13 16:56), Kohei KaiGai wrote:
>>> What about your plan to upstream contrib/pgsql_fdw module on the upcoming
>>> commit-fest?
>>
>> I will post pgsql_fdw patch (though it will be renamed to
>> postgresql_fdw) in opening CF (2012 Sep), as soon as I resolve a bug in
>> ANALYZE support, maybe on tomorrow. :-)
>>
>>> Even though I understand the point I noticed (miss-synchronization of sub-
>>> transaction block between local and remote side) is never easy problem to
>>> solve, it is worth to get the patch on the table of discussion.
>>> In my opinion, it is an idea to split-off the transaction control portion as
>>> a limitation of current version. For example, just raise an error when
>>> the foreign-table being referenced in sub-transaction block; with explicit
>>> description in the document.
>>
>> I agree not to support synchronize TX block between remote and local, at
>> least in next CF (I mean keeping remote TX open until local COMMIT or
>> ABORT). It would require 2PC and many issues to be solved, so I'd like
>> to focus fundamental part first. OTOH, using foreign tables in
>> sub-transaction seems essential to me.
>>
>>> Anyway, let me pick up your patch for reviewing. And, I hope you to prepare
>>> contrib/pgsql_fdw patch based on this patch.
>>
>> Thanks for your volunteer :-)
>>
>> Regards,
>> --
>> Shigeru HANADA
>
>
>
> --
> KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

(2012/10/09 0:30), Kohei KaiGai wrote:
> The attached patch is a revised one according to my previous
> suggestion. It re-defines "PQconninfoOption *options" as static
> variable with NULL initial value, then, PQconndefaults() shall
> be invoked at once. The default options never changed during
> duration of the backend process, so here is no reason why we
> allocate and free this object for each validator invocation.

Sorry for delayed response. It seems reasonable, so I just fixed
obsolete comment and indent. Please see attached v3 patch which was
rebased against latest head of master.

> If it is also OK for you, I'd like to take over this patch to comitter.
> This patch is prerequisite of postgresql_fdw, so I hope this patch
> getting merged soon.

Please go ahead. :-)

Regards,
--
Shigeru HANADA

Hanada-san,

It is fair enough for me.
So, I'd like to hand over this patch for committers.

Thanks,

2012/10/9 Shigeru HANADA <shigeru(dot)hanada(at)gmail(dot)com>:
> (2012/10/09 0:30), Kohei KaiGai wrote:
>>
>> The attached patch is a revised one according to my previous
>> suggestion. It re-defines "PQconninfoOption *options" as static
>> variable with NULL initial value, then, PQconndefaults() shall
>> be invoked at once. The default options never changed during
>> duration of the backend process, so here is no reason why we
>> allocate and free this object for each validator invocation.
>
>
> Sorry for delayed response. It seems reasonable, so I just fixed obsolete
> comment and indent. Please see attached v3 patch which was rebased against
> latest head of master.
>
>
>> If it is also OK for you, I'd like to take over this patch to comitter.
>> This patch is prerequisite of postgresql_fdw, so I hope this patch
>> getting merged soon.
>
>
> Please go ahead. :-)
>
> Regards,
> --
> Shigeru HANADA
>

--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

Shigeru HANADA <shigeru(dot)hanada(at)gmail(dot)com> writes:
> (2012/10/09 0:30), Kohei KaiGai wrote:
>> If it is also OK for you, I'd like to take over this patch to comitter.
>> This patch is prerequisite of postgresql_fdw, so I hope this patch
>> getting merged soon.

> Please go ahead. :-)

While reviewing this patch, I realized that we can't just summarily
delete the built-in postgresql_fdw_validator function: if we do, any
existing foreign-data wrapper definitions that depend on it will fail
to load into 9.3 servers. This would at least cause problems during
pg_upgrade for anyone who has set up dblink foreign servers according
to the current recommendation.

So I think we can't remove that functionality just yet. What we could
do is adjust postgresql_fdw_validator to throw a WARNING that it's
deprecated. This wouldn't prevent it from being used during dump/reload
scenarios, but it would put people on notice that the code will be
removed eventually. Without such a warning, it's not clear that we'll
ever be able to remove it without getting complaints.

However, I'm not sure where that leaves us with respect to the original
goal of getting rid of use of that function name. Thoughts?

For the moment I'm going to commit just the dblink changes in the patch,
and leave the core code alone pending agreement on what to do about the
upgrade-path issue.

regards, tom lane

Shigeru HANADA <shigeru(dot)hanada(at)gmail(dot)com> writes:
> [ dblink_fdw_validator.v3.patch ]

I've committed the dblink portion of this with some mostly-cosmetic
adjustments. We still need a plan for getting to a point where it's
safe to remove postgresql_fdw_validator.

regards, tom lane

Sorry for delayed response.

On 2012/10/11, at 5:28, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> So I think we can't remove that functionality just yet. What we could
> do is adjust postgresql_fdw_validator to throw a WARNING that it's
> deprecated. This wouldn't prevent it from being used during dump/reload
> scenarios, but it would put people on notice that the code will be
> removed eventually. Without such a warning, it's not clear that we'll
> ever be able to remove it without getting complaints.

After reading discussion about deprecating RULE, I realized that we should
be conservative when changing existing user-visible behavior. Agreed that
we can't remove postgresql_fdw_validator without announcing for some
releases.

> However, I'm not sure where that leaves us with respect to the original
> goal of getting rid of use of that function name. Thoughts?

Sorry, I had misunderstood the problem :-(. In my proposal, postgresql_fdw
uses public schema, as other contrib modules do, so its validator can live
with existing pg_catalog.postgresql_fdw_validator. IMHO we should
remove postgresql_fdw_validator sooner or later, but we don't need to hurry
to remove existing postgresql_fdw_validator from core.

Of course we must ensure that postgresql_fdw never uses in-core validator,
and dblink and other product never use postgresql_fdw's validator. To
achieve this, how about to use a schema, say postgresql_fdw, for
postgresql_fdw by specifying "schema" option in extension control file?
We need to qualify function names, so relocatable should be false. This
requires users of postgresql_fdw to set search_path or qualify
postgresql_fdw's functions and views every time, but it seems acceptable.

In addition, this approach would prevent pollution of public schema.

Comments?

Regards,
--
Shigeru HANADA
shigeru(dot)hanada(at)gmail(dot)com

On Fri, Oct 19, 2012 at 7:17 AM, Shigeru HANADA
<shigeru(dot)hanada(at)gmail(dot)com> wrote:
> However, I'm not sure where that leaves us with respect to the original
> goal of getting rid of use of that function name. Thoughts?
>
> Sorry, I had misunderstood the problem :-(. In my proposal, postgresql_fdw
> uses public schema, as other contrib modules do, so its validator can live
> with existing pg_catalog.postgresql_fdw_validator. IMHO we should
> remove postgresql_fdw_validator sooner or later, but we don't need to hurry
> to remove existing postgresql_fdw_validator from core.
>
> Of course we must ensure that postgresql_fdw never uses in-core validator,
> and dblink and other product never use postgresql_fdw's validator. To
> achieve this, how about to use a schema, say postgresql_fdw, for
> postgresql_fdw by specifying "schema" option in extension control file?
> We need to qualify function names, so relocatable should be false. This
> requires users of postgresql_fdw to set search_path or qualify
> postgresql_fdw's functions and views every time, but it seems acceptable.
>
> In addition, this approach would prevent pollution of public schema.

It seems to me that this is a case of the tail wagging the dog. The
original reason we ran into this issue is because there were some
people (I forget who, sorry) who insisted that this had to be renamed
from pgsql_fdw to postgresql_fdw. That change then caused this naming
conflict. Now, normally what we do if we have a naming conflict is we
rename one of the two things so that we don't have a naming conflict.
If we've determined that we can't rename postgresql_fdw_validator for
reasons of backward compatibility, then we should rename this new
thing instead. We of course do not have to use the original
pgsql_fdw_validator name; it can be postgres_fdw_validator or
postgree_fdw_validator or prostgreskewell_fdw_validator or whatever
the consensus bikeshed position is. Moving it to another schema does
not particularly appeal to me as it seems that having two
identically-named validators in different schemas will be even more
confusing than having two similarly-named validators in the same
schema.

If we're unwilling to tolerate committing this under some other name,
and we're also unwilling to remove postgresql_fdw_validator, then
we're essentially asking that we wait 4 or 5 years (or however long it
will take to deprecate postgresql_fdw_validator) to commit this very
important functionality to the server on the basis of the fact that
we've got a trivial name collision. That seems excessive in the
extreme. I'm frankly sorta shocked how much delay this issue has
*already* caused. I suspect there are other issues with regard this
patch that are much more worthy of our attention.

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

2012/10/19 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Fri, Oct 19, 2012 at 7:17 AM, Shigeru HANADA
> <shigeru(dot)hanada(at)gmail(dot)com> wrote:
>> However, I'm not sure where that leaves us with respect to the original
>> goal of getting rid of use of that function name. Thoughts?
>>
>> Sorry, I had misunderstood the problem :-(. In my proposal, postgresql_fdw
>> uses public schema, as other contrib modules do, so its validator can live
>> with existing pg_catalog.postgresql_fdw_validator. IMHO we should
>> remove postgresql_fdw_validator sooner or later, but we don't need to hurry
>> to remove existing postgresql_fdw_validator from core.
>>
>> Of course we must ensure that postgresql_fdw never uses in-core validator,
>> and dblink and other product never use postgresql_fdw's validator. To
>> achieve this, how about to use a schema, say postgresql_fdw, for
>> postgresql_fdw by specifying "schema" option in extension control file?
>> We need to qualify function names, so relocatable should be false. This
>> requires users of postgresql_fdw to set search_path or qualify
>> postgresql_fdw's functions and views every time, but it seems acceptable.
>>
>> In addition, this approach would prevent pollution of public schema.
>
> It seems to me that this is a case of the tail wagging the dog. The
> original reason we ran into this issue is because there were some
> people (I forget who, sorry) who insisted that this had to be renamed
> from pgsql_fdw to postgresql_fdw. That change then caused this naming
> conflict. Now, normally what we do if we have a naming conflict is we
> rename one of the two things so that we don't have a naming conflict.
> If we've determined that we can't rename postgresql_fdw_validator for
> reasons of backward compatibility, then we should rename this new
> thing instead. We of course do not have to use the original
> pgsql_fdw_validator name; it can be postgres_fdw_validator or
> postgree_fdw_validator or prostgreskewell_fdw_validator or whatever
> the consensus bikeshed position is.
>
IIRC, the reason why postgresql_fdw instead of pgsql_fdw was
no other fdw module has shorten naming such as ora_fdw for
Oracle.
However, I doubt whether it is enough strong reason to force to
solve the technical difficulty; naming conflicts with existing user
visible features.
Isn't it worth to consider to back to the pgsql_fdw_validator
naming again?

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

Sorry for long absence.

On Sat, Oct 20, 2012 at 4:24 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
> IIRC, the reason why postgresql_fdw instead of pgsql_fdw was
> no other fdw module has shorten naming such as ora_fdw for
> Oracle.
> However, I doubt whether it is enough strong reason to force to
> solve the technical difficulty; naming conflicts with existing user
> visible features.
> Isn't it worth to consider to back to the pgsql_fdw_validator
> naming again?

AFAIR, in the discussion about naming of the new FDW, another
name postgres_fdw was suggested as well as postgresql_fdw, and
I chose longer one at that time. Perhaps only a few people
feel that "postgres" is shortened name of postgresql. How
about using postgres_fdw for PG-FDW?

Once we chose the different name, postgresql_fdw_validator can
be live with postgres_fdw, though their names seem little
confusing.

In addition, it would be worth mentioning that it's not
recommended to use postgresql_fdw_validator as validator of a
third-party's FDW to avoid dependency.

Regards,
--
Shigeru HANADA

Sorry for long absence.

On Sat, Oct 20, 2012 at 4:24 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
> IIRC, the reason why postgresql_fdw instead of pgsql_fdw was
> no other fdw module has shorten naming such as ora_fdw for
> Oracle.
> However, I doubt whether it is enough strong reason to force to
> solve the technical difficulty; naming conflicts with existing user
> visible features.
> Isn't it worth to consider to back to the pgsql_fdw_validator
> naming again?

AFAIR, in the discussion about naming of the new FDW, another
name postgres_fdw was suggested as well as postgresql_fdw, and I
chose the one more familiar to me at that time. I think that only few
people feel that "postgres" is shortened name of
postgresql.

How about using postgres_fdw for PG-FDW?

Regards,
--
Shigeru HANADA

On Thu, Nov 15, 2012 at 02:33:21PM +0900, Shigeru Hanada wrote:
> On Sat, Oct 20, 2012 at 4:24 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
> > IIRC, the reason why postgresql_fdw instead of pgsql_fdw was
> > no other fdw module has shorten naming such as ora_fdw for
> > Oracle.
> > However, I doubt whether it is enough strong reason to force to
> > solve the technical difficulty; naming conflicts with existing user
> > visible features.
> > Isn't it worth to consider to back to the pgsql_fdw_validator
> > naming again?
>
> AFAIR, in the discussion about naming of the new FDW, another
> name postgres_fdw was suggested as well as postgresql_fdw, and I
> chose the one more familiar to me at that time. I think that only few
> people feel that "postgres" is shortened name of
> postgresql.
>
> How about using postgres_fdw for PG-FDW?

I couldn't agree more with Robert's comments[1]. Furthermore, this name only
shows up in calls to {CREATE|ALTER} FOREIGN DATA WRAPPER, which means 99.9% of
users would write "CREATE EXTENSION postgresql_fdw" and never even see the
name. I'd take "postgresql_fdw_whoops_names_are_a_big_commitment" if it meant
settling this issue 30 days earlier than we'd otherwise settle it.

Notwithstanding, I propose "postgresql.org/contrib/postgresql_fdw/validator".
Since the sole code that ought to reference the name lives in
contrib/postgresql_fdw/*.sql, the verbosity and double-quotation will cause no
appreciable harm. If anything, it will discourage ill-advised users.

Thanks,
nm

[1] http://archives.postgresql.org/message-id/CA+TgmobzOCV9RWUXO=xM_NkzrmPYZ0LgVuWhxyzueThZEqJHqw@mail.gmail.com

On 11/16/2012 08:08 AM, Noah Misch wrote:
> On Thu, Nov 15, 2012 at 02:33:21PM +0900, Shigeru Hanada wrote:
>> On Sat, Oct 20, 2012 at 4:24 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>>> IIRC, the reason why postgresql_fdw instead of pgsql_fdw was
>>> no other fdw module has shorten naming such as ora_fdw for
>>> Oracle.
>>> However, I doubt whether it is enough strong reason to force to
>>> solve the technical difficulty; naming conflicts with existing user
>>> visible features.
>>> Isn't it worth to consider to back to the pgsql_fdw_validator
>>> naming again?
>> AFAIR, in the discussion about naming of the new FDW, another
>> name postgres_fdw was suggested as well as postgresql_fdw, and I
>> chose the one more familiar to me at that time. I think that only few
>> people feel that "postgres" is shortened name of
>> postgresql.
>>
>> How about using postgres_fdw for PG-FDW?
> I couldn't agree more with Robert's comments[1]. Furthermore, this name only
> shows up in calls to {CREATE|ALTER} FOREIGN DATA WRAPPER, which means 99.9% of
> users would write "CREATE EXTENSION postgresql_fdw" and never even see the
> name. I'd take "postgresql_fdw_whoops_names_are_a_big_commitment" if it meant
> settling this issue 30 days earlier than we'd otherwise settle it.
>
> Notwithstanding, I propose "postgresql.org/contrib/postgresql_fdw/validator".
> Since the sole code that ought to reference the name lives in
> contrib/postgresql_fdw/*.sql, the verbosity and double-quotation will cause no
> appreciable harm. If anything, it will discourage ill-advised users.
Was there any further progress on this? Committing of the postgresql_fdw
seems to be stalled on a naming issue that has a couple of reasonable
resolutions available, and it'd be nice to get it in as a contrib module.

https://commitfest.postgresql.org/action/patch_view?id=940

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

2013/1/18 Craig Ringer <craig(at)2ndquadrant(dot)com>:
> On 11/16/2012 08:08 AM, Noah Misch wrote:
>
> On Thu, Nov 15, 2012 at 02:33:21PM +0900, Shigeru Hanada wrote:
>
> On Sat, Oct 20, 2012 at 4:24 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>
> IIRC, the reason why postgresql_fdw instead of pgsql_fdw was
> no other fdw module has shorten naming such as ora_fdw for
> Oracle.
> However, I doubt whether it is enough strong reason to force to
> solve the technical difficulty; naming conflicts with existing user
> visible features.
> Isn't it worth to consider to back to the pgsql_fdw_validator
> naming again?
>
> AFAIR, in the discussion about naming of the new FDW, another
> name postgres_fdw was suggested as well as postgresql_fdw, and I
> chose the one more familiar to me at that time. I think that only few
> people feel that "postgres" is shortened name of
> postgresql.
>
> How about using postgres_fdw for PG-FDW?
>
> I couldn't agree more with Robert's comments[1]. Furthermore, this name
> only
> shows up in calls to {CREATE|ALTER} FOREIGN DATA WRAPPER, which means 99.9%
> of
> users would write "CREATE EXTENSION postgresql_fdw" and never even see the
> name. I'd take "postgresql_fdw_whoops_names_are_a_big_commitment" if it
> meant
> settling this issue 30 days earlier than we'd otherwise settle it.
>
> Notwithstanding, I propose
> "postgresql.org/contrib/postgresql_fdw/validator".
> Since the sole code that ought to reference the name lives in
> contrib/postgresql_fdw/*.sql, the verbosity and double-quotation will cause
> no
> appreciable harm. If anything, it will discourage ill-advised users.
>
> Was there any further progress on this? Committing of the postgresql_fdw
> seems to be stalled on a naming issue that has a couple of reasonable
> resolutions available, and it'd be nice to get it in as a contrib module.
>
> https://commitfest.postgresql.org/action/patch_view?id=940
>
The current patch adopts "postgres_fdw" as name; that does never conflict
with existing functions, and well means what does this extension provide.
Previously, it was named "pgsql_fdw" but it was unpopular because of some
reasons; such as we don't call Oracle as Ora, why we call postgresql as pgsql?

I think, both of naming are good. It will give right impression for users about
functionality of this extension, and also add a new killer feature to v9.3.
If we spent waste of time for this topic any more, nobody will get happy.

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>