> On Mar 4, 2019, at 4:22 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Paul Ramsey <pramsey@cleverelephant.ca> writes:
>>> On Mar 4, 2019, at 2:52 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> BTW, if you'd like me to review the code you added for this, I'd be happy
>>> to do so. I've never looked at PostGIS' innards, but probably I can make
>>> sense of the code for this despite that.
>
>> I would be ecstatic for a review, I'm sure I've left a million loose threads dangling.
>
> I took a look, and saw that you'd neglected to check pseudoconstantness
> of the non-index argument, so this'd fail on cases like ST_DWithin(x, y)
> where x is indexed and y is another column in the same table. Also
> I thought the handling of commutation could be done better. Attached is
> a suggested patch atop your f731c1b7022381dbf627cae311c3d37791bf40c3 to
> fix those and a couple of nitpicky other things. (I haven't tested this,
> mind you.)
>
> One thing that makes me itch, but I didn't address in the attached,
> is that expandFunctionOid() is looking up a function by name without
> any schema-qualification. That'll fail outright if PostGIS isn't in
> the search path, and even if it is, you've got security issues there.
> One way to address this is to assume that the expandfn is in the same
> schema as the ST_XXX function you're attached to, so you could
> do "get_namespace_name(get_func_namespace(funcid))" and then include
> that in the list passed to LookupFuncName.
Thanks for the patch, I’ve applied and smoothed and taken your advice on schema-qualified lookups as well.
> Also, this might be as-intended but I was wondering: I'd sort of expected
> you to make, eg, _ST_DWithin() and ST_DWithin() into exact synonyms.
> They aren't, since the former is not connected to the support function.
> Is that intentional? I guess if you had a situation where you wanted to
> force non-use of an index, being able to use _ST_DWithin() for that would
> be helpful.
Yes, this is by design. Other parts of the internal code base still like access to _ST_Functions, and there’s a
non-zerochance that some 3rd party callers still want them too. There’s a certain utility in having “guaranteed not
indexed”things so you can combine them with your other indexes of choice (particularly given the insane zoo of indexes
builtagainst geometry).
Again, many many thanks for your help! Next stop, costing.
P.
