On Thu, Nov 16, 2017 at 9:37 AM, amul sul <sula...@gmail.com> wrote: > Fixed in the 001 patch. > > IMHO, this function is not meant for a user, so that instead of ereport() cant > we simply return false? TWIW, I have attached 003 patch which replaces all > erepots() by return false.
I don't think just returning false is very helpful behavior, because the user may not realize that the problem is that the function is being called incorrectly rather than that the call is correct and the answer is false. I took your 0001 patch and made extensive modifications to it. I replaced your regression tests from 0002 with a new set that I wrote myself. The result is attached here. This version makes different decisions about how to handle the various problem cases than you did; it returns NULL for a NULL input or an OID for which no relation exists, and throws specific error messages for the other cases, matching the parser error messages that CREATE TABLE would issue where possible, but with a different error code. It also checks that the types match (which your patch did not, and which I'm fairly sure could crash the server), makes the function work when invoked using the explicit VARIADIC syntax (which seems fairly useless here but there's no in-core precedent for variadic function which doesn't support that case), and fixes the function header comment to describe the behavior we committed rather than the older behavior you had in earlier patch versions. As far as I can tell, this should nail things down pretty tight, but it would be great if someone can try to find a case where it still breaks. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
0001-Fix-multiple-problems-with-satisfies_hash_partition.patch
Description: Binary data