Hi Kaigai-san,
(2011/11/20 2:42), Kohei KaiGai wrote:
> I'm still under reviewing of your patch, so the comment is not overall, sorry.
Thanks for the review!
> I'm not sure whether the logic of is_foreign_expr() is appropriate.
> It checks oid of the function within FuncExpr and OpExpr to disallow to push
> down user-defined functions.
> However, if a user-defined operator is implemented based on built-in functions
> with different meaning, is it really suitable to push-down?
> E.g) It is available to define '=' operator using int4ne, even though
> quite nonsense.
> So, I'd like to suggest to check oid of the operator; whether it is a
> built-in, or not.
>
> On the other hand, this hard-wired restriction may damage to the purpose of
> this module; that enables to handle a query on multiple nodes in parallel.
> I'm not sure whether it is right design that is_foreign_expr() returns false
> when the supplied expr contains mutable functions.
>
> Probably, here are two perspectives. The one want to make sure functions works
> with same manner in all nodes. The other want to distribute processing
> of queries
> as possible as we can. Here is a trade-off between these two perspectives.
> So, how about an idea to add a guc variable to control the criteria of
> pushing-down.
I agree that allowing users to control which function/operator should be
pushed down is useful, but GUC seems too large as unit of switching
behavior. "Routine Mapping", a mechanism which is defined in SQL/MED
standard, would be the answer for this issue. It can be used to map a
local routine (a procedure or a function) to something on a foreign
server. It is like user mapping, but it has mapping name. Probably it
would have these attributes:
pg_catalog.pg_routine_mapping rmname name rmprocid regproc rmserverid oid rmfdwoptions
text[]
If we have routine mapping, FDW authors can provide default mappings
within extension installation, and users can customize them. Maybe FDWs
will want to push down only functions/operators which have routine
mapping entries, so providing common routine which returns mapping
information of given function/operator, say GetRoutineMapping(procid,
serverid), is useful.
Unfortunately we don't have it at the moment, I'll fix pgsql_fdw so that
it pushes down only built-in operators, including scalar-array operators.
Regards,
--
Shigeru Hanada