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.
--
Michael