| From: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
|---|---|
| To: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
| Cc: | Andrew Dunstan <andrew(at)dunslane(dot)net>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Corey Huinker <corey(dot)huinker(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: bump minimum supported version of psql and pg_{dump,dumpall,upgrade} to v10 |
| Date: | 2026-06-30 20:18:51 |
| Message-ID: | akQkqy1P8vw-1qC9@nathan |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Tue, Jun 30, 2026 at 12:36:39PM +0900, Fujii Masao wrote:
> Thanks for updating the patches!
Appreciate the review.
> Regarding 0001 patch:
>
> Wouldn't it make sense to document the minimum supported server
> version also in the pg_dumpall docs?
Yes, I think so. However, since that's an additive change and not directly
related to $subject, I'd like to tackle it separately. Folks might have
preferences on the wording.
> /*
> * We allow the server to be back to 9.2, and up to any minor release of
> * our own major version. (See also version check in pg_dump.c.)
> */
> if (my_version != server_version_temp
> && (server_version_temp < 90200 ||
> (server_version_temp / 100) > (my_version / 100)))
>
> Shouldn't this be updated as well?
Oops, yes.
> Regarding 0002 patch:
>
> - if (GET_MAJOR_VERSION(old_cluster.major_version) < 902)
> + if (GET_MAJOR_VERSION(old_cluster.major_version) < 10)
>
> Shouldn't this be "< 1000" instead of "< 10"?
Yes.
> According to the following git grep results, there still seem to be
> several code paths for pre-v10 support. Should those also be updated or
> removed?
>
> $ git grep -E "GET_MAJOR_VERSION.* 9"
> src/bin/pg_upgrade/controldata.c: if
> (GET_MAJOR_VERSION(cluster->bin_version) <= 906)
> src/bin/pg_upgrade/controldata.c: if
> (GET_MAJOR_VERSION(cluster->major_version) <= 902)
> src/bin/pg_upgrade/controldata.c: else if
> (GET_MAJOR_VERSION(cluster->major_version) >= 906)
> src/bin/pg_upgrade/controldata.c: if
> (GET_MAJOR_VERSION(cluster->major_version) <= 902)
> src/bin/pg_upgrade/server.c:
> (GET_MAJOR_VERSION(cluster->major_version) <= 902) ?
Fixed. I thought I had handled these, but apparently they got lost
somewhere.
> # --swap can't be used to upgrade from versions older than 10, so just skip
> # the test if the old cluster version is too old.
> if ($old->pg_version < 10 && $mode eq "--swap")
>
> In pg_upgrade/t/006_transfer_modes.pl, shouldn't this simply be
> "if ($old->pg_version < 10)" so that the test is skipped whenever
> the old cluster is pre-v10?
I think we can remove it entirely. We don't block for <v9.2 today, so I
see no need to make it robust against invalid test setups.
> Regarding 0003 patch:
>
> /*
> * These object types were introduced later than our support cutoff of
> * server version 9.2. We use the VersionedQuery infrastructure so that
> * we don't send certain-to-fail queries to older servers.
> */
>
> In psql/tab-complete.in.c, shouldn't this comment be updated as well?
This also got lost along the way. Fixed.
> Overall, the following git grep results seem to suggest that there are several
> comments referring to pre-v10 support that may also need updating:
>
> $ git grep " 9\.[0-6]" | grep -E "pg_dump|pg_upgrade|psql" | grep -v "\.po:"
I didn't see anything else that I thought obviously needed to be changed.
Please let me know if you feel otherwise.
--
nathan
| Attachment | Content-Type | Size |
|---|---|---|
| v6-0001-pg_dump-pg_dumpall-bump-minimum-supported-version.patch | text/plain | 31.0 KB |
| v6-0002-pg_upgrade-bump-minimum-supported-version-to-v10.patch | text/plain | 42.8 KB |
| v6-0003-psql-bump-minimum-supported-version-to-v10.patch | text/plain | 24.6 KB |
| v6-0004-run-pgindent-and-pgperltidy.patch | text/plain | 65.2 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Dean Rasheed | 2026-06-30 20:21:27 | Re: Global temporary tables |
| Previous Message | Alexander Korotkov | 2026-06-30 19:38:00 | Re: Add SPLIT PARTITION/MERGE PARTITIONS commands |