Horton

Thread

Commits

  1. Remove some code related to 7.3 and older servers from tools of src/bin/

  1. reindexdb & clusterdb broken against pre-7.3 servers

    Julien Rouhaud <rjuju123@gmail.com> — 2019-05-06T08:04:45Z

    Hi,

It seems that 582edc369cd caused $subject.

Trivial fix attached, though I obviously didn't actually test it
against such server.

  2. Re: reindexdb & clusterdb broken against pre-7.3 servers

    Julien Rouhaud <rjuju123@gmail.com> — 2019-05-06T08:17:05Z

    On Mon, May 6, 2019 at 10:04 AM Julien Rouhaud <rjuju123@gmail.com> wrote:
>
> Hi,
>
> It seems that 582edc369cd caused $subject.
>
> Trivial fix attached, though I obviously didn't actually test it
> against such server.

Ahem, correct fix attached.  I'm going to get a coffee and hide for
the rest of the day.

  3. Re: reindexdb & clusterdb broken against pre-7.3 servers

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2019-05-06T12:32:44Z

    On 5/6/19 4:17 AM, Julien Rouhaud wrote:
> On Mon, May 6, 2019 at 10:04 AM Julien Rouhaud <rjuju123@gmail.com> wrote:
>> Hi,
>>
>> It seems that 582edc369cd caused $subject.
>>
>> Trivial fix attached, though I obviously didn't actually test it
>> against such server.
> Ahem, correct fix attached.  I'm going to get a coffee and hide for
> the rest of the day.



Why do we even have code referring to pre-7.3 servers? Wouldn't it be
simpler just to remove that code?


cheers


andrew


-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  4. Re: reindexdb & clusterdb broken against pre-7.3 servers

    Michael Paquier <michael@paquier.xyz> — 2019-05-06T12:49:47Z

    On Mon, May 06, 2019 at 08:32:44AM -0400, Andrew Dunstan wrote:
> Why do we even have code referring to pre-7.3 servers? Wouldn't it be
> simpler just to remove that code?

Even for pg_dump, we only support servers down to 8.0.  Let's nuke
this code.
--
Michael

  5. Re: reindexdb & clusterdb broken against pre-7.3 servers

    Stephen Frost <sfrost@snowman.net> — 2019-05-06T13:45:39Z

    Greetings,

* Michael Paquier (michael@paquier.xyz) wrote:
> On Mon, May 06, 2019 at 08:32:44AM -0400, Andrew Dunstan wrote:
> > Why do we even have code referring to pre-7.3 servers? Wouldn't it be
> > simpler just to remove that code?
> 
> Even for pg_dump, we only support servers down to 8.0.  Let's nuke
> this code.

Agreed.

Seems like we should probably have all of our client tools in-sync
regarding what version they support down to.  There's at least some
code in psql that tries to work with pre-8.0 too (around tablespaces and
savepoints, specifically, it looks like), but I have doubts that recent
changes to psql have been tested back to pre-8.0.

At least... for the client tools that support multiple major versions.
Seems a bit unfortunate that we don't really define formally anywhere
which tools in src/bin/ work with multiple major versions and which
don't, even though that's a pretty big distinction and one that matters
to packagers and users.

Thanks,

Stephen

  6. Re: reindexdb & clusterdb broken against pre-7.3 servers

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-05-06T13:49:07Z

    Michael Paquier <michael@paquier.xyz> writes:
> On Mon, May 06, 2019 at 08:32:44AM -0400, Andrew Dunstan wrote:
>> Why do we even have code referring to pre-7.3 servers? Wouldn't it be
>> simpler just to remove that code?

> Even for pg_dump, we only support servers down to 8.0.  Let's nuke
> this code.

+1.  I think psql claims to support down to 7.4, but that's still not
a precedent for trying to handle pre-7.3.  Also, the odds that we'd
not break this code path again in future seem pretty bad.

			regards, tom lane

  7. Re: reindexdb & clusterdb broken against pre-7.3 servers

    Julien Rouhaud <rjuju123@gmail.com> — 2019-05-06T15:31:17Z

    On Mon, May 6, 2019 at 3:49 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Michael Paquier <michael@paquier.xyz> writes:
> > On Mon, May 06, 2019 at 08:32:44AM -0400, Andrew Dunstan wrote:
> >> Why do we even have code referring to pre-7.3 servers? Wouldn't it be
> >> simpler just to remove that code?
>
> > Even for pg_dump, we only support servers down to 8.0.  Let's nuke
> > this code.
>
> +1.  I think psql claims to support down to 7.4, but that's still not
> a precedent for trying to handle pre-7.3.  Also, the odds that we'd
> not break this code path again in future seem pretty bad.

WFM.  Updated patch attached, I also removed another similar chunk in
the same file while at it.

  8. Re: reindexdb & clusterdb broken against pre-7.3 servers

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-05-06T15:34:24Z

    Julien Rouhaud <rjuju123@gmail.com> writes:
> WFM.  Updated patch attached, I also removed another similar chunk in
> the same file while at it.

Uh, that looks backwards:

@@ -146,10 +146,6 @@ connectDatabase(const char *dbname, const char *pghost,
 		exit(1);
 	}
 
-	if (PQserverVersion(conn) >= 70300)
-		PQclear(executeQuery(conn, ALWAYS_SECURE_SEARCH_PATH_SQL,
-							 progname, echo));
-
 	return conn;
 }
 

			regards, tom lane

  9. Re: reindexdb & clusterdb broken against pre-7.3 servers

    Julien Rouhaud <rjuju123@gmail.com> — 2019-05-06T15:39:18Z

    On Mon, May 6, 2019 at 5:34 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Julien Rouhaud <rjuju123@gmail.com> writes:
> > WFM.  Updated patch attached, I also removed another similar chunk in
> > the same file while at it.
>
> Uh, that looks backwards:

Argh, sorry :(

I'm definitely done for today.

  10. Re: reindexdb & clusterdb broken against pre-7.3 servers

    Michael Paquier <michael@paquier.xyz> — 2019-05-07T00:46:07Z

    On Mon, May 06, 2019 at 05:39:18PM +0200, Julien Rouhaud wrote:
> I'm definitely done for today.

Looks good to me, so committed.
--
Michael

  11. Re: reindexdb & clusterdb broken against pre-7.3 servers

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-05-07T02:23:07Z

    Michael Paquier <michael@paquier.xyz> writes:
> On Mon, May 06, 2019 at 05:39:18PM +0200, Julien Rouhaud wrote:
>> I'm definitely done for today.

> Looks good to me, so committed.

The originally-complained-of breakage exists in all active branches,
so is it really OK to commit this only in HEAD?

			regards, tom lane

  12. Re: reindexdb & clusterdb broken against pre-7.3 servers

    Michael Paquier <michael@paquier.xyz> — 2019-05-07T03:18:15Z

    On Mon, May 06, 2019 at 10:23:07PM -0400, Tom Lane wrote:
> The originally-complained-of breakage exists in all active branches,
> so is it really OK to commit this only in HEAD?

I did not think that it would be that critical for back-branches, but
I don't mind going ahead and remove the code there as well.  Are there
any objections with it?

Also, wouldn't we want instead to apply on back-branches the first
patch proposed on this thread which fixes the query generation for
this pre-7.3 related code?
--
Michael

  13. Re: reindexdb & clusterdb broken against pre-7.3 servers

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-05-07T03:24:31Z

    Michael Paquier <michael@paquier.xyz> writes:
> On Mon, May 06, 2019 at 10:23:07PM -0400, Tom Lane wrote:
>> The originally-complained-of breakage exists in all active branches,
>> so is it really OK to commit this only in HEAD?

> I did not think that it would be that critical for back-branches, but
> I don't mind going ahead and remove the code there as well.  Are there
> any objections with it?

> Also, wouldn't we want instead to apply on back-branches the first
> patch proposed on this thread which fixes the query generation for
> this pre-7.3 related code?

Given that we pushed out the bad code a year ago and nobody's complained,
I think it's safe to assume that no one is using any supported release
with a pre-7.3 server.

It's reasonable to doubt that this is the only problem the affected
applications would have with such a server, too.  I don't see a lot
of point in "fixing" this code unless somebody actually tests that.

			regards, tom lane

  14. Re: reindexdb & clusterdb broken against pre-7.3 servers

    Michael Paquier <michael@paquier.xyz> — 2019-05-07T03:39:13Z

    On Mon, May 06, 2019 at 11:24:31PM -0400, Tom Lane wrote:
> It's reasonable to doubt that this is the only problem the affected
> applications would have with such a server, too.  I don't see a lot
> of point in "fixing" this code unless somebody actually tests that.

Okay, point taken.  I'll go apply that to the back-branches as well.
--
Michael

  15. Re: reindexdb & clusterdb broken against pre-7.3 servers

    Michael Paquier <michael@paquier.xyz> — 2019-05-07T05:29:57Z

    On Tue, May 07, 2019 at 12:39:13PM +0900, Michael Paquier wrote:
> Okay, point taken.  I'll go apply that to the back-branches as well.

And done.
--
Michael