Re: [HACKERS] proposal: rounding up time value less than its unit.
Hi Stephen, As you said, I'm not good at English, so I'm glad you handle this thread. I'll wait for the good changing. Thank you very very much! regards, -- Tomonari Katsumata 2014-09-23 14:23 GMT+09:00 Stephen Frost : > * Tom Lane (t...@sss.pgh.pa.us) wrote: > > Stephen Frost writes: > > > To clarify- we'll simply swap from (essentially) floor() to ceil() for > > > handling all GUC_with_unit to internal_unit conversions, document that, > > > and note it in the release notes as a possible behavior change and move > > > on. > > > > Worksforme. > > Great, thanks. I'll wait a couple days to see if anyone else wants to > voice a concern. > > Tomonari, don't worry about updating the patch (unless you really want > to) as I suspect I'll be changing around the wording anyway. No > offense, please, but I suspect English isn't your first language and > it'll be simpler for me if I just handle it. Thanks a lot for the bug > report and discussion and I'll be sure to credit you for it in the > commit. > > Thanks again! > > Stephen >
Re: [HACKERS] Commitfest status
Hi, I've update my entry. [rounding up time value less than its unit] https://commitfest.postgresql.org/action/patch_view?id=1507 regards, - Tomonari Katsumata (2014/09/12 7:03), Tomas Vondra wrote: > On 11.9.2014 21:14, Petr Jelinek wrote: >> On 11/09/14 18:59, Tomas Vondra wrote: >>> On 10.9.2014 22:39, Heikki Linnakangas wrote: >>>> The bad news is that the rest don't seem to moving graduating from the >>>> Needs Review state. >>> >>> ISTM that many patches >>> >>> (b) in 'waiting on author' are not really waiting, because the author >>> already responded / posted a new version of the patch >>> >>> Except that the patch status was not updated, whis makes it really >>> difficult to spot patches that currently need a review :-( >>> >> >> I think that still means patch is 'waiting for author' as author is >> responsible for changing this. > > In that case it was meant as a plea to the authors to update this ;-) > > > Tomas > > -- 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] proposal: rounding up time value less than its unit.
Hi, I'm sorry for slow reaction. I don't care whether rounding up or down it, although this title has 'rounding up'. (I just only come up with it. I'm sorry for my imprudence) I'm thinking about a method which users get quick awareness it. Now, it's okay not to change current behavior except non-zero value yields a zero. A zero rounded down from non-zero gets an error. I attached new patch. This includes a document about above behavior as Heikki suggested. regards, -- Tomonari Katsumata 2014-08-27 6:49 GMT+09:00 David G Johnston : > Tom Lane-2 wrote > > Robert Haas < > > > robertmhaas@ > > > > writes: > >> I liked David Johnston's even stronger suggestion upthread: make it an > >> error to specify a value requires rounding of any kind. In other > >> words, if the minimum granularity is 1 minute, you can specify that as > >> 60 seconds instead, but if you write 59 seconds, we error out. Maybe > >> that seems pedantic, but I don't think users will much appreciate the > >> discovery that 30 seconds means 60 seconds. They'll be happier to be > >> told that up front than having to work it out afterward. > > > > I think this is totally wrong. The entire point of the GUC units system, > > or at least a large part of the point, is that users should not have to > > be intimately aware of the units in which a given value is measured > > internally. And that in turn means that the units had better be such > > that users won't find them overly coarse. If it matters a lot whether > > 59 seconds gets rounded to 60, then we didn't make a good choice of units > > for the GUC in question; and we should fix that choice, not mess with the > > rounding rules. > > > > The case where this argument falls down is for "special" values, such as > > where zero means something quite different from the smallest nonzero > > value. Peter suggested upthread that we should redefine any GUC values > > for which that is true, but (a) I think that loses on backwards > > compatibility grounds, and (b) ISTM zero is probably always special to > > some extent. A zero time delay for example is not likely to work. > > > > Maybe we should leave the rounding behavior alone (there's not much > > evidence that rounding in one direction is worse than another; although > > I'd also be okay with changing to round-to-nearest), and confine > ourselves > > to throwing an error for the single case that an apparently nonzero input > > value is truncated/rounded to zero as a result of units conversion. > > To Andres' point: > > SELECT unit, count(*) FROM pg_settings WHERE unit <> '' GROUP BY unit; (9.3 > / Ubuntu) > > min (1 - log_rotation_age) > s (10) > ms (13) > > kb (7) > 8kb (6) > > I don't know about the size implications but they seem to be non-existent. > That any setting critically matters at +/- 1s or 1ms doesn't seem likely in > practice. Even +/- 1min for a setting, if it did matter at extreme scale, > would be recognizable by the user in practice as a rounding artifact and > compensated for. > > At this point throwing an error for any precision that results in less than > the default precision is my preference. I would not change the rounding > rules for the simple reason that there is no obvious improvement to be had > and so why introduce pointless change that - however marginal and unlikely > - > will be user-visible. > > The complaint to overcome is avoiding an interpretation of "zero" when the > precision of the input is less than the GUC unit. Lacking any concrete > complaints about our round-down policy I don't see where a change there is > worthwhile. > > Fixing zero as a special value falls under the same category. As > mathematically pure as using infinity may be the trade-off for practicality > and usability seems, even in light of this complaint, like the correct one > to have made. > > David J. > > > > > > > > > -- > View this message in context: > http://postgresql.1045698.n5.nabble.com/proposal-rounding-up-time-value-less-than-its-unit-tp5811102p5816409.html > Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > error_for_less-than_required_time-unit.patch Description: Binary data -- 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] proposal: rounding up time value less than its unit.
Thank you for the comments. It was a bug in my patch as another developer says. I've not considered about the value 'zero', sorry. I attached new patch. This patch rounds up the value when only it's less than required unit. Like below. (unit: min) 0->0 0s->0 10s->1 70s->1 Although my original complaint is fixed, I'm worried about this change will make users confusing. Is it better to raise a message(ex. INFO) when a value less than required unit is set? 2014-08-21 21:00 GMT+09:00 Heikki Linnakangas : > On 07/10/2014 09:52 AM, Tomonari Katsumata wrote: > >> Hi, >> >> Several couple weeks ago, I posted a mail to pgsql-doc. >> http://www.postgresql.org/message-id/53992ff8.2060...@po.ntts.co.jp >> >> In this thread, I concluded that it's better to >> round up the value which is less than its unit. >> Current behavior (rounding down) has undesirable setting risk, >> because some GUCs have special meaning for 0. >> >> And then I made a patch for this. >> Please check the attached patch. >> > > The patch also rounds a zero up to one. A naked zero with no unit is not > affected, but e.g if you have "log_rotation_age=0s", it will not disable > the feature as you might expect, but set it to 1 minute. Should we do > something about that? > > If we're going to explain the rounding up in the manual, we also need to > explain the normal rule, which is to round down. How about this: > > --- a/doc/src/sgml/config.sgml > +++ b/doc/src/sgml/config.sgml > @@ -44,6 +44,15 @@ > (seconds), min (minutes), h > (hours), and d (days). Note that the multiplier > for memory units is 1024, not 1000. > + > + > + If a memory or time setting is specified with more precision than the > + implicit unit of the setting, it is rounded down. However, if > rounding > + down would yield a zero, it is rounded up to one instead. For > example, > + the implicit unit of log_rotation_age so if > + you set it to 150s, it will be rounded down to two > + minutes. However, if you set it to 10s, it will be > + rounded up to one minute. > > > - Heikki > > > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > time-unit-guc-round-up_v2.patch Description: Binary data -- 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] proposal: rounding up time value less than its unit.
Hi Robert, Thank you for checking this! I've added it to commitfest. https://commitfest.postgresql.org/action/patch_view?id=1507 regards, Tomonari Katsumata 2014-07-12 6:07 GMT+09:00 Robert Haas : > On Thu, Jul 10, 2014 at 2:52 AM, Tomonari Katsumata > wrote: > > Several couple weeks ago, I posted a mail to pgsql-doc. > > http://www.postgresql.org/message-id/53992ff8.2060...@po.ntts.co.jp > > > > In this thread, I concluded that it's better to > > round up the value which is less than its unit. > > Current behavior (rounding down) has undesirable setting risk, > > because some GUCs have special meaning for 0. > > > > And then I made a patch for this. > > Please check the attached patch. > > Thanks for the patch. Please add it here: > > https://commitfest.postgresql.org/action/commitfest_view/open > > -- > Robert Haas > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers >
[HACKERS] proposal: rounding up time value less than its unit.
Hi,
Several couple weeks ago, I posted a mail to pgsql-doc.
http://www.postgresql.org/message-id/53992ff8.2060...@po.ntts.co.jp
In this thread, I concluded that it's better to
round up the value which is less than its unit.
Current behavior (rounding down) has undesirable setting risk,
because some GUCs have special meaning for 0.
And then I made a patch for this.
Please check the attached patch.
regards,
---
Tomonari Katsumata
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 04ddd73..9aaffb0 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -43,7 +43,9 @@
are ms (milliseconds), s
(seconds), min (minutes), h
(hours), and d (days). Note that the multiplier
- for memory units is 1024, not 1000.
+ for memory units is 1024, not 1000. And also note that if you set
+ less value than which is expected for the time setting, the value
+ would be rounded up.
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 3a31a75..8ba4879 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -5122,9 +5122,11 @@ parse_int(const char *value, int *result, int flags, const char **hintmsg)
switch (flags & GUC_UNIT_TIME)
{
case GUC_UNIT_S:
+ val += (val < MS_PER_S) ? MS_PER_S : 0;
val /= MS_PER_S;
break;
case GUC_UNIT_MIN:
+ val += (val < MS_PER_MIN) ? MS_PER_MIN : 0;
val /= MS_PER_MIN;
break;
}
@@ -5138,6 +5140,7 @@ parse_int(const char *value, int *result, int flags, const char **hintmsg)
val *= MS_PER_S;
break;
case GUC_UNIT_MIN:
+ val += (val < S_PER_MIN) ? S_PER_MIN : 0;
val /= S_PER_MIN;
break;
}
--
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] 9.4 release notes
Hi, I have two comments about 9.4 release notes. 1. typo >Pg_upgrade now uses -U to specify the user name (Bruce Momjian) It should be pg_upgrade. 2. undesirable link >Allow pg_recvlogical to receive data logical decoding data (Andres Freund) The term of "pg_recvlogical" jumps to a page of pg_receivexlog. It should jump to pg_recvlogical(app-pgrecvlogical.html). regards, ---- Tomonari Katsumata -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] small typo about comment in xlog.c
Hi, I'm reading xlog.c, and I noticed a comment of do_pg_abort_backup is typo. ... 10247 * NB: This is only for aborting a non-exclusive backup that doesn't write 10248 * backup_label. A backup started with pg_stop_backup() needs to be finished 10249 * with pg_stop_backup(). ... I think "A backup started with pg_stop_backup()" should be "A backup started with pg_start_backup()". This is a bug about source comment, so it's not big problem. But I want to fix the comment. See attached patch. regards, -- Tomonari Katsumata diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 0b4a5f6..3551d94 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -10245,7 +10245,7 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p) * an error handler. * * NB: This is only for aborting a non-exclusive backup that doesn't write - * backup_label. A backup started with pg_stop_backup() needs to be finished + * backup_label. A backup started with pg_start_backup() needs to be finished * with pg_stop_backup(). */ void -- 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] Little confusing things about client_min_messages.
Hi 2014-03-10 23:45 GMT+09:00 Tom Lane : > Tomonari Katsumata writes: > > Adding FATAL and PANIC to client_min_messages is done at below-commit. > > 8ac386226d76b29a9f54c26b157e04e9b8368606 > > > http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=8ac386226d76b29a9f54c26b157e04e9b8368606 > > > According to the commit log, it seems that the purpose > > is suppressing to be sent error message to client when "DROP TABLE". > > In those days(pre 8.1), we did not have "DROP IF EXISTS" syntax, > > so it was useful. > > > If this was the reason, now(from 8.2) we have "DROP IF EXISTS" syntax, > > Uh, that was one example of what it might be good for; I doubt that the > use-case has now vanished entirely. While I'm still dubious about the > reliability of suppressing error messages, if people have been using this > type of coding for nearly 10 years then it probably works well enough > ... and more to the point, they won't thank us for arbitrarily removing > it. > Maybe so. > > I think we should leave established practice alone here. It might be > confusing at first glance, but that doesn't mean it's the wrong thing. > > > I see. If we delete it, it maybe become more confusing thing. Thank you for your opinion. regards, --- Tomonari Katsumata
Re: [HACKERS] Little confusing things about client_min_messages.
Hi Tom, Bruce,
Thank you for your response.
(2014/03/09 2:12), Tom Lane wrote:
> Bruce Momjian writes:
>> On Sat, Mar 8, 2014 at 11:31:22AM -0500, Tom Lane wrote:
>>> Tomonari Katsumata writes:
>>>> [ client_min_messages = info is not documented ]
>
>>> That's intentional, because it's not a useful setting. Even more so
>>> for the other two.
>
>> Well, 'info' is between other settings we do document, so I am not clear
>> why info should be excluded. It is because we always output INFO to the
>> client? From elog.c:
>
>> if (ClientAuthInProgress)
>> output_to_client = (elevel >= ERROR);
>> else
>> output_to_client = (elevel >= client_min_messages ||
>> elevel == INFO);
>
> Right, so if you did set it to that, it wouldn't be functionally different
> from NOTICE.
>
I understand it's intensional.
We can set INFO but it doesn't have any difference from NOTICE
because all INFO messages are sent to client.
So it is hidden from the document.
The word "valid" in the document has meant "meaningful".
I've misread it was "setable".
> So no, I don't think we ought to be advertising these as suggested
> values. A saner proposed patch would be to remove them from the
> valid values altogether. We probably had some good reason for leaving
> them in the list back when, but I'm having a hard time reconstructing
> what that would be.
>
Adding FATAL and PANIC to client_min_messages is done at below-commit.
8ac386226d76b29a9f54c26b157e04e9b8368606
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=8ac386226d76b29a9f54c26b157e04e9b8368606
According to the commit log, it seems that the purpose
is suppressing to be sent error message to client when "DROP TABLE".
In those days(pre 8.1), we did not have "DROP IF EXISTS" syntax,
so it was useful.
If this was the reason, now(from 8.2) we have "DROP IF EXISTS" syntax,
so we could delete FATAL and PANIC from client_min_messages valid value.
Attached patch do it. Please check it.
regards,
---
Tomonari Katsumata
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 2811f11..cbaf264 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -3796,8 +3796,7 @@ local0.*/var/log/postgresql
Valid values are DEBUG5,
DEBUG4, DEBUG3, DEBUG2,
DEBUG1, LOG, NOTICE,
-WARNING, ERROR, FATAL,
-and PANIC. Each level
+WARNING, and ERROR. Each level
includes all the levels that follow it. The later the level,
the fewer messages are sent. The default is
NOTICE. Note that LOG has a different
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index c76edb4..b7cfe14 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -242,8 +242,6 @@ static const struct config_enum_entry client_message_level_options[] = {
{"notice", NOTICE, false},
{"warning", WARNING, false},
{"error", ERROR, false},
- {"fatal", FATAL, true},
- {"panic", PANIC, true},
{NULL, 0, false}
};
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Little confusing things about client_min_messages.
Hi,
I noticed that the behavior of client_min_messages do not have
a consistency in document and 'pg_settings', postgresql.conf.
the behaviro is:
I can set 'INFO', 'FATAL' and 'PANIC' as the valid value.
postgres=# set client_min_messages to info;
SET
postgres=# set client_min_messages to fatal;
SET
postgres=# set client_min_messages to panic;
SET
document says:
DEBUG1, LOG, NOTICE,
WARNING, ERROR, FATAL,
and PANIC. Each level
I couldn't understand the reason of disappearing 'INFO' from the document.
pg_settings says:
{debug5,debug4,debug3,debug2,debug1,log,notice,warning,error}
and
postgresql.conf says:
#client_min_messages = notice # values in order of decreasing
detail:
# debug5
# debug4
# debug3
# debug2
# debug1
# log
# notice
# warning
# error
also I couldn't understand the reason of disappearing
'info', 'fatal' and 'panic' from them.
My proposal is all valid values should be present for users.
I fixed this, please see the attached patch.
regards,
---
Tomonari Katsumata
clear_client_min_messages.patch
Description: Binary data
--
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] [BUG] Archive recovery failure on 9.3+.
Hi Heikki, I need PostgreSQL9.3 which fixed this problem. It didn't happen in PostgreSQL9.2, so I agree with your proposal which changes are done against 93_STABLE and master. Can you fix this in next release(9.3.3)? Tomonari Katsumata (2014/01/13 20:16), Heikki Linnakangas wrote: > On 01/09/2014 10:55 PM, Josh Berkus wrote: >> On 01/09/2014 12:05 PM, Heikki Linnakangas wrote: >>> Actually, why is the partially-filled 00010002 file >>> archived in the first place? Looking at the code, it's been like that >>> forever, but it seems like a bad idea. If the original server is still >>> up and running, and writing more data to that file, what will happen is >>> that when the original server later tries to archive it, it will fail >>> because the partial version of the file is already in the archive. Or >>> worse, the partial version overwrites a previously archived more >>> complete version. >> >> Oh! This explains some transient errors I've seen. >>> Wouldn't it be better to not archive the old segment, and instead switch >>> to a new segment after writing the end-of-recovery checkpoint, so that >>> the segment on the new timeline is archived sooner? >> >> It would be better to zero-fill and switch segments, yes. We should >> NEVER be in a position of archiving two different versions of the same >> segment. > > Ok, I think we're in agreement that that's the way to go for master. > > Now, what to do about back-branches? On one hand, I'd like to apply the same fix to all stable branches, as the current behavior is silly and always has been. On the other hand, we haven't heard any complaints about it, so we probably shouldn't fix what ain't broken. Perhaps we should apply it to 9.3, as that's where we have the acute problem the OP reported. Thoughts? > > In summary, I propose that we change master and REL9_3_STABLE to not archive the partial segment from previous timeline. Older branches will keep the current behavior. > > - Heikki > -- 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] [BUG] Archive recovery failure on 9.3+.
Hi,
Somebody is reading this thread?
This problem seems still remaining on REL9_3_STABLE.
Many users would face this problem, so we should
resolve this in next release.
I think his patch is reasonable to fix this problem.
Please check this again.
regards,
--
Tomonari Katsumata
2013/12/12 Kyotaro HORIGUCHI
> Hello, we happened to see server crash on archive recovery under
> some condition.
>
> After TLI was incremented, there should be the case that the WAL
> file for older timeline is archived but not for that of the same
> segment id but for newer timeline. Archive recovery should fail
> for the case with PANIC error like follows,
>
> | PANIC: record with zero length at 0/1820D40
>
> Replay script is attached. This issue occured for 9.4dev, 9.3.2,
> and not for 9.2.6 and 9.1.11. The latter search pg_xlog for the
> TLI before trying archive for older TLIs.
>
> This occurrs during fetching checkpoint redo record in archive
> recovery.
>
> > if (checkPoint.redo < RecPtr)
> > {
> > /* back up to find the record */
> > record = ReadRecord(xlogreader, checkPoint.redo, PANIC, false);
>
> And this is caused by that the segment file for older timeline in
> archive directory is preferred to that for newer timeline in
> pg_xlog.
>
> Looking into pg_xlog before trying the older TLIs in archive like
> 9.2- fixes this issue. The attached patch is one possible
> solution for 9.4dev.
>
> Attached files are,
>
> - recvtest.sh: Replay script. Step 1 and 2 makes the condition
>and step 3 causes the issue.
>
> - archrecvfix_20131212.patch: The patch fixes the issue. Archive
>recovery reads pg_xlog before trying older TLI in archive
>similarly to 9.1- by this patch.
>
> regards,
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
>
>
> --
> 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] The PostgreSQL License requires "LICENSE" file?
(2013/09/04 2:34), Josh Berkus wrote: > >>>> I've checked the PostgreSQL License, >>>> and had a question. >>>> http://opensource.org/licenses/PostgreSQL >> >> I don't know who opensource.org are, but their idea of "the PostgreSQL >> license" doesn't appear to match the actual license text. > > The OSI, which is the organization which defines whether licenses are > open source or not. We requested that they list The PostgreSQL License > on their site after Red Hat determined that our license had sufficent > textual differences from the BSD License to be a different license. > > I've requested that the spurious "file name" reference be removed. > Thank you for checking it soon. I understands that "the PostgreSQL license" does not require any specified file name and missing "LICENSE" is not problem. I'm Sorry for the stupid question. regards, Tomonari Katsumata -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] The PostgreSQL License requires "LICENSE" file?
Hi, PostgreSQL is released under "the PostgreSQL License". http://www.postgresql.org/about/licence/ I've checked the PostgreSQL License, and had a question. http://opensource.org/licenses/PostgreSQL It seems that the license requres a "LICENSE" file in the target product [quote] > Then put the license into a file called "LICENSE" in > your software distribution. But I couldn't find "LICENSE" file in current PostgreSQL source tree, instead found a "COPYRIGHT" file. It seems the contents of the "COPYRIGTH" file are all required by the license. So I have confused why PostgreSQL has a "COPYRIGHT" file instead of "LICENSE". Anybody knows the reason? And is this non problem thing? regards, - Tomonari Katsumata
Re: [HACKERS] How to create read-only view on 9.3
Hi, (2013/08/14 5:24), Josh Berkus wrote: > On 08/13/2013 11:18 AM, Tom Lane wrote: >> Hannu Krosing writes: >>> If you earlier used views for granting limited read access to some views >>> you definitely did not want view users suddenly gain also write access to >>> underlying table. >> >> Unless you'd explicitly granted those users insert/update/delete privilege >> on the view, they wouldn't suddenly be able to do something new in 9.3, >> because no such privileges are granted by default. If you had granted >> such privileges, you don't have much of a leg to stand on for complaining >> that now they can do it. > > Ah, ok. I hadn't gotten to the testing phase yet. > > I think we should have a script available for revoking all write privs > on all views and link it from somewhere (the release notes?), but I > don't see any need to change anything in the release. > Yes, I was not thinking about changing current 9.3 behavior. So I think it's enough to know the impact and how to avoid that on the release notes. thanks a lot! regards, --- NTT Software Corporation Tomonari Katsumata -- 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] How to create read-only view on 9.3
Hi Szymon, Thank you for response. >> Could you show an example? > I do below things on one server. The path to database cluster and port are different with each other. [9.2.4] initdb --no-locale -E UTF8 pg_ctl start createdb testdb psql testdb -c "create table tbl(i int)" psql testdb -c "insert into tbl values (generate_series(1,10))" psql testdb -c "create view v as select * from tbl" [9.3beta2] pg_dump -p testdb > /tmp/92dmp.dmp initdb --no-locale -E UTF8 pg_ctl start createdb testdb psql testdb -f /tmp/92dmp.dmp After all, the view v became updatable view. --- $ psql testdb psql (9.3beta2) Type "help" for help. testdb=# select * from v; i 1 2 3 4 5 6 7 8 9 10 (10 rows) testdb=# insert into v values (11); INSERT 0 1 testdb=# select * from v; i 1 2 3 4 5 6 7 8 9 10 11 (11 rows) regards, ---- NTT Software Corporation Tomonari Katsumata (2013/08/13 19:16), Szymon Guz wrote: > On 13 August 2013 11:43, Tomonari Katsumata < > katsumata.tomon...@po.ntts.co.jp> wrote: > >> Hi, >> >> Could anyone tell me how to create read-only view on >> PostgreSQL 9.3 ? >> >> I've been testing updatable views and noticed that >> all simple views are updatable. >> >> When I use pg_dump for upgrading from PostgreSQL 9.2 >> to PostgreSQL 9.3 and if the databse has views, >> all views are updatable on the restored database. >> >> I want to make these views read-only like PostgreSQL9.2. >> How can I do this? Should I make access control on users ? >> (Sorry, I couldn't find any explanations on document.) >> >> regards, >> >> NTT Software Corporation >> Tomonari Katsumata >> >> >> >> Could you show an example? > > Szymon > -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] How to create read-only view on 9.3
Hi, Could anyone tell me how to create read-only view on PostgreSQL 9.3 ? I've been testing updatable views and noticed that all simple views are updatable. When I use pg_dump for upgrading from PostgreSQL 9.2 to PostgreSQL 9.3 and if the databse has views, all views are updatable on the restored database. I want to make these views read-only like PostgreSQL9.2. How can I do this? Should I make access control on users ? (Sorry, I couldn't find any explanations on document.) regards, NTT Software Corporation Tomonari Katsumata -- 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] Should we remove "not fast" promotion at all?
Hi,
I understood it's too late to change the feature.
I hope it will be revised in 9.4!
(2013/08/09 4:13), Josh Berkus wrote:
> On 08/08/2013 11:01 AM, Andres Freund wrote:
>
>> I don't think anybody working on related areas of the code thinks it's
>> rock solid.
>> But anyway, I just don't see the downside of allowing problem
>> analysis. You're free to do more testing, review, whatever before the
>> release.
>
> I'm 100% with you that we ought to keep the slow failover code around
> and accessible to debugging. What I'm asking is whether it should still
> be explicitly available to users, and the default. Based on your
> feedback, it's sounding like it should be.
>
In my opinion, the default behavior "fast promote" is ok.
Because we don't have any explicit problem with it for now.
And we shouldn't mention about "normal promote" in document.
I think it will make user confused if do so.
By the way, I'm thinking about when these trigger files(*)
are unlinked.
(*)Now I treat these three files
1) a file specified in trigger_file
2) ${PGDATA}/promote
3) ${PGDATA}/fast_promote
Current source has a problem in some corner cases.
It occurs by the timing of detecting these files.
for example:
--
case1:
createing 1) and executing "pg_ctl promote" occur at the same time.
case2:
creating 1) and promoting at recovery-end(by recovery_target_xxx)
occur at the same time.
--
1) is created, but promoting completes by another trigger.
Both cases 1) remains on the server.
If user doesn't know it and make a standby on the server,
the standby will promote soon.
I think this is not so big problem, but not user-friendly.
Against this, I'm thinking unlinking these files before
starting recovery.
This should be fixed in 9.4 too?
-
Tomonari Katsumata
--
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] Should we remove "not fast" promotion at all?
Hi,
2013/8/6 Tom Lane
> Fujii Masao writes:
> > On Tue, Aug 6, 2013 at 11:40 AM, Andres Freund
> wrote:
> >> FWIW I'd rather keep plain promotion for a release or two. TBH, I have a
> >> bit of trust issues regarding the new method, and I'd like to be able to
> >> test potential issues against a stock postgres by doing a normal instead
> >> of a fast promotion.
>
> > So we should add new option specifying the promotion mode, into pg_ctl?
> > Currently pg_ctl cannot trigger the normal promotion.
>
> It would be silly to add such an option if we want to remove the old mode
> in a release or two.
>
> I think what Andres is suggesting is to leave it as-is for 9.4 and then
> remove the old code in 9.5 or 9.6. Which seems prudent to me.
>
> How about giving trigger_file an ability to chose "fast promote" and
"normal promote"
like the triggerfile of pg_standby.
It means that if the contents of the trigger_file is empty or 'smart' then
do "normal promote",
and it's 'fast' then do "fast promote".
I think this change would be smaller than change to pg_ctl.
And this would allow us to treat ${PGDATA}/promote and trigger_file only.
(because ${PGDATA}/fast_promote is not created automatically)
regards,
---
Tomonari Katsumata
Re: [HACKERS] comment for "fast promote"
Hi,
I made a patch for REL9_3_STABLE which gets rid of
old promote processing. please check it.
This patch make PostgreSQL do fast promoting(*) always.
(*) which means skipping long checkpoint before increasing
timeline.
And after this, I'll do make another patch for unlinking files which are
created by user as a trigger_file or "pg_ctl promote" command.
---
Tomonari Katsumata
2013/7/30 Fujii Masao
> On Sat, Jul 27, 2013 at 6:57 PM, Tomonari Katsumata
> wrote:
> > Hi,
> >
> >
> >>>> Yes, it prevents PROMOTE_SIGNAL_FILE from remaining even if
> >>>> both promote files exist.
> >>>>
> >>> The command("unlink(PROMOTE_SIGNAL_FILE)") here is for
> >>> unusualy case.
> >>> Because the case is when done both procedures below.
> >>> - user create "promote" file on PGDATA
> >>> - user issue "pg_ctl promote"
> >>>
> >>> I understand the reason.
> >>> But I think it's better to unlink(PROMOTE_SIGNAL_FILE) before
> >>> unlink(FAST_PROMOTE_SIGNAL_FILE).
> >>> Because FAST_PROMOTE_SIGNAL_FILE is definetly there but
> >>> PROMOTE_SIGNAL_FILE is sometimes there or not there.
> >>
> >> I could not understand why that's better. Could you elaborate that?
> >>
> > I'm sorry for less explanation.
> >
> > I've thought that errno would be set ENOENT and
> > this may lead something wrong.
> > I checked this and I know it's not problem.
> >
> > sorry for confusing you.
> >
> >
> >
> >>> And I have another question linking this behavior.
> >>> I think TriggerFile should be removed too.
> >>> This is corner-case but it will happen.
> >>> How do you think of it ?
> >>
> >> I don't have strong opinion about that. I've never heard the complaint
> >> about that current behavior so far.
> >>
> > For example, please imagine the cascading replication environment and
> > using old master as a standby without copying the timeline history file
> > to new standby.
> >
> > ---
> > 1. replicating 3 servers(A,B,C)
> > A->B->C
> > ("trigger_file = /tmp/trig" is set in recovery_recovery.conf on B and C.)
> >
> > 2. stop server A and promoting server B with "touch /tmp/trig;pg_ctl
> > promote"
>
> Why do you need to both create the trigger file and run pg_ctl promote?
>
> Anyway, if the patch is useful for fail-safe and it doesn't break the
> current
> behavior, I'd be happy to apply it. You are suggesting that we should
> remove
> the trigger file in CheckForStandbyTrigger() even if pg_ctl promote is
> executed.
> But there can be some cases where we can get out of the WAL replay loop,
> for example, reach the recovery_target_xxx. So ISTM we should try to remove
> both the trigger file and "promote" file at the end of recovery
> instead. Thought?
>
> > B->C
> > (/tmp/trig file remains on server B)
> >
> > 4. stop server B and promoting server C with "pg_ctl promote"
> > C
> >
> > 5. making server B connect for standby of server C
> > C->B
> > -
> >
> > In step5 server B will promote as soon as it starts,
> > because "/tmp/trig" is stil there.
> >
> >
> >
> >>>> One question is that: we really still need to support normal promote?
> >>>> pg_ctl promote provides only way to do fast promotion. If we want to
> >>>> do normal promotion, we need to create PROMOTE_SIGNAL_FILE
> >>>> and send the SIGUSR1 signal to postmaster by hand. This seems messy.
> >>>>
> >>>> I think that we should remove normal promotion at all, or change
> >>>> pg_ctl promote so that provides also the way to do normal promotion.
> >>>>
> >>> I think he merit of "fast promote" is
> >>> - allowing quick connection by skipping checkpoint
> >>> and its demerit is
> >>> - taking little bit longer when crash-recovery
> >>>
> >>> If it is seldom to happen its crash soon after promoting
> >>> and "fast promte" never breaks consistency of database cluster,
> >>> I think we don't need normal promotion.
> >>
> >> You can execute checkpoint after fast promotion for that.
> >>
> > OK.
> > Then I think we should do below things.
> > - removing normal promotion at all from source
> > - adding the know-how you suggest on document
>
> IMO either is necessary.
>
> Regards,
>
> --
> Fujii Masao
>
getting_rid_of_old_promote.patch
Description: Binary data
--
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] comment for "fast promote"
Hi,
>>> Yes, it prevents PROMOTE_SIGNAL_FILE from remaining even if
>>> both promote files exist.
>>>
>> The command("unlink(PROMOTE_SIGNAL_FILE)") here is for
>> unusualy case.
>> Because the case is when done both procedures below.
>> - user create "promote" file on PGDATA
>> - user issue "pg_ctl promote"
>>
>> I understand the reason.
>> But I think it's better to unlink(PROMOTE_SIGNAL_FILE) before
>> unlink(FAST_PROMOTE_SIGNAL_FILE).
>> Because FAST_PROMOTE_SIGNAL_FILE is definetly there but
>> PROMOTE_SIGNAL_FILE is sometimes there or not there.
>
> I could not understand why that's better. Could you elaborate that?
>
I'm sorry for less explanation.
I've thought that errno would be set ENOENT and
this may lead something wrong.
I checked this and I know it's not problem.
sorry for confusing you.
>> And I have another question linking this behavior.
>> I think TriggerFile should be removed too.
>> This is corner-case but it will happen.
>> How do you think of it ?
>
> I don't have strong opinion about that. I've never heard the complaint
> about that current behavior so far.
>
For example, please imagine the cascading replication environment and
using old master as a standby without copying the timeline history file
to new standby.
---
1. replicating 3 servers(A,B,C)
A->B->C
("trigger_file = /tmp/trig" is set in recovery_recovery.conf on B and C.)
2. stop server A and promoting server B with "touch /tmp/trig;pg_ctl
promote"
B->C
(/tmp/trig file remains on server B)
4. stop server B and promoting server C with "pg_ctl promote"
C
5. making server B connect for standby of server C
C->B
-
In step5 server B will promote as soon as it starts,
because "/tmp/trig" is stil there.
>>> One question is that: we really still need to support normal promote?
>>> pg_ctl promote provides only way to do fast promotion. If we want to
>>> do normal promotion, we need to create PROMOTE_SIGNAL_FILE
>>> and send the SIGUSR1 signal to postmaster by hand. This seems messy.
>>>
>>> I think that we should remove normal promotion at all, or change
>>> pg_ctl promote so that provides also the way to do normal promotion.
>>>
>> I think he merit of "fast promote" is
>> - allowing quick connection by skipping checkpoint
>> and its demerit is
>> - taking little bit longer when crash-recovery
>>
>> If it is seldom to happen its crash soon after promoting
>> and "fast promte" never breaks consistency of database cluster,
>> I think we don't need normal promotion.
>
> You can execute checkpoint after fast promotion for that.
>
OK.
Then I think we should do below things.
- removing normal promotion at all from source
- adding the know-how you suggest on document
Are there any objection?
regards,
---
Tomonari Katsumata
Re: [HACKERS] comment for "fast promote"
Hi Fujii-san,
Thank you for response.
(2013/07/25 21:15), Fujii Masao wrote:
> On Thu, Jul 25, 2013 at 5:33 PM, Tomonari Katsumata
> wrote:
>> Hi,
>>
>> Now I'm seeing xlog.c in 93_stable for studying "fast promote",
>> and I have a question.
>>
>> I think it has an extra unlink command for "promote" file.
>> (on 9937 line)
>> ---
>> 9934 if (stat(FAST_PROMOTE_SIGNAL_FILE, &stat_buf) == 0)
>> 9935 {
>> 9936 unlink(FAST_PROMOTE_SIGNAL_FILE);
>> 9937 unlink(PROMOTE_SIGNAL_FILE);
>> 9938 fast_promote = true;
>> 9939 }
>> ---
>>
>> Is this command necesary ?
>
> Yes, it prevents PROMOTE_SIGNAL_FILE from remaining even if
> both promote files exist.
>
The command("unlink(PROMOTE_SIGNAL_FILE)") here is for
unusualy case.
Because the case is when done both procedures below.
- user create "promote" file on PGDATA
- user issue "pg_ctl promote"
I understand the reason.
But I think it's better to unlink(PROMOTE_SIGNAL_FILE) before
unlink(FAST_PROMOTE_SIGNAL_FILE).
Because FAST_PROMOTE_SIGNAL_FILE is definetly there but
PROMOTE_SIGNAL_FILE is sometimes there or not there.
And I have another question linking this behavior.
I think TriggerFile should be removed too.
This is corner-case but it will happen.
How do you think of it ?
> One question is that: we really still need to support normal promote?
> pg_ctl promote provides only way to do fast promotion. If we want to
> do normal promotion, we need to create PROMOTE_SIGNAL_FILE
> and send the SIGUSR1 signal to postmaster by hand. This seems messy.
>
> I think that we should remove normal promotion at all, or change
> pg_ctl promote so that provides also the way to do normal promotion.
>
I think he merit of "fast promote" is
- allowing quick connection by skipping checkpoint
and its demerit is
- taking little bit longer when crash-recovery
If it is seldom to happen its crash soon after promoting
and "fast promte" never breaks consistency of database cluster,
I think we don't need normal promotion.
(of course we need to put detail about promotion on document.)
regards,
NTT Software Corporation
Tomonari Katsumata
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] comment for "fast promote"
Hi,
Now I'm seeing xlog.c in 93_stable for studying "fast promote",
and I have a question.
I think it has an extra unlink command for "promote" file.
(on 9937 line)
---
9934 if (stat(FAST_PROMOTE_SIGNAL_FILE, &stat_buf) == 0)
9935 {
9936 unlink(FAST_PROMOTE_SIGNAL_FILE);
9937 unlink(PROMOTE_SIGNAL_FILE);
9938 fast_promote = true;
9939 }
---
Is this command necesary ?
regards,
------
NTT Software Corporation
Tomonari Katsumata
--
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] dividing privileges for replication role.
Hi, Michael 2013/1/23 Michael Paquier > > > On Wed, Jan 23, 2013 at 5:46 PM, Tomonari Katsumata < > t.katsumata1...@gmail.com> wrote: > >> ex: >> >> primary_conninfo = 'port=5432 standby_mode=master-cascade' >> primary_conninfo = 'port=5432 standby_mode=master-only' >> primary_conninfo = 'port=5432 standby_mode=cascade-only' >> >> I think it will be able to do same control with privilege controlling. >> And it won't be contrary to the policy(no data is stored in system >> catalogs). >> >> Because it is corner-case, I should not do anything about this? >> (Am I concerning too much?) >> > Just curious, but... What is your primary goal with this patch? > Solving the cyclic problem? > > No, I'm not thinking about solving the cyclic problem directly. It is too difficult for me. And the goal of my patch is adding some selections to avoid it for users. NTT Software Corporation Tomonari Katsumata
Re: [HACKERS] dividing privileges for replication role.
Hi, Tom Thank you for comments. > Tomonari Katsumata writes: > >> Why is it better to do this with a privilege, rather than just using > >> pg_hba.conf? > > > > You are right. > > Handling with pg_hba.conf is an easy way. > > > But I think many users think about switch over, so > > the pg_hba.conf is same on master and standby. > > it's not convinient that we have to rewrite pg_hba.conf > > whenever switch over occurs. > > > In the other hand, using a privilege, although we have to prepare > > each roles before, we don't need to rewrite pg_hba.conf. > > > That sounds good, but if the behavior is controlled by a privilege > (ie, it's stored in system catalogs) then it's impossible to have > different settings on different slave servers --- or indeed to change > the settings locally on a slave at all. You can only change settings > on the master and let the change replicate to all the slaves. > Yes, I'm thinking changing settings on the master and the settings are propagating to all slaves. > Quite aside from whether you want to manage things like that, what happens if > your master has crashed and you find you need to change the settings on > the way to getting a slave to take over? > > The crash-recovery worry is one of the main reasons that things like > pg_hba.conf aren't stored in system catalogs already. It's not always > convenient to need a running server before you can change the settings. > I understand that the approach in my patch(using pribileges for controlling its behavior) does not match the policy. But I'm still thinking I should put a something to controle the behavior. Then, how about to add a new option "standby_mode" to Database Connection Control Functions like application_name. ex: primary_conninfo = 'port=5432 standby_mode=master-cascade' primary_conninfo = 'port=5432 standby_mode=master-only' primary_conninfo = 'port=5432 standby_mode=cascade-only' I think it will be able to do same control with privilege controlling. And it won't be contrary to the policy(no data is stored in system catalogs). Because it is corner-case, I should not do anything about this? (Am I concerning too much?) regards, NTT Software Corporation Tomonari Katsumata
Re: [HACKERS] dividing privileges for replication role.
Hi, Magnus, Josh, Michael, Craig
Thank you for comments and registring to CommitFest.
>> I made a patch to divide privileges for replication role.
>>
>> Currently(9.2), the privilege for replication role is
>> true / false which means that standby server is able to
>> connect to another server or not with the replication role.
>
>Why is it better to do this with a privilege, rather than just using
>pg_hba.conf? It doesn't represent an actual "permission level" after
>all - it's just an administrative "flag" to say you can't connect.
>Which AFAICS can just as easily be handled in pg_hba.conf? I guess I'm
>missing something?
>
You are right.
Handling with pg_hba.conf is an easy way.
But I think many users think about switch over, so
the pg_hba.conf is same on master and standby.
it's not convinient that we have to rewrite pg_hba.conf
whenever switch over occurs.
In the other hand, using a privilege, although we have to prepare
each roles before, we don't need to rewrite pg_hba.conf.
So I think it's better with a privilege than using only pg_hba.conf
>> In this patch, I made below.
>> a) adding new privileges for replication:"MASTER REPLICATION" and
"CASCADE
>> REPLICATION"
>>"MASTER REPLICATION": Replication-connection to master server is only
>> allowed
>>"CASCADE REPLICATION": Replication-connection to cascade server is
only
>> allowed
>>("REPLICATION" already implemented means replication-connection to
both
>> servers is allowed)
>
>This seems to me a case of making things more complicated for everyone
>in order to satisfy a very narrow use-case. It certainly doesn't seem
>to me to do anything about the "accidental cycle" issue. Am I missing
>something?
>
I agreed that it is a very narrow use-case and accidental thing.
But I think we should provide a kind of method to avoid it,
because it has been different of before release.
And I don't think it's complicated, because "REPLICATION" and
"NOREPLICATION" do same behavior with before release.
>> a) adding new privileges for replication:"MASTER REPLICATION" and
"CASCADE
>> REPLICATION"
>>
>>"MASTER REPLICATION": Replication-connection to master server is only
>> allowed
>>"CASCADE REPLICATION": Replication-connection to cascade server is
only
>> allowed
>>("REPLICATION" already implemented means replication-connection to
both
>> servers is allowed)
>>
>This does not really solve the case you reported because, as reported in
>your bug, you could still have each slave connecting to each other using
>the privilege CASCADE REPLICATION. It makes even the privilege level more
>complicated.
>
Yes, the patch can not solve the case at all.
I just added a parameter for users.
It is responsibility of users to connect with a right role.
>What would be necessary to solve your problem would be to have each standby
>being aware that it is connected to a unique master. This is not really an
>issue with privileges but more of something like having a standby scanning
>its upper cluster node tree and check if there is a master connected. While
>checking the cluster node tree, you will also need to be aware if a node
>has already been found when you scanned it to be sure that the same node
>has not been scanned, what would mean that you are in a cycle.
>
I think this is very complicated.
At least, now I can't solve it...
If someday we can detect it, this kind of switch will be needed.
Because some users may need the cyclic situation.
I'm not insisting to use replication-role, but
I want something to control this behavior.
regards,
NTT Software Corporation
Tomonari Katsumata
[HACKERS] dividing privileges for replication role.
Hi,
I made a patch to divide privileges for replication role.
Currently(9.2), the privilege for replication role is
true / false which means that standby server is able to
connect to another server or not with the replication role.
This management and cascading replication make a strange behavior.
Because cascading replication is able to connect to another standby server,
we can see the cyclic situation.
This behavior has been discussed on Hackers-list(1),
but the conclusion was that's difficult to detect the situation.
(1) http://www.postgresql.org/message-id/50d12e8f.8000...@agliodbs.com
And then, I've reported a Bug-list(2) about this.
In this discussion, an idea that controlling
replication-connection with GUC parameter or privileges on
replication role comes up.
I think these can not avoid cyclic situation but will make some help for
DBA.
(2)
http://www.postgresql.org/message-id/e1ttvvj-0004b3...@wrigleys.postgresql.org
In this patch, I made below.
a) adding new privileges for replication:"MASTER REPLICATION" and "CASCADE
REPLICATION"
"MASTER REPLICATION": Replication-connection to master server is only
allowed
"CASCADE REPLICATION": Replication-connection to cascade server is only
allowed
("REPLICATION" already implemented means replication-connection to both
servers is allowed)
b) addin above options in createuser command
--master-replication
--cascade-replication
c) dumping pg_authid.rolreplication value in pg_dumpall
is changed by server version like this:
from 9.1
true -> master-replication
false -> noreplication
from 9.2
true -> replication(master & cascade)
false -> noreplication
I've not write any documents and tests for this yet,
but I want any comments whether this change is needed or not.
regards,
-
NTT Software Corporation
Tomonari Katsumata
*** a/src/backend/commands/user.c
--- b/src/backend/commands/user.c
***
*** 250,256 CreateRole(CreateRoleStmt *stmt)
if (dcanlogin)
canlogin = intVal(dcanlogin->arg) != 0;
if (disreplication)
! isreplication = intVal(disreplication->arg) != 0;
if (dconnlimit)
{
connlimit = intVal(dconnlimit->arg);
--- 250,256
if (dcanlogin)
canlogin = intVal(dcanlogin->arg) != 0;
if (disreplication)
! isreplication = intVal(disreplication->arg);
if (dconnlimit)
{
connlimit = intVal(dconnlimit->arg);
***
*** 352,358 CreateRole(CreateRoleStmt *stmt)
/* superuser gets catupdate right by default */
new_record[Anum_pg_authid_rolcatupdate - 1] = BoolGetDatum(issuper);
new_record[Anum_pg_authid_rolcanlogin - 1] = BoolGetDatum(canlogin);
! new_record[Anum_pg_authid_rolreplication - 1] =
BoolGetDatum(isreplication);
new_record[Anum_pg_authid_rolconnlimit - 1] = Int32GetDatum(connlimit);
if (password)
--- 352,358
/* superuser gets catupdate right by default */
new_record[Anum_pg_authid_rolcatupdate - 1] = BoolGetDatum(issuper);
new_record[Anum_pg_authid_rolcanlogin - 1] = BoolGetDatum(canlogin);
! new_record[Anum_pg_authid_rolreplication - 1] =
Int32GetDatum(isreplication);
new_record[Anum_pg_authid_rolconnlimit - 1] = Int32GetDatum(connlimit);
if (password)
***
*** 732,738 AlterRole(AlterRoleStmt *stmt)
if (isreplication >= 0)
{
! new_record[Anum_pg_authid_rolreplication - 1] =
BoolGetDatum(isreplication > 0);
new_record_repl[Anum_pg_authid_rolreplication - 1] = true;
}
--- 732,738
if (isreplication >= 0)
{
! new_record[Anum_pg_authid_rolreplication - 1] =
Int32GetDatum(isreplication);
new_record_repl[Anum_pg_authid_rolreplication - 1] = true;
}
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***
*** 555,561 static void processCASbits(int cas_bits, int location, const
char *constrType,
LEADING LEAKPROOF LEAST LEFT LEVEL LIKE LIMIT LISTEN LOAD LOCAL
LOCALTIME LOCALTIMESTAMP LOCATION LOCK_P
! MAPPING MATCH MAXVALUE MINUTE_P MINVALUE MODE MONTH_P MOVE
NAME_P NAMES NATIONAL NATURAL NCHAR NEXT NO NONE
NOT NOTHING NOTIFY NOTNULL NOWAIT NULL_P NULLIF
--- 555,561
LEADING LEAKPROOF LEAST LEFT LEVEL LIKE LIMIT LISTEN LOAD LOCAL
LOCALTIME LOCALTIMESTAMP LOCATION LOCK_P
! MAPPING MASTER MATCH MAXVALUE MINUTE_P MINVALUE MODE MONTH_P MOVE
NAME_P NAMES NATIONAL NATURAL NCHAR NEXT NO NONE
NOT NOTHING NOTIFY NOTNULL NOWAIT NULL_P NULLIF
***
*** 571,577 static void processCASbits(int cas_bits, int location, const
char *constrType,
QUOTE
