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: > + <function>SPI_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 <symbol>InvalidOid</symbol> on error. On error, > + <varname>SPI_result</varname> is set to > + <symbol>SPI_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