Re: [PATCH] postgres_fdw extension support

On 2015-10-06 06:28:34 -0700, Paul Ramsey wrote:
> +/*
> + * is_shippable
> + * Is this object (proc/op/type) shippable to foreign server?
> + * Check cache first, then look-up whether (proc/op/type) is
> + * part of a declared extension if it is not cached.
> + */
> +bool
> +is_shippable(Oid objnumber, Oid classnumber, List *extension_list)
> +{
> + ShippableCacheKey key;
> + ShippableCacheEntry *entry;
> +
> + /* Always return false if the user hasn't set the "extensions" option */
> + if (extension_list == NIL)
> + return false;
> +
> + /* Find existing cache, if any. */
> + if (!ShippableCacheHash)
> + InitializeShippableCache();
> +
> + /* Zero out the key */
> + memset(&key, 0, sizeof(key));
> +
> + key.objid = objnumber;
> + key.classid = classnumber;
> +
> + entry = (ShippableCacheEntry *)
> + hash_search(ShippableCacheHash,
> + (void *) &key,
> + HASH_FIND,
> + NULL);
> +
> + /* Not found in ShippableCacheHash cache. Construct new entry. */
> + if (!entry)
> + {
> + /*
> + * Right now "shippability" is exclusively a function of whether
> + * the obj (proc/op/type) is in an extension declared by the user.
> + * In the future we could additionally have a whitelist of functions
> + * declared one at a time.
> + */
> + bool shippable = lookup_shippable(objnumber, classnumber, extension_list);
> +
> + /*
> + * Don't create a new hash entry until *after* we have the shippable
> + * result in hand, as the shippable lookup might trigger a cache
> + * invalidation.
> + */
> + entry = (ShippableCacheEntry *)
> + hash_search(ShippableCacheHash,
> + (void *) &key,
> + HASH_ENTER,
> + NULL);
> +
> + entry->shippable = shippable;
> + }
> +
> + if (!entry)
> + return false;
> + else
> + return entry->shippable;
> +}

Maybe I'm missing something major here. But given that you're looking up
solely based on Oid objnumber, Oid classnumber, how would this cache
work if there's two foreign servers with different extension lists?

Greetings,

Andres Freund