On Wed, Jan 18, 2017 at 2:00 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Wed, Jan 18, 2017 at 5:26 AM, Peter Eisentraut
> <peter.eisentraut@2ndquadrant.com> wrote:
>> On 1/10/17 1:52 AM, Michael Paquier wrote:
>>> I don't see any problems with 0001.
>>
>> I was wondering, should we rename funcname -> name, and funcargs ->
>> args, or perhaps the whole FuncWithArgs struct, so there is no confusion
>> when used with operators?
>
> FuncWithArgs implies that this is related to a function, so removing
> func as prefix may make things cleaner.
>
>>> One comment though: there are still many list_make2() or even
>>> list_make3 calls for some object types. Would it make sense to replace
>>> those lists with a decided number of items by a Node and simplify the
>>> interface?
>>
>> (I don't see any list_make3.)
>
> Indeed, I am watching too much code.
>
>> It would be nice to refine this further,
>> but the remaining uses are quite marginal. The main problem was that
>> before you had to create singleton lists and then unpack them, because
>> there was no other way. The remaining uses are more genuine lists or lcons.
>
> OK. Of course, I am not saying that this patch in particular should
> shake more the world. I have been just trying to point out future
> potential improvements and keep a trace of them in the archives while
> thinking about it.
>
>>> In 0005, a nit:
>>> +DROP FUNCTION functest_IS_1(int, int, text), functest_IS_2(int),
>>> functest_IS_3(int);
>>> -- Cleanups
>>> The DROP query could be moved below the cleanup comment.
>>
>> I can do that, but the idea was that the commands below the cleanups
>> line weren't really tests.
>
> That's a nit, you can ignore that.
>
>>> While looking at 0006... DROP POLICY and DROP RULE could be unified. I
>>> just noticed that while reading the code.
>>
>> DROP TRIGGER also looks similar. drop_type3 then. ;-)
>
> Or drop_type_on, drop_type_on_table, etc.
I am marking the patch as returned with feedback, as this thread has
died 2 weeks ago.
--
Michael