RE: add \dpS to psq [16beta1]

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi,
Thank you for developing a good feature.
I found while testing PostgreSQL 16 Beta 1 that the output of the \? 
metacommand did not include \dS, \dpS. 
The attached patch changes the output of the \? meta command to:

Current output
psql=# \? 
  \z  [PATTERN]  same as \dp
  \dp [PATTERN]  list table, view, and sequence access privileges

Patched output
psql=# \?
  \dp[S]  [PATTERN]  list table, view, and sequence access privileges
  \z[S]   [PATTERN]  same as \dp

Regards,
Noriyoshi Shinoda

-Original Message-
From: Nathan Bossart  
Sent: Tuesday, January 10, 2023 2:46 AM
To: Dean Rasheed 
Cc: Maxim Orlov ; Andrew Dunstan ; 
Michael Paquier ; Tom Lane ; Isaac 
Morland ; Justin Pryzby ; Pavel 
Luzanov ; pgsql-hack...@postgresql.org
Subject: Re: add \dpS to psql

On Sat, Jan 07, 2023 at 11:18:59AM +, Dean Rasheed wrote:
> It might be true that temp tables aren't usually interesting from a 
> permissions point of view, but it's not hard to imagine situations 
> where interesting things do happen. It's also probably the case that 
> most users won't have many temp tables, so I don't think including 
> them by default will be particularly intrusive.
> 
> Also, from a user perspective, I think it would be something of a POLA 
> violation for \dp[S] and \dt[S] to include different sets of tables, 
> though I appreciate that we do that now. There's nothing in the docs 
> to indicate that that's the case.

Agreed.

> Anyway, I've pushed the v2 patch as-is. If anyone feels strongly 
> enough that we should change its behaviour for temp tables, then we 
> can still discuss that.

Thanks!

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com 




psql_dpS_metacommand_v1.diff
Description: psql_dpS_metacommand_v1.diff

RE: [16Beta1][doc] pgstat: Track time of the last scan of a relation

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi, David.

> I've now pushed this change.
Thank you so much.

-Original Message-
From: David Rowley  
Sent: Monday, June 5, 2023 2:37 PM
To: Shinoda, Noriyoshi (PN Japan FSIP) 
Cc: PostgreSQL-development ; dp...@pgadmin.org; 
and...@anarazel.de; br...@momjian.us; v...@postgresfriends.org
Subject: Re: [16Beta1][doc] pgstat: Track time of the last scan of a relation

On Wed, 31 May 2023 at 15:57, Shinoda, Noriyoshi (PN Japan FSIP) 
 wrote:
>  PostgreSQL 16 Beta1, added last access time to pg_stat_all_tables and 
> pg_stat_all_indexes views by this patch [1].
> According to the documentation [2], the data type of the columns added to 
> these views is 'timestamptz'.
> However, columns of the same data type in pg_stat_all_tables.last_vacuum, 
> last_analyze and other tables are unified to 'timestamp with time zone'. The 
> attached patch changes the data type of the added column from timestamptz to 
> timestamp with time zone.

I've now pushed this change.

David

RE: [16Beta1][doc] pgstat: Track time of the last scan of a relation

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi, Thanks for your comment.
As you say, it would be difficult to unify the data types in all documents 
right now.
The patch I attached the other day unifies only the newly added columns in 
monitoring.sgml to "timestamp with time zone".

Regards,
Noriyoshi Shinoda
-Original Message-
From: David Rowley  
Sent: Wednesday, May 31, 2023 3:14 PM
To: Shinoda, Noriyoshi (PN Japan FSIP) 
Cc: PostgreSQL-development ; dp...@pgadmin.org; 
and...@anarazel.de; br...@momjian.us; v...@postgresfriends.org
Subject: Re: [16Beta1][doc] pgstat: Track time of the last scan of a relation

On Wed, 31 May 2023 at 15:57, Shinoda, Noriyoshi (PN Japan FSIP) 
 wrote:
> According to the documentation [2], the data type of the columns added to 
> these views is 'timestamptz'.
> However, columns of the same data type in pg_stat_all_tables.last_vacuum, 
> last_analyze and other tables are unified to 'timestamp with time zone'. The 
> attached patch changes the data type of the added column from timestamptz to 
> timestamp with time zone.

I agree that it would be good to make those consistently use timestamp with 
time zone for all columns of that type in the docs for pg_stat_all_tables.

More generally, it might be good if we did it for the entire docs:

doc $ git grep "timestamptz" | wc -l
17
doc $ git grep "timestamp with time zone" | wc -l
74

Clearly "timestamp with time zone" is much more commonly used.

The bar is probably set a bit higher for changing the longer-established ones, 
however.

David

[16Beta1][doc] pgstat: Track time of the last scan of a relation

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi, hackers.
 PostgreSQL 16 Beta1, added last access time to pg_stat_all_tables and 
pg_stat_all_indexes views by this patch [1].
According to the documentation [2], the data type of the columns added to these 
views is 'timestamptz'. 
However, columns of the same data type in pg_stat_all_tables.last_vacuum, 
last_analyze and other tables are unified to 'timestamp with time zone'. The 
attached patch changes the data type of the added column from timestamptz to 
timestamp with time zone.

[1] pgstat: Track time of the last scan of a relation
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=c037471832e1ec3327f81eebbd8892e5c1042fe0
[2] pg_stat_activity view
https://www.postgresql.org/docs/16/monitoring-stats.html#MONITORING-PG-STAT-ALL-TABLES-VIEW

Regards,
Noriyoshi Shinoda


pg_stat_all_tables_doc_v1.diff
Description: pg_stat_all_tables_doc_v1.diff

[16Beta1][doc] Add BackendType for standalone backends

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi, hackers.
In PostgreSQL 16 Beta 1, standalone backend was added to the backend type by 
this patch [1]. I think this patch will change the value of backend_type column 
in the pg_stat_activity view, but it's not explained in the documentation. The 
attached patch fixes monitoring.sgml.

[1] Add BackendType for standalone backends
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=0c679464a837079acc75ff1d45eaa83f79e05690

Regards, 
Noriyoshi Shinoda


monitoring_sgml_v1.diff
Description: monitoring_sgml_v1.diff

RE: running logical replication as the subscription owner

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi hackers,
Thank you for developing a great feature. 
The following commit added a column to the pg_subscription catalog.
 
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=482675987bcdffb390ae735cfd5f34b485ae97c6
 
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=c3afe8cf5a1e465bd71e48e4bc717f5bfdc7a7d6

I found that the documentation of the pg_subscription catalog was missing an 
explanation of the columns subrunasowner and subpasswordrequired, so I attached 
a patch. Please fix the patch if you have a better explanation.

Regards,
Noriyoshi Shinoda

-Original Message-
From: Robert Haas  
Sent: Wednesday, April 5, 2023 1:10 AM
To: Noah Misch 
Cc: Jeff Davis ; Jelte Fennema ; 
pgsql-hack...@postgresql.org; Andres Freund 
Subject: Re: running logical replication as the subscription owner

On Mon, Apr 3, 2023 at 10:09 PM Noah Misch  wrote:
> I gather we agree on what to do for v16, which is good.

I have committed the patches.

-- 
Robert Haas
EDB: http://www.enterprisedb.com 




pg_subscription_doc_v1.diff
Description: pg_subscription_doc_v1.diff

RE: run pgindent on a regular basis / scripted manner

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi,
I tried the committed pgindent.
The attached small patch changes spaces in the usage message to tabs.
Options other than --commit start with a tab.

Regards,
Noriyoshi Shinoda

From: Andrew Dunstan 
Sent: Thursday, February 9, 2023 7:10 AM
To: Jelte Fennema 
Cc: Robert Haas ; Tom Lane ; Justin 
Pryzby ; Andres Freund ; Noah Misch 
; Peter Geoghegan ; Bruce Momjian 
; Magnus Hagander ; Alvaro Herrera 
; Stephen Frost ; Jesse Zhang 
; pgsql-hack...@postgresql.org
Subject: Re: run pgindent on a regular basis / scripted manner



On 2023-02-08 We 12:06, Jelte Fennema wrote:

With the new patch --commit works as expected for me now. And sounds

good to up the script a bit afterwards.






Thanks, I have committed this. Still looking at Robert's other request.



cheers



andrew

--

Andrew Dunstan

EDB: https://www.enterprisedb.com


pgindent_usage_v1.diff
Description: pgindent_usage_v1.diff

RE: Perform streaming logical transactions by background workers and parallel apply

[Shinoda, Noriyoshi (PN Japan FSIP)]

Thanks for the reply.

> Thanks for reporting. I think for LogicalParallelApplyStateChange we'd better 
> document it in a consistent style with LogicalSyncStateChange, 
> so I have slightly adjusted the patch for the same.

I think the description in the patch you attached is better.

Regards,
Noriyoshi Shinoda

-Original Message-
From: houzj.f...@fujitsu.com  
Sent: Monday, January 9, 2023 7:15 PM
To: Shinoda, Noriyoshi (PN Japan FSIP) ; Amit Kapila 

Cc: Masahiko Sawada ; wangw.f...@fujitsu.com; Peter 
Smith ; shiy.f...@fujitsu.com; PostgreSQL Hackers 
; Dilip Kumar 
Subject: RE: Perform streaming logical transactions by background workers and 
parallel apply

On Monday, January 9, 2023 5:32 PM Shinoda, Noriyoshi (PN Japan FSIP) 
 wrote:
> 
> Hi, Thanks for the great new feature.
> 
> Applied patches include adding wait events LogicalParallelApplyMain, 
> LogicalParallelApplyStateChange.
> However, it seems that monitoring.sgml only contains descriptions for 
> pg_locks. The attached patch adds relevant wait event information.
> Please update if you have a better description.

Thanks for reporting. I think for LogicalParallelApplyStateChange we'd better 
document it in a consistent style with LogicalSyncStateChange, so I have 
slightly adjusted the patch for the same.

Best regards,
Hou zj

RE: Perform streaming logical transactions by background workers and parallel apply

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi, Thanks for the great new feature.

Applied patches include adding wait events LogicalParallelApplyMain, 
LogicalParallelApplyStateChange. 
However, it seems that monitoring.sgml only contains descriptions for pg_locks. 
The attached patch adds relevant wait event information.
Please update if you have a better description.

Noriyoshi Shinoda
-Original Message-
From: Amit Kapila  
Sent: Monday, January 9, 2023 5:51 PM
To: houzj.f...@fujitsu.com
Cc: Masahiko Sawada ; wangw.f...@fujitsu.com; Peter 
Smith ; shiy.f...@fujitsu.com; PostgreSQL Hackers 
; Dilip Kumar 
Subject: Re: Perform streaming logical transactions by background workers and 
parallel apply

On Sun, Jan 8, 2023 at 11:32 AM houzj.f...@fujitsu.com  
wrote:
>
> On Sunday, January 8, 2023 11:59 AM houzj.f...@fujitsu.com 
>  wrote:
> > Attach the updated patch set.
>
> Sorry, the commit message of 0001 was accidentally deleted, just 
> attach the same patch set again with commit message.
>

Pushed the first (0001) patch.

--
With Regards,
Amit Kapila.




monitoring_wait_event_v1.diff
Description: monitoring_wait_event_v1.diff

RE: pg_upgrade: Make testing different transfer modes easier

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi, 
With the addition of --copy option, pg_upgrade now has three possible transfer 
mode options. Currently, an error does not occur even if multiple transfer 
modes are specified. For example, we can also run "pg_upgrade --link --copy 
--clone" command. As discussed in Horiguchi-san's previous email, options like 
"--mode=link|copy|clone" can prevent this problem.
The attached patch uses the current implementation and performs a minimum check 
to prevent multiple transfer modes from being specified.

Regards,
Noriyoshi Shinoda
-Original Message-
From: Peter Eisentraut  
Sent: Saturday, December 17, 2022 2:44 AM
To: Daniel Gustafsson 
Cc: PostgreSQL Hackers 
Subject: Re: pg_upgrade: Make testing different transfer modes easier

On 14.12.22 10:40, Daniel Gustafsson wrote:
>> On 14 Dec 2022, at 08:04, Peter Eisentraut 
>>  wrote:
>>
>> On 07.12.22 17:33, Peter Eisentraut wrote:
>>> I think if we want to make this configurable on the fly, and environment 
>>> variable would be much easier, like
>>>  my $mode = $ENV{PG_TEST_PG_UPGRADE_MODE} || '--copy';
>>
>> Here is an updated patch set that incorporates this idea.
> 
> I would prefer a small note about it in src/bin/pg_upgrade/TESTING to 
> document it outside of the code, but otherwise LGTM.
> 
> + $mode,
>   '--check'
>   ],
> 
> ...
> 
> - '-p', $oldnode->port, '-P', $newnode->port
> + '-p', $oldnode->port, '-P', $newnode->port,
> + $mode,
>   ],
> 
> Minor nitpick, but while in there should we take the opportunity to 
> add a trailing comma on the other two array declarations which now ends with 
> --check?
> It's good Perl practice and will make the code consistent.

committed with these changes





pg_upgrade_check_mode_v1.diff
Description: pg_upgrade_check_mode_v1.diff

RE: Improve logging when using Huge Pages

[Shinoda, Noriyoshi (PN Japan FSIP)]

Thanks for your comment. 
I understand that some people don't like noise log. However, I don't understand 
the feeling of disliking the one-line log that is output when the instance is 
started. 
In both MySQL and Oracle Database, a log is output if it fails to acquire 
HugePages with the same behavior as huge_pages=try in PostgreSQL.

Regards,
Noriyoshi Shinoda


-Original Message-
From: Thomas Munro  
Sent: Thursday, November 3, 2022 10:10 AM
To: Shinoda, Noriyoshi (PN Japan FSIP) 
Cc: Jacob Champion ; Masahiko Sawada 
; Fujii Masao ; Justin 
Pryzby ; PostgreSQL-development 
; Julien Rouhaud ; Tom Lane 
; Kyotaro Horiguchi 
Subject: Re: Improve logging when using Huge Pages

On Wed, Aug 3, 2022 at 8:42 PM Shinoda, Noriyoshi (PN Japan FSIP) 
 wrote:
> > As discussed in [1], we're taking this opportunity to return some patchsets 
> > that don't appear to be getting enough reviewer interest.
> Thank you for your helpful comments.
> As you say, my patch doesn't seem to be of much interest to reviewers either.
> I will discard the patch I proposed this time and consider it again.

I wonder if the problem here is that people are reluctant to add noise
to every starting system.   There are people who have not configured
their system and don't want to see that noise, and then some people have 
configured their system and would like to know about it if it doesn't work so 
they can be aware of that, but don't want to use "off"
because they don't want a hard failure.  Would it be better if there were a new 
level "try_log" (or something), which only logs a message if it tries and fails?

[PG15 Doc] remove "tty" connect string from manual

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hello, hackers.

As of PostgreSQL 14, "tty" in the libpq connection string has already been 
removed with the commit below.
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=14d9b37607ad30c3848ea0f2955a78436eff1268

But https://www.postgresql.org/docs/15/libpq-connect.html#LIBPQ-CONNSTRING 
still says "Ignored (formerly, this specified where to send server debug 
output)". The attached patch removes the "tty" item.

Regards,
Noriyoshi Shinoda


libpq-connect-v1.diff
Description: libpq-connect-v1.diff

RE: Improve logging when using Huge Pages

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hello,

> As discussed in [1], we're taking this opportunity to return some patchsets 
> that don't appear to be getting enough reviewer interest.
Thank you for your helpful comments.
As you say, my patch doesn't seem to be of much interest to reviewers either.
I will discard the patch I proposed this time and consider it again.

Regards,
Noriyoshi Shinoda

-Original Message-
From: Jacob Champion  
Sent: Tuesday, August 2, 2022 5:45 AM
To: Shinoda, Noriyoshi (PN Japan FSIP) ; Masahiko 
Sawada ; Fujii Masao 
Cc: Justin Pryzby ; PostgreSQL-development 
; Julien Rouhaud ; Tom Lane 
; Kyotaro Horiguchi 
Subject: Re: Improve logging when using Huge Pages

As discussed in [1], we're taking this opportunity to return some patchsets 
that don't appear to be getting enough reviewer interest.

This is not a rejection, since we don't necessarily think there's anything 
unacceptable about the entry, but it differs from a standard "Returned with 
Feedback" in that there's probably not much actionable feedback at all. Rather 
than code changes, what this patch needs is more community interest. You might

- ask people for help with your approach,
- see if there are similar patches that your code could supplement,
- get interested parties to agree to review your patch in a CF, or
- possibly present the functionality in a way that's easier to review
  overall.

(Doing these things is no guarantee that there will be interest, but it's 
hopefully better than endlessly rebasing a patchset that is not receiving any 
feedback from the community.)

Once you think you've built up some community support and the patchset is ready 
for review, you (or any interested party) can resurrect the patch entry by 
visiting

https://commitfest.postgresql.org/38/3310/ 

and changing the status to "Needs Review", and then changing the status again 
to "Move to next CF". (Don't forget the second step; hopefully we will have 
streamlined this in the near future!)

Thanks,
--Jacob

[1] https://postgr.es/m/86140760-8ba5-6f3a-3e6e-5ca6c060b...@timescale.com 

RE: Add --{no-,}bypassrls flags to createuser

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi,
Thanks to the developers and reviewers.
The attached small patch fixes the message in "createuser --help" command. The 
patch has changed to specify a time stamp for the --valid-for option. I don't 
think the SGML description needs to be modified.

Regards,
Noriyoshi Shinoda
-Original Message-
From: Michael Paquier  
Sent: Wednesday, July 13, 2022 12:25 PM
To: Kyotaro Horiguchi 
Cc: shinya11.k...@oss.nttdata.com; nathandboss...@gmail.com; 
przemys...@sztoch.pl; david.g.johns...@gmail.com; robertmh...@gmail.com; 
dan...@yesql.se; pgsql-hack...@postgresql.org
Subject: Re: Add --{no-,}bypassrls flags to createuser

On Thu, May 26, 2022 at 04:47:46PM +0900, Kyotaro Horiguchi wrote:
> FWIW, the "fancy" here causes me to think about something likely to 
> cause syntax breakage of the query to be sent.
> 
> createuser -a 'user"1' -a 'user"2' 'user"3'
> createuser -v "2023-1-1'; DROP TABLE public.x; select '" hoge

That would be mostly using spaces here, to make sure that quoting is correctly 
applied.

> BUT, thses should be prevented by the functions enumerated above. So, 
> I don't think we need them.

Mostly.  For example, the test for --valid-until can use a timestamp with 
spaces to validate the use of appendStringLiteralConn().  A second thing is 
that --member was checked, but not --admin, so I have renamed regress_user2 to 
"regress user2" for that to apply a maximum of coverage, and applied the patch.

One thing that I found annoying is that this made the list of options of 
createuser much harder to follow.  That's not something caused by this patch as 
many options have accumulated across the years and there is a kind pattern 
where the connection options were listed first, but I have cleaned up that 
while on it.  A second area where this could be done is createdb, as it could 
be easily expanded if the backend query gains support for more stuff, but that 
can happen when it makes more sense.
--
Michael


createuser_help_v1.diff
Description: createuser_help_v1.diff

RE: PG15 beta1 fix pg_stats_ext/pg_stats_ext_exprs view manual

[Shinoda, Noriyoshi (PN Japan FSIP)]

Thanks for your comment. sorry for the late reply.
I hope it will be fixed during the period of PostgreSQL 15 Beta.

Regards,

Noriyoshi Shinoda
-Original Message-
From: Justin Pryzby  
Sent: Tuesday, June 14, 2022 11:30 PM
To: Shinoda, Noriyoshi (PN Japan FSIP) 
Cc: pgsql-hack...@postgresql.org; Tomas Vondra 
Subject: Re: PG15 beta1 fix pg_stats_ext/pg_stats_ext_exprs view manual

On Tue, May 24, 2022 at 08:19:27PM -0500, Justin Pryzby wrote:
> On Wed, May 25, 2022 at 01:08:12AM +0000, Shinoda, Noriyoshi (PN Japan FSIP) 
> wrote:
> > Hi hackers,
> > Thanks to all the developers. The attached patch updates the manual for the 
> > pg_stats_ext and pg_stats_ext_exprs view.
> > The current pg_stats_ext/pg_stats_ext_exprs view manual are missing the 
> > inherited column. This column was added at the same time as the stxdinherit 
> > column in the pg_statistic_ext_data view. The attached patch adds the 
> > missing description. If there is a better description, please correct it.
> > 
> > Commit: Add stxdinherit flag to pg_statistic_ext_data
> > 
> > INVALID URI REMOVED
> > tgresql.git;a=commit;h=269b532aef55a579ae02a3e8e8df14101570dfd9__;!!
> > NpxR!kBff64MGwFvJU4EPtHmXM1YogdVCJKoc9-TAYGJxy_9p_MMVUGE0GJaL4KGVqY5
> > dTBlzhU6k0odtBi1Wv_fZ$
> > Current Manual: 
> > https://www.postgresql.org/docs/15/view-pg-stats-ext.html 
> > 
> > INVALID URI REMOVED
> > pg-stats-ext-exprs.html__;!!NpxR!kBff64MGwFvJU4EPtHmXM1YogdVCJKoc9-T
> > AYGJxy_9p_MMVUGE0GJaL4KGVqY5dTBlzhU6k0odtBvG3tq9F$
> 
> Thanks for copying me.
> 
> This looks right, and uses the same language as pg_stats and pg_statistic.
> 
> But, I'd prefer if it didn't say "inheritance child", since that now 
> sounds like it means "a child which is using inheritance" and not just "any 
> child".
> 
> I'd made a patch for that, for which I'll create a separate thread shortly.

The thread I started [0] has stalled out, so your patch seems seems fine, since 
it's consistent with pre-existing docs.

[0] https://www.postgresql.org/message-id/20220525013248.go19...@telsasoft.com 

--
Justin

PG15 beta1 fix pg_stats_ext/pg_stats_ext_exprs view manual

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi hackers,
Thanks to all the developers. The attached patch updates the manual for the 
pg_stats_ext and pg_stats_ext_exprs view.
The current pg_stats_ext/pg_stats_ext_exprs view manual are missing the 
inherited column. This column was added at the same time as the stxdinherit 
column in the pg_statistic_ext_data view. The attached patch adds the missing 
description. If there is a better description, please correct it.

Commit: Add stxdinherit flag to pg_statistic_ext_data

https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=269b532aef55a579ae02a3e8e8df14101570dfd9
Current Manual: 
https://www.postgresql.org/docs/15/view-pg-stats-ext.html
https://www.postgresql.org/docs/15/view-pg-stats-ext-exprs.html

Regards,
Noriyoshi Shinoda


pg_stats_ext_doc_v1.diff
Description: pg_stats_ext_doc_v1.diff

PG15 beta1 fix pg_database view document

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi hackers,
Thanks to all the developers. The attached patch updates the manual for the 
pg_database catalog.
The current pg_database view definition is missing the datlocprovider column. 
The attached patch adds this column info.
https://www.postgresql.org/docs/15/catalog-pg-database.html

Regards,
Noriyoshi Shinoda


pg_database_doc_v1.diff
Description: pg_database_doc_v1.diff

PG15 beta1 fix pg_stat_recovery_prefetch view manual

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi hackers,
Thanks to all the developers. The attached patch updates the manual for the 
pg_stat_recovery_prefetch view.
The current pg_stat_recovery_prefetch view definition is missing the 
stats_reset column. The attached patch adds information in the stats_reset 
column.

https://www.postgresql.org/docs/15/monitoring-stats.html#MONITORING-PG-STAT-RECOVERY-PREFETCH

Regards,
Noriyoshi Shinoda


pg_stat_recovery_prefetch_doc_v1.diff
Description: pg_stat_recovery_prefetch_doc_v1.diff

RE: PG15 beta1 fix pg_stat_statements view document

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi,

Thank you for your comment.
I attached the fixed patch.

-Original Message-
From: Michael Paquier  
Sent: Saturday, May 21, 2022 12:33 PM
To: Nathan Bossart 
Cc: Shinoda, Noriyoshi (PN Japan FSIP) ; 
PostgreSQL-development ; mag...@hagander.net
Subject: Re: PG15 beta1 fix pg_stat_statements view document

On Fri, May 20, 2022 at 04:04:29PM -0700, Nathan Bossart wrote:
> I think there is a typo in the change to the jit_optimization_time 
> section, but otherwise it looks good to me.

Yes, as of "double precisiodouble precision".  All these four fields are indeed 
doubles in the code, for what looks like a copy-pasto from 57d6aea.  Will fix.
--
Michael


pg_stat_statements_doc_v2.diff
Description: pg_stat_statements_doc_v2.diff

PG15 beta1 fix pg_stat_statements view document

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi hackers,

The attached patch modifies the pg_stat_statements view documentation updated 
in PostgreSQL 15 Beta 1.
The data type of the following columns in the pg_stat_statements view is bigint 
in the current document, 
but it is actually double precision.
jit_generation_time
jit_inlining_time
jit_optimization_time
jit_emission_time

Regards,
Noriyoshi Shinoda


pg_stat_statements_doc_v1.diff
Description: pg_stat_statements_doc_v1.diff

RE: Documentation issue with pg_stat_recovery_prefetch

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi, 

Thank you for developing the new feature.
The pg_stat_recovery_prefetch view documentation doesn't seem to have a 
description of the stats_reset column. The attached small patch adds a 
description of the stats_reset column.

Regards,
Noriyoshi Shinoda

-Original Message-
From: Thomas Munro  
Sent: Tuesday, April 12, 2022 6:23 PM
To: sirisha chamarthi 
Cc: PostgreSQL Hackers 
Subject: Re: Documentation issue with pg_stat_recovery_prefetch

On Tue, Apr 12, 2022 at 8:11 AM sirisha chamarthi  
wrote:
> I was going through pg_stat_recovery_prefetch documentation and saw an issue 
> with formatting. Attached a small patch to fix the issue. This is the first 
> time I am sending an email to hackers. Please educate me if I miss something.

Thanks Sirisha!

Ouch, that's embarrassing.  My best guess is that I might have screwed that up 
a long time ago while rebasing an early development version over commit 
92f94686, which changed the link style and moved paragraphs around, and then 
never noticed that it was wrong.
Researching that made me notice another problem: the table was using the 3 
column layout from a couple of years ago, because I had also missed the style 
change in commit a0427506.  Oops.  Fixed.




pg_stat_recovery_prefetch_doc_v1.diff
Description: pg_stat_recovery_prefetch_doc_v1.diff

RE: WIP: WAL prefetch (another approach)

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi,
Thank you for your reply. 
I missed the message, sorry.

Regards,
Noriyoshi Shinoda

-Original Message-
From: Thomas Munro  
Sent: Tuesday, April 12, 2022 6:28 PM
To: Shinoda, Noriyoshi (PN Japan FSIP) 
Cc: Justin Pryzby ; Tomas Vondra 
; Stephen Frost ; Andres 
Freund ; Jakub Wartak ; Alvaro 
Herrera ; Tomas Vondra 
; Dmitry Dolgov <9erthali...@gmail.com>; David 
Steele ; pgsql-hackers 
Subject: Re: WIP: WAL prefetch (another approach)

On Tue, Apr 12, 2022 at 9:03 PM Shinoda, Noriyoshi (PN Japan FSIP) 
 wrote:
> Thank you for developing the great feature. I tested this feature and checked 
> the documentation. Currently, the documentation for the 
> pg_stat_prefetch_recovery view is included in the description for the 
> pg_stat_subscription view.
>
> INVALID URI REMOVED
> toring-stats.html*MONITORING-PG-STAT-SUBSCRIPTION__;Iw!!NpxR!xRu7zc4Hc
> ZppB-32Fp3YfESPqJ7B4AOP_RF7QuYP-kCWidoiJ5txu9CW8sX61TfwddE$

Hi!  Thanks.  I had just committed a fix before I saw your message, because 
there was already another report here:

https://www.postgresql.org/message-id/flat/cakrakevk-lrhmdyt6x_p33ef6dcorm2jed5h_ehdrdv0res...@mail.gmail.com
 

RE: WIP: WAL prefetch (another approach)

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi, 
Thank you for developing the great feature. I tested this feature and checked 
the documentation. Currently, the documentation for the 
pg_stat_prefetch_recovery view is included in the description for the 
pg_stat_subscription view.

https://www.postgresql.org/docs/devel/monitoring-stats.html#MONITORING-PG-STAT-SUBSCRIPTION

It is also not displayed in the list of "28.2. The Statistics Collector".
https://www.postgresql.org/docs/devel/monitoring.html

The attached patch modifies the pg_stat_prefetch_recovery view to appear as a 
separate view.

Regards,
Noriyoshi Shinoda
-Original Message-
From: Thomas Munro  
Sent: Friday, April 8, 2022 10:47 AM
To: Justin Pryzby 
Cc: Tomas Vondra ; Stephen Frost 
; Andres Freund ; Jakub Wartak 
; Alvaro Herrera ; Tomas 
Vondra ; Dmitry Dolgov <9erthali...@gmail.com>; 
David Steele ; pgsql-hackers 
Subject: Re: WIP: WAL prefetch (another approach)

On Fri, Apr 8, 2022 at 12:55 AM Justin Pryzby  wrote:
> The docs seem to be wrong about the default.
>
> +are not yet in the buffer pool, during recovery.  Valid values are
> +off (the default), on and
> +try.  The setting try 
> + enables

Fixed.

> +   concurrency and distance, respectively.  By default, it is set to
> +   try, which enabled the feature on systems where
> +   posix_fadvise is available.
>
> Should say "which enables".

Fixed.

> Curiously, I reported a similar issue last year.

Sorry.  I guess both times we only agreed on what the default should be in the 
final review round before commit, and I let the docs get out of sync (well, the 
default is mentioned in two places and I apparently ended my search too soon, 
changing only one).  I also found another recently obsoleted sentence: the one 
about showing nulls sometimes was no longer true.  Removed.




pg_stat_recovery_prefetch_doc_v1.diff
Description: pg_stat_recovery_prefetch_doc_v1.diff

RE: Expose JIT counters/timing in pg_stat_statements

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi,
thank you for the great features.

The attached small patch changes the data type in the document.
The following columns are actually double precision but bigint in the docs.
jit_generation_time
jit_inlining_time
jit_optimization_time
jit_emission_count

Regards,
Noriyoshi Shinoda

From: Magnus Hagander 
Sent: Friday, April 8, 2022 8:47 PM
To: Julien Rouhaud 
Cc: PostgreSQL Developers 
Subject: Re: Expose JIT counters/timing in pg_stat_statements



On Tue, Mar 8, 2022 at 4:08 AM Julien Rouhaud 
mailto:rjuju...@gmail.com>> wrote:
On Mon, Mar 07, 2022 at 01:40:34PM +0100, Magnus Hagander wrote:
>
> I wonder if there might be an interesting middle ground, or if that is
> making it too much. That is, we could have an
> Option 3:
> jit_count
> total_jit_time - for sum of functions+inlining+optimization+emission time
> min_jit_time - for sum of functions+inlining+optimization+emission time
> max_jit_time - for sum of functions+inlining+optimization+emission time
> mean_jit_time - for sum of functions+inlining+optimization+emission time
> stddev_jit_time - for sum of functions+inlining+optimization+emission time
> jit_functions
> jit_generation_time
> jit_inlining_count
> jit_inlining_time
> jit_optimization_count
> jit_optimization_time
> jit_emission_count
> jit_emission_time
>
> That is, we'd get the more detailed timings across the total time, but
> not on the details. But that might be overkill.

I also thought about it but it seems overkill.  pg_stat_statements view is
already very big, and I think that the JIT time should be somewhat stable, at
least compared to how much a query execution time can vary depending on the
parameters.  This approach would also be a bit useless if you change the
costing of underlying JIT operation.

> But -- here's an updated patched based on Option 2.

Thanks!

Code-wide, the patch looks good.  For the doc, it seems that you documented
jit_inlining_count three times rather than documenting jit_optimization_count
and jit_emission_count.

Oops, thanks and fixed.


I don't think we can add tests there, and having a test for every new counter
being >= 0 seems entirely useless, however there should be a new test added for
the "oldextversions" test to make sure that there's no issue with old SQL / new
shlib compatibility.  And looking at it I see that it was already missed for
version 1.9 :(

Indeed. Fixed here.

Michael had already applied a patch that took us to 1.10 and added that test, 
so I've just updated it here. I don't think we normally bump the version twice 
int he same day, so I just mergd the SQL script changes as well.

PFA a "final" version for the CI to run.

--
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/


pg_stat_statements_jit_doc_v1.diff
Description: pg_stat_statements_jit_doc_v1.diff

RE: Column Filtering in Logical Replication

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hello, 

The 'prattrs' column has been added to the pg_publication_rel catalog, 
but the current commit to catalog.sgml seems to have added it to 
pg_publication_namespace. 
The attached patch fixes this.

Regards,
Noriyoshi Shinoda
-Original Message-
From: Tomas Vondra  
Sent: Saturday, March 26, 2022 9:35 AM
To: Amit Kapila 
Cc: Peter Eisentraut ; 
houzj.f...@fujitsu.com; Alvaro Herrera ; Justin Pryzby 
; Rahila Syed ; Peter Smith 
; pgsql-hackers ; 
shiy.f...@fujitsu.com
Subject: Re: Column Filtering in Logical Replication

On 3/26/22 01:18, Tomas Vondra wrote:
>
> ...
> 
> I went over the patch again, polished the commit message a bit, and 
> pushed. May the buildfarm be merciful!
> 

There's a couple failures immediately after the push, which caused me a minor 
heart attack. But it seems all of those are strange failures related to 
configure (which the patch did not touch at all), on animals managed by Andres. 
And a couple animals succeeded since then.

So I guess the animals were reconfigured, or something ...


regards

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company




prattrs_column_fix_v1.diff
Description: prattrs_column_fix_v1.diff

RE: ICU for global collation

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi, 
Thank you to all the developers.
I found that the description of the pg_database.daticulocale column was not 
written in the documentation. 
The attached small patch adds a description of the daticulocale column to 
catalogs.sgml.

Regards,
Noriyoshi Shinoda
-Original Message-
From: Peter Eisentraut  
Sent: Thursday, March 17, 2022 7:29 PM
To: Julien Rouhaud 
Cc: pgsql-hackers ; Daniel Verite 

Subject: Re: ICU for global collation

On 15.03.22 08:41, Julien Rouhaud wrote:
 The locale object in ICU is an identifier that specifies a 
 particular locale and has fields for language, country, and an 
 optional code to specify further variants or subdivisions. These 
 fields also can be represented as a string with the fields separated by an 
 underscore.
>>
>> I think the Localization chapter needs to be reorganized a bit, but 
>> I'll leave that for a separate patch.
> 
> WFM.

I ended up writing a bit of content for that chapter.

>>> While on that topic, the doc should probably mention that default 
>>> ICU collations can only be deterministic.
>>
>> Well, there is no option to do otherwise, so I'm not sure where/how 
>> to mention that.  We usually don't document options that don't exist. 
>> ;-)
> 
> Sure, but I'm afraid that users may still be tempted to use ICU 
> locales like
> und-u-ks-level2 from the case_insensitive example in the doc and hope 
> that it will work accordingly.  Or maybe it's just me that still sees 
> ICU as dark magic and want to be extra cautious.

I have added this to the CREATE DATABASE ref page.

> Unrelated, but I just realized that we have PGC_INTERNAL gucs for 
> lc_ctype and lc_collate.  Should we add one for icu_locale too?

I'm not sure.  I think the existing ones are more for backward compatibility 
with the time before it was settable per database.

>>> in AlterCollation(), pg_collation_actual_version(), 
>>> AlterDatabaseRefreshColl() and pg_database_collation_actual_version():
>>>
>>> -   datum = SysCacheGetAttr(COLLOID, tup, Anum_pg_collation_collcollate, 
>>> );
>>> -   Assert(!isnull);
>>> -   newversion = get_collation_actual_version(collForm->collprovider, 
>>> TextDatumGetCString(datum));
>>> +   datum = SysCacheGetAttr(COLLOID, tup, collForm->collprovider == 
>>> COLLPROVIDER_ICU ? Anum_pg_collation_colliculocale : 
>>> Anum_pg_collation_collcollate, );
>>> +   if (!isnull)
>>> +   newversion = get_collation_actual_version(collForm->collprovider, 
>>> TextDatumGetCString(datum));
>>> +   else
>>> +   newversion = NULL;
>>>
>>> The columns are now nullable, but can you actually end up with a 
>>> null locale name in the expected field without manual DML on the 
>>> catalog, corruption or similar?  I think it should be a plain error 
>>> explaining the inconsistency rather then silently assuming there's 
>>> no version.  Note that at least
>>> pg_newlocale_from_collation() asserts that the specific libc/icu 
>>> field it's interested in isn't null.
>>
>> This is required because the default collations's fields are null now.
> 
> Yes I saw that, but that's a specific exception.  Detecting whether 
> it's the DEFAULT_COLLATION_OID or not and raise an error when a null 
> value isn't expected seems like it could be worthwhile.

I have fixed that as you suggest.

> So apart from the few details mentioned above I'm happy with this patch!

committed!




pg_database_iculocale_v1.diff
Description: pg_database_iculocale_v1.diff

RE: row filtering for logical replication

[Shinoda, Noriyoshi (PN Japan FSIP)]

> You can give it for multiple tables. See below as an example:

Thank you very much. I understood.

Regards,
Noriyoshi Shinoda
-Original Message-
From: Amit Kapila  
Sent: Thursday, February 24, 2022 11:25 AM
To: Shinoda, Noriyoshi (PN Japan FSIP) 
Cc: Euler Taveira ; houzj.f...@fujitsu.com; Peter Smith 
; Alvaro Herrera ; Greg 
Nancarrow ; vignesh C ; Ajin Cherian 
; tanghy.f...@fujitsu.com; Dilip Kumar 
; Rahila Syed ; Peter Eisentraut 
; Önder Kalacı ; 
japin ; Michael Paquier ; David 
Steele ; Craig Ringer ; Amit 
Langote ; PostgreSQL Hackers 

Subject: Re: row filtering for logical replication

On Thu, Feb 24, 2022 at 7:43 AM Shinoda, Noriyoshi (PN Japan FSIP) 
 wrote:
>
> Hi,
> Thank you for developing of the great feature.
> If multiple tables are specified when creating a PUBLICATION, is it 
> supposed that the WHERE clause condition is given to only one table?
>

You can give it for multiple tables. See below as an example:

> --- operation log ---
> postgres=> CREATE TABLE data1(c1 INT PRIMARY KEY, c2 VARCHAR(10)); 
> CREATE TABLE postgres=> CREATE TABLE data2(c1 INT PRIMARY KEY, c2 
> VARCHAR(10)); CREATE TABLE postgres=> CREATE PUBLICATION pub1 FOR 
> TABLE data1,data2 WHERE (c1 < 1000); CREATE PUBLICATION

postgres=# CREATE PUBLICATION pub_data_1 FOR TABLE data1 WHERE (c1 > 10), data2 
WHERE (c1 < 1000); CREATE PUBLICATION

--
With Regards,
Amit Kapila.

RE: row filtering for logical replication

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi, 
Thank you for developing of the great feature. 
If multiple tables are specified when creating a PUBLICATION, 
is it supposed that the WHERE clause condition is given to only one table? 
I attached the operation log below.

--- operation log ---
postgres=> CREATE TABLE data1(c1 INT PRIMARY KEY, c2 VARCHAR(10));
CREATE TABLE
postgres=> CREATE TABLE data2(c1 INT PRIMARY KEY, c2 VARCHAR(10));
CREATE TABLE
postgres=> CREATE PUBLICATION pub1 FOR TABLE data1,data2 WHERE (c1 < 1000);
CREATE PUBLICATION
postgres=> \d data1
  Table "public.data1"
 Column | Type  | Collation | Nullable | Default
+---+---+--+-
 c1 | integer   |   | not null |
 c2 | character varying(10) |   |  |
Indexes:
"data1_pkey" PRIMARY KEY, btree (c1)
Publications:
"pub1"

postgres=> \d data2
  Table "public.data2"
 Column | Type  | Collation | Nullable | Default
+---+---+--+-
 c1 | integer   |   | not null |
 c2 | character varying(10) |   |  |
Indexes:
"data2_pkey" PRIMARY KEY, btree (c1)
Publications:
"pub1" WHERE (c1 < 1000)

postgres=> SELECT prrelid, prqual FROM pg_publication_rel;
 prrelid |   prqual
-+-
   16408 |
   16413 | {OPEXPR :opno 97 :opfuncid 66 :opresulttype 16 :opretset false :opcol
lid 0 :inputcollid 0 :args ({VAR :varno 1 :varattno 1 :vartype 23 :vartypmod -1
:varcollid 0 :varlevelsup 0 :varnosyn 1 :varattnosyn 1 :location 53} {CONST :con
sttype 23 :consttypmod -1 :constcollid 0 :constlen 4 :constbyval true :constisnu
ll false :location 58 :constvalue 4 [ -24 3 0 0 0 0 0 0 ]}) :location 56}
(2 rows)

Regards,
Noriyoshi Shinoda

-Original Message-
From: Amit Kapila  
Sent: Wednesday, February 23, 2022 11:06 AM
To: Euler Taveira 
Cc: houzj.f...@fujitsu.com; Peter Smith ; Alvaro Herrera 
; Greg Nancarrow ; vignesh C 
; Ajin Cherian ; 
tanghy.f...@fujitsu.com; Dilip Kumar ; Rahila Syed 
; Peter Eisentraut ; 
Önder Kalacı ; japin ; Michael 
Paquier ; David Steele ; Craig Ringer 
; Amit Langote ; PostgreSQL 
Hackers 
Subject: Re: row filtering for logical replication

On Tue, Feb 22, 2022 at 4:47 AM Euler Taveira  wrote:
>
> On Thu, Feb 17, 2022, at 3:36 AM, Amit Kapila wrote:
>
> As there is a new version, I would like to wait for a few more days 
> before committing. I am planning to commit this early next week (by
> Tuesday) unless others or I see any more things that can be improved.
>
> Amit, I don't have additional comments or suggestions. Let's move on. 
> Next topic. :-)
>

Pushed!

--
With Regards,
Amit Kapila.

RE: refactoring basebackup.c

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi, Hackers.
Thank you for developing a great feature.
The current help message shown below does not seem to be able to specify the 
'client-' or 'server-' for lz4 compression.
 --compress = {[{client, server}-]gzip, lz4, none}[:LEVEL]

The attached small patch fixes the help message as follows:
 --compress = {[{client, server}-]{gzip, lz4}, none}[:LEVEL]

Regards,
Noriyoshi Shinoda
-Original Message-
From: Robert Haas  
Sent: Saturday, February 12, 2022 12:50 AM
To: Justin Pryzby 
Cc: Jeevan Ladhe ; Dipesh Pandit 
; Abhijit Menon-Sen ; Dmitry Dolgov 
<9erthali...@gmail.com>; Jeevan Ladhe ; Mark 
Dilger ; pgsql-hack...@postgresql.org; tushar 

Subject: Re: refactoring basebackup.c

On Fri, Feb 11, 2022 at 10:29 AM Justin Pryzby  wrote:
> FYI: there's a couple typos in the last 2 patches.

Hmm. OK. But I don't consider "can be optionally specified" incorrect or worse 
than "can optionally be specified".

I do agree that spelling words correctly is a good idea.

--
Robert Haas
EDB: http://www.enterprisedb.com 




pg_basebackup_help_v1.diff
Description: pg_basebackup_help_v1.diff

RE: refactoring basebackup.c

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi, 
Thank you for committing a great feature. I have tested the committed features. 
The attached small patch fixes the output of the --help message. In the 
previous commit, only gzip and none were output, but in the attached patch, 
client-gzip and server-gzip are added.

Regards,
Noriyoshi Shinoda
-Original Message-
From: Robert Haas  
Sent: Saturday, January 22, 2022 3:33 AM
To: Dipesh Pandit ; Michael Paquier 

Cc: Jeevan Ladhe ; tushar 
; Dmitry Dolgov <9erthali...@gmail.com>; Mark 
Dilger ; pgsql-hack...@postgresql.org
Subject: Re: refactoring basebackup.c

On Wed, Jan 19, 2022 at 4:26 PM Robert Haas  wrote:
> I spent some time thinking about test coverage for the server-side 
> backup code today and came up with the attached (v12-0003).

I committed the base backup target patch yesterday, and today I updated the 
remaining code in light of Michael Paquier's commit 
5c649fe153367cdab278738ee4aebbfd158e0546. Here is the resulting patch.

Michael, I am proposing to that we remove this message as part of this commit:

-pg_log_info("no value specified for compression
level, switching to default");

I think most people won't want to specify a compression level, so emitting a 
message when they don't seems too verbose.

--
Robert Haas
EDB: http://www.enterprisedb.com 


pg_basebackup_option_v1.diff
Description: pg_basebackup_option_v1.diff

RE: Improve logging when using Huge Pages

[Shinoda, Noriyoshi (PN Japan FSIP)]

Sawada-san, Fujii-san,

Thank you for your reviews.

In a database with huge_pages = on / off explicitly set, if memory allocation 
fails, the instance cannot be started, so I think that additional logs are 
unnecessary.
The attached patch outputs the log only when huge_pages = try, and outputs the 
requested size if the acquisition of Huge Pages fails.

I have a completely different approach, setting GUC 
shared_memory_size_in_huge_pages to zero if the Huge Pages acquisition fails. 
This GUC is currently calculated independently of getting Huge Pages. The 
attached patch does not include this specification.

Regards,
Noriyoshi Shinoda

-Original Message-
From: Masahiko Sawada [mailto:sawada.m...@gmail.com] 
Sent: Thursday, November 11, 2021 2:45 PM
To: Fujii Masao 
Cc: Justin Pryzby ; Shinoda, Noriyoshi (PN Japan FSIP) 
; PostgreSQL-development 
; Julien Rouhaud ; Tom Lane 
; Kyotaro Horiguchi 
Subject: Re: Improve logging when using Huge Pages

On Tue, Nov 2, 2021 at 1:24 AM Fujii Masao  wrote:
>
>
>
> On 2021/10/29 7:05, Justin Pryzby wrote:
> > Hi,
> >
> > On Wed, Oct 27, 2021 at 06:39:46AM +, Shinoda, Noriyoshi (PN Japan 
> > FSIP) wrote:
> >> Thank you for your comment.
> >> The attached patch stops message splitting.
> >> This patch also limits the timing of message output when huge_pages = try 
> >> and HugePages is not used.
>
> The log message should be reported even when huge_pages=off (and huge 
> pages are not used)? Otherwise we cannot determine whether huge pages 
> are actually used or not when no such log message is found in the server log.
>
> Or it's simpler and more intuitive to log the message "Anonymous 
> shared memory was allocated with huge pages" only when huge pages are 
> successfully requested? If that message is logged, we can determine 
> that huge pages are used whatever the setting is. OTOH, if there is no 
> such message, we can determine that huge pages are not used for some 
> reasons, e.g., OS doesn't support huge pages, shared_memory_type is not set 
> to mmap, etc.

If users want to know whether the shared memory is allocated with huge pages, I 
think it’s more intuitive to emit the log only on success when huge_pages = 
on/try.  On the other hand, I guess that users might want to use the message to 
adjust vm.nr_hugepages when it is not allocated with huge pages. In this case, 
it’d be better to log the message on failure with the request memory size (or 
whatever reason for the failure). That is, we end up logging such a message on 
failure when huge_pages = on/try.

Regards,

--
Masahiko Sawada
EDB:  https://www.enterprisedb.com/ 


huge_pages_log_v10.diff
Description: huge_pages_log_v10.diff

RE: Improve logging when using Huge Pages

[Shinoda, Noriyoshi (PN Japan FSIP)]

Fujii-san, 

Thank you for your comment.
As advised by Justin, I modified the comment according to the style guide and 
split the if statement.
As you say, the NOTICE log was deleted as it may not be needed.

Regards,
Noriyoshi Shinoda
-Original Message-
From: Fujii Masao [mailto:masao.fu...@oss.nttdata.com] 
Sent: Tuesday, November 2, 2021 11:35 PM
To: Shinoda, Noriyoshi (PN Japan FSIP) ; 
pgsql-hack...@postgresql.org; Masahiko Sawada 
Cc: rjuju...@gmail.com; t...@sss.pgh.pa.us; Kyotaro Horiguchi 
; Justin Pryzby 
Subject: Re: Improve logging when using Huge Pages



On 2021/11/02 18:31, Shinoda, Noriyoshi (PN Japan FSIP) wrote:
> Fujii-san, Sawada-san,
> 
> Thank you for your comment.
> 
>> Also, with the patch, the log message is emitted also during initdb and 
>> starting up in single user mode:
> 
> Certainly the log output when executing the initdb command was a noise.
> The attached patch reflects the comments and uses IsPostmasterEnvironment to 
> suppress the output message.

Thanks for updating the patch!

+   ereport(IsPostmasterEnvironment ? LOG : NOTICE, 
(errmsg("Anonymous 
+shared memory was allocated without huge pages.")));

This change causes the log message to be output with NOTICE level even when 
IsPostmasterEnvironment is false. But do we really want to log that NOTICE 
message in that case? Instead, isn't it better to just output the log message 
with LOG level only when IsPostmasterEnvironment is true?


Justin and I posted other comments upthread. Could you consider whether it's 
worth applying those comments to the patch?


Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION


huge_pages_log_v9.diff
Description: huge_pages_log_v9.diff

RE: Improve logging when using Huge Pages

[Shinoda, Noriyoshi (PN Japan FSIP)]

Fujii-san, Sawada-san,

Thank you for your comment.

> Also, with the patch, the log message is emitted also during initdb and 
> starting up in single user mode:

Certainly the log output when executing the initdb command was a noise.
The attached patch reflects the comments and uses IsPostmasterEnvironment to 
suppress the output message.

Regards,
Noriyoshi Shinoda

-Original Message-
From: Fujii Masao [mailto:masao.fu...@oss.nttdata.com] 
Sent: Tuesday, November 2, 2021 1:25 AM
To: Masahiko Sawada ; Shinoda, Noriyoshi (PN Japan FSIP) 

Cc: pgsql-hack...@postgresql.org; rjuju...@gmail.com; t...@sss.pgh.pa.us; 
Kyotaro Horiguchi ; Justin Pryzby 

Subject: Re: Improve logging when using Huge Pages



On 2021/10/29 16:00, Masahiko Sawada wrote:
> Which is noisy. Perhaps it's better to log it only when 
> IsPostmasterEnvironment is true.

+1

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION


huge_pages_log_v8.diff
Description: huge_pages_log_v8.diff

RE: Improve logging when using Huge Pages

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi,
Thank you for your comment.
The attached patch stops message splitting.
This patch also limits the timing of message output when huge_pages = try and 
HugePages is not used.

Regards,
Noriyoshi Shinoda

-Original Message-
From: Justin Pryzby [mailto:pry...@telsasoft.com] 
Sent: Friday, October 22, 2021 11:38 AM
To: Shinoda, Noriyoshi (PN Japan FSIP) 
Cc: masao.fu...@oss.nttdata.com; pgsql-hack...@postgresql.org; 
rjuju...@gmail.com; t...@sss.pgh.pa.us; Kyotaro Horiguchi 

Subject: Re: Improve logging when using Huge Pages

+   ereport(LOG, (errmsg("Anonymous shared memory was 
+ allocated %s huge pages.", with_hugepages ? "with" : "without")));

You shouldn't break a sentence into pieces like this, since it breaks 
translation.  You don't want an untranslated "without" to appear in the middle 
of the translated message.

There are cases where a component *shouldn't* be translated, like this one:
where "numeric" should not be translated.

src/backend/utils/adt/numeric.c: 
errmsg("invalid input syntax for type %s: \"%s\"",
src/backend/utils/adt/numeric.c-
"numeric", str)));

--
Justin


huge_pages_log_v7.diff
Description: huge_pages_log_v7.diff

RE: Improve logging when using Huge Pages

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi, all.
Thank you for your comment.

> Probably I understood your point. But isn't it more confusing to users?
As you say, I think the last patch was rather confusing. For this reason, I 
simply reconsidered it.
The attached patch just outputs a log like your advice on acquiring Huge Page.
It is possible to limit the log output trigger only when huge_pages=try, but is 
it better not to always output it?

Regards,
Noriyoshi Shinoda

-Original Message-
From: Kyotaro Horiguchi [mailto:horikyota@gmail.com] 
Sent: Monday, September 27, 2021 11:40 AM
To: pry...@telsasoft.com
Cc: masao.fu...@oss.nttdata.com; Shinoda, Noriyoshi (PN Japan FSIP) 
; pgsql-hack...@postgresql.org; rjuju...@gmail.com; 
t...@sss.pgh.pa.us
Subject: Re: Improve logging when using Huge Pages

At Tue, 21 Sep 2021 19:23:22 -0500, Justin Pryzby  wrote 
in 
> On Wed, Sep 22, 2021 at 02:03:11AM +0900, Fujii Masao wrote:
> > Another idea is to output "Anonymous shared memory was allocated 
> > with  huge pages" when it's successfully allocated with huge pages, 
> > and to output  "Anonymous shared memory was allocated without huge pages"
> >  when it's successfully allocated without huge pages. I'm not sure 
> > if users  may think even this message is noisy, though.
> 
> +1

+1. Positive phrase looks better.

> Maybe it could show the page size instead of "with"/without:
> "Shared memory allocated with 4k/1MB/1GB pages."

+1.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center


huge_pages_log_v6.diff
Description: huge_pages_log_v6.diff

RE: Improve logging when using Huge Pages

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi,

Thank you for your comment.

> I was afraid that logging the message like "could not ..." every time when 
> the server starts up would surprise users unnecessarily.
> Because the message sounds like it reports a server error.

Fujii-san, 
I was worried about the same thing as you.
So the attached patch gets the value of the kernel parameter vm.nr_hugepages, 
and if it is the default value of zero, the log level is the same as before. 
On the other hand, if any value is set, the level is set to LOG.
I hope I can find a better message other than this kind of implementation.

Regards,
Noriyoshi Shinoda

-Original Message-
From: Kyotaro Horiguchi [mailto:horikyota@gmail.com] 
Sent: Friday, September 17, 2021 1:15 PM
To: masao.fu...@oss.nttdata.com
Cc: pry...@telsasoft.com; Shinoda, Noriyoshi (PN Japan FSIP) 
; pgsql-hack...@postgresql.org; rjuju...@gmail.com; 
t...@sss.pgh.pa.us
Subject: Re: Improve logging when using Huge Pages

At Fri, 17 Sep 2021 00:13:41 +0900, Fujii Masao  
wrote in 
> 
> 
> On 2021/09/08 11:17, Kyotaro Horiguchi wrote:
> > I don't dislike the message, but I'm not sure I like the message is 
> > too verbose, especially about it has DETAILS. It seems to me 
> > something like the following is sufficient, or I'd like see it even 
> > more concise.
> > "fall back anonymous shared memory to non-huge pages: required %zu 
> > bytes for huge pages"
> > If we emit an error message for other than mmap failure, it would be 
> > like the following.
> > "fall back anonymous shared memory to non-huge pages: huge pages not 
> > available"
> 
> How about simpler message like "disabling huge pages" or "disabling 
> huge pages due to lack of huge pages available"?

Honestly, I cannot have conficence on my wording, but "disabling huge pages" 
souds like somthing that happens on the OS layer.  "did not use/gave up using 
huge pages for anounymous shared memory" might work well, I'm not sure, 
though...

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center


huge_pages_log_v5.diff
Description: huge_pages_log_v5.diff

RE: Improve logging when using Huge Pages

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hello,

Thank you everyone for comments.
I have attached a patch that simply changed the message like the advice from 
Horiguchi-san.

> Even with the patch, there are still some cases where huge pages is disabled 
> silently. We should report something even in these cases?
> For example, in the platform where huge pages is not supported, it's silently 
> disabled when huge_pages=try.

The area where this patch is written is inside the "#ifdef MAP_HUGETLB #endif" 
block.
For this reason, I think it is excluded from binaries created in an environment 
that does not have the MAP_HUGETLB macro.

> One big concern about the patch is that log message is always reported when 
> shared memory fails to be allocated with huge pages enabled when 
> huge_pages=try. Since 
> huge_pages=try is the default setting, many users would see this new log 
> message whenever they start the server. Those who don't need huge pages but 
> just use the default 
> setting might think that such log messages would be noisy.

This patch is meant to let the admin know that HugePages isn't being used, so 
I'm sure you're right. I have no idea what to do so far.

Regards,
Noriyoshi Shinoda

-Original Message-
From: Kyotaro Horiguchi [mailto:horikyota@gmail.com] 
Sent: Wednesday, September 8, 2021 11:18 AM
To: pry...@telsasoft.com
Cc: masao.fu...@oss.nttdata.com; Shinoda, Noriyoshi (PN Japan FSIP) 
; pgsql-hack...@postgresql.org; rjuju...@gmail.com; 
t...@sss.pgh.pa.us
Subject: Re: Improve logging when using Huge Pages

At Tue, 7 Sep 2021 08:16:53 -0500, Justin Pryzby  wrote 
in 
> On Tue, Sep 07, 2021 at 07:12:36PM +0900, Fujii Masao wrote:
> > One big concern about the patch is that log message is always 
> > reported when shared memory fails to be allocated with huge pages 
> > enabled when huge_pages=try. Since huge_pages=try is the default 
> > setting, many users would see this new log message whenever they 
> > start the server. Those who don't need huge pages but just use the 
> > default setting might think that such log messages would be noisy.
> 
> I don't see this as any issue.  We're only talking about a single 
> message on each restart, which would be added in a major release.  If 
> it's a problem, the message could be a NOTICE or INFO, and it won't be shown 
> by default.
> 
> I think it should say "with/out huge pages" without 
> "enabled/disabled", without "again", and without "The server", like:
> 
> +   (errmsg("could not map anonymous 
> shared memory (%zu bytes)"
> +   " with huge pages.", 
> allocsize),
> +errdetail("Anonymous shared memory 
> will be mapped "
> +   "without huge 
> + pages.")));

I don't dislike the message, but I'm not sure I like the message is too 
verbose, especially about it has DETAILS. It seems to me something like the 
following is sufficient, or I'd like see it even more concise.

"fall back anonymous shared memory to non-huge pages: required %zu bytes for 
huge pages"

If we emit an error message for other than mmap failure, it would be like the 
following.

"fall back anonymous shared memory to non-huge pages: huge pages not available"

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center


huge_pages_log_v4.diff
Description: huge_pages_log_v4.diff

RE: Improve logging when using Huge Pages

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hello,

Thank you everyone for comments.
In the thread [1] that Horiguchi told me about, there is already a review going 
on about GUC for HugePages memory.
For this reason, I have removed the new GUC implementation and attached a patch 
that changes only the message at instance startup.

[1]
https://www.postgresql.org/message-id/20210903.141206.103927759882272221.hor

Regards,
Noriyoshi Shinoda

-Original Message-
From: Fujii Masao [mailto:masao.fu...@oss.nttdata.com] 
Sent: Saturday, September 4, 2021 1:36 AM
To: Tom Lane 
Cc: Kyotaro Horiguchi ; Shinoda, Noriyoshi (PN Japan 
FSIP) ; rjuju...@gmail.com; 
pgsql-hack...@postgresql.org
Subject: Re: Improve logging when using Huge Pages



On 2021/09/03 23:27, Tom Lane wrote:
> Fujii Masao  writes:
>> IMO, if the level is promoted to LOG, the message should be updated 
>> so that it follows the error message style guide. But I agree that 
>> simpler message would be better in this case. So what about something 
>> like the following?
> 
>> LOG: could not map anonymous shared memory (%zu bytes) with huge 
>> pages enabled
>> HINT: The server will map anonymous shared memory again with huge pages 
>> disabled.
> 
> That is not a hint.  Maybe it qualifies as errdetail, though.

Yes, agreed.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION


huge_pages_log_v3.diff
Description: huge_pages_log_v3.diff

RE: New predefined roles- 'pg_read/write_all_data'

[Shinoda, Noriyoshi (PN Japan FSIP)]

Thank you for your quick response.
I understood the specifications from your explanation.

Regards,
Noriyoshi Shinoda

From: Stephen Frost [mailto:sfr...@snowman.net]
Sent: Sunday, September 5, 2021 8:50 PM
To: Shinoda, Noriyoshi (PN Japan FSIP) 
Cc: Anastasia Lubennikova ; Michael Banck 
; gkokola...@pm.me; 
pgsql-hackers@lists.postgresql.org
Subject: Re: New predefined roles- 'pg_read/write_all_data'

Greetings,

On Sun, Sep 5, 2021 at 07:43 Shinoda, Noriyoshi (PN Japan FSIP) 
mailto:noriyoshi.shin...@hpe.com>> wrote:
I have tested this new feature with PostgreSQL 14 Beta 3 environment.
I created a user granted with pg_write_all_data role and executed UPDATE and 
DELETE statements on tables owned by other users.
If there is no WHERE clause, it can be executed as expected, but if the WHERE 
clause is specified, an error of permission denied will occur.
Is this the expected behavior?

A WHERE clause requires SELECT rights on the table/columns referenced and if no 
SELECT rights were granted then a permission denied error is the correct 
result, yes. Note that pg_write_all_data, as documented, does not include 
SELECT rights.

Thanks,

Stephen

RE: New predefined roles- 'pg_read/write_all_data'

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi hackers,

I have tested this new feature with PostgreSQL 14 Beta 3 environment.
I created a user granted with pg_write_all_data role and executed UPDATE and 
DELETE statements on tables owned by other users.
If there is no WHERE clause, it can be executed as expected, but if the WHERE 
clause is specified, an error of permission denied will occur.
Is this the expected behavior?
The WHERE clause is not specified in the regression test (privileges.sql).

Below is the execution log.

postgres=# CREATE USER owner1 PASSWORD 'owner1';
CREATE ROLE
postgres=# CREATE USER write1 PASSWORD 'write1';
CREATE ROLE
postgres=# GRANT pg_write_all_data TO write1;
GRANT ROLE
postgres=# SET SESSION AUTHORIZATION owner1;
SET
postgres=> CREATE TABLE data1(c1 INT, c2 VARCHAR(10));
CREATE TABLE
postgres=> INSERT INTO data1 VALUES (generate_series(1, 10), 'data1');
INSERT 0 10
postgres=> SET SESSION AUTHORIZATION write1;
SET
postgres=> INSERT INTO data1 VALUES (0, 'data1');   -- success
INSERT 0 1
postgres=> UPDATE data1 SET c2='update' WHERE c1=0; -- fail
ERROR:  permission denied for table data1
postgres=> DELETE FROM data1 WHERE c1=0;-- fail
ERROR:  permission denied for table data1
postgres=> UPDATE data1 SET c2='update';-- success
UPDATE 11
postgres=> DELETE FROM data1;   -- success
DELETE 11
postgres=> SELECT version();
  version

 PostgreSQL 14beta3 on x86_64-pc-linux-gnu, compiled by gcc (GCC) 4.8.5 
20150623 (Red Hat 4.8.5-39), 64-bit
(1 row)
-

Regards,
Noriyoshi Shinoda

-Original Message-
From: Stephen Frost [mailto:sfr...@snowman.net] 
Sent: Saturday, August 28, 2021 7:34 AM
To: Michael Banck 
Cc: gkokola...@pm.me; Anastasia Lubennikova ; 
pgsql-hackers@lists.postgresql.org
Subject: Re: New predefined roles- 'pg_read/write_all_data'

Greetings,

* Michael Banck (michael.ba...@credativ.de) wrote:
> On Thu, Apr 01, 2021 at 04:00:06PM -0400, Stephen Frost wrote:
> > diff --git a/doc/src/sgml/user-manag.sgml 
> > b/doc/src/sgml/user-manag.sgml index d171b13236..fe0bdb7599 100644
> > --- a/doc/src/sgml/user-manag.sgml
> > +++ b/doc/src/sgml/user-manag.sgml
> > @@ -518,6 +518,24 @@ DROP ROLE doomed_role;
> >
> >   
> >   
> > +  
> > +   pg_read_all_data
> > +   Read all data (tables, views, sequences), as if having SELECT
> > +   rights on those objects, and USAGE rights on all schemas, even 
> > without
> > +   having it explicitly.  This role does not have the role attribute
> > +   BYPASSRLS set.  If RLS is being used, an 
> > administrator
> > +   may wish to set BYPASSRLS on roles which this 
> > role is
> > +   GRANTed to.
> > +  
> > +  
> > +   pg_write_all_data
> > +   Write all data (tables, views, sequences), as if having 
> > INSERT,
> > +   UPDATE, and DELETE rights on those objects, and USAGE rights on all
> > +   schemas, even without having it explicitly.  This role does not 
> > have the
> > +   role attribute BYPASSRLS set.  If RLS is being 
> > used,
> > +   an administrator may wish to set BYPASSRLS on 
> > roles
> > +   which this role is GRANTed to.
> > +  
> 
> Shouldn't those "SELECT", "INSERT" etc. be wrapped in  tags?

Yeah, good point, fixed.

Thanks!

Stephen

RE: Improve logging when using Huge Pages

[Shinoda, Noriyoshi (PN Japan FSIP)]

Fujii-san, Julien-san

Thank you very much for your comment.
I followed your comment and changed the elog function to ereport function and 
also changed the log level. The output message is the same as in the case of 
non-HugePages memory acquisition failure.I did not simplify the error messages 
as it would have complicated the response to the preprocessor.

> I agree that the message should be promoted to a higher level.  But I 
> think we should also make that information available at the SQL level, 
> as the log files may be truncated / rotated before you need the info, 
> and it can be troublesome to find the information at the OS level, if 
> you're lucky enough to have OS access.

In the attached patch, I have added an Internal GUC 'using_huge_pages' to know 
that it is using HugePages. This parameter will be True only if the instance is 
using HugePages.

Regards,
Noriyoshi Shinoda

-Original Message-
From: Fujii Masao [mailto:masao.fu...@oss.nttdata.com] 
Sent: Wednesday, September 1, 2021 2:06 AM
To: Julien Rouhaud ; Shinoda, Noriyoshi (PN Japan FSIP) 

Cc: pgsql-hack...@postgresql.org
Subject: Re: Improve logging when using Huge Pages



On 2021/08/31 15:57, Julien Rouhaud wrote:
> On Tue, Aug 31, 2021 at 1:37 PM Shinoda, Noriyoshi (PN Japan FSIP) 
>  wrote:
>>
>> In the current version, when GUC huge_pages=try, which is the default 
>> setting, no log is output regardless of the success or failure of the 
>> HugePages acquisition. If you want to output logs, you need to set 
>> log_min_messages=DEBUG3, but it will output a huge amount of extra logs.
>> With huge_pages=try setting, if the kernel parameter vm.nr_hugepages is not 
>> enough, the administrator will not notice that HugePages is not being used.
>> I think it should output a log if HugePages was not available.

+1

-   elog(DEBUG1, "mmap(%zu) with MAP_HUGETLB failed, huge 
pages disabled: %m",
+   elog(WARNING, "mmap(%zu) with MAP_HUGETLB failed, huge 
pages 
+disabled: %m",

elog() should be used only for internal errors and low-level debug logging.
So per your proposal, elog() is not suitable here. Instead, ereport() should be 
used.

The log level should be LOG rather than WARNING because this message indicates 
the information about server activity that administrators are interested in.

The message should be updated so that it follows the Error Message Style Guide.
https://www.postgresql.org/docs/devel/error-style-guide.html 

With huge_pages=on, if shared memory fails to be allocated, the error message 
is reported currently. Even with huge_page=try, this error message should be 
used to simplify the code as follows?

 errno = mmap_errno;
-   ereport(FATAL,
+   ereport((huge_pages == HUGE_PAGES_ON) ? FATAL : LOG,
 (errmsg("could not map anonymous shared 
memory: %m"),
  (mmap_errno == ENOMEM) ?
  errhint("This error usually means that 
PostgreSQL's request "



> I agree that the message should be promoted to a higher level.  But I 
> think we should also make that information available at the SQL level, 
> as the log files may be truncated / rotated before you need the info, 
> and it can be troublesome to find the information at the OS level, if 
> you're lucky enough to have OS access.

+1

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION


huge_pages_log_v2.diff
Description: huge_pages_log_v2.diff

Improve logging when using Huge Pages

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi Hackers,

In the current version, when GUC huge_pages=try, which is the default setting, 
no log is output regardless of the success or failure of the HugePages 
acquisition. If you want to output logs, you need to set 
log_min_messages=DEBUG3, but it will output a huge amount of extra logs.
With huge_pages=try setting, if the kernel parameter vm.nr_hugepages is not 
enough, the administrator will not notice that HugePages is not being used.
I think it should output a log if HugePages was not available.

By the way, in MySQL with almost the same architecture, the following log is 
output at the Warning level.

[Warning] [MY-012677] [InnoDB] Failed to allocate 138412032 bytes. errno 1
[Warning] [MY-012679] [InnoDB] Using conventional memory pool

The attached small patch outputs a log at the WARNING level when huge_pages = 
try and if the acquisition of HugePages fails.

Regards, 
Noriyoshi Shinoda



huge_pages_log_v1.diff
Description: huge_pages_log_v1.diff

RE: WIP: WAL prefetch (another approach)

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi, 

Thank you for developing a great feature. I tested this feature and checked the 
documentation.
Currently, the documentation for the pg_stat_prefetch_recovery view is included 
in the description for the pg_stat_subscription view.

https://www.postgresql.org/docs/devel/monitoring-stats.html#MONITORING-PG-STAT-SUBSCRIPTION

It is also not displayed in the list of "28.2. The Statistics Collector".
https://www.postgresql.org/docs/devel/monitoring.html

The attached patch modifies the pg_stat_prefetch_recovery view to appear as a 
separate view.

Regards,
Noriyoshi Shinoda

-Original Message-
From: Thomas Munro [mailto:thomas.mu...@gmail.com] 
Sent: Saturday, April 10, 2021 5:46 AM
To: Justin Pryzby 
Cc: Tomas Vondra ; Stephen Frost 
; Andres Freund ; Jakub Wartak 
; Alvaro Herrera ; Tomas 
Vondra ; Dmitry Dolgov <9erthali...@gmail.com>; 
David Steele ; pgsql-hackers 
Subject: Re: WIP: WAL prefetch (another approach)

On Sat, Apr 10, 2021 at 8:37 AM Justin Pryzby  wrote:
> Did you see this?
> INVALID URI REMOVED
> 278MB0483490FEAC879DCA5ED583DD2739*40GV0P278MB0483.CHEP278.PROD.OUTLOO
> K.COM__;JQ!!NpxR!wcPrhiB2CaHRtywGoh9Ap0M-kH1m07hGI37-ycYRGCPgCqGs30lRS
> KicsXacduEXHxI$
>
> I meant to mail you so you could include it in the same commit, but 
> forgot until now.

Done, thanks.




pg_stat_prefetch_recovery_doc_v1.diff
Description: pg_stat_prefetch_recovery_doc_v1.diff

RE: list of extended statistics on psql

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi, hackers.

I tested this committed feature. 
It doesn't seem to be available to non-superusers due to the inability to 
access pg_statistics_ext_data. 
Is this the expected behavior?

--- operation ---
postgres=> CREATE STATISTICS stat1_data1 ON c1, c2 FROM data1;
CREATE STATISTICS
postgres=> ANALYZE data1;
ANALYZE
postgres=> SELECT * FROM pg_statistic_ext;
  oid  | stxrelid |   stxname   | stxnamespace | stxowner | stxstattarget | 
stxkeys | stxkind
---+--+-+--+--+---+-+-
 16393 |16385 | stat1_data1 | 2200 |16384 |-1 | 1 2 
| {d,f,m}
(1 row)

postgres=> \dX
ERROR:  permission denied for table pg_statistic_ext_data
postgres=>
postgres=> \connect postgres postgres
You are now connected to database "postgres" as user "postgres".
postgres=#
postgres=# \dX
   List of extended statistics
 Schema |Name |Definition | Ndistinct | Dependencies |MCV
+-+---+---+--+---
 public | stat1_data1 | c1, c2 FROM data1 | built | built| requested
(1 row)

--- operation ---

Regards,
Noriyoshi Shinoda

-Original Message-
From: Tomas Vondra [mailto:tomas.von...@enterprisedb.com] 
Sent: Sunday, January 17, 2021 8:32 AM
To: Julien Rouhaud ; Tatsuro Yamada 

Cc: Alvaro Herrera ; Tomas Vondra 
; Michael Paquier ; Pavel 
Stehule ; PostgreSQL Hackers 

Subject: Re: list of extended statistics on psql



On 1/15/21 5:19 PM, Tomas Vondra wrote:
> 
> 
> On 1/15/21 9:47 AM, Julien Rouhaud wrote:
>> On Wed, Jan 13, 2021 at 10:22:05AM +0900, Tatsuro Yamada wrote:
>>> Hi Tomas,
>>>
>>> On 2021/01/13 7:48, Tatsuro Yamada wrote:
 On 2021/01/12 20:08, Tomas Vondra wrote:
> On 1/12/21 2:57 AM, Tatsuro Yamada wrote:
>> On 2021/01/09 9:01, Tomas Vondra wrote:
> ...>
>>> While working on that, I realized that 'defined' might be a bit 
>>> ambiguous, I initially thought it means 'NOT NULL' (which it does not).
>>> I propose to change it to 'requested' instead. Tatsuro, do you 
>>> agree, or do you think 'defined' is better?
>>
>> Regarding the status of extended stats, I think the followings:
>>
>>    - "defined": it shows the extended stats defined only. We 
>> can't know
>>     whether it needs to analyze or not. I agree this 
>> name was
>>      ambiguous. Therefore we should replace it with a 
>> more suitable
>>     name.
>>    - "requested": it shows the extended stats needs something. Of 
>> course,
>>     we know it needs to ANALYZE because we can create the 
>> patch.
>>     However, I feel there is a little ambiguity for DBA.
>>     To solve this, it would be better to write an 
>> explanation of
>>     the status in the document. For example,
>>
>> ==
>> The column of the kind of extended stats (e. g. Ndistinct) shows some 
>> statuses.
>> "requested" means that it needs to gather data by ANALYZE. 
>> "built" means ANALYZE
>>    was finished, and the planner can use it. NULL means that it doesn't 
>> exists.
>> ==
>>
>> What do you think? :-D
>>
>
> Yes, that seems reasonable to me. Will you provide an updated patch?


 Sounds good. I'll send the updated patch today.
>>>
>>>
>>>
>>> I updated the patch to add the explanation of the extended stats' statuses.
>>> Please feel free to modify the patch to improve it more clearly.
>>>
>>> The attached files are:
>>> 0001: Add psql \dx and the fixed document
>>> 0002: Regression test for psql \dX
>>> app-psql.html: Created by "make html" command (You can check the
>>>explanation of the statuses easily, probably)
>>
>> Hello Yamada-san,
>>
>> I reviewed the patch and don't have specific complaints, it all looks good!
>>
>> I'm however thinking about the "requested" status.  I'm wondering if 
>> it could lead to people think that an ANALYZE is scheduled and will happen 
>> soon.
>> Maybe "defined" or "declared" might be less misleading, or even 
>> "waiting for analyze"?
>>
> 
> Well, the "defined" option is not great either, because it can be 
> interpreted as "NOT NULL" - that's why I proposed "requested". Not 
> sure about "declared" - I wouldn't use it in this context, but I'm not 
> a native speaker so maybe it's OK.
> 

I've pushed this, keeping the "requested". If we decide that some other term is 
a better choice, we can tweak that later of course.

Thanks Tatsuro-san for the patience!


regards

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

RE: pgsql: Add key management system

[Shinoda, Noriyoshi (PN Japan FSIP)]

Hi,

Thank you for developing a great feature. I tested it immediately.
The attached patch changes the value of the --file-encryption-keylen option of 
the initdb command to mandatory. No value is currently required.
I also changed the help message and the manual.

Regards,
Noriyoshi Shinoda

-Original Message-
From: Bruce Momjian [mailto:br...@momjian.us] 
Sent: Saturday, December 26, 2020 4:01 AM
To: Erik Rijkers 
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: pgsql: Add key management system

On Fri, Dec 25, 2020 at 07:30:01PM +0100, Erik Rijkers wrote:
> On 2020-12-25 16:19, Bruce Momjian wrote:
> 
> > Add key management system
> > doc/src/sgml/database-encryption.sgml |  97 +
> 
> Attached are a few typos.
> 
> I also noticed that this option does not occur in the initdb --help:
> 
>   -u  --copy-encryption-keys
> 
> Was that deliberate?

No.  :-(  Attached patch applied.  Thanks.

-- 
  Bruce Momjian  https://momjian.us 
  EnterpriseDB https://enterprisedb.com 

  The usefulness of a cup is in its emptiness, Bruce Lee



keylength.diff
Description: keylength.diff