Re: [COMMITTERS] pgsql: Separate block sampling functions

2015-05-18 Thread Andrew Dunstan


On 05/15/2015 07:44 AM, Andrew Dunstan wrote:


On 05/15/2015 06:04 AM, Simon Riggs wrote:
On 15 May 2015 at 04:59, Tom Lane t...@sss.pgh.pa.us 
mailto:t...@sss.pgh.pa.us wrote:


The difference there was that that was specifically adding a new
feature
of value to FDWs.  This is just drive-by breakage.


I think that comment is reasonable. I will continue with my commits 
of tablesample, then return to see if we can improve/revert the API 
breakage.






OK, good, but I'm not going to accept Michael's pull request until the 
API is stable.







What is the current state of this? Are we sticking with what Tom 
classified as drive-by breakage?


cheers

andrew




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


Re: [COMMITTERS] pgsql: Separate block sampling functions

2015-05-18 Thread Simon Riggs
On 18 May 2015 at 10:53, Tom Lane t...@sss.pgh.pa.us wrote:

 Andrew Dunstan and...@dunslane.net writes:
  What is the current state of this? Are we sticking with what Tom
  classified as drive-by breakage?

 Since I was the one complaining, I'm willing to do the legwork to
 insert a compatibility shim.  I probably won't get to it today though,
 this being release wrap day.  In the meantime please leave
 FileTextArrayFDW as-is.


Sorry, I was hoping for input from Petr but he's just started leave, so its
up to us.

-- 
Simon Riggshttp://www.2ndQuadrant.com/
http://www.2ndquadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training  Services


Re: [COMMITTERS] pgsql: Separate block sampling functions

2015-05-15 Thread Michael Paquier
On Fri, May 15, 2015 at 8:44 PM, Andrew Dunstan and...@dunslane.net wrote:

 On 05/15/2015 06:04 AM, Simon Riggs wrote:

 On 15 May 2015 at 04:59, Tom Lane t...@sss.pgh.pa.us
 mailto:t...@sss.pgh.pa.us wrote:

 The difference there was that that was specifically adding a new
 feature
 of value to FDWs.  This is just drive-by breakage.


 I think that comment is reasonable. I will continue with my commits of
 tablesample, then return to see if we can improve/revert the API breakage.




 OK, good, but I'm not going to accept Michael's pull request until the API
 is stable.

Well, I guess that this is going to be incorrect in any case now.
Still any API change can be done cleanly in a matter of minutes for
this FDW.
-- 
Michael


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


Re: [COMMITTERS] pgsql: Separate block sampling functions

2015-05-15 Thread Simon Riggs
On 15 May 2015 at 04:59, Tom Lane t...@sss.pgh.pa.us wrote:


 The difference there was that that was specifically adding a new feature
 of value to FDWs.  This is just drive-by breakage.


I think that comment is reasonable. I will continue with my commits of
tablesample, then return to see if we can improve/revert the API breakage.

-- 
Simon Riggshttp://www.2ndQuadrant.com/
http://www.2ndquadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training  Services


Re: [COMMITTERS] pgsql: Separate block sampling functions

2015-05-15 Thread Andrew Dunstan


On 05/15/2015 06:04 AM, Simon Riggs wrote:
On 15 May 2015 at 04:59, Tom Lane t...@sss.pgh.pa.us 
mailto:t...@sss.pgh.pa.us wrote:


The difference there was that that was specifically adding a new
feature
of value to FDWs.  This is just drive-by breakage.


I think that comment is reasonable. I will continue with my commits of 
tablesample, then return to see if we can improve/revert the API 
breakage.






OK, good, but I'm not going to accept Michael's pull request until the 
API is stable.


cheers

andrew


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


Re: [COMMITTERS] pgsql: Separate block sampling functions

2015-05-14 Thread Tom Lane
Simon Riggs si...@2ndquadrant.com writes:
 Separate block sampling functions

This patch broke buildfarm member crake.

regards, tom lane


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


Re: [COMMITTERS] pgsql: Separate block sampling functions

2015-05-14 Thread Tom Lane
Michael Paquier michael.paqu...@gmail.com writes:
 On Fri, May 15, 2015 at 12:03 PM, Simon Riggs si...@2ndquadrant.com wrote:
 On 15 May 2015 at 03:50, Tom Lane t...@sss.pgh.pa.us wrote:
 Simon Riggs si...@2ndquadrant.com writes:
 Separate block sampling functions

 This patch broke buildfarm member crake.

 OK, thanks. I missed that amongst the other unrelated failures. Looking now.

 This needs a patch to file_text_array_fdw which I think is available here:
 https://github.com/adunstan/file_text_array_fdw
 Simon, do you mind if I send a pull request?

TBH, I think that this patch itself was a bad idea and should be reverted.
I don't object to changing APIs used by external modules when there's a
good reason to break them, but having looked at this patch all I see is
change for the sake of change.  What new functionality have you introduced?

Or to put it more baldly: it's likely that you've broken quite a large
number of third-party FDWs, not just this one.  A lot of people have
probably copied-and-pasted what was in the contrib FDWs.

regards, tom lane


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


Re: [COMMITTERS] pgsql: Separate block sampling functions

2015-05-14 Thread Simon Riggs
On 15 May 2015 at 03:50, Tom Lane t...@sss.pgh.pa.us wrote:

 Simon Riggs si...@2ndquadrant.com writes:
  Separate block sampling functions

 This patch broke buildfarm member crake.


OK, thanks. I missed that amongst the other unrelated failures. Looking
now.

-- 
Simon Riggshttp://www.2ndQuadrant.com/
http://www.2ndquadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training  Services


Re: [COMMITTERS] pgsql: Separate block sampling functions

2015-05-14 Thread Simon Riggs
On 15 May 2015 at 04:06, Michael Paquier michael.paqu...@gmail.com wrote:

 On Fri, May 15, 2015 at 12:03 PM, Simon Riggs si...@2ndquadrant.com
 wrote:
  On 15 May 2015 at 03:50, Tom Lane t...@sss.pgh.pa.us wrote:
 
  Simon Riggs si...@2ndquadrant.com writes:
   Separate block sampling functions
 
  This patch broke buildfarm member crake.
 
 
  OK, thanks. I missed that amongst the other unrelated failures. Looking
 now.

 This needs a patch to file_text_array_fdw which I think is available here:
 https://github.com/adunstan/file_text_array_fdw
 Simon, do you mind if I send a pull request?


Hmm, guess that explains it then, I was just scratching my head.

Yes please sort that out.

-- 
Simon Riggshttp://www.2ndQuadrant.com/
http://www.2ndquadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training  Services


Re: [COMMITTERS] pgsql: Separate block sampling functions

2015-05-14 Thread Tom Lane
Michael Paquier michael.paqu...@gmail.com writes:
 On Fri, May 15, 2015 at 12:22 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 TBH, I think that this patch itself was a bad idea and should be reverted.
 I don't object to changing APIs used by external modules when there's a
 good reason to break them, but having looked at this patch all I see is
 change for the sake of change.  What new functionality have you introduced?

 If you look at the TABLESAMPLE patch, separating the block sampling
 into a separate facility makes quite some sense.

Sure, but there was no need to break FDWs that were using an API that
was perfectly reasonable for ANALYZE support.

Had you not made random changes to the argument lists, we could have
solved this with some compatibility macros...

 Or to put it more baldly: it's likely that you've broken quite a large
 number of third-party FDWs, not just this one.  A lot of people have
 probably copied-and-pasted what was in the contrib FDWs.

 make_foreignscan() has been changed as well by 1a8a4e5c..

The difference there was that that was specifically adding a new feature
of value to FDWs.  This is just drive-by breakage.

regards, tom lane


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


Re: [COMMITTERS] pgsql: Separate block sampling functions

2015-05-14 Thread Michael Paquier
On Fri, May 15, 2015 at 12:03 PM, Simon Riggs si...@2ndquadrant.com wrote:
 On 15 May 2015 at 03:50, Tom Lane t...@sss.pgh.pa.us wrote:

 Simon Riggs si...@2ndquadrant.com writes:
  Separate block sampling functions

 This patch broke buildfarm member crake.


 OK, thanks. I missed that amongst the other unrelated failures. Looking now.

This needs a patch to file_text_array_fdw which I think is available here:
https://github.com/adunstan/file_text_array_fdw
Simon, do you mind if I send a pull request?
-- 
Michael


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


Re: [COMMITTERS] pgsql: Separate block sampling functions

2015-05-14 Thread Michael Paquier
On Fri, May 15, 2015 at 12:12 PM, Simon Riggs si...@2ndquadrant.com wrote:
 On 15 May 2015 at 04:06, Michael Paquier michael.paqu...@gmail.com wrote:

 On Fri, May 15, 2015 at 12:03 PM, Simon Riggs si...@2ndquadrant.com
 wrote:
  On 15 May 2015 at 03:50, Tom Lane t...@sss.pgh.pa.us wrote:
 
  Simon Riggs si...@2ndquadrant.com writes:
   Separate block sampling functions
 
  This patch broke buildfarm member crake.
 
 
  OK, thanks. I missed that amongst the other unrelated failures. Looking
  now.

 This needs a patch to file_text_array_fdw which I think is available here:
 https://github.com/adunstan/file_text_array_fdw
 Simon, do you mind if I send a pull request?


 Hmm, guess that explains it then, I was just scratching my head.

 Yes please sort that out.

Sent a patch here:
https://github.com/adunstan/file_text_array_fdw/pull/1
-- 
Michael


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


Re: [COMMITTERS] pgsql: Separate block sampling functions

2015-05-14 Thread Michael Paquier
On Fri, May 15, 2015 at 12:22 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Michael Paquier michael.paqu...@gmail.com writes:
 On Fri, May 15, 2015 at 12:03 PM, Simon Riggs si...@2ndquadrant.com wrote:
 On 15 May 2015 at 03:50, Tom Lane t...@sss.pgh.pa.us wrote:
 Simon Riggs si...@2ndquadrant.com writes:
 Separate block sampling functions

 This patch broke buildfarm member crake.

 OK, thanks. I missed that amongst the other unrelated failures. Looking now.

 This needs a patch to file_text_array_fdw which I think is available here:
 https://github.com/adunstan/file_text_array_fdw
 Simon, do you mind if I send a pull request?

 TBH, I think that this patch itself was a bad idea and should be reverted.
 I don't object to changing APIs used by external modules when there's a
 good reason to break them, but having looked at this patch all I see is
 change for the sake of change.  What new functionality have you introduced?

If you look at the TABLESAMPLE patch, separating the block sampling
into a separate facility makes quite some sense.

 Or to put it more baldly: it's likely that you've broken quite a large
 number of third-party FDWs, not just this one.  A lot of people have
 probably copied-and-pasted what was in the contrib FDWs.

make_foreignscan() has been changed as well by 1a8a4e5c..
-- 
Michael


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