Re: doc: add missing "id" attributes to extension packaging page
> But why are there no anchors next to> items on that page? For example,> > how do I get the link for the> "Meta Commands" subsection?I can't look at the > code right now, but I suspect the headers are refsections (not sections) > which this patch does not add links for yet.I already have plans to add this > in a follow-up patch at some point, but while I had already added ids to all > section elements in the previous patch that added ids, this has yet to be > done for all refsect elements (wich is not a small effort again).Regards,Brar
Re: Add id's to various elements in protocol.sgml
On 24.02.2022 at 16:46, Alvaro Herrera wrote: On 2022-Feb-24, Dagfinn Ilmari Mannsåker wrote: Peter Eisentraut writes: Is there a way to obtain those URLs other than going into the HTML sources and checking if there is an anchor near where you want go? I use the jump-to-anchor extension: https://github.com/brettz9/jump-to-anchor/ Some sites have javascript that adds a link next to the element that becomes visible when hovering, e.g. the NAME and other headings on https://metacpan.org/pod/perl. Would it be possible to create such anchor links as part of the XSL stylesheets for HTML? Initially I thought that most use cases would involve developers who would be perfectly capable of extracting the id they need from the html sources but I agree that making that a bit more comfortable (especially given the fact that others do that too) seems worthwhile. I'll investiogate our options and report back.
Re: Add id's to various elements in protocol.sgml
On 24.02.2022 at 17:07, Brar Piening wrote: On 24.02.2022 at 16:46, Alvaro Herrera wrote: Would it be possible to create such anchor links as part of the XSL stylesheets for HTML? I'll investiogate our options and report back. Yes, that would be possible. In fact appending a link and optionally adding a tiny bit of CSS like I show below does the trick. The major problem in that regard would probably be my lack of XSLT/docbook skills but if no one can jump in here, I can see if I can make it work. Obviously adding the links via javascript would also work (and even be easier for me personally) but that seems like the second best solution to me since it involves javascript where no javasript is needed. Personally I consider having ids to link to and making them more comfortable to use/find as orthogonal problems in that case (mostly developer documentation) so IMHO solving this doesn't necessarily need to hold back the original patch. Insert # ... .variablelist a.anchor { visibility: hidden; } .variablelist *:hover > a.anchor, .variablelist a.anchor:focus { visibility: visible; }
Re: Add id's to various elements in protocol.sgml
On Feb 25, 2022 at 14:31, Peter Eisentraut wrote: I think that kind of stuff would be added in via the web site stylesheets, so you wouldn't have to deal with XSLT at all. True for the CSS but adding the HTML (#) will need either XSLT or Javascript.
Re: Add id's to various elements in protocol.sgml
On 28.02.2022 at 10:24, Peter Eisentraut wrote: On 28.02.22 09:41, Brar Piening wrote: On Feb 25, 2022 at 14:31, Peter Eisentraut wrote: I think that kind of stuff would be added in via the web site stylesheets, so you wouldn't have to deal with XSLT at all. True for the CSS but adding the HTML (#) will need either XSLT or Javascript. That is already done by your proposed patch, isn't it? No it isn't. My proposed patch performs the simple task of adding ids to the dt elements (e.g. ). This makes them usable as targets for links but they remain invisible to users of the docs who don't know about them, and unusable to users who don't know how to extract them from the HTML source code. The links (e.g. #) aren't added by the current XSLT transformations from Docbooc to HTML. Adding them would create a visible element (I propose a hash '#') next to the description term (inside the element after the text) that you can click on to put the link into the address bar, from where it can be copied for further usage.
Re: Add id's to various elements in protocol.sgml
On Feb 25, 2022 at 06:36, Brar Piening wrote: The major problem in that regard would probably be my lack of XSLT/docbook skills but if no one can jump in here, I can see if I can make it work. Ok, I've figured it out. Attached is an extended version of the patch that changes the XSL and CSS stylesheets to add links to the ids that are visible when hovering. diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml index 1c5ab00879..cb138b53ad 100644 --- a/doc/src/sgml/protocol.sgml +++ b/doc/src/sgml/protocol.sgml @@ -1810,7 +1810,7 @@ Replication commands are logged in the server log when The commands accepted in replication mode are: - + IDENTIFY_SYSTEM IDENTIFY_SYSTEM @@ -1875,7 +1875,7 @@ The commands accepted in replication mode are: - + SHOW name SHOW @@ -1899,7 +1899,7 @@ The commands accepted in replication mode are: - + TIMELINE_HISTORY tli TIMELINE_HISTORY @@ -2084,7 +2084,7 @@ The commands accepted in replication mode are: - + CREATE_REPLICATION_SLOT slot_name [ TEMPORARY ] { PHYSICAL [ RESERVE_WAL ] | LOGICAL output_plugin [ EXPORT_SNAPSHOT | NOEXPORT_SNAPSHOT | USE_SNAPSHOT | TWO_PHASE ] } @@ -2095,7 +2095,7 @@ The commands accepted in replication mode are: - + READ_REPLICATION_SLOT slot_name READ_REPLICATION_SLOT @@ -2143,7 +2143,7 @@ The commands accepted in replication mode are: - + START_REPLICATION [ SLOT slot_name ] [ PHYSICAL ] XXX/XXX [ TIMELINE tli ] START_REPLICATION @@ -2201,7 +2201,7 @@ The commands accepted in replication mode are: - + XLogData (B) @@ -2270,7 +2270,7 @@ The commands accepted in replication mode are: - + Primary keepalive message (B) @@ -2334,7 +2334,7 @@ The commands accepted in replication mode are: - + Standby status update (F) @@ -2415,7 +2415,7 @@ The commands accepted in replication mode are: - + Hot Standby feedback message (F) @@ -2497,7 +2497,7 @@ The commands accepted in replication mode are: - + START_REPLICATION SLOT slot_name LOGICAL XXX/XXX [ ( option_name [ option_value ] [, ...] ) ] @@ -2572,7 +2572,7 @@ The commands accepted in replication mode are: - + DROP_REPLICATION_SLOT slot_name WAIT DROP_REPLICATION_SLOT @@ -3266,7 +3266,7 @@ of any individual CopyData message cannot be interpretable on their own.) - + AuthenticationOk (B) @@ -3311,7 +3311,7 @@ AuthenticationOk (B) - + AuthenticationKerberosV5 (B) @@ -3355,7 +3355,7 @@ AuthenticationKerberosV5 (B) - + AuthenticationCleartextPassword (B) @@ -3399,7 +3399,7 @@ AuthenticationCleartextPassword (B) - + AuthenticationMD5Password (B) @@ -3454,7 +3454,7 @@ AuthenticationMD5Password (B) - + AuthenticationSCMCredential (B) @@ -3499,7 +3499,7 @@ AuthenticationSCMCredential (B) - + AuthenticationGSS (B) @@ -3544,7 +3544,7 @@ AuthenticationGSS (B) - + AuthenticationGSSContinue (B) @@ -3599,7 +3599,7 @@ AuthenticationGSSContinue (B) - + AuthenticationSSPI (B) @@ -3644,7 +3644,7 @@ AuthenticationSSPI (B) - + AuthenticationSASL (B) @@ -3705,7 +3705,7 @@ following: - + AuthenticationSASLContinue (B) @@ -3760,7 +3760,7 @@ AuthenticationSASLContinue (B) - + AuthenticationSASLFinal (B) @@ -3816,7 +3816,7 @@ AuthenticationSASLFinal (B) - + BackendKeyData (B) @@ -3873,7 +3873,7 @@ BackendKeyData (B) - + Bind (F) @@ -4026,7 +4026,7 @@ Bind (F) - + BindComplete (B) @@ -4061,7 +4061,7 @@ BindComplete (B) - + CancelRequest (F) @@ -4119,7 +4119,7 @@ CancelRequest (F) - + Close (F) @@ -4176,7 +4176,7 @@ Close (F) - + CloseComplete (B) @@ -4211,7 +4211,7 @@ CloseComplete (B) - + CommandComplete (B) @@ -4310,7 +4310,7 @@ CommandComplete (B) - + CopyData (F & B) @@ -4356,7 +4356,7 @@ CopyData (F & B) - + CopyDone (F & B) @@ -4391,7 +4391,7 @@ CopyDone (F & B) - + CopyFail (F) @@ -4436,7 +4436,7 @@ CopyFail (F) - + CopyInResponse (B) @@ -4512,7 +4512,7 @@ CopyInResponse (B) - + CopyOutResponse (B) @@ -4585,7 +4585,7 @@ CopyOutResponse (B) - + CopyBothResponse (B) @@ -4658,7 +4658,7 @@ CopyBothResponse (B) - + DataRow (B) @@ -4730,7 +4730,7 @@ DataRow (B) - + Describe (F) @@ -4787,7 +4787,7 @@ Describe (F) - + EmptyQueryResponse (B) @@ -4823,7 +4823,7 @@ EmptyQueryResponse (B) - + ErrorResponse (B) @@ -4889,7 +4889,7 @@ ErrorResponse (B) - + Execute (F) @@ -4946,7
Re: Add id's to various elements in protocol.sgml
On Feb 28, 2022 at 21:06, Chapman Flack wrote: I think that in other recent examples I've seen, there might be (something like a) consensus forming around the Unicode LINK SYMBOL 🔗 rather than # as the symbol for such things. I intentionally opted for an ASCII character as that definitely won't cause any display/font/portability issues but changing that is no problem. ... and now that the concept is proven, how hard would it be to broaden that template's pattern to apply to all the other DocBook constructs (such as section headings) that emit anchors? As long as we stick to manually assigned ids in the same way my patch does it, it shouldn't be too hard. Speaking of autogenerated ids, I failed to make use of them since I wasn't able to reproduce the same autogenerated id twice in order to use it for the link. Also I'm not sure how well the autogenerated ids are reproducible over time/versions/builds, and if it is wise to use them as targets to link to from somewhere else.
Re: Add id's to various elements in protocol.sgml
On Mar 01, 2022 at 18:27, Brar Piening wrote: On Feb 28, 2022 at 21:06, Chapman Flack wrote: I think that in other recent examples I've seen, there might be (something like a) consensus forming around the Unicode LINK SYMBOL 🔗 rather than # as the symbol for such things. I intentionally opted for an ASCII character as that definitely won't cause any display/font/portability issues but changing that is no problem. TBH I don't like the visual representation of the unicode link symbol (U+1F517) in my browser. It's a bold black fat thing that doesn't inherit colors. I've tried to soften it by decreasing the size but that doesn't really solve it for me. Font support also doesn't seem overwhelming. Anyway, I've changed my patch to use it so that you can judge it yourself. ... and now that the concept is proven, how hard would it be to broaden that template's pattern to apply to all the other DocBook constructs (such as section headings) that emit anchors? As long as we stick to manually assigned ids in the same way my patch does it, it shouldn't be too hard. Patch is attached. I don't think it should get applied this way, though. The fact that you only get links for section headers that have manually assigned ids would be pretty surprising for users of the docs and in some files (e.g. protocol-flow.html) only every other section has a manually assigned id. It would be easy to emit a message (or even fail) whenever the template fails to find an id and then manually assign ids until they are everywhere (currently that means all varlistentries and sections) but that would a) be quite some work and b) make the patch quite heavy, so I wouldn't even start this before there is really consensus that this is the right direction. diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml index 1c5ab00879..cb138b53ad 100644 --- a/doc/src/sgml/protocol.sgml +++ b/doc/src/sgml/protocol.sgml @@ -1810,7 +1810,7 @@ Replication commands are logged in the server log when The commands accepted in replication mode are: - + IDENTIFY_SYSTEM IDENTIFY_SYSTEM @@ -1875,7 +1875,7 @@ The commands accepted in replication mode are: - + SHOW name SHOW @@ -1899,7 +1899,7 @@ The commands accepted in replication mode are: - + TIMELINE_HISTORY tli TIMELINE_HISTORY @@ -2084,7 +2084,7 @@ The commands accepted in replication mode are: - + CREATE_REPLICATION_SLOT slot_name [ TEMPORARY ] { PHYSICAL [ RESERVE_WAL ] | LOGICAL output_plugin [ EXPORT_SNAPSHOT | NOEXPORT_SNAPSHOT | USE_SNAPSHOT | TWO_PHASE ] } @@ -2095,7 +2095,7 @@ The commands accepted in replication mode are: - + READ_REPLICATION_SLOT slot_name READ_REPLICATION_SLOT @@ -2143,7 +2143,7 @@ The commands accepted in replication mode are: - + START_REPLICATION [ SLOT slot_name ] [ PHYSICAL ] XXX/XXX [ TIMELINE tli ] START_REPLICATION @@ -2201,7 +2201,7 @@ The commands accepted in replication mode are: - + XLogData (B) @@ -2270,7 +2270,7 @@ The commands accepted in replication mode are: - + Primary keepalive message (B) @@ -2334,7 +2334,7 @@ The commands accepted in replication mode are: - + Standby status update (F) @@ -2415,7 +2415,7 @@ The commands accepted in replication mode are: - + Hot Standby feedback message (F) @@ -2497,7 +2497,7 @@ The commands accepted in replication mode are: - + START_REPLICATION SLOT slot_name LOGICAL XXX/XXX [ ( option_name [ option_value ] [, ...] ) ] @@ -2572,7 +2572,7 @@ The commands accepted in replication mode are: - + DROP_REPLICATION_SLOT slot_name WAIT DROP_REPLICATION_SLOT @@ -3266,7 +3266,7 @@ of any individual CopyData message cannot be interpretable on their own.) - + AuthenticationOk (B) @@ -3311,7 +3311,7 @@ AuthenticationOk (B) - + AuthenticationKerberosV5 (B) @@ -3355,7 +3355,7 @@ AuthenticationKerberosV5 (B) - + AuthenticationCleartextPassword (B) @@ -3399,7 +3399,7 @@ AuthenticationCleartextPassword (B) - + AuthenticationMD5Password (B) @@ -3454,7 +3454,7 @@ AuthenticationMD5Password (B) - + AuthenticationSCMCredential (B) @@ -3499,7 +3499,7 @@ AuthenticationSCMCredential (B) - + AuthenticationGSS (B) @@ -3544,7 +3544,7 @@ AuthenticationGSS (B) - + AuthenticationGSSContinue (B) @@ -3599,7 +3599,7 @@ AuthenticationGSSContinue (B) - + AuthenticationSSPI (B) @@ -3644,7 +3644,7 @@ AuthenticationSSPI (B) - + AuthenticationSASL (B) @@ -3705,7 +3705,7 @@ fo
Re: Add id's to various elements in protocol.sgml
On 02.03.2022 at 10:37, Peter Eisentraut wrote: I have applied the part of your patch that adds the id's. The discussion about the formatting aspect can continue. Thank you! I've generated some data by outputting the element name whenever a section or varlistentry lacks an id. That's how the situation in the docs currently looks like: element | count --+--- sect2 | 275 sect3 | 94 sect4 | 20 simplesect | 20 varlistentry | 3976 Looking at this, I think that manually assigning an id to all ~400 sections currently lacking one to make them referable in a consistent way is a bit of work but feasible. Once we consitently have stable ids on section headers IMHO it makes sense to also expose them as links. I'd probably also make the stylesheet emit a non-terminating message/comment whenever it finds a section without id in order to help keeping the layout consistent over time. With regard to varlistentry I'd suggest to decide whether to add ids or not on a case by case base. I already offered to add ids to long lists upon request but I wouldn't want to blindly add ~4k ids that nobody cares about. That part can also grow over time by people adding ids as they deem them useful. Any objections/thoughts?
Re: Add id's to various elements in protocol.sgml
On 02.03.2022 at 18:54, Chapman Flack wrote: Perhaps there are a bunch of variablelists where no one cares about linking to any of the entries. So maybe a useful non-terminating message to add eventually would be one that identifies any varlistentry lacking an id, with a variablelist where at least one other entry has an id. That sounds like areasonable approach for now. Is there anybody objecting to pursue this? Do you need more examples how it would look like? It would be a bit hurtful to generate a patch that manually adds ~600 ids just to have it rejected as unwanted.
Re: Add id's to various elements in protocol.sgml
On 09.03.2022 at 20:43, Brar Piening wrote: Attached is a pretty huge patch that adds ids to all sections and all the varlistentries where the containing variablelist already had at least one id (plus a few additional ones that I stumbled upon and deemed useful). It also adds html links next to the respective heading in the html documentation and emits a build message and a comment when a section or a relevant (see before) varlistentry doesn't have an id. I have uploaded a doc build with the patch applied to https://pgdocs.piening.info/ to make it easier for you all to review the results and see what is there and what isn't and how it feels UI-wise. You may want to look at https://pgdocs.piening.info/app-psql.html where the patch adds ids and links to all varlistentries but doesn't do so for the headings (because they are refsect1 headings not sect1 headings). https://pgdocs.piening.info/protocol-flow.html is pretty much the opposite. The patch adds ids and links to the headings (they are sect2 headings) but doesn't add them to the varlistentries (yet - because I mostly sticked to the algorithm suggested at https://www.postgresql.org/message-id/621FAF40.5070507%40anastigmatix.net to contain the workload).
Re: Minor documentation error regarding streaming replication protocol
Jeff Davis wrote: Attached a simple patch that enforces an all-ASCII restore point name rather than documenting the possibility of non-ASCII text. +1 This is probably the best solution because it gives guarantees to the client without causing compatibility issues with old clients. Thanks! Brar
Add id's to various elements in protocol.sgml
When working with the Frontend/Backend Protocol implementation in Npgsql and discussing things with the team, I often struggle with the fact that you can't set deep links to individual message formats in the somewhat lengthy html docs pages. The attached patch adds id's to various elements in protocol.sgml to make them more accesssible via the public html documentation interface. I've tried to follow the style that was already there in a few of the elements. Do you consider this useful and worth merging? Is there anything I can improve? Best Regards, Brar diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml index 34a7034282..e22dc5f3b0 100644 --- a/doc/src/sgml/protocol.sgml +++ b/doc/src/sgml/protocol.sgml @@ -1810,7 +1810,7 @@ Replication commands are logged in the server log when The commands accepted in replication mode are: - + IDENTIFY_SYSTEM IDENTIFY_SYSTEM @@ -1875,7 +1875,7 @@ The commands accepted in replication mode are: - + SHOW name SHOW @@ -1899,7 +1899,7 @@ The commands accepted in replication mode are: - + TIMELINE_HISTORY tli TIMELINE_HISTORY @@ -2084,7 +2084,7 @@ The commands accepted in replication mode are: - + CREATE_REPLICATION_SLOT slot_name [ TEMPORARY ] { PHYSICAL [ RESERVE_WAL ] | LOGICAL output_plugin [ EXPORT_SNAPSHOT | NOEXPORT_SNAPSHOT | USE_SNAPSHOT | TWO_PHASE ] } @@ -2095,7 +2095,7 @@ The commands accepted in replication mode are: - + READ_REPLICATION_SLOT slot_name READ_REPLICATION_SLOT @@ -2143,7 +2143,7 @@ The commands accepted in replication mode are: - + START_REPLICATION [ SLOT slot_name ] [ PHYSICAL ] XXX/XXX [ TIMELINE tli ] START_REPLICATION @@ -2201,7 +2201,7 @@ The commands accepted in replication mode are: - + XLogData (B) @@ -2270,7 +2270,7 @@ The commands accepted in replication mode are: - + Primary keepalive message (B) @@ -2334,7 +2334,7 @@ The commands accepted in replication mode are: - + Standby status update (F) @@ -2415,7 +2415,7 @@ The commands accepted in replication mode are: - + Hot Standby feedback message (F) @@ -2497,7 +2497,7 @@ The commands accepted in replication mode are: - + START_REPLICATION SLOT slot_name LOGICAL XXX/XXX [ ( option_name [ option_value ] [, ...] ) ] @@ -2572,7 +2572,7 @@ The commands accepted in replication mode are: - + DROP_REPLICATION_SLOT slot_name WAIT DROP_REPLICATION_SLOT @@ -2886,7 +2886,7 @@ The commands accepted in replication mode are: - + BASE_BACKUP [ LABEL 'label' ] [ PROGRESS ] [ FAST ] [ WAL ] [ NOWAIT ] [ MAX_RATE rate ] [ TABLESPACE_MAP ] [ NOVERIFY_CHECKSUMS ] [ MANIFEST manifest_option ] [ MANIFEST_CHECKSUMS checksum_algorithm ] @@ -3138,7 +3138,7 @@ of any individual CopyData message cannot be interpretable on their own.) - + AuthenticationOk (B) @@ -3183,7 +3183,7 @@ AuthenticationOk (B) - + AuthenticationKerberosV5 (B) @@ -3227,7 +3227,7 @@ AuthenticationKerberosV5 (B) - + AuthenticationCleartextPassword (B) @@ -3271,7 +3271,7 @@ AuthenticationCleartextPassword (B) - + AuthenticationMD5Password (B) @@ -3326,7 +3326,7 @@ AuthenticationMD5Password (B) - + AuthenticationSCMCredential (B) @@ -3371,7 +3371,7 @@ AuthenticationSCMCredential (B) - + AuthenticationGSS (B) @@ -3416,7 +3416,7 @@ AuthenticationGSS (B) - + AuthenticationGSSContinue (B) @@ -3471,7 +3471,7 @@ AuthenticationGSSContinue (B) - + AuthenticationSSPI (B) @@ -3516,7 +3516,7 @@ AuthenticationSSPI (B) - + AuthenticationSASL (B) @@ -3577,7 +3577,7 @@ following: - + AuthenticationSASLContinue (B) @@ -3632,7 +3632,7 @@ AuthenticationSASLContinue (B) - + AuthenticationSASLFinal (B) @@ -3688,7 +3688,7 @@ AuthenticationSASLFinal (B) - + BackendKeyData (B) @@ -3745,7 +3745,7 @@ BackendKeyData (B) - + Bind (F) @@ -3898,7 +3898,7 @@ Bind (F) - + BindComplete (B) @@ -3933,7 +3933,7 @@ BindComplete (B) - + CancelRequest (F) @@ -3991,7 +3991,7 @@ CancelRequest (F) - + Close (F) @@ -4048,7 +4048,7 @@ Close (F) - + CloseComplete (B) @@ -4083,7 +4083,7 @@ CloseComplete (B) - + CommandComplete (B) @@ -4182,7 +4182,7 @@ CommandComplete (B) - + CopyData (F & B) @@ -4228,7 +4228,7 @@ CopyData (F & B) - + CopyDone (F & B) @@ -4263,7 +4263,7 @@ CopyDone (F & B) - + CopyFail (F) @@ -4308,7 +4308,7 @@ CopyFail (F) - +
Re: Add id's to various elements in protocol.sgml
On Dec 14, 2021 at 20:47, Alvaro Herrera wrote: Hmm, I think we tend to avoid xreflabels; see https://www.postgresql.org/message-id/8315c0ca-7758-8823-fcb6-f37f9413e...@2ndquadrant.com Ok, thank you for the hint. I added them because doesn't automatically generate labels and they were present in the current docs for CREATE_REPLICATION_SLOT (https://github.com/postgres/postgres/blob/22bd3cbe0c284758d7174321f5596763095cdd55/doc/src/sgml/protocol.sgml#L1944). After reading the aforementioned thread to https://www.postgresql.org/message-id/20200611223836.GA2507%40momjian.us I infer the following conclusions: a) Do *not* include xreflabel for elements that get numbered. b) There should be some general utility for the xreflabel, not just the linking needs of one particular source location. c) Generally, xreflabels are a bit of antipattern, so there need to be solid arguments in favor of adding more. Since I can't argue towards some general utility for the xreflabels and don't have any other solid argument in favor of adding more, I will remove them from my current patch but leave the existing ones intact. Objections?
Re: Add id's to various elements in protocol.sgml
On Dec 15, 2021 at 15:49, Alvaro Herrera wrote: On 2021-Dec-15, Brar Piening wrote: Since I can't argue towards some general utility for the xreflabels and don't have any other solid argument in favor of adding more, I will remove them from my current patch but leave the existing ones intact. Yeah, I think not adding them until we have a use for them might be wisest. A new version of the patch that doesn't add xreflabels is attached. Thanks for your support. diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml index 34a7034282..0bca5831b1 100644 --- a/doc/src/sgml/protocol.sgml +++ b/doc/src/sgml/protocol.sgml @@ -1810,7 +1810,7 @@ Replication commands are logged in the server log when The commands accepted in replication mode are: - + IDENTIFY_SYSTEM IDENTIFY_SYSTEM @@ -1875,7 +1875,7 @@ The commands accepted in replication mode are: - + SHOW name SHOW @@ -1899,7 +1899,7 @@ The commands accepted in replication mode are: - + TIMELINE_HISTORY tli TIMELINE_HISTORY @@ -2084,7 +2084,7 @@ The commands accepted in replication mode are: - + CREATE_REPLICATION_SLOT slot_name [ TEMPORARY ] { PHYSICAL [ RESERVE_WAL ] | LOGICAL output_plugin [ EXPORT_SNAPSHOT | NOEXPORT_SNAPSHOT | USE_SNAPSHOT | TWO_PHASE ] } @@ -2095,7 +2095,7 @@ The commands accepted in replication mode are: - + READ_REPLICATION_SLOT slot_name READ_REPLICATION_SLOT @@ -2143,7 +2143,7 @@ The commands accepted in replication mode are: - + START_REPLICATION [ SLOT slot_name ] [ PHYSICAL ] XXX/XXX [ TIMELINE tli ] START_REPLICATION @@ -2201,7 +2201,7 @@ The commands accepted in replication mode are: - + XLogData (B) @@ -2270,7 +2270,7 @@ The commands accepted in replication mode are: - + Primary keepalive message (B) @@ -2334,7 +2334,7 @@ The commands accepted in replication mode are: - + Standby status update (F) @@ -2415,7 +2415,7 @@ The commands accepted in replication mode are: - + Hot Standby feedback message (F) @@ -2497,7 +2497,7 @@ The commands accepted in replication mode are: - + START_REPLICATION SLOT slot_name LOGICAL XXX/XXX [ ( option_name [ option_value ] [, ...] ) ] @@ -2572,7 +2572,7 @@ The commands accepted in replication mode are: - + DROP_REPLICATION_SLOT slot_name WAIT DROP_REPLICATION_SLOT @@ -2886,7 +2886,7 @@ The commands accepted in replication mode are: - + BASE_BACKUP [ LABEL 'label' ] [ PROGRESS ] [ FAST ] [ WAL ] [ NOWAIT ] [ MAX_RATE rate ] [ TABLESPACE_MAP ] [ NOVERIFY_CHECKSUMS ] [ MANIFEST manifest_option ] [ MANIFEST_CHECKSUMS checksum_algorithm ] @@ -3138,7 +3138,7 @@ of any individual CopyData message cannot be interpretable on their own.) - + AuthenticationOk (B) @@ -3183,7 +3183,7 @@ AuthenticationOk (B) - + AuthenticationKerberosV5 (B) @@ -3227,7 +3227,7 @@ AuthenticationKerberosV5 (B) - + AuthenticationCleartextPassword (B) @@ -3271,7 +3271,7 @@ AuthenticationCleartextPassword (B) - + AuthenticationMD5Password (B) @@ -3326,7 +3326,7 @@ AuthenticationMD5Password (B) - + AuthenticationSCMCredential (B) @@ -3371,7 +3371,7 @@ AuthenticationSCMCredential (B) - + AuthenticationGSS (B) @@ -3416,7 +3416,7 @@ AuthenticationGSS (B) - + AuthenticationGSSContinue (B) @@ -3471,7 +3471,7 @@ AuthenticationGSSContinue (B) - + AuthenticationSSPI (B) @@ -3516,7 +3516,7 @@ AuthenticationSSPI (B) - + AuthenticationSASL (B) @@ -3577,7 +3577,7 @@ following: - + AuthenticationSASLContinue (B) @@ -3632,7 +3632,7 @@ AuthenticationSASLContinue (B) - + AuthenticationSASLFinal (B) @@ -3688,7 +3688,7 @@ AuthenticationSASLFinal (B) - + BackendKeyData (B) @@ -3745,7 +3745,7 @@ BackendKeyData (B) - + Bind (F) @@ -3898,7 +3898,7 @@ Bind (F) - + BindComplete (B) @@ -3933,7 +3933,7 @@ BindComplete (B) - + CancelRequest (F) @@ -3991,7 +3991,7 @@ CancelRequest (F) - + Close (F) @@ -4048,7 +4048,7 @@ Close (F) - + CloseComplete (B) @@ -4083,7 +4083,7 @@ CloseComplete (B) - + CommandComplete (B) @@ -4182,7 +4182,7 @@ CommandComplete (B) - + CopyData (F & B) @@ -4228,7 +4228,7 @@ CopyData (F & B) - + CopyDone (F & B) @@ -4263,7 +4263,7 @@ CopyDone (F & B) - + CopyFail (F) @@ -4308,7 +4308,7 @@ CopyFail (F) - + CopyInResponse (B) @@ -4384,7 +4384,7 @@ CopyInResponse (B) - + CopyOutResponse (B)
Re: Add id's to various elements in protocol.sgml
On Dec 17, 2021 at 08:13, Peter Eisentraut wrote: On 15.12.21 16:59, Brar Piening wrote: On Dec 15, 2021 at 15:49, Alvaro Herrera wrote: On 2021-Dec-15, Brar Piening wrote: Since I can't argue towards some general utility for the xreflabels and don't have any other solid argument in favor of adding more, I will remove them from my current patch but leave the existing ones intact. Yeah, I think not adding them until we have a use for them might be wisest. A new version of the patch that doesn't add xreflabels is attached. Now this patch adds a bunch of ids, but you can't use them to link to, because as soon as you do, you will get complaints about a missing xreflabel. So what is the remaining purpose? The purpose is that you can directly link to the id in the public html docs which still gets generated (e. g. https://www.postgresql.org/docs/14/protocol-replication.html#PROTOCOL-REPLICATION-BASE-BACKUP). Essentially it gives people discussing the protocol and pointing to a certain command or message format the chance to link to the very thing they are discussing instead of the top of the lengthy html page.
Re: Add id's to various elements in protocol.sgml
On 20.12.2021 at 16:09, Robert Haas wrote: As a data point, this is something I have also wanted to do, from time to time. I am generally of the opinion that any place the documentation has a long list of things, which should add ids, so that people can link to the particular thing in the list to which they want to draw someone's attention. Thank you. If there is consensus on generally adding links to long lists I'd take suggestions for other places where people think that this would make sense and amend my patch.
Re: doc: add missing "id" attributes to extension packaging page
On 20.03.2023 at 19:47, Gregory Stark (as CFM) wrote: Looks like a lot of good work was happening on this patch right up until mid-February. Is there a lot of work left? Do you think you'll have a chance to wrap it up this commitfest for this release? Thanks for the ping. I had another look this morning and I think I can probably finish this by the end of the week.
Re: doc: add missing "id" attributes to extension packaging page
On 17.01.2023 at 23:43, Karl O. Pinc wrote: It's good you asked. After poking about the XSLT 1.0 spec to refresh my memory I abandoned the "mode" approach entirely. The real "right way" is to use the import mechanism. It actually is not. After quite some time trying to figure out why things don't work as intended, I ended up reading the XSLT 1.0 spec. As the name already suggests, is closely related to with the difference that it *applies* a *template rule* from an imported style sheet instead of applying a template rule from the current style sheet (https://www.w3.org/TR/1999/REC-xslt-19991116#apply-imports). What it does not do is *calling* a *named template* (https://www.w3.org/TR/1999/REC-xslt-19991116#named-templates). What this basically means is that in XSLT 1.0 you can use to override template rules () but you cannot use it to override named templates (). If you want to override named templates you basically have to duplicate and change them. While there are mechanisms to call overriden named templates in XSLT 3, this is out of scope here, since we're bound to XSLT 1.0 As a consequence, there was little I could change in the initial patch to avoid the code duplication and all attempts to do so, ultimately led to even longer and more complex code without really reducing the amount of duplication. The approach actually does work in the varlistentry case, although this doesn't really change a lot regarding the length of the patch (it's a bit nicer though since in this case it really avoids duplication). I've also taken the advice to terminate the build and print the xpath if a required id is missing. The attached patch is my best-effort approach to implement discoverable links. Best regards, Brar diff --git a/doc/src/sgml/pgwalinspect.sgml b/doc/src/sgml/pgwalinspect.sgml index 62d9f9eb22..9a0241a8d6 100644 --- a/doc/src/sgml/pgwalinspect.sgml +++ b/doc/src/sgml/pgwalinspect.sgml @@ -157,7 +157,7 @@ combined_size_percentage | 2.8634072910530795 - + pg_get_wal_block_info(start_lsn pg_lsn, end_lsn pg_lsn) returns setof record diff --git a/doc/src/sgml/stylesheet-html-common.xsl b/doc/src/sgml/stylesheet-html-common.xsl index 3f85ea9536..9df2782ce4 100644 --- a/doc/src/sgml/stylesheet-html-common.xsl +++ b/doc/src/sgml/stylesheet-html-common.xsl @@ -301,120 +301,4 @@ set toc,title - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 6 - - - - - - http://www.w3.org/1999/xhtml";> - - - -clear: both - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - # - - - - - - id_link - - # - - - - - - - A - - element at path ' - -/ - - - [@ - - = ' - - '] - - - ' is missing an id. Please add one to make it usable - as stable anchor in the public HTML documentation. - - - - - - diff --git a/doc/src/sgml/stylesheet.css b/doc/src/sgml/stylesheet.css index 15bcc95d41..cc14efa1ca 100644 --- a/doc/src/sgml/stylesheet.css +++ b/doc/src/sgml/stylesheet.css @@ -169,13 +169,3 @@ acronym{ font-style: inherit; } width: 75%; } } - -/* Links to ids of headers and definition terms */ -a.id_link { -color: inherit; -visibility: hidden; -} - -*:hover > a.id_link { -visibility: visible; -}
Re: doc: add missing "id" attributes to extension packaging page
On 23.03.2023 at 04:09, Karl O. Pinc wrote: You're quite right. I clearly didn't have my XSLT turned on. Importing only works when templates are matched, not called by name. Sorry for the extra work I've put you through. No problem. As always I've learnt something which may help me in the future. You've put in a lot of good work. I'm attaching 2 patches with only minor changes. Thanks. When comparing things I also realized that I had accidentally created a reversed patch. Thanks for fixing this. 001-add-needed-ids_v1.patch This separates out the addition of ids from the XSLT changes, just to keep things tidy. Content is from your patch. +1 002-make_html_ids_discoverable_v4.patch I changed the linked text, the #, so that the leading space is not linked. This is arguable, as the extra space makes it easier to put the mouse on the region. But it seems tidy. I tend to prefer a slightly bigger mouseover-region but I don't really mind. I've tided up so the lines are no longer than 80 chars. +1 This looks awesome. I love the xpath! I've changed the format of the error message. What do you think? (Try it out by _not_ applying 001-add-needed-ids_v1.patch.) Also, the error message now has leading and trailing newlines to make it stand out. I'm normally against this sort of thing but thought I'd add it anyway for others to review. +1 I'm ready to send these on to a committer but if you don't like what I did please send more patches for me to review. I like it and think it's ready for commiter. Outstanding questions (for committer?): The 002-make_html_ids_discoverable_v4.patch generates xhtml , , etc. attributes using a XSLT element with a "namespace" attribute. I'm not sure I follow. I cannot see any namespacing weirdness in my output. Are you using the v1.79.2 styleshhets? What character should be used to represent a link anchor? It's not the first time this is coming up. See my response in the old thread: https://www.postgresql.org/message-id/e50193ea-ca5c-e178-026a-f3fd8942252d%40gmx.de Personally I'd advise to stick with ASCII for now. In any case changing the symbol at some point would be a very minor effort if we deem it necessary. Maybe this could be part of some general overhaul of the visual apperance and website styling by a person with more talent for this than I have. Regards, Brar
Re: doc: add missing "id" attributes to extension packaging page
On 23.03.2023 at 10:35, Alvaro Herrera wrote: As with the patch, we'll need to patch the CSS used in the website for the docs too, as that's the most important place where docs are visited. See this commit for an example: https://git.postgresql.org/gitweb/?p=pgweb.git;a=commitdiff;h=0b89ea0fff28d29ed177c82a274144453e3c7f82 In order to test locally that your patched stylesheet works correctly, you'd have to compile the docs with "make html STYLE=website" in the doc subdir, and tweak one of the CSS files there (I think it's docs-complete.css) so that it references your local copy instead of fetching it from the website. Thanks I'll take care of this tonight. I'm not clear on what exactly becomes visible when one hovers over what. Can you please share a screenshot? I could, but since hover effects don't really come across in screenshots I've posted a build of the docs including the patch to https://pgdocs.piening.info See https://pgdocs.piening.info/app-psql.html as an example.
Re: doc: add missing "id" attributes to extension packaging page
On 23.03.2023 at 10:35, Alvaro Herrera wrote: As with the patch, we'll need to patch the CSS used in the website for the docs too, as that's the most important place where docs are visited. Ok, I've created and tested a patch for this too. Since the need for ids is starting to grow again (ecb696527c added an id to a varlistentry in doc/src/sgml/ref/create_subscription.sgml) I've also amended the add-needed-ids patch once again so that the build does not fail after applying the make_html_ids_discoverable patch. I've also attached the (unchanged) make_html_ids_discoverable patch for convenience so this email now contains two patches for postgresql (ending with .postgresql.patch) and one patch for pgweb (ending with .pgweb.patch). TBH I'm a bit afraid that people will immediately start complaining about the failing docs builds after this got applied since it forces them to add ids to all varlistenries in a variablelist if they add one, which can be perceived as quite a burden (also committers and reviewers will have to get used to always watch out for failing docs builds because of this). Since breaking the build on missing ids was an intentional decision we can theoretically soften this by only issuing a warning or removing the check for missing id's altogether but this would probably defeat the purpose since it would lead to an increasing number of entries that lack an id after a while. Regards, Brar diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml index 3836d13ad3..1432c67c2a 100644 --- a/doc/src/sgml/logical-replication.sgml +++ b/doc/src/sgml/logical-replication.sgml @@ -252,7 +252,7 @@ additional columns not provided by the published table. Any such columns will be filled with the default value as specified in the definition of the target table. However, logical replication in binary format is more - restrictive. See the binary + restrictive. See the binary option of CREATE SUBSCRIPTION for details. diff --git a/doc/src/sgml/pgwalinspect.sgml b/doc/src/sgml/pgwalinspect.sgml index 9a0241a8d6..62d9f9eb22 100644 --- a/doc/src/sgml/pgwalinspect.sgml +++ b/doc/src/sgml/pgwalinspect.sgml @@ -157,7 +157,7 @@ combined_size_percentage | 2.8634072910530795 - + pg_get_wal_block_info(start_lsn pg_lsn, end_lsn pg_lsn) returns setof record diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml index e92346edef..f0ab6a918e 100644 --- a/doc/src/sgml/ref/alter_subscription.sgml +++ b/doc/src/sgml/ref/alter_subscription.sgml @@ -178,7 +178,7 @@ ALTER SUBSCRIPTION name RENAME TO < origin parameter. - See the binary + See the binary option of CREATE SUBSCRIPTION for details about copying pre-existing data in binary format. diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml index 9d4b9d4e33..605b11bc67 100644 --- a/doc/src/sgml/ref/create_subscription.sgml +++ b/doc/src/sgml/ref/create_subscription.sgml @@ -61,7 +61,7 @@ CREATE SUBSCRIPTION subscription_nameParameters - + subscription_name @@ -70,7 +70,7 @@ CREATE SUBSCRIPTION subscription_name - + CONNECTION 'conninfo' @@ -81,7 +81,7 @@ CREATE SUBSCRIPTION subscription_name - + PUBLICATION publication_name [, ...] @@ -90,7 +90,7 @@ CREATE SUBSCRIPTION subscription_name - + WITH ( subscription_parameter [= value] [, ... ] ) @@ -102,7 +102,7 @@ CREATE SUBSCRIPTION subscription_name - + connect (boolean) @@ -129,7 +129,7 @@ CREATE SUBSCRIPTION subscription_name - + create_slot (boolean) @@ -145,7 +145,7 @@ CREATE SUBSCRIPTION subscription_name - + enabled (boolean) @@ -156,7 +156,7 @@ CREATE SUBSCRIPTION subscription_name - + slot_name (string) @@ -185,7 +185,7 @@ CREATE SUBSCRIPTION subscription_name - + binary (boolean) @@ -222,7 +222,7 @@ CREATE SUBSCRIPTION subscription_name - + copy_data (boolean) @@ -243,7 +243,7 @@ CREATE SUBSCRIPTION subscription_name - + streaming (enum) @@ -271,7 +271,7 @@ CREATE SUBSCRIPTION subscription_name - + synchronous_commit (enum) @@ -303,7 +303,7 @@ CREATE SUBSCRIPTION subscription_name - + two_phase (boolean) @@ -334,7 +334,7 @@ CREATE SUBSCRIPTION subscription_name -
Re: doc: add missing "id" attributes to extension packaging page
On 23.03.2023 at 23:31, Karl O. Pinc wrote: Hi Brar, It occurs to me that I had not actually tested the way the anchor is put only after the last term in a varlistentry. (The code looked obviously right and should work if any varlistentry terms have anchors, which they do, but) Have you tested this? Yes, I have. See https://pgdocs.piening.info/app-psql.html#APP-PSQL-META-COMMAND-DE for an extreme case.
Re: doc: add missing "id" attributes to extension packaging page
On 24.03.2023 at 05:09, Karl O. Pinc wrote: Hi Brar, An observation: The # that shows up when hovering over section-level headings is styled as the section-level heading is. But the # that shows up when hovering over varlistentrys has the default text style. This works for me. It's nice to have the "section #"s look like the section heading. But the varlistentry's terms are smaller than the normal font, and their line width is less heavy than normal. I'm not really invested one way or the other, but I find it kind of nice that the varlistentry's #s are easier to click on and more noticable because they're slightly larger than might be expected. TBH I didn't bother a lot with this. Most of the time it's actually not the font size but rather the font-family which gets inherited from the parent element if you don't set it explicitly. The link just inherits everithing (including the color, which I have set to inherit explicitly since links don't inherit the parent's color by default) from it's parent, which is the HTML element (ultimately the inheritance probably goes up to the element style in pretty much all cases). In some instances the input element contains elements that are styled differently in the output (e.g.: which translates to HTML which has "font-family: monospace;") which makes the # from the link appear differently than the visible element it appears after. Since (after tweaking the color) the general visual appearence looked ok to me, I didn't bother with this any further. Regards, Brar
Re: doc: add missing "id" attributes to extension packaging page
On 24.03.2023 at 06:48, Brar Piening wrote: Since (after tweaking the color) the general visual appearence looked ok to me, I didn't bother with this any further. Also, if we go on with this we'll probably end up in an almost prototypical bikeshedding scenario where PostgreSQL itself is the nuclear power plant and the visual appearence of the hover links on the documentation website is the color of the bikeshed. ;-)
Re: doc: add missing "id" attributes to extension packaging page
On 02.01.2023 at 21:53, Karl O. Pinc wrote: If the author will look over my version of the patch I believe it can be approved and sent on to the committers. LGTM. Thanks! Brar
Re: doc: add missing "id" attributes to extension packaging page
On 09.01.2023 at 03:38, vignesh C wrote: There are couple of commitfest entries for this: https://commitfest.postgresql.org/41/4041/ https://commitfest.postgresql.org/41/4042/ Can one of them be closed? I've split the initial patch into two parts upon Álvaro's request in [1] so that we can discuss them separately https://commitfest.postgresql.org/41/4041/ is tracking the patch you've been trying to apply and that I've just sent a rebased version for. It only adds (invisible) ids to the HTML documentation and can be closed once you've applied the patch. https://commitfest.postgresql.org/41/4042/ is tracking a different patch that makes the ids and the corresponding links discoverable at the HTML surface. Hover one of the psql options in [2] to see the behavior. This one still needs reviewing and there is no discussion around it yet. Regards, Brar [1] https://www.postgresql.org/message-id/20221206083809.3kaygnh2xswoxslj%40alvherre.pgsql [2] https://pgdocs.piening.info/app-psql.html#APP-PSQL-OPTION-PORT
Re: doc: add missing "id" attributes to extension packaging page
On 09.01.2023 at 21:18, Tom Lane wrote: It's not great to have multiple CF entries pointing at the same email thread --- it confuses both people and bots. Next time please split off a thread for each distinct patch. I agree. I had overestimated the cfbot's ability to handle branched threads. I'll create separate threads next time. * AFAIK our practice is to use "-" never "_" in XML ID attributes. You weren't very consistent about that even within this patch, and the overall effect would have been to have no standard about that at all, which doesn't seem great. I changed them all to "-". Noted. Maybe it's worth to write a short paragraph about Ids and their style somewhere in the docs (e.g. Appendix J.5). * I got rid of a couple of "-et-al" additions, because it did not seem like a good precedent. That would tempt people to modify existing ID tags when adding variables to an entry, which'd defeat the purpose I think. I tried to use it sparsely, mostly where a varlistentry had multiple child items and I had arbitrarily pick one of them. It's not important, though. I'm curious how you solved this. * I fixed a couple of things that looked like typos or unnecessary inconsistencies. I have to admit that my eyes glazed over after awhile, so there might be remaining infelicities. I'm all for consistency. The only places where I intentionally refrained from being consistent was where I felt Ids would get too long or where there were already ids in place that didn't match my naming scheme. It's probably going to be necessary to have follow-on patches, because I'm sure there is stuff in the pipeline that adds more ID-less tags. Or do we have a way to create warnings about that? Agreed. And yes, we do have a limited way to create warnings (that's part of the other patch). I'm unqualified to review CSS stuff, so you'll need to get somebody else to review that patch. But I'd suggest reposting it, else the cfbot is going to start whining that the patch-of-record in this thread no longer applies. I will do that. Thanks for your feedback! Regards, Brar
Re: doc: add missing "id" attributes to extension packaging page
On 09.01.2023 at 23:28, Karl O. Pinc wrote: On Mon, 09 Jan 2023 15:18:18 -0500 Tom Lane wrote: It's probably going to be necessary to have follow-on patches, because I'm sure there is stuff in the pipeline that adds more ID-less tags. Or do we have a way to create warnings about that? I am unclear on how to make warnings with xslt. You can make a listing of problems, but who would read it if the build completed successfully? You can make errors and abort. You can emit warnings to the command line or you can abort with an error. I've opted for warnings + comments in the output in the styling patch. The biggest issue about errors and warnings is the fact that xslt does not process files in a line-based way which makes it pretty much impossible to give hints where the problem causing the warning is located. Since everything is bound together via XML entities, you can't even tell the source file. I've worked around this by also emitting an HTML comment to the output so that I can find a somewhat unique string next to it and the grep the documentation sources for this string. It's a bit ugly but the best I could come up with. I'll repost a rebased version of the styling patch in a minute. Regards, Brar
Re: doc: add missing "id" attributes to extension packaging page
On 10.01.2023 at 06:28, Brar Piening wrote: I'll repost a rebased version of the styling patch in a minute. After checking that there's no need for rebasing I'm reposting the original patch here, to make cfbot pick it up as the latest one in a somewhat screwed up thread mixing two patches (sorry for that - won't happen again). Althoug the patch is pretty compact you probably need some understanding of both XSLT and CSS to understand and judge the changes it introduces. It pretty much does two things: 1. Make html ids discoverable in the browser by adding a link with a hover effect to items sections and varlistentries that have an id. Hover one of the psql options and click on the hash mark in [1] to see the behavior. 2. Emit a warning to the command line and a comment to the HTML output when the docs build runs into a section without id or a varlistentry without id where at least one entry in the varlist already has an id. The original mail for the patch is at [2], the commitfest entry is at [3] and the initial discussion leading to this patch starts at [4]. Regards, Brar [1] https://pgdocs.piening.info/app-psql.html#APP-PSQL-OPTION-PORT [2] https://www.postgresql.org/message-id/d6695820-af71-5e84-58b0-ff9f1c189603%40gmx.de [3] https://commitfest.postgresql.org/41/4042/ [4] https://www.postgresql.org/message-id/4364ab38-a475-a1fc-b104-ecd6c72010d0%40enterprisedb.com diff --git a/doc/src/sgml/stylesheet-html-common.xsl b/doc/src/sgml/stylesheet-html-common.xsl index 9df2782ce4..111b03d6fc 100644 --- a/doc/src/sgml/stylesheet-html-common.xsl +++ b/doc/src/sgml/stylesheet-html-common.xsl @@ -301,4 +301,115 @@ set toc,title + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 6 + + + + + + http://www.w3.org/1999/xhtml";> + + + +clear: both + + + + + + + + + + + + + + + + + + + + + + + # + + + + + + id_link + + # + + + + + + + + element without id. Please add an id to make it usable + as stable anchor in the public HTML documentation. + + + Please add an id to the + + element in the sgml source code. + + + + + + diff --git a/doc/src/sgml/stylesheet.css b/doc/src/sgml/stylesheet.css index 6410a47958..e4174e0613 100644 --- a/doc/src/sgml/stylesheet.css +++ b/doc/src/sgml/stylesheet.css @@ -163,3 +163,13 @@ acronym{ font-style: inherit; } width: 75%; } } + +/* Links to ids of headers and definition terms */ +a.id_link { +color: inherit; +visibility: hidden; +} + +*:hover > a.id_link { +visibility: visible; +}
Re: pgsql: Doc: add XML ID attributes to and tags.
On 12.01.2023 at 00:05, Tom Lane wrote: Peter Eisentraut writes: Any reason the new ids in create_database.sgml deviate from the normal naming schemes used everywhere else? Is it to preserve the existing create-database-strategy? Maybe we should rename that one and make the new ones consistent? I don't remember every single choice I made but the general goal was to at least stay consistent within a varlist/[sub]section/file/chapter and *never* change pre-existing ids since somebody may already be pointing to them. After all it is just an identifier that is supposed to be unique and should not hurt our aesthetic feelings too much. The consistency is mostly because we tend to like it and maybe also to avoid collisions when making up new ids but I doubt that anybody will ever try to remember an id or infer one form knowledge about the thing it should be pointing at. I consider it a pretty opaque string that is meant for copy-paste from a browser to some editing window. It is all in our head and as a matter of fact we could be using UUIDs as Ids and save us from any further consistency issues. It's just that they look so ugly. You'd have to ask Brar that, I didn't question his choices too much. I have no objection to changing things as you suggest.I'm hesitant to rename very many pre-existing IDs for fear of breaking peoples' bookmarks, but changing create-database-strategy doesn't seem like a big deal. Personally I'd only d this for ids that haven't been "released" as official documentation (even as "devel" since the new things tend to attract more discussions and probably linking). I very much consider URLs as UI and go long ways to keep them consistent (HTTP 3xx is a friend of mine) as you never know who might be pointing at them from where and making them a moving target defeats their purpose and probably hurt more than some inconsistency. Regards, Brar
Re: Doc: Rework contrib appendix -- informative titles, tweaked sentences
On 03.01.2023 at 01:00, Karl O. Pinc wrote: Attached is a patch: contrib_v1.patch It modifies Appendix F, the contrib directory. Review: The patch applies cleanly (1334b79a35 - 2023-01-14 18:05:09 +0900). It adds a brief explanatory part to the headers of all contrib modules which I consider as very useful, especially when looking at the TOC in contrib.html where currently newcomers would need to click through all the links to even get an idea what the various modules do. The explanatory parts added make sense to me, althogh I'm not an expert in all the different contrib modules. Appendix F. now reads as "Additional Supplied Modules and Extensions" instead of "Appendix F. Additional Supplied Modules" which IMHO proprely reflects what it is about. The original title probably comes from the pre-extension-era. There is also some minor rewording of sentences in contrib.sgml that in general looks like an improvment to me. In conclusion I cannot see why this patch should not be applied in it's current form so I deem it ready for commiter. Regards, Brar
Re: doc: add missing "id" attributes to extension packaging page
On 17.01.2023 at 02:05, Karl O. Pinc wrote: Or maybe the right way is to set a mode at the very top, the first apply-templates call, and not mess with the built-in templates at all. (You'd write your own "postgres-mode" templates the same way, to "wrap" and call the default templates.) Think of the mode as an implicit argument that's preserved and passed down through each template invocation without having to be explicitly specified by the calling code. I think the document you're missing is [1]. There are multiple ways to customize DocBook XSL output and it sounds like you want me to write a customization layer which I didn't do because there is precedent that the typical "way to do it" (TM) in the PostgreSQL project is [2]. Regards, Brar [1] http://www.sagehill.net/docbookxsl/CustomizingPart.html [2] http://www.sagehill.net/docbookxsl/ReplaceTemplate.html
Re: doc: add missing "id" attributes to extension packaging page
On 17.01.2023 at 14:12, Karl O. Pinc wrote: If you're not willing to try I am willing to see if I can produce an example to work from. My XSLT is starting to come back. I'm certainly willing to try but I'd appreciate an example in any case. My XSLT skills are mostly learning by doing combined with trial and error. Regards, Brar
Re: doc: add missing "id" attributes to extension packaging page
On 17.01.2023 at 23:43, Karl O. Pinc wrote: It's good you asked. After poking about the XSLT 1.0 spec to refresh my memory I abandoned the "mode" approach entirely. The real "right way" is to use the import mechanism. I've attached a patch that "wraps" the section.heading template and adds extra stuff to the HTML output generated by the stock template. (example_section_heading_override.patch) Thanks! I'll give it a proper look this weekend. Regards, Brar
Re: doc: add missing "id" attributes to extension packaging page
On 24.03.2023 at 10:45, Alvaro Herrera wrote: But why are there no anchors next to items on that page? For example, how do I get the link for the "Meta Commands" subsection? I somehow knew that responding from a crappy mobile phone e-mail client will mess up the format and the thread... For those trying to follow the thread in the archives: my response (it's probably a refsect which isn't supported yet) ended up here: https://www.postgresql.org/message-id/1N1fn0-1qd4xd1MyG-011ype%40mail.gmx.net Regards, Brar
Re: doc: add missing "id" attributes to extension packaging page
On 23.03.2023 at 20:08, Brar Piening wrote: Since the need for ids is starting to grow again (ecb696527c added an id to a varlistentry in doc/src/sgml/ref/create_subscription.sgml) I've also amended the add-needed-ids patch once again so that the build does not fail after applying the make_html_ids_discoverable patch. New add-needed-ids patch since it was outdated again. Regards, Brar diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 71730cc52f..b0546f47b4 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -1132,7 +1132,7 @@ include_dir 'conf.d' - + scram_iterations (integer) scram_iterations configuration parameter diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml index 3836d13ad3..1432c67c2a 100644 --- a/doc/src/sgml/logical-replication.sgml +++ b/doc/src/sgml/logical-replication.sgml @@ -252,7 +252,7 @@ additional columns not provided by the published table. Any such columns will be filled with the default value as specified in the definition of the target table. However, logical replication in binary format is more - restrictive. See the binary + restrictive. See the binary option of CREATE SUBSCRIPTION for details. diff --git a/doc/src/sgml/pgwalinspect.sgml b/doc/src/sgml/pgwalinspect.sgml index 9a0241a8d6..62d9f9eb22 100644 --- a/doc/src/sgml/pgwalinspect.sgml +++ b/doc/src/sgml/pgwalinspect.sgml @@ -157,7 +157,7 @@ combined_size_percentage | 2.8634072910530795 - + pg_get_wal_block_info(start_lsn pg_lsn, end_lsn pg_lsn) returns setof record diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml index e92346edef..f0ab6a918e 100644 --- a/doc/src/sgml/ref/alter_subscription.sgml +++ b/doc/src/sgml/ref/alter_subscription.sgml @@ -178,7 +178,7 @@ ALTER SUBSCRIPTION name RENAME TO < origin parameter. - See the binary + See the binary option of CREATE SUBSCRIPTION for details about copying pre-existing data in binary format. diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml index 9d4b9d4e33..605b11bc67 100644 --- a/doc/src/sgml/ref/create_subscription.sgml +++ b/doc/src/sgml/ref/create_subscription.sgml @@ -61,7 +61,7 @@ CREATE SUBSCRIPTION subscription_nameParameters - + subscription_name @@ -70,7 +70,7 @@ CREATE SUBSCRIPTION subscription_name - + CONNECTION 'conninfo' @@ -81,7 +81,7 @@ CREATE SUBSCRIPTION subscription_name - + PUBLICATION publication_name [, ...] @@ -90,7 +90,7 @@ CREATE SUBSCRIPTION subscription_name - + WITH ( subscription_parameter [= value] [, ... ] ) @@ -102,7 +102,7 @@ CREATE SUBSCRIPTION subscription_name - + connect (boolean) @@ -129,7 +129,7 @@ CREATE SUBSCRIPTION subscription_name - + create_slot (boolean) @@ -145,7 +145,7 @@ CREATE SUBSCRIPTION subscription_name - + enabled (boolean) @@ -156,7 +156,7 @@ CREATE SUBSCRIPTION subscription_name - + slot_name (string) @@ -185,7 +185,7 @@ CREATE SUBSCRIPTION subscription_name - + binary (boolean) @@ -222,7 +222,7 @@ CREATE SUBSCRIPTION subscription_name - + copy_data (boolean) @@ -243,7 +243,7 @@ CREATE SUBSCRIPTION subscription_name - + streaming (enum) @@ -271,7 +271,7 @@ CREATE SUBSCRIPTION subscription_name - + synchronous_commit (enum) @@ -303,7 +303,7 @@ CREATE SUBSCRIPTION subscription_name - + two_phase (boolean) @@ -334,7 +334,7 @@ CREATE SUBSCRIPTION subscription_name - + disable_on_error (boolean) @@ -346,7 +346,7 @@ CREATE SUBSCRIPTION subscription_name - + origin (string)
Re: doc: add missing "id" attributes to extension packaging page
On 28.03.2023 at 00:11, Peter Smith wrote: FYI, there is a lot of overlap between this last attachment and the patches of Kuroda-san's current thread here [1] which is also adding ids to create_subscription.sgml. (Anyway, I guess you might already be aware of that other thread because your new ids look like they have the same names as those chosen by Kuroda-san) -- [1] https://www.postgresql.org/message-id/flat/CAHut%2BPvzo6%3DKKLqMR6-mAQdM%2Bj_dse0eUreGmrFouL7gbLbv2w%40mail.gmail.com#7da8d0e3b73096375847c16c856b4aed Thanks, I actually was not aware of this. Also, kudos for capturing the missing id and advocating for consistency regarding ids even before this is actively enforced. This nurtures my optimism that consistency might actually be achieveable without everybody getting angry at me because my patch will enforce it. Regarding the overlap, I currently try to make it as easy as possible for a potential committer and I'm happy to rebase my patch upon request or if Kuroda-san's patch gets applied first. Regards, Brar
Re: doc: add missing "id" attributes to extension packaging page
On 29.03.2023 at 06:52, Hayato Kuroda (Fujitsu) wrote: FYI - my patch is pushed Thanks! Could you please rebase your patch? Done and tested. Patch is attached. Regards, Brar diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index fcb53c6997..5c50347c58 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -1132,7 +1132,7 @@ include_dir 'conf.d' - + scram_iterations (integer) scram_iterations configuration parameter diff --git a/doc/src/sgml/pgwalinspect.sgml b/doc/src/sgml/pgwalinspect.sgml index 9a0241a8d6..62d9f9eb22 100644 --- a/doc/src/sgml/pgwalinspect.sgml +++ b/doc/src/sgml/pgwalinspect.sgml @@ -157,7 +157,7 @@ combined_size_percentage | 2.8634072910530795 - + pg_get_wal_block_info(start_lsn pg_lsn, end_lsn pg_lsn) returns setof record
Re: doc: add missing "id" attributes to extension packaging page
On 04.04.2023 at 16:54, Peter Eisentraut wrote: First of all, it works very nicely and is very useful. Very welcome. Thank you! The XSLT implementation looks sound to me. It would be a touch better if it had some comments about which parts of the templates were copied from upstream stylesheets and which were changed. There are examples of such commenting in the existing customization layer. Also, avoid introducing whitespace differences during said copying. I will amend the patch if we agree that this is the way forward. However, I wonder if this is the right way to approach this. I don't think we should put these link markers directly into the HTML. It feels like this is the wrong layer. For example, if you have CSS turned off, then all these # marks show up by default. I'd consider this a feature rather than a problem but this is certainly debatable. I cannot reliably predict what expectations a user who is browsing the docs with CSS disabled might have. The opposite is true too. If we'd move the id links feature to javascript, a user who has javascript disabled will not see them. Is this what they'd want? I don't know. Also, while about 1-2% of users have Javascript disabled, I haven't heard of disabling CSS except for debugging purposes. In general I'd consider the fact that CSS or Javascript might be disabled a niche problem that isn't really worth much debating but there is definitely something to consider regarding people using screen readers who might suffer from one or the other behavior and I'd definitely be interested what behavior these users would expect. Would they want to use the id link feature or would the links rather disrupt their reading experience - I have no idea TBH and I hate speculating about other people's preferences. It seems to me that the correct way to do this is to hook in some JavaScript that does this transformation directly on the DOM. Then we don't need to carry this presentation detail in the HTML. Moreover, it would avoid tight coupling between the website and the documentation sources. You can produce the exact same DOM, that part seems okay, just do it elsewhere. Was this approach considered? I didn't see it in the thread. I briefly touched the topic in [1] and [2] but we didin't really follow up on the best approach. Regards, Brar [1] https://www.postgresql.org/message-id/68b9c435-d017-93cc-775a-c604db9ec683%40gmx.de [2] https://www.postgresql.org/message-id/a75b6d7c-3fa4-d6a8-cf23-6b5180237392%40gmx.de
Re: doc: add missing "id" attributes to extension packaging page
On 06.04.2023 at 11:06, Peter Eisentraut wrote: On 04.04.23 21:52, Brar Piening wrote: The XSLT implementation looks sound to me. It would be a touch better if it had some comments about which parts of the templates were copied from upstream stylesheets and which were changed. There are examples of such commenting in the existing customization layer. Also, avoid introducing whitespace differences during said copying. I will amend the patch if we agree that this is the way forward. Ok, it appears everyone agrees that this is the correct approach. Please post an updated patch. There have been so many partial patches posted recently, I'm not sure which one is the most current one and who is really the author. Attached are two patches: 001-make_html_ids_discoverable_v5.postgresql.patch which needs to be applied to the postgresql repository. It adds the XSLT to generate the id links and the CSS to hide/display them. I've added comments as suggested above. 002-add-discoverable-id-style_v1.pgweb.patch which needs to be applied to the pgweb repository. It adds the CSS to the offical documentation site. At the moment (commit 983ec23007) there are no missing ids, so the build should just work after applying the patch but, as we already know, this may change with every commit that gets added. Reviewer is Karl O. Pink Author is Brar Piening (with some additions from Karl O. Pink) Best regards, Brar diff --git a/media/css/main.css b/media/css/main.css index 5f8bfdd5..c3464cd0 100644 --- a/media/css/main.css +++ b/media/css/main.css @@ -1175,6 +1175,16 @@ code, padding-right: 2em; } +/** Links to ids of headers and definition terms */ +#docContent a.id_link { + color: inherit; + visibility: hidden; +} + +#docContent *:hover > a.id_link { + visibility: visible; +} + /** * Various callout boxes for docs, including warning, caution, note, tip */ diff --git a/doc/src/sgml/stylesheet-html-common.xsl b/doc/src/sgml/stylesheet-html-common.xsl index bb6429ef7c..8e5d9912d5 100644 --- a/doc/src/sgml/stylesheet-html-common.xsl +++ b/doc/src/sgml/stylesheet-html-common.xsl @@ -309,4 +309,137 @@ set toc,title + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 6 + + + + + + http://www.w3.org/1999/xhtml";> + + + +clear: both + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + # + + + + + + id_link + +# + + + + + + + + Ids are required in order to provide the public + HTML documentation with stable URLs for < + + > element content; id missing at: + +/ + + + [@ + + = ' + + '] + + + + + + + + + diff --git a/doc/src/sgml/stylesheet.css b/doc/src/sgml/stylesheet.css index cc14efa1ca..15bcc95d41 100644 --- a/doc/src/sgml/stylesheet.css +++ b/doc/src/sgml/stylesheet.css @@ -169,3 +169,13 @@ acronym{ font-style: inherit; } width: 75%; } } + +/* Links to ids of headers and definition terms */ +a.id_link { +color: inherit; +visibility: hidden; +} + +*:hover > a.id_link { +visibility: visible; +}
Re: doc: add missing "id" attributes to extension packaging page
On 13.04.2023 at 10:31, Peter Eisentraut wrote: The first patch has been committed. Yay - thank you! The second patch should be sent to pgsql-www for integrating into the web site. Done via [1]. Thanks for the hint. Side project: I noticed that these new hover links don't appear in the single-page HTML output (make postgres.html), even though the generated HTML source code looks correct. Maybe someone has an idea there. I feel responsible for the feature to work for all use cases where it makes sense. I'll investigate this and post back. Regards, Brar [1] https://www.postgresql.org/message-id/d987a4a7-62c3-7e0c-860f-1c96fc2117d9%40gmx.de
Re: doc: add missing "id" attributes to extension packaging page
On 06.12.2022 at 01:55, Ian Lawrence Barwick wrote: Oh, now you mention it, I vaguely recall seeing those. However the thread stalled back in March and the patches don't seem to have made it to a CommitFest entry. Yes, my patches added quite a few ids and also some xsl/css logic to make them more discoverable in the browser but I had gotten the impression that nobody besides me cares about this, so I didn't push it any further. Brar, would you like to add an entry so they don't get lost? See: https://commitfest.postgresql.org/41/ Yes. I can certainly add them to the commitfest although I'm not sure if they still apply cleanly. I can also rebase or extend them if somebody cares. Regards, Brar
Re: doc: add missing "id" attributes to extension packaging page
On 06.12.2022 at 09:38, Alvaro Herrera wrote: I care. The problem last time is that we were in the middle of the last commitfest, so we were (or at least I was) distracted by other stuff. Ok, thanks. That's appreciated and understood. Looking at the resulting psql page, https://pgdocs.piening.info/app-psql.html#APP-PSQL-OPTIONS-EXPANDED I note that the ID for the -x option is called "options-blah". I understand where does this come from: it's the "expanded" bit in the "options" section. However, put together it's a bit silly to have "options" in plural there; it would make more sense to have it be https://pgdocs.piening.info/app-psql.html#APP-PSQL-OPTION-EXPANDED (where you can read more naturally "the expanded option for psql"). How laborious would it be to make it so? No problem. I've already done it. Your second link should work now. It'll probably have some conflicts, yeah. I've updated and rebased my branch on current master now. I would welcome separate patches: one to add the IDs, another for the XSL/CSS stuff. That allows us to discuss them separately. I'll send two patches in two separate e-mails in a moment. Regards, Brar
Re: doc: add missing "id" attributes to extension packaging page
On 06.12.2022 at 18:59, Brar Piening wrote: On 06.12.2022 at 09:38, Alvaro Herrera wrote: I would welcome separate patches: one to add the IDs, another for the XSL/CSS stuff. That allows us to discuss them separately. I'll send two patches in two separate e-mails in a moment. This is patch no 2 that adds links to html elements with ids to make them visible on the HTML surface when hovering the element. Regards, Brar diff --git a/doc/src/sgml/stylesheet-html-common.xsl b/doc/src/sgml/stylesheet-html-common.xsl index 9df2782ce4..111b03d6fc 100644 --- a/doc/src/sgml/stylesheet-html-common.xsl +++ b/doc/src/sgml/stylesheet-html-common.xsl @@ -301,4 +301,115 @@ set toc,title + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 6 + + + + + + http://www.w3.org/1999/xhtml";> + + + +clear: both + + + + + + + + + + + + + + + + + + + + + + + # + + + + + + id_link + + # + + + + + + + + element without id. Please add an id to make it usable + as stable anchor in the public HTML documentation. + + + Please add an id to the + + element in the sgml source code. + + + + + + diff --git a/doc/src/sgml/stylesheet.css b/doc/src/sgml/stylesheet.css index 6410a47958..e4174e0613 100644 --- a/doc/src/sgml/stylesheet.css +++ b/doc/src/sgml/stylesheet.css @@ -163,3 +163,13 @@ acronym{ font-style: inherit; } width: 75%; } } + +/* Links to ids of headers and definition terms */ +a.id_link { +color: inherit; +visibility: hidden; +} + +*:hover > a.id_link { +visibility: visible; +}
Re: doc: add missing "id" attributes to extension packaging page
On 06.12.2022 at 19:11, Brar Piening wrote: The current statistics for docbook elements with/without ids after applying the patch are the following: Somehow my e-mail client destroyed the table. That's how it was supposed to look like: name | with_id | without_id | id_coverage | min_id_len | max_id_len ---+-++-++ sect2 | 870 | 0 | 100.00 | 7 | 57 sect1 | 739 | 0 | 100.00 | 4 | 46 refentry | 307 | 0 | 100.00 | 8 | 37 sect3 | 206 | 0 | 100.00 | 11 | 57 chapter | 77 | 0 | 100.00 | 5 | 24 sect4 | 28 | 0 | 100.00 | 16 | 47 biblioentry | 23 | 0 | 100.00 | 6 | 15 simplesect | 20 | 0 | 100.00 | 24 | 39 appendix | 15 | 0 | 100.00 | 7 | 23 part | 8 | 0 | 100.00 | 5 | 20 co | 4 | 0 | 100.00 | 18 | 30 figure | 3 | 0 | 100.00 | 13 | 28 reference | 3 | 0 | 100.00 | 14 | 18 anchor | 1 | 0 | 100.00 | 21 | 21 bibliography | 1 | 0 | 100.00 | 8 | 8 book | 1 | 0 | 100.00 | 10 | 10 index | 1 | 0 | 100.00 | 11 | 11 legalnotice | 1 | 0 | 100.00 | 13 | 13 preface | 1 | 0 | 100.00 | 9 | 9 glossentry | 119 | 14 | 89.47 | 13 | 32 table | 285 | 162 | 63.76 | 12 | 56 example | 27 | 16 | 62.79 | 12 | 42 refsect3 | 5 | 3 | 62.50 | 19 | 24 refsect2 | 41 | 56 | 42.27 | 10 | 36 varlistentry | 1701 | 3172 | 34.91 | 9 | 64 footnote | 5 | 18 | 21.74 | 17 | 32 step | 28 | 130 | 17.72 | 7 | 28 refsect1 | 151 | 1333 | 10.18 | 15 | 40 informaltable | 1 | 15 | 6.25 | 25 | 25 phrase | 2 | 94 | 2.08 | 20 | 26 indexterm | 5 | 3262 | 0.15 | 17 | 26 variablelist | 1 | 813 | 0.12 | 21 | 21 function | 4 | 4011 | 0.10 | 12 | 28 entry | 11 | 17740 | 0.06 | 21 | 40 para | 3 | 25734 | 0.01 | 21 | 27
Re: doc: add missing "id" attributes to extension packaging page
On 18.01.2023 at 06:50, Brar Piening wrote: I'll give it a proper look this weekend. It turns out I didn't get a round tuit. ... and I'm afraid I probably will not be able to work on this until mid/end February so we'll have to move this to the next commitfest until somebody wants to take it over and push it through.
Doc patch for Logical Replication Message Formats (PG14)
Hello Hackers, while amending Npgsql to account for the Logical Streaming Replication Protocol changes in PostgreSQL 14 I stumbled upon two documentation inaccuracies in the Logical Replication Message Formats documentation (https://www.postgresql.org/docs/devel/protocol-logicalrep-message-formats.html) that have been introduced (or rather omitted) with the recent changes to allow pgoutput to send logical decoding messages (https://github.com/postgres/postgres/commit/ac4645c0157fc5fcef0af8ff571512aa284a2cec) and to allow logical replication to transfer data in binary format (https://github.com/postgres/postgres/commit/9de77b5453130242654ff0b30a551c9c862ed661). 1. The content of the logical decoding message in the 'Message' message is prefixed with a length field (Int32) which isn't documented yet. See https://github.com/postgres/postgres/blob/69a58bfe4ab05567a8fab8bdce7f3095ed06b99c/src/backend/replication/logical/proto.c#L388 2. The TupleData may now contain the byte 'b' as indicator for binary data which isn't documented yet. See https://github.com/postgres/postgres/blob/69a58bfe4ab05567a8fab8bdce7f3095ed06b99c/src/include/replication/logicalproto.h#L83 and https://github.com/postgres/postgres/blob/69a58bfe4ab05567a8fab8bdce7f3095ed06b99c/src/backend/replication/logical/proto.c#L558. The attached documentation patch fixes both. Best regards, Brar diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml index bc2a2feb0b..53b3f1f651 100644 --- a/doc/src/sgml/protocol.sgml +++ b/doc/src/sgml/protocol.sgml @@ -6498,6 +6498,18 @@ Message + + + +Int32 + + + +Length of the content. + + + + Byten @@ -7430,6 +7442,19 @@ TupleData + +Or + + + +Byte1('b') + + + +Identifies the data as binary value. + + + Int32
Re: Doc patch for Logical Replication Message Formats (PG14)
Amit Kapila wrote: On Mon, Jun 21, 2021 at 12:26 PM Brar Piening wrote: Hello Hackers, while amending Npgsql to account for the Logical Streaming Replication Protocol changes in PostgreSQL 14 I stumbled upon two documentation inaccuracies in the Logical Replication Message Formats documentation (https://www.postgresql.org/docs/devel/protocol-logicalrep-message-formats.html) that have been introduced (or rather omitted) with the recent changes to allow pgoutput to send logical decoding messages (https://github.com/postgres/postgres/commit/ac4645c0157fc5fcef0af8ff571512aa284a2cec) and to allow logical replication to transfer data in binary format (https://github.com/postgres/postgres/commit/9de77b5453130242654ff0b30a551c9c862ed661). 1. The content of the logical decoding message in the 'Message' message is prefixed with a length field (Int32) which isn't documented yet. See https://github.com/postgres/postgres/blob/69a58bfe4ab05567a8fab8bdce7f3095ed06b99c/src/backend/replication/logical/proto.c#L388 2. The TupleData may now contain the byte 'b' as indicator for binary data which isn't documented yet. See https://github.com/postgres/postgres/blob/69a58bfe4ab05567a8fab8bdce7f3095ed06b99c/src/include/replication/logicalproto.h#L83 and https://github.com/postgres/postgres/blob/69a58bfe4ab05567a8fab8bdce7f3095ed06b99c/src/backend/replication/logical/proto.c#L558. The attached documentation patch fixes both. Yeah, I think these should be fixed and your patch looks good to me in that regard. After looking at the docs once again I have another minor amendment (new patch attached). diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml index bc2a2feb0b..7d29308abc 100644 --- a/doc/src/sgml/protocol.sgml +++ b/doc/src/sgml/protocol.sgml @@ -6498,6 +6498,18 @@ Message + + + +Int32 + + + +Length of the content. + + + + Byten @@ -7430,6 +7442,19 @@ TupleData + +Or + + + +Byte1('b') + + + +Identifies the data as binary value. + + + Int32 @@ -7446,8 +7471,8 @@ TupleData -The value of the column, in text format. (A future release -might support additional formats.) +The value of the column, eiter in binary or in text format. +(As specified in the preceding format byte). n is the above length.
Re: Doc patch for Logical Replication Message Formats (PG14)
Amit Kapila schrieb: On Mon, Jun 21, 2021 at 4:13 PM Brar Piening wrote: Amit Kapila wrote: After looking at the docs once again I have another minor amendment (new patch attached). +The value of the column, eiter in binary or in text format. Typo. /eiter/either Fixed - thanks! diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml index bc2a2feb0b..7d29308abc 100644 --- a/doc/src/sgml/protocol.sgml +++ b/doc/src/sgml/protocol.sgml @@ -6498,6 +6498,18 @@ Message + + + +Int32 + + + +Length of the content. + + + + Byten @@ -7430,6 +7442,19 @@ TupleData + +Or + + + +Byte1('b') + + + +Identifies the data as binary value. + + + Int32 @@ -7446,8 +7471,8 @@ TupleData -The value of the column, in text format. (A future release -might support additional formats.) +The value of the column, either in binary or in text format. +(As specified in the preceding format byte). n is the above length.
Minor documentation error regarding streaming replication protocol
While implementing streaming replication client functionality for Npgsql I stumbled upon a minor documentation error at https://www.postgresql.org/docs/current/protocol-replication.html The "content" return value for the TIMELINE_HISTORYcommand is advertised as bytea while it is in fact raw ASCII bytes. How did I find out? Since the value I get doesn't start with a "\x" and contains ascii text, although I've bytea_outputset to hex, I first thought that the streaming replication protocol simply doesn't honor bytea_output, but then I realized that I also get unencoded tabs and newlines which wouldn't be possible if the value woud be passed through byteaout. This is certainly a minor problem since the timeline history file only contains generated strings that are ASCII-only, so just using the unencoded bytes is actually easier than decoding bytea. OTOH it did cost me a few hours (writing a bytea decoder and figuring out why it doesn't work by looking at varlena.c and proving the docs wrong) so I want to point this out here since it is possibly an unintended behavior or at least a documentation error. Also I'm wary of taking dependency on an undocumented implementation detail that could possibly change at any point. Regards, Brar
Re: Minor documentation error regarding streaming replication protocol
Brar Piening wrote: > This is certainly a minor problem After thinking about it a little more I think that at least the fact that even the row description message advertises the content as bytea could be considered as a bug - no?
Aw: Re: Minor documentation error regarding streaming replication protocol
Bruce Momjian wrote: >> Good point. The reporter was assuming the data would come to the client >> in the bytea output format specified by the GUC, e.g. \x..., so that >> doesn't happen either. As I said before, it is more raw bytes, but we >> don't have an SQL data type for that. > I did some more research on this. It turns out timeline 'content' is > the only BYTEA listed in the protocol docs, even though it just passes C > strings to pq_sendbytes(), just like many other fields like the field > above it, the timeline history filename. The proper fix is to change > the code to return the timeline history file contents as TEXT instead of > BYTEA. In the light of what Michael wrote above, I don't think that this is really enough. If the timeline history file can contain strings which "may not be made just of ASCII characters" this would probably make the client side assume that the content is being sent as TEXT encoded in client_encoding which again isn't true. In the worst case this could lead to nasty decoding bugs on the client side which could even result in security issues. Since you probably can't tell in which encoding the aforementioned "recovery target name" was written to the timeline history file, I agree with Michael that BYTEA is probably the sanest way to send this file. IMO the best way out of this is to either really encode the content as BYTEA by passing it through byteaout() and by that escaping characters <0x20 and >0x7e, or to document that the file is being sent "as raw bytes that can be read as 'bytea Escape Format' by parsers compatible with byteain()" (this works because byteain() doesn't check whether bytes <0x20 or >0x7e are actually escaped). Again, reading the raw bytes, either via byteain() or just as raw bytes, isn't really a problem and I don't want to bring you into a situation where the cure is worse than the disease.
Aw: Re: Re: Minor documentation error regarding streaming replication protocol
Tom Lane wrote: > Well, let's label it as text and then in the description > of the message, note that the field contains the raw file contents, > whose encoding is unknown to the server. +1 This would definitely have solved my initial issue and looks like a sane way forward without over-engineering the problem or causing any backwards-compatibility issues. Regards, Brar Piening