On Fri, Aug 13, 2010 at 12:55 PM, Joseph Adams <joeyadams3.14...@gmail.com> wrote: > Indeed, a built-in JSON data type will certainly not need it. > However, users may want to return enums from procedures written in C, > and this function provides a way to do it.
Yeah, but I can't see accepting it on speculation. We'll add it if and when it's clear that it will be generally useful. >> Incidentally, if we were going to accept this function, I think we'd >> need to add some kind of a check to throw an error if any of the >> labels can't be mapped onto an Oid. Otherwise, an error in this area >> might lead to difficult-to-find misbehavior. > > I agree. Perhaps an ereport(ERROR, ...) would be better, since it > would not be hard for a user to cause an enum mapping error (even in a > production database) by updating a stored procedure in C but not > updating the corresponding enum (or vice versa, if enum labels are > renamed). Right... > Fair enough. Perhaps the comment about FN_EXTRA (which explains > fn_extra in more detail) could be reworded (to talk about using > fcinfo->flinfo->fn_extra manually) and included in the documentation > at xfunc-c.html . fn_extra currently only gets a passing mention in > the discussion about set-returning functions. Feel to propose a patch to that comment. >>> pg_substring, pg_encoding_substring >>> * Useful-ometer: ()-------o >>> * Rationale: The JSONPath code uses it / will use it for extracting >>> substrings, which is probably not a very useful feature (but who am I >>> to say that). This function could probably benefit the >>> text_substring() function in varlena.c , but it would take a bit of >>> work to ensure it continues to comply with standards. >> >> I'm more positive about this idea. If you can make this generally >> useful, I'd encourage you to do that. On a random coding style note, >> I see that you have two copies of the following code, which can fairly >> obviously be written in a single line rather than five, assuming it's >> actually safe. >> >> + if (sub_end + len > e) >> + { >> + Assert(false); /* Clipped multibyte >> character */ >> + break; >> + } > > If I simply say Assert(sub_end + len <= e), the function will yield a > range hanging off the edge of the input string (out of bounds). The > five lines include a safeguard against that when assertion checking is > off. This could happen if the input string has a clipped multibyte > character at the end. Perhaps I should just drop the assertions and > make it so if there's a clipped character at the end, it'll be > ignored, no big deal. I think maybe what you want is ereport(ERROR). It should never be possible for user action to trip an Assert(); Assert() is only to catch coding mistakes. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers