Re: [HACKERS] Specifying the unit in storage parameter
On Thu, Aug 28, 2014 at 12:55 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Wed, Aug 27, 2014 at 10:59 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Not necessarily, because it's harmless. It's there for purely aesthetical reasons, so it's your choice whether to add it or not. Having it there is slightly easier on somebody reading the code, perhaps. Agreed. On my side, that's up to you Fujii-san. The patch does what it states, I only think that this extra 0 should be added either everywhere or nowhere. Yep. I added extra 0 everywhere. Ok, I just applied the patch. Thanks for the review! Not mandatory either: drop test_param_unit in the regression tests after running the test queries. I don't have strong opinion about this. There are many tables which regression test creates but doesn't drop. But if you strongly think that the table must be dropped, I'm OK with that. Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Specifying the unit in storage parameter
On Thu, Aug 28, 2014 at 4:20 PM, Fujii Masao masao.fu...@gmail.com wrote: I don't have strong opinion about this. There are many tables which regression test creates but doesn't drop. But if you strongly think that the table must be dropped, I'm OK with that. This remark is just to limit the amount of trash in the database used for regression tests. But then if we'd remove everything we would lack handy material for tests on utilities like database-wide thingies of the type VACUUM, REINDEX, pg_dump, etc. And we can just drop the database used for regressions to clean up everything. So that's not mandatory at all. I tend to always clean up objects in my patches touching regressions to limit interactions with other tests, but I guess that's up to the person who wrote the code to decide. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Specifying the unit in storage parameter
Michael Paquier wrote: This remark is just to limit the amount of trash in the database used for regression tests. But then if we'd remove everything we would lack handy material for tests on utilities like database-wide thingies of the type VACUUM, REINDEX, pg_dump, etc. And we can just drop the database used for regressions to clean up everything. So that's not mandatory at all. I tend to always clean up objects in my patches touching regressions to limit interactions with other tests, but I guess that's up to the person who wrote the code to decide. Leaving lingering objects is not a bad thing, particularly if they have unusual properties; they enable somebody pg_dump'ing the database which can be a good test for pg_dump. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Specifying the unit in storage parameter
On Tue, Aug 26, 2014 at 3:27 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Fujii Masao wrote: On Thu, Aug 21, 2014 at 4:20 PM, Michael Paquier michael.paqu...@gmail.com wrote: Looking at the patch, the parameter fillfactor in the category RELOPT_KIND_HEAP (the first element in intRelOpts of reloptions.c) is not updated with the new field. It is only a one-line change. @@ -97,7 +97,7 @@ static relopt_int intRelOpts[] = Packs table pages only to this percentage, RELOPT_KIND_HEAP }, - HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100 + HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100, 0 }, Oh, good catch. I wonder why I did such a mistake... Uninitialized elements at end of struct are filled with zeroes. Yeah, that's the reason why I could not notice the problem at compile time. We do have other examples of this -- for instance, config_generic in the guc.c tables are almost always only 5 members long even though the struct is quite a bit longer than that. Most entries do not even have flags set. So you imply that the trailing zero (which the patch adds as flag) in the reloption struct should be dropped? Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Specifying the unit in storage parameter
Fujii Masao wrote: On Tue, Aug 26, 2014 at 3:27 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Fujii Masao wrote: On Thu, Aug 21, 2014 at 4:20 PM, Michael Paquier michael.paqu...@gmail.com wrote: Looking at the patch, the parameter fillfactor in the category RELOPT_KIND_HEAP (the first element in intRelOpts of reloptions.c) is not updated with the new field. It is only a one-line change. @@ -97,7 +97,7 @@ static relopt_int intRelOpts[] = Packs table pages only to this percentage, RELOPT_KIND_HEAP }, - HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100 + HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100, 0 }, Oh, good catch. I wonder why I did such a mistake... Uninitialized elements at end of struct are filled with zeroes. Yeah, that's the reason why I could not notice the problem at compile time. Right -- it's not something the compiler would warn you about. We do have other examples of this -- for instance, config_generic in the guc.c tables are almost always only 5 members long even though the struct is quite a bit longer than that. Most entries do not even have flags set. So you imply that the trailing zero (which the patch adds as flag) in the reloption struct should be dropped? Not necessarily, because it's harmless. It's there for purely aesthetical reasons, so it's your choice whether to add it or not. Having it there is slightly easier on somebody reading the code, perhaps. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Specifying the unit in storage parameter
On Wed, Aug 27, 2014 at 10:59 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Not necessarily, because it's harmless. It's there for purely aesthetical reasons, so it's your choice whether to add it or not. Having it there is slightly easier on somebody reading the code, perhaps. On my side, that's up to you Fujii-san. The patch does what it states, I only think that this extra 0 should be added either everywhere or nowhere. Not mandatory either: drop test_param_unit in the regression tests after running the test queries. Regards, -- Michael
Re: [HACKERS] Specifying the unit in storage parameter
On Thu, Aug 21, 2014 at 4:20 PM, Michael Paquier
michael.paqu...@gmail.com wrote:
On Fri, Aug 8, 2014 at 12:32 PM, Fujii Masao masao.fu...@gmail.com wrote:
This is not user-friendly. I'd like to propose the attached patch which
introduces the infrastructure which allows us to specify the unit when
setting INTEGER storage parameter like autovacuum_vacuum_cost_delay.
Comment? Review?
This patch makes autovacuum_vacuum_cost_delay more consistent with
what is at server level. So +1.
Thanks for reviewing the patch!
Looking at the patch, the parameter fillfactor in the category
RELOPT_KIND_HEAP (the first element in intRelOpts of reloptions.c) is
not updated with the new field. It is only a one-line change.
@@ -97,7 +97,7 @@ static relopt_int intRelOpts[] =
Packs table pages only to this percentage,
RELOPT_KIND_HEAP
},
- HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100
+ HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100, 0
},
Oh, good catch. I wonder why I did such a mistake...
Attached is the updated version of the patch.
Except that, I tested as well the patch and it works as expected. The
documentation, as well as the regression tests remain untouched, but I
guess that this is fine (not seeing similar tests in regressions, and
documentation does not specify the unit for a given parameter).
I think that it's worth adding the regression test for this feature.
Attached patch updates the regression test.
Regards,
--
Fujii Masao
*** a/src/backend/access/common/reloptions.c
--- b/src/backend/access/common/reloptions.c
***
*** 97,103 static relopt_int intRelOpts[] =
Packs table pages only to this percentage,
RELOPT_KIND_HEAP
},
! HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100
},
{
{
--- 97,103
Packs table pages only to this percentage,
RELOPT_KIND_HEAP
},
! HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100, 0
},
{
{
***
*** 105,111 static relopt_int intRelOpts[] =
Packs btree index pages only to this percentage,
RELOPT_KIND_BTREE
},
! BTREE_DEFAULT_FILLFACTOR, BTREE_MIN_FILLFACTOR, 100
},
{
{
--- 105,111
Packs btree index pages only to this percentage,
RELOPT_KIND_BTREE
},
! BTREE_DEFAULT_FILLFACTOR, BTREE_MIN_FILLFACTOR, 100, 0
},
{
{
***
*** 113,119 static relopt_int intRelOpts[] =
Packs hash index pages only to this percentage,
RELOPT_KIND_HASH
},
! HASH_DEFAULT_FILLFACTOR, HASH_MIN_FILLFACTOR, 100
},
{
{
--- 113,119
Packs hash index pages only to this percentage,
RELOPT_KIND_HASH
},
! HASH_DEFAULT_FILLFACTOR, HASH_MIN_FILLFACTOR, 100, 0
},
{
{
***
*** 121,127 static relopt_int intRelOpts[] =
Packs gist index pages only to this percentage,
RELOPT_KIND_GIST
},
! GIST_DEFAULT_FILLFACTOR, GIST_MIN_FILLFACTOR, 100
},
{
{
--- 121,127
Packs gist index pages only to this percentage,
RELOPT_KIND_GIST
},
! GIST_DEFAULT_FILLFACTOR, GIST_MIN_FILLFACTOR, 100, 0
},
{
{
***
*** 129,135 static relopt_int intRelOpts[] =
Packs spgist index pages only to this percentage,
RELOPT_KIND_SPGIST
},
! SPGIST_DEFAULT_FILLFACTOR, SPGIST_MIN_FILLFACTOR, 100
},
{
{
--- 129,135
Packs spgist index pages only to this percentage,
RELOPT_KIND_SPGIST
},
! SPGIST_DEFAULT_FILLFACTOR, SPGIST_MIN_FILLFACTOR, 100, 0
},
{
{
***
*** 137,143 static relopt_int intRelOpts[] =
Minimum number of tuple updates or deletes prior to vacuum,
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 0, INT_MAX
},
{
{
--- 137,143
Minimum number of tuple updates or deletes prior to vacuum,
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 0, INT_MAX, 0
},
{
{
***
*** 145,151 static relopt_int intRelOpts[] =
Minimum number of tuple inserts, updates or deletes prior to analyze,
RELOPT_KIND_HEAP
},
! -1, 0, INT_MAX
},
{
{
--- 145,151
Minimum number of tuple inserts, updates or deletes prior to analyze,
RELOPT_KIND_HEAP
},
! -1, 0, INT_MAX, 0
},
{
{
***
*** 153,159 static relopt_int intRelOpts[] =
Vacuum cost delay in milliseconds, for autovacuum,
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 0, 100
},
{
{
--- 153,159
Vacuum cost delay in milliseconds, for autovacuum,
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 0, 100, GUC_UNIT_MS
},
{
{
***
*** 161,167 static relopt_int intRelOpts[] =
Vacuum cost amount available before napping, for autovacuum,
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 1, 1
},
{
{
--- 161,167
Re: [HACKERS] Specifying the unit in storage parameter
Fujii Masao wrote: On Thu, Aug 21, 2014 at 4:20 PM, Michael Paquier michael.paqu...@gmail.com wrote: Looking at the patch, the parameter fillfactor in the category RELOPT_KIND_HEAP (the first element in intRelOpts of reloptions.c) is not updated with the new field. It is only a one-line change. @@ -97,7 +97,7 @@ static relopt_int intRelOpts[] = Packs table pages only to this percentage, RELOPT_KIND_HEAP }, - HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100 + HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100, 0 }, Oh, good catch. I wonder why I did such a mistake... Uninitialized elements at end of struct are filled with zeroes. We do have other examples of this -- for instance, config_generic in the guc.c tables are almost always only 5 members long even though the struct is quite a bit longer than that. Most entries do not even have flags set. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Specifying the unit in storage parameter
On Fri, Aug 8, 2014 at 12:32 PM, Fujii Masao masao.fu...@gmail.com wrote: This is not user-friendly. I'd like to propose the attached patch which introduces the infrastructure which allows us to specify the unit when setting INTEGER storage parameter like autovacuum_vacuum_cost_delay. Comment? Review? This patch makes autovacuum_vacuum_cost_delay more consistent with what is at server level. So +1. Looking at the patch, the parameter fillfactor in the category RELOPT_KIND_HEAP (the first element in intRelOpts of reloptions.c) is not updated with the new field. It is only a one-line change. @@ -97,7 +97,7 @@ static relopt_int intRelOpts[] = Packs table pages only to this percentage, RELOPT_KIND_HEAP }, - HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100 + HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100, 0 }, Except that, I tested as well the patch and it works as expected. The documentation, as well as the regression tests remain untouched, but I guess that this is fine (not seeing similar tests in regressions, and documentation does not specify the unit for a given parameter). Regards, -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Specifying the unit in storage parameter
On 08/07/2014 08:32 PM, Fujii Masao wrote: This is not user-friendly. I'd like to propose the attached patch which introduces the infrastructure which allows us to specify the unit when setting INTEGER storage parameter like autovacuum_vacuum_cost_delay. Comment? Review? No review, but thank you for doing this! -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Specifying the unit in storage parameter
Hi,
We can specify the unit when setting autovacuum_vacuum_cost_delay
GUC as follows.
ALTER SYSTEM SET autovacuum_vacuum_cost_delay TO '80ms';
OTOH we cannot specify the unit when setting autovacuum_vacuum_cost_delay
as storage parameter as follows.
CREATE TABLE test (col1 int) WITH (autovacuum_vacuum_cost_delay = '80ms');
ERROR: invalid value for integer option
autovacuum_vacuum_cost_delay: 80ms
This is not user-friendly. I'd like to propose the attached patch which
introduces the infrastructure which allows us to specify the unit when
setting INTEGER storage parameter like autovacuum_vacuum_cost_delay.
Comment? Review?
Regards,
--
Fujii Masao
*** a/src/backend/access/common/reloptions.c
--- b/src/backend/access/common/reloptions.c
***
*** 105,111 static relopt_int intRelOpts[] =
Packs btree index pages only to this percentage,
RELOPT_KIND_BTREE
},
! BTREE_DEFAULT_FILLFACTOR, BTREE_MIN_FILLFACTOR, 100
},
{
{
--- 105,111
Packs btree index pages only to this percentage,
RELOPT_KIND_BTREE
},
! BTREE_DEFAULT_FILLFACTOR, BTREE_MIN_FILLFACTOR, 100, 0
},
{
{
***
*** 113,119 static relopt_int intRelOpts[] =
Packs hash index pages only to this percentage,
RELOPT_KIND_HASH
},
! HASH_DEFAULT_FILLFACTOR, HASH_MIN_FILLFACTOR, 100
},
{
{
--- 113,119
Packs hash index pages only to this percentage,
RELOPT_KIND_HASH
},
! HASH_DEFAULT_FILLFACTOR, HASH_MIN_FILLFACTOR, 100, 0
},
{
{
***
*** 121,127 static relopt_int intRelOpts[] =
Packs gist index pages only to this percentage,
RELOPT_KIND_GIST
},
! GIST_DEFAULT_FILLFACTOR, GIST_MIN_FILLFACTOR, 100
},
{
{
--- 121,127
Packs gist index pages only to this percentage,
RELOPT_KIND_GIST
},
! GIST_DEFAULT_FILLFACTOR, GIST_MIN_FILLFACTOR, 100, 0
},
{
{
***
*** 129,135 static relopt_int intRelOpts[] =
Packs spgist index pages only to this percentage,
RELOPT_KIND_SPGIST
},
! SPGIST_DEFAULT_FILLFACTOR, SPGIST_MIN_FILLFACTOR, 100
},
{
{
--- 129,135
Packs spgist index pages only to this percentage,
RELOPT_KIND_SPGIST
},
! SPGIST_DEFAULT_FILLFACTOR, SPGIST_MIN_FILLFACTOR, 100, 0
},
{
{
***
*** 137,143 static relopt_int intRelOpts[] =
Minimum number of tuple updates or deletes prior to vacuum,
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 0, INT_MAX
},
{
{
--- 137,143
Minimum number of tuple updates or deletes prior to vacuum,
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 0, INT_MAX, 0
},
{
{
***
*** 145,151 static relopt_int intRelOpts[] =
Minimum number of tuple inserts, updates or deletes prior to analyze,
RELOPT_KIND_HEAP
},
! -1, 0, INT_MAX
},
{
{
--- 145,151
Minimum number of tuple inserts, updates or deletes prior to analyze,
RELOPT_KIND_HEAP
},
! -1, 0, INT_MAX, 0
},
{
{
***
*** 153,159 static relopt_int intRelOpts[] =
Vacuum cost delay in milliseconds, for autovacuum,
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 0, 100
},
{
{
--- 153,159
Vacuum cost delay in milliseconds, for autovacuum,
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 0, 100, GUC_UNIT_MS
},
{
{
***
*** 161,167 static relopt_int intRelOpts[] =
Vacuum cost amount available before napping, for autovacuum,
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 1, 1
},
{
{
--- 161,167
Vacuum cost amount available before napping, for autovacuum,
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 1, 1, 0
},
{
{
***
*** 169,175 static relopt_int intRelOpts[] =
Minimum age at which VACUUM should freeze a table row, for autovacuum,
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 0, 10
},
{
{
--- 169,175
Minimum age at which VACUUM should freeze a table row, for autovacuum,
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 0, 10, 0
},
{
{
***
*** 177,183 static relopt_int intRelOpts[] =
Minimum multixact age at which VACUUM should freeze a row multixact's, for autovacuum,
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 0, 10
},
{
{
--- 177,183
Minimum multixact age at which VACUUM should freeze a row multixact's, for autovacuum,
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 0, 10, 0
},
{
{
***
*** 185,191 static relopt_int intRelOpts[] =
Age at which to autovacuum a table to prevent transaction ID wraparound,
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
},
! -1, 1, 20
},
{
{
--- 185,191
Age at which
Re: [HACKERS] Specifying the unit in storage parameter
Fujii Masao wrote: Hi, We can specify the unit when setting autovacuum_vacuum_cost_delay GUC as follows. ALTER SYSTEM SET autovacuum_vacuum_cost_delay TO '80ms'; OTOH we cannot specify the unit when setting autovacuum_vacuum_cost_delay as storage parameter as follows. CREATE TABLE test (col1 int) WITH (autovacuum_vacuum_cost_delay = '80ms'); ERROR: invalid value for integer option autovacuum_vacuum_cost_delay: 80ms This is not user-friendly. No disagreement here. I'd like to propose the attached patch which introduces the infrastructure which allows us to specify the unit when setting INTEGER storage parameter like autovacuum_vacuum_cost_delay. Comment? Review? Hm, what's with the parse_int signature change and the hintmsg thing? Is it just me or the patch is incomplete? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Specifying the unit in storage parameter
On Fri, Aug 8, 2014 at 12:56 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Fujii Masao wrote: Hi, We can specify the unit when setting autovacuum_vacuum_cost_delay GUC as follows. ALTER SYSTEM SET autovacuum_vacuum_cost_delay TO '80ms'; OTOH we cannot specify the unit when setting autovacuum_vacuum_cost_delay as storage parameter as follows. CREATE TABLE test (col1 int) WITH (autovacuum_vacuum_cost_delay = '80ms'); ERROR: invalid value for integer option autovacuum_vacuum_cost_delay: 80ms This is not user-friendly. No disagreement here. I'd like to propose the attached patch which introduces the infrastructure which allows us to specify the unit when setting INTEGER storage parameter like autovacuum_vacuum_cost_delay. Comment? Review? Hm, what's with the parse_int signature change and the hintmsg thing? Is it just me or the patch is incomplete? Sorry, probably I failed to see your point. You mean that the signature of parse_int needs to be changed so that it includes the hintmsg as the argument? If yes, there is no problem. Both signature and function body of parse_int has already included the hingmsg as the argument so far. Am I missing something? Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Specifying the unit in storage parameter
Fujii Masao wrote: On Fri, Aug 8, 2014 at 12:56 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Hm, what's with the parse_int signature change and the hintmsg thing? Is it just me or the patch is incomplete? Sorry, probably I failed to see your point. You mean that the signature of parse_int needs to be changed so that it includes the hintmsg as the argument? If yes, there is no problem. Both signature and function body of parse_int has already included the hingmsg as the argument so far. Am I missing something? I just mean that the parse_int function body is not touched by your patch, unless I am failing to see something. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Specifying the unit in storage parameter
On Fri, Aug 8, 2014 at 2:12 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Fujii Masao wrote: On Fri, Aug 8, 2014 at 12:56 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Hm, what's with the parse_int signature change and the hintmsg thing? Is it just me or the patch is incomplete? Sorry, probably I failed to see your point. You mean that the signature of parse_int needs to be changed so that it includes the hintmsg as the argument? If yes, there is no problem. Both signature and function body of parse_int has already included the hingmsg as the argument so far. Am I missing something? I just mean that the parse_int function body is not touched by your patch, unless I am failing to see something. Yes, my patch doesn't change the parse_int function at all because I didn't think such change is required for the purpose (i.e., just allows us to specify the unit in the setting of storage parameters). But, you might find the reason why it needs to be changed? Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
