Re: pg_waldump and PREPARE

2019-11-19 Thread btkimurayuzk
I did not see any problems in this version of the patch. The 
information

displayed by pg_waldump for the PREPARE record is sufficient for use.


Thanks Andrey and Michael for the review! I committed the patch.

Regards,



Hi,
There is a mistake in the comment in the definition of 
xl_xact_relfilenodes.

This is a small patch to correct it.

Regards,

Yu Kimuradiff --git a/src/include/access/xact.h b/src/include/access/xact.h
index 42b76cb4dd..9d2899dea1 100644
--- a/src/include/access/xact.h
+++ b/src/include/access/xact.h
@@ -239,7 +239,7 @@ typedef struct xl_xact_subxacts
 
 typedef struct xl_xact_relfilenodes
 {
-	int			nrels;			/* number of subtransaction XIDs */
+	int			nrels;			/* number of relations */
 	RelFileNode xnodes[FLEXIBLE_ARRAY_MEMBER];
 } xl_xact_relfilenodes;
 #define MinSizeOfXactRelfilenodes offsetof(xl_xact_relfilenodes, xnodes)


Re: Allow CREATE OR REPLACE VIEW to rename the columns

2019-11-19 Thread btkimurayuzk

Barring any objection, I'm thinking to commit this patch.

Regards,


Build and All Test has passed .
Looks good to me .

Regards,




Re: Add SQL function to show total block numbers in the relation

2019-11-12 Thread btkimurayuzk

Size in block number is useless for those who doesn't understand the
notion of block, or block size. Those who understands the notion
should come up with the simple formula (except the annoying
casts). Anyone can find the clue to the base values by searching the
document in the Web with the keywords "block size" and "relation size"
or even with "table size". (FWIW, I would even do the same for the new
function if any...) If they need it so frequently, a user-defined
function is easily made up.

regards.



I didn't know about the existence of the user-defined function .
I fully understood , Thanks .

Regards,

Yu Kimura





Re: Resume vacuum and autovacuum from interruption and cancellation

2019-11-07 Thread btkimurayuzk



+   VACOPT_RESUME = 1 << 8/* resume from the previous point */

I think this unused ENUM value is not needed.

Regards,

Yu Kimura





Re: Add SQL function to show total block numbers in the relation

2019-11-07 Thread btkimurayuzk

btkimurayuzk  writes:

I propose new simple sql query, which shows total block numbers in the
relation.
...
Of cource, we can know this value such as
select (pg_relation_size('t') /
current_setting('block_size')::bigint)::int;


I don't really see why the existing solution isn't sufficient.


I think it's a little difficult to introduce the block size using two 
values `current block size` and `reference size`
for beginners who are not familiar with the internal structure of 
Postgres,


This is the reason why the existing solution was insufficient.

What do you think?

Regards,
Yu Kimura




Re: pgbench - extend initialization phase control

2019-11-07 Thread btkimurayuzk

2019-11-06 11:31 に Fujii Masao さんは書きました:
On Wed, Nov 6, 2019 at 6:23 AM Fabien COELHO  
wrote:



Hello,

>>> - for (step = initialize_steps; *step != '\0'; step++)
>>> + for (const char *step = initialize_steps; *step != '\0'; step++)
>
> But I still wonder why we should apply such change here.

Because it removes one declaration and reduces the scope of one 
variable?


> If there is the reason why this change is necessary here,

Nope, such changes are never necessary.

> I'm OK with that. But if not, basically I'd like to avoid the change.
> Otherwise it may make the back-patch a bit harder
> when we change the surrounding code.

I think that this is small enough so that it can be managed, if any 
back

patch occurs on the surrounding code, which is anyway pretty unlikely.

> Attached is the slightly updated version of the patch. Based on your
> patch, I added the descriptions about logging of "g" and "G" steps into
> the doc, and did some cosmetic changes. Barrying any objections,
> I'm thinking to commit this patch.

I'd suggest:

"to print one message each ..." -> "to print one message every ..."

"to print no progress ..." -> "not to print any progress ..."

I would not call "fprintf(stderr" twice in a row if I can call it 
once.


Thanks for the suggestion!
I updated the patch in that way and committed it!

This commit doesn't include the change "for (const char ...)"
and "merge two fprintf into one" ones that we were discussing.
Because they are trivial but I'm not sure if they are improvements
or not, yet. If they are, probably it's better to apply such changes
to all the places having the similar issues. But that seems overkill.



> While reviewing the patch, I found that current code allows space
> character to be specified in -I. That is, checkInitSteps() accepts
> space character. Why should we do this?

> Probably I understand why runInitSteps() needs to accept space character
> (because "v" in the specified string with -I is replaced with a space
> character when --no-vacuum option is given).

Yes, that is the reason, otherwise the string would have to be 
shifted.


> But I'm not sure why that's also necessary in checkInitSteps(). Instead,
> we should treat a space character as invalid in checkInitSteps()?

I think that it may break --no-vacuum, and I thought that there may be
other option which remove things, eventually. Also, having a NO-OP 
looks

ok to me.


As far as I read the code, checkInitSteps() checks the initialization
steps that users specified. The initialization steps string that
"v" was replaced with blank character is not given to checkInitSteps().
So ISTM that dropping the handling of blank character from
checkInitSteps() doesn't break --no-vacuum.


This is a patch which does not allow space character in -I options .

Regard,
Yu Kimura

diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 14dbc4510c..95b23895ff 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -4080,7 +4080,7 @@ checkInitSteps(const char *initialize_steps)
 
 	for (const char *step = initialize_steps; *step != '\0'; step++)
 	{
-		if (strchr(ALL_INIT_STEPS " ", *step) == NULL)
+		if (strchr(ALL_INIT_STEPS, *step) == NULL)
 		{
 			fprintf(stderr,
 	"unrecognized initialization step \"%c\"\n",


Add SQL function to show total block numbers in the relation

2019-10-30 Thread btkimurayuzk

Hello,


I propose new simple sql query, which shows total block numbers in the 
relation.


I now reviewing this patch (https://commitfest.postgresql.org/25/2211/) 
and I think,
it is usefull for knowing how many blocks there are in the relation to 
determine whether we use VACUUM RESUME or not.


Of cource, we can know this value such as

select (pg_relation_size('t') / 
current_setting('block_size')::bigint)::int;



but I think it is a litte bit complex.



Comment and feedback are very welcome.

Regards ,


Yu Kimuradiff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 28eb322f3f..99834e7286 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -21290,6 +21290,9 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());

 pg_relation_size

+   
+pg_relation_block_number
+   

 pg_size_bytes

@@ -21363,6 +21366,15 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
 Shorthand for pg_relation_size(..., 'main')

   
+  
+   
+pg_relation_block_number(relation regclass)
+
+   bigint
+   
+Shorthand for pg_relation_block_number(..., 'main')
+   
+  
   

 pg_size_bytes(text)
@@ -21504,6 +21516,11 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
 

 
+   
+pg_relation_block_number accepts the OID or name of a table
+and returns the number of blocks of that relation.
+   
+

 pg_size_pretty can be used to format the result of one of
 the other functions in a human-readable way, using bytes, kB, MB, GB or TB
diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c
index a87e7214e9..2462d65570 100644
--- a/src/backend/utils/adt/dbsize.c
+++ b/src/backend/utils/adt/dbsize.c
@@ -23,6 +23,7 @@
 #include "commands/tablespace.h"
 #include "miscadmin.h"
 #include "storage/fd.h"
+#include "storage/bufmgr.h"
 #include "utils/acl.h"
 #include "utils/builtins.h"
 #include "utils/numeric.h"
@@ -335,6 +336,25 @@ pg_relation_size(PG_FUNCTION_ARGS)
 	PG_RETURN_INT64(size);
 }
 
+Datum
+pg_relation_block_number(PG_FUNCTION_ARGS)
+{
+	Oid			relOid = PG_GETARG_OID(0);
+	Relation	rel;
+	int64		size;
+
+	rel = try_relation_open(relOid, AccessShareLock);
+
+	if (rel == NULL)
+		PG_RETURN_NULL();
+
+	size = RelationGetNumberOfBlocks(rel);
+
+	relation_close(rel, AccessShareLock);
+
+	PG_RETURN_INT64(size);
+}
+
 /*
  * Calculate total on-disk size of a TOAST relation, including its indexes.
  * Must not be applied to non-TOAST relations.
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 58ea5b982b..b68a523d3b 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6929,6 +6929,10 @@
   descr => 'disk space usage for the specified fork of a table or index',
   proname => 'pg_relation_size', provolatile => 'v', prorettype => 'int8',
   proargtypes => 'regclass text', prosrc => 'pg_relation_size' },
+{ oid => '2228',
+  descr => 'total block number for the main fork of the specified table or index',
+  proname => 'pg_relation_block_number', provolatile => 'v', prorettype => 'int8',
+  proargtypes => 'regclass', prosrc => 'pg_relation_block_number' },
 { oid => '2286',
   descr => 'total disk space usage for the specified table and associated indexes',
   proname => 'pg_total_relation_size', provolatile => 'v', prorettype => 'int8',