pgsql: Separate block sampling functions

Simon Riggs <simon@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

On 15 May 2015 at 03:50, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Simon Riggs <simon@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 Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/&gt;
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Fri, May 15, 2015 at 12:03 PM, Simon Riggs <simon@2ndquadrant.com> wrote:

On 15 May 2015 at 03:50, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Simon Riggs <simon@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

On 15 May 2015 at 04:06, Michael Paquier <michael.paquier@gmail.com> wrote:

On Fri, May 15, 2015 at 12:03 PM, Simon Riggs <simon@2ndquadrant.com>
wrote:

On 15 May 2015 at 03:50, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Simon Riggs <simon@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 Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/&gt;
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Michael Paquier <michael.paquier@gmail.com> writes:

On Fri, May 15, 2015 at 12:03 PM, Simon Riggs <simon@2ndquadrant.com> wrote:

On 15 May 2015 at 03:50, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Simon Riggs <simon@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

On Fri, May 15, 2015 at 12:12 PM, Simon Riggs <simon@2ndquadrant.com> wrote:

On 15 May 2015 at 04:06, Michael Paquier <michael.paquier@gmail.com> wrote:

On Fri, May 15, 2015 at 12:03 PM, Simon Riggs <simon@2ndquadrant.com>
wrote:

On 15 May 2015 at 03:50, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Simon Riggs <simon@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

On Fri, May 15, 2015 at 12:22 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Michael Paquier <michael.paquier@gmail.com> writes:

On Fri, May 15, 2015 at 12:03 PM, Simon Riggs <simon@2ndquadrant.com> wrote:

On 15 May 2015 at 03:50, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Simon Riggs <simon@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

Michael Paquier <michael.paquier@gmail.com> writes:

On Fri, May 15, 2015 at 12:22 PM, Tom Lane <tgl@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

On 15 May 2015 at 04:59, Tom Lane <tgl@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 Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/&gt;
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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

On 15 May 2015 at 04:59, Tom Lane <tgl@sss.pgh.pa.us
<mailto:tgl@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

On Fri, May 15, 2015 at 8:44 PM, Andrew Dunstan <andrew@dunslane.net> wrote:

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

On 15 May 2015 at 04:59, Tom Lane <tgl@sss.pgh.pa.us
<mailto:tgl@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

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 <tgl@sss.pgh.pa.us
<mailto:tgl@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

Andrew Dunstan <andrew@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.

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

On 18 May 2015 at 10:53, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andrew Dunstan <andrew@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 Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/&gt;
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

I wrote:

Andrew Dunstan <andrew@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.

I put in a shim --- crake should be happy again on its next run.

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