[pgpool-hackers: 3761] Re: Using volatile property instead of black_function_list

[Hou, Zhijie]

Hi Ishii san

> > I think it works.
> >
> > And I hava some comments on the previous patch which use FORMATTED flag.
> >
> > Currently in function pool_search_relcache, first we use the
> function(table) name to match the right cache.
> >
> > 		if (strcasecmp(relcache->cache[i].dbname, dbname) == 0 &&
> > 			strcasecmp(relcache->cache[i].relname, table)
> == 0)
> >
> > if cache does not exists, we use "relcache->sql" to check the system
> catalog.
> >
> > 		snprintf(query, sizeof(query), relcache->sql, table);
> >
> > In function is_immutable_function, we change the "relcache->sql" every
> time we call is_immutable_function.
> > (when relcache has not been inited, we change local variables "query",
> > but "relcache->sql" will be indirectly modified with the local
> > variables "query" in pool_create_relcache)
> >
> > 		if(!relcache)
> > 		{
> > 			snprintf(query, sizeof(query),
> IS_STABLE_FUNCTION_QUERY,
> > 					"=", (char *) lsecond(names),
> (char *) linitial(names));
> > 		}
> > 		else
> > 		{
> > 			snprintf(relcache->sql, sizeof(relcache->sql),
> IS_STABLE_FUNCTION_QUERY,
> > 					"=", (char *) lsecond(names),
> (char *) linitial(names));
> >
> > 		}
> >
> > So ,I think it did use the correct sql, and the result is correct, what
> do you think?
> 
> Yes, as far as is_immutable_function concerns, it should work.  But now
> we want to check not only pg_proc.provolatile = 'i' case but
> pg_proc.provolatile = 'v' case. To make the code smaller and simpler, I
> want to unify pg_proc.provolatile = 'i' case and pg_proc.provolatile = 'v'
> case. If we want to stick with your method, we need to have two distinct
> relation cache: one is for pg_proc.provolatile = 'i case, and the other
> is for pg_proc.provolatile = 'v' case, which I want to avoid (actually we
> need three because function_volatile_property() in my patch allows to
> accept three types of volatile property).
> 
> > If both methods work, may be we'd better use the shorter cache key?
> 
> I agree that shorter cache key is better, but I guess the difference is
> atually subtle and I don't think it worth the more complex and longer code.
> 
> If we concern the performance of the relation cache, probably we'd better
> to think about using a hash table, rather than a plain array which is
> currently used.

Thanks for your explanation, I understand the point.
You are right, this patch is actually better.

For the relation cache, May be we can add a todo? 

Best regards,
houzj