fdw validation function vs zero catalog id

Martin Pihlak <martin.pihlak@gmail.com> writes:

It appears that the function for validating generic options to a FDW,
SERVER and USER MAPPING is always passed a catalog oid of 0. Whereas
it should actually be passed the oid of the catalog that the options
apply to.

According to what? I can't find any documentation whatsoever on what
arguments that function is supposed to get.

regards, tom lane

Tom Lane wrote:

According to what? I can't find any documentation whatsoever on what
arguments that function is supposed to get.

regards, tom lane

According to
http://www.postgresql.org/docs/8.4/static/sql-createforeigndatawrapper.html:

"The validator function must take two arguments: one of type text[], which
will contain the array of options as stored in the system catalogs, and one
of type oid, which will be the OID of the system catalog containing the
options, or zero if the context is not known."

regards,
Martin

Martin Pihlak <martin.pihlak@gmail.com> writes:

Tom Lane wrote:

According to what? I can't find any documentation whatsoever on what
arguments that function is supposed to get.

According to
http://www.postgresql.org/docs/8.4/static/sql-createforeigndatawrapper.html:

"The validator function must take two arguments: one of type text[], which
will contain the array of options as stored in the system catalogs, and one
of type oid, which will be the OID of the system catalog containing the
options, or zero if the context is not known."

Hmm, dunno how I missed that. But anyway ISTM the current code conforms
to that specification just fine. I think what you're really lobbying
for is that we remove the "or zero" escape hatch and insist that the
backend code do whatever it has to do to supply a correct OID. This
patch shows that that's not too hard right now, but are there going to
be future situations where it's harder? What was the motivation for
including the escape hatch in the first place?

regards, tom lane

Tom Lane wrote:

"The validator function must take two arguments: one of type text[], which
will contain the array of options as stored in the system catalogs, and one
of type oid, which will be the OID of the system catalog containing the
options, or zero if the context is not known."

Hmm, dunno how I missed that. But anyway ISTM the current code conforms
to that specification just fine. I think what you're really lobbying

It certainly looks like a bug to me -- while performing CREATE or ALTER on a
SQL/MED object, the catalog must surely be known, and one would expect that
the validator function is passed the actual catalog id. Otherwise there would
be no point for the validator function to accept the catalog id at all.

for is that we remove the "or zero" escape hatch and insist that the
backend code do whatever it has to do to supply a correct OID. This
patch shows that that's not too hard right now, but are there going to
be future situations where it's harder? What was the motivation for
including the escape hatch in the first place?

The validator is run for the generic options specified to CREATE/ALTER FDW,
SERVER and USER MAPPING (+ possible future SQL/MED objects). In this case the
catalog is always known. Also we can assume that the catalog is known, if a user
runs the validator directly. So yes, AFAICS there is no case for the "or zero".

regards,
Martin

I wrote:

The validator is run for the generic options specified to CREATE/ALTER FDW,
SERVER and USER MAPPING (+ possible future SQL/MED objects). In this case the
catalog is always known. Also we can assume that the catalog is known, if a user
runs the validator directly. So yes, AFAICS there is no case for the "or zero".

Updated patch attached. This now also removes the "or zero" note from
the documentation and modifies postgresql_fdw_validator() to assume that
a valid catalog oid is always passed.

regards,
Martin

Martin Pihlak wrote:

I wrote:

The validator is run for the generic options specified to CREATE/ALTER FDW,
SERVER and USER MAPPING (+ possible future SQL/MED objects). In this case the
catalog is always known. Also we can assume that the catalog is known, if a user
runs the validator directly. So yes, AFAICS there is no case for the "or zero".

Updated patch attached. This now also removes the "or zero" note from
the documentation and modifies postgresql_fdw_validator() to assume that
a valid catalog oid is always passed.

Committed. I don't foresee any scenario either where we wouldn't know
the catalog ID.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com