Hi Pavel, I like the idea of using parameter name instead of $n symbols.
However, I am slightly worried that, at execution time if we want to know the parameter position in the actual function signature, then it will become difficult to get that from the corresponding datum variable. I don't have any use-case for that though. But apart from this concern, idea looks good to me.
Understand - but it was reason why I implemented this function - when I have to search parameter name via offset, I cannot to use string searching. When you know the parameter name, you can use a string searching in text editor, in pager.
It is better supported now, then current behave.
Make sense.
BTW, instead of doing all these changes, I have done these changes this way:
- /* Build variable and add to datum list */ - argvariable = plpgsql_build_variable(buf, 0, - argdtype, false); + /* + * Build variable and add to datum list. If there's a name for + * the argument, then use that else use $n name. + */ + argvariable = plpgsql_build_variable((argnames && argnames[i][0] != '\0') ? + argnames[i] : buf, + 0, argdtype, false);
This requires no new variable and thus no more changes elsewhere.
Attached patch with these changes. Please have a look.
Looks great - I added check to NULL only
Looks good. I have not made those changes in my earlier patch as I did not want to update other code which is not touched by this patch.
Anyways, your changes related to NULL check seems reasonable.
However, in attached patch I have fixed indentation.
Passing it on to the committer.
Thanks
--
Jeevan Chalke Principal Software Engineer, Product Development EnterpriseDB Corporation The Enterprise PostgreSQL Company
