On Sat, Oct 26, 2013 at 11:17:19AM -0200, Rodolfo Campero wrote: > The attached patch add support of domains over arrays to PL/Python (eg: > CREATE DOMAIN my_domain AS integer[]). > > Basically it just uses get_base_element_type instead of get_element_type > in plpy_typeio.c, and uses domain_check before returning a sequence as > array in PLySequence_ToArray whenever appropriate.
Generally looks fine. Please lose the C++ comments though, this style is not used in Postgres sources.
Done.
> There's one line I'm not sure about; I modified a switch statement (line > 427): > switch (element_type ? element_type : getBaseType(arg->typoid)) > The rationale is that when element_type is set, it is already a base type, > because there is no support of arrays of domains in PostgreSQL, but this > may not held true in the future.
Was there any actual need to modify that? Or was it just performance optimization? ATM it creates asymmetry between PLy_output_datum_func2 and PLy_input_datum_func2.
If it's just performace optimization, then it should be done in both functions, but seems bad idea to do it in this patch. So I think it's better to leave it out.
There was no actual need to modify that, so I dropped that change in this new patch.
There are other cosmetic changes in this patch, wrt previous version (not preexistent code):
* adjusted alignment of variable name "rv" in line 12
* reworded comment in line 850, resulting in more than 80 characters, so I splitted the comment into a multiline comment following the surrounding style.