po 4. 9. 2023 v 14:24 odesÃlatel Jelte Fennema <postg...@jeltef.nl> napsal:
> On Sun, 3 Sept 2023 at 20:58, Pavel Stehule <pavel.steh...@gmail.com> > wrote: > > here is an try > > Overall it does what I had in mind. Below a few suggestions: > > +int > +PQprotocolSubversion(const PGconn *conn) > > Ugh, it's quite annoying that the original PQprotocolVersion only > returns the major version and thus we need this new function. It > seems like it would be much nicer if it returned a number similar to > PQserverVersion. I think it might be nicer to change PQprotocolVersion > to do that than to add another function. We could do: > > return PG_PROTOCOL_MAJOR(conn->pversion) * 100 + > PG_PROTOCOL_MINOR(conn->pversion); > > or even: > > if (PG_PROTOCOL_MAJOR(conn->pversion) == 3 && > PG_PROTOCOL_MINOR(conn->pversion)) > return 3; > else > return PG_PROTOCOL_MAJOR(conn->pversion) * 100 + > PG_PROTOCOL_MINOR(conn->pversion); > > The second option would be safest backwards compatibility wise, but in > practice you would only get another value than 3 (or 0) when > connecting to pre 7.4 servers. That seems old enough that I don't > think anyone is actually calling this function. **I'd like some > feedback from others on this though.** > Both versions look a little bit strange to me. I have not strong opinion about it, but I am not sure if is best to change contract after 20 years ago commit from Jun 2003 efc3a25bb02ada63158fe7006673518b005261ba I prefer to introduce a new function - it is ten lines of code. The form is not important - it can be a full number or minor number. It doesn't matter I think. But my opinion in this area is not strong, and I like to see feedback from others too. It is true that this feature and interface is not fully complete. Reards Pavel > + /* The protocol 3.0 is required */ > + if (PG_PROTOCOL_MAJOR(their_version) == 3) > + conn->pversion = their_version; > > Let's compare against the actual PG_PROTOCOL_EARLIEST and > PG_PROTOCOL_LATEST to determine if the version is supported or not. >
