Re: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review]
On Wednesday, March 13, 2013 8:06 PM Andres Freund wrote: On 2013-03-13 18:52:48 +0530, Amit Kapila wrote: On Wednesday, March 13, 2013 6:44 PM Andres Freund wrote: On 2013-03-13 18:38:12 +0530, Amit Kapila wrote: On Wednesday, March 13, 2013 6:10 PM Andres Freund wrote: On 2013-03-12 10:46:53 +0530, Amit Kapila wrote: Do you mean to say that because some variables can only be set after restart can lead to inconsistency, or is it because of asynchronous nature of pg_reload_conf()? As long as SET PERSISTENT cannot be executed inside a transaction - or only takes effect after its end - there doesn't seem to be any problem executing ProcessConfigFile() directly. Do you mean to say we call directly ProcessConfigFile() at end of SET PERSISTENT instead Of pg_reload_conf() but in that case would it load the variables for other backends? I'd say do both. Yes, we would evaluate config potentially twice. Who cares. Messages inside non-postmaster environments are only output at DEBUG2 anyway. I could see your point, when you say do both that you want that in current session, the values will be immediately available which can make user happy. However if there is any error during function ProcessConfigFile(), it could be little inconvenient for user as the setting would have been done in file but memory processing has created problem. But thats pretty independent from this? If anything it allows for *better* reporting of problems since you could convert the log level to WARNING if ProcessConfigFile() is executed in foreground - which at least interactive sessions normally will noramlly be displayed for the user. I checked in ProcessConfigFile(), error level will be DEBUG2 for IsUnderPostmaster. So it should not be a problem. There is another observation from Greg Smith's test-7 (refer his Review mail), that if we call pg_reload_conf() after SET Persistent, it is probably causing some memory leak which leads to delay of further connections. I am checking that problem that why calling pg_reload_conf() leads to memory increase which further causes delay in connection. I think once the reason for same is clear, we can decide on how to load conf file after command execution. With Regards, Amit Kapila. -- 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] Statistics and selectivity estimation for ranges
On 01.03.2013 16:22, Alexander Korotkov wrote: On Wed, Mar 13, 2013 at 11:10 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 01.03.2013 16:22, Alexander Korotkov wrote: frac = area / (length2 - length1); you can get NaN result. I've especially adjusted the code to get more of less correct result in this case. Hmm, good point. I think I managed to fix those cases in the attached version. Is there any other corner case that I missed? Did you try test case by Jeff Davis on this thread? http://www.postgresql.org/message-id/1355167304.3896.37.camel@jdavis I try it with attached version of patch and get NaN estimate. Thanks, fixed that too. Committed with a little bit more clean up and fixes. Thank you for bearing with this long process :-). And many thanks Jeff for the review, and sorry that I forgot to credit you for that in the commit message. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Foreign table feedback
Hi all, Is there a way to ensure we don't end up with a foreign table feedback loop? For example: CREATE SERVER pgserver FOREIGN DATA WRAPPER postgres_fdw OPTIONS (dbname 'postgres'); -- note how I've forgotten to specify host or port CREATE USER MAPPING FOR PUBLIC SERVER pgserver; CREATE FOREIGN TABLE test (id int) SERVER pgserver; postgres=# SELECT * FROM test; FATAL: sorry, too many clients already ERROR: could not connect to server pgserver DETAIL: FATAL: sorry, too many clients already STATEMENT: DECLARE c1 CURSOR FOR SELECT id FROM public.test ERROR: could not connect to server pgserver DETAIL: FATAL: sorry, too many clients already CONTEXT: Remote SQL command: SELECT id FROM public.test STATEMENT: FETCH 100 FROM c1 ERROR: could not connect to server pgserver DETAIL: FATAL: sorry, too many clients already ... This of course carries on until there's 100 lines in the CONTEXT message, and all connections, which remain in use after the error. Of course the same issue happens if you try to create a foreign table to another foreign table on another server, which points back to the one you're creating. -- Thom -- 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] Foreign table feedback
Thom Brown t...@linux.com writes: Is there a way to ensure we don't end up with a foreign table feedback loop? Yeah, I've run into that too :-(. I'm not sure how big a deal it is for real-world use, but certainly the loopback server the postgres_fdw regression test sets up is rather risky. But I don't really want to disallow chains of foreign tables, so it's hard to see how to prevent it without breaking possibly-useful cases. regards, tom lane -- 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] Identity projection
I'm starting to review this patch now, and I'm having a hard time with this particular design decision: Amit Kapila amit.kap...@huawei.com writes: We cannot directly compare expressions in target list as even if expressions are equal, below node (ex. APPEND) will not do projection, and hence expr will not be evaluated. I think this is nonsense. Whatever the tlist of the lower node is, that is supposed to describe what it's going to return. That node might not be able to do projection, but that doesn't mean that something underneath it didn't. So I think this patch is missing a bet by not accepting equal() expressions. Did you have a test case showing that this wouldn't work? regards, tom lane -- 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] scanner/parser minimization
On 13.03.2013 10:50, Simon Riggs wrote: On 2 March 2013 18:47, Heikki Linnakangashlinnakan...@vmware.com wrote: Interestingly, the yy_transition array generated by flex used to be much smaller: 8.3: 22072 elements 8.4: 62623 elements master: 64535 elements The big jump between 8.3 and 8.4 was caused by introduction of the unicode escapes: U'foo' [UESCAPE 'x'] . And in particular, the error rule for the UESCAPE, which we use to avoid backtracking. I experimented with a patch that uses two extra flex states to shorten the error rules, see attached. The idea is that after lexing a unicode literal like U'foo', you enter a new state, in which you check whether an UESCAPE 'x' follows. This slashes the size of the array to 36581 elements. +1 to do this sooner rather than later I hear no objection, so committed. (after fixing some small bugs in the patch, and adding some comments) - 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] scanner/parser minimization
Heikki Linnakangas hlinnakan...@vmware.com writes: I hear no objection, so committed. (after fixing some small bugs in the patch, and adding some comments) Please keep psqlscan.l in sync with scan.l. regards, tom lane -- 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] WIP: index support for regexp search
On Wed, Jan 23, 2013 at 7:29 PM, Tom Lane t...@sss.pgh.pa.us wrote:
Heikki Linnakangas hlinnakan...@vmware.com writes:
On 23.01.2013 09:36, Alexander Korotkov wrote:
On Wed, Jan 23, 2013 at 6:08 AM, Tom Lanet...@sss.pgh.pa.us wrote:
The biggest problem is that I really don't care for the idea of
contrib/pg_trgm being this cozy with the innards of regex_t.
The only option I see now is to provide a method like export_cnfa
which
would export corresponding CNFA in fixed format.
Yeah, I think that makes sense. The transformation code in trgm_regexp.c
would probably be more readable too, if it didn't have to deal with the
regex guts representation of the CNFA. Also, once you have intermediate
representation of the original CNFA, you could do some of the
transformation work on that representation, before building the
tranformed graph containing trigrams. You could eliminate any
non-alphanumeric characters, joining states connected by arcs with
non-alphanumeric characters, for example.
It's not just the CNFA though; the other big API problem is with mapping
colors back to characters. Right now, that not only knows way too much
about a part of the regex internals we have ambitions to change soon,
but it also requires pg_wchar2mb_with_len() and lowerstr(), neither of
which should be known to the regex library IMO. So I'm not sure how we
divvy that up sanely. To be clear: I'm not going to insist that we have
to have a clean API factorization before we commit this at all. But it
worries me if we don't even know how we could get to that, because we
are going to need it eventually.
Now I have following idea about API.
Put code of stage 2 (transform the original CNFA into an automaton-like
graph) into regex engine. It would use API which describes what exactly are
we going to extract from CNFA. This API could look like this.
typedef char *Prefix;
typedef char *ArcLabel;
typedef struct
{
Prefix newPrefix;
ArcLabel label;
} ArcInfo;
typedef struct
{
Prefix (*getInitialPrefix) ();
bool (*prefixContains) (Prefix prefix1, Prefix prefix2);
Prefix * (*getPrefixes) (Prefix prefix, color c, int *n);
ArcInfo * (*getArcs) (Prefix prefix, color c, int *n);
void (*freePrefix) (Prefix prefix);
void (*freeArcLabel) (ArcLabel arcLabel);
} CFNATransformAPI;
getInitialPrefix returns initial prefix value like now this code does:
initkey.prefix.colors[0] = UNKNOWN_COLOR;
initkey.prefix.colors[1] = UNKNOWN_COLOR;
prefixContains are exactly same as function with this name.
getPrefixes and getArcs cycle step work of addKeys an addArcs.
freePrefix and freeArcLabel frees used memory of Prefix and ArcLabel
strutures.
Additionally regex engine should provide correct way to examine colormap.
int getColorCharsCount(colormap *cm, color c);
pg_wchar *getColorChars(colormap *cm, color c);
getColorCharsCount would return -1 if this color should be considered as
unexpandable.
Any thoughts?
--
With best regards,
Alexander Korotkov.
Re: [HACKERS] matview join view error
Tom Lane t...@sss.pgh.pa.us wrote: Erikjan Rijkers e...@xs4all.nl writes: With 9.3devel, I can't seem to join a matview to a view; surely that should be allowed? Fixed, thanks for the report. Thanks for the fix while I was away. Regression test for this case added. -- Kevin Grittner 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
Re: [HACKERS] matview patch readability/correctness gripe
Kevin Grittner kgri...@ymail.com wrote: Tom Lane t...@sss.pgh.pa.us wrote: diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c index a1a9808e5d94959218b415ed34c46579c478c177..896326615753f2344b466eb180080174ddeda31d 100644 *** a/src/backend/rewrite/rewriteDefine.c --- b/src/backend/rewrite/rewriteDefine.c *** DefineQueryRewrite(char *rulename, *** 356,362 */ checkRuleResultList(query-targetList, RelationGetDescr(event_relation), ! true); /* * ... there must not be another ON SELECT rule already ... --- 357,364 */ checkRuleResultList(query-targetList, RelationGetDescr(event_relation), ! event_relation-rd_rel-relkind != ! RELKIND_MATVIEW); /* * ... there must not be another ON SELECT rule already ... IMO this is either flat-out wrong, or an unmaintainable crock, or both. So this either needs to be reverted, or refactored into two arguments not one, with checkRuleResultList being updated to account honestly and readably for whatever it's supposed to be doing here. Will review in light of your comments. Yeah, it was flat-out wrong. I managed to misread the comment which describes the parameter, although in retrospect it seems clear enough. Reverted. Thanks for noticing that. -- Kevin Grittner 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
Re: [HACKERS] Identity projection
I wrote: ... So I think this patch is missing a bet by not accepting equal() expressions. I've committed this with that logic, a comment explaining exactly why this is the way to do it, and some other cosmetic improvements. regards, tom lane -- 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] Foreign table feedback
But I don't really want to disallow chains of foreign tables, so it's hard to see how to prevent it without breaking possibly-useful cases. As long as we can prevent such a loop from crashing Postgres. -- 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
Re: [HACKERS] Foreign table feedback
On 14 March 2013 20:32, Josh Berkus j...@agliodbs.com wrote: But I don't really want to disallow chains of foreign tables, so it's hard to see how to prevent it without breaking possibly-useful cases. As long as we can prevent such a loop from crashing Postgres. It won't, or at least it really shouldn't. It'll just take up all connections until that session ends. -- Thom -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: postgres_fdw vs data formatting GUCs (was Re: [HACKERS] [v9.3] writable foreign tables)
On Tue, Mar 12, 2013 at 11:51 AM, Tom Lane t...@sss.pgh.pa.us wrote: Daniel Farina dan...@heroku.com writes: Okay, I see. So inverting the thinking I wrote earlier: how about hearkening carefully to any ParameterStatus messages on the local side before entering the inner loop of dblink.c:materializeResult as to set the local GUC (and carefully dropping it back off after materializeResult) so that the the _in functions can evaluate the input in the same relevant GUC context as the remote side? Yeah, watching the remote side's datestyle and intervalstyle and matching them (for both input and output) would probably work. Alright, so I've been whacking at this and there's one interesting thing to ask about: saving and restoring the local GUCs. There are a bunch of things about GUCs besides their value that are maintained, such as their 'source', so writing a little ad-hoc save/restore is not going to do the right thing. Luckily, something much closer to the right thing is done for SET LOCAL with transactions and subtransactions, to push and pop GUC contexts: guc.h: extern int NewGUCNestLevel(void); extern void AtEOXact_GUC(bool isCommit, int nestLevel); By and large looking at the mechanics of these two functions, the latter in particular has very little to do with the transaction machinery in general. It's already being called from a bunch of places that don't, to me, seem to really indicate a intrinsic connection to transactions, e.g. do_analyze_rel, ri_triggers, and postgres_fdw. I think in those cases the justification for settings of 'isCommit' is informed by the mechanism more than the symbol name or its comments. I count about ten call sites that are like this. So, I can add one more such use of AtEOXact_GUC for the dblink fix, but would it also be appropriate to find some new names for the concepts (instead of AtEOXact_GUC and isCommit) here to more accurately express what's going on? I'll perhaps do that after finishing up the dblink fix if I receive some positive response on the matter. However, if for some reason I *shouldn't* use that machinery in dblink, I'd appreciate the guidance. -- fdr -- 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] Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
2013-03-13 21:28 keltezéssel, Boszormenyi Zoltan írta: 2013-03-13 13:45 keltezéssel, Andres Freund írta: On 2013-03-13 09:09:24 +0100, Boszormenyi Zoltan wrote: 2013-03-13 07:42 keltezéssel, Craig Ringer írta: On 03/12/2013 06:27 AM, Craig Ringer wrote: Think also about the case where someone wants to change multiple values together and having just some set and not others would be inconsistent. Yeah, that's a killer. The reload would need to be scheduled for COMMIT time, it can't be done by `SET PERSISTENT` directly. Thinking about this some more, I'm not sure this is a good idea. Right now, SET takes effect immediately. Always, without exception. Delaying SET PERSISTENT's effects until commit would make it inconsistent with SET's normal behaviour. However, *not* delaying it would make it another quirky not-transactional not-atomic command. That's OK, but if it's not going to follow transactional semantics it should not be allowed to run within a transaction, like VACUUM . Writing the changes immediately but deferring the reload until commit seems to be the worst of those two worlds. I was thinking about it a little. There is a hook that runs at the end of (sub-)transactions. It can be abused for this purpose to make SET PERSISTENT transactional. The subtransactions can also stack these settings, by forgetting settings upon ROLLBACK [ TO SAVEPOINT ] and overwriting previous settings upon COMMIT and RELEASE SAVEPOINT. All it needs is a list and maintaining intermediate pointers when entering into a new level of SAVEPOINT. The functions to register such hooks are in src/include/access/xact.h: extern void RegisterXactCallback(XactCallback callback, void *arg); extern void UnregisterXactCallback(XactCallback callback, void *arg); extern void RegisterSubXactCallback(SubXactCallback callback, void *arg); extern void UnregisterSubXactCallback(SubXactCallback callback, void *arg); (sub)xact commit/abort already calls AtEOXact_GUC(commit, nestlevel). So you wouldn't even need that. Yes, thanks. It seems we could add another value to enum GucStackState, like GUC_SET_PERSISTENT - and process those only if commit nestlevel == 1. Maybe it's not needed, only enum GucAction needs a new GUC_ACTION_PERSISTENT value since that's what has any business in push_old_value(). Adding two new members to GucStack like these are enough bool has_persistent; config_var_value persistent; and SET PERSISTENT can be treated as GUC_SET. push_old_value() can merge GUC values set in the same transaction level. It seems both were needed. The attached patch makes SET PERSISTENT transactional and uses one setting per file. It uses the currently existing parsing and validating code and because of this, the patch is half the size of v12 from Amit. Best regards, Zoltán Böszörményi Everytime you see one with commit nestlevel 1 you put them into them into the stack one level up. This seems like its somewhat in line with the way SET LOCAL is implemented? On the other hand, it would make a lot of sense to implement it as one setting per file or extending the code to allow modifying settings in bulk. The one setting per file should be easier and it would also allow extensions to drop their settings automatically into the automatic config directory. I don't know who mentioned this idea about extensions but it also came up a while back. I still think one-setting-per-file is the right way to go, yes. Greetings, Andres Freund -- -- Zoltán Böszörményi Cybertec Schönig Schönig GmbH Gröhrmühlgasse 26 A-2700 Wiener Neustadt, Austria Web: http://www.postgresql-support.de http://www.postgresql.at/ diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index ae6ee60..8713fd8 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -158,6 +158,22 @@ SET ENABLE_SEQSCAN TO OFF; require superuser permission to change via commandSET/command or commandALTER/. /para + +para + Another way to change configuration parameters persistently is by + use of xref linkend=SQL-SET + command, for example: +screen +SET PERSISTENT max_connections To 10; +/screen + This command will allow users to change values persistently + through SQL command. The new value will be effective immediately in + the session and the last persistent value for a transaction is + committed into its own configuration file. It becomes effective + cluster-wide after reloading the server configuration (acronymSIGHUP/) + or server startup. The effect of this command is similar to when + user manually changes values in filenamepostgresql.conf/filename. +/para /sect2 sect2 id=config-setting-examining diff --git a/doc/src/sgml/keywords.sgml b/doc/src/sgml/keywords.sgml index 576fd65..ed19784 100644 --- a/doc/src/sgml/keywords.sgml +++ b/doc/src/sgml/keywords.sgml @@ -3388,6 +3388,13 @@ entry/entry /row
Re: [HACKERS] pg_dump selectively ignores extension configuration tables
On 03/13/2013 04:16 PM, Dimitri Fontaine wrote:
Joe Conway m...@joeconway.com writes:
I think it should dump the user data portion, especially since that
matches what pg_dump would do if you did not specify the table or schema.
+1
If you don't have time slots to fix that by then, I will have a look at
fixing that while in beta.
Here is a patch against 9.1. If there is agreement with the approach
I'll redo for 9.2 and git head and apply.
Joe
--
Joe Conway
credativ LLC: http://www.credativ.us
Linux, PostgreSQL, and general Open Source
Training, Service, Consulting, 24x7 Support
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 964823f..4e0b45c 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -13883,10 +13883,6 @@ getExtensionMembership(ExtensionInfo extinfo[], int numExtensions)
int nconfigitems;
int nconditionitems;
- /* Tables of not-to-be-dumped extensions shouldn't be dumped */
- if (!curext-dobj.dump)
- continue;
-
if (parsePGArray(extconfig, extconfigarray, nconfigitems)
parsePGArray(extcondition, extconditionarray, nconditionitems)
nconfigitems == nconditionitems)
@@ -13896,18 +13892,51 @@ getExtensionMembership(ExtensionInfo extinfo[], int numExtensions)
for (j = 0; j nconfigitems; j++)
{
TableInfo *configtbl;
+Oid configtbloid = atooid(extconfigarray[j]);
+bool dumpobj = curext-dobj.dump;
-configtbl = findTableByOid(atooid(extconfigarray[j]));
+configtbl = findTableByOid(configtbloid);
if (configtbl configtbl-dataObj == NULL)
{
/*
- * Note: config tables are dumped without OIDs regardless
- * of the --oids setting. This is because row filtering
- * conditions aren't compatible with dumping OIDs.
+ * Tables of not-to-be-dumped extensions shouldn't be dumped
+ * unless the table or its schema is explicitly included
*/
- makeTableDataInfo(configtbl, false);
- if (strlen(extconditionarray[j]) 0)
- configtbl-dataObj-filtercond = strdup(extconditionarray[j]);
+ if (!curext-dobj.dump)
+ {
+ /* check table explicitly requested */
+ if (table_include_oids.head != NULL
+ simple_oid_list_member(table_include_oids,
+ configtbloid))
+ dumpobj = true;
+
+ /* check table's schema explicitly requested */
+ if (configtbl-dobj.namespace-dobj.dump)
+ dumpobj = true;
+ }
+
+ /* check table excluded by an exclusion switch */
+ if (table_exclude_oids.head != NULL
+ simple_oid_list_member(table_exclude_oids,
+configtbloid))
+ dumpobj = false;
+
+ /* check schema excluded by an exclusion switch */
+ if (simple_oid_list_member(schema_exclude_oids,
+ configtbl-dobj.namespace-dobj.catId.oid))
+ dumpobj = false;
+
+ if (dumpobj)
+ {
+ /*
+ * Note: config tables are dumped without OIDs regardless
+ * of the --oids setting. This is because row filtering
+ * conditions aren't compatible with dumping OIDs.
+ */
+ makeTableDataInfo(configtbl, false);
+ if (strlen(extconditionarray[j]) 0)
+ configtbl-dataObj-filtercond = strdup(extconditionarray[j]);
+ }
}
}
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Assertion failure when promoting node by deleting recovery.conf and restart node
Hi, When trying to *promote* a slave as master by removing recovery.conf and restarting node, I found an assertion failure on master branch: LOG: database system was shut down in recovery at 2013-03-15 10:22:27 JST TRAP: FailedAssertion(!(ControlFile-minRecoveryPointTLI != 1), File: xlog.c, Line: 4954) (gdb) bt #0 0x7f95af03b2c5 in raise () from /usr/lib/libc.so.6 #1 0x7f95af03c748 in abort () from /usr/lib/libc.so.6 #2 0x0086ce71 in ExceptionalCondition (conditionName=0x8f2af0 !(ControlFile-minRecoveryPointTLI != 1), errorType=0x8f0813 FailedAssertion, fileName=0x8f076b xlog.c, lineNumber=4954) at assert.c:54 #3 0x004fe499 in StartupXLOG () at xlog.c:4954 #4 0x006f9d34 in StartupProcessMain () at startup.c:224 #5 0x0050ef92 in AuxiliaryProcessMain (argc=2, argv=0x7fffa6fc3d20) at bootstrap.c:423 #6 0x006f8816 in StartChildProcess (type=StartupProcess) at postmaster.c:4956 #7 0x006f39e9 in PostmasterMain (argc=6, argv=0x1c950a0) at postmaster.c:1237 #8 0x0065d59b in main (argc=6, argv=0x1c950a0) at main.c:197 Ok, this is not the cleanest way to promote a node as it doesn't do any safety checks relation at promotion but 9.2 and previous versions allowed to do that properly. The assertion has been introduced by commit 3f0ab05 in order to record properly minRecoveryPointTLI in control file at the end of recovery in the case of a crash. However, in the case of a slave node properly shutdown in recovery which is then restarted as a master, the code path of this assertion is taken. What do you think of the patch attached? It avoids the update of recoveryTargetTLI and recoveryTargetIsLatest if the node has been shutdown while in recovery. Another possibility could be to add in the assertion some conditions based on the state of controlFile but I think it is more consistent simply not to update those fields. Regards, -- Michael 20130315_crash_tli.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] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
Kyotaro HORIGUCHI horiguchi.kyot...@lab.ntt.co.jp writes: [ format-width-20130305.patch ] Applied with some mostly-cosmetic adjustments. I also took the liberty of changing some of the error message texts to line up more closely with the expanded documentation (eg, use format specifier not conversion specifier because that's the phrase used in the docs). regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: postgres_fdw vs data formatting GUCs (was Re: [HACKERS] [v9.3] writable foreign tables)
Daniel Farina dan...@heroku.com writes: On Tue, Mar 12, 2013 at 11:51 AM, Tom Lane t...@sss.pgh.pa.us wrote: Yeah, watching the remote side's datestyle and intervalstyle and matching them (for both input and output) would probably work. Alright, so I've been whacking at this and there's one interesting thing to ask about: saving and restoring the local GUCs. There are a bunch of things about GUCs besides their value that are maintained, such as their 'source', so writing a little ad-hoc save/restore is not going to do the right thing. Right, you should use NewGUCNestLevel/AtEOXact_GUC. Look at the fixes I committed in postgres_fdw a day or two ago for an example. So, I can add one more such use of AtEOXact_GUC for the dblink fix, but would it also be appropriate to find some new names for the concepts (instead of AtEOXact_GUC and isCommit) here to more accurately express what's going on? Meh. I guess we could invent an EndGUCNestLevel that's a wrapper around AtEOXact_GUC, but I'm not that excited about it ... regards, tom lane -- 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] [v9.3] writable foreign tables
On Mon, Mar 11, 2013 at 3:06 PM, Tom Lane t...@sss.pgh.pa.us wrote: Greg Stark st...@mit.edu writes: It feels a bit like unpredictable magic to have DEFAULT mean one thing and omitted columns mean something else. Agreed. The current code behaves that way, but I think that's indisputably a bug not behavior we want to keep. I'm not entirely convinced that's a bug. Both behaviors seem useful, and there has to be some way to specify each one. But I just work here. -- 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
Re: [HACKERS] Identity projection
On Thursday, March 14, 2013 8:35 PM Tom Lane wrote: I'm starting to review this patch now, and I'm having a hard time with this particular design decision: Amit Kapila amit.kap...@huawei.com writes: We cannot directly compare expressions in target list as even if expressions are equal, below node (ex. APPEND) will not do projection, and hence expr will not be evaluated. I think this is nonsense. Whatever the tlist of the lower node is, that is supposed to describe what it's going to return. That node might not be able to do projection, but that doesn't mean that something underneath it didn't. So I think this patch is missing a bet by not accepting equal() expressions. Did you have a test case showing that this wouldn't work? I have missed the point that lower nodes would have done the expression evaluation during projection. But I think before your checkin, below comment in grouping planner might be misleading: /* * If the top-level plan node is one that cannot do expression * evaluation, we must insert a Result node to project the * desired tlist. */ Now because top-level node cannot do expression evaluation, but some lower node would have done it. Here the need seems to be only in case when the top-level plan node tlist is not desired tlist. Why do we need expression evaluation inside comment? With Regards, Amit Kapila. -- 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] Identity projection
On Friday, March 15, 2013 12:52 AM Tom Lane wrote: I wrote: ... So I think this patch is missing a bet by not accepting equal() expressions. I've committed this with that logic, a comment explaining exactly why this is the way to do it, and some other cosmetic improvements. Thank you. With Regards, Amit Kapila. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
