Horton
Menu

Thread

  1. fdw validation function vs zero catalog id

    Martin Pihlak <martin.pihlak@gmail.com> — 2009-12-20T13:39:10Z

    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.

Attached patch fixes the issue by passing the proper catalog id from
transformGenericOptions().

PS. I personally would like this applied to 8.4 as well -- it'd enable
us to write a proper validator for pl/proxy fdw.

regards,
Martin

  2. Re: fdw validation function vs zero catalog id

    Tom Lane <tgl@sss.pgh.pa.us> — 2009-12-20T17:29:16Z

    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

  3. Re: fdw validation function vs zero catalog id

    Martin Pihlak <martin.pihlak@gmail.com> — 2009-12-20T22:26:23Z

    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

  4. Re: fdw validation function vs zero catalog id

    Tom Lane <tgl@sss.pgh.pa.us> — 2009-12-20T22:43:40Z

    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

  5. Re: fdw validation function vs zero catalog id

    Martin Pihlak <martin.pihlak@gmail.com> — 2009-12-21T12:59:30Z

    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

  6. Re: fdw validation function vs zero catalog id

    Martin Pihlak <martin.pihlak@gmail.com> — 2009-12-22T09:30:42Z

    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

  7. Re: fdw validation function vs zero catalog id

    Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> — 2009-12-23T12:24:42Z

    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