>> I spent a little more time looking at this patch today. I think that the >> patch >> should actually send NegotiateProtocolVersion when *either* the requested >> version is differs from the latest one we support *or* an unsupported >> protocol >> option is present. Otherwise, you only find out about unsupported protocol >> options if you also request a newer minor version, which isn't good, because >> it >> makes it hard to add new protocol options *without* bumping the protocol >> version.
It makes sense from a maintainability point of view. >> Here's an updated version with that change and a proposed commit message. I have tested the new patch and it works great. The comments look good as well. Thanks, Badrul >> -----Original Message----- >> From: Robert Haas [mailto:robertmh...@gmail.com] >> Sent: Wednesday, November 15, 2017 1:12 PM >> To: Badrul Chowdhury <bac...@microsoft.com> >> Cc: Tom Lane <t...@sss.pgh.pa.us>; Satyanarayana Narlapuram >> <satyanarayana.narlapu...@microsoft.com>; Craig Ringer >> <cr...@2ndquadrant.com>; Peter Eisentraut <pete...@gmx.net>; Magnus >> Hagander <mag...@hagander.net>; PostgreSQL-development <pgsql- >> hack...@postgresql.org> >> Subject: Re: [HACKERS] Re: protocol version negotiation (Re: Libpq >> PGRES_COPY_BOTH - version compatibility) >> >> On Mon, Oct 30, 2017 at 9:19 PM, Badrul Chowdhury >> <bac...@microsoft.com> wrote: >> > Thank you for the comprehensive review! We are very much in the early >> stages of contributing to the PG community and we clearly have lots to learn, >> but we look forward to becoming proficient and active members of the pg >> community. >> > >> > Regarding the patch, I have tested it extensively by hand and it works >> > great. >> >> I spent a little more time looking at this patch today. I think that the >> patch >> should actually send NegotiateProtocolVersion when *either* the requested >> version is differs from the latest one we support *or* an unsupported >> protocol >> option is present. Otherwise, you only find out about unsupported protocol >> options if you also request a newer minor version, which isn't good, because >> it >> makes it hard to add new protocol options *without* bumping the protocol >> version. >> >> Here's an updated version with that change and a proposed commit message. >> >> -- >> Robert Haas >> EnterpriseDB: >> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.ent >> erprisedb.com&data=02%7C01%7Cbachow%40microsoft.com%7Ce37b69223 >> a144d1e5b7408d52c6d8171%7C72f988bf86f141af91ab2d7cd011db47%7C1 >> %7C0%7C636463771208784375&sdata=1%2FDylQIfS2rI2RwIVyZnDCUbzRQJe >> V4YM8J496QkpiQ%3D&reserved=0 >> The Enterprise PostgreSQL Company
