<div class="em-message"><font face="arial">Hi guys,</font><br /><br />sorry, I'm stuck in an unfamiliar webmail.<br
/><br/>I checked the patch Petr just posted.<br /><a
href="http://archives.postgresql.org/pgsql-hackers/2012-03/msg00482.php">http://archives.postgresql.org/pgsql-hackers/2012-03/msg00482.php</a><br
/><br/>I have two comments. First, I notice that the documentation changes has two places that describe the columns
thata function checker returns -- one in the "plhandler" page, another in the create language page. I think it should
existonly on one of those, probably the create language one; and the plhandler page should just say "the checker should
complywith the specification at <create language>". Or something like that. Also, the fact that the tuple
descriptionis prose makes it hard to read; I think it should be a table -- three columns: name, type, description.<br
/><br/>My second comment is that the checker tuple descriptor seems to have changed in the code. In the patch I
posted,the FunctionCheckerDesc() function was not static; in this patch it has been made static. But what I intended
wasthat the other places that need a descriptor for anything would use this function to get one, instead of building
themby hand. There are two such places currently, one in CreateProceduralLanguage. I think this should be simply
walkingthe tupdesc->attrs array to create the arrays it needs for the ProcedureCreate call -- shoud be a rather easy
change. The other place is plpgsql's report_error(). Honestly I don't like this function at all due to the way it's
assumingwhat the tupledesc looks like. I'm not sure how to improve it, however, but it seems wrong to me. One reason
todo this this way (i.e. centralize knowledge of what the tupdesc looks like) is that otherwise they get out of sync --
Inotice that CreateProcedureLanguage now knows that there are 15 columns while the other places believe there are only
11. <br/><br /><br /></div>