Subject: Re: proposal: make NOTIFY list de-duplication optional
To: =?UTF-8?Q?Filip_Rembia=c5=82kowski?= <filip.rembialkowski@gmail.com>,
 Brendan Jurd <direvus@gmail.com>
References: 
 <CAP_rwwn2z0gPOn8GuQ3qDVS5+HgEcG2EzEOyiJtcA=vpDEhoCg@mail.gmail.com>
 <CA+TgmoYiKedu0vUXFQHKpJ1COmT7sNJd2KOLLtXQTQc6Ztgkug@mail.gmail.com>
 <7647.1454723392@sss.pgh.pa.us>
 <CADxJZo16qGNKhROpM-7hg6xgxNMarh2kBW=2suOCv+5EbPUfsw@mail.gmail.com>
 <CAP_rww=xRoBfbAqkywCHTs3jDxF9zknh1zLkrJKxZGykRtEHQg@mail.gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Robert Haas <robertmhaas@gmail.com>,
 Pgsql Hackers <pgsql-hackers@postgresql.org>
From: Vik Fearing <vik@2ndquadrant.fr>
Message-ID: <56B7224C.4090807@2ndquadrant.fr>
Date: Sun, 7 Feb 2016 11:54:04 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Thunderbird/38.5.1
MIME-Version: 1.0
In-Reply-To: 
 <CAP_rww=xRoBfbAqkywCHTs3jDxF9zknh1zLkrJKxZGykRtEHQg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

On 02/07/2016 03:42 AM, Filip Rembiałkowski wrote:
> +1
> 
> ... and a patch (only adding ALL keyword, no hash table implemented yet).

Please stop top-posting, it's very disruptive.  My comments are below,
where they belong.

> On Sat, Feb 6, 2016 at 2:35 PM, Brendan Jurd <direvus@gmail.com> wrote:
>> On Sat, 6 Feb 2016 at 12:50 Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>>
>>> Robert Haas <robertmhaas@gmail.com> writes:
>>>> I agree with what Merlin said about this:
>>>>
>>>> http://www.postgresql.org/message-id/CAHyXU0yoHe8Qc=yC10AHU1nFiA1tbHsg+35Ds-oEueUapo7t4g@mail.gmail.com
>>>
>>> Yeah, I agree that a GUC for this is quite unappetizing.
>>
>>
>> How would you feel about a variant for calling NOTIFY?
>>
>> The SQL syntax could be something like "NOTIFY [ALL] channel, payload" where
>> the ALL means "just send the notification already, nobody cares whether
>> there's an identical one in the queue".
>>
>> Likewise we could introduce a three-argument form of pg_notify(text, text,
>> bool) where the final argument is whether you are interested in removing
>> duplicates.
>>
>> Optimising the remove-duplicates path is still probably a worthwhile
>> endeavour, but if the user really doesn't care at all about duplication, it
>> seems silly to force them to pay any performance price for a behaviour they
>> didn't want, no?

On 02/07/2016 03:42 AM, Filip Rembiałkowski wrote:
> +1
>
> ... and a patch (only adding ALL keyword, no hash table implemented yet).


I only read through the patch, I didn't compile it or test it, but I
have a few comments:

You left the duplicate behavior with subtransactions, but didn't mention
it in the documentation.  If I do  NOTIFY DISTINCT chan, 'msg';  then I
expect only distinct notifications but I'll get duplicates if I'm in a
subtransaction.  Either the documentation or the code needs to be fixed.

I seem to remember some discussion about not using DEFAULT parameters in
system functions so you should leave the old function alone and create a
new function with your use_all parameter.  I don't recall the exact
reason why so hopefully someone else will enlighten me.

There is also no mention in the documentation about what happens if I do:

    NOTIFY ALL chan, 'msg';
    NOTIFY ALL chan, 'msg';
    NOTIFY DISTINCT chan, 'msg';
    NOTIFY ALL chan, 'msg';

Without testing, I'd say I'd get two messages, but it should be
explicitly mentioned somewhere.
-- 
Vik Fearing                                          +33 6 46 75 15 36
http://2ndQuadrant.fr     PostgreSQL : Expertise, Formation et Support


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers