Re: Table rewrite supporting functions for event triggers
On Thu, Sep 12, 2024 at 08:52:00AM -0400, Greg Sabino Mullane wrote:
> I do like the simplicity of the bitmap:
>
> if (reason & 1)
> print "Table has changed from logged to unlogged"
> if (reason & 2)
> print "Default has been changed"
>
> versus with text[]:
>
> foreach reason in tablereason[]
> if reason.match_exact("ALTER_PERSISTENCE")
> print "Table has changed from logged to unlogged"
> if reason.match_regex("DEFAULT")
> print "Default has been changed"
> ...
Okay. I am not going to be annoying with compatibility, then :D
> My initial reaction was that this is indeed a rare case, and to avoid
> putting that level of code detail in the docs. Your argument is a good one,
> but it still feels wrong to put that there. Yes, this puts a little more
> onus on future developers, but updating the docs is already a core
> requirement for patches.
>
> (On reflection, maybe reverse it - put a code comment in event_trigger.h
> reminding people to also update the docs? But again, that's seems like
> something obvious anyway for someone making that change.)
I am not so sure, TBH. One example: I know these values in
tablecmds.c for some time because that's an area I tend to focus on
for bug fixes, but forgot entirely about the SQL function in event
triggers that feed on it until I found your post. A comment in
event_trigger.h to mention the doc update would work for me. That
would be impossible to miss.
--
Michael
signature.asc
Description: PGP signature
Re: Table rewrite supporting functions for event triggers
On Wed, Sep 11, 2024 at 10:14:27AM -0400, Greg Sabino Mullane wrote: > I dunno - so would we smush them together and return something like: > > "ALTER_PERSISTENCE and COLUMN_REWRITE" If multiple are set, let's just make it text[], then. > That would be a step backwards for anyone possibly using that integer > programatically to (for example) give a pretty user-facing message about > why the event was triggered. I don't know either how much people are relying on these numbers in applications. If this is like what we do in the regression tests and print it in notice messages within a PL/pgSQL function, that's not going to matter. Or just have a separate function.. Do you have a comment about mentioning the variables or the header in the docs for the stable branches? I'm aware that this is a rare practice, but so is this function's design. My argument is greppability between the code and the docs, mainly, to not miss an update of the docs if more reasons are added. That would be unlikely, but a backpatch of a reason is not impossible ABI-wise. -- Michael signature.asc Description: PGP signature
Re: Undocumented optionality of handler_statements
On Tue, Jul 23, 2024 at 01:25:39PM +0200, Philipp Salvisberg wrote: > read "optional" as "mandatory". They're optional, like in empty being optional. If not specified, the block goes to its END. > Therefore, I suggest to change this example by adding a NULL > statement as in other examples. This change would make the > documentation consistent and handle the optionality of > handler_statements as an implementation detail. I created a patch > for plpgsql.sgml based on the master branch, adding a NULL statement > in empty exception handlers (see attached file > doc_patch_using_null_stmt_instead_of_empty_exception_handler_v1.diff). These examples have been around for 20 years with, and I think that it is helpful to show this pattern as well. So if I were to do something about that, I would suggest the attached. -- Michael diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml index 78e4983139..3a5e7bc296 100644 --- a/doc/src/sgml/plpgsql.sgml +++ b/doc/src/sgml/plpgsql.sgml @@ -2804,9 +2804,9 @@ BEGIN statements EXCEPTION WHEN condition OR condition ... THEN -handler_statements + handler_statements WHEN condition OR condition ... THEN - handler_statements + handler_statements ... END; @@ -2821,8 +2821,8 @@ END; abandoned, and control passes to the EXCEPTION list. The list is searched for the first condition matching the error that occurred. If a match is found, the - corresponding handler_statements are - executed, and then control passes to the next statement after + corresponding handler_statements, if + specified are executed, and then control passes to the next statement after END. If no match is found, the error propagates out as though the EXCEPTION clause were not there at all: the error can be caught by an enclosing block with signature.asc Description: PGP signature
Re: Table rewrite supporting functions for event triggers
On Tue, Sep 03, 2024 at 09:34:02PM +0200, Laurenz Albe wrote: > On Tue, 2024-09-03 at 11:54 -0400, Greg Sabino Mullane wrote: >> How about something like this? > > This patch looks good to me. -Returns a code explaining the reason(s) for rewriting. The exact -meaning of the codes is release dependent. +Returns a code explaining the reason(s) for rewriting. The value is +a bitmap built from the following values: 1 (the table has changed +persistence), 2 (a column has changed a default value), 4 (a column +has a new data type), and 8 (the table access method has changed). Agreed that the user experience with this function is poor and that the documentation should be improved. Still, I am not sure that this is optimal. On top of the values, how about adding the variable names and also mention that these are defined in event_trigger.h? Putting the documentation change aside for a bit, could it be better to redesign this function and return a text value rather than an integer? We could directly return the names, minus "AT_REWRITE_", for instance. -- Michael signature.asc Description: PGP signature
Re: Undocumented optionality of handler_statements
On Mon, Jul 22, 2024 at 01:55:52PM +, PG Doc comments form wrote: > In > https://www.postgresql.org/docs/16/plpgsql-control-structures.html#PLPGSQL-ERROR-TRAPPING > handler_statements are documented as optional. > > However, the following example shows that handler_statements can be omitted. You have a good point. This could be clarified better in the documentation by making handler_statements conditional with square brackets around it. I'd rather add an extra sentence to tell that not specifying handler_statements is equivalent to taking no action. Perhaps you would like to write a patch? -- Michael signature.asc Description: PGP signature
Re: A minor bug in the doc of "SQL Functions Returning Sets" in xfunc.sgml.
On Fri, Jul 19, 2024 at 10:46:04AM +0900, 日向充 wrote: > I have found executable examples that do not work correctly > in the doc of "SQL Functions Returning Sets" in xfunc.sgml. > So I fixed the examples as follows. > - Changed CREATE TABLE tab > '(y int, z int)' to '(x int, y int, z int)' > - Changed INSERT INTO tab > '(1, 2), (3, 4), (5, 6), (7, 8)' > to '(1, 2, 3), (4, 5, 6), (7, 8, 9), (10, 11, 12)' > - Changed CREATE FUNCTION sum_n_product_with_tab > '(x int, OUT sum int, OUT product int)' > to '(int, OUT sum int, OUT product int)' > - Changed CREATE FUNCTION sum_n_product_with_tab > 'SELECT $1 + tab.y, $1 * tab.y FROM tab;' > to 'SELECT $1 + tab.x, $1 * tab.x FROM tab;' > - Changed result of "SELECT * FROM sum_n_product_with_tab(10);" > > The above will improve the results of examples as follows in this chapter. > > Do you think? Not sure that this is worth changing. The examples work OK when taken in isolation or are able to demonstrate the point they want to show. In short, not all these queries are here to be compatible with the contents in the same area. See for example the case of the "nodes" table on the same page, created nowhere. "tab" is just a more generic table name that's more spread. -- Michael signature.asc Description: PGP signature
Re: column_name of ALTER MATERIALIZED VIEW should only refer to an existing column
On Wed, May 22, 2024 at 02:12:36PM +0900, Michael Paquier wrote: > Will fix once we are out of release freeze time on HEAD. Thanks! And done as of dd087e1c13bf. -- Michael signature.asc Description: PGP signature
Re: column_name of ALTER MATERIALIZED VIEW should only refer to an existing column
On Wed, May 22, 2024 at 02:59:37AM +0200, Erik Wienhold wrote: > Here's a patch for $SUBJECT. Looks like the current wording was copied > from ALTER TABLE. In ALTER VIEW we correctly state that column_name > must be an existing column. Fun. You are right, none of the patterns supported by this query are able to add an attribute, so this new formulation makes sense. Will fix once we are out of release freeze time on HEAD. Thanks! -- Michael signature.asc Description: PGP signature
Re: [PATCH] Fix link to pg_ident_file_mappings
On Wed, Feb 21, 2024 at 08:50:59AM +0100, Daniel Gustafsson wrote: > On 21 Feb 2024, at 03:24, Erik Wienhold wrote: >> >> The docs on pg_reload_conf() in v15, v16, and devel have an incorrect >> link to pg_ident_file_mappings. The attached patch fixes that. > > Nice catch, will fix. Thanks you both. This looks like a copy-pasto from a2c84990bea7.\ -- Michael signature.asc Description: PGP signature
Re: Delete description of trigger file
On Sat, Dec 02, 2023 at 05:52:50PM +0900, Shinya Kato wrote: > Thanks for the review. > Yes, that's true. A new patch is attached. Thanks, applied. -- Michael signature.asc Description: PGP signature
Re: Delete description of trigger file
On Fri, Dec 01, 2023 at 11:54:14AM +0900, Shinya Kato wrote: > later disconnected, the standby goes back to step 1 and tries to > restore the file from the archive again. This loop of retries from the > archive, pg_wal, and via streaming replication goes > on until the server > -is stopped or failover is triggered by a trigger file. > +is stopped. Just removing this information looks incorrect to me, because a promotion would cause the retries to stop the WAL lookups. Shouldn't the last part of this sentence be reworded as of a "or is promoted"? -- Michael signature.asc Description: PGP signature
Re: Typo in "43.9.1. Reporting Errors and Messages"?
On Wed, Nov 01, 2023 at 09:18:47AM +0900, Michael Paquier wrote: > So you mean something like the attached then? Fixed that with f8b96c211da0 down to 11, in time for next week's release set. -- Michael signature.asc Description: PGP signature
Re: Typo in "43.9.1. Reporting Errors and Messages"?
On Tue, Oct 31, 2023 at 09:00:00PM +0300, Alexander Lakhin wrote: > I don't remember details, but I think the primary reason for the change > was that "RAISE_EXCEPTION" occurred in the whole tree only once (before > 66bde49d96). Now I see, that I had chosen the wrong replacement — I agree > with Euler, change to "raise_exception" would be more appropriate. Indeed, it looks like the origin of the confusion is the casing here, so changing to "raise_exception" like in the appendix sounds good to me: https://www.postgresql.org/docs/devel/errcodes-appendix.html So you mean something like the attached then? > (I've found a similar mention of ERRCODE_xxx in btree.sgml: > Before doing so, the function should check the sign > of offset: if it is less than zero, raise > error ERRCODE_INVALID_PRECEDING_OR_FOLLOWING_SIZE (22013) > with error text like invalid preceding or following size in window > function. > but I think that's okay here, because that identifier supposed to be used > as-is in ereport/elog.) Yep, still this one is not that old (0a459cec96d3). -- Michael diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml index 83f867f91d..5977534a62 100644 --- a/doc/src/sgml/plpgsql.sgml +++ b/doc/src/sgml/plpgsql.sgml @@ -3942,7 +3942,7 @@ RAISE unique_violation USING MESSAGE = 'Duplicate user ID: ' || user_id; If no condition name nor SQLSTATE is specified in a RAISE EXCEPTION command, the default is to use -ERRCODE_RAISE_EXCEPTION (P0001). +raise_exception (P0001). If no message text is specified, the default is to use the condition name or SQLSTATE as message text. signature.asc Description: PGP signature
Re: Minipatch concerning tags + new spelling/punctuation patch
On Mon, Oct 23, 2023 at 12:33:03PM +0300, Maxim Yablokov wrote: > We have also prepared a small patch concerning spelling, punctuation, etc. > We've decided to send it separately, so it'll be easier to review. Please > have a look. Yep, these are improvements. So applied and backpatched (all these applied down to 16, except for one down to 11). > (just in case: e.kiryan...@postgrespro.ru, e.indrupsk...@postgrespro.ru, > o.sibirya...@postgrespro.ru, m.yablo...@postgrespro.ru) Thanks for the list. I've used that in the commit message. -- Michael signature.asc Description: PGP signature
Re: Minipatch concerning tags
On Sat, Oct 21, 2023 at 04:11:50PM +0900, Michael Paquier wrote: > But not this one. This could just be "WAL senders", but we also use > the term "walsender" in physical and logical replication areas, so a > plural looks OK for me here, leaving it as it is now. I have reverted this change, added one markup for a catalog_xmin that was missed, fixed a few whitespaces, and applied it on HEAD. Thanks! -- Michael signature.asc Description: PGP signature
Re: Minipatch concerning tags
On Fri, Oct 20, 2023 at 11:22:55AM +0300, Maxim Yablokov wrote: > PostgresPro documentation team (Ekaterina Kiryanova, Elena Indrupskaya, Oleg > Sibiryakov and me) have prepared a patch with some improvements concerning > missed/inapt tags. Please have a look. Most of what you are proposing makes sense. > - It means that, for walsenders which are lagging (if any), some WAL > records up > + It means that, for walsenders that are lagging (if > any), But not this one. This could just be "WAL senders", but we also use the term "walsender" in physical and logical replication areas, so a plural looks OK for me here, leaving it as it is now. -- Michael signature.asc Description: PGP signature
Re: Documentation does not mention that basebackup could not be used on newer major version
On Mon, Sep 18, 2023 at 10:08:30PM +0200, Laurenz Albe wrote: > On Mon, 2023-09-18 at 15:29 +, PG Doc comments form wrote: >> My main issue is that `pg_basebackup` page does not mention that this backup >> is compatible only with current version of database. I can not do basebackup >> on v11 and restore that on v15, for example. > > I think that that is amply documented by the fact that pg_basebackup is > *not* mentioned in https://www.postgresql.org/docs/current/upgrading.html . Yeah. The issue with this one is that it does a direct physical copy of the files. You cannot expect pg_basebackup to be able to do all the work that pg_upgrade would do underground, like binary upgrades, and more. > You can't expect us to enumerate every tool that is not suitable for > upgrading. Just noting in passing. There is a lot of maintenance effort for downward compatibility (tools like pg_dump and pg_basebackup at version N are able to work with a backend version older, say at N-1). Upward compabitility may work in some cases, even for dumps, still these would likely require extra manipulation to be able to load to a version of the backend older than the version of pg_dump used. Being able to use pg_basebackup to work with older backend versions is a really important property we try to keep available. -- Michael signature.asc Description: PGP signature
Re: Improvement of clarity in pg_ctl command docummentation
On Sat, Jul 15, 2023 at 05:57:40AM +, PG Doc comments form wrote: > I was reading the documentation about pg_ctl and there everything was well > written about the usage and option that can be used with the pg_ctl command > but there is not mentioned that we can not run pg_ctl command as root and > why we cant run it as root. Yes, I guess that you are right in the fact that we don't document pg_ctl cannot be run as root. The short explanation behind this restriction is that the PostgreSQL backend is considered as having the same rights as the OS user running it, and we don't want to encourage unsecure behaviors where systems could be easily compromised. -- Michael signature.asc Description: PGP signature
Re: Change "two" to "three" for decades of development in history
On Fri, Jun 23, 2023 at 10:50:20PM -0400, Bruce Momjian wrote: > Applied patch is "With decades of development". Thanks. -- Michael signature.asc Description: PGP signature
Re: Change "two" to "three" for decades of development in history
On Fri, Jun 23, 2023 at 03:17:26AM +0200, Erik Wienhold wrote: > An SGML entity [0] or an xsltproc stringparam [1] looks viable. Question is > how to calculate the number of decades in the Makefile. It's trivial in SQL > :) - University of California at Berkeley. With over two decades of + University of California at Berkeley. With over three decades of development behind it, PostgreSQL is now the most advanced open-source database available anywhere. Seeing what has been committed, I don't think that such complications are necessary if this sentence is reworded without numbers. For instance: "With multiple decades of development behind it, PostgreSQL.." My 2c. -- Michael signature.asc Description: PGP signature
Re: Typo
On Tue, May 23, 2023 at 08:52:25PM +, PG Doc comments form wrote: > There appears to be a typo, here: > https://www.postgresql.org/docs/current/history.html#:~:text=Postgres95%20code%20was%20completely%20ANSI%20C. > A word or two should be added between 'completely' and 'ANSI C', such as > 're-written in', or 're-coded using', or some such. This is the current sentence, and it sounds kind of OK to me, FWIW: "Postgres95 code was completely ANSI C and trimmed in size by 25%. -- Michael signature.asc Description: PGP signature
Re: Certificate authentication docs in multiple places
On Mon, Apr 10, 2023 at 09:47:08AM +0100, Steve Atkins wrote: > Yes, I’ll do that once I get some time to get the doc toolchain working. Well, if you don't want to spend time setting up a toolchain, feel free to send a patch just hacking the SGML files. I am OK to take it from there, as long as the idea behind the patch is clear. -- Michael signature.asc Description: PGP signature
Re: Certificate authentication docs in multiple places
On Tue, Mar 28, 2023 at 04:28:24PM +0200, Peter Eisentraut wrote: > Sure, some cross-linking between those two sections seems sensible. Steve, would you like to propose a patch? -- Michael signature.asc Description: PGP signature
Re: incorrect info in dblink examples
On Mon, Feb 27, 2023 at 07:24:37PM +, PG Doc comments form wrote: > In your examples, AS t1(proname name, prosrc text) > should actually be AS t1(proname text, prosrc text) > > This occurs frequently in the documentation at the following link: > https://www.postgresql.org/docs/current/contrib-dblink-function.html Why? pg_proc.proname uses "name" as data type, so this is not wrong. -- Michael signature.asc Description: PGP signature
Re: Add missing meson arguments in docs
On Wed, Feb 22, 2023 at 09:24:37AM +0100, Peter Eisentraut wrote: > Yeah, this is just in the long tail of things to work through. I am going through that, and found out what can be done to do coverage reports. I will post a patch separately on -hackers. -- Michael signature.asc Description: PGP signature
Re: Add missing meson arguments in docs
On Tue, Feb 21, 2023 at 10:40:12AM +0100, Jelte Fennema wrote: > The -Dcassert and -Db_coverage of the meson build didn't show in the > docs that they needed to be passed true or false. All other options > specified the arguments they expected. This patch fixes that. Indeed, values have to be specified after the equal sign of each option, so applied what you are suggesting here. -Db_coverage is part of the core options of meson because it is not listed in meson_options.txt, no? Documenting the switch is fine, but it seems like we lack contents here. The paragraph describing -Db_coverage links to regress-coverage, still this only has instructions for ./configure. Shouldn't we have some docs to show one how to achieve the same with meson? I'd guess that a separate section for meson would make the most sense, for as long as we support both methods in parallel. https://wiki.postgresql.org/wiki/Meson tells nothing about that, for one. -- Michael signature.asc Description: PGP signature
Re: meson doc: Is "-Dtap-tests" a typo?
On Tue, Feb 14, 2023 at 09:06:13AM +, Katsuragi Yuta wrote:
> The parameter name "-Dtap-tests" in the meson's doc seems to be a typo.
> I think "-Dtap_tests" is the correct name. Attached patch fixes that.
>
> I got an unknown options error by using "-Dtap-tests=enabled". However,
> meson_options.txt told me the correct name was "-Dtap_tests".
- -Dtap-tests={ auto | enabled | disabled }
+ -Dtap_tests={ auto | enabled | disabled }
And meson.build refers to the same term, as of "tap_tests":
# Check whether tap tests are enabled or not
tap_tests_enabled = false
tapopt = get_option('tap_tests')
So you are indeed right. Nice catch. Will fix.
--
Michael
signature.asc
Description: PGP signature
Re: Naming of network_ops vs. inet_ops for SP-GIST
On Tue, Jan 24, 2023 at 10:38:28PM -0500, Tom Lane wrote: > Works for me. Thanks for checking, applied that. -- Michael signature.asc Description: PGP signature
Re: Naming of network_ops vs. inet_ops for SP-GIST
On Tue, Jan 24, 2023 at 03:22:44PM -0500, Tom Lane wrote: > I wonder whether we shouldn't just revert this table to > showing opclass names, and avert our eyes from the theoretical > inconsistency. Michael, looks like it was your 7a1cd5260 > that changed it; what do you think? Yes, the docs should be fixed here. The intention is not to show the operator families but the names of the opclasses. I can only spot one difference in SpGiST for network_ops -> inet_ops as of the report. BRIN, GIN and GiST look to be clean after a second lookup. I don't have a strong opinion about the naming inconsistency between the opclass name and the opfamily name in this case, though, couldn't it create more problems than actually fix something? Anyway, attached is a patch for the docs. Thoughts? -- Michael diff --git a/doc/src/sgml/spgist.sgml b/doc/src/sgml/spgist.sgml index 00432512de..102f8627bd 100644 --- a/doc/src/sgml/spgist.sgml +++ b/doc/src/sgml/spgist.sgml @@ -91,18 +91,7 @@ |>> (box,box) - kd_point_ops - |>> (point,point) - <-> (point,point) - - << (point,point) - >> (point,point) - <<| (point,point) - ~= (point,point) - <@ (point,box) - - - network_ops + inet_ops << (inet,inet) @@ -117,6 +106,17 @@ >= (inet,inet) && (inet,inet) + + kd_point_ops + |>> (point,point) + <-> (point,point) + + << (point,point) + >> (point,point) + <<| (point,point) + ~= (point,point) + <@ (point,box) + poly_ops << (polygon,polygon) signature.asc Description: PGP signature
Re: Doc fix
On Mon, Nov 21, 2022 at 03:14:20PM +0100, Guillaume Lelarge wrote: > While translating the new minor release docs, I've found an issue. Patch > attached fixes this issue. The issue is only available on v11 and v12. > Patch made on v11, but should also work on v12. Indeed, applied to REL_12_STABLE and REL_11_STABLE. Thanks for the report and the patch, Guillaume. -- Michael signature.asc Description: PGP signature
Re: list of flags that pg_settings_get_flags reports
On Thu, Nov 03, 2022 at 07:53:55PM -0400, Tom Lane wrote: > Agreed, done. Thanks for taking care of that! -- Michael signature.asc Description: PGP signature
Re: jsonlog cursor_position type is wrong.
On Mon, Oct 24, 2022 at 02:04:48PM +0900, Michael Paquier wrote: > Thanks, that's wrong :/ Anyway, this one is on me, so applied. Thanks for the patch and the report! -- Michael signature.asc Description: PGP signature
Re: jsonlog cursor_position type is wrong.
On Mon, Oct 24, 2022 at 12:53:26AM -0400, Tom Lane wrote: > Tatsuo Ishii writes: >> Yeah, that's a typo. Patch attached. Thanks, that's wrong :/ > Shouldn't it be "integer"? When it comes down to the data types of a JSON object, these are referred as "number", as these can be either integers or floating points. See: https://www.w3schools.com/js/js_json_datatypes.asp -- Michael signature.asc Description: PGP signature
Re: Minor documentation fixes
On Tue, Sep 27, 2022 at 03:58:10PM +0300, Elena Indrupskaya wrote: > My colleagues from the Postgres Pro documentation team and I noticed a few > minor inconsistencies in recently updated sections of the PostgreSQL 15 > documentation. The attached patch fixes them. > > Two contributor names in the Release Notes are changed to match the names of > commit authors with those in the Acknowledgements list. This is mostly in the area of the release notes. Bruce? -- Michael signature.asc Description: PGP signature
Re: Documentation fixes
On Fri, Sep 30, 2022 at 06:10:01PM +0300, Ekaterina Kiryanova wrote: > I've prepared a small patch to fix a broken link and some inconsistencies > with upper/lower case letters. > Please take a look. Thanks Ekaterina, this looks fine to me so applied. The cosmetic part in func.sgml is older than 15 and all the remaining parts apply to PG >= 15 so I have done that only down to REL_15_STABLE. -- Michael signature.asc Description: PGP signature
Re: PostgreSQL 15 minor documentation improvements
On Thu, Aug 18, 2022 at 12:46:41PM +0300, Maxim Yablokov wrote: > Yeah, sorry about that, I fixed it in the new patch. > As for the tag, it is written above: > pg_basebackup --target=shell > And also in backup.sgml there is the following: > createdb -T template0 class="parameter">dbname > So I believe that we can keep here. Well, both are commands, but it is true that we use both patterns in the docs, still I have switched both to use , and applied the patch after tweaking things a bit. (Note as well that the various SIG* use instead of , but I have not bothered about changing that). -- Michael signature.asc Description: PGP signature
Re: PostgreSQL 15 minor documentation improvements
On Wed, Aug 17, 2022 at 06:09:13PM +0300, Максим Яблоков wrote: > I have prepared a small patch with possible changes of these places, and > also a separate patch with some improvements concerning missed/inapt tags. > Please have a look. archive_command is used only when archive_library is not set AFAIK, but an archive_library could also freely use an archive_command if it wishes to. But, yes, I agree that the current wording in backup.sgml is kind of confusing because of this reason, so I am fine to have a reference to both archive_library *and* archive_command in this area of the docs. All WAL records required for the backup must contain sufficient full-page writes, which requires you to enable full_page_writes on the primary and -not to use a tool in your archive_library to remove -full-page writes from WAL files. +not to use a tool to remove full-page writes from WAL files. Hmm. My opinion here is to do a simplification, and remove simply the last part of the paragraph about tools that manipulate WAL files as the first sentence makes it clear, in my opinion, that if those FPWs are not around the server could become kaput. Most of the changes in PGSQL15_tags_fix.patch seem right to me. Still, you'd better check that the docs compile, as of: > --- a/doc/src/sgml/basebackup-to-shell.sgml > +++ b/doc/src/sgml/basebackup-to-shell.sgml > @@ -12,9 +12,9 @@ >called shell. This makes it possible to run >pg_basebackup --target=shell or, depending on how this >module is configured, > - pg_basebackup --target=shell:DETAIL_STRING, and cause > - a server command chosen by the server administrator to be executed for > - each tar archive generated by the backup process. The command will receive > + pg_basebackup > --target=shell:DETAIL_STRING, I am pretty sure that this line is going to cause a compilation failure of the docs. Anyway, this should be use a markup, no? -- Michael signature.asc Description: PGP signature
Re: PostgreSQL 15 minor fixes in protocol.sgml
On Mon, Aug 01, 2022 at 11:00:20PM +0300, Ekaterina Kiryanova wrote:
> I didn't include it in the patch but I also suggest removing single quotes
> around 'method' for the COMPRESSION option to help avoid confusion. (All the
> supported compression methods consist of a single word so in my opinion
> there is no need to use quotes in this case.)
> -- COMPRESSION
> 'method'
Other options use quotes as well in their description in this area.
> I've also noticed that there are two ways to describe an option: "If set to
> true" / "If true". As far as I know, the option here is specified by its
> name rather than being explicitly set to true so "if true" seems to be more
> correct, and this could be a slight improvement for this page. Please
> correct me if I'm wrong.
Both sound pretty much the same to me.
> Another point worth mentioning is that only this file contains the phrase
> "two-phase transaction". I believe that "two-phase commit transaction" or
> "transaction prepared for two-phase commit" depending on the situation would
> be better wording.
"Prepare for two-phase commit" may be clearer?
> And finally, could you please clarify this part?
> -- The end LSN of the prepare transaction.
> Is it a typo of "prepared transaction"? Or is it the LSN of the transaction
> for Prepare?
> If it's the latter, perhaps it'd make more sense to capitalize it.
Hmm. The internals of 63cf61c refer to a "STREAM PREPARE", still the
protocol docs are quite messy ("prepare", "prepare timestamp", etc.)
so more consistency would be appropriate, it seems. Amit?
The part for the protocol messages with 2PC and logical replication
could use a larger rework. I have left these for now, and fixed the
rest of the typos you have found.
--
Michael
signature.asc
Description: PGP signature
Re: list of flags that pg_settings_get_flags reports
On Wed, Jul 20, 2022 at 01:51:36PM +0900, Fujii Masao wrote: > Attached is the updated version of the patch. It separates the list > for GUC flags from the table entry for pg_settings_get_flags() and > adds the table for it at the bottom of the existing function table. Thanks a lot for sending a patch. This looks fine to me. -- Michael signature.asc Description: PGP signature
Re: list of flags that pg_settings_get_flags reports
On Fri, Jul 15, 2022 at 03:40:08PM +0200, Alvaro Herrera wrote: > I think it should be a separate table, like tables 9.75-9.77 lists > properties for various functions. Err, sorry for missing that. I did not notice that a list would be treated as separate lines in the existing function table. Adding a table at the bottom of the function list sounds fine to me. I can handle it if you want, as that's something I have committed originally. -- Michael signature.asc Description: PGP signature
Re: Typo in examples in "8.14.5. jsonb Subscripting"
On Thu, Jul 07, 2022 at 12:27:01AM +, PG Doc comments form wrote:
> Towards the end of the section, the code examples given have comments where
> the JSON is using single quotes instead of double quotes around the
> properties. For example this:
>
> -- Where jsonb_field was {}, it is now {'a': [{'b': 1}]}
>
> ...should use double quotes around the "a" and "b" properties and read as
> follows:
>
> -- Where jsonb_field was {}, it is now {"a": [{"b": 1}]}
Indeed. I can see only the two places you are pointing at as being
incorrect. Will fix.
--
Michael
signature.asc
Description: PGP signature
Re: proposal: convert comments in documents to html comments
On Tue, Jun 28, 2022 at 12:42:08PM +0900, Noboru Saito wrote: > If you want to separate hashes or add links, > I think it would be better to stop the current comment and make it a paragraph > (I think it is possible to keep them folded > when converted to html even if they are paragraphs). > I agree with doing it that way, but it may not be possible right away. Hmm. Could it be better to tweak git_changelog so as the commit ID is reordered in the contents generated then? Then the doc rules could be tweaked to feed on that? I have noticed that your patch is not able to put a commit with its correct item. The first item of a section is not aligned looks associated to nothing, while the last item has no commit associated to it. -- Michael signature.asc Description: PGP signature
Re: proposal: convert comments in documents to html comments
On Sun, Jun 26, 2022 at 09:19:21PM +0900, Noboru Saito wrote: > Currently the comments in the document are removed when converting to html. > I propose to keep them as html comments. OK. The format of the HTML pages is rather clean, so this would be rather readable. > The release notes have the git commit information in the comments, > it would be great to have it in the html comments as well. > > That can be done with the attached patch. One argument that could go against that is the extra amount of data it generates in the page that gets loaded. How do things change for the release notes? But we are likely talking about an increase of 70kB to perhaps 100kB, so it does not matter much those days :) Another is that you cannot notice those comments except when looking at the HTML source, which is something that most users won't really notice, while others willing to look at those particular commits could just ping this information from the SGML files, because they should have the code around anyway. -- Michael signature.asc Description: PGP signature
Re: Spelling Mistake
On Wed, May 18, 2022 at 03:02:04AM +, PG Doc comments form wrote: > There are 3 periods instead of 2 which I believe is incorrect. It seems to me that those three periods refer to the root path of the source tree that you'd need to type by yourself to reach it. -- Michael signature.asc Description: PGP signature
Re: Missing tags ALTER ROUTINE and DROP ROUTINE in Event Trigger Firing Matrix
Hi Leslie, On Sat, Mar 05, 2022 at 10:15:33AM +, PG Doc comments form wrote: > "ALTER ROUTINE" and "DROP ROUTINE" are not listed in the "Event Trigger > Firing Matrix" documentation page, even though these tags seem supported > from PostgreSQL 11 onwards. > > It would be really useful to mention them, since alterations and deletions > of functions and procedures through ALTER ROUTINE and DROP ROUTINE commands > won't be caught by a trigger firing only on "ALTER PROCEDURE" and "ALTER > FUNCTION" tags (respectively "DROP PROCEDURE" and "DROP FUNCTION" tags). Thanks for the report. Indeed, the table is missing that ALTER ROUTINE would fire the events ddl_command_start and ddl_command_end. DROP ROUTINE fires on sql_drop, ddl_command_start and ddl_command_end. I have applied a patch to add this information to the docs down to 11. Regards, -- Michael signature.asc Description: PGP signature
Re: add free space map link in pg_freespacemap page
On Tue, Mar 08, 2022 at 06:34:34PM +0900, Dong Wook Lee wrote: > Patch looks good to me. Okay, applied then. -- Michael signature.asc Description: PGP signature
Re: add free space map link in pg_freespacemap page
On Tue, Mar 08, 2022 at 09:31:49AM +0900, Dong Wook Lee wrote:
> I don't know about it, so should I use at FSM for all the
> acronyms in the section dedicated to acronyms?
Yes, I would do that on consistency grounds. Your idea to add a link
to the section describing what a FSM is from pgfreespacemap.sgml is
also a good one.
While looking around, I have also noticed some inconsistencies within
the contents of pageinspect and all that leads me to the patch
attached. Does that look fine to you?
--
Michael
diff --git a/doc/src/sgml/pageinspect.sgml b/doc/src/sgml/pageinspect.sgml
index 24b5e463ed..55513cf522 100644
--- a/doc/src/sgml/pageinspect.sgml
+++ b/doc/src/sgml/pageinspect.sgml
@@ -31,9 +31,11 @@
relation and returns a copy as a bytea value. This allows a
single time-consistent copy of the block to be obtained.
fork should be 'main' for
- the main data fork, 'fsm' for the free space map,
- 'vm' for the visibility map, or 'init'
- for the initialization fork.
+ the main data fork, 'fsm' for the
+ free space map,
+ 'vm' for the
+ visibility map, or
+ 'init' for the initialization fork.
@@ -136,7 +138,7 @@ test=# SELECT page_checksum(get_raw_page('pg_class', 0), 0);
fsm_page_contents shows the internal node structure
- of an FSM page. For example:
+ of an FSM page. For example:
test=# SELECT fsm_page_contents(get_raw_page('pg_class', 'fsm', 0));
@@ -147,7 +149,7 @@ test=# SELECT fsm_page_contents(get_raw_page('pg_class', 'fsm', 0));
See src/backend/storage/freespace/README for more
- information on the structure of an FSM page.
+ information on the structure of an FSM page.
diff --git a/doc/src/sgml/pgfreespacemap.sgml b/doc/src/sgml/pgfreespacemap.sgml
index 5025498249..1f7867d9b9 100644
--- a/doc/src/sgml/pgfreespacemap.sgml
+++ b/doc/src/sgml/pgfreespacemap.sgml
@@ -9,10 +9,10 @@
The pg_freespacemap module provides a means for examining the
- free space map (FSM). It provides a function called
- pg_freespace, or two overloaded functions, to be
- precise. The functions show the value recorded in the free space map for
- a given page, or for all pages in the relation.
+ free space map (FSM).
+ It provides a function called pg_freespace, or two
+ overloaded functions, to be precise. The functions show the value recorded in
+ the free space map for a given page, or for all pages in the relation.
@@ -36,7 +36,7 @@
Returns the amount of free space on the page of the relation, specified
- by blkno, according to the FSM.
+ by blkno, according to the FSM.
@@ -50,7 +50,8 @@
Displays the amount of free space on each page of the relation,
- according to the FSM. A set of (blkno bigint, avail int2)
+ according to the FSM. A set of
+ (blkno bigint, avail int2)
tuples is returned, one tuple for each page in the relation.
@@ -112,8 +113,8 @@ postgres=# SELECT * FROM pg_freespace('foo', 7);
Original version by Mark Kirkwood mar...@paradise.net.nz.
- Rewritten in version 8.4 to suit new FSM implementation by Heikki
- Linnakangas hei...@enterprisedb.com
+ Rewritten in version 8.4 to suit new FSM implementation
+ by Heikki Linnakangas hei...@enterprisedb.com
diff --git a/doc/src/sgml/storage.sgml b/doc/src/sgml/storage.sgml
index 7136bbe7a3..f4b9f66589 100644
--- a/doc/src/sgml/storage.sgml
+++ b/doc/src/sgml/storage.sgml
@@ -603,10 +603,11 @@ tuple would otherwise be too big.
Each heap and index relation, except for hash indexes, has a Free Space Map
-(FSM) to keep track of available space in the relation. It's stored
-alongside the main relation data in a separate relation fork, named after the
-filenode number of the relation, plus a _fsm suffix. For example,
-if the filenode of a relation is 12345, the FSM is stored in a file called
+(FSM) to keep track of available space in the relation.
+It's stored alongside the main relation data in a separate relation fork,
+named after the filenode number of the relation, plus a _fsm
+suffix. For example, if the filenode of a relation is 12345, the
+FSM is stored in a file called
12345_fsm, in the same directory as the main relation file.
signature.asc
Description: PGP signature
Re: add free space map link in pg_freespacemap page
On Mon, Mar 07, 2022 at 11:08:16AM +0100, Laurenz Albe wrote: > On Sun, 2022-03-06 at 23:42 +0900, Dong Wook Lee wrote: >> >> The pg_freespacemap module provides a means for >> examining the >> - free space map (FSM). It provides a function called >> + free space map (FSM). It provides a >> function called >> pg_freespace, or two overloaded functions, to be >> precise. The functions show the value recorded in the free space map for >> a given page, or for all pages in the relation. >> >> so I propose a patch to fix it. > > +1 Shouldn't you use an here? FSM is a term listed in the section dedicated to acronyms. -- Michael signature.asc Description: PGP signature
Re: COMMENT ON lock
On Wed, Jan 19, 2022 at 01:45:07PM +0900, Michael Paquier wrote: > No objections to add more details for all that. Will fix. And done as of b2a76bb. -- Michael signature.asc Description: PGP signature
Re: COMMENT ON lock
On Tue, Jan 18, 2022 at 03:37:35PM +0100, Laurenz Albe wrote: > On Tue, 2022-01-18 at 14:11 +, nikolai.berkoff wrote: >> I wanted to know what lock the COMMENT ON command took and I had to look in >> the >> source code, makes sense to me that it should be documented. I've also added >> it to the list of commands that take a SHARE UPDATE EXCLUSIVE lock. > > +1. No objections to add more details for all that. Will fix. -- Michael signature.asc Description: PGP signature
Re: The pg_stop_backup will return one row with three values.
On Mon, Dec 20, 2021 at 07:19:32AM -0500, Dave Cramer wrote: > The docs say it returns 3 values ? They do. Here is the relevant part: https://www.postgresql.org/docs/devel/functions-admin.html#FUNCTIONS-ADMIN-BACKUP "The result of the function is a single record. The lsn column holds the backup's ending write-ahead log location (which again can be ignored). The second and third columns are NULL when ending an exclusive backup; after a non-exclusive backup they hold the desired contents of the label and tablespace map files." For exclusive backups, where pg_stop_backup() is used without an argument, 1 row is returned with the LSN marking the end of the backup. When pg_stop_backup(true) is used, you would get one row with three attributes: the end LSN, the label file as NULL and a tablespace map as NULL. For non-exclusive backups, pg_stop_backup(false) returns those three fields, all of them being not NULL. pg_stop_backup() without an argument cannot be used for non-exclusive backups. -- Michael signature.asc Description: PGP signature
Re: broken link to "SELinux guide" from sepgsql
On Thu, Oct 28, 2021 at 01:34:40PM +0200, Daniel Gustafsson wrote: > AFAICT it's the same document, but updated. If we want to leave 9.6 in as > much > of a working state as possible I think it makes sense to change to that one. > I'm happy to do it if you concur. Sure. That's fine by me. -- Michael signature.asc Description: PGP signature
Re: broken link to "SELinux guide" from sepgsql
On Wed, Oct 27, 2021 at 05:09:41PM +0900, Michael Paquier wrote: > Indeed, that's broken. Will fix as per your suggestion. And done down to 10 as of cc1853b. 9.6 used a different link that was already dead, and this version will be EOL'd soon, so I did not bother changing it. -- Michael signature.asc Description: PGP signature
Re: broken link to "SELinux guide" from sepgsql
On Wed, Oct 27, 2021 at 02:01:24PM +0700, Anton Voloshin wrote: > on https://www.postgresql.org/docs/current/sepgsql.html > the "SELinux User's and Administrator's Guide" link to > https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/7/html/SELinux_Users_and_Administrators_Guide/ > is broken, leads to 404. > > Proper URI would be, apparently, > https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/7/html/selinux_users_and_administrators_guide/index Indeed, that's broken. Will fix as per your suggestion. -- Michael signature.asc Description: PGP signature
Re: [PATCH] doc/queries.sgml: add missing comma
On Tue, Oct 19, 2021 at 06:41:54PM -0700, David G. Johnston wrote: > On Tue, Oct 19, 2021 at 2:33 PM Tom Lane wrote: >> ... although I think this text is mine, so naturally I'd think >> that. Anyone else have an opinion? > > If I read that aloud to myself there is a comma after iteration. Agreed, adding a comma feels more natural. Non-native speaker here, however. -- Michael signature.asc Description: PGP signature
Re: ALTER TABLE ... SET DATA TYPE removes statistics
On Tue, Oct 19, 2021 at 12:16:44PM -0300, Alvaro Herrera wrote: > Dunno, putting it in the middle of the existing paragraph looks odd to > me. I would put it in a separate one instead, as in the attached. Fine by me. Thanks! -- Michael signature.asc Description: PGP signature
Re: ALTER TABLE ... SET DATA TYPE removes statistics
On Mon, Oct 18, 2021 at 05:15:59PM -0300, Euler Taveira wrote: > I agree that it might surprise an user and it would be good to document it. > However, it does not belong to the description. I would add it to the Notes > section at the end of the ALTER TABLE page. No objections to the suggested addition and the location of the addition (paragraph of SET DATA TYPE rather than "Notes"), but I think that the phrasing could be better: "The column's statistics are removed, hence a follow-up ANALYZE is suited to update the statistics to the new column type." -- Michael signature.asc Description: PGP signature
Re: Minor doc fixes
On Mon, Sep 27, 2021 at 06:07:57PM +0900, Michael Paquier wrote: > Thanks. These are good catches. Now that 14.0 has been tagged, applied. -- Michael signature.asc Description: PGP signature
Re: Minor doc fixes
On Mon, Sep 27, 2021 at 11:42:40AM +0300, Ekaterina Kiryanova wrote: > I've come across some minor inconsistencies in PostgreSQL 14 docs. > Please review the attached patch. Thanks. These are good catches. -- Michael signature.asc Description: PGP signature
Re: Missing mention of autovacuum_work_mem
On Sat, Sep 25, 2021 at 07:51:15AM +0900, Michael Paquier wrote: > WFM. This has been applied and backpatched as of 1ba8410. -- Michael signature.asc Description: PGP signature
Re: create event trigger docs
On Fri, Sep 24, 2021 at 02:36:58PM -0400, rir wrote: > The CREATE EVENT TRIGGER synopsis uses the > bare word 'filter_value' when it seems > filter_variable > was intended. Right, there was one incorrect place. Thanks! -- Michael signature.asc Description: PGP signature
Re: Missing mention of autovacuum_work_mem
On Fri, Sep 24, 2021 at 12:21:40PM -0300, Euler Taveira wrote: > Good point. However, I prefer "if set". WFM. -- Michael signature.asc Description: PGP signature
Re: Missing mention of autovacuum_work_mem
On Thu, Sep 23, 2021 at 10:48:37AM +0200, Laurenz Albe wrote: > Thanks, and +1 from me. maintenance_work_mem would be used in the context of autovacuum if autovacuum_work_mem is -1, but it seems to me that the suggested wording sounds like only autovacuum_work_mem is used and that it would never fall back to maintenance_work_mem, no? I would suggest the addition of "if specified" in the new part within parenthesis -- Michael signature.asc Description: PGP signature
Re: Inaccurate usage of "which" instead of "that" in parallel.sgml
On Wed, Sep 01, 2021 at 10:34:42PM +0900, Michael Paquier wrote: > Thanks Elena for caring about that. Agreed that these are > improvements. And done. There were extra conflicts with 10 and 9.6 simple enough to solve, so backpatched all the way down. -- Michael signature.asc Description: PGP signature
Re: Inaccurate usage of "which" instead of "that" in parallel.sgml
On Wed, Sep 01, 2021 at 01:24:18PM +0300, Elena Indrupskaya wrote: > Since the content of parallel.sgml is not new, the documentation for earlier > versions of PostgreSQL has the same inaccuracies, and it would be great to > fix them there too. The patch also applies to versions 12 and 13, except one > @@ section). Thanks Elena for caring about that. Agreed that these are improvements. -- Michael signature.asc Description: PGP signature
Re: Error building for 64-bit Windows (10)
On Thu, May 27, 2021 at 07:21:16PM +0900, 近藤雄太 wrote: > First, the config option that specified the kerberos installation > directory was still "krb5", so we changed it to "gss". Thanks Kondo-san. Wow. So this has been actually untested for many years. The stuff got renamed in the MSVC scripts as of 74a72ec from 2014. > Secondly, kerberos SDK (include, lib) was not installed on hamerkop, so > we installed it. After that, we made sure that the "include" (not "inc") > directory existed under the kerberos installation directory > "C:\Program Files\MIT\Kerberos" which we set to "gss". > > Please check. From what I can see, hamerkop now fails to build, which is what I would expect with the current code. I'll go fix this stuff and backpatch, that should bring this buildfarm to green. -- Michael signature.asc Description: PGP signature
Re: pg_type_d.h location incorrect
On Wed, May 26, 2021 at 05:40:45AM +, tanghy.f...@fujitsu.com wrote: > You're right, backtick notation doesn't work at my windows machine. > But I made the fix in accordance with the pg-doc as below. So maybe > we need a fix there, too? > > https://www.postgresql.org/docs/current/libpq-pgservice.html > a system-wide file at `pg_config --sysconfdir`/pg_service.conf Using catalog/pg_type_d.h sounds enough to me as well. While on it, we could adjust include/common/relpath.h in pgbuffercache.sgml? We tend to include the full path for things not generated. -- Michael signature.asc Description: PGP signature
Re: No ENABLE_GSS in generated Visual Studio project files
On Mon, May 24, 2021 at 07:39:27PM +, PG Doc comments form wrote:
> I tried to build using Visual Studio 2019.
> Although I have enable the kerberos support in the configuration file,
> the generated Visual Studio project files do not define "ENABLE_GSS".
> Running the binaries (as a service) resulted in the following error in the
> log file:
I may be missing something, of course, but the MSVC build code of HEAD
does the following in Solution.pm:
ENABLE_GSS => $self->{options}->{gss} ? 1 : undef,
So perhaps you really forgot to specify a path in config.pl?
--
Michael
signature.asc
Description: PGP signature
Re: pg_monitor role description
On Thu, May 20, 2021 at 06:11:40AM +, PG Doc comments form wrote:
> "This role is a member of pg_read_all_settings, pg_read_all_stats and
> pg_stat_scan_tables."
> Is it correct sentence?
> It seems for me that pg_read_all_stats is a member of pg_monitor. But not
> vice versa.
Here is what I am getting:
=# \dgS pg_monitor
List of roles
Role name | Attributes | Member of
+--+--
pg_monitor | Cannot login |
{pg_read_all_settings,pg_read_all_stats,pg_stat_scan_tables}
=# \dgS pg_read_all_data
List of roles
Role name | Attributes | Member of
--+--+---
pg_read_all_data | Cannot login | {}
So the docs look correct to me.
--
Michael
signature.asc
Description: PGP signature
Re: Error building for 64-bit Windows (10)
On Thu, May 20, 2021 at 11:04:05AM +0900, 近藤雄太 wrote: >> @ Owners of hamerkop, could you look at updating the krb5 installation >> on this animal? This include path used seems out of date compared to >> the MSI installers upstream provides. > > OK. We'll have it done in a few days. Thanks! -- Michael signature.asc Description: PGP signature
Re: Error building for 64-bit Windows (10)
On Wed, May 19, 2021 at 10:56:23AM -0400, Andrew Dunstan wrote: > I have no idea - it's not my animal. Maybe ask the owner > Oops, I thought that this was yours. I am adding those folks in CC of this thread. > I guess maybe I should look at adding kerberos to one of the animals I > do control. That would be nice. I think that hamerkop should really update its installation of krb5 as a first step. @ Owners of hamerkop, could you look at updating the krb5 installation on this animal? This include path used seems out of date compared to the MSI installers upstream provides. -- Michael signature.asc Description: PGP signature
Re: Error building for 64-bit Windows (10)
Hi Andrew, On Tue, May 18, 2021 at 08:15:35AM -0500, Brian Ye wrote: > Thanks for the reply. > Yes I also saw that after installing 64-bit, the 32-bit "bin" and "include" > directories were removed. > I think the content of the "include" are common for both 32- and 64-bit. > Windows can run both 32-bit and > 64-bit binaries so removing these 2 directories is probably okay. Just my > guess. > Again, thanks! > Brian Ye hamerkop is the only buildfarm member testing krb5 builds with MSVC on Windows. The path used in this case is c:\Program Files\MIT\Kerberos\ so the patch of this thread is going to break the builds there if this relies on "inc/" for the include path. Is this the original include folder used for this installation of krb5? Thanks, -- Michael signature.asc Description: PGP signature
Re: Error building for 64-bit Windows (10)
On Mon, May 17, 2021 at 08:07:02PM +, PG Doc comments form wrote:
> The Solution.pm file has the following lines:
> if ($self->{options}->{gss})
> {
> $proj->AddIncludeDir($self->{options}->{gss} . '\inc\krb5');
> $proj->AddLibrary($self->{options}->{gss} .
> '\lib\i386\krb5_32.lib');
> $proj->AddLibrary($self->{options}->{gss} .
> '\lib\i386\comerr32.lib');
> $proj->AddLibrary($self->{options}->{gss} .
> '\lib\i386\gssapi32.lib');
> }
> I had to change them to the following or the compiling failed:
> if ($self->{options}->{gss})
> {
> $proj->AddIncludeDir($self->{options}->{gss} . '\include');
> $proj->AddIncludeDir($self->{options}->{gss} . '\include\krb5');
> $proj->AddLibrary($self->{options}->{gss} .
> '\lib\amd64\krb5_64.lib');
> $proj->AddLibrary($self->{options}->{gss} .
> '\lib\amd64\comerr64.lib');
> $proj->AddLibrary($self->{options}->{gss} .
> '\lib\amd64\gssapi64.lib');
Yes, you are right. I have been playing with the deliverables we
recommend in the docs as of [1], and there are a couple of gotchas
here:
- For the 32b and 64b MSI installer, the include path is not "inc",
but "include". So I could not get the installation on Win32 to work
either on HEAD.
- There is a sub-path called "include/krb5", which is not really
necessary except if we use krb5.h, but we don't. Upstream code
recommends actually to use krb5/krb5.h, meaning that only "include/"
would be sufficient. Keeping "include/krb5/" around is not a big deal
either.
This has not been changed in ages, so perhaps few have bothered.
Anyway, the attached patch fixes both the 32b and 64b builds for me.
Another interesting thing is that the installation of krb5 for amd64
and i386 cannot co-exist together, so installing one removes the
second automatically.
[1]: https://web.mit.edu/Kerberos/dist/index.html
--
Michael
diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index 3c5fe5dddc..05866af925 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -1023,10 +1023,20 @@ sub AddProject
}
if ($self->{options}->{gss})
{
- $proj->AddIncludeDir($self->{options}->{gss} . '\inc\krb5');
- $proj->AddLibrary($self->{options}->{gss} . '\lib\i386\krb5_32.lib');
- $proj->AddLibrary($self->{options}->{gss} . '\lib\i386\comerr32.lib');
- $proj->AddLibrary($self->{options}->{gss} . '\lib\i386\gssapi32.lib');
+ $proj->AddIncludeDir($self->{options}->{gss} . '\include');
+ $proj->AddIncludeDir($self->{options}->{gss} . '\include\krb5');
+ if ($self->{platform} eq 'Win32')
+ {
+ $proj->AddLibrary($self->{options}->{gss} . '\lib\i386\krb5_32.lib');
+ $proj->AddLibrary($self->{options}->{gss} . '\lib\i386\comerr32.lib');
+ $proj->AddLibrary($self->{options}->{gss} . '\lib\i386\gssapi32.lib');
+ }
+ else
+ {
+ $proj->AddLibrary($self->{options}->{gss} . '\lib\amd64\krb5_64.lib');
+ $proj->AddLibrary($self->{options}->{gss} . '\lib\amd64\comerr64.lib');
+ $proj->AddLibrary($self->{options}->{gss} . '\lib\amd64\gssapi64.lib');
+ }
}
if ($self->{options}->{iconv})
{
signature.asc
Description: PGP signature
Re: [DOC] pg_stat_replication_slots representation style inconsisitant
On Fri, Apr 30, 2021 at 03:48:27AM +, tanghy.f...@fujitsu.com wrote: > Indeed, column added. Thanks. I was looking at this thread, and please note that there is no need to do anything here as the WAL prefetch has been reverted for now as of c2dc1934. -- Michael signature.asc Description: PGP signature
Re: location of pgpass.conf
On Sun, May 02, 2021 at 11:27:22AM +0800, Julien Rouhaud wrote: > I think you can open %APPDATA% in Windows explorer to get the correct location > if needed. Yes, specifying an environment variable in the explorer works. This is in line with what src/port/path.c does, so I see no actual bugs here. If we were to change this path, I would be scared about the amount of breakages this would create, see a3f98d5 from 2005 that began using %APPDATA% as equivalent for $HOME. -- Michael signature.asc Description: PGP signature
Re: Typo in DATATYPE-JSONPATH
On Fri, Apr 16, 2021 at 08:11:53AM +0200, Erik Rijkers wrote: > FWIW, I agree with this and already sent a doc-patch [1], which is > be applied from 12, 13, and master. Arf. I completely forgot about this thread but I marked it as something to look at. Will do so now. -- Michael signature.asc Description: PGP signature
Re: INCLUDING COMPRESSION
On Thu, Apr 15, 2021 at 11:24:07PM +0900, Fujii Masao wrote: > I'm not sure why. But +1 to make them in alphabetical order. > Patch attached. LGTM. -- Michael signature.asc Description: PGP signature
Re: Typo in psql doc
On Thu, Apr 15, 2021 at 09:55:53AM +0200, Ludovic Kuty wrote: > Yes indeed, thanks. I re-tested the example this morning and it worked > correctly with the space. I guess I had messed up things when I first tried > it. After seeing Tom's argument that this behavior become more consistent in 9.2, and that e4c7619 introduced it initially because of what ~9.1 was doing, I have considered both points and just applied a small patch for HEAD with 1840d9f to remove the space. -- Michael signature.asc Description: PGP signature
Re: INCLUDING COMPRESSION
On Wed, Apr 14, 2021 at 11:46:58PM +0900, Fujii Masao wrote:
> The syntax for like_option in CREATE TABLE docs seems to forget to mention
> INCLUDING COMPRESSION option. I think the following fix is necessary.
> Patch attached.
>
> -{ INCLUDING | EXCLUDING } { COMMENTS | CONSTRAINTS | DEFAULTS | GENERATED |
> IDENTITY | INDEXES | STATISTICS | STORAGE | ALL }
> +{ INCLUDING | EXCLUDING } { COMMENTS | COMPRESSION | CONSTRAINTS | DEFAULTS
> | GENERATED | IDENTITY | INDEXES | STATISTICS | STORAGE | ALL }
Indeed. May I ask at the same time why gram.y (TableLikeOption) and
parsenodes.h (CREATE_TABLE_LIKE_COMPRESSION) don't classify this new
option in alphabetical order with the rest? Ordering them makes
easier a review of them.
--
Michael
signature.asc
Description: PGP signature
Re: Typo in psql doc
On Wed, Apr 14, 2021 at 10:12:02AM -0400, Tom Lane wrote: > As you say, both ways now give the same result. Since it's not the > point of this example to illustrate \set's space-eating behavior, > it might be clearer to revert the addition of the space. Oh, interesting point. I did not notice that this was different before. -- Michael signature.asc Description: PGP signature
Re: Typo in psql doc
On Tue, Apr 13, 2021 at 07:57:54AM +, PG Doc comments form wrote: > There is a spurious space inside the documentation: > \set HISTFILE ~/.psql_history- :DBNAME > instead of > \set HISTFILE ~/.psql_history-:DBNAME Both commands sey in your .psqlrc results in the same path being used, as I guess that psqlscanslash.l eats all the whitespaces in-between. So the documentation is not wrong here (see also commit e4c7619). -- Michael signature.asc Description: PGP signature
Re: mingw.org fails to load anything of value
On Sun, Apr 04, 2021 at 09:47:08AM -0400, Jonathan S. Katz wrote: > Diving deeper while updating the links, I stumbled across this note: > > "The MinGW.org web‑site is undergoing an overhaul, whilst in the process > of transferring to a new hosting provider. During this transitional > phase, some pages may be temporarily unavailable."[1] > > So it appears this is part of a migration. > > Anyway, here is a patch updating the URLs. Are we sure that mingw.org is completely dead? The message showing up on their site seems to mean that this URL change is just temporary? -- Michael signature.asc Description: PGP signature
Re: fix old confusing JSON example
On Sat, Apr 03, 2021 at 02:01:38PM +0200, Erik Rijkers wrote: > Attached is a small but confusing mistake in the json documentation > (a @@ instead of @?) that has been there since version 12. (It took > me quite some time to figure that out while testing with the recent > SQL/JSON patches -- which I initially blamed). Please note that pgsql-committers is the mailing list with emails generated automatically for each commit done in the main repository. For anything related to the docs, pgsql-docs is more adapted, so I am redirecting this thread there. -- Michael signature.asc Description: PGP signature
Re: TLS acronym
On Thu, Mar 25, 2021 at 12:19:36AM +0100, Daniel Gustafsson wrote: > As discussed in the NSS thread, we've had TLS defined as an since > commit c6763156589 in 2014 without actually having it defined in > acronyms.sgml. > > The attached adds the definition linking to the Wikipedia entry for TLS. Sounds like a good idea to me. Thanks! -- Michael signature.asc Description: PGP signature
Re: incoorect restore_command
On Thu, Feb 25, 2021 at 06:03:57PM +0900, Fujii Masao wrote: > Regarding the section "Standalone Hot Backups", all the directories and > file seem to be placed under /var/lib/pgsql, so at least for me it looks a bit > strange to change only the path of archive directory. So I don't think that > we need to do this change. > > -tar -rf /var/lib/pgsql/backup.tar /var/lib/pgsql/archive/ > +tar -rf /var/lib/pgsql/backup.tar /mnt/server/archivedir/ > > Same as above. Okay. I found the thing a bit inconsistent while looking at the whole picture. Anyway, dropped those two parts. > -archive_command = 'gzip < %p > /var/lib/pgsql/archive/%f' > +archive_command = 'gzip < %p > /mnt/server/archivedir/%f.gz' > >You will then need to use gunzip during > recovery: > > -restore_command = 'gunzip < /mnt/server/archivedir/%f > %p' > +restore_command = 'gunzip < /mnt/server/archivedir/%f.gz > %p' > > LGTM. Thanks for the patch! Thanks for the review. I have applied only this part, then. -- Michael signature.asc Description: PGP signature
Re: pgbench: supports PGDATABASE / warning about -d
On Wed, Feb 24, 2021 at 02:29:31PM +0900, Michael Paquier wrote: > Yeah, let's fix that. Adding only PGDATABASE to the list of > environment variables supported does not look enough to me, so I > propose the simple patch attached to clarify what happens to dbname in > more details. Applied this one as of 9e9b5c0. 5aaa584 is the original change that updated the doc of pgbench to mention all the supported environment variable, so this has been backpatched down to 12. -- Michael signature.asc Description: PGP signature
Re: incoorect restore_command
On Wed, Feb 24, 2021 at 08:15:59PM +0900, Fujii Masao wrote:
> But I have one question; why do those commands use different
> archive directories? Isn't it better to use the same one?
storage.sgml uses /var/lib/pgsql/data for the location of the data,
and the archive path is a mix between /mnt/server/archivedir/ and
/var/lib/pgsql/archive/. However, the former is used for
pg_archivecleanup and in postgresql.conf.sample, so why not just using
/mnt/server/archivedir/ in backup.sgml? Please see the attached.
--
Michael
diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index 3c8aaed0b6..d8a60f7529 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -1484,7 +1484,7 @@ restore_command = 'cp /mnt/server/archivedir/%f %p'
on, and set up an archive_command that performs
archiving only when a switch file exists. For example:
-archive_command = 'test ! -f /var/lib/pgsql/backup_in_progress || (test ! -f /var/lib/pgsql/archive/%f && cp %p /var/lib/pgsql/archive/%f)'
+archive_command = 'test ! -f /var/lib/pgsql/backup_in_progress || (test ! -f /mnt/server/archivedir/%f && cp %p /mnt/server/archivedir/%f)'
This command will perform archiving when
/var/lib/pgsql/backup_in_progress exists, and otherwise
@@ -1501,7 +1501,7 @@ psql -c "select pg_start_backup('hot_backup');"
tar -cf /var/lib/pgsql/backup.tar /var/lib/pgsql/data/
psql -c "select pg_stop_backup();"
rm /var/lib/pgsql/backup_in_progress
-tar -rf /var/lib/pgsql/backup.tar /var/lib/pgsql/archive/
+tar -rf /var/lib/pgsql/backup.tar /mnt/server/archivedir/
The switch file /var/lib/pgsql/backup_in_progress is
created first, enabling archiving of completed WAL files to occur.
@@ -1520,11 +1520,11 @@ tar -rf /var/lib/pgsql/backup.tar /var/lib/pgsql/archive/
If archive storage size is a concern, you can use
gzip to compress the archive files:
-archive_command = 'gzip < %p > /var/lib/pgsql/archive/%f'
+archive_command = 'gzip < %p > /mnt/server/archivedir/%f.gz'
You will then need to use gunzip during recovery:
-restore_command = 'gunzip < /mnt/server/archivedir/%f > %p'
+restore_command = 'gunzip < /mnt/server/archivedir/%f.gz > %p'
signature.asc
Description: PGP signature
Re: Inaccuracy in wal_receiver_status_interval parameter description
On Wed, Feb 24, 2021 at 11:16:57PM +1000, Dmitriy Kuzmin wrote: > Will this change be made in the documentation for all Postgresql versions? This wording has been introduced back in 2011 as of b186523, and nobody complained about that until now, so I did not see a strong need to back-patch it. Would people prefer a back-patch for that? -- Michael signature.asc Description: PGP signature
Re: incoorect restore_command
On Wed, Feb 24, 2021 at 07:24:17AM +0100, Philipp Gramzow wrote: > I agree, using a proper extension would be more straightforward. I'm sure > that's the reason why someone changed our archive_command. Okay, let's do so in the docs then. Others may have comments to offer, so I'll first wait a bit before applying my suggestion from upthread. -- Michael signature.asc Description: PGP signature
Re: incoorect restore_command
On Mon, Feb 22, 2021 at 07:36:28AM +, PG Doc comments form wrote:
> I've been trying out saving and restoring compressed archive logs. The
> restore_command stated in the docs at "25.3.6.2. Compressed Archive Logs"
> ('gunzip < /mnt/server/archivedir/%f > %p') did not work for me, because the
> archive_command ('gzip < %p > /var/lib/pgsql/archive/%f') alters the
> filename to %f.gz
On which platform please? Using a pipe with gzip does not alter the
output file name where the data is pushed to.
> I had to change the restore_command to 'gunzip <
> /mnt/server/archivedir/%f.gz > %p'.
Now, I kind of agree that compressing a file and not using a proper
.gz extension for its name can be confusing. So what about the
attached to tweak both archive_command and restore_command in this
section of the docs?
--
Michael
diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index 3c8aaed0b6..089102a599 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -1520,11 +1520,11 @@ tar -rf /var/lib/pgsql/backup.tar /var/lib/pgsql/archive/
If archive storage size is a concern, you can use
gzip to compress the archive files:
-archive_command = 'gzip < %p > /var/lib/pgsql/archive/%f'
+archive_command = 'gzip < %p > /var/lib/pgsql/archive/%f.gz'
You will then need to use gunzip during recovery:
-restore_command = 'gunzip < /mnt/server/archivedir/%f > %p'
+restore_command = 'gunzip < /mnt/server/archivedir/%f.gz > %p'
signature.asc
Description: PGP signature
Re: pgbench: supports PGDATABASE / warning about -d
On Mon, Feb 22, 2021 at 11:39:46AM +, PG Doc comments form wrote: > 1) pgbench has always supported the PGDATABASE env variable, but it is not > listed along PGHOST/PGPORT & PGUSER in > https://www.postgresql.org/docs/13/pgbench.html#id-1.9.4.10.8. > PGDATABASE is listed in the pages of the psql/pg_dump/createdb... > utilities Yeah, let's fix that. Adding only PGDATABASE to the list of environment variables supported does not look enough to me, so I propose the simple patch attached to clarify what happens to dbname in more details. > 2) In "Common options", it would be useful to add a warning that "-d means > --debug and does NOT define the database, contrary to most other PostgreSQL > utilities". > (It seems to work and the consequence is a flood of SQL orders on the > screen.) I don't agree with this part. -- Michael diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml index faa7c26b0a..2ec0580a79 100644 --- a/doc/src/sgml/ref/pgbench.sgml +++ b/doc/src/sgml/ref/pgbench.sgml @@ -151,6 +151,18 @@ pgbench options d + + dbname + + +Specifies the name of the database to test in. If this is +not specified, the environment variable +PGDATABASE is used. If that is not set, the +user name specified for the connection is used. + + + + -i --initialize @@ -900,6 +912,7 @@ pgbench options d +PGDATABASE PGHOST PGPORT PGUSER signature.asc Description: PGP signature
Re: Inaccuracy in wal_receiver_status_interval parameter description
On Sun, Feb 21, 2021 at 09:21:41PM -0300, Euler Taveira wrote: > LGTM. Thanks, applied. -- Michael signature.asc Description: PGP signature
Re: Inaccuracy in wal_receiver_status_interval parameter description
On Sat, Feb 20, 2021 at 07:49:21PM +1000, Dmitriy Kuzmin wrote: > I suppose it could be something like this: > "...Setting this parameter to zero disables status updates on a scheduled > basis completely. However there are certain conditions when updates are > still being sent. For example when startup process completes processing WAL > files or when standby is in synchronous mode and synchronous_commit is set > to remote_apply. This parameter can only be set in the postgresql.conf file > or on the server command line." That's an idea. While looking at that I found confusing that the sentence "Setting this parameter to zero disables status updates completely" was completely separate of the rest, where it sounds like even forced messages are disabled if the parameter value is zero, but I think that we should outline that this only applies to the scheduled replies. Attached is what I get. What do you think? -- Michael diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index e81141e45c..752466bb00 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -4503,15 +4503,16 @@ ANY num_sync ( signature.asc Description: PGP signature
Re: Inaccuracy in wal_receiver_status_interval parameter description
On Tue, Feb 16, 2021 at 07:24:04AM +, PG Doc comments form wrote: > The documentation says that setting wal_receiver_status_interval to 0 > disable updates of replication status completely. However walreceiver keep > sending status in some cases. For example, when startup has finished > processing WALs and start waiting for more: > https://github.com/postgres/postgres/blob/master/src/backend/access/transam/xlog.c#L12598-L12609 > It would be helpful to document in what cases status updates are still being > sent. The docs say only that: "Updates are sent each time the write or flush positions change, or at least as often as specified by this parameter." So it could make sense to complete a bit this paragraph with some words about the places where WalRcvForceReply() or similar logic is used. The case of the end of the WAL stream ending would be an extra one. How would you formulate that and what are the cases you think would be worth mentioning? -- Michael signature.asc Description: PGP signature
Re: is_local parameter on set_config function
On Thu, Jan 28, 2021 at 11:32:42AM +, PG Doc comments form wrote: > I have just used the set_config(...) function for the first time and it took > me a bit of experimenting to understand the effect of the is_local > parameter. My suggestion would be to add a bit of explanation on that. This is equivalent to the LOCAL clause in the SET query: https://www.postgresql.org/docs/9.5/sql-set.html -- Michael signature.asc Description: PGP signature
Re: TRUNCATE VIEW
On Tue, Jan 26, 2021 at 09:54:48AM +0900, Michael Paquier wrote: > Yes, this page is wrong to tell that. This one has been fixed with 32bef75 down to 12. > There is a second thing that I find confusing in the docs of > TRUNCATE: > https://www.postgresql.org/docs/devel/sql-truncate.html > > This does not mention at all partitioned tables, describing only > "descendant tables". Partitioning is a case of "descendant tables" > but that's a bit confusing IMO and a reader would need to guess that. Not sure what to do about this part yet. -- Michael signature.asc Description: PGP signature
Re: TRUNCATE VIEW
On Mon, Jan 25, 2021 at 08:53:39PM +0530, harisai hari wrote: > Thank you! Yes, this page is wrong to tell that. There is a second thing that I find confusing in the docs of TRUNCATE: https://www.postgresql.org/docs/devel/sql-truncate.html This does not mention at all partitioned tables, describing only "descendant tables". Partitioning is a case of "descendant tables" but that's a bit confusing IMO and a reader would need to guess that. Thoughts? -- Michael signature.asc Description: PGP signature
Re: scram-sha-256 authentication
On Tue, Jan 05, 2021 at 09:12:58AM -0500, Jonathan S. Katz wrote: > I am not sure what your end goal is, but there are a few ways to create > the hashed SCRAM verifier: > > - Using the \password flag in "psql" > - Using one of the connection drivers that interfaces with libpq's > PQencryptPasswordConn function[2] > - Some driver's handle the password hashing independently Another thing to be careful about is the value of password_encryption in postgresql.conf. The default has been changed to scram-sha-256 in c7eab0e, meaning that this change will be available in Postgres 14~. But if your environment is using the default configuration of 11, that may be set to "md5". -- Michael signature.asc Description: PGP signature
Re: doc: index items for pg_stat_progress_xxx views
On Thu, Nov 12, 2020 at 05:05:10PM +0900, Fujii Masao wrote: > The index items for pg_stat_progress_xxx views point to the > "Viewing Statistics" section, but not to the dedicated section > (e.g., "ANALYZE Progress Reporting") for each view. IMO this is > very inconvenient when finding the section describing each > pg_stat_progress_xxx view, from the index. So what about adding > new pointer to the section for each view in the index? > Patch attached. +1. -- Michael signature.asc Description: PGP signature
Re: replication wordsmithing / clarifications
On Sun, Oct 04, 2020 at 01:08:56PM -0400, Robert Treat wrote: > Merci monsieur! De rien, monsieur. -- Michael signature.asc Description: PGP signature
