Re: [HACKERS] Row-security on updatable s.b. views
On 01/30/2014 01:25 PM, Craig Ringer wrote: On 01/29/2014 09:47 PM, Craig Ringer wrote: https://github.com/ringerc/postgres/compare/rls-9.4-upd-sb-views i.e. https://github.com/ringerc/postgres.git , branch rls-9.4-upd-sb-views (subject to rebasing) or the non-rebased tag rls-9.4-upd-sb-views-v2 Pushed an update to the branch. New update tagged rls-9.4-upd-sb-views-v3 . Fixes an issue with rowmarking that stems from the underlying updatable s.b. views patch. Other tests continue to fail, this isn't ready yet. Specifically: - Needs checks in AT INHERITS, AT SET ROW SECURITY, and CT INHERITS to prohibit any combination of inheritance and row-security, per: http://www.postgresql.org/message-id/52ea01c3.70...@2ndquadrant.com - row-security rule recursion detection isn't solved yet, it just overflows the stack. - COPY doesn't know anything about row-security - I'm just starting to chase some odd errors in the tests, ERROR: failed to find unique expression in subplan tlist and ERROR: could not open file base/16384/30070: No such file or directory. Their cause/origin is not yet known, but they're specific to when row-security policy is being applied. - policies based on current_user don't remember current_user when rows are pulled from refcursor returned by a security definer function. There is a chunk of work here. Anybody who wants row-security to happen for 9.4, please pick something and pitch in. (Or we could just decide that my rebased and tweaked version of KaiGai's original patch internal query structure twiddling aside, is the best way forward after all. That leaves only the last item to deal with.) -- Craig Ringer http://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] [bug fix] pg_ctl fails with config-only directory
Hi, personally I really dislike constructs like you used: #ifdef WIN32 if (check_if_admin) #endif check_root(progname); It is hard to read and may confuse editors. Can you rewrite it? The rest looks fine to me. Best regards, -- Christian Kruse http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services pgp56dqGkFa4q.pgp Description: PGP signature
Re: [HACKERS] Regression tests failing if not launched on db regression
Hi, For the documentation patch, I propose the attached to avoid future confusions. Comments? It might make sense to back-patch as well. Compiles, didn't find any typos and I think it is comprehensible. Looks fine for me. Best regards, -- Christian Kruse http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services pgpCD1puVcmJQ.pgp Description: PGP signature
Re: [HACKERS] Suspicion of a compiler bug in clang: using ternary operator in ereport()
On 2014-01-30 08:32:20 +0100, Christian Kruse wrote: Hi Tom, On 29/01/14 20:06, Tom Lane wrote: Christian Kruse christ...@2ndquadrant.com writes: Your reasoning sounds quite logical to me. Thus I did a grep -RA 3 ereport src/* | less and looked for ereport calls with errno in it. I found quite a few, attached you will find a patch addressing that issue. Committed. Great! Thanks! I found a couple of errors in your patch, but I think everything is addressed in the patch as committed. While I understand most modifications I'm a little bit confused by some parts. Have a look at for example this one: + *errstr = psprintf(_(failed to look up effective user id %ld: %s), + (long) user_id, +errno ? strerror(errno) : _(user does not exist)); Why is it safe here to use errno? It is possible that the _() function changes errno, isn't it? But the evaluation order is strictly defined here, no? First the boolean check for errno, then *either* strerror(errno), *or* the _(). Greetings, Andres Freund -- Andres Freund http://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] Suspicion of a compiler bug in clang: using ternary operator in ereport()
Hi, On 30/01/14 10:15, Andres Freund wrote: While I understand most modifications I'm a little bit confused by some parts. Have a look at for example this one: + *errstr = psprintf(_(failed to look up effective user id %ld: %s), + (long) user_id, +errno ? strerror(errno) : _(user does not exist)); Why is it safe here to use errno? It is possible that the _() function changes errno, isn't it? But the evaluation order is strictly defined here, no? First the boolean check for errno, then *either* strerror(errno), *or* the _(). Have a look at the psprintf() call: we first have a _(failed to look up effective user id %ld: %s) as an argument, then we have a (long) user_id and after that we have a ternary expression using errno. Isn't it possible that the first _() changes errno? Best regards, -- Christian Kruse http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services pgpYj53H5GFar.pgp Description: PGP signature
Re: [HACKERS] patch: option --if-exists for pg_dump
Hi Pavel, it should be fixed by http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=b152c6cd0de1827ba58756e24e18110cf902182acommit Ok. Good. Sorry I didn't update my sources. Done now. Thanks Also, I didn't quite understand these lines of comments: /* * Descriptor string (te-desc) should not be same as object * specifier for DROP STATEMENT. The DROP DEFAULT has not * IF EXISTS clause - has not sense. */ Will you please rephrase ? I can try it - . A content of te-desc is usually substring of DROP STATEMENT with one related exception - CONSTRAINT. Independent to previous sentence - ALTER TABLE ALTER COLUMN DROP DEFAULT doesn't support IF EXISTS - and therefore it should not be injected. is it ok? Fine with me. Thanks -- Jeevan B Chalke Principal Software Engineer, Product Development EnterpriseDB Corporation The Enterprise PostgreSQL Company
Re: [HACKERS] [PATCH] Filter error log statements by sqlstate
Hi Oskari, Are you planning to work on what Tom has suggested ? It make sense to me as well. What are your views on that ? Thanks -- Jeevan B Chalke Principal Software Engineer, Product Development EnterpriseDB Corporation The Enterprise PostgreSQL Company
[HACKERS] 94 ignores missing selectlist
This seems wrong: in HEAD, an empty select list does *not* generate a error warning anymore: $ psql -X psql (9.4devel_HEAD_20140130_0805_571addd) Type help for help. testdb=# create table t as select i from generate_series(1,3) as f(i); SELECT 3 testdb=# select * from t; i --- 1 2 3 (3 rows) testdb=# select from t; -- (3 rows) Surely the behaviour of v9.3, which complains healthily with: ERROR: syntax error at or near from LINE 1: select from t; is to be preferred. thanks, Erik Rijkers -- 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] 94 ignores missing selectlist
On 1/30/14, 10:44 AM, Erik Rijkers wrote: This seems wrong: in HEAD, an empty select list does *not* generate a error warning anymore: This is intentional, see 1b4f7f93b4693858cb983af3cd557f6097dab67b Regards, Marko Tiikkaja -- 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] [PATCH] Filter error log statements by sqlstate
30.01.2014 11:37, Jeevan Chalke kirjoitti: Hi Oskari, Are you planning to work on what Tom has suggested ? It make sense to me as well. What are your views on that ? Tom's suggestions make sense to me, but unfortunately I haven't had time to work on this feature recently so I don't think it'll make it to 9.4 unless I can complete it during FOSDEM. I updated https://github.com/saaros/postgres/tree/log-by-sqlstate some time ago based on Tom's first set of comments but the tree is still missing changes suggested in the last email. / Oskari -- 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] Add min and max execute statement time in pg_stat_statement
(2014/01/30 8:29), Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: I could live with just stddev. Not sure others would be so happy. FWIW, I'd vote for just stddev, on the basis that min/max appear to add more to the counter update time than stddev does; you've got this: + e-counters.total_sqtime += total_time * total_time; versus this: + if (e-counters.min_time total_time || e-counters.min_time == EXEC_TIME_INIT) + e-counters.min_time = total_time; + if (e-counters.max_time total_time) + e-counters.max_time = total_time; I think on most modern machines, a float multiply-and-add is pretty darn cheap; a branch that might or might not be taken, OTOH, is a performance bottleneck. Similarly, the shared memory footprint hit is more: two new doubles for min/max versus one for total_sqtime (assuming we're happy with the naive stddev calculation). If we felt that min/max were of similar value to stddev then this would be mere nitpicking. But since people seem to agree they're worth less, I'm thinking the cost/benefit ratio isn't there. Why do you surplus worried about cost in my patch? Were three double variables assumed a lot of memory, or having lot of calculating cost? My test result showed LWlock bottele-neck is urban legend. If you have more fair test pattern, please tell me, I'll do it if the community will do fair judge. Regards, -- Mitsumasa KONDO 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] Add min and max execute statement time in pg_stat_statement
On Wed, Jan 29, 2014 at 8:48 PM, KONDO Mitsumasa kondo.mitsum...@lab.ntt.co.jp wrote: I'd like to know the truth and the fact in your patch, rather than your argument:-) I see. [part of sample sqls file, they are executed in pgbench] SELECT 1-1/9+5*8*6+5+9-6-3-4/9%7-2%7/5-9/7+2+9/7-1%5%9/3-4/4-9/1+5+5/6/5%2+1*2*3-8/8+5-3-8-4/8+5%2*2-2-5%6+4 SELECT 1%9%7-8/5%6-1%2*2-7+9*6-2*6-9+1-2*9+6+7*8-4-2*1%3/7-1%4%9+4+7/5+4/2-3-5%8/3/7*6-1%8*6*1/7/2%5*6/2-3-9 SELECT 1%3*2/8%6%5-3%1+1/6*6*5/9-2*5+6/6*5-1/2-6%4%8/7%2*7%5%9%5/9%1%1-3-9/2*1+1*6%8/2*4/1+6*7*1+5%6+9*1-9*6 [methods] method 1: with old pgss, pg_stat_statements.max=1000(default) method 2: with old pgss with my patch, pg_stat_statements.max=1000(default) peter 1 : with new pgss(Peter's patch), pg_stat_statements.max=5000(default) peter 2 : with new pgss(Peter's patch), pg_stat_statements.max=1000 [for reference] method 5:with old pgss, pg_stat_statements.max=5000 method 6:with old pgss with my patch, pg_stat_statements.max=5000 [results] method | try1 | try2 | try3 | degrade performance ratio - method 1 | 6.546 | 6.558 | 6.638 | 0% (reference score) method 2 | 6.527 | 6.556 | 6.574 | 1% peter 1 | 5.204 | 5.203 | 5.216 |20% peter 2 | 4.241 | 4.236 | 4.262 |35% method 5 | 5.931 | 5.848 | 5.872 |11% method 6 | 5.794 | 5.792 | 5.776 |12% So, if we compare the old pg_stat_statements and old default with the new pg_stat_statements and the new default (why not? The latter still uses less shared memory than the former), by the standard of this benchmark there is a regression of about 20%. But you also note that there is an 11% regression in the old pg_stat_statements against *itself* when used with a higher pg_stat_statements.max setting of 5,000. This is completely contrary to everyone's prior understanding that increasing the size of the hash table had a very *positive* effect on performance by relieving cache pressure and thereby causing less exclusive locking for an entry_dealloc(). Do you suppose that this very surprising result might just be because this isn't a very representative benchmark? Nothing ever has the benefit of *not* having to exclusive lock. -- Peter Geoghegan -- 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] patch: option --if-exists for pg_dump
Hello
patch with updated comment
regards
Pavel
2014-01-30 Jeevan Chalke jeevan.cha...@enterprisedb.com:
Hi Pavel,
it should be fixed by
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=b152c6cd0de1827ba58756e24e18110cf902182acommit
Ok. Good.
Sorry I didn't update my sources. Done now. Thanks
Also, I didn't quite understand these lines of comments:
/*
* Descriptor string (te-desc) should not be
same as object
* specifier for DROP STATEMENT. The DROP
DEFAULT has not
* IF EXISTS clause - has not sense.
*/
Will you please rephrase ?
I can try it - .
A content of te-desc is usually substring of DROP STATEMENT with one
related exception - CONSTRAINT.
Independent to previous sentence - ALTER TABLE ALTER COLUMN DROP DEFAULT
doesn't support IF EXISTS - and therefore it should not be injected.
is it ok?
Fine with me.
Thanks
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
commit e72e3ae9003d2c8eea0e687122a2f31d21674b81
Author: Pavel Stehule pavel.steh...@gooddata.com
Date: Thu Jan 30 11:28:40 2014 +0100
fix comment
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
new file mode 100644
index 8d45f24..c39767b
*** a/doc/src/sgml/ref/pg_dump.sgml
--- b/doc/src/sgml/ref/pg_dump.sgml
*** PostgreSQL documentation
*** 650,655
--- 650,665
/varlistentry
varlistentry
+ termoption--if-exists/option/term
+ listitem
+para
+ It uses conditional commands (with literalIF EXISTS/literal
+ clause) for cleaning database schema.
+/para
+ /listitem
+ /varlistentry
+
+ varlistentry
termoption--disable-dollar-quoting//term
listitem
para
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
new file mode 100644
index 5c6a101..ba6583d
*** a/doc/src/sgml/ref/pg_dumpall.sgml
--- b/doc/src/sgml/ref/pg_dumpall.sgml
*** PostgreSQL documentation
*** 301,306
--- 301,316
/varlistentry
varlistentry
+ termoption--if-exists/option/term
+ listitem
+para
+ It uses conditional commands (with literalIF EXISTS/literal
+ clause) for cleaning database schema.
+/para
+ /listitem
+ /varlistentry
+
+ varlistentry
termoption--inserts/option/term
listitem
para
diff --git a/doc/src/sgml/ref/pg_restore.sgml b/doc/src/sgml/ref/pg_restore.sgml
new file mode 100644
index 717da42..55326d5
*** a/doc/src/sgml/ref/pg_restore.sgml
--- b/doc/src/sgml/ref/pg_restore.sgml
***
*** 490,495
--- 490,505
/varlistentry
varlistentry
+ termoption--if-exists/option/term
+ listitem
+para
+ It uses conditional commands (with literalIF EXISTS/literal
+ clause) for cleaning database schema.
+/para
+ /listitem
+ /varlistentry
+
+ varlistentry
termoption--no-data-for-failed-tables/option/term
listitem
para
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
new file mode 100644
index 6927968..83f7216
*** a/src/bin/pg_dump/pg_backup.h
--- b/src/bin/pg_dump/pg_backup.h
*** typedef struct _restoreOptions
*** 113,118
--- 113,119
char *superuser; /* Username to use as superuser */
char *use_role; /* Issue SET ROLE to this */
int dropSchema;
+ int if_exists;
const char *filename;
int dataOnly;
int schemaOnly;
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
new file mode 100644
index 7fc0288..a7061d3
*** a/src/bin/pg_dump/pg_backup_archiver.c
--- b/src/bin/pg_dump/pg_backup_archiver.c
*** RestoreArchive(Archive *AHX)
*** 413,420
/* Select owner and schema as necessary */
_becomeOwner(AH, te);
_selectOutputSchema(AH, te-namespace);
! /* Drop it */
! ahprintf(AH, %s, te-dropStmt);
}
}
--- 413,496
/* Select owner and schema as necessary */
_becomeOwner(AH, te);
_selectOutputSchema(AH, te-namespace);
!
! if (*te-dropStmt != '\0')
! {
! /* Inject IF EXISTS clause to DROP part when it is required. */
! if (ropt-if_exists)
! {
! char buffer[40];
! char *mark;
! char *dropStmt = te-dropStmt;
! PQExpBuffer ftStmt = createPQExpBuffer();
!
! /* ALTER TABLE should be enahnced to ALTER TABLE IF EXISTS */
! if (strncmp(dropStmt, ALTER TABLE, 11) == 0)
! {
! /*
! * Prepare fault tolerant statement, but
! * ensure unique IF EXISTS option.
! */
! if (strncmp(dropStmt + 11, IF EXISTS, 10) != 0)
! {
!
Re: [HACKERS] pg_upgrade: make the locale comparison more tolerating
Hi Pavel, I looked at your latest cleanup patch. Yes its looks more cleaner in term equivalent_locale equivalent_encoding separate functions. I did run the test again on latest patch and all looks good. Marking it as ready for committer. Regards, On Fri, Jan 24, 2014 at 9:49 PM, Pavel Raiskup prais...@redhat.com wrote: Rushabh, really sorry I have to re-create the patch and thanks a lot for looking at it! Looking at the patch once again, I see that there were at least two problems. Firstly, I used the equivalent_locale function also on the encoding values. Even if that should not cause bugs (as it should result in strncasecmp anyway), it was not pretty.. The second problem was assuming that the locale specifier A is not longer then locale specifier B. Comparisons like 'en_US.utf8' with 'en_US_.utf8' would result in success. Bug resulting from this mistake is not real probably but it is not nice anyway.. Rather cleaning the patch once more, attached, Pavel -- Rushabh Lathia
Re: [HACKERS] Performance Improvement by reducing WAL for Update Operation
On Thu, Jan 30, 2014 at 12:23 PM, Amit Kapila amit.kapil...@gmail.com wrote: On Wed, Jan 29, 2014 at 8:13 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: Few observations in patch (back-to-pglz-like-delta-encoding-1): 1. +#define pgrb_hash_unroll(_p, hindex) \ + hindex = hindex ^ ((_p)[0] 8) shouldn't it shift by 6 rather than by 8. 2. + if (bp - bstart = result_max) + return false; I think for nothing or lesser match case it will traverse whole tuple. Can we optimize such that if there is no match till 75%, we can bail out. Ideally, I think if we don't find any match in first 50 to 60%, we should come out. 3. pg_rbcompress.h is missing. I am still working on patch back-to-pglz-like-delta-encoding-1 to see if it works well for all cases, but thought of sharing what I have done till now to you. After basic verification of back-to-pglz-like-delta-encoding-1, I will take the data with both the patches and report the same. Apart from above, the only other thing which I found problematic is below code in find match + while (*ip == *hp thislen maxlen) + thislen++; It should be while (*ip++ == *hp++ thislen maxlen) Please confirm. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.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] ALTER TABLESPACE ... MOVE ALL TO ...
On Tue, Jan 21, 2014 at 1:33 AM, Simon Riggs si...@2ndquadrant.com wrote:
On 20 January 2014 17:00, Stephen Frost sfr...@snowman.net wrote:
* Tom Lane (t...@sss.pgh.pa.us) wrote:
What if you're a superuser and you want to move everybody's objects
(perhaps in preparation for dropping the tablespace)? I think there's
value in both the ALL and OWNED forms.
A superuser is considered to 'own' all objects and so 'ALL' and 'OWNED'
above would be the same when issued by a superuser, in the current
implementation.
Looking at DROP OWNED and REASSIGN OWNED, they operate at the more
specific level of OWNED == relowner rather than if the role is
considered an 'owner' of the object through role membership, as you are
implying above.
As such, I'll rework this to be more in-line with the existing OWNED BY
semantics of REASSIGN OWNED BY and DROP OWNED BY, which means we'd have:
ALTER TABLESPACE name MOVE [ ALL | OWNED [ BY reluser ] ]
[ TABLES | INDEXES | MATERIALIZED VIEWS ] TO name opt_nowait
eg:
ALTER TABLESPACE tblspc1 MOVE ALL TO tblspc2;
ALTER TABLESPACE tblspc1 MOVE OWNED TO tblspc2;
ALTER TABLESPACE tblspc1 MOVE OWNED BY myrole TO tblspc2;
ALTER TABLESPACE tblspc1 MOVE TABLES OWNED BY myrole TO tblspc2;
ALTER TABLESPACE tblspc1 MOVE ALL OWNED BY myrole TO tblspc2;
Sounds great, thanks.
We should add the tab-completion for ALTER TABLESPACE MOVE?
Attached does that.
Regards,
--
Fujii Masao
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
***
*** 1622,1633 psql_completion(char *text, int start, int end)
COMPLETE_WITH_CONST(IDENTITY);
}
! /* ALTER TABLESPACE foo with RENAME TO, OWNER TO, SET, RESET */
else if (pg_strcasecmp(prev3_wd, ALTER) == 0
pg_strcasecmp(prev2_wd, TABLESPACE) == 0)
{
static const char *const list_ALTERTSPC[] =
! {RENAME TO, OWNER TO, SET, RESET, NULL};
COMPLETE_WITH_LIST(list_ALTERTSPC);
}
--- 1622,1633
COMPLETE_WITH_CONST(IDENTITY);
}
! /* ALTER TABLESPACE foo with RENAME TO, OWNER TO, SET, RESET, MOVE */
else if (pg_strcasecmp(prev3_wd, ALTER) == 0
pg_strcasecmp(prev2_wd, TABLESPACE) == 0)
{
static const char *const list_ALTERTSPC[] =
! {RENAME TO, OWNER TO, SET, RESET, MOVE, NULL};
COMPLETE_WITH_LIST(list_ALTERTSPC);
}
***
*** 1649,1654 psql_completion(char *text, int start, int end)
--- 1649,1675
COMPLETE_WITH_LIST(list_TABLESPACEOPTIONS);
}
+ /* ALTER TABLESPACE foo MOVE ALL|TABLES|INDEXES|MATERIALIZED VIEWS */
+ else if (pg_strcasecmp(prev4_wd, ALTER) == 0
+ pg_strcasecmp(prev3_wd, TABLESPACE) == 0
+ pg_strcasecmp(prev_wd, MOVE) == 0)
+ {
+ static const char *const list_TABLESPACEMOVETARGETS[] =
+ {ALL, TABLES, INDEXES, MATERIALIZED VIEWS, NULL};
+
+ COMPLETE_WITH_LIST(list_TABLESPACEMOVETARGETS);
+ }
+ else if ((pg_strcasecmp(prev4_wd, TABLESPACE) == 0
+ pg_strcasecmp(prev2_wd, MOVE) == 0) ||
+ (pg_strcasecmp(prev5_wd, TABLESPACE) == 0
+ pg_strcasecmp(prev3_wd, MOVE) == 0
+ pg_strcasecmp(prev2_wd, MATERIALIZED) == 0))
+ {
+ static const char *const list_TABLESPACEMOVEOPTIONS[] =
+ {OWNED BY, TO, NULL};
+
+ COMPLETE_WITH_LIST(list_TABLESPACEMOVEOPTIONS);
+ }
/* ALTER TEXT SEARCH */
else if (pg_strcasecmp(prev3_wd, ALTER) == 0
***
*** 2559,2566 psql_completion(char *text, int start, int end)
* but we may as well tab-complete both: perhaps some users prefer one
* variant or the other.
*/
! else if (pg_strcasecmp(prev3_wd, FETCH) == 0 ||
! pg_strcasecmp(prev3_wd, MOVE) == 0)
{
static const char *const list_FROMIN[] =
{FROM, IN, NULL};
--- 2580,2588
* but we may as well tab-complete both: perhaps some users prefer one
* variant or the other.
*/
! else if ((pg_strcasecmp(prev3_wd, FETCH) == 0 ||
! pg_strcasecmp(prev3_wd, MOVE) == 0)
! pg_strcasecmp(prev_wd, TO) != 0)
{
static const char *const list_FROMIN[] =
{FROM, IN, NULL};
--
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 patch (v2) for updatable security barrier views
On 30 January 2014 05:36, Craig Ringer cr...@2ndquadrant.com wrote: On 01/29/2014 08:29 PM, Dean Rasheed wrote: On 29 January 2014 11:34, Craig Ringer cr...@2ndquadrant.com wrote: On 01/23/2014 06:06 PM, Dean Rasheed wrote: On 21 January 2014 09:18, Dean Rasheed dean.a.rash...@gmail.com wrote: Yes, please review the patch from 09-Jan (http://www.postgresql.org/message-id/CAEZATCUiKxOg=voovja2s6g-sixzzxg18totggp8zobq6qn...@mail.gmail.com). After further testing I found a bug --- it involves having a security barrier view on top of a base relation that has a rule that rewrites the query to have a different result relation, and possibly also a different command type, so that the securityQuals are no longer on the result relation, which is a code path not previously tested and the rowmark handling was wrong. That's probably a pretty obscure case in the context of security barrier views, but that code path would be used much more commonly if RLS were built on top of this. Fortunately the fix is trivial --- updated patch attached. This is the most recent patch I see, and the one I've been working on top of. Are there any known tests that this patch fails? None that I've been able to come up with. I've found an issue. I'm not sure if it can be triggered from SQL, but it affects in-code users who add their own securityQuals. expand_security_quals fails to update any rowMarks that may point to a relation being expanded. If the relation isn't the target relation, this causes a rowmark to refer to a RTE with no relid, and thus failure in the executor. Relative patch against updatable s.b. views attached (for easier reading), along with a new revision of updatable s.b. views that incorporates the patch. This isn't triggered by FOR SHARE / FOR UPDATE rowmark on a security barrier view because the flattening to securityQuals and re-expansion in the optimizer is only done for _target_ security barrier views. For target views, different rowmark handling applies, so they don't trigger it either. This is triggered by adding securityQuals to a non-target relation during row-security processing, though. Hmm, looks like this is a pre-existing bug. The first thing I tried was to reproduce it using SQL, so I used a RULE to turn a DELETE into a SELECT FOR UPDATE. This is pretty dumb, but it shows the problem: CREATE TABLE foo (a int); CREATE RULE foo_del_rule AS ON DELETE TO foo DO INSTEAD SELECT * FROM foo FOR UPDATE; DELETE FROM foo; ERROR: no relation entry for relid 1 So I think this should be fixed independently of the updatable s.b. view code. Regards, Dean -- 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] [PATCH] Support for pg_stat_archiver view
On Wed, Jan 29, 2014 at 12:35 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Wed, Jan 29, 2014 at 10:55 AM, Fujii Masao masao.fu...@gmail.com wrote: On Wed, Jan 29, 2014 at 3:07 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Anybody knows about this patch? http://www.postgresql.org/message-id/508dfec9.4020...@uptime.jp Though I'm not sure whether Nagayasu is still working on that patch, it's worth thinking to introduce that together with pg_stat_archiver. This patch uses globalStats to implement the new stat fields for walwriter, I think that we should use a different structure for clarity and to be in-line with what is done now with the archiver part. Is there clear reason why we should define separate structure for each global stats view? If we introduce something like pg_stat_walwriter later, it seems better to use only one structure, i.e., globalStats, for all global stats views. Which would simplify the code and can reduce the number of calls of fwrite()/fread(). 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] patch: option --if-exists for pg_dump
Hi Pavel,
Now patch looks good to me and I think it is in good shape to pass it on to
the committer as well.
However, I have
- Tweaked few comments
- Removed white-space errors
- Fixed typos
- Fixed indentation
Attached patch with my changes. However entire design and code logic is
untouched.
Please have a quick look and pass it on to committor if you have no issues
OR
ask me to assign it to the committor, NO issues either way.
Feel free to reject my changes if you didn't like them.
Thanks
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 8d45f24..c39767b 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -650,6 +650,16 @@ PostgreSQL documentation
/varlistentry
varlistentry
+ termoption--if-exists/option/term
+ listitem
+ para
+It uses conditional commands (with literalIF EXISTS/literal
+clause) for cleaning database schema.
+ /para
+ /listitem
+ /varlistentry
+
+ varlistentry
termoption--disable-dollar-quoting//term
listitem
para
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
index 5c6a101..ba6583d 100644
--- a/doc/src/sgml/ref/pg_dumpall.sgml
+++ b/doc/src/sgml/ref/pg_dumpall.sgml
@@ -301,6 +301,16 @@ PostgreSQL documentation
/varlistentry
varlistentry
+ termoption--if-exists/option/term
+ listitem
+ para
+It uses conditional commands (with literalIF EXISTS/literal
+clause) for cleaning database schema.
+ /para
+ /listitem
+ /varlistentry
+
+ varlistentry
termoption--inserts/option/term
listitem
para
diff --git a/doc/src/sgml/ref/pg_restore.sgml b/doc/src/sgml/ref/pg_restore.sgml
index 717da42..55326d5 100644
--- a/doc/src/sgml/ref/pg_restore.sgml
+++ b/doc/src/sgml/ref/pg_restore.sgml
@@ -490,6 +490,16 @@
/varlistentry
varlistentry
+ termoption--if-exists/option/term
+ listitem
+ para
+It uses conditional commands (with literalIF EXISTS/literal
+clause) for cleaning database schema.
+ /para
+ /listitem
+ /varlistentry
+
+ varlistentry
termoption--no-data-for-failed-tables/option/term
listitem
para
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 6927968..83f7216 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -113,6 +113,7 @@ typedef struct _restoreOptions
char *superuser; /* Username to use as superuser */
char *use_role; /* Issue SET ROLE to this */
int dropSchema;
+ int if_exists;
const char *filename;
int dataOnly;
int schemaOnly;
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 7fc0288..c08a0d3 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -413,8 +413,84 @@ RestoreArchive(Archive *AHX)
/* Select owner and schema as necessary */
_becomeOwner(AH, te);
_selectOutputSchema(AH, te-namespace);
-/* Drop it */
-ahprintf(AH, %s, te-dropStmt);
+
+if (*te-dropStmt != '\0')
+{
+ /* Inject IF EXISTS clause to DROP part when required. */
+ if (ropt-if_exists)
+ {
+ char buffer[40];
+ char *mark;
+ char *dropStmt = te-dropStmt;
+ PQExpBuffer ftStmt = createPQExpBuffer();
+
+ /*
+ * Need to inject IF EXISTS clause after ALTER TABLE
+ * part in ALTER TABLE .. DROP statement
+ */
+ if (strncmp(dropStmt, ALTER TABLE, 11) == 0)
+ {
+ /*
+ * Prepare fault tolerant statement, but ensure
+ * unique IF EXISTS option.
+ */
+ if (strncmp(dropStmt + 11, IF EXISTS, 10) != 0)
+ {
+appendPQExpBuffer(ftStmt,
+ ALTER TABLE IF EXISTS);
+dropStmt = dropStmt + 11;
+ }
+ }
+
+ /*
+ * A content of te-desc is usually substring of a DROP
+ * statement with one related exception - CONSTRAINTs.
+ *
+ * Independent to previous sentence, ALTER TABLE ..
+ * ALTER COLUMN .. DROP DEFAULT statement does not
+ * support IF EXISTS clause and therefore it should not
+ * be injected.
+ */
+ if (strcmp(te-desc, DEFAULT) != 0)
+ {
+ if (strcmp(te-desc, CONSTRAINT) == 0 ||
+strcmp(te-desc, CHECK CONSTRAINT) == 0 ||
+strcmp(te-desc, FK CONSTRAINT) == 0)
+strcpy(buffer, DROP CONSTRAINT);
+ else
+snprintf(buffer, sizeof(buffer), DROP %s,
+ te-desc);
+
+ mark = strstr(dropStmt, buffer);
+ }
+ else
+ mark = NULL;
+
+ if (mark != NULL)
+ {
+ size_t l = strlen(buffer);
+
+ /* Inject IF EXISTS clause if not alredy present */
+ if (strncmp(mark + l, IF
Re: [HACKERS] [bug fix] postgres.exe fails to start on Windows Server 2012 due to ASLR
I'm sorry for the late reply. I was unable to access email. From: Craig Ringer cr...@2ndquadrant.com On 01/24/2014 06:42 PM, MauMau wrote: The customer is using 64-bit PostgreSQL 9.1.x Which x? 9.1.6. Does this issue also occur on 9.3.2, or in 9.4 HEAD, when tested on Win2k12? I'm sure it should. The release note doesn't have any reference to this issue. Another user who reported this issue in pgsql-general experienced this with 9.2.4. In addition, Another customer reported to me that psql failed to connect to PostgreSQL 9.1.9 running on Windows Server 2012 R2, about once out of 7 times. I believe this needs immediate fix. Regards MauMau -- 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 fix] pg_ctl stop times out when it should respond quickly
From: Ronan Dunklau ronan.dunk...@dalibo.com There is no regression tests covering this bugfix, althought I don't know if it would be practical to implement them. Thanks for reviewing the patch. I'm glad to know that it seems OK. Unfortunately, the current regression test system cannot handle the testing of pg_ctl. Regards MauMau -- 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] patch: option --if-exists for pg_dump
Hello All is ok Thank you Pavel 2014-01-30 Jeevan Chalke jeevan.cha...@enterprisedb.com: Hi Pavel, Now patch looks good to me and I think it is in good shape to pass it on to the committer as well. However, I have - Tweaked few comments - Removed white-space errors - Fixed typos - Fixed indentation Attached patch with my changes. However entire design and code logic is untouched. Please have a quick look and pass it on to committor if you have no issues OR ask me to assign it to the committor, NO issues either way. Feel free to reject my changes if you didn't like them. Thanks -- Jeevan B Chalke Principal Software Engineer, Product Development EnterpriseDB Corporation The Enterprise PostgreSQL Company
Re: [HACKERS] patch: option --if-exists for pg_dump
OK. Assigned it to committer. Thanks for the hard work. On Thu, Jan 30, 2014 at 6:16 PM, Pavel Stehule pavel.steh...@gmail.comwrote: Hello All is ok Thank you Pavel -- Jeevan B Chalke Principal Software Engineer, Product Development EnterpriseDB Corporation The Enterprise PostgreSQL Company
Re: [HACKERS] patch: option --if-exists for pg_dump
2014-01-30 Jeevan Chalke jeevan.cha...@enterprisedb.com: OK. Assigned it to committer. Thanks for the hard work. Thank you for review Pavel On Thu, Jan 30, 2014 at 6:16 PM, Pavel Stehule pavel.steh...@gmail.comwrote: Hello All is ok Thank you Pavel -- Jeevan B Chalke Principal Software Engineer, Product Development EnterpriseDB Corporation The Enterprise PostgreSQL Company
Re: [HACKERS] Add CREATE support to event triggers
Hi, Alvaro Herrera alvhe...@2ndquadrant.com writes: So here's a patch implementing the ideas expressed in this thread. There are two new SQL functions: I spent some time reviewing the patch and tried to focus on a higher level review, as I saw Andres already began with the lower level stuff. The main things to keep in mind here are: - this patch enables running Event Triggers anytime a new object is created, in a way that the user code is run once the object already made it through the catalogs; - the Event Trigger code has access to the full details about every created object, so it's not tied to a command but really the fact that an object just was created in the catalogs; (it's important with serial and primary key sub-commands) - facilities are provided so that it's possible to easily build an SQL command that if executed would create the exact same object again; - the facilities around passing the created object details and building a SQL command are made in such a way that it's trivially possible to hack away the captured objects properties before producing again a new SQL command. After careful study and thinking, it appears to me that the proposed patch addresses the whole range of features we expect here, and is both flexible enough for the users and easy enough to maintain. The event being fired once the objects are available in the catalogs makes it possible for the code providing the details in the JSON format to complete the parsetree with necessary information. Current state of the patch is not ready for commit yet, independant of code details some more high-level work needs to be done: - preliminary commit It might be a good idea to separate away some pre-requisites of this patch and commit them separately: the OID publishing parts and allowing an Event Trigger to get fired after CREATE but without the extra detailed JSON formated information might be good commits already, and later add the much needed details about what did happen. - document the JSON format I vote for going with the proposed format, because it actually allows to implement both the audit and replication features we want, with the capability of hacking schema, data types, SQL representation etc; and because I couldn't think of anything better than what's proposed here ;-) - other usual documentation I don't suppose I have to expand on what I mean here… - fill-in other commands Not all commands are supported in the submitted patch. I think once we have a clear documentation on the general JSON formating and how to use it as a user, we need to include support for all CREATE commands that we have. I see nothing against extending when this work has to bo done until after the CF, as long as it's fully done before beta. After all it's only about filling in minutia at this point. - review the JSON producing code It might be possible to use more of the internal supports for JSON now that the format is freezing. - regression tests The patch will need some. The simpler solution is to add a new regression test entry and exercise all the CREATE commands in there, in a specific schema, activating an event trigger that outputs the JSON detailed information each time (the snitch() example). Best would be to have some pretty indented output of the JSON to help with reviewing diffs, and I have to wonder about JSON object inner-ordering if we're going to do that. No other ideas on this topic from me. The JSON parsing is done in event_trigger.c. This code should probably live elsewhere, but I again hesitate to put it in json.c or jsonfuncs.c, at least until some discussion about its general applicability takes place. I see that as useful enough if it can be made to work without the special fmt fields somehow, with a nice default formatting ability. In particular, being able to build some intermediate object with json_agg then call the formating/expanding function on top of that might be quite useful. That said, I don't think we have enough time to attack this problem now, I think it would be wiser to address your immediate problem separately in your patch and clean it later (next release) with sharing code and infrastructure and offering a more generally useful tool. At least we will have some feedback about the Event Trigger specific context then. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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 patch (v2) for updatable security barrier views
On 30 January 2014 11:55, Dean Rasheed dean.a.rash...@gmail.com wrote: Hmm, looks like this is a pre-existing bug. The first thing I tried was to reproduce it using SQL, so I used a RULE to turn a DELETE into a SELECT FOR UPDATE. This is pretty dumb, but it shows the problem: CREATE TABLE foo (a int); CREATE RULE foo_del_rule AS ON DELETE TO foo DO INSTEAD SELECT * FROM foo FOR UPDATE; DELETE FROM foo; ERROR: no relation entry for relid 1 So I think this should be fixed independently of the updatable s.b. view code. Looks to me there isn't much use case for turning DML into a SELECT - where would we expect the output to go to if the caller wasn't prepared to handle the result rows? IMHO we should simply prohibit such cases rather than attempt to fix the fact they don't work. We know for certain nobody relies on this behaviour because its broken already. -- Simon Riggs http://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] Prohibit row-security + inheritance in 9.4?
Craig, all, * Craig Ringer (cr...@2ndquadrant.com) wrote: I'm considering just punting on inheritance for 9.4, adding checks to prohibit inheritance from being added to a rel with row security and prohibiting any rel with inheritance from being given a row-security policy. I'm alright with punting on it for 9.4, certainly we need to get the row-security work done soon if we're going to have any chance of having this in 9.4 (and I have to admit, I suspect we're pushing the envelope here already wrt getting row-security into 9.4). Earlier discussions seemed to settle on each relation having its own row-security quals applied independently. So quals on a parent rel did not cascade down to child rels. This strikes me as a bit odd- isn't this against how we handle the GRANT system when it comes to inheiritance? That is to say- if you have access to query the parent, then you'll get rows back from the child and I believe everyone feels that makes perfect sense. That gives you a consistent view of the data in a child rel when querying via the parent vs directly, which is good. It's surprising when you query via the parent and see rows the parent's row-security qualifier doesn't permit, but that surprising behaviour is consistent with other surprising things in inheritance, like a primary key on the parent not constraining rows inserted into children. While it agrees with the PK issue, that's a data consistency concern rather than a security concern, and to contrast this with our existing security model- if you don't have access to the parent then you can't query against it; doesn't matter if you have access to the child or not. The trouble is that this isn't going to work when applying row-security rules using the security barriers support. Security quals get expanded before inheritance expansion so that they're copied to all child rels. Just what you'd expect when querying a relation that's a parent of an inheritance tree via a view. Right. It's what you'd expect to happen when querying a parent rel with row-security, too. Parent quals are applied to children. But that then gives you an inconsistent view of a rel's contents based on whether you query it via a parent or directly. ... Just how our existing GRANT system works. If you want to constrain the children in the same way as the parent, then the user can add to the row-security on the children to match the parent. If the user wants to have one view for the entire inheiritance tree then they need to only implement the row-security on the parent and not grant any access for users on the children (or, if they're paranoid, add row-security to the children which are essentially deny-all). If we force everything to behave the same between querying the parent and querying the children then we cut off various scenarios of partitioning where users are allowed to query specific children but not the parent or query the parent to get things they're not able to see directly from the children. This matches the each rel stands alone comment below, from my perspective, but seems to result in a different behavior from what you describe here. Perhaps that's because each rel standing alone, when it comes to inheiritance, looks at the parent as a simple combination of all the rows from the parent plus the children and then filtering them based on the row-security of the parent. I embarked upon that because of the concern that was expressed here about the way KaiGai's row-security patch fiddles directly with remapping attributes during planning; rebuilding row-security on top of updatable security barrier views was seen as a cleaner approach. I agree with this approach of adding row-security over top of updatable security barrier views. 1. Prohibit (in CREATE TABLE ... INHERITS, ALTER TABLE ... INHERITS, and ALTER TABLE ... SET ROW SECURITY) any parent or child rel from having row-security policy, i.e. punt it until 9.5; 2. Do another round of security qual expansion that fetches quals from pg_rowsecurity *after* inheritance expansion, giving us the each-relation-stands-alone behaviour; Again, this doesn't match what I think of as each-relation-stands-alone. 3. Accept the inconsistent view of child rel contents in exchange for the otherwise sane behaviour of applying a parent's quals to children; document that if you don't want this, don't grant users direct access to the child tables; Or write identical row-security rules on the children (or deny-all row-security to force the user to use the parent). 4. attempt to pull quals from parents when querying a child rel directly. That strikes me as borderline insane. ;) Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] INTERVAL overflow detection is terribly broken
On Mon, Jan 27, 2014 at 10:48:16PM -0500, Bruce Momjian wrote: On Mon, Jan 27, 2014 at 07:19:21PM -0500, Tom Lane wrote: Bruce Momjian br...@momjian.us writes: Oh, one odd thing about this patch. I found I needed to use INT64_MAX, but I don't see it used anywhere else in our codebase. Is this OK? Is there a better way? Most of the overflow tests in int.c and int8.c are coded to avoid relying on the MIN or MAX constants; which seemed like better style at the time. Yes, I looked at those but they seemed like overkill for interval. For a case where there was an int64 multiplied by a double, then cast back to an int64, I checked the double against INT64_MAX before casting to an int64. I'm not sure whether relying on INT64_MAX to exist is portable. The only use I found was in pgbench: #ifndef INT64_MAX #define INT64_MAX INT64CONST(0x7FFF) #endif so I have just added that to my patch, and INT64_MIN: #ifndef INT64_MIN #define INT64_MIN (-INT64CONST(0x7FFF) - 1) #endif This is only used for HAVE_INT64_TIMESTAMP. Adjusted patch applied for PG 9.4. Thanks for the report. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] Suspicion of a compiler bug in clang: using ternary operator in ereport()
Christian Kruse christ...@2ndquadrant.com writes: Have a look at the psprintf() call: we first have a _(failed to look up effective user id %ld: %s) as an argument, then we have a (long) user_id and after that we have a ternary expression using errno. Isn't it possible that the first _() changes errno? While I haven't actually read the gettext docs, I'm pretty sure that gettext() is defined to preserve errno. It's supposed to be something that you can drop into existing printf's without thinking, and if it mangled errno that would certainly not be the case. If this isn't true, we've got probably hundreds of places that would need fixing, most of them of the form printf(_(...), strerror(errno)). 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] Suspicion of a compiler bug in clang: using ternary operator in ereport()
Tom Lane wrote: Christian Kruse christ...@2ndquadrant.com writes: Have a look at the psprintf() call: we first have a _(failed to look up effective user id %ld: %s) as an argument, then we have a (long) user_id and after that we have a ternary expression using errno. Isn't it possible that the first _() changes errno? While I haven't actually read the gettext docs, I'm pretty sure that gettext() is defined to preserve errno. It's supposed to be something that you can drop into existing printf's without thinking, and if it mangled errno that would certainly not be the case. It specifically says: ERRORS errno is not modified. -- Á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] Suspicion of a compiler bug in clang: using ternary operator in ereport()
Hi, On 30/01/14 10:01, Tom Lane wrote: While I haven't actually read the gettext docs, I'm pretty sure that gettext() is defined to preserve errno. It's supposed to be something that you can drop into existing printf's without thinking, and if it mangled errno that would certainly not be the case. Thanks for your explanation. I verified reading the man page and it explicitly says: ERRORS errno is not modified. Best regards, -- Christian Kruse http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services pgpHCNemua8zx.pgp Description: PGP signature
Re: [HACKERS] jsonb and nested hstore
ok, great. This is really fabulous. So far most everything feels
natural and good.
I see something odd in terms of the jsonb use case coverage. One of
the major headaches with json deserialization presently is that
there's no easy way to easily move a complex (record- or array-
containing) json structure into a row object. For example,
create table bar(a int, b int[]);
postgres=# select jsonb_populate_record(null::bar, '{a: 1, b:
[1,2]}'::jsonb, false);
ERROR: cannot populate with a nested object unless use_json_as_text
is true
If find the use_json_as_text argument here to be pretty useless
(unlike in the json_build to_record variants where it least provides
some hope for an escape hatch) for handling this since it will just
continue to fail:
postgres=# select jsonb_populate_record(null::bar, '{a: 1, b:
[1,2]}'::jsonb, true);
ERROR: missing ] in array dimensions
OTOH, the nested hstore handles this no questions asked:
postgres=# select * from populate_record(null::bar, 'a=1,
b={1,2}'::hstore);
a | b
---+---
1 | {1,2}
So, if you need to convert a complex json to a row type, the only
effective way to do that is like this:
postgres=# select* from populate_record(null::bar, '{a: 1, b:
[1,2]}'::json::hstore);
a | b
---+---
1 | {1,2}
Not a big deal really. But it makes me wonder (now that we have the
internal capability of properly mapping to a record) why *both* the
json/jsonb populate record variants shouldn't point to what the nested
hstore behavior is when the 'as_text' flag is false. That would
demolish the error and remove the dependency on hstore in order to do
effective rowtype mapping. In an ideal world the json_build
'to_record' variants would behave similarly I think although there's
no existing hstore analog so I'm assuming it's a non-trival amount of
work.
Now, if we're agreed on that, I then also wonder if the 'as_text'
argument needs to exist at all for the populate functions except for
backwards compatibility on the json side (not jsonb). For non-complex
structures it does best effort casting anyways so the flag is moot.
Well, I could certainly look at making the populate_record{set} and
to_record{set} logic handle types that are arrays or composites inside
the record. It might not be terribly hard to do - not sure.
A quick analysis suggests that this is fixable with fairly minimal
disturbance in the jsonb case. In the json case it would probably
involve reparsing the inner json. That's probably doable, because the
routines are all reentrant, but not likely to be terribly efficient. It
will also be a deal more work.
cheers
andrew
--
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] Add min and max execute statement time in pg_stat_statement
KONDO Mitsumasa kondo.mitsum...@lab.ntt.co.jp writes: (2014/01/30 8:29), Tom Lane wrote: If we felt that min/max were of similar value to stddev then this would be mere nitpicking. But since people seem to agree they're worth less, I'm thinking the cost/benefit ratio isn't there. Why do you surplus worried about cost in my patch? Were three double variables assumed a lot of memory, or having lot of calculating cost? My test result showed LWlock bottele-neck is urban legend. If you have more fair test pattern, please tell me, I'll do it if the community will do fair judge. [ shrug... ] Standard pgbench tests are useless for measuring microscopic issues like this one; whatever incremental cost is there will be swamped by client communication and parse/plan overhead. You may have proven that the overhead isn't 10% of round-trip query time, but we knew that without testing. If I were trying to measure the costs of these changes, I'd use short statements executed inside loops in a plpgsql function. And I'd definitely do it on a machine with quite a few cores (and a running session on each one), so that spinlock contention might conceivably happen often enough to be measurable. If that still says that the runtime cost is down in the noise, we could then proceed to debate whether min/max are worth a 10% increase in the size of the shared pgss hash table. Because by my count, that's about what two more doubles would be, now that we removed the query texts from the hash table --- a table entry is currently 21 doublewords (at least on 64-bit machines) and we're debating increasing it to 22 or 24. 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] inherit support for foreign tables
Shigeru Hanada shigeru.han...@gmail.com writes: At the moment we don't use attstorage for foreign tables, so allowing SET STORAGE against foreign tables never introduce visible change except \d+ output of foreign tables. But IMO such operation should not allowed because users would be confused. So I changed ATExecSetStorage() to skip on foreign tables. I think this is totally misguided. Who's to say that some weird FDW might not pay attention to attstorage? I could imagine a file-based FDW using that to decide whether to compress columns, for instance. Admittedly, the chances of that aren't large, but it's pretty hard to argue that going out of our way to prevent it is a useful activity. 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] GIN improvements part2: fast scan
On 01/30/2014 01:53 AM, Tomas Vondra wrote: (3) A file with explain plans for 4 queries suffering ~2x slowdown, and explain plans with 9.4 master and Heikki's patches is available here: http://www.fuzzy.cz/tmp/gin/queries.txt All the queries have 6 common words, and the explain plans look just fine to me - exactly like the plans for other queries. Two things now caught my eye. First some of these queries actually have words repeated - either exactly like database database or in negated form like !anything anything. Second, while generating the queries, I use dumb frequency, where only exact matches count. I.e. write != written etc. But the actual number of hits may be much higher - for example write matches exactly just 5% documents, but using @@ it matches more than 20%. I don't know if that's the actual cause though. I tried these queries with the data set you posted here: http://www.postgresql.org/message-id/52e4141e.60...@fuzzy.cz. The reason for the slowdown is the extra consistent calls it causes. That's expected - the patch certainly does call consistent in situations where it didn't before, and if the pre-consistent checks are not able to eliminate many tuples, you lose. So, what can we do to mitigate that? Some ideas: 1. Implement the catalog changes from Alexander's patch. That ought to remove the overhead, as you only need to call the consistent function once, not both ways. OTOH, currently we only call the consistent function if there is only one unknown column. If with the catalog changes, we always call the consistent function even if there are more unknown columns, we might end up calling it even more often. 2. Cache the result of the consistent function. 3. Make the consistent function cheaper. (How? Magic?) 4. Use some kind of a heuristic, and stop doing the pre-consistent checks if they're not effective. Not sure what the heuristic would look like. The caching we could easily do. It's very simple and very effective, as long as the number of number of entries is limited. The amount of space required to cache all combinations grows exponentially, so it's only feasible for up to 10 or so entries. - 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] Add min and max execute statement time in pg_stat_statement
Peter Geoghegan p...@heroku.com writes: On Wed, Jan 29, 2014 at 8:48 PM, KONDO Mitsumasa kondo.mitsum...@lab.ntt.co.jp wrote: method | try1 | try2 | try3 | degrade performance ratio - method 1 | 6.546 | 6.558 | 6.638 | 0% (reference score) method 2 | 6.527 | 6.556 | 6.574 | 1% peter 1 | 5.204 | 5.203 | 5.216 |20% peter 2 | 4.241 | 4.236 | 4.262 |35% method 5 | 5.931 | 5.848 | 5.872 |11% method 6 | 5.794 | 5.792 | 5.776 |12% So, if we compare the old pg_stat_statements and old default with the new pg_stat_statements and the new default (why not? The latter still uses less shared memory than the former), by the standard of this benchmark there is a regression of about 20%. But you also note that there is an 11% regression in the old pg_stat_statements against *itself* when used with a higher pg_stat_statements.max setting of 5,000. This is completely contrary to everyone's prior understanding that increasing the size of the hash table had a very *positive* effect on performance by relieving cache pressure and thereby causing less exclusive locking for an entry_dealloc(). Do you suppose that this very surprising result might just be because this isn't a very representative benchmark? Nothing ever has the benefit of *not* having to exclusive lock. If I understand this test scenario properly, there are no duplicate queries, so that each iteration creates a new hashtable entry (possibly evicting an old one). And it's a single-threaded test, so that there can be no benefit from reduced locking. I don't find the results particularly surprising given that. We knew going in that the external-texts patch would slow down creation of a new hashtable entry: there's no way that writing a query text to a file isn't slower than memcpy'ing it into shared memory. The performance argument for doing it anyway is that by greatly reducing the size of hashtable entries, it becomes more feasible to size the hashtable large enough so that it's seldom necessary to evict hashtable entries. It's always going to be possible to make the patch look bad by testing cases where no savings in hashtable evictions is possible; which is exactly what this test case does. But I don't think that's relevant to real-world usage. pg_stat_statements isn't going to be useful unless there's a great deal of repeated statement execution, so what we should be worrying about is the case where a table entry exists not the case where it doesn't. At the very least, test cases where there are *no* repeats are not representative of cases people would be using pg_stat_statements with. As for the measured slowdown with larger hashtable size (but still smaller than the number of distinct queries in the test), my money is on the larger table not fitting in L3 cache or something like that. 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] Prohibit row-security + inheritance in 9.4?
On Thu, Jan 30, 2014 at 2:39 AM, Craig Ringer cr...@2ndquadrant.com wrote: Earlier discussions seemed to settle on each relation having its own row-security quals applied independently. So quals on a parent rel did not cascade down to child rels. Do you have a link to that prior discussion? I don't remember reaching consensus specifically on that point. And at any rate, if the implementation complexity mitigates strongly in the other direction, that may be a sign that we should consider revising our opinion on what the right thing to do is. The problem with just accepting that feature A doesn't work with feature B and releasing anyway is that, in many cases, nobody ever gets around to fixing it. We have some of those warts already, and I'm not keen to add more. -- 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] Add min and max execute statement time in pg_stat_statement
I wrote: If I understand this test scenario properly, there are no duplicate queries, so that each iteration creates a new hashtable entry (possibly evicting an old one). And it's a single-threaded test, so that there can be no benefit from reduced locking. After looking more closely, it's *not* single-threaded, but each pgbench thread is running through the same sequence of 1 randomly generated SQL statements. So everything depends on how nearly those clients stay in step. I bet they'd stay pretty nearly in step, though --- any process lagging behind would find all the hashtable entries already created, and thus be able to catch up relative to the ones in the lead which are being slowed by having to write out their query texts. So it seems fairly likely that this scenario is greatly stressing the case where multiple processes redundantly create the same hashtable entry. In any case, while the same table entry does get touched once-per-client over a short interval, there's no long-term reuse of table entries. So I still say this isn't at all representative of real-world use of pg_stat_statements. 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] Add min and max execute statement time in pg_stat_statement
On Thu, Jan 30, 2014 at 12:23 PM, Tom Lane t...@sss.pgh.pa.us wrote: I wrote: If I understand this test scenario properly, there are no duplicate queries, so that each iteration creates a new hashtable entry (possibly evicting an old one). And it's a single-threaded test, so that there can be no benefit from reduced locking. After looking more closely, it's *not* single-threaded, but each pgbench thread is running through the same sequence of 1 randomly generated SQL statements. So everything depends on how nearly those clients stay in step. I bet they'd stay pretty nearly in step, though --- any process lagging behind would find all the hashtable entries already created, and thus be able to catch up relative to the ones in the lead which are being slowed by having to write out their query texts. So it seems fairly likely that this scenario is greatly stressing the case where multiple processes redundantly create the same hashtable entry. In any case, while the same table entry does get touched once-per-client over a short interval, there's no long-term reuse of table entries. So I still say this isn't at all representative of real-world use of pg_stat_statements. One could test it with each pgbench thread starting at a random point in the same sequence and wrapping at the end. -- 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] Patch: show xid and xmin in pg_stat_activity and pg_stat_replication
On Wed, Jan 29, 2014 at 3:11 PM, Simon Riggs si...@2ndquadrant.com wrote: On 14 January 2014 08:38, Christian Kruse christ...@2ndquadrant.com wrote: Hi, On 13/01/14 20:06, Heikki Linnakangas wrote: On 12/17/2013 04:58 PM, Christian Kruse wrote: attached you will find a patch for showing the current transaction id (xid) and the xmin of a backend in pg_stat_activty and the xmin in pg_stat_replication. Docs. Thanks, update with updated docs is attached. Looks simple enough and useful for working out which people are holding up CONCURRENT activities. I've not been involved with this patch, so any objections to me doing final review and commit? Nope, but I think this patch is broken. It looks to me like it's conflating the process offset in the BackendStatus array with its backendId, which does not seem like a good idea even if it happens to work at present. And the way BackendIdGetProc() is used looks unsafe, too: the contents might no longer be valid by the time we read them. I suspect we should have a new accessor function that takes a backend ID and copies the xid and xmin to pointers provided by the client while holding the lock. I also note that the docs seem to need some copy-editing: + entryThe current xref linked=ddl-system-columnsxmin value./xref/entry -- 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] jsonb and nested hstore
On Thu, Jan 30, 2014 at 9:50 AM, Andrew Dunstan and...@dunslane.net wrote:
Now, if we're agreed on that, I then also wonder if the 'as_text'
argument needs to exist at all for the populate functions except for
backwards compatibility on the json side (not jsonb). For non-complex
structures it does best effort casting anyways so the flag is moot.
Well, I could certainly look at making the populate_record{set} and
to_record{set} logic handle types that are arrays or composites inside the
record. It might not be terribly hard to do - not sure.
A quick analysis suggests that this is fixable with fairly minimal
disturbance in the jsonb case. In the json case it would probably involve
reparsing the inner json. That's probably doable, because the routines are
all reentrant, but not likely to be terribly efficient. It will also be a
deal more work.
Right. Also the text json functions are already in the wild anyways
-- that's not in the scope of this patch so if they need to be fixed
that could be done later.
ISTM then the right course of action is to point jsonb 'populate'
variants at hstore implementation, not the text json one and remove
the 'as text' argument. Being able to ditch that argument is the main
reason why I think this should be handled now (not forcing hstore
dependency to handle complex json is gravy).
People handling json as text would then invoke a ::jsonb cast trading
off performance for flexibility which is perfectly fine. If you
agree, perhaps we can HINT the error in certain places that return
ERROR: cannot call json_populate_record on a nested object that the
jsonb variant can be used as a workaround.
merlin
--
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] Add min and max execute statement time in pg_stat_statement
Robert Haas robertmh...@gmail.com writes: One could test it with each pgbench thread starting at a random point in the same sequence and wrapping at the end. Well, the real point is that 1 distinct statements all occurring with exactly the same frequency isn't a realistic scenario: any hashtable size less than 1 necessarily sucks, any size = 1 is perfect. I'd think that what you want to test is a long-tailed frequency distribution (probably a 1/N type of law) where a small number of statements account for most of the hits and there are progressively fewer uses of less common statements. What would then be interesting is how does the performance change as the hashtable size is varied to cover more or less of that distribution. 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] Add min and max execute statement time in pg_stat_statement
BTW ... it occurs to me to wonder if it'd be feasible to keep the query-texts file mmap'd in each backend, thereby reducing the overhead to write a new text to about the cost of a memcpy, and eliminating the read cost in pg_stat_statements() altogether. It's most likely not worth the trouble; but if a more-realistic benchmark test shows that we actually have a performance issue there, that might be a way out without giving up the functional advantages of Peter's patch. 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] jsonb and nested hstore
On 01/30/2014 12:34 PM, Merlin Moncure wrote:
On Thu, Jan 30, 2014 at 9:50 AM, Andrew Dunstan and...@dunslane.net wrote:
Now, if we're agreed on that, I then also wonder if the 'as_text'
argument needs to exist at all for the populate functions except for
backwards compatibility on the json side (not jsonb). For non-complex
structures it does best effort casting anyways so the flag is moot.
Well, I could certainly look at making the populate_record{set} and
to_record{set} logic handle types that are arrays or composites inside the
record. It might not be terribly hard to do - not sure.
A quick analysis suggests that this is fixable with fairly minimal
disturbance in the jsonb case. In the json case it would probably involve
reparsing the inner json. That's probably doable, because the routines are
all reentrant, but not likely to be terribly efficient. It will also be a
deal more work.
Right. Also the text json functions are already in the wild anyways
-- that's not in the scope of this patch so if they need to be fixed
that could be done later.
ISTM then the right course of action is to point jsonb 'populate'
variants at hstore implementation, not the text json one and remove
the 'as text' argument. Being able to ditch that argument is the main
reason why I think this should be handled now (not forcing hstore
dependency to handle complex json is gravy).
We can't reference any hstore code in jsonb. There is no guarantee that
hstore will even be loaded.
We'd have to move that code from hstore to jsonb_support.c and then make
hstore refer to it.
cheers
andrew
--
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] jsonb and nested hstore
On 01/30/2014 06:45 PM, Andrew Dunstan wrote:
On 01/30/2014 12:34 PM, Merlin Moncure wrote:
On Thu, Jan 30, 2014 at 9:50 AM, Andrew Dunstan and...@dunslane.net
wrote:
Now, if we're agreed on that, I then also wonder if the 'as_text'
argument needs to exist at all for the populate functions except for
backwards compatibility on the json side (not jsonb). For
non-complex
structures it does best effort casting anyways so the flag is moot.
Well, I could certainly look at making the populate_record{set} and
to_record{set} logic handle types that are arrays or composites
inside the
record. It might not be terribly hard to do - not sure.
A quick analysis suggests that this is fixable with fairly minimal
disturbance in the jsonb case.
As row_to_json() works with arbitrarily complex nested types (for
example row having a field
of type array of another (table)type containing arrays of third type) it
would be really nice if
you can get the result back into that row without too much hassle.
and it should be ok to treat json as source type and require it to be
translated to jsonb
for more complex operations
In the json case it would probably involve
reparsing the inner json. That's probably doable, because the
routines are
all reentrant, but not likely to be terribly efficient. It will also
be a
deal more work.
Right. Also the text json functions are already in the wild anyways
-- that's not in the scope of this patch so if they need to be fixed
that could be done later.
ISTM then the right course of action is to point jsonb 'populate'
variants at hstore implementation, not the text json one and remove
the 'as text' argument. Being able to ditch that argument is the main
reason why I think this should be handled now (not forcing hstore
dependency to handle complex json is gravy).
We can't reference any hstore code in jsonb. There is no guarantee
that hstore will even be loaded.
We'd have to move that code from hstore to jsonb_support.c and then
make hstore refer to it.
Or just copy it and leave hstore alone - the code duplication is not
terribly huge
here and hstore might still want to develop independently.
Cheers
--
Hannu Krosing
PostgreSQL Consultant
Performance, Scalability and High Availability
2ndQuadrant Nordic OÜ
--
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] extension_control_path
Hi. Now it looks fine for me. 2014-01-28 Dimitri Fontaine dimi...@2ndquadrant.fr: Hi, Sergey Muraviov sergey.k.murav...@gmail.com writes: Now patch applies cleanly and works. :-) Cool ;-) But I have some notes: 1. There is an odd underscore character in functions find_in_extension_control_path and list_extension_control_paths: \extension_control__path\ Fixed in the new version of the patch, attached. 2. If we have several versions of one extension in different directories (which are listed in extension_control_path parameter) then we get strange output from pg_available_extensions and pg_available_extension_versions views (Information about extension, whose path is at the beginning of the list, is duplicated). And only one version of the extension can be created. Fixed. 3. It would be fine to see an extension control path in pg_available_extensions and pg_available_extension_versions views (in separate column or within of extension name). I think the on-disk location is an implementation detail and decided in the attached version not to change those system view definitions. 4. Perhaps the CREATE EXTENSION command should be improved to allow creation of the required version of the extension. So we can use different versions of extensions in different databases. Fixed in the attached. I also fixed ALTER EXTENSION UPDATE to search for udpate scripts in the same directory where the main control file is found, but I suspect this part requires more thinking. When we ALTER EXTENSION UPDATE we might now have several places where we find extname.control files, with possibly differents default_version properties. In the attached, we select the directory containing the control file where default_version matches the already installed extension version. That matches with a model where the new version of the extension changes the default_version in an auxiliary file. We might want to instead match on the default_version in the control file to match with the new version we are asked to upgrade to. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Best regards, Sergey Muraviov
Re: [HACKERS] Patch: compiling the docs under Gentoo
Christian Kruse wrote: +Since Gentoo often supports different versions of a package to be +installed you have to tell the PostgreSQL build environment where the +Docbook DTD is located: +programlisting +cd /path/to/postgresql/sources/doc +make DOCBOOKSTYLE=/usr/share/sgml/docbook/sgml-dtd-4.2 +/programlisting AFAICS this should be handled in config/docbook.m4 by adding sgml/docbook/sgml-dtd-4.2 to the list already there. Maybe a wildcard could be used to avoid the version dependency, not sure. -- Á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] jsonb and nested hstore
On 01/30/2014 01:03 PM, Hannu Krosing wrote:
On 01/30/2014 06:45 PM, Andrew Dunstan wrote:
On 01/30/2014 12:34 PM, Merlin Moncure wrote:
On Thu, Jan 30, 2014 at 9:50 AM, Andrew Dunstan and...@dunslane.net
wrote:
Now, if we're agreed on that, I then also wonder if the 'as_text'
argument needs to exist at all for the populate functions except for
backwards compatibility on the json side (not jsonb). For
non-complex
structures it does best effort casting anyways so the flag is moot.
Well, I could certainly look at making the populate_record{set} and
to_record{set} logic handle types that are arrays or composites
inside the
record. It might not be terribly hard to do - not sure.
A quick analysis suggests that this is fixable with fairly minimal
disturbance in the jsonb case.
As row_to_json() works with arbitrarily complex nested types (for
example row having a field
of type array of another (table)type containing arrays of third type) it
would be really nice if
you can get the result back into that row without too much hassle.
and it should be ok to treat json as source type and require it to be
translated to jsonb
for more complex operations
Might be possible.
In the json case it would probably involve
reparsing the inner json. That's probably doable, because the
routines are
all reentrant, but not likely to be terribly efficient. It will also
be a
deal more work.
Right. Also the text json functions are already in the wild anyways
-- that's not in the scope of this patch so if they need to be fixed
that could be done later.
ISTM then the right course of action is to point jsonb 'populate'
variants at hstore implementation, not the text json one and remove
the 'as text' argument. Being able to ditch that argument is the main
reason why I think this should be handled now (not forcing hstore
dependency to handle complex json is gravy).
We can't reference any hstore code in jsonb. There is no guarantee
that hstore will even be loaded.
We'd have to move that code from hstore to jsonb_support.c and then
make hstore refer to it.
Or just copy it and leave hstore alone - the code duplication is not
terribly huge
here and hstore might still want to develop independently.
We have gone to great deal of trouble to make jsonb and nested hstore
more or less incarnations of the same thing. The new hstore relies
heavily on the new jsonb. So what you're suggesting is the opposite of
what's been developed these last months.
cheers
andrew
--
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] pgindent behavior we could do without
On Thu, Jul 18, 2013 at 12:27:21AM -0400, Tom Lane wrote: It's always annoyed me that pgindent insists on adjusting vertical whitespace around #else and related commands. This has, for example, rendered src/include/storage/barrier.h nigh illegible: you get things like /* * lwsync orders loads with respect to each other, and similarly with stores. * But a load can be performed before a subsequent store, so sync must be used * for a full memory barrier. */ #define pg_memory_barrier() __asm__ __volatile__ (sync : : : memory) #define pg_read_barrier() __asm__ __volatile__ (lwsync : : : memory) #define pg_write_barrier() __asm__ __volatile__ (lwsync : : : memory) #elif defined(__alpha) || defined(__alpha__)/* Alpha */ which makes it look like this block of code has something to do with Alpha. By chance, I noticed today that this misbehavior comes from a discretely identifiable spot, to wit lines 289-290 in src/tools/pgindent/pgindent: # Remove blank line(s) before #else, #elif, and #endif $source =~ s!\n\n+(\#else|\#elif|\#endif)!\n$1!g; This seems pretty broken to me: why exactly is whitespace there such a bad idea? Not only that, but the next action is concerned with undoing some of the damage this rule causes: # Add blank line before #endif if it is the last line in the file $source =~ s!\n(#endif.*)\n\z!\n\n$1\n!; I assert that we should simply remove both of these bits of code, as just about every committer on the project is smarter about when to use vertical whitespace than this program is. OK, I have developed the attached patch that shows the code change of removing the test for #else/#elif/#endif. You will see that the new output has odd blank lines for cases like: #ifndef WIN32 static intcopy_file(const char *fromfile, const char *tofile, bool force); -- #else static intwin32_pghardlink(const char *src, const char *dst); -- #endif I can't think of a way to detect tight blocks like the above from cases where there are sizable blocks, like: #ifndef WIN32 static intcopy_file(const char *fromfile, const char *tofile, bool force); ... ... ... #else static intwin32_pghardlink(const char *src, const char *dst); ... ... ... #endif Ideas? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] pgindent behavior we could do without
Bruce Momjian br...@momjian.us writes: On Thu, Jul 18, 2013 at 12:27:21AM -0400, Tom Lane wrote: I assert that we should simply remove both of these bits of code, as just about every committer on the project is smarter about when to use vertical whitespace than this program is. OK, I have developed the attached patch that shows the code change of removing the test for #else/#elif/#endif. You will see that the new output has odd blank lines for cases like: #ifndef WIN32 static intcopy_file(const char *fromfile, const char *tofile, bool force); -- #else static intwin32_pghardlink(const char *src, const char *dst); -- #endif Hm. So actually, that code is trying to undo excess vertical whitespace that something earlier in pgindent added? Where is that happening? 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] CREATE EVENT TRIGGER syntax
On Fri, Aug 9, 2013 at 09:12:03AM -0400, Robert Haas wrote: On Mon, Aug 5, 2013 at 4:53 PM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Bruce Momjian br...@momjian.us writes: So do we want to keep that AND in the 9.3beta and 9.4 documentation? The grammar as in gram.y still allows the AND form, and I think we're used to maintain documentation that matches the code here. So I think it makes sense to remove both capabilities as we failed to deliver any other filter. But if we wanted to clean that, what about having the grammar check for the only one item we support rather than waiting until into CreateEventTrigger() to ereport a syntax error? I have found that it's generally better to recognize such errors in the post-parse phase rather than during parsing. When you start adding more options, that tends to quickly become the only workable option anyway. OK, so I am assuming there is no additional work to do this area. Thanks. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] jsonb and nested hstore
Andrew Dunstan and...@dunslane.net writes: On 01/30/2014 01:03 PM, Hannu Krosing wrote: On 01/30/2014 06:45 PM, Andrew Dunstan wrote: We'd have to move that code from hstore to jsonb_support.c and then make hstore refer to it. Or just copy it and leave hstore alone - the code duplication is not terribly huge here and hstore might still want to develop independently. We have gone to great deal of trouble to make jsonb and nested hstore more or less incarnations of the same thing. The new hstore relies heavily on the new jsonb. So what you're suggesting is the opposite of what's been developed these last months. If so, why would you be resistant to pushing more code out of hstore and into jsonb? 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
[HACKERS] Review: tests for client programs
Hello This patch creates a infrastructure for client side testing environment 1. Surely we want this patch, this infrastructure - lot of important functionality is not covered by tests - pg_dump, pg_basebackup, ... Without infrastructure is terrible work to create some tests. 2. This patch does exactly what was proposed 3. New functionality has zero impact on our server side or client side software (performance, quality, readability, ..) 4. I was able to use this patch cleanly and it works 5. I didn't test it on windows 6. I found only few issues: a) Configure doesn't check a required IRC::Run module b) Prepared tests fails when PostgreSQL server was up - should be checked and should to raise a valuable error message c) I am not sure if infrastructure is enough - for test pg_dump I miss a comparation of produced file with some other expected file. This objection is not blocker - someone can enhance it (when will write tests of pg_dump for example). Regards Pavel Stehule 2014-01-29 Pavel Stehule pavel.steh...@gmail.com: Hello I am looking on this patch. It is great idea, and I am sure, so we want this patch - it was requested and proposed more time. Some tips: a) possibility to test only selected tests b) possibility to verify generated file against expected file c) detection some warnings (expected/unexpected) p.s. some tests fails when other Postgres is up. It should be checked on start.and raise warning or stop testing. Regards Pavel ok 4 - pg_ctl initdb waiting for server to startLOG: could not bind IPv6 socket: Address already in use HINT: Is another postmaster already running on port 5432? If not, wait a few seconds and retry. LOG: could not bind IPv4 socket: Address already in use HINT: Is another postmaster already running on port 5432? If not, wait a few seconds and retry. WARNING: could not create listen socket for localhost FATAL: could not create any TCP/IP sockets stopped waiting pg_ctl: could not start server Examine the log output. not ok 5 - pg_ctl start -w # Failed test 'pg_ctl start -w' # at /home/pavel/src/postgresql/src/bin/pg_ctl/../../../src/test/perl/TestLib.pm line 89. waiting for server to startLOG: could not bind IPv6 socket: Address already in use HINT: Is another postmaster already running on port 5432? If not, wait a few seconds and retry. LOG: could not bind IPv4 socket: Address already in use HINT: Is another postmaster already running on port 5432? If not, wait a few seconds and retry. WARNING: could not create listen socket for localhost FATAL: could not create any TCP/IP sockets stopped waiting pg_ctl: could not start server Examine the log output. not ok 6 - second pg_ctl start succeeds # Failed test 'second pg_ctl start succeeds' # at /home/pavel/src/postgresql/src/bin/pg_ctl/../../../src/test/perl/TestLib.pm line 89. pg_ctl: PID file /home/pavel/src/postgresql/src/bin/pg_ctl/testf2KW/data/postmaster.pid does not exist Is server running? not ok 7 - pg_ctl stop -w # Failed test 'pg_ctl stop -w' # at /home/pavel/src/postgresql/src/bin/pg_ctl/../../../src/test/perl/TestLib.pm line 89. ok 8 - second pg_ctl stop fails pg_ctl: PID file /home/pavel/src/postgresql/src/bin/pg_ctl/testf2KW/data/postmaster.pid does not exist Is server running? starting server anyway pg_ctl: could not read file /home/pavel/src/postgresql/src/bin/pg_ctl/testf2KW/data/postmaster.opts not ok 9 - pg_ctl restart with server not running # Failed test 'pg_ctl restart with server not running' # at /home/pavel/src/postgresql/src/bin/pg_ctl/../../../src/test/perl/TestLib.pm line 89. pg_ctl: PID file /home/pavel/src/postgresql/src/bin/pg_ctl/testf2KW/data/postmaster.pid does not exist Is server running? starting server anyway pg_ctl: could not read file /home/pavel/src/postgresql/src/bin/pg_ctl/testf2KW/data/postmaster.opts not ok 10 - pg_ctl restart with server running # Failed test 'pg_ctl restart with server running' # at /home/pavel/src/postgresql/src/bin/pg_ctl/../../../src/test/perl/TestLib.pm line 89. pg_ctl: PID file /home/pavel/src/postgresql/src/bin/pg_ctl/testf2KW/data/postmaster.pid does not exist Is server running? Bailout called. Further testing stopped: system pg_ctl stop -D /home/pavel/src/postgresql/src/bin/pg_ctl/testf2KW/data -m fast failed: 256 Bail out! system pg_ctl stop -D /home/pavel/src/postgresql/src/bin/pg_ctl/testf2KW/data -m fast failed: 256 FAILED--Further testing stopped: system pg_ctl stop -D /home/pavel/src/postgresql/src/bin/pg_ctl/testf2KW/data -m fast failed: 256 make[1]: *** [check] Error 255 make[1]: Leaving directory `/home/pavel/src/postgresql/src/bin/pg_ctl' make: *** [check-pg_ctl-recurse] Error 2 make: Leaving directory `/home/pavel/src/postgresql/src/bin'
Re: [HACKERS] pgindent behavior we could do without
On Thu, Jan 30, 2014 at 01:46:34PM -0500, Tom Lane wrote: Bruce Momjian br...@momjian.us writes: On Thu, Jul 18, 2013 at 12:27:21AM -0400, Tom Lane wrote: I assert that we should simply remove both of these bits of code, as just about every committer on the project is smarter about when to use vertical whitespace than this program is. OK, I have developed the attached patch that shows the code change of removing the test for #else/#elif/#endif. You will see that the new output has odd blank lines for cases like: #ifndef WIN32 static intcopy_file(const char *fromfile, const char *tofile, bool force); -- #else static intwin32_pghardlink(const char *src, const char *dst); -- #endif Hm. So actually, that code is trying to undo excess vertical whitespace that something earlier in pgindent added? Where is that happening? I am afraid it is _inside_ BSD indent, and if ever looked at that code, you would not want to go in there. :-O -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] jsonb and nested hstore
On 01/30/2014 01:50 PM, Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: On 01/30/2014 01:03 PM, Hannu Krosing wrote: On 01/30/2014 06:45 PM, Andrew Dunstan wrote: We'd have to move that code from hstore to jsonb_support.c and then make hstore refer to it. Or just copy it and leave hstore alone - the code duplication is not terribly huge here and hstore might still want to develop independently. We have gone to great deal of trouble to make jsonb and nested hstore more or less incarnations of the same thing. The new hstore relies heavily on the new jsonb. So what you're suggesting is the opposite of what's been developed these last months. If so, why would you be resistant to pushing more code out of hstore and into jsonb? I'm not. Above I suggested exactly that. I was simply opposed to Hannu's suggestion that instead of making hstore refer to the adopted code we maintain two copies of code that does essentially the same thing. cheers andrew -- 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] Triggers on foreign tables
On Thu, Jan 23, 2014 at 03:17:35PM +0100, Ronan Dunklau wrote: - for after triggers, the whole queuing mechanism is bypassed for foreign tables. This is IMO acceptable, since foreign tables cannot have constraints or constraints triggers, and thus have not need for deferrable execution. This design avoids the need for storing and retrieving/identifying remote tuples until the query or transaction end. Whether an AFTER ROW trigger is deferred determines whether it runs at the end of the firing query or at the end of the firing query's transaction. In all cases, every BEFORE ROW trigger of a given query fires before any AFTER ROW trigger of the same query. SQL requires that. This proposal would give foreign table AFTER ROW triggers a novel firing time; let's not do that. I think the options going forward are either (a) design a way to queue foreign table AFTER ROW triggers such that we can get the old and/or new rows at the end of the query or (b) not support AFTER ROW triggers on foreign tables for the time being. I did not know this was mandated by the standard. The attached patch tries to solve this problem by allocating a tuplestore in the global afterTriggers structure. This tuplestore is used for the whole transaction, and uses only work_mem per transaction. Both old and new tuples are stored in this tuplestore. Some additional bookkeeping is done on the afterTriggers global structure, to keep track of the number of inserted tuples, and the current read pointer position. The tuples are identified by their order of insertion during the transaction. I think this could benefit from some support in the tuplestore API, by allowing arbitrary seek without the need to store more ReadPointers. I initially tried to keep track of them by allocating read pointers on the tuple store, but it turned out to be so expensive that I had to find another way (24bytes per stored tuple, which are not reclaimable until the end of the transaction). What do you think about this approach ? Is there something I missed which would make it not sustainable ? Seems basically reasonable. I foresee multiple advantages from having one tuplestore per query level as opposed to one for the entire transaction. You would remove the performance trap of backing up the tuplestore by rescanning. It permits reclaiming memory and disk space in AfterTriggerEndQuery() rather than at end of transaction. You could remove ate_ptr1 and ate_ptr2 from AfterTriggerEventDataFDW and just store the flags word: depending on AFTER_TRIGGER_2CTIDS, grab either the next one or the next two tuples from the tuplestore. Using work_mem per AfterTriggerBeginQuery() instead of per transaction is no problem. What do you think of that design change? If you do pursue that change, make sure the code still does the right thing when it drops queued entries during subxact abort. I do not have access to the standard specification, any advice regarding specs compliance would be welcomed. http://wiki.postgresql.org/wiki/Developer_FAQ#Where_can_I_get_a_copy_of_the_SQL_standards.3F There is still one small issue with the attached patch: modifications to the tuple performed by the foreign data wrapper (via the returned TupleTableSlot in ExecForeignUpdate and ExecForeignInsert hooks) are not visible to the AFTER trigger. This could be fixed by merging the planslot containing the resjunk columns with the returned slot before calling the trigger, but I'm not really sure how to safely perform that. Any advice ? Currently, FDWs are permitted to skip returning columns not actually referenced by any RETURNING clause. I would change that part of the API contract to require returning all columns when an AFTER ROW trigger is involved. You can't get around doing that by merging old column values, because, among other reasons, an INSERT does not have those values at all. I'm not sure this should be part of the API contract: it would make implementing a FDW more complicated than it is now. The attached patch hooks on rewriteTargetListIU to add the missing targets to the returning clause, when needed. You're effectively faking the presence of a RETURNING list so today's conforming FDWs will already do the right thing? Clever. Please don't change unrelated whitespace. Please use pgindent to fix the formatting of your new code. It's fine to introduce occasional whitespace errors, but they're unusually-plentiful here. I think its done now. One problem I have with running pgindent is that I accidentally add chunks that were modified only by pgindent. Yep; that is a pain. Note that pgindent can't fix many unrelated whitespace changes. For example, if you add or remove a blank line, pgindent won't interfere. (We would not want it to interfere, because the use of blank lines is up to the code author.) You will still need to read through your
Re: [HACKERS] Re: Patch to add support of IF NOT EXISTS to others CREATE statements
2014-01-19 Tom Lane t...@sss.pgh.pa.us: Stephen Frost sfr...@snowman.net writes: * Robert Haas (robertmh...@gmail.com) wrote: I kind of don't see the point of having IF NOT EXISTS for things that have OR REPLACE, and am generally in favor of implementing OR REPLACE rather than IF NOT EXISTS where possible. The point is usually to get the object to a known state, and OR REPLACE will generally accomplish that better than IF NOT EXISTS. However, if the object has complex structure (like a table that contains data) then replacing it is a bad plan, so IF NOT EXISTS is really the best you can do - and it's still useful, even if it does require more care. This patch is in the most recent commitfest and marked as Ready for Committer, so I started reviewing it and came across the above. I find myself mostly agreeing with the above comments from Robert, but it doesn't seem like we've really done a comprehensive review of the various commands to make a 'command' decision on each as to if it should have IF NOT EXISTS or OR REPLACE options. There's been pretty extensive theorizing about this in the past (try searching the pghackers archives for CINE and COR), and I think the rough consensus was that it's hard to do COR sensibly for objects containing persistent state (ie tables) or with separately-declarable substructure (again, mostly tables, though composite types have some of the same issues). However, if COR does make sense then CINE is an inferior alternative, because of the issue about not knowing the resulting state of the object for sure. Given this list I would absolutely reject CINE for aggregates (why in the world would we make them act differently from functions?), and likewise for casts, collations, operators, and types. I don't see any reason not to prefer COR for these object kinds. There is room for argument about the text search stuff, though, because of the fact that some of the text search object types have separately declarable substructure. The one difficulty that I do see with the 'OR REPLACE' option is when we can't simply replace an existing object due to dependencies on the existing definition of that object. Still, if that's the case, wouldn't you want an error? The main knock on COR is that there's no way for the system to completely protect itself from the possibility that you replaced the object definition with something that behaves incompatibly. For instance, if we had COR for collations and you redefined a collation, that might (or might not) break indexes whose ordering depends on that collation. However, we already bought into that type of risk when we invented COR for functions, and by and large there have been few complaints about it. The ability to substitute an improved version of a function seems to be worth the risks of substituting a broken version. regards, tom lane I agree with Tom proposal - CINE - where object holds data, COR everywhere else. But it means, so all functionality from this patch have to be rewritten :( Regards Pavel -- 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] Add min and max execute statement time in pg_stat_statement
On Thu, Jan 30, 2014 at 9:57 AM, Tom Lane t...@sss.pgh.pa.us wrote: BTW ... it occurs to me to wonder if it'd be feasible to keep the query-texts file mmap'd in each backend, thereby reducing the overhead to write a new text to about the cost of a memcpy, and eliminating the read cost in pg_stat_statements() altogether. It's most likely not worth the trouble; but if a more-realistic benchmark test shows that we actually have a performance issue there, that might be a way out without giving up the functional advantages of Peter's patch. There could be a worst case for that scheme too, plus we'd have to figure out how to make in work with windows, which in the case of mmap() is not a sunk cost AFAIK. I'm skeptical of the benefit of pursuing that. I'm totally unimpressed with the benchmark as things stand. It relies on keeping 64 clients in perfect lockstep as each executes 10,000 queries that are each unique snowflakes. Though even though they're unique snowflakes, and even though there are 10,000 of them, everyone executes the same one at exactly the same time relative to each other, in exactly the same order as quickly as possible. Even still, the headline reference score of -35% is completely misleading, because it isn't comparing like with like in terms of has table size. This benchmark incidentally recommends that we reduce the default hash table size to improve performance when the hash table is under pressure, which is ludicrous. It's completely backwards. You could also use the benchmark to demonstrate that the overhead of calling pg_stat_statements() is ridiculously high, since like creating a new query text, that only requires a shared lock too. This is an implausibly bad worst case for larger hash table sizes in pg_stat_statements generally. 5,000 entries is enough for the large majority of applications. But for those that hit that limit, in practice they're still going to find the vast majority of queries already in the table as they're executed. If they don't, they can double or triple their max setting, because the shared memory overhead is so low. No one has any additional overhead once their query is in the hash table already. In reality, actual applications could hardly be further from the perfectly uniform distribution of distinct queries presented here. -- Peter Geoghegan -- 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] Issue with PGC_BACKEND parameters
On 12/22/2013 11:30 PM, Amit Kapila wrote:
I had observed one problem with PGC_BACKEND parameters while testing patch
for ALTER SYSTEM command.
Problem statement: If I change PGC_BACKEND parameters directly in
postgresql.conf and then do pg_reload_conf() and reconnect, it will
still show the old value.
Detailed steps
1. Start server with default settings
2. Connect Client
3. show log_connections; -- it will show as off, this is correct.
4. Change log_connections in postgresql.conf to on
5. issue command select pg_reload_conf() in client (which is started in step-2)
6. Connect a new client
7. show log_connections; -- it will show as off, this is in-correct.
The problem is in step-7, it should show as on.
This problem occur only in Windows.
The reason for this problem is that in WINDOWS, when a new session is
started it will load the changed parameters in new backend by
global/config_exec_params file. The flow is in
SubPostmasterMain()-read_nondefault_variables()-set_config_option().
In below code in function set_config_option(), it will not allow to change
PGC_BACKEND variable and even in comments it has mentioned that only
postmaster will be allowed to change and the same will propagate to
subsequently started backends, but this is not TRUE for Windows.
switch (record-context)
{
..
..
case PGC_BACKEND:
if (context == PGC_SIGHUP)
{
/*
* If a PGC_BACKEND parameter is changed in
the config file,
* we want to accept the new value in the
postmaster (whence
* it will propagate to subsequently-started
backends), but
* ignore it in existing backends.
This is a tad klugy, but
* necessary because we don't re-read the
config file during
* backend start.
*/
if (IsUnderPostmaster)
return -1;
}
}
I think to fix the issue we need to pass the information whether PGC_BACKEND
parameter is allowed to change in set_config_option() function.
One way is to pass a new parameter.
Yesterday, I again thought about this issue and found that we can handle it by
checking IsInitProcessingMode() which will be True only during backend startup
which is what we need here.
Please find the attached patch to fix this issue.
I think that this issue should be fixed in PostgreSQL, because
currently PGC_BACKEND
parameters doesn't work on Windows.
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -5605,9 +5605,12 @@ set_config_option(const char *name, const char
*value,
* it will propagate to subsequently-started
backends), but
* ignore it in existing backends.This is a tad
klugy, but
* necessary because we don't re-read the config file
during
- * backend start.
+ * backend start. However for windows, we need to process
+ * config file during backend start for non-default
parameters,
+ * so we need to allow change of PGC_BACKEND during backend
+ * startup.
*/
-if (IsUnderPostmaster)
+if (IsUnderPostmaster !IsInitProcessingMode())
return -1;
}
else if (context != PGC_POSTMASTER context !=
PGC_BACKEND
I think this change looks OK. Does anyone else have any comments before
I test and apply it? I presume it's a bugfix that should be backpatched.
cheers
andrew
--
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] updated emacs configuration
On Wed, Jan 29, 2014 at 11:21 PM, Bruce Momjian br...@momjian.us wrote: On Wed, Jan 29, 2014 at 07:31:29PM -0500, Tom Lane wrote: Bruce Momjian br...@momjian.us writes: I have cleaned up entab.c so I am ready to add a new option that removes tabs from only comments. Would you like me to create that and provide a diff at a URL? It would have to be run against all back branches. If you think you can actually tell the difference reliably in entab, sure, give it a go. OK, I have modified entab.c in a private patch to only process text inside comments, and not process leading whitespace, patch attached. I basically ran 'entab -o -t4 -d' on the C files. The result are here, in context, plain, and unified format: http://momjian.us/expire/entab_comment.cdiff http://momjian.us/expire/entab_comment.pdiff http://momjian.us/expire/entab_comment.udiff and their line counts: 89741 entab_comment.cdiff 26351 entab_comment.pdiff 50503 entab_comment.udiff I compute 6627 lines as modified. What I did not do is handle _only_ cases with periods before the tabs. Should I try that? I don't have any comment on that specific point, but I took a quick look through one of these diffs and it looks like a lot of churn for no improvement. So I'm not sure what we want to do here, but I don't think it's this. -- 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] updated emacs configuration
Robert Haas escribió: On Wed, Jan 29, 2014 at 11:21 PM, Bruce Momjian br...@momjian.us wrote: I compute 6627 lines as modified. What I did not do is handle _only_ cases with periods before the tabs. Should I try that? I don't have any comment on that specific point, but I took a quick look through one of these diffs and it looks like a lot of churn for no improvement. So I'm not sure what we want to do here, but I don't think it's this. I, on the contrary, think that the cases that are preceded by a period are an improvement, and the rest are not (the opposite, actually, so better not change those). Maybe there are specific cases in which a period-tab sequence should be kept, but that seems rarer. I didn't check the entire diff obviously, so I can't comment on how good about detecting comments the new entab code is. -- Á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] Issue with PGC_BACKEND parameters
Andrew Dunstan and...@dunslane.net writes: On 12/22/2013 11:30 PM, Amit Kapila wrote: - * backend start. + * backend start. However for windows, we need to process + * config file during backend start for non-default parameters, + * so we need to allow change of PGC_BACKEND during backend + * startup. */ -if (IsUnderPostmaster) +if (IsUnderPostmaster !IsInitProcessingMode()) return -1; } I think this change looks OK. The comment is pretty awful, since this is neither Windows-specific nor a read of the config file. Perhaps more like However, in EXEC_BACKEND builds we load nondefault settings from the CONFIG_EXEC_PARAMS file during backend start. In that situation we should accept PGC_SIGHUP settings, so as to have the same value as if we'd forked from the postmaster. Also, I think that the extra test should only be made #ifdef EXEC_BACKEND, so as to minimize the risk of breaking things. Not that this isn't pretty darn fragile anyway; I think testing IsInitProcessingMode here is a very random way to detect this case. I wonder if it'd be better to pass down an explicit flag indicating that we're doing read_nondefault_variables(). If we don't do that, maybe an Assert(IsInitProcessingMode()) in read_nondefault_variables() would be a good thing. 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] Issue with PGC_BACKEND parameters
On 01/30/2014 03:12 PM, Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: On 12/22/2013 11:30 PM, Amit Kapila wrote: - * backend start. + * backend start. However for windows, we need to process + * config file during backend start for non-default parameters, + * so we need to allow change of PGC_BACKEND during backend + * startup. */ -if (IsUnderPostmaster) +if (IsUnderPostmaster !IsInitProcessingMode()) return -1; } I think this change looks OK. The comment is pretty awful, since this is neither Windows-specific nor a read of the config file. Perhaps more like However, in EXEC_BACKEND builds we load nondefault settings from the CONFIG_EXEC_PARAMS file during backend start. In that situation we should accept PGC_SIGHUP settings, so as to have the same value as if we'd forked from the postmaster. Also, I think that the extra test should only be made #ifdef EXEC_BACKEND, so as to minimize the risk of breaking things. Not that this isn't pretty darn fragile anyway; I think testing IsInitProcessingMode here is a very random way to detect this case. I wonder if it'd be better to pass down an explicit flag indicating that we're doing read_nondefault_variables(). If we don't do that, maybe an Assert(IsInitProcessingMode()) in read_nondefault_variables() would be a good thing. OK, I've added your comment to the commitfest item and marked it as Waiting on Author. cheers andrew -- 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] updated emacs configuration
On Thu, Jan 30, 2014 at 3:04 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Robert Haas escribió: On Wed, Jan 29, 2014 at 11:21 PM, Bruce Momjian br...@momjian.us wrote: I compute 6627 lines as modified. What I did not do is handle _only_ cases with periods before the tabs. Should I try that? I don't have any comment on that specific point, but I took a quick look through one of these diffs and it looks like a lot of churn for no improvement. So I'm not sure what we want to do here, but I don't think it's this. I, on the contrary, think that the cases that are preceded by a period are an improvement, and the rest are not (the opposite, actually, so better not change those). Maybe there are specific cases in which a period-tab sequence should be kept, but that seems rarer. I didn't check the entire diff obviously, so I can't comment on how good about detecting comments the new entab code is. Well, the thing I really didn't like was this sort of thing: - /* Oid subtype = PG_GETARG_OID(3); */ + /* Oid subtype = PG_GETARG_OID(3); */ Or this: - * Original author: Simon Riggs si...@2ndquadrant.com - * Current maintainer: Simon Riggs + * Original author: Simon Riggs si...@2ndquadrant.com + * Current maintainer: Simon Riggs Or this: - * dirpath - the directory name supplied on the command line - * configpath - optional configuration directory - * envVarName - the name of an environment variable to get if dirpath is NULL + * dirpath - the directory name supplied on the command line + * configpath- optional configuration directory + * envVarName- the name of an environment variable to get if dirpath is NULL Or this: - mp_int_copy(a, b); /* ok: 0 = r b */ - mp_int_copy(q, a); /* ok: q = a */ + mp_int_copy(a, b); /* ok: 0 = r b */ + mp_int_copy(q, a); /* ok: q = a */ I do agree with you some of the changes-after-periods look like improvements; what I meant to say is that there is an awful lot of churn in this patch that is unrelated to that particular point. -- 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] Add min and max execute statement time in pg_stat_statement
Peter Geoghegan p...@heroku.com writes: On Thu, Jan 30, 2014 at 9:57 AM, Tom Lane t...@sss.pgh.pa.us wrote: BTW ... it occurs to me to wonder if it'd be feasible to keep the query-texts file mmap'd in each backend, thereby reducing the overhead to write a new text to about the cost of a memcpy, and eliminating the read cost in pg_stat_statements() altogether. It's most likely not worth the trouble; but if a more-realistic benchmark test shows that we actually have a performance issue there, that might be a way out without giving up the functional advantages of Peter's patch. There could be a worst case for that scheme too, plus we'd have to figure out how to make in work with windows, which in the case of mmap() is not a sunk cost AFAIK. I'm skeptical of the benefit of pursuing that. Well, it'd be something like #ifdef HAVE_MMAP then use mmap, else use what's there today. But I agree that it's not something to pursue unless we see a more credible demonstration of a problem. Presumably any such code would have to be prepared to remap if the file grows larger than it initially allowed for; and that would be complicated and perhaps have unwanted failure modes. In reality, actual applications could hardly be further from the perfectly uniform distribution of distinct queries presented here. Yeah, I made the same point in different words. I think any realistic comparison of this code to what we had before needs to measure a workload with a more plausible query frequency distribution. 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] updated emacs configuration
Alvaro Herrera alvhe...@2ndquadrant.com writes: Robert Haas escribió: I don't have any comment on that specific point, but I took a quick look through one of these diffs and it looks like a lot of churn for no improvement. So I'm not sure what we want to do here, but I don't think it's this. I, on the contrary, think that the cases that are preceded by a period are an improvement, and the rest are not (the opposite, actually, so better not change those). Maybe there are specific cases in which a period-tab sequence should be kept, but that seems rarer. I concur with Alvaro; the only cases I want to see changed are period followed by a tab that would be exactly two spaces. 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] updated emacs configuration
On Thu, Jan 30, 2014 at 03:32:27PM -0500, Robert Haas wrote: Or this: - mp_int_copy(a, b); /* ok: 0 = r b */ - mp_int_copy(q, a); /* ok: q = a */ + mp_int_copy(a, b); /* ok: 0 = r b */ + mp_int_copy(q, a); /* ok: q = a */ I do agree with you some of the changes-after-periods look like improvements; what I meant to say is that there is an awful lot of churn in this patch that is unrelated to that particular point. Let me work on a entab patch that does replacements in comments only after periods and post the results. I should be done in an hour. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] [PATCH] pg_sleep(interval)
On Thu, Oct 17, 2013 at 9:11 AM, Vik Fearing vik.fear...@dalibo.com wrote:
On 10/17/2013 02:42 PM, Robert Haas wrote:
On Thu, Oct 17, 2013 at 8:26 AM, Vik Fearing vik.fear...@dalibo.com wrote:
On 10/17/2013 10:03 AM, Fabien COELHO wrote:
My guess is that it won't be committed if there is a single but it
might break one code or surprise one user somewhere in the universe,
but I wish I'll be proven wrong. IMO, returned with feedback on a 1
liner is really akin to rejected.
I have attached here an entirely new patch (new documentation and
everything) that should please everyone. It no longer overloads
pg_sleep(double precision) but instead add two new functions:
* pg_sleep_for(interval)
* pg_sleep_until(timestamp with time zone)
Because it's no longer overloading the original pg_sleep, Robert's
ambiguity objection is no more.
Also, I like how it reads aloud: SELECT pg_sleep_for('5 minutes');
If people like this, I'll reject the current patch and add this one to
the next commitfest.
I find that naming relatively elegant. However, you've got to
schema-qualify every function and operator used in the definitions, or
you're creating a search-path security vulnerability.
Good catch. Updated patch attached.
Committed.
--
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] Regression tests failing if not launched on db regression
On Thu, Jan 30, 2014 at 3:32 AM, Christian Kruse christ...@2ndquadrant.com wrote: For the documentation patch, I propose the attached to avoid future confusions. Comments? It might make sense to back-patch as well. Compiles, didn't find any typos and I think it is comprehensible. I took a look at this with a view to committing it but on examination I'm not sure this is the best way to proceed. The proposed text documents that the tests should be run in a database called regression, but the larger documentation chapter of which this section is a part never explains how to run them anywhere else, so it feels a bit like telling a ten-year-old not to burn out the clutch. The bit about not changing enable_* probably belongs, if anywhere, in section 30.2, on test evaluation, rather than 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] Add min and max execute statement time in pg_stat_statement
On Thu, Jan 30, 2014 at 12:32 PM, Tom Lane t...@sss.pgh.pa.us wrote: In reality, actual applications could hardly be further from the perfectly uniform distribution of distinct queries presented here. Yeah, I made the same point in different words. I think any realistic comparison of this code to what we had before needs to measure a workload with a more plausible query frequency distribution. Even though that distribution just doesn't square with anybody's reality, you can still increase the pg_stat_statements.max setting to 10k and the problem goes away at little cost (a lower setting is better, but a setting high enough to cache everything is best). But you're not going to have terribly much use for pg_stat_statements anywayif you really do experience churn at that rate with 5,000 possible entries, the module is ipso facto useless, and should be disabled. -- Peter Geoghegan -- 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] Patch: show xid and xmin in pg_stat_activity and pg_stat_replication
On 30 January 2014 17:27, Robert Haas robertmh...@gmail.com wrote: On Wed, Jan 29, 2014 at 3:11 PM, Simon Riggs si...@2ndquadrant.com wrote: On 14 January 2014 08:38, Christian Kruse christ...@2ndquadrant.com wrote: Hi, On 13/01/14 20:06, Heikki Linnakangas wrote: On 12/17/2013 04:58 PM, Christian Kruse wrote: attached you will find a patch for showing the current transaction id (xid) and the xmin of a backend in pg_stat_activty and the xmin in pg_stat_replication. Docs. Thanks, update with updated docs is attached. Looks simple enough and useful for working out which people are holding up CONCURRENT activities. I've not been involved with this patch, so any objections to me doing final review and commit? Nope, but I think this patch is broken. It looks to me like it's conflating the process offset in the BackendStatus array with its backendId, which does not seem like a good idea even if it happens to work at present. And the way BackendIdGetProc() is used looks unsafe, too: the contents might no longer be valid by the time we read them. I suspect we should have a new accessor function that takes a backend ID and copies the xid and xmin to pointers provided by the client while holding the lock. I also note that the docs seem to need some copy-editing: + entryThe current xref linked=ddl-system-columnsxmin value./xref/entry Thanks, saved me the trouble of a detailed review... good catches. -- Simon Riggs http://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] inherit support for foreign tables
On Thu, Jan 30, 2014 at 11:04 AM, Tom Lane t...@sss.pgh.pa.us wrote: Shigeru Hanada shigeru.han...@gmail.com writes: At the moment we don't use attstorage for foreign tables, so allowing SET STORAGE against foreign tables never introduce visible change except \d+ output of foreign tables. But IMO such operation should not allowed because users would be confused. So I changed ATExecSetStorage() to skip on foreign tables. I think this is totally misguided. Who's to say that some weird FDW might not pay attention to attstorage? I could imagine a file-based FDW using that to decide whether to compress columns, for instance. Admittedly, the chances of that aren't large, but it's pretty hard to argue that going out of our way to prevent it is a useful activity. I think that's a pretty tenuous position. There are already FDW-specific options sufficient to let a particular FDW store whatever kinds of options it likes; letting the user set options that were only ever intended to be applied to tables just because we can seems sort of dubious. I'm tempted by the idea of continuing to disallow SET STORAGE on an unvarnished foreign table, but allowing it on an inheritance hierarchy that contains at least one real table, with the semantics that we quietly ignore the foreign tables and apply the operation to the plain tables. -- 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] inherit support for foreign tables
Robert Haas robertmh...@gmail.com writes: On Thu, Jan 30, 2014 at 11:04 AM, Tom Lane t...@sss.pgh.pa.us wrote: I think this is totally misguided. Who's to say that some weird FDW might not pay attention to attstorage? I could imagine a file-based FDW using that to decide whether to compress columns, for instance. Admittedly, the chances of that aren't large, but it's pretty hard to argue that going out of our way to prevent it is a useful activity. I think that's a pretty tenuous position. There are already FDW-specific options sufficient to let a particular FDW store whatever kinds of options it likes; letting the user set options that were only ever intended to be applied to tables just because we can seems sort of dubious. I'm tempted by the idea of continuing to disallow SET STORAGE on an unvarnished foreign table, but allowing it on an inheritance hierarchy that contains at least one real table, with the semantics that we quietly ignore the foreign tables and apply the operation to the plain tables. [ shrug... ] By far the easiest implementation of that is just to apply the catalog change to all of them. According to your assumptions, it'll be a no-op on the foreign tables anyway. 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] jsonb and nested hstore - small docpatch
On Thu, January 30, 2014 20:07, Andrew Dunstan wrote: Updated patches for both pieces. Included is some tidying done by [ nested-hstore-9.patch.gz ] Here is a small doc-patch to Table F-6. hstore Operators It corrects its booleans in the 'Result' column ( t and f instead of true and false ). Thanks, Erik Rijkers -- 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] jsonb and nested hstore - small docpatch
On Thu, January 30, 2014 23:15, Erik Rijkers wrote:
On Thu, January 30, 2014 20:07, Andrew Dunstan wrote:
Updated patches for both pieces. Included is some tidying done by
[ nested-hstore-9.patch.gz ]
Here is a small doc-patch to Table F-6. hstore Operators
It corrects its booleans in the 'Result' column ( t and f instead of true
and false ).
I mean, here it is...
--- doc/src/sgml/hstore.sgml.orig 2014-01-30 22:39:52.970474354 +0100
+++ doc/src/sgml/hstore.sgml 2014-01-30 22:57:27.630698633 +0100
@@ -286,7 +286,7 @@
entrytypeboolean//entry
entryget boolean value for key (literalNULL/ if not boolean or not present)/entry
entryliteral'a =gt; 42.0, b =gt; true'::hstore ?gt; 'b'/literal/entry
- entryliteraltrue/literal/entry
+ entryliteralt/literal/entry
/row
row
@@ -294,7 +294,7 @@
entrytypeboolean//entry
entryget boolean value for array index (literalNULL/ if not boolean or not present)/entry
entryliteral'[false,null,44]'::hstore ?gt; 0/literal/entry
- entryliteralfalse/literal/entry
+ entryliteralf/literal/entry
/row
row
@@ -318,7 +318,7 @@
entrytypeboolean//entry
entryget boolean value for key path (literalNULL/ if not boolean or not present)/entry
entryliteral'foo =gt; {bar =gt; true}'::hstore #?gt; '{foo,bar}'/literal/entry
- entryliteraltrue/literal/entry
+ entryliteralt/literal/entry
/row
row
@@ -366,7 +366,7 @@
entrytypeboolean//entry
entrydoes typehstore/ contain key?/entry
entryliteral'a=gt;1'::hstore ? 'a'/literal/entry
- entryliteraltrue/literal/entry
+ entryliteralt/literal/entry
/row
row
@@ -374,7 +374,7 @@
entrytypeboolean//entry
entrydoes typehstore/ contain array index?/entry
entryliteral'[a,b,c]'::hstore ? 2/literal/entry
- entryliteraltrue/literal/entry
+ entryliteralt/literal/entry
/row
row
@@ -382,7 +382,7 @@
entrytypeboolean//entry
entrydoes typehstore/ contain key path?/entry
entryliteral'[1, 2, {foo=gt;hi}]'::hstore #? '{2,foo}'/literal/entry
- entryliteraltrue/literal/entry
+ entryliteralt/literal/entry
/row
row
@@ -390,7 +390,7 @@
entrytypeboolean//entry
entrydoes typehstore/ contain all specified keys?/entry
entryliteral'a=gt;1,b=gt;2'::hstore ?amp; ARRAY['a','b']/literal/entry
- entryliteraltrue/literal/entry
+ entryliteralt/literal/entry
/row
row
@@ -398,7 +398,7 @@
entrytypeboolean//entry
entrydoes typehstore/ contain any of the specified keys?/entry
entryliteral'a=gt;1,b=gt;2'::hstore ?| ARRAY['b','c']/literal/entry
- entryliteraltrue/literal/entry
+ entryliteralt/literal/entry
/row
row
@@ -406,7 +406,7 @@
entrytypeboolean//entry
entrydoes left operand contain right?/entry
entryliteral'a=gt;b, b=gt;1, c=gt;NULL'::hstore @gt; 'b=gt;1'/literal/entry
- entryliteraltrue/literal/entry
+ entryliteralt/literal/entry
/row
row
@@ -414,7 +414,7 @@
entrytypeboolean//entry
entryis left operand contained in right?/entry
entryliteral'a=gt;c'::hstore lt;@ 'a=gt;b, b=gt;1, c=gt;NULL'/literal/entry
- entryliteralfalse/literal/entry
+ entryliteralf/literal/entry
/row
row
--
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] Fwd: Request for error explaination || Adding a new integer in indextupleData Structure
On 1/28/14, 3:59 PM, Rohit Goyal wrote: The data from IndexTupleData is written to disk, and then read back in again. Did you initdb a new database cluster after you made your change? If you did the initdb with the original code, and then tried to point your new code at the old disk files, that is very unlikely to work, as format is now different. Cheers, Jeff Hi Jeff and Tom, Thanks you so much. I was making the mistake you mentioned in the last mail. :) The real issue here is that you need to bump the catalog version number (sorry, but I don't know where that is in code). -- Jim C. Nasby, Data Architect j...@nasby.net 512.569.9461 (cell) http://jim.nasby.net -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Making strxfrm() blobs in indexes work
On more occasions than I care to recall, someone has suggested that it
would be valuable to do something with strxfrm() blobs in order to
have cheaper locale-aware text comparisons. One obvious place to do so
would be in indexes, but in the past that has been dismissed on the
following grounds:
* Index-only scans need fully formed datums to work, and strxfrm() is
a one-way function (or so I believe). There is no reason to think that
the original string can be reassembled from the blob, so clearly that
won't fly.
* There is a high cost to be paid in storage overhead. Even for
collations like en_US.UTF-8, that can mean that the blob is as much
as 3-4 times larger than the original text string. Who is to say that
we'll come out ahead even with the savings of just doing a strcmp()
rather than a strcoll()?
So, even though using strxfrm() blobs can be much faster, it seems
hard to find a place to use them. A compelling improvement in
performance for index scans is so close, and yet so far. But having
mulled it over during my recent work on various aspects of the B-Tree
code, I believe that there is a way that we can cheat and come out
ahead with strxfrm() blobs in indexes.
I have a sample dataset that I like to work off of. It's the database
of the Mouse Genome Database, available from
http://www.informatics.jax.org/. This is real data, from a real
Postgres database, and so I recommend working off one of their weekly
dumps if you're looking for a real dataset that is big enough to
exercise most interesting things, and yet not too big. It seems that
there is an entire ecosystem of tools for medical researchers around
the dataset, and they have the right attitude about opening it all up,
which is pretty cool.
Anyway, I quickly threw together the following query, which shows the
break-down of leaf pages, inner pages and root pages among the largest
indexes, and the proportion of each per index:
[local] pg@mouse=# with tots as (
SELECT count(*) c, type, relname from
(select relname, relpages, generate_series(1, relpages - 1) i
from pg_class c join pg_namespace n on c.relnamespace = n.oid where
relkind = 'i' and nspname = 'mgd') r,
lateral (select * from bt_page_stats('mgd.' || relname, i)) u
group by relname, type)
select tots.relname, relpages -1 as non_meta_pages, c, c/sum(c)
over(partition by tots.relname) as prop_of_index, type from tots join
pg_class c on c.relname = tots.relname order by 2 desc, 1, type;
relname | non_meta_pages | c
| prop_of_index| type
++++--
acc_accession_0| 106181 |
520 | 0.00489729801000178940 | i
acc_accession_0| 106181 |
105660 | 0.99509328410920974562 | l
acc_accession_0| 106181 |
1 | 0.09417880788464979610 | r
acc_accession_idx_accid| 106181 |
520 | 0.00489729801000178940 | i
acc_accession_idx_accid| 106181 |
105660 | 0.99509328410920974562 | l
acc_accession_idx_accid| 106181 |
1 | 0.09417880788464979610 | r
mgi_notechunk_idx_note | 101127 |
4817 | 0.04763317412758214918 | i
mgi_notechunk_idx_note | 101127 |
96309 | 0.95235693731644367973 | l
mgi_notechunk_idx_note | 101127 |
1 | 0.09888555974171091795 | r
acc_accession_1| 80507 |
302 | 0.00375122660141354168 | i
acc_accession_1| 80507 |
80204 | 0.99623635211844932739 | l
acc_accession_1| 80507 |
1 | 0.12421280137130932714 | r
acc_accession_idx_prefixpart | 80507 |
302 | 0.00375122660141354168 | i
acc_accession_idx_prefixpart | 80507 |
80204 | 0.99623635211844932739 | l
acc_accession_idx_prefixpart | 80507 |
1 | 0.12421280137130932714 | r
***SNIP***
To the surprise of no one, typically ~99.5% of B-Tree index pages are
leaf pages. I do see one index on a column of very large text strings
(a notes column) that is only about 95% leaf pages, which is omitted
here. But the general picture is that over 99% of non-meta pages are
leaf pages. Again, this is hardly surprising: That's more or less why
B-Tree indexes work so well when partially cached.
I'm sure anyone that has read this far knows where I'm going with
this: why can't we just have strxfrm() blobs in the inner pages,
implying large savings for a big majority of text comparisons that
service index scans, without bloating the indexes too badly, and
without breaking anything? We only use inner pages to find leaf
Re: [HACKERS] pg_upgrade: make the locale comparison more tolerating
Pavel Raiskup prais...@redhat.com writes: [ 0001-pg_upgrade-make-the-locale-comparison-more-toleratin.patch ] Committed with minor adjustments (mostly, wordsmithing the comments). Although this was categorized as a bug fix, I'm not sure it's worth taking the risk of back-patching. We've not seen very many reports of problems of this type. Of course, you're free to carry it as a patch in Red Hat's packages if you differ ... 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] jsonb and nested hstore
On Thu, Jan 30, 2014 at 1:07 PM, Andrew Dunstan and...@dunslane.net wrote:
On 01/29/2014 04:56 PM, Andrew Dunstan wrote:
On 01/29/2014 01:03 PM, Andrew Dunstan wrote:
On 01/27/2014 10:43 PM, Andrew Dunstan wrote:
On 01/26/2014 05:42 PM, Andrew Dunstan wrote:
Here is the latest set of patches for nested hstore and jsonb.
Because it's so large I've broken this into two patches and compressed
them. The jsonb patch should work standalone. The nested hstore patch
depends on it.
All the jsonb functions now use the jsonb API - there is no more
turning jsonb into text and reparsing it.
At this stage I'm going to be starting cleanup on the jsonb code
(indentation, error messages, comments etc.) as well get getting up some
jsonb docs.
Here is an update of the jsonb part of this. Charges:
* there is now documentation for jsonb
* most uses of elog() in json_funcs.c are replaced by ereport().
* indentation fixes and other tidying.
No changes in functionality.
Further update of jsonb portion.
Only change in functionality is the addition of casts between jsonb and
json.
The other changes are the merge with the new json functions code, and
rearrangement of the docs changes to make them less ugly. Essentially I
moved the indexterm tags right out of the table as is done in some other
parts pf the docs. That makes the entry tags much clearer to read.
Updated to apply cleanly after recent commits.
Updated patches for both pieces. Included is some tidying done by Teodor,
and fixes for remaining whitespace issues. This now passes git diff --check
master cleanly for me.
Something seems off:
postgres=# create type z as (a int, b int[]);
CREATE TYPE
postgres=# create type y as (a int, b z[]);
CREATE TYPE
postgres=# create type x as (a int, b y[]);
CREATE TYPE
-- test a complicated construction
postgres=# select row(1, array[row(1, array[row(1, array[1,2])::z])::y])::x;
row
-
(1,{(1,\\{(1,{1,2})}\\)})
postgres=# select hstore(row(1, array[row(1, array[row(1,
array[1,2])::z])::y])::x);
hstore
--
a=1,
b={\(1,\\\{\\(1,\\{1,2}\\)\\}\\\)\}
here, the output escaping has leaked into the internal array
structures. istm we should have a json expressing the internal
structure. It does (weirdly) map back however:
postgres=# select populate_record(null::x, hstore(row(1, array[row(1,
array[row(1, array[1,2])::z])::y])::x));
populate_record
-
(1,{(1,\\{(1,{1,2})}\\)})
OTOH, if I go via json route:
postgres=# select row_to_json(row(1, array[row(1, array[row(1,
array[1,2])::z])::y])::x);
row_to_json
---
{a:1,b:[{a:1,b:[{a:1,b:[1,2]}]}]}
so far, so good. let's push to hstore:
postgres=# select row_to_json(row(1, array[row(1, array[row(1,
array[1,2])::z])::y])::x)::jsonb::hstore;
row_to_json
---
a=1, b=[{a=1, b=[{a=1, b=[1, 2]}]}]
this ISTM is the 'right' behavior. but what if we bring it back to
record object?
postgres=# select populate_record(null::x, row_to_json(row(1,
array[row(1, array[row(1, array[1,2])::z])::y])::x)::jsonb::hstore);
ERROR: malformed array literal: {{a=1, b={{a=1, b={1, 2}
yikes. The situation as I read it is that (notwithstanding my comments
upthread) there is no clean way to slide rowtypes to/from hstore and
jsonb while preserving structure. IMO, the above query should work
and the populate function record above should return the internally
structured row object, not the text escaped version.
merlin
--
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] Making strxfrm() blobs in indexes work
Peter Geoghegan p...@heroku.com writes: On more occasions than I care to recall, someone has suggested that it would be valuable to do something with strxfrm() blobs in order to have cheaper locale-aware text comparisons. One obvious place to do so would be in indexes, but in the past that has been dismissed on the following grounds: * Index-only scans need fully formed datums to work, and strxfrm() is a one-way function (or so I believe). There is no reason to think that the original string can be reassembled from the blob, so clearly that won't fly. * There is a high cost to be paid in storage overhead. Even for collations like en_US.UTF-8, that can mean that the blob is as much as 3-4 times larger than the original text string. Who is to say that we'll come out ahead even with the savings of just doing a strcmp() rather than a strcoll()? Quite aside from the index bloat risk, this effect means a 3-4x reduction in the maximum string length that can be indexed before getting the dreaded Values larger than 1/3 of a buffer page cannot be indexed error. Worse, a value insertion might well succeed, with the failure happening only (much?) later when that entry is chosen as a page split boundary. It's possible that TOAST compression of the strings would save you, but I'm not convinced of that; it certainly doesn't seem like we could guarantee no post-insertion failures that way. Also, detoasting of strings that hadn't been long enough to need toasting before could easily eat any savings. I'm sure anyone that has read this far knows where I'm going with this: why can't we just have strxfrm() blobs in the inner pages, implying large savings for a big majority of text comparisons that service index scans, without bloating the indexes too badly, and without breaking anything? We only use inner pages to find leaf pages. They're redundant copies of the data within the index. It's a cute idea though, and perhaps worth pursuing as long as you've got the pitfalls in mind. 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] Prohibit row-security + inheritance in 9.4?
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 01/30/2014 10:41 PM, Stephen Frost wrote: Earlier discussions seemed to settle on each relation having its own row-security quals applied independently. So quals on a parent rel did not cascade down to child rels. This strikes me as a bit odd- isn't this against how we handle the GRANT system when it comes to inheiritance? That is to say- if you have access to query the parent, then you'll get rows back from the child and I believe everyone feels that makes perfect sense. Thanks for taking the time to look at this. I agree that it's odd. The trouble is that there's a conflict between two makes perfect senses here. I expect to get all rows, including inherited rows, filtered by a parent's predicate back when you query the parent. Fair enough. I also expect that when I query a child table, I'll see the same rows I see when I query it via the parent table. Especially in the common case of an empty parent table. One of these has to give, we can't have both. I'm speaking with an outside party who has an inheritance-based data model they want to apply row-security to. Hopefully that'll shed some light on practical implications. There's another bit of fun too: If you have a policy on a child, and query the child via the parent, should the child policy be applied? The immediate thought is obviously - but then, often the child and parent policies are going to be the same, in which case it's a duplicate filter step. Here security trumps efficiency; I think we just apply both, and leave proving they're identical and skipping the child's as an optimization problem for later. It's what you'd expect to happen when querying a parent rel with row-security, too. Parent quals are applied to children. But that then gives you an inconsistent view of a rel's contents based on whether you query it via a parent or directly. ... Just how our existing GRANT system works. True; it's possible to be able to query the parent, but not its children, at present. Treating row-security checks as permission checks, that'd make this consistent. The difference is that you get a nice error telling you what's going on currently, not a potentially WTF-y different resultset. If you want to constrain the children in the same way as the parent, then the user can add to the row-security on the children to match the parent. Yes, with the caveat mentioned above that this will cause multiple nested row-security policies when querying the parent; each child's policy will get applied, *and* the parent's policy will get applied. Inefficient and ugly if they're the same. If the user wants to have one view for the entire inheiritance tree then they need to only implement the row-security on the parent and not grant any access for users on the children (or, if they're paranoid, add row-security to the children which are essentially deny-all). That works if the children are used for partitioning/inheritance, where no direct access (or at least no read access) is required. It doesn't work so well when they're actually being used in a real inheritance model, where they'll have their own additional attributes that can only be accessed via the child. In that case the only option is to apply a policy to each child too. If we apply the parent policy when querying via the parent, we get multiple nested layers of policy, but it still works. If we force everything to behave the same between querying the parent and querying the children then we cut off various scenarios of partitioning where users are allowed to query specific children but not the parent or query the parent to get things they're not able to see directly from the children. That's a fair concern. Note that the second one won't work if we apply child policies when querying via the parent though; there'd be no way to see rows via the parent that you can't see via the child. 4. attempt to pull quals from parents when querying a child rel directly. That strikes me as borderline insane. ;) I'm glad to hear it, because it'd be inefficient and horrifying to implement. - -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.15 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJS6u+5AAoJELBXNkqjr+S2QkEH/iXg4qww3KfcKLW/dDHWUL/g 33T/168ZGgeAvgFgTQdrgZmM8bUDsNnCD/GdH4PBmNWMlxwTeHmdANBEsKgDCL7r Fu4HuF0JFEQMqHPtZSKUIXxW1KYEnWjISd+4YDQqor3aH03OV3z4vFEgAi73truR kcqOe/xyeuPQDPe/9UTtiyIT2g/sQaeXhNqQx+queKYwjgYTIZgkUs0y46lH4pAK nvWcWCscPIJ4bFpMr3joJQiFwegRaVIcAac89uZHL5iuMKPzp5lfEfWHUmTreZLu 1gPjRxWTcOhNZoaVpVCBA+Gsqw0255IxWKKD8I2RYPp0bK88t42cCB3aHejAjzo= =cB2U -END PGP SIGNATURE- -- 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] Prohibit row-security + inheritance in 9.4?
On 01/31/2014 01:18 AM, Robert Haas wrote: On Thu, Jan 30, 2014 at 2:39 AM, Craig Ringer cr...@2ndquadrant.com wrote: Earlier discussions seemed to settle on each relation having its own row-security quals applied independently. So quals on a parent rel did not cascade down to child rels. Do you have a link to that prior discussion? Not to hand; I'm basing that on discussion with KaiGai, and on the implementation of his RLS patch. The patch goes far out of its way to ensure that policies on a parent relation aren't applied to children, only to rows taken directly from the parent. I read some discussion on the topic when I was first reviewing all the old threads for this, but didn't see anything that seemed to conclusively decide on that approach. -- Craig Ringer http://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] Making strxfrm() blobs in indexes work
On Thu, Jan 30, 2014 at 4:34 PM, Tom Lane t...@sss.pgh.pa.us wrote: Quite aside from the index bloat risk, this effect means a 3-4x reduction in the maximum string length that can be indexed before getting the dreaded Values larger than 1/3 of a buffer page cannot be indexed error. Worse, a value insertion might well succeed, with the failure happening only (much?) later when that entry is chosen as a page split boundary. That's not hard to prevent. If that should happen, we don't go with the strxfrm() datum. We have a spare IndexTuple bit we could use to mark when the optimization was applied. So we consider the appropriateness of a regular strcoll() or a strxfrm() in all contexts (in a generic and extensible manner, but that's essentially what we do). I'm not too discouraged by this restriction, because in practice it won't come up very often. I'm sure anyone that has read this far knows where I'm going with this: why can't we just have strxfrm() blobs in the inner pages, implying large savings for a big majority of text comparisons that service index scans, without bloating the indexes too badly, and without breaking anything? We only use inner pages to find leaf pages. They're redundant copies of the data within the index. It's a cute idea though, and perhaps worth pursuing as long as you've got the pitfalls in mind. I'll think about pursuing it. I might prefer to declare it as fair game for anyone else that wants to do it. -- Peter Geoghegan -- 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] jsonb and nested hstore
On 01/30/2014 07:21 PM, Merlin Moncure wrote:
Something seems off:
postgres=# create type z as (a int, b int[]);
CREATE TYPE
postgres=# create type y as (a int, b z[]);
CREATE TYPE
postgres=# create type x as (a int, b y[]);
CREATE TYPE
-- test a complicated construction
postgres=# select row(1, array[row(1, array[row(1, array[1,2])::z])::y])::x;
row
-
(1,{(1,\\{(1,{1,2})}\\)})
postgres=# select hstore(row(1, array[row(1, array[row(1,
array[1,2])::z])::y])::x);
hstore
--
a=1,
b={\(1,\\\{\\(1,\\{1,2}\\)\\}\\\)\}
here, the output escaping has leaked into the internal array
structures. istm we should have a json expressing the internal
structure.
What has this to do with json at all? It's clearly a failure in the
hstore() function.
It does (weirdly) map back however:
postgres=# select populate_record(null::x, hstore(row(1, array[row(1,
array[row(1, array[1,2])::z])::y])::x));
populate_record
-
(1,{(1,\\{(1,{1,2})}\\)})
OTOH, if I go via json route:
postgres=# select row_to_json(row(1, array[row(1, array[row(1,
array[1,2])::z])::y])::x);
row_to_json
---
{a:1,b:[{a:1,b:[{a:1,b:[1,2]}]}]}
so far, so good. let's push to hstore:
postgres=# select row_to_json(row(1, array[row(1, array[row(1,
array[1,2])::z])::y])::x)::jsonb::hstore;
row_to_json
---
a=1, b=[{a=1, b=[{a=1, b=[1, 2]}]}]
this ISTM is the 'right' behavior. but what if we bring it back to
record object?
postgres=# select populate_record(null::x, row_to_json(row(1,
array[row(1, array[row(1, array[1,2])::z])::y])::x)::jsonb::hstore);
ERROR: malformed array literal: {{a=1, b={{a=1, b={1, 2}
yikes. The situation as I read it is that (notwithstanding my comments
upthread) there is no clean way to slide rowtypes to/from hstore and
jsonb while preserving structure. IMO, the above query should work
and the populate function record above should return the internally
structured row object, not the text escaped version.
And this is a failure in populate_record().
I think we possibly need to say that handling of nested composites and
arrays is an area that needs further work. OTOH, the refusal of
json_populate_record() and json_populate_recordset() to handle these in
9.3 has not generated a flood of complaints, so I don't think it's a
tragedy, just a limitation, which should be documented if it's not
already. (And of course hstore hasn't handled nested anything before now.)
Meanwhile, maybe Teodor can fix the two hstore bugs shown here.
cheers
andrew
--
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] inherit support for foreign tables
On Thu, Jan 30, 2014 at 5:05 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Thu, Jan 30, 2014 at 11:04 AM, Tom Lane t...@sss.pgh.pa.us wrote: I think this is totally misguided. Who's to say that some weird FDW might not pay attention to attstorage? I could imagine a file-based FDW using that to decide whether to compress columns, for instance. Admittedly, the chances of that aren't large, but it's pretty hard to argue that going out of our way to prevent it is a useful activity. I think that's a pretty tenuous position. There are already FDW-specific options sufficient to let a particular FDW store whatever kinds of options it likes; letting the user set options that were only ever intended to be applied to tables just because we can seems sort of dubious. I'm tempted by the idea of continuing to disallow SET STORAGE on an unvarnished foreign table, but allowing it on an inheritance hierarchy that contains at least one real table, with the semantics that we quietly ignore the foreign tables and apply the operation to the plain tables. [ shrug... ] By far the easiest implementation of that is just to apply the catalog change to all of them. According to your assumptions, it'll be a no-op on the foreign tables anyway. Well, there's some point to that, too, I suppose. What do others think? -- 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] Prohibit row-security + inheritance in 9.4?
On Thursday, January 30, 2014, Craig Ringer cr...@2ndquadrant.com wrote: This strikes me as a bit odd- isn't this against how we handle the GRANT system when it comes to inheiritance? That is to say- if you have access to query the parent, then you'll get rows back from the child and I believe everyone feels that makes perfect sense. Thanks for taking the time to look at this. I agree that it's odd. The trouble is that there's a conflict between two makes perfect senses here. I expect to get all rows, including inherited rows, filtered by a parent's predicate back when you query the parent. Fair enough. I also expect that when I query a child table, I'll see the same rows I see when I query it via the parent table. Especially in the common case of an empty parent table. I don't see where this follows at all- clearly, you already get a subset of rows from the child than if you queried the parent because there are other children. If you are first playing with inheritance in PG then it might seem odd for that to be the case. Ditto for what you describe where the child returns more rows than the parent, but these things need to simply be documented as this is how it works for those cases where both are reasonable possibilities and we need to pick one. Personally, I don't see the suggestion that we filter rows accessed via the child based on quals of the parent as making any sense. I feel the same about applying child quals when querying through the parent as we don't apply GRANT permissions that way. Using the parent and using the child are two different paths by which to access the data and which you are using is what drives what you will see. One of these has to give, we can't have both. I agree that we can't do both. I'm speaking with an outside party who has an inheritance-based data model they want to apply row-security to. Hopefully that'll shed some light on practical implications. Is there a case which can't be implemented if the two are independent as I am describing? There are cases which can NOT be implemented if we force the two paths to be handled identically but I feel the approach where we keep them independently managed is flexible to allow the other cases if people want them. There's another bit of fun too: If you have a policy on a child, and query the child via the parent, should the child policy be applied? No! We do not do that for GRANT and I do not see doing it for row security either. The immediate thought is obviously - but then, often the child and parent policies are going to be the same, in which case it's a duplicate filter step. Here security trumps efficiency; I think we just apply both, and leave proving they're identical and skipping the child's as an optimization problem for later. No, if you apply both then you reduce the ability of the user to set it up to meet their needs. Allowing these paths to be managed independent allows more flexibility. If it adds a bit of bookkeeping for users who wish to allow access to both the child and the parent directly then tools can be written to manage that. ... Just how our existing GRANT system works. True; it's possible to be able to query the parent, but not its children, at present. It was actually changed not all that long ago to be this way because having a query against the parent *sometimes* fail when hitting a certain child table was not sensible. Treating row-security checks as permission checks, that'd make this consistent. The difference is that you get a nice error telling you what's going on currently, not a potentially WTF-y different resultset. I understand where you're coming from but this strikes me as a documentation/definition issue and not really a cause for concern or against POLA. These are complex and important topics that anyone who cares about security needs to understand. If you want to constrain the children in the same way as the parent, then the user can add to the row-security on the children to match the parent. Yes, with the caveat mentioned above that this will cause multiple nested row-security policies when querying the parent; each child's policy will get applied, *and* the parent's policy will get applied. Inefficient and ugly if they're the same. No. You misunderstand. When it comes to querying the parent only the parents would apply. Yes, this may mean the parent has to have more than it would otherwise but there would not need to be any redundant evaluation- perhaps redundant definition, but that's not the same. If the user wants to have one view for the entire inheiritance tree then they need to only implement the row-security on the parent and not grant any access for users on the children (or, if they're paranoid, add row-security to the children which are essentially deny-all). That works if the children are used for partitioning/inheritance, where no direct access (or at least no read access) is
Re: [HACKERS] Making strxfrm() blobs in indexes work
Peter Geoghegan p...@heroku.com writes: On Thu, Jan 30, 2014 at 4:34 PM, Tom Lane t...@sss.pgh.pa.us wrote: Quite aside from the index bloat risk, this effect means a 3-4x reduction in the maximum string length that can be indexed before getting the dreaded Values larger than 1/3 of a buffer page cannot be indexed error. Worse, a value insertion might well succeed, with the failure happening only (much?) later when that entry is chosen as a page split boundary. That's not hard to prevent. If that should happen, we don't go with the strxfrm() datum. We have a spare IndexTuple bit we could use to mark when the optimization was applied. You'd need a bit per column, no? 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] Making strxfrm() blobs in indexes work
On Thu, Jan 30, 2014 at 4:45 PM, Peter Geoghegan p...@heroku.com wrote: So we consider the appropriateness of a regular strcoll() or a strxfrm() in all contexts (in a generic and extensible manner, but that's essentially what we do). I'm not too discouraged by this restriction, because in practice it won't come up very often. I meant: We consider the appropriateness of a strxfrm() + strcmp() against the pre-strfxfrm()'d scanKey datum, when the optimization is not in force, as against just a plain strcmp() when it is. -- Peter Geoghegan -- 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] Making strxfrm() blobs in indexes work
On Thu, Jan 30, 2014 at 5:04 PM, Tom Lane t...@sss.pgh.pa.us wrote: That's not hard to prevent. If that should happen, we don't go with the strxfrm() datum. We have a spare IndexTuple bit we could use to mark when the optimization was applied. You'd need a bit per column, no? I don't think so. It would be no big deal if it was all or nothing. -- Peter Geoghegan -- 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] jsonb and nested hstore
On Thu, Jan 30, 2014 at 4:52 PM, Andrew Dunstan and...@dunslane.net wrote:
On 01/30/2014 07:21 PM, Merlin Moncure wrote:
postgres=# select hstore(row(1, array[row(1, array[row(1,
array[1,2])::z])::y])::x);
hstore
--
a=1,
b={\(1,\\\{\\(1,\\{1,2}\\)\\}\\\)\}
here, the output escaping has leaked into the internal array
structures. istm we should have a json expressing the internal
structure.
What has this to do with json at all? It's clearly a failure in the hstore()
function.
yeah -- meant to say 'hstore' there. Also I'm not sure that it's
'wrong'; it's just doing what it always did. That brings up another
point: are there any interesting cases of compatibility breakage? I'm
inclined not to care about this particular case though...
array[row(1, array[row(1, array[1,2])::z])::y])::x)::jsonb::hstore);
ERROR: malformed array literal: {{a=1, b={{a=1, b={1,
2}
yikes. The situation as I read it is that (notwithstanding my comments
upthread) there is no clean way to slide rowtypes to/from hstore and
jsonb while preserving structure. IMO, the above query should work
and the populate function record above should return the internally
structured row object, not the text escaped version.
And this is a failure in populate_record().
I think we possibly need to say that handling of nested composites and
arrays is an area that needs further work. OTOH, the refusal of
json_populate_record() and json_populate_recordset() to handle these in 9.3
has not generated a flood of complaints, so I don't think it's a tragedy,
just a limitation, which should be documented if it's not already. (And of
course hstore hasn't handled nested anything before now.)
Meanwhile, maybe Teodor can fix the two hstore bugs shown here.
While not a flood, there certainly have been complaints. See
http://postgresql.1045698.n5.nabble.com/Best-way-to-populate-nested-composite-type-from-JSON-td5770566.html
http://osdir.com/ml/postgresql-pgsql-general/2014-01/msg00205.html
But, if we had to drop this in the interests of time I'd rather see
the behavior cauterized off so that it errored out 'not supported' (as
json_populate does) that attempt to implement the wrong behavior.
merlin
--
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] [PATCH] pg_sleep(interval)
On 01/30/2014 09:48 PM, Robert Haas wrote:
On Thu, Oct 17, 2013 at 9:11 AM, Vik Fearing vik.fear...@dalibo.com wrote:
On 10/17/2013 02:42 PM, Robert Haas wrote:
On Thu, Oct 17, 2013 at 8:26 AM, Vik Fearing vik.fear...@dalibo.com wrote:
On 10/17/2013 10:03 AM, Fabien COELHO wrote:
My guess is that it won't be committed if there is a single but it
might break one code or surprise one user somewhere in the universe,
but I wish I'll be proven wrong. IMO, returned with feedback on a 1
liner is really akin to rejected.
I have attached here an entirely new patch (new documentation and
everything) that should please everyone. It no longer overloads
pg_sleep(double precision) but instead add two new functions:
* pg_sleep_for(interval)
* pg_sleep_until(timestamp with time zone)
Because it's no longer overloading the original pg_sleep, Robert's
ambiguity objection is no more.
Also, I like how it reads aloud: SELECT pg_sleep_for('5 minutes');
If people like this, I'll reject the current patch and add this one to
the next commitfest.
I find that naming relatively elegant. However, you've got to
schema-qualify every function and operator used in the definitions, or
you're creating a search-path security vulnerability.
Good catch. Updated patch attached.
Committed.
Thanks!
--
Vik
--
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] GiST support for inet datatypes
On 01/23/2014 11:22 PM, Emre Hasegeli wrote: inet-gist - I realized that create extension btree_gist fails with the patch. ERROR: could not make operator class gist_inet_ops be default for type inet DETAIL: Operator class inet_ops already is the default. I think making the new operator class default makes more sense. Name conflict can be a bigger problem. Should I rename the new operator class? I agree that the new operator class should be the default one, it is more useful and also not buggy. No idea about the naming. The only thing is that I think your index would do poorly on the case where all values share a prefix before the netmask but have different values after the netmask, since gist_union and gist_penalty does not care about the bits after the netmask. Am I correct? Have you thought anything about this? Yes, I have tried to construct the index with ip_maxbits() instead of ip_bits() but I could not make it work with the cidr type. It can be done by adding operator as a parameter for union, penalty and picksplit functions on the GiST framework. I am not sure it worths the trouble. It would only help the basic comparison operators and it would slow down other operators because network length comparison before network address would not be possible for the intermediate nodes. I mentioned this behavior of the operator class on the paragraph added to the documentation. I think this is fine since it does not seem like a very useful case to support to me. Would be worth doing only if it is easy to do. A separate concern: we still have not come to a decision about operators for inet here. I do not like the fact that the operators differ between ranges and inet. But I feel this might be out of scope for this patch. Does any third party want to chime in with an opinion? Current inet/cidr is contained within = is contained within or equals contains = contains or equals Range types @ contains range @ range is contained by overlap (have points in common) strictly left of strictly right of inet-selfuncs - Here I have to honestly admit I am not sure if I can tell if your selectivity function is correct. Your explanation sounds convincing and the general idea of looking at the histogram is correct. I am just have no idea if the details are right. I tried to improve it in this version. I hope it is more readable now. I used the word inclusion instead of overlap, made some changes on the algorithm to make it more suitable to the other operators. Using the histogram for inclusion which is originally for basic comparison, is definitely not correct. It is an empirical approach. I have tried several versions of it, tested them with different data sets and thought it is better than nothing. Thanks for the updates. The code in this version is cleaner and easier to follow. I am not convinced of your approach to calculating the selectivity from the histogram. The thing I have the problem with is the clever trickery involved with how you handle different operator types. I prefer the clearer code of the range types with how calc_hist_selectivity_scalar is used. Is there any reason for why that approach would not work here or result in worse code? I have attached a patch where I improved the language of your comment describing the workings of the selectivity estimation. Could you please check it so I did not change the meaning of any of it? I see from the tests that you still are missing selectivity functions for operators, what is your plan for this? -- Andreas Karlsson diff --git a/src/backend/utils/adt/network_selfuncs.c b/src/backend/utils/adt/network_selfuncs.c new file mode 100644 index 87a7390..3b99afe *** a/src/backend/utils/adt/network_selfuncs.c --- b/src/backend/utils/adt/network_selfuncs.c *** inet_opr_order(Oid operator) *** 172,181 * * Calculates histogram selectivity for the subnet inclusion operators of * the inet type. In the normal case, the return value is between 0 and 1. ! * It should be corrected with MVC selectivity and null fraction. If * the constant is less than the first element or greater than the last * element of the histogram the return value will be 0. If the histogram ! * does not available, the return value will be -1. * * The histogram is originally for the basic comparison operators. Only * the common bits of the network part and the lenght of the network part --- 172,181 * * Calculates histogram selectivity for the subnet inclusion operators of * the inet type. In the normal case, the return value is between 0 and 1. ! * It should be corrected with the MVC selectivity and null fraction. If * the constant is less than the first element or greater than the last * element of the histogram the return value will be 0. If the histogram ! * is not available, the return value will be -1. * * The
Re: [HACKERS] Making strxfrm() blobs in indexes work
On Thu, Jan 30, 2014 at 3:49 PM, Peter Geoghegan p...@heroku.com wrote: So ISTM that we could come up with an infrastructure, possibly just for insertion scanKeys (limiting the code footprint of all of this) in order to inner-page-process datums at this juncture, and store a blob instead, for later savings on walking the tree. I believe that this technique has applicability beyond strxfrm(), and a type-naive infrastructure could be developed to support it, with a degree of complexity not too far in excess of, say, the SortSupport infrastructure. The realization that we don't really need to recover the original information directly from the inner pages gives us scope to push things in several useful directions, I suspect. Techniques around B-Trees that are useful for common workloads may have problems around something that rhymes with combatants, since it stands to reason that many were developed within industry, and some of the major vendors are known to be very combative. This whole area seems safe, though. After all, Singleton wrote in a 1969 paper ALGORITHM 347: AN EFFICIENT ALGORITHM FOR SORTING WITH MINIMAL STORAGE that ACM members can download: This FORTRAN subroutine was tested on a CDC 6400 computer. For random uniform numbers, sorting times divided by n log^2 n were nearly constant at 20.2 X 10^-6 for 100 n 10,000, with a time of 0.202 seconds for 1000 items. This subroutine was also hand-compiled for the same computer to produce a more efficient machine code. In this version the constant of proportionality was 5.2 X 10^-6, with a time of 0.052 seconds for 1000 items. In both cases, integer comparisons were used to order normalized floating-point numbers. This looks very much like prior art for our purposes. I'm pretty sure that the problem was back then they didn't have dedicated floating-point units, so they had to do what a contemporary embedded systems engineer would call floating point emulation, at great expense. Better to avoid a more expensive comparisons when you can. -- Peter Geoghegan -- 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] Prohibit row-security + inheritance in 9.4?
On 01/31/2014 09:01 AM, Stephen Frost wrote: I don't see where this follows at all- clearly, you already get a subset of rows from the child than if you queried the parent because there are other children. Er, what? I don't see what you're saying here. Currently, when you query the parent, you see rows from other children (superset). You don't ever _not_ see rows from the child that you would see when querying the child directly. If you are first playing with inheritance in PG then it might seem odd for that to be the case. Ditto for what you describe where the child returns more rows than the parent, but these things need to simply be documented as this is how it works for those cases where both are reasonable possibilities and we need to pick one. I'm increasingly inclined to agree. Everything else is too messy, and creates inflexible limitations for users. Personally, I don't see the suggestion that we filter rows accessed via the child based on quals of the parent as making any sense. Neither do I; that (point 4, original post) was pretty much a way to make the other approaches look better by comparison ;-) I feel the same about applying child quals when querying through the parent as we don't apply GRANT permissions that way. Using the parent and using the child are two different paths by which to access the data and which you are using is what drives what you will see. That's a reasonable way to explain it, and consistent with the privileges model already used for inheritance. Is there a case which can't be implemented if the two are independent as I am describing? There are cases which can NOT be implemented if we force the two paths to be handled identically but I feel the approach where we keep them independently managed is flexible to allow the other cases if people want them. The only case prevented is one where access to the child via the parent shows rows that the parent's row-security qual would hide, because the child's qual doesn't. Personally I think that's ugly anyway; I don't want to support that, and have only been looking at it because it'd solve the consistency issues. Since the user can achieve this with: SELECT ... FROM ONLY parent UNION ALL SELECT ... FROM ONLY child1 I think it's fine to just apply the parent qual to all children. There's another bit of fun too: If you have a policy on a child, and query the child via the parent, should the child policy be applied? No! We do not do that for GRANT and I do not see doing it for row security either. If we're approaching this as different entry point, different policy, that makes sense, and I'm increasingly pesuaded by that view of things. Treating row-security checks as permission checks, that'd make this consistent. The difference is that you get a nice error telling you what's going on currently, not a potentially WTF-y different resultset. I understand where you're coming from but this strikes me as a documentation/definition issue and not really a cause for concern or against POLA. These are complex and important topics that anyone who cares about security needs to understand. I'm happy with that. It's clear that there isn't going to be any way to do this that doesn't result in _some_ kind of surprising behaviour, so it's just a matter of being clear about where the astonishment lies. So what we're talking about here (conveniently, exactly what's currently impemented) is to: Apply the policy of the relation actually named in the query before inheritance expansion. If the relation has children expanded during planning, allow the parent policy to be copied to those children. The children are _not_ checked for their own row-security policies when the appendrel is created, and any child policies are _not_ applied. That's consistent with how security barrier views (and views in general) work, and it means that the policy on a relation is applied consistently to all rows, including rows from child relations. As we discussed, it's also consistent with relation-level GRANTs for access to relations. The trade-off is that it creates inconsistent views of the contents of the data depending on whether you query via a parent, or query a child directly, and it means that child policies are ignored when querying via a parent relation. Since that's what I've already written, I'm happy with that ;-) -- Craig Ringer http://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] Shave a few instructions from child-process startup sequence
On 1/20/14, 8:08 PM, Alvaro Herrera wrote: Peter Eisentraut escribió: src/backend/postmaster/postmaster.c:2255: indent with spaces. +else src/backend/postmaster/postmaster.c:2267: indent with spaces. +break; I just checked the Jenkins page for this patch: http://pgci.eisentraut.org/jenkins/job/postgresql_commitfest_world/243/ just to make sure I can figure out what it means. You reported it as build unstable in the commitfest entry: https://commitfest.postgresql.org/action/patch_view?id=1277 However, looking at Jenkins, I couldn't figure out what the problem is. In this case, it was the whitespace violation. (Yeah, I'm constantly debating with myself whether it's worth reporting that, but at the moment I'm still on the side of the fence that wants to make people submit clean patches.) In general, it's sometimes a bit hard to find out what caused the build to fail. Jenkins can detect and report that for standard tools (e.g., compiler warnings, JUnit test results), but not for our custom test tools. Another issue is that the build is running with make -k, so the issue could be somewhere in the middle of the build log. I'm exploring new plugins to improve that, as it's a significant problem. -- 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] [Review] inherit support for foreign tables
(2014/01/28 22:01), Etsuro Fujita wrote: (2014/01/27 21:49), Shigeru Hanada wrote: Is it too big change that making ANALYZE command to handle foreign tables too even if no table name was specified? IIRC, performance issue was the reason to exclude foreign tables from auto-analyze processing. ANALYZEing large database contains local huge data also takes long time. One idea to avoid unexpected long processing is to add option to foreign tables to mark it as not-auto-analyzable. Maybe I didn't express my idea clearly. Sorry for that. I don't think that we now allow the ANALYZE command to handle foreign tables when no table name is specified with the command. I think that we allow the ANALYZE command to handle an inheritance tree that includes foreign tables when the name of the parent table is specified, without ignoring such foreign tables in the caluculation. ISTM it would be possible to do so if we introduce a new parameter, say, vac_mode, which indicates wether vacuum() is called with a specific table or not. I'll try to modify the ANALYZE command to do so on top of you patch. Done on top of your patch, foreign_inherit.patch, not the latest version, foreign_inherit-v2.patch. As I proposed, an inheritance tree that includes foreign tables is now ANALYZEd, without ignoring such foreign tables in the inherited-stats computation, if the name of the parent table is specified with the ANALYZE command. (That has been done by a small modification of analyze.c, thanks to [1].) The ANALYZE command with no tablename or the autovacuum worker skips the inherited-stats computation itself for inheritance trees that includes foreign tables, which is different from the original patch. To distinguish the ANALYZE-with-a-tablename command from the others (ie, the ANALYZE-with-no-tablename command or the autovacuum worker), I've introduced a new parameter, vacmode, in vacuum(), and thus called analyze_rel() with that parameter. Attached is the modified patch. Could you review the patch? Thanks, [1] http://www.postgresql.org/message-id/e1sgfoo-0006zf...@gemulon.postgresql.org Best regards, Etsuro Fujita *** a/doc/src/sgml/ddl.sgml --- b/doc/src/sgml/ddl.sgml *** *** 258,263 CREATE TABLE products ( --- 258,274 even if the value came from the default value definition. /para + note +para + Note that constraints can be defined on foreign tables too, but such + constraints are not enforced on insert or update. Those constraints are + assertive, and work only to tell planner that some kind of optimization + such as constraint exclusion can be considerd. This seems useless, but + allows us to use foriegn table as child table (see + xref linkend=ddl-inherit) to off-load to multiple servers. +/para + /note + sect2 id=ddl-constraints-check-constraints titleCheck Constraints/title *** *** 2021,2028 CREATE TABLE capitals ( /para para !In productnamePostgreSQL/productname, a table can inherit from !zero or more other tables, and a query can reference either all rows of a table or all rows of a table plus all of its descendant tables. The latter behavior is the default. For example, the following query finds the names of all cities, --- 2032,2039 /para para !In productnamePostgreSQL/productname, a table or foreign table can !inherit from zero or more other tables, and a query can reference either all rows of a table or all rows of a table plus all of its descendant tables. The latter behavior is the default. For example, the following query finds the names of all cities, *** a/doc/src/sgml/ref/alter_foreign_table.sgml --- b/doc/src/sgml/ref/alter_foreign_table.sgml *** *** 42,47 ALTER FOREIGN TABLE [ IF EXISTS ] replaceable class=PARAMETERname/replaceab --- 42,49 ALTER [ COLUMN ] replaceable class=PARAMETERcolumn_name/replaceable SET ( replaceable class=PARAMETERattribute_option/replaceable = replaceable class=PARAMETERvalue/replaceable [, ... ] ) ALTER [ COLUMN ] replaceable class=PARAMETERcolumn_name/replaceable RESET ( replaceable class=PARAMETERattribute_option/replaceable [, ... ] ) ALTER [ COLUMN ] replaceable class=PARAMETERcolumn_name/replaceable OPTIONS ( [ ADD | SET | DROP ] replaceable class=PARAMETERoption/replaceable ['replaceable class=PARAMETERvalue/replaceable'] [, ... ]) + INHERIT replaceable class=PARAMETERparent_table/replaceable + NO INHERIT replaceable class=PARAMETERparent_table/replaceable OWNER TO replaceable class=PARAMETERnew_owner/replaceable OPTIONS ( [ ADD | SET | DROP ] replaceable class=PARAMETERoption/replaceable ['replaceable class=PARAMETERvalue/replaceable'] [, ... ]) /synopsis *** *** 178,183 ALTER FOREIGN TABLE [ IF EXISTS ] replaceable class=PARAMETERname/replaceab --- 180,205 /varlistentry
[HACKERS] Regarding google summer of code
Hello everyone, My name is Anirudh Subramanian and I am a graduate student in Computer Science. I would like to participate in Google Summer of Code and would like to contribute to postgresql. I am not familiar with the postgresql codebase yet but will surely get started in the near future. Right now as part of my coursework I am building Is there any list of project ideas that have been decided for Google Summer of Code 2014 that I can look at. Cheers, Anirudh Subramanian
Re: [HACKERS] Regarding google summer of code
Hi Anirudh, On Fri, Jan 31, 2014 at 12:23 PM, Anirudh anirudh2...@gmail.com wrote: Hello everyone, My name is Anirudh Subramanian and I am a graduate student in Computer Science. I would like to participate in Google Summer of Code and would like to contribute to postgresql. I am not familiar with the postgresql codebase yet but will surely get started in the near future. Right now as part of my coursework I am building Is there any list of project ideas that have been decided for Google Summer of Code 2014 that I can look at. Cheers, Check out this page: http://www.postgresql.org/developer/summerofcode/ -- Amit -- 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] pgindent behavior we could do without
On Thu, Jan 30, 2014 at 01:52:55PM -0500, Bruce Momjian wrote: On Thu, Jan 30, 2014 at 01:46:34PM -0500, Tom Lane wrote: Bruce Momjian br...@momjian.us writes: On Thu, Jul 18, 2013 at 12:27:21AM -0400, Tom Lane wrote: I assert that we should simply remove both of these bits of code, as just about every committer on the project is smarter about when to use vertical whitespace than this program is. OK, I have developed the attached patch that shows the code change of removing the test for #else/#elif/#endif. You will see that the new output has odd blank lines for cases like: #ifndef WIN32 static intcopy_file(const char *fromfile, const char *tofile, bool force); -- #else static intwin32_pghardlink(const char *src, const char *dst); -- #endif Hm. So actually, that code is trying to undo excess vertical whitespace that something earlier in pgindent added? Where is that happening? I am afraid it is _inside_ BSD indent, and if ever looked at that code, you would not want to go in there. :-O OK, seven hours later, I have fixed pg_bsd_indent to no longer insert blank lines above #elif/#else/#endif, and therefore removed the special case code from pgindent. You will need to download version 1.3 of pg_bsd_indent for this to work, and pgindent will complain if it doesn't find the right pg_bsd_indent version. Do we need to go an clean up any places where pgindent has suppressed blank lines above #elif/#else/#endif in the past? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] Issue with PGC_BACKEND parameters
On Fri, Jan 31, 2014 at 2:01 AM, Andrew Dunstan and...@dunslane.net wrote: On 01/30/2014 03:12 PM, Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: On 12/22/2013 11:30 PM, Amit Kapila wrote: - * backend start. + * backend start. However for windows, we need to process + * config file during backend start for non-default parameters, + * so we need to allow change of PGC_BACKEND during backend + * startup. */ -if (IsUnderPostmaster) +if (IsUnderPostmaster !IsInitProcessingMode()) return -1; } I think this change looks OK. The comment is pretty awful, since this is neither Windows-specific nor a read of the config file. Perhaps more like However, in EXEC_BACKEND builds we load nondefault settings from the CONFIG_EXEC_PARAMS file during backend start. In that situation we should accept PGC_SIGHUP settings, so as to have the same value as if we'd forked from the postmaster. Changed as per suggestion. Also, I think that the extra test should only be made #ifdef EXEC_BACKEND, so as to minimize the risk of breaking things. Agreed and changed the patch as per suggestion. Not that this isn't pretty darn fragile anyway; I think testing IsInitProcessingMode here is a very random way to detect this case. I wonder if it'd be better to pass down an explicit flag indicating that we're doing read_nondefault_variables(). My first idea was to add a parameter, but set_config_option is getting called from multiple places and this case doesn't seem to be generic enough to add a parameter to commonly used function, so I found another way of doing it. I agree that adding a new parameter would be a better fix, but just seeing the places from where it get called, I thought of doing it other way, however if you feel strongly about it, I can change the patch to pass a new parameter to set_config_option(). If we don't do that, maybe an Assert(IsInitProcessingMode()) in read_nondefault_variables() would be a good thing. Added Assert in read_nondefault_variables(). OK, I've added your comment to the commitfest item and marked it as Waiting on Author. Thanks for Review. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com set_guc_backend_params_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
