On 2018-11-08 15:08:03 +0100, Antonin Houska wrote: > Aleksandr Parfenov <asp...@gmail.com> wrote: > > > I fixed a typo and some comments. Please find new version attached. > > I've checked this verstion too. > > * is_simple_stable_function() > > instead of fetching HeapTuple from the syscache manually, you might want to > consider using functions from lsyscache.c (get_func_rettype, get_func_retset, > etc.), or adding a function that returns (subset of) the fields you need in a > single call. > > * pull_up_simple_function(): > > As you assume that ret->functions is a single-item list > > Assert(list_length(rte->functions) == 1); > > the following iteration is not necessary: > > foreach(lc, functions_list) > > Also, there seems to be a lot of copy & paste from pull_up_simple_values(), so > some refactoring would make sense.
Given this I think the appropriate state of the CF entry would have been waiting-for-author, not needs review. Or alternatively returned-with-feedback or rejected. I'm a bit confused as to why the patch was moved to the next CF twice? - Andres
