[HACKERS] Re: [Review] Add SPI_gettypmod() to return a field's typemod from a TupleDesc / audit of [E] TODO items
On Sun, Jul 07, 2013 at 08:15:00PM -0400, Noah Misch wrote: I mildly recommend we reject this patch as such, remove the TODO item, remove the XXX comments this patch removes, and plan not to add more trivial SPI wrappers. Seeing just the one response consistent with that view, done. -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [Review] Add SPI_gettypmod() to return a field's typemod from a TupleDesc
I find the SPI interface support functions quaint. They're thin wrappers, of ancient origin, around standard backend coding patterns. They have the anti-feature of communicating certain programming errors through return value/SPI_result rather than elog()/Assert(). The chance that we could substantially refactor the underlying primary backend APIs and data structures while keeping these SPI wrappers unchanged seems slight. On Tue, Jun 25, 2013 at 07:19:58PM -0700, Mark Wong wrote: +functionSPI_gettypmod/function returns the type-specific data supplied +at table creation time. For example: the max length of a varchar field. SPI callers typically have no business interpreting the value, that being the distinct purview of each type implementation. The text type does set its typmod to 4 + max length, but other code should not know that. SPI callers can use this to convey a typmod for later use, though. + para +The type-specific data supplied at table creation time of the specified +column or symbolInvalidOid/symbol on error. On error, +varnameSPI_result/varname is set to +symbolSPI_ERROR_NOATTRIBUTE/symbol. + /para You have it returning -1, not InvalidOid. Per Amit's review last year, I'm wary of returning -1 in the error case. But I suspect most callers will, like the two callers you add, make a point of never passing an invalid argument and then not bother checking for error. So, no big deal. I mildly recommend we reject this patch as such, remove the TODO item, remove the XXX comments this patch removes, and plan not to add more trivial SPI wrappers. If consensus goes otherwise, I think we should at least introduce SPI_getcollation() at the same time. Code that needs to transfer one of them very often needs to transfer the other. Having API coverage for just one makes it easier for hackers to miss that. Thanks, nm -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: [Review] Add SPI_gettypmod() to return a field's typemod from a TupleDesc
On Sun, 2013-07-07 at 20:15 -0400, Noah Misch wrote: I mildly recommend we reject this patch as such, remove the TODO item, remove the XXX comments this patch removes, and plan not to add more trivial SPI wrappers. If consensus goes otherwise, I think we should at least introduce SPI_getcollation() at the same time. Code that needs to transfer one of them very often needs to transfer the other. Having API coverage for just one makes it easier for hackers to miss that. The question is, what would one do with those values? It's hard to see when you would need the typmod and the collation of a result set. There might be cases, but enough to provide a special API for it? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: [Review] Add SPI_gettypmod() to return a field's typemod from a TupleDesc
On Sun, Jul 07, 2013 at 08:55:01PM -0400, Peter Eisentraut wrote: On Sun, 2013-07-07 at 20:15 -0400, Noah Misch wrote: I mildly recommend we reject this patch as such, remove the TODO item, remove the XXX comments this patch removes, and plan not to add more trivial SPI wrappers. If consensus goes otherwise, I think we should at least introduce SPI_getcollation() at the same time. Code that needs to transfer one of them very often needs to transfer the other. Having API coverage for just one makes it easier for hackers to miss that. The question is, what would one do with those values? It's hard to see when you would need the typmod and the collation of a result set. There might be cases, but enough to provide a special API for it? Good point. One of the ways PL/pgSQL uses it is to feed a result datum back into a future query as a Param node. -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
