Re: [HACKERS] Removing faulty hyperLogLog merge function

2016-04-27 Thread Robert Haas 
On Wed, Apr 27, 2016 at 1:37 AM, Michael Paquier
 wrote:
>> I haven't followed this issue all that closely, but to me it seems
>> pretty clear. If the function is brand new to 9.6, buggy, and not even
>> used anywhere, I cannot imagine why we would leave it in the tree.
>
> +1. We should definitely not encourage its use for 3rd-part plugins.

So, now there's clearly a consensus.  Committed.  :-)

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

Re: [HACKERS] Removing faulty hyperLogLog merge function

2016-04-26 Thread Michael Paquier 
On Wed, Apr 27, 2016 at 12:08 PM, Joe Conway  wrote:
> On 04/26/2016 07:23 PM, Robert Haas wrote:
>> On Tue, Apr 26, 2016 at 9:35 PM, Tom Lane  wrote:
>>> Robert Haas  writes:
 I'm not prepared to commit this over the objection offered by Tomas
 Vondra on that thread.
>>>
>>> FWIW, I agree with Peter that we should remove this code.  We know that it
>>> is buggy.  Leaving it there constitutes an "attractive nuisance" --- that
>>> is, I'm afraid that someone will submit a patch that depends on that
>>> function, and that we might forget that the function is broken and commit
>>> said patch.
>>>
>>> Tomas' objection would be reasonable if a fix was simple, but so far as
>>> I can tell from the thread, it's not.  In particular, Peter doesn't trust
>>> the upstream patch in question.  But whether or not you trust it, doing
>>> nothing is not a sane choice.  The reasonable alternatives are to remove
>>> the merge function or sync the upstream patch.
>>
>> Now I agree with that.  And now we do not have a 1-1 tie on which
>> alternative to prefer, which is a good start towards a consensus.  Any
>> other views?
>
> I haven't followed this issue all that closely, but to me it seems
> pretty clear. If the function is brand new to 9.6, buggy, and not even
> used anywhere, I cannot imagine why we would leave it in the tree.

+1. We should definitely not encourage its use for 3rd-part plugins.
-- 
Michael


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

Re: [HACKERS] Removing faulty hyperLogLog merge function

2016-04-26 Thread Joe Conway 
On 04/26/2016 07:23 PM, Robert Haas wrote:
> On Tue, Apr 26, 2016 at 9:35 PM, Tom Lane  wrote:
>> Robert Haas  writes:
>>> I'm not prepared to commit this over the objection offered by Tomas
>>> Vondra on that thread.
>>
>> FWIW, I agree with Peter that we should remove this code.  We know that it
>> is buggy.  Leaving it there constitutes an "attractive nuisance" --- that
>> is, I'm afraid that someone will submit a patch that depends on that
>> function, and that we might forget that the function is broken and commit
>> said patch.
>>
>> Tomas' objection would be reasonable if a fix was simple, but so far as
>> I can tell from the thread, it's not.  In particular, Peter doesn't trust
>> the upstream patch in question.  But whether or not you trust it, doing
>> nothing is not a sane choice.  The reasonable alternatives are to remove
>> the merge function or sync the upstream patch.
> 
> Now I agree with that.  And now we do not have a 1-1 tie on which
> alternative to prefer, which is a good start towards a consensus.  Any
> other views?

I haven't followed this issue all that closely, but to me it seems
pretty clear. If the function is brand new to 9.6, buggy, and not even
used anywhere, I cannot imagine why we would leave it in the tree.

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] Removing faulty hyperLogLog merge function

2016-04-26 Thread Robert Haas 
On Tue, Apr 26, 2016 at 9:35 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> I'm not prepared to commit this over the objection offered by Tomas
>> Vondra on that thread.
>
> FWIW, I agree with Peter that we should remove this code.  We know that it
> is buggy.  Leaving it there constitutes an "attractive nuisance" --- that
> is, I'm afraid that someone will submit a patch that depends on that
> function, and that we might forget that the function is broken and commit
> said patch.
>
> Tomas' objection would be reasonable if a fix was simple, but so far as
> I can tell from the thread, it's not.  In particular, Peter doesn't trust
> the upstream patch in question.  But whether or not you trust it, doing
> nothing is not a sane choice.  The reasonable alternatives are to remove
> the merge function or sync the upstream patch.

Now I agree with that.  And now we do not have a 1-1 tie on which
alternative to prefer, which is a good start towards a consensus.  Any
other views?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

Re: [HACKERS] Removing faulty hyperLogLog merge function

2016-04-26 Thread Tom Lane 
Robert Haas  writes:
> I'm not prepared to commit this over the objection offered by Tomas
> Vondra on that thread.

FWIW, I agree with Peter that we should remove this code.  We know that it
is buggy.  Leaving it there constitutes an "attractive nuisance" --- that
is, I'm afraid that someone will submit a patch that depends on that
function, and that we might forget that the function is broken and commit
said patch.

Tomas' objection would be reasonable if a fix was simple, but so far as
I can tell from the thread, it's not.  In particular, Peter doesn't trust
the upstream patch in question.  But whether or not you trust it, doing
nothing is not a sane choice.  The reasonable alternatives are to remove
the merge function or sync the upstream patch.

regards, tom lane


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

Re: [HACKERS] Removing faulty hyperLogLog merge function

2016-04-26 Thread Robert Haas 
On Tue, Apr 26, 2016 at 7:54 PM, Peter Geoghegan  wrote:
> On Tue, Apr 26, 2016 at 4:47 PM, Robert Haas  wrote:
>>> You don't want to remove buggy code that is currently unused simply
>>> because it might be useful to have that functionality in the future?
>>
>> No, I don't want to remove code that somebody thinks we should fix
>> instead of removing on your say-so.
>
> I don't think that that's an efficient use of anyone's time. If
> somebody proposes a patch with functionality that needs to merge HLL
> states, then they can add an implementation at that time.

Peter, if there is consensus on this patch, I will commit it.  If
there isn't, I won't.  If you don't like that, sorry.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

Re: [HACKERS] Removing faulty hyperLogLog merge function

2016-04-26 Thread Peter Geoghegan 
On Tue, Apr 26, 2016 at 4:47 PM, Robert Haas  wrote:
>> You don't want to remove buggy code that is currently unused simply
>> because it might be useful to have that functionality in the future?
>
> No, I don't want to remove code that somebody thinks we should fix
> instead of removing on your say-so.

I don't think that that's an efficient use of anyone's time. If
somebody proposes a patch with functionality that needs to merge HLL
states, then they can add an implementation at that time.

-- 
Peter Geoghegan


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

Re: [HACKERS] Removing faulty hyperLogLog merge function

2016-04-26 Thread Robert Haas 
On Tue, Apr 26, 2016 at 7:46 PM, Peter Geoghegan  wrote:
> On Tue, Apr 26, 2016 at 4:41 PM, Robert Haas  wrote:
>> I'm not prepared to commit this over the objection offered by Tomas
>> Vondra on that thread.
>
> You don't want to remove buggy code that is currently unused simply
> because it might be useful to have that functionality in the future?

No, I don't want to remove code that somebody thinks we should fix
instead of removing on your say-so.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

Re: [HACKERS] Removing faulty hyperLogLog merge function

2016-04-26 Thread Peter Geoghegan 
On Tue, Apr 26, 2016 at 4:41 PM, Robert Haas  wrote:
> I'm not prepared to commit this over the objection offered by Tomas
> Vondra on that thread.

You don't want to remove buggy code that is currently unused simply
because it might be useful to have that functionality in the future?

-- 
Peter Geoghegan


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

Re: [HACKERS] Removing faulty hyperLogLog merge function

2016-04-26 Thread Robert Haas 
On Mon, Apr 25, 2016 at 10:39 PM, Peter Geoghegan  wrote:
> The function hyperLogLogMerge() is faulty [1]. It has no current
> callers, though. I propose that we rip it out, as in the attached
> patch.
>
> [1] 
> http://www.postgresql.org/message-id/CAM3SWZT-i6R9JU5YXa8MJUou2_r3LfGJZpQ9tYa1BYxfkj0=c...@mail.gmail.com

I'm not prepared to commit this over the objection offered by Tomas
Vondra on that thread.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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