Thanks for the comments Greg, please find my comments inline below. On Tue, Feb 9, 2021 at 2:27 PM Greg Nancarrow <gregn4...@gmail.com> wrote: > > On Mon, Feb 8, 2021 at 8:17 PM vignesh C <vignes...@gmail.com> wrote: > > > > > > > > I think what we want to do is mark default_transaction_read_only as > > > GUC_REPORT, instead. That will give a reliable report of what the > > > state of its GUC is, and you can combine it with is_hot_standby > > > to decide whether the session should be considered read-only. > > > If you don't get those two GUC values during connection, then you > > > can fall back on "SHOW transaction_read_only". > > > > > > > I have made a patch for the above with the changes suggested and > > rebased it with the head code. > > Attached v21 patch which has the changes for the same. > > Thoughts? > > Further to my other doc change feedback, I can only spot the following > minor things (otherwise the changes that you have made seek OK to me). > > 1) doc/src/sgml/protocol.sgml > > <varname>default_transaction_read_only</varname> and > <varname>in_hot_standby</varname> were not reported by releases before > 14.) > > should be: > > <varname>default_transaction_read_only</varname> and > <varname>in_hot_standby</varname> were not reported by releases before > 14.0) >
Modified. > 2) doc/src/sgml/high-availability,sgml > > <para> > During hot standby, the parameter <varname>in_hot_standby</varname> and > <varname>default_transaction_read_only</varname> are always true and may > not be changed. > > should be: > > <para> > During hot standby, the parameters <varname>in_hot_standby</varname> and > <varname>transaction_read_only</varname> are always true and may > not be changed. > > > [I believe that there's only checks on attempts to change > "transaction_read_only" when in hot_standby, not > "default_transaction_read_only"; see check_transaction_read_only()] > Modified. > 3) src/interfaces/libpq/fe-connect.c > > In rejectCheckedReadOrWriteConnection() and > rejectCheckedStandbyConnection(), now that host and port info are > emitted separately and are not included in each error message string > (as parameters in a format string), I think those functions should use > appendPQExpBufferStr() instead of appendPQExpBuffer(), as it's more > efficient if there is just a single string argument. > Modified. These comments are handled in v22 patch posted in my earlier mail. Regards, VIgnesh
