Re: [HACKERS] [PROPOSAL] Temporal query processing with range types
On Fri, Oct 6, 2017 at 1:22 PM, Paul A Jungwirth wrote: > On Fri, Jul 22, 2016 at 4:15 AM, Anton Dignös wrote: >> We would like to contribute to PostgreSQL a solution that supports the query >> processing of "at each time point". The basic idea is to offer two new >> operators, NORMALIZE and ALIGN, whose purpose is to adjust (or split) the >> ranges of tuples so that subsequent queries can use the usual grouping and >> equality conditions to get the intended results. > > I just wanted to chime in and say that the work these people have done > is *amazing*. I read two of their papers yesterday [1, 2], and if you > are interested in temporal data, I encourage you to read them too. The > first one is only 12 pages and quite readable. After that the second > is easy because it covers a lot of the same ground but adds "scaling" > of values when a tuple is split, and some other interesting points. > Their contributions could be used to implement SQL:2011 syntax but go > way beyond that. > I've also been following this feature with great interest, and would definitely throw whatever tiny weight I have, sitting out here in the the peanut gallery, behind accepting the ALIGN and NORMALIZE syntax. I estimate that about a third of the non-trivial queries in the primary project I work on (and have, on Postgres, for the last 13+ years) would be simpler with support of the proposed syntax, and some of the most complex business logic would be simplified nearly to the point of triviality. Anyway, that's my $0.02. Thank you, Anton and Peter! -- Mike -- 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] [BUGS] BUG #14682: row level security not work with partitioned table
On Wed, Jun 7, 2017 at 9:49 AM, Mike Palmiotto wrote: > One thing that concerns me is the first EXPLAIN plan from regress_rls_dave: > +EXPLAIN (COSTS OFF) SELECT * FROM part_document WHERE f_leak(dtitle); > + QUERY PLAN > + > + Append > + InitPlan 1 (returns $0) > + -> Index Scan using uaccount_pkey on uaccount > + Index Cond: (pguser = CURRENT_USER) > + -> Seq Scan on part_document_fiction > + Filter: ((cid <> 55) AND (cid <> 55) AND (cid < 99) AND > (dlevel <= $0) AND f_leak(dtitle)) > + -> Seq Scan on part_document_satire > + Filter: ((cid <> 55) AND (cid <> 55) AND (cid < 99) AND > (dlevel <= $0) AND f_leak(dtitle)) > +(8 rows) > > I would expect that both part_document_satire (cid == 55) and > part_document_nonfiction (cid == 99) would be excluded from the > explain, but only cid < 99 seems to work. Interestingly, when I change > policy pp1r to cid < 55, I see the following: > > +EXPLAIN (COSTS OFF) SELECT * FROM part_document WHERE f_leak(dtitle); > +QUERY PLAN > +--- > + Append > + InitPlan 1 (returns $0) > + -> Index Scan using uaccount_pkey on uaccount > + Index Cond: (pguser = CURRENT_USER) > + -> Seq Scan on part_document_fiction > + Filter: ((cid < 55) AND (cid <> 55) AND (cid < 99) AND > (dlevel <= $0) AND f_leak(dtitle)) > +(6 rows) > > Is this a demonstration of a non-immutable function backing the > operator and thus not being able to filter it from the planner, or is > it a bug? Assuming my digging is correct, there's some other explanation for this not working as expected... postgres=> select po.oprname, pp.proname, pp.provolatile from pg_proc pp join pg_operator po on pp.proname::text = po.oprcode::text where po.oprname = '<>' and pp.proname like 'int%ne'; oprname | proname | provolatile -+-+- <> | int4ne | i <> | int2ne | i <> | int24ne | i <> | int42ne | i <> | int8ne | i <> | int84ne | i <> | int48ne | i <> | interval_ne | i <> | int28ne | i <> | int82ne | i (10 rows) Thoughts? Thanks, -- Mike Palmiotto Software Engineer Crunchy Data Solutions https://crunchydata.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] [BUGS] BUG #14682: row level security not work with partitioned table
On Tue, Jun 6, 2017 at 9:12 PM, Michael Paquier wrote: > On Wed, Jun 7, 2017 at 9:52 AM, Joe Conway wrote: >> Thanks Mike. I'll take a close look to verify output correctnes, but I >> am concerned that the new tests are unnecessarily complex. Any other >> opinions on that? > > Some tests would be good to have. Now, if I read those regression > tests correctly, this is using 10 relations where two would be enough, > one as the parent relation and one as a partition. Then policies apply > on the parent relation. The same kind of policy is defined 4 times, > and there is bloat with GRANT and ALTER TABLE commands. I ended up narrowing it down to 4 tables (one parent and 3 partitions) in order to demonstrate policy sorting and order of RLS/partition constraint checking. It should be much more straight-forward now, but let me know if there are any further recommended changes. One thing that concerns me is the first EXPLAIN plan from regress_rls_dave: +EXPLAIN (COSTS OFF) SELECT * FROM part_document WHERE f_leak(dtitle); + QUERY PLAN + + Append + InitPlan 1 (returns $0) + -> Index Scan using uaccount_pkey on uaccount + Index Cond: (pguser = CURRENT_USER) + -> Seq Scan on part_document_fiction + Filter: ((cid <> 55) AND (cid <> 55) AND (cid < 99) AND (dlevel <= $0) AND f_leak(dtitle)) + -> Seq Scan on part_document_satire + Filter: ((cid <> 55) AND (cid <> 55) AND (cid < 99) AND (dlevel <= $0) AND f_leak(dtitle)) +(8 rows) I would expect that both part_document_satire (cid == 55) and part_document_nonfiction (cid == 99) would be excluded from the explain, but only cid < 99 seems to work. Interestingly, when I change policy pp1r to cid < 55, I see the following: +EXPLAIN (COSTS OFF) SELECT * FROM part_document WHERE f_leak(dtitle); +QUERY PLAN +--- + Append + InitPlan 1 (returns $0) + -> Index Scan using uaccount_pkey on uaccount + Index Cond: (pguser = CURRENT_USER) + -> Seq Scan on part_document_fiction + Filter: ((cid < 55) AND (cid <> 55) AND (cid < 99) AND (dlevel <= $0) AND f_leak(dtitle)) +(6 rows) Is this a demonstration of a non-immutable function backing the operator and thus not being able to filter it from the planner, or is it a bug? > > +SELECT * FROM part_document; > + did | cid | dlevel | dauthor | dtitle > +-+-++---+- > + 1 | 11 | 1 | regress_rls_bob | my first novel > Adding an "ORDER BY did" as well here would make the test output more > predictable. Done. Thanks, -- Mike Palmiotto Software Engineer Crunchy Data Solutions https://crunchydata.com From 8c55045bd2d856fe4707582de0270c26d3a4c285 Mon Sep 17 00:00:00 2001 From: Mike Palmiotto Date: Wed, 24 May 2017 16:54:49 + Subject: [PATCH] Add RLS support to partitioned tables This is needed to get RLS policies to apply to the parent partitioned table. Without this change partitioned tables are skipped. --- src/backend/rewrite/rewriteHandler.c | 3 +- src/test/regress/expected/rowsecurity.out | 425 ++ src/test/regress/sql/rowsecurity.sql | 148 +++ 3 files changed, 575 insertions(+), 1 deletion(-) diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index 35ff8bb..6cd73c1 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -1835,7 +1835,8 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown) /* Only normal relations can have RLS policies */ if (rte->rtekind != RTE_RELATION || - rte->relkind != RELKIND_RELATION) + (rte->relkind != RELKIND_RELATION && + rte->relkind != RELKIND_PARTITIONED_TABLE)) continue; rel = heap_open(rte->relid, NoLock); diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out index 7bf2936..792d24e 100644 --- a/src/test/regress/expected/rowsecurity.out +++ b/src/test/regress/expected/rowsecurity.out @@ -899,6 +899,431 @@ EXPLAIN (COSTS OFF) SELECT * FROM t1 WHERE f_leak(b); Filter: f_leak(b) (7 rows) +-- +-- Partitioned Tables +-- +SET SESSION AUTHORIZATION regress_rls_alice; +CREATE TABLE part_document ( +did int, +cid int, +dlevel int not null, +dauthor name, +dtitle text +) PARTITION BY RANGE (cid); +GRANT ALL ON part_document TO public; +-- Create partitions for document categories +CREATE TABLE part_document_fiction
Re: [HACKERS] [BUGS] BUG #14682: row level security not work with partitioned table
On Tue, Jun 6, 2017 at 4:07 PM, Joe Conway wrote:
> On 06/06/2017 11:57 AM, Mike Palmiotto wrote:
>> On Mon, Jun 5, 2017 at 10:36 AM, Robert Haas wrote:
>>> On Mon, Jun 5, 2017 at 10:20 AM, Joe Conway wrote:
>>>> Unless Robert objects, I'll work with Mike to get a fix posted and
>>>> committed in the next day or two.
>>>
>>> That would be great. Thanks.
>>
>> I have the updated patch with rowsecurity regression tests and rebased
>> on master. I've run these and verified locally by feeding
>> rowsecurity.sql to psql, but have yet to get the full regression suite
>> passing -- it's failing on the constraints regtest and then gets stuck
>> in recovery. Undoubtedly something to do with my
>> configuration/environment over here. I'm working through those issues
>> right now. In the meantime, if you want to see the regression tests as
>> they stand, please see the attached patch.
>
> The constraints test passes here, so presumably something you borked
> locally. I only see a rowsecurity failure, which is not surprising since
> your patch does not include the changes to expected output ;-)
> Please resend with src/test/regress/expected/rowsecurity.out included.
It was indeed an issue on my end. Attached are the rowsecurity
regression tests and the expected out. Unsurprisingly, all tests pass,
because I said so. :)
Let me know if you want me to make any revisions.
Thanks,
--
Mike Palmiotto
Software Engineer
Crunchy Data Solutions
https://crunchydata.com
From 08432d93ed753a1e5cd4585ccf00e900abbd685f Mon Sep 17 00:00:00 2001
From: Mike Palmiotto
Date: Wed, 24 May 2017 16:54:49 +
Subject: [PATCH] Add RLS support to partitioned tables
This is needed to get RLS policies to apply to the parent partitioned table.
Without this change partitioned tables are skipped.
---
src/backend/rewrite/rewriteHandler.c | 3 +-
src/test/regress/expected/rowsecurity.out | 812 ++
src/test/regress/sql/rowsecurity.sql | 222
3 files changed, 1036 insertions(+), 1 deletion(-)
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 35ff8bb..6cd73c1 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -1835,7 +1835,8 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
/* Only normal relations can have RLS policies */
if (rte->rtekind != RTE_RELATION ||
- rte->relkind != RELKIND_RELATION)
+ (rte->relkind != RELKIND_RELATION &&
+ rte->relkind != RELKIND_PARTITIONED_TABLE))
continue;
rel = heap_open(rte->relid, NoLock);
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index 7bf2936..1e35498 100644
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -899,6 +899,818 @@ EXPLAIN (COSTS OFF) SELECT * FROM t1 WHERE f_leak(b);
Filter: f_leak(b)
(7 rows)
+--
+-- Partitioned Tables
+--
+SET SESSION AUTHORIZATION regress_rls_alice;
+CREATE TABLE part_category (
+cidint primary key,
+cname text
+);
+GRANT ALL ON part_category TO public;
+INSERT INTO part_category VALUES
+(11, 'fiction'),
+(55, 'satire'),
+(99, 'nonfiction');
+CREATE TABLE part_document (
+did int,
+cid int,
+dlevel int not null,
+dauthor name,
+dtitle text
+) PARTITION BY RANGE (cid);
+GRANT ALL ON part_document TO public;
+-- Create partitions for document categories
+CREATE TABLE part_document_fiction (
+ LIKE part_document INCLUDING ALL
+) PARTITION BY RANGE (dlevel);
+CREATE TABLE part_document_satire (
+ LIKE part_document INCLUDING ALL
+) PARTITION BY RANGE (dlevel);
+CREATE TABLE part_document_nonfiction (
+ LIKE part_document INCLUDING ALL
+) PARTITION BY RANGE (dlevel);
+ALTER TABLE part_document ATTACH PARTITION part_document_fiction FOR VALUES FROM ('11') TO ('12');
+ALTER TABLE part_document ATTACH PARTITION part_document_satire FOR VALUES FROM ('55') TO ('56');
+ALTER TABLE part_document ATTACH PARTITION part_document_nonfiction FOR VALUES FROM ('99') TO ('100');
+-- Create partitions for document levels
+CREATE TABLE part_document_fiction_1 (LIKE part_document_fiction INCLUDING ALL);
+CREATE TABLE part_document_fiction_2 (LIKE part_document_fiction INCLUDING ALL);
+CREATE TABLE part_document_satire_1 (LIKE part_document_satire INCLUDING ALL);
+CREATE TABLE part_document_satire_2 (LIKE part_document_satire INCLUDING ALL);
+CREATE TABLE part_document_nonfiction_1 (LIKE part_document_nonfiction INCLUDING ALL);
+CREATE TABLE part_document_nonfiction_2 (LIKE part_document_nonfiction INCLUDING ALL);
+GRANT ALL ON part_document_fiction_1 TO p
Re: [HACKERS] [BUGS] BUG #14682: row level security not work with partitioned table
On Mon, Jun 5, 2017 at 10:36 AM, Robert Haas wrote:
> On Mon, Jun 5, 2017 at 10:20 AM, Joe Conway wrote:
>> Unless Robert objects, I'll work with Mike to get a fix posted and
>> committed in the next day or two.
>
> That would be great. Thanks.
I have the updated patch with rowsecurity regression tests and rebased
on master. I've run these and verified locally by feeding
rowsecurity.sql to psql, but have yet to get the full regression suite
passing -- it's failing on the constraints regtest and then gets stuck
in recovery. Undoubtedly something to do with my
configuration/environment over here. I'm working through those issues
right now. In the meantime, if you want to see the regression tests as
they stand, please see the attached patch.
Thanks,
--
Mike Palmiotto
Software Engineer
Crunchy Data Solutions
https://crunchydata.com
From 48a9586881872d4b8c9ca77e0c0da48db611e326 Mon Sep 17 00:00:00 2001
From: Mike Palmiotto
Date: Wed, 24 May 2017 16:54:49 +
Subject: [PATCH] Add RLS support to partitioned tables
This is needed to get RLS policies to apply to the parent partitioned table.
Without this change partitioned tables are skipped.
---
src/backend/rewrite/rewriteHandler.c | 3 +-
src/test/regress/sql/rowsecurity.sql | 223 +++
2 files changed, 225 insertions(+), 1 deletion(-)
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 35ff8bb..6cd73c1 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -1835,7 +1835,8 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
/* Only normal relations can have RLS policies */
if (rte->rtekind != RTE_RELATION ||
- rte->relkind != RELKIND_RELATION)
+ (rte->relkind != RELKIND_RELATION &&
+ rte->relkind != RELKIND_PARTITIONED_TABLE))
continue;
rel = heap_open(rte->relid, NoLock);
diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql
index 1b6896e..c4ba136 100644
--- a/src/test/regress/sql/rowsecurity.sql
+++ b/src/test/regress/sql/rowsecurity.sql
@@ -44,6 +44,7 @@ GRANT EXECUTE ON FUNCTION f_leak(text) TO public;
-- BASIC Row-Level Security Scenario
SET SESSION AUTHORIZATION regress_rls_alice;
+
CREATE TABLE uaccount (
pguser name primary key,
seclv int
@@ -308,6 +309,228 @@ SET row_security TO OFF;
SELECT * FROM t1 WHERE f_leak(b);
EXPLAIN (COSTS OFF) SELECT * FROM t1 WHERE f_leak(b);
+--
+-- Partitioned Tables
+--
+
+SET SESSION AUTHORIZATION regress_rls_alice;
+
+CREATE TABLE part_category (
+cidint primary key,
+cname text
+);
+GRANT ALL ON part_category TO public;
+INSERT INTO part_category VALUES
+(11, 'fiction'),
+(55, 'satire'),
+(99, 'nonfiction');
+
+CREATE TABLE part_document (
+did int,
+cid int,
+dlevel int not null,
+dauthor name,
+dtitle text
+) PARTITION BY RANGE (cid);
+GRANT ALL ON part_document TO public;
+
+-- Create partitions for document categories
+CREATE TABLE part_document_fiction (
+ LIKE part_document INCLUDING ALL
+) PARTITION BY RANGE (dlevel);
+
+CREATE TABLE part_document_satire (
+ LIKE part_document INCLUDING ALL
+) PARTITION BY RANGE (dlevel);
+
+CREATE TABLE part_document_nonfiction (
+ LIKE part_document INCLUDING ALL
+) PARTITION BY RANGE (dlevel);
+
+ALTER TABLE part_document ATTACH PARTITION part_document_fiction FOR VALUES FROM ('11') TO ('12');
+ALTER TABLE part_document ATTACH PARTITION part_document_satire FOR VALUES FROM ('55') TO ('56');
+ALTER TABLE part_document ATTACH PARTITION part_document_nonfiction FOR VALUES FROM ('99') TO ('100');
+
+-- Create partitions for document levels
+CREATE TABLE part_document_fiction_1 (LIKE part_document_fiction INCLUDING ALL);
+CREATE TABLE part_document_fiction_2 (LIKE part_document_fiction INCLUDING ALL);
+CREATE TABLE part_document_satire_1 (LIKE part_document_satire INCLUDING ALL);
+CREATE TABLE part_document_satire_2 (LIKE part_document_satire INCLUDING ALL);
+CREATE TABLE part_document_nonfiction_1 (LIKE part_document_nonfiction INCLUDING ALL);
+CREATE TABLE part_document_nonfiction_2 (LIKE part_document_nonfiction INCLUDING ALL);
+
+GRANT ALL ON part_document_fiction_1 TO public;
+GRANT ALL ON part_document_fiction_2 TO public;
+GRANT ALL ON part_document_satire_1 TO public;
+GRANT ALL ON part_document_satire_2 TO public;
+GRANT ALL ON part_document_nonfiction_1 TO public;
+GRANT ALL ON part_document_nonfiction_2 TO public;
+
+ALTER TABLE part_document_fiction ATTACH PARTITION part_document_fiction_1 FOR VALUES FROM ('1') TO ('2');
+ALTER TABLE part_document_fiction ATTACH PARTITION part_document_fiction_2 FOR VALUES FROM ('2') TO ('3');
+ALTER TABLE part_document_sat
Re: [HACKERS] partitioned tables and contrib/sepgsql
On Thu, Apr 6, 2017 at 5:52 PM, Joe Conway wrote: > On 04/06/2017 12:35 PM, Tom Lane wrote: >> Joe Conway writes: >>> Any thoughts on whether 0001a and 0001b ought to be backpatched? I'm >>> thinking not given the lack of past complaints but it might make sense >>> to do. >> >> I think 0001a absolutely needs to be, because it is fixing what is really >> an ABI violation: sepgsql_needs_fmgr_hook is supposed to return our notion >> of bool, but as things stand it's returning _Bool (which is why the >> compiler is complaining). Now we might get away with that on most >> hardware, but on platforms where those are different widths, it's possible >> to imagine function-return conventions that would make it fail. >> >> 0001b seems to only be needed for compilers that aren't smart enough >> to see that tclass won't be referenced for RELKIND_INDEX, so it's >> just cosmetic. > > Ok, committed/pushed that way. > > I found some missing bits in the 0002 patch -- new version attached. > Will wait on new regression tests before committing, but I expect we'll > have those by end of today and be able to commit the rest tomorrow. Attached are the regression test updates for partitioned tables. Thanks, -- Mike Palmiotto Software Engineer Crunchy Data Solutions https://crunchydata.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] partitioned tables and contrib/sepgsql
On Wed, Apr 5, 2017 at 1:22 PM, Mike Palmiotto
wrote:
> On Wed, Apr 5, 2017 at 1:19 PM, Tom Lane wrote:
>> Peter Eisentraut writes:
>>> On 4/5/17 12:04, Tom Lane wrote:
>>>> Conclusion: Fedora's gcc is playing fast and loose somehow with the
>>>> command "#include "; that does not include the file
>>>> you'd think it does, it does something magic inside the compiler.
>>>> The magic evidently includes not complaining about duplicate macro
>>>> definitions for true and false.
>>
>>> Perhaps -Wsystem-headers would change the outcome in your case.
>>
>> Hah, you're right: with that,
>>
>> In file included from /usr/include/selinux/label.h:9:0,
>> from label.c:40:
>> /usr/lib/gcc/x86_64-redhat-linux/6.3.1/include/stdbool.h:34:0: warning:
>> "true" redefined
>> #define true 1
>>
>> In file included from ../../src/include/postgres.h:47:0,
>> from label.c:11:
>> ../../src/include/c.h:206:0: note: this is the location of the previous
>> definition
>> #define true ((bool) 1)
>>
>> and likewise for "false". So that mystery is explained.
>>
>> I stand by my previous patch suggestion, except that we can replace
>> the parenthetical comment with something like "(We don't care if
>> redefines "true"/"false"; those are close enough.)".
>>
>
> Sounds good. Updated patchset will include that verbiage, along with
> some regression tests for partitioned tables.
Looks like the label regression test is failing even without these changes.
Weirdly enough, this only happens in one place:
84 SELECT objtype, objname, label FROM pg_seclabels
85 WHERE provider = 'selinux' AND objtype = 'column' AND (objname
like 't3.%' OR objname like 't4.%');
I haven't figured out why this may be (yet), but it seems like we
shouldn't assume the order of output from a SELECT. As I understand
it, the order of the output is subject to change with changes to the
planner/configuration.
I'm going to hold the partition table regression changes for a
separate patch and include some ORDER BY fixes. Will post tomorrow
In the meantime, attached are the latest and greatest patches.
Thanks,
--
Mike Palmiotto
Software Engineer
Crunchy Data Solutions
https://crunchydata.com
From 50969159f0fd7c93a15bfb7b9046512399d0b099 Mon Sep 17 00:00:00 2001
From: Mike Palmiotto
Date: Wed, 29 Mar 2017 14:59:37 +
Subject: [PATCH 3/3] Add partitioned table support to sepgsql
Account for RELKIND_PARTITIONED_RELATIONS in sepgsql and treat the objects like
regular relations. This allows for proper object_access hook behavior for
partitioned tables.
---
contrib/sepgsql/label.c| 3 ++-
contrib/sepgsql/relation.c | 30 +-
2 files changed, 23 insertions(+), 10 deletions(-)
diff --git a/contrib/sepgsql/label.c b/contrib/sepgsql/label.c
index f7d79ab..1a1ec57 100644
--- a/contrib/sepgsql/label.c
+++ b/contrib/sepgsql/label.c
@@ -789,7 +789,8 @@ exec_object_restorecon(struct selabel_handle * sehnd, Oid catalogId)
case RelationRelationId:
relForm = (Form_pg_class) GETSTRUCT(tuple);
-if (relForm->relkind == RELKIND_RELATION)
+if (relForm->relkind == RELKIND_RELATION ||
+ relForm->relkind == RELKIND_PARTITIONED_TABLE)
objtype = SELABEL_DB_TABLE;
else if (relForm->relkind == RELKIND_SEQUENCE)
objtype = SELABEL_DB_SEQUENCE;
diff --git a/contrib/sepgsql/relation.c b/contrib/sepgsql/relation.c
index b794abe..0d8905c 100644
--- a/contrib/sepgsql/relation.c
+++ b/contrib/sepgsql/relation.c
@@ -54,12 +54,14 @@ sepgsql_attribute_post_create(Oid relOid, AttrNumber attnum)
ObjectAddress object;
Form_pg_attribute attForm;
StringInfoData audit_name;
+ char relkind;
/*
* Only attributes within regular relation have individual security
* labels.
*/
- if (get_rel_relkind(relOid) != RELKIND_RELATION)
+ relkind = get_rel_relkind(relOid);
+ if (relkind != RELKIND_RELATION && relkind != RELKIND_PARTITIONED_TABLE)
return;
/*
@@ -135,8 +137,10 @@ sepgsql_attribute_drop(Oid relOid, AttrNumber attnum)
{
ObjectAddress object;
char *audit_name;
+ char relkind;
- if (get_rel_relkind(relOid) != RELKIND_RELATION)
+ relkind = get_rel_relkind(relOid);
+ if (relkind != RELKIND_RELATION && relkind != RELKIND_PARTITIONED_TABLE)
return;
/*
@@ -167,8 +171,11 @@ sepgsql_attribute_relabel(Oid relOid, AttrNumber attnum,
{
ObjectAddress object;
char *audit_name;
+ char relkind;
- if (get_rel_relkind(relOid) != RELKIND_RELATION)
+ relkind = get_rel_relkind(relOid);
+
+ if (relkind != RELKIND_RELATION && relkind != RELKIND_PARTITIONED_TABLE)
ereport
Re: [HACKERS] partitioned tables and contrib/sepgsql
On Wed, Apr 5, 2017 at 1:19 PM, Tom Lane wrote: > Peter Eisentraut writes: >> On 4/5/17 12:04, Tom Lane wrote: >>> Conclusion: Fedora's gcc is playing fast and loose somehow with the >>> command "#include "; that does not include the file >>> you'd think it does, it does something magic inside the compiler. >>> The magic evidently includes not complaining about duplicate macro >>> definitions for true and false. > >> Perhaps -Wsystem-headers would change the outcome in your case. > > Hah, you're right: with that, > > In file included from /usr/include/selinux/label.h:9:0, > from label.c:40: > /usr/lib/gcc/x86_64-redhat-linux/6.3.1/include/stdbool.h:34:0: warning: > "true" redefined > #define true 1 > > In file included from ../../src/include/postgres.h:47:0, > from label.c:11: > ../../src/include/c.h:206:0: note: this is the location of the previous > definition > #define true ((bool) 1) > > and likewise for "false". So that mystery is explained. > > I stand by my previous patch suggestion, except that we can replace > the parenthetical comment with something like "(We don't care if > redefines "true"/"false"; those are close enough.)". > Sounds good. Updated patchset will include that verbiage, along with some regression tests for partitioned tables. Thanks, -- Mike Palmiotto Software Engineer Crunchy Data Solutions https://crunchydata.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] partitioned tables and contrib/sepgsql
On Tue, Apr 4, 2017 at 10:19 AM, Joe Conway wrote:
> On 04/04/2017 06:45 AM, Robert Haas wrote:
>> On Mon, Apr 3, 2017 at 12:02 PM, Joe Conway wrote:
>>>> 0002 looks extremely straightforward, but I wonder if we could get one
>>>> of the people on this list who knows about sepgsql to have a look?
>>>> (Stephen Frost, Joe Conway, KaiGai Kohei...)
>>>
>>> Will have a look later today.
>>
>> I think it is now tomorrow, unless your time zone is someplace very
>> far away. :-)
>
> OBE -- I have scheduled time in 30 minutes from now, after I have gotten
> my first cup of coffee ;-)
After some discussion off-list, I've rebased and udpated the patches.
Please see attached for further review.
Thanks,
--
Mike Palmiotto
Software Engineer
Crunchy Data Solutions
https://crunchydata.com
From be692f8cc94d74033494d140c310e932c705e785 Mon Sep 17 00:00:00 2001
From: Mike Palmiotto
Date: Wed, 29 Mar 2017 14:59:37 +
Subject: [PATCH 2/2] Add partitioned table support to sepgsql
Account for RELKIND_PARTITIONED_RELATIONS in sepgsql and treat the objects like
regular relations. This allows for proper object_access hook behavior for
partitioned tables.
---
contrib/sepgsql/label.c| 3 ++-
contrib/sepgsql/relation.c | 33 +++--
2 files changed, 25 insertions(+), 11 deletions(-)
diff --git a/contrib/sepgsql/label.c b/contrib/sepgsql/label.c
index 8a72503..c66581a 100644
--- a/contrib/sepgsql/label.c
+++ b/contrib/sepgsql/label.c
@@ -787,7 +787,8 @@ exec_object_restorecon(struct selabel_handle * sehnd, Oid catalogId)
case RelationRelationId:
relForm = (Form_pg_class) GETSTRUCT(tuple);
-if (relForm->relkind == RELKIND_RELATION)
+if (relForm->relkind == RELKIND_RELATION ||
+ relForm->relkind == RELKIND_PARTITIONED_TABLE)
objtype = SELABEL_DB_TABLE;
else if (relForm->relkind == RELKIND_SEQUENCE)
objtype = SELABEL_DB_SEQUENCE;
diff --git a/contrib/sepgsql/relation.c b/contrib/sepgsql/relation.c
index 4dc48a0..4fcebc1 100644
--- a/contrib/sepgsql/relation.c
+++ b/contrib/sepgsql/relation.c
@@ -54,12 +54,14 @@ sepgsql_attribute_post_create(Oid relOid, AttrNumber attnum)
ObjectAddress object;
Form_pg_attribute attForm;
StringInfoData audit_name;
+ char relkind;
/*
* Only attributes within regular relation have individual security
* labels.
*/
- if (get_rel_relkind(relOid) != RELKIND_RELATION)
+ relkind = get_rel_relkind(relOid);
+ if (relkind != RELKIND_RELATION && relkind != RELKIND_PARTITIONED_TABLE)
return;
/*
@@ -135,8 +137,10 @@ sepgsql_attribute_drop(Oid relOid, AttrNumber attnum)
{
ObjectAddress object;
char *audit_name;
+ char relkind;
- if (get_rel_relkind(relOid) != RELKIND_RELATION)
+ relkind = get_rel_relkind(relOid);
+ if (relkind != RELKIND_RELATION && relkind != RELKIND_PARTITIONED_TABLE)
return;
/*
@@ -167,8 +171,11 @@ sepgsql_attribute_relabel(Oid relOid, AttrNumber attnum,
{
ObjectAddress object;
char *audit_name;
+ char relkind;
- if (get_rel_relkind(relOid) != RELKIND_RELATION)
+ relkind = get_rel_relkind(relOid);
+
+ if (relkind != RELKIND_RELATION && relkind != RELKIND_PARTITIONED_TABLE)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot set security label on non-regular columns")));
@@ -209,8 +216,11 @@ sepgsql_attribute_setattr(Oid relOid, AttrNumber attnum)
{
ObjectAddress object;
char *audit_name;
+ char relkind;
+
+ relkind = get_rel_relkind(relOid);
- if (get_rel_relkind(relOid) != RELKIND_RELATION)
+ if (relkind != RELKIND_RELATION && relkind != RELKIND_PARTITIONED_TABLE)
return;
/*
@@ -290,6 +300,7 @@ sepgsql_relation_post_create(Oid relOid)
switch (classForm->relkind)
{
+ case RELKIND_PARTITIONED_TABLE:
case RELKIND_RELATION:
tclass = SEPG_CLASS_DB_TABLE;
break;
@@ -336,7 +347,8 @@ sepgsql_relation_post_create(Oid relOid)
true);
/*
- * Assign the default security label on the new relation
+ * Assign the default security label on the new relation or partitioned
+ * table.
*/
object.classId = RelationRelationId;
object.objectId = relOid;
@@ -344,10 +356,10 @@ sepgsql_relation_post_create(Oid relOid)
SetSecurityLabel(&object, SEPGSQL_LABEL_TAG, rcontext);
/*
- * We also assigns a default security label on columns of the new regular
- * tables.
+ * We also assign a default security label on columns of a new table.
*/
- if (classForm->relkind == RELKIND_RELATION)
+ if (classForm->relkind == RELKIND_RELATION ||
+ classForm->relkind == RELKIND_PARTITIONED_TABLE)
{
Relation arel;
ScanKeyData akey;
@@ -422,6 +434,7 @@ sepgsql_relation_drop(Oid relOid)
relkind = get_rel_relkind(relOid);
switch (relkind)
{
+ case RELKIND_PARTITIONED_TABLE:
case RELKIND_RELATION:
tclass = SEPG_CLASS_DB_TABLE
Re: [HACKERS] partitioned tables and contrib/sepgsql
On Fri, Mar 31, 2017 at 8:23 PM, Robert Haas wrote: > On Fri, Mar 31, 2017 at 2:14 PM, Mike Palmiotto > wrote: >> Attached you will find two patches, which were rebased on master as of >> 156d388 (applied with `git am --revert [patch file]`). The first gets >> rid of some pesky compiler warnings and the second implements the >> sepgsql handling of partitioned tables. > > 0001 has the problem that we have a firm rule against putting any > #includes whatsoever before "postgres.h". This stdbool issue has come > up before, though, and I fear we're going to need to do something > about it. Yeah, I recall this rule. The only things I can really think of to solve the problem are: 1) #define _STDBOOL_H in postgres's c.h once bool and the like have been defined, so we can avoid re-definition. 2) Enforce that any code utilizing the stdbool header manage the re-definition with some combination of #undef/#define/#typedef and document the issue somewhere. I'd be more inclined to believe 2) is the correct solution, since 1) is more of a hack than anything. Thoughts? > > 0002 looks extremely straightforward, but I wonder if we could get one > of the people on this list who knows about sepgsql to have a look? > (Stephen Frost, Joe Conway, KaiGai Kohei...) I welcome any and all feedback. Thanks, -- Mike Palmiotto Software Engineer Crunchy Data Solutions https://crunchydata.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] partitioned tables and contrib/sepgsql
On Fri, Mar 31, 2017 at 2:14 PM, Mike Palmiotto wrote: > On Mon, Mar 27, 2017 at 12:09 PM, Mike Palmiotto > wrote: >> On Mon, Mar 27, 2017 at 11:46 AM, Robert Haas wrote: >>> >>> Note that sepgsql hasn't been updated to work with RLS yet, either, >>> but we didn't regard that as an open item for RLS, or if we did the >>> resolution was just to document it. I am not opposed to giving a >>> little more time to get this straightened out, but if a patch doesn't >>> show up fairly soon then I think we should just document that sepgsql >>> doesn't support partitioned tables in v10. sepgsql has a fairly >>> lengthy list of implementation restrictions already, so one more is >>> not going to kill anybody -- or if it will then that person should >>> produce a patch soon. >> >> Okay, I'll make sure I get something fleshed out today or tomorrow. > > Apologies for the delay. I was waffling over whether to reference > PartitionedRelationId in sepgsql, but ended up deciding to just handle > RELKIND_PARTITIONED_TABLE and treat the classOid as > RelationRelationId. Seeing as there is a relid in pg_class which > corresponds to the partitioned table, this chosen route seemed > acceptable. Newest patches remove cruft from said waffling. No need to include pg_partitioned_table.h if we're not referencing PartitionedRelationId. > > Here is a demonstration of the partitioned table working with sepgsql hooks: > https://gist.github.com/anonymous/b10f476a95ae9cdd39b83ef872d4b1e6 > > Attached you will find two patches, which were rebased on master as of > 156d388 (applied with `git am --revert [patch file]`). The first gets > rid of some pesky compiler warnings and the second implements the > sepgsql handling of partitioned tables. That should have read `git am --reject [patch file]`. Apologies for the inaccuracy. Thanks, -- Mike Palmiotto Software Engineer Crunchy Data Solutions https://crunchydata.com From 82ff9a4e18d3baefa5530b8515e46cf8225519de Mon Sep 17 00:00:00 2001 From: Mike Palmiotto Date: Tue, 28 Mar 2017 16:44:54 + Subject: [PATCH 1/2] Silence some sepgsql compiler warnings selinux/label.h includes stdbool.h, which redefines the bool type and results in a warning: assignment from incompatible pointer type for sepgsql_fmgr_hook. Move selinux/label.h above postgres.h, so the bool type is properly defined. Additionally, sepgsql throws compiler warnings due to possibly uninitialized tclass in code paths for indexes. Set tclass to a bogus -1 to silence these warnings. --- contrib/sepgsql/label.c| 4 ++-- contrib/sepgsql/relation.c | 8 ++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/contrib/sepgsql/label.c b/contrib/sepgsql/label.c index 1a8f884..a404cd7 100644 --- a/contrib/sepgsql/label.c +++ b/contrib/sepgsql/label.c @@ -8,6 +8,8 @@ * * - */ +#include + #include "postgres.h" #include "access/heapam.h" @@ -37,8 +39,6 @@ #include "sepgsql.h" -#include - /* * Saved hook entries (if stacked) */ diff --git a/contrib/sepgsql/relation.c b/contrib/sepgsql/relation.c index ab98a9b..fd6fe8b 100644 --- a/contrib/sepgsql/relation.c +++ b/contrib/sepgsql/relation.c @@ -300,8 +300,10 @@ sepgsql_relation_post_create(Oid relOid) tclass = SEPG_CLASS_DB_VIEW; break; case RELKIND_INDEX: - /* deal with indexes specially; no need for tclass */ + /* other indexes are handled specially below; set tclass to -1 to + * silence compiler warning */ sepgsql_index_modify(relOid); + tclass = -1; goto out; default: /* ignore other relkinds */ @@ -432,7 +434,9 @@ sepgsql_relation_drop(Oid relOid) /* ignore indexes on toast tables */ if (get_rel_namespace(relOid) == PG_TOAST_NAMESPACE) return; - /* other indexes are handled specially below; no need for tclass */ + /* other indexes are handled specially below; set tclass to -1 to + * silence compiler warning */ + tclass = -1; break; default: /* ignore other relkinds */ -- 2.7.4 From b3a44aa37b5724ea88fb0d1110ba18be6618e283 Mon Sep 17 00:00:00 2001 From: Mike Palmiotto Date: Wed, 29 Mar 2017 14:59:37 + Subject: [PATCH 2/2] Add partitioned table support to sepgsql Account for RELKIND_PARTITIONED_RELATIONS in sepgsql and treat the objects like regular relations. This allows for proper create/alter/drop hook behavior for partitioned tables. --- contrib/sepgsql/label.c| 3 ++- contrib/sepgsql/relation.c | 33 +++-- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/contrib/sepgsql/label.c b/contrib/sepgsql/label.c index a404cd7..88a04a4 100644 --- a/contrib/sepgsql/label.c +++ b/contrib/sepgsql/label.c @@ -779,7 +779,8 @@ ex
Re: [HACKERS] partitioned tables and contrib/sepgsql
On Mon, Mar 27, 2017 at 12:09 PM, Mike Palmiotto
wrote:
> On Mon, Mar 27, 2017 at 11:46 AM, Robert Haas wrote:
>>
>> Note that sepgsql hasn't been updated to work with RLS yet, either,
>> but we didn't regard that as an open item for RLS, or if we did the
>> resolution was just to document it. I am not opposed to giving a
>> little more time to get this straightened out, but if a patch doesn't
>> show up fairly soon then I think we should just document that sepgsql
>> doesn't support partitioned tables in v10. sepgsql has a fairly
>> lengthy list of implementation restrictions already, so one more is
>> not going to kill anybody -- or if it will then that person should
>> produce a patch soon.
>
> Okay, I'll make sure I get something fleshed out today or tomorrow.
Apologies for the delay. I was waffling over whether to reference
PartitionedRelationId in sepgsql, but ended up deciding to just handle
RELKIND_PARTITIONED_TABLE and treat the classOid as
RelationRelationId. Seeing as there is a relid in pg_class which
corresponds to the partitioned table, this chosen route seemed
acceptable.
Here is a demonstration of the partitioned table working with sepgsql hooks:
https://gist.github.com/anonymous/b10f476a95ae9cdd39b83ef872d4b1e6
Attached you will find two patches, which were rebased on master as of
156d388 (applied with `git am --revert [patch file]`). The first gets
rid of some pesky compiler warnings and the second implements the
sepgsql handling of partitioned tables.
Thanks,
--
Mike Palmiotto
Software Engineer
Crunchy Data Solutions
https://crunchydata.com
From 06463a4545c1cd0a2740e201d06a36b78dc2da8c Mon Sep 17 00:00:00 2001
From: Mike Palmiotto
Date: Wed, 29 Mar 2017 14:59:37 +
Subject: [PATCH 2/2] Add partitioned table support to sepgsql
Account for RELKIND_PARTITIONED_RELATIONS in sepgsql and treat the objects like
regular relations. This allows for proper create/alter/drop hook behavior for
partitioned tables.
---
contrib/sepgsql/hooks.c| 1 +
contrib/sepgsql/label.c| 4 +++-
contrib/sepgsql/relation.c | 34 --
3 files changed, 28 insertions(+), 11 deletions(-)
diff --git a/contrib/sepgsql/hooks.c b/contrib/sepgsql/hooks.c
index 93cc8de..89e71e3 100644
--- a/contrib/sepgsql/hooks.c
+++ b/contrib/sepgsql/hooks.c
@@ -15,6 +15,7 @@
#include "catalog/pg_class.h"
#include "catalog/pg_database.h"
#include "catalog/pg_namespace.h"
+#include "catalog/pg_partitioned_table.h"
#include "catalog/pg_proc.h"
#include "commands/seclabel.h"
#include "executor/executor.h"
diff --git a/contrib/sepgsql/label.c b/contrib/sepgsql/label.c
index a404cd7..546 100644
--- a/contrib/sepgsql/label.c
+++ b/contrib/sepgsql/label.c
@@ -23,6 +23,7 @@
#include "catalog/pg_class.h"
#include "catalog/pg_database.h"
#include "catalog/pg_namespace.h"
+#include "catalog/pg_partitioned_table.h"
#include "catalog/pg_proc.h"
#include "commands/dbcommands.h"
#include "commands/seclabel.h"
@@ -779,7 +780,8 @@ exec_object_restorecon(struct selabel_handle * sehnd, Oid catalogId)
case RelationRelationId:
relForm = (Form_pg_class) GETSTRUCT(tuple);
-if (relForm->relkind == RELKIND_RELATION)
+if (relForm->relkind == RELKIND_RELATION ||
+ relForm->relkind == RELKIND_PARTITIONED_TABLE)
objtype = SELABEL_DB_TABLE;
else if (relForm->relkind == RELKIND_SEQUENCE)
objtype = SELABEL_DB_SEQUENCE;
diff --git a/contrib/sepgsql/relation.c b/contrib/sepgsql/relation.c
index fd6fe8b..93022d4 100644
--- a/contrib/sepgsql/relation.c
+++ b/contrib/sepgsql/relation.c
@@ -19,6 +19,7 @@
#include "catalog/pg_attribute.h"
#include "catalog/pg_class.h"
#include "catalog/pg_namespace.h"
+#include "catalog/pg_partitioned_table.h"
#include "commands/seclabel.h"
#include "lib/stringinfo.h"
#include "utils/builtins.h"
@@ -54,12 +55,14 @@ sepgsql_attribute_post_create(Oid relOid, AttrNumber attnum)
ObjectAddress object;
Form_pg_attribute attForm;
StringInfoData audit_name;
+ char relkind;
/*
* Only attributes within regular relation have individual security
* labels.
*/
- if (get_rel_relkind(relOid) != RELKIND_RELATION)
+ relkind = get_rel_relkind(relOid);
+ if (relkind != RELKIND_RELATION && relkind != RELKIND_PARTITIONED_TABLE)
return;
/*
@@ -135,8 +138,10 @@ sepgsql_attribute_drop(Oid relOid, AttrNumber attnum)
{
ObjectAddress object;
char *audit_name;
+ char relkind;
- if (get_rel_relkind(relOid) != RELKIND_RELATION)
+ relkind = get_rel_relkind(relOid);
+ if (relkind != RELKIND_RELATION && relkind != RELKIND_PARTITIONED_TABLE)
return;
/*
@@ -167,8 +172,11 @@ sepgsql_attribut
Re: [HACKERS] partitioned tables and contrib/sepgsql
On Mon, Mar 27, 2017 at 11:46 AM, Robert Haas wrote: > On Mon, Mar 27, 2017 at 11:22 AM, Mike Palmiotto > wrote: >> On Mon, Mar 27, 2017 at 10:47 AM, Robert Haas wrote: >>> On Thu, Mar 9, 2017 at 9:47 AM, Stephen Frost wrote: >>>> While going over the contrib modules, I noticed that sepgsql was not >>>> updated for partitioned tables. What that appears to mean is that it's >>>> not possible to define labels on partitioned tables. >>> >>> It works for me: >>> >>> rhaas=# load 'dummy_seclabel'; >>> LOAD >>> rhaas=# create table foo (a int, b text) partition by range (a); >>> CREATE TABLE >>> rhaas=# security label on table foo is 'classified'; >>> SECURITY LABEL >>> >>> What exactly is the problem you're seeing? >> >> IIRC the initial concern was that contrib/sepgsql was not updated for >> RELKIND_PARTITIONED_TABLE, so the post-create hook would not work >> properly for partitioned tables. >> >> I've looked into it a bit and saw a post-alter hook in >> StoreCatalogInheritance1. It seems like it may just be an issue of >> adding the RELKIND_PARTITIONED_TABLE to sepgsql_relation_post_create. > > I thought that kind of thing might be the issue, but it didn't seem to > match Stephen's description. Adding RELKIND_PARTITIONED_TABLE to that > function seems like it would probably be sufficient to make that hook > work, although that would need to be tested, but there are numerous > other references to RELKIND_RELATION in contrib/sepgsql, some of which > probably also need to be updated to consider > RELKIND_PARTITIONED_TABLE. Agreed. > > In my view, this is really an enhancement to sepgsql to handle a new > PostgreSQL feature rather than a defect in the partitioning commit, so > I don't think it falls on Amit Langote (as the author) or me (as the > committer) to fix. There might similarly be updates to sepgsql to do > something with permissions on logical replication's new publication > and subscription objects, but we should similarly regard those as > possible new features, not things that Petr or Peter need to work on. I'll keep these items in mind for future reference. Thanks. > Note that sepgsql hasn't been updated to work with RLS yet, either, > but we didn't regard that as an open item for RLS, or if we did the > resolution was just to document it. I am not opposed to giving a > little more time to get this straightened out, but if a patch doesn't > show up fairly soon then I think we should just document that sepgsql > doesn't support partitioned tables in v10. sepgsql has a fairly > lengthy list of implementation restrictions already, so one more is > not going to kill anybody -- or if it will then that person should > produce a patch soon. Okay, I'll make sure I get something fleshed out today or tomorrow. -- Mike Palmiotto Software Engineer Crunchy Data Solutions https://crunchydata.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] partitioned tables and contrib/sepgsql
On Mon, Mar 27, 2017 at 10:47 AM, Robert Haas wrote: > On Thu, Mar 9, 2017 at 9:47 AM, Stephen Frost wrote: >> While going over the contrib modules, I noticed that sepgsql was not >> updated for partitioned tables. What that appears to mean is that it's >> not possible to define labels on partitioned tables. > > It works for me: > > rhaas=# load 'dummy_seclabel'; > LOAD > rhaas=# create table foo (a int, b text) partition by range (a); > CREATE TABLE > rhaas=# security label on table foo is 'classified'; > SECURITY LABEL > > What exactly is the problem you're seeing? IIRC the initial concern was that contrib/sepgsql was not updated for RELKIND_PARTITIONED_TABLE, so the post-create hook would not work properly for partitioned tables. I've looked into it a bit and saw a post-alter hook in StoreCatalogInheritance1. It seems like it may just be an issue of adding the RELKIND_PARTITIONED_TABLE to sepgsql_relation_post_create. -- Mike Palmiotto Software Engineer Crunchy Data Solutions https://crunchydata.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Misleading bgworker log message
A handful of rather surprising errors showed up in our log extract this morning, along the lines of: 2017-03-17 05:01:55 CDT [5400]: [1-1] @ FATAL: 57P01: terminating connection due to administrator command After a moment of more than a little astonishment, a look at the full log revealed that, while the message implies some type of intervention from a human , what's actually happening is the termination of parallel query worker threads due to a statement timeout. I gather this is happening because the same mechanism is used to cancel the process in both cases. If that's indeed the case, I wonder if it's possible to re phrase the termination message so it does not suggest an admin was involved. __ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * *
Re: [HACKERS] partitioned tables and contrib/sepgsql
On Thu, Mar 9, 2017 at 9:47 AM, Stephen Frost wrote: > Greetings, > > While going over the contrib modules, I noticed that sepgsql was not > updated for partitioned tables. What that appears to mean is that it's > not possible to define labels on partitioned tables. As I recall, > accessing the parent of a table will, similar to the GRANT system, not > perform checkes against the child tables, meaning that there's no way to > have SELinux checks properly enforced when partitioned tables are being > used. I'll start taking a look at this. Presumably we'd just extend existing object_access_hooks to cover partitioned tables? > > This is an issue which should be resolved for PG10, so I'll add it to > the open items list. I'll grab it. Thanks. --Mike -- 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] removing tsearch2
There is also a mechanism for the results of the Perl module's "make test" to be reported to a site which aggregates and reports them by Perl version and OS - a sort of distributed build farm. See for example http://matrix.cpantesters.org/?dist=DBD-Pg+3.5.3 __ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * * On Mon, Feb 27, 2017 at 4:02 PM, David E. Wheeler wrote: > On Feb 27, 2017, at 1:53 PM, Bruce Momjian wrote: > > > Oh, does CPAN distribute compiled modules or requires users to compile > > them. > > Like PGXN, it formally does not care, but its implementation expects > source code distributions what will be built and installed by users. Note > that the vast majority of those modules, -- even pure Perl modules -- are > built with make. > > So users typically get their Perl modules in one of these ways: > > 1. As binaries from their distribution’s package manager. These tend to be > updated manually by volunteers and not integrated into CPAN, though there > are solutions such as [rpmcpan](https://github.com/iovation/rpmcpan) and > [PPM](http://www.activestate.com/activeperl/ppm-perl-modules) which do > regular distro package builds. > > 2. As source code from CPAN, from which they are compiled (when > necessary), built, and installed by the user or a build system such as > [Homebrew](https://brew.sh). > > Best, > > David > >
Re: [HACKERS] application_name in process name?
On Sun, Jul 17, 2016 at 10:34 AM, Tom Lane wrote: > It occurs to me that we could also remove the update_process_title GUC: > what you would do is configure a process_title pattern that doesn't > include the %-escape for current command tag, and the infrastructure > could notice that that escape isn't present and skip unnecessary updates. > The same kind of trick could be used for other potentially-expensive > items like the lock "waiting" flag. > This seems like an interesting project for learning my way around gucs and logging. Could you elaborate a little on the cost considerations?
[HACKERS] application_name in process name?
There are times when it would be useful to have the application_name connection parameter displayed in the process name - and thus in ps and pg_top - in addition to the user and database name. Would there be any downside to this? If it were done, are there any suggestions on how it could be added the process name so as to minimize impact on anything that might be trying to parse that line? __ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * *
Re: [HACKERS] Fuzzy substring searching with the pg_trgm extension
On Thu, Feb 11, 2016 at 8:11 AM, Teodor Sigaev wrote: >> I have attached a new version of the patch. It fixes error of operators >> <->> and >> %>: >> - operator <->> did not pass the regression test in CentOS 32 bit (gcc >> 4.4.7 >> 20120313). >> - operator %> did not pass the regression test in FreeBSD 32 bit (gcc >> 4.2.1 >> 20070831). >> >> It was because of variable optimization by gcc. > > > Fixed with volatile modifier, right? > > I'm close to push this patches, but I still doubt in names, and I'd like to > see comment from English speackers: > 1 sml_limit GUC variable (options: similarity_limit, sml_threshold) > 2 subword_similarity(). Actually, it finds most similar word (not > substring!) from whole string. word_similarity? word_in_string_similarity? > At least for this English speaker, substring_similarity is not confusing even if it's not internally accurate, but English is a strange language. Because I want the bike shed to be blue, how does query_string_similarity sound instead? If that's overly precise, then word_similarity would be fine with me. Thanks, -- Mike Rylander -- 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] old bug in full text parser
On Wed, Feb 10, 2016 at 4:28 AM, Oleg Bartunov wrote:
> It looks like there is a very old bug in full text parser (somebody pointed
> me on it), which appeared after moving tsearch2 into the core. The problem
> is in how full text parser process hyphenated words. Our original idea was
> to report hyphenated word itself as well as its parts and ignore hyphen.
> That was how tsearch2 works.
>
> This behaviour was changed after moving tsearch2 into the core:
> 1. hyphen now reported by parser, which is useless.
> 2. Hyphenated words with numbers ('4-dot', 'dot-4') processed differently
> than ones with plain text words like 'four-dot', no hyphenated word itself
> reported.
>
> I think we should consider this as a bug and produce fix for all supported
> versions.
>
The Evergreen project has long depended on tsearch2 (both as an
extension and in-core FTS), and one thing we've struggled with is date
range parsing such as birth and death years for authors in the form of
1979-2014, for instance. Strings like that end up being parsed as two
lexems, "1979" and "-2014". We work around this by pre-normalizing
strings matching /(\d+)-(\d+)/ into two numbers separated by a space
instead of a hyphen, but if fixing this bug would remove the need for
such a preprocessing step it would be a great help to us. Would such
strings be parsed "properly" into lexems of the form of "1979" and
"2014" with you proposed change?
Thanks!
--
Mike Rylander
> After investigation we found this commit:
>
> commit 73e6f9d3b61995525785b2f4490b465fe860196b
> Author: Tom Lane
> Date: Sat Oct 27 19:03:45 2007 +
>
> Change text search parsing rules for hyphenated words so that digit
> strings
> containing decimal points aren't considered part of a hyphenated word.
> Sync the hyphenated-word lookahead states with the subsequent
> part-by-part
> reparsing states so that we don't get different answers about how much
> text
> is part of the hyphenated word. Per my gripe of a few days ago.
>
>
> 8.2.23
>
> select tok_type, description, token from ts_debug('dot-four');
> tok_type | description | token
> -+---+--
> lhword | Latin hyphenated word | dot-four
> lpart_hword | Latin part of hyphenated word | dot
> lpart_hword | Latin part of hyphenated word | four
> (3 rows)
>
> select tok_type, description, token from ts_debug('dot-4');
> tok_type | description | token
> -+---+---
> hword | Hyphenated word | dot-4
> lpart_hword | Latin part of hyphenated word | dot
> uint| Unsigned integer | 4
> (3 rows)
>
> select tok_type, description, token from ts_debug('4-dot');
> tok_type | description| token
> --+--+---
> uint | Unsigned integer | 4
> lword| Latin word | dot
> (2 rows)
>
> 8.3.23
>
> select alias, description, token from ts_debug('dot-four');
> alias | description | token
> -+-+--
> asciihword | Hyphenated word, all ASCII | dot-four
> hword_asciipart | Hyphenated word part, all ASCII | dot
> blank | Space symbols | -
> hword_asciipart | Hyphenated word part, all ASCII | four
> (4 rows)
>
> select alias, description, token from ts_debug('dot-4');
>alias | description | token
> ---+-+---
> asciiword | Word, all ASCII | dot
> int | Signed integer | -4
> (2 rows)
>
> select alias, description, token from ts_debug('4-dot');
>alias | description| token
> ---+--+---
> uint | Unsigned integer | 4
> blank | Space symbols| -
> asciiword | Word, all ASCII | dot
> (3 rows)
>
>
> Regards,
> Oleg
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: In-core regression tests for replication, cascading, archiving, PITR, etc.
On Thu, Nov 19, 2015 at 10:05 AM, Robert Haas wrote: > On Wed, Nov 18, 2015 at 10:21 AM, Alvaro Herrera > wrote: > > In my days of Perl, it was starting to become frowned upon to call > > subroutines without parenthesizing arguments. Is that no longer the > > case? > As I understand it, there are several reasons not to make function calls in Perl without parenthesis. Whether they are good reasons is a question for the user. Modern Perl <http://onyxneon.com/books/modern_perl/> chapter 5 covers most of them. ______ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * *
Re: [HACKERS] Foreign Data Wrapper
Inspecting the multicorn source was helpful.
For the purposes of anyone searching the mailing lists, I needed to access
the quals natively. The code that helped me figure out how to do this was
in the native odbc fdw here:
https://github.com/ZhengYang/odbc_fdw/blob/master/odbc_fdw.c
particularly the odbcBeginForeignScan() and odbcIterateForeignScan(), and
odbcGetQual() functions.
Thanks
On Fri, Nov 13, 2015 at 1:24 PM, Corey Huinker
wrote:
> On Fri, Nov 13, 2015 at 1:46 PM, Big Mike wrote:
>
>> Writing a Foreign Data Wrapper and interested in isolating the WHERE
>> clause to speed up the access of an indexed file on my filesystem. I'm
>> attempting to understand the inner workings of how the data is retrieved so
>> I'm writing code to just handle one case at the moment: WHERE clause on a
>> single column in the foreign 'table'.
>>
>> SELECT * FROM t WHERE testval = 1
>>
>> I have this code so far, an implementation of the IterateForeignScan
>> interface.
>>
>> static TupleTableSlot *
>> bIterateForeignScan(ForeignScanState *node) {
>> ...
>> RestrictInfo *rinfo = (RestrictInfo *)node->ss.ps.qual;
>> ...
>> }
>>
>> yet am not familiar with what I need to do to pick apart RestrictInfo in
>> order to gather 'testvar', '=', and '1' separately so I can interpret and
>> pass those through to my file parser.
>>
>> Am I going about this the correct way or is there another path I should
>> follow?
>>
>
>
> I would look at http://multicorn.org/ which gives you a working python
> framework. You subclass their ForeignDataWrapper class, override the
> __init__() and execute() functions, and that's about it. The execute()
> function has a list called quals that would set you up for the filtering
> you want to do.
>
> I would get the foreign data wrapper fully debugged this way before
> attempting to refactor in C. And when you do finally re-code, you can see
> what multicorn itself did to implement your filters.
>
>
>
>
>
>
>
>
[HACKERS] warning: HS_KEY redefined (9.5 beta2)
Google says this was present in beta1. ( http://www.postgresql.org/message-id/5596a162.30...@dunslane.net) Still seems broken, at least for me. Built with Perl 5.22. uname -m = x86_64 uname -r = 2.6.32-504.12.2.el6.x86_64 __ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * *
[HACKERS] Foreign Data Wrapper
Writing a Foreign Data Wrapper and interested in isolating the WHERE clause
to speed up the access of an indexed file on my filesystem. I'm attempting
to understand the inner workings of how the data is retrieved so I'm
writing code to just handle one case at the moment: WHERE clause on a
single column in the foreign 'table'.
SELECT * FROM t WHERE testval = 1
I have this code so far, an implementation of the IterateForeignScan
interface.
static TupleTableSlot *
bIterateForeignScan(ForeignScanState *node) {
...
RestrictInfo *rinfo = (RestrictInfo *)node->ss.ps.qual;
...
}
yet am not familiar with what I need to do to pick apart RestrictInfo in
order to gather 'testvar', '=', and '1' separately so I can interpret and
pass those through to my file parser.
Am I going about this the correct way or is there another path I should
follow?
Re: [HACKERS] Within CF app, "Bug Fixes" should be "Bug Fixes/Refactoring"
On Thu, Oct 29, 2015 at 2:45 PM, Michael Paquier wrote: > On Thu, Oct 29, 2015 at 8:33 PM, Peter Geoghegan wrote: > > I think that within the CF app, we should either rename the patch > > topic "Bug Fixes" to "Bug Fixes/Refactoring", or introduce a new > > "Refactoring" topic. I prefer the first approach. > > I would vote for the second approach, with a separate category for > refactoring. > > So if a bug fix involved some refactoring, which would you put it under? Or would you expect separate CF entries for the refactoring and the fix? __ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * *
Re: [HACKERS] perlcritic
David wrote: > I believe there are ways to get perlcritic to keep quiet about things > we don't find relevant. Maybe that's a better way to use it. > There are indeed. A .perlcriticrc file can suppress (or add) either individual rules or groups of rules. I use one to ignore the ones I disagree with, along with the comment form to ignore specific cases. I see perlcritic as functioning for me along the same lines as a style guide, giving a consistency that helps with long term maintainability. It also helps keep me from straying into golf. ^_^
Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.
Something like that would be helpful. I just had to stop one after an hour and have no idea how much longer it would have taken. __ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * * On Fri, Jul 24, 2015 at 5:41 PM, Josh Berkus wrote: > On 07/24/2015 11:06 AM, Jim Nasby wrote: > > On 7/23/15 5:18 AM, Thakur, Sameer wrote: > >> Hello, > >>> >logged > 25 times > >> Sorry, it is much lower at 7 times. Does not change overall point though > > > > I think it's related to the problem of figuring out how many dead tuples > > you expect to find in the overall heap, which you need to do to have any > > hope of this being a comprehensive estimate. > > What about just reporting scanned pages/total pages ? That would be > easy and cheap to track. It would result in some herky-jerky > "progress", but would still be an improvement over the feedback we don't > have now. > > -- > Josh Berkus > PostgreSQL Experts Inc. > http://pgexperts.com > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers >
Re: [HACKERS] pg_upgrade + Ubuntu
On Fri, Jul 10, 2015 at 2:10 PM, Alvaro Herrera wrote: > Joshua D. Drake wrote: > > > > On 07/10/2015 11:01 AM, Mike Blackwell wrote: > > >Does pg_config show the correct location? > > Good idea but: > > > > postgres@ly19:~$ pg_config > > You need to install postgresql-server-dev-X.Y for building a server-side > > extension or libpq-dev for building a client-side application. > > > > Which is worse having to install yet another package or having a command > > line option? > > It seems to me that this is a Debian packaging issue, not an upstream > issue, isn't it? If you want to fix the problem in this way, then > surely whatever package contains pg_upgrade should also contain > pg_config. > > Indeed. An interesting packaging choice. I'd think it belongs to an admin category along with pg_upgrade, pg_dump, etc., rather than a development package. Surely it could be useful for admin scripts? __ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/>
Re: [HACKERS] pg_upgrade + Ubuntu
Does pg_config show the correct location? If so, perhaps pg_upgrade could get the .conf location the same way rather than requiring a command line option. __ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * * On Fri, Jul 10, 2015 at 12:40 PM, Joshua D. Drake wrote: > > Hackers, > > Simple problem (I think): > > 9.4 version of pg_upgrade said: > > "/usr/lib/postgresql/9.4/bin/pg_ctl" -w -l "pg_upgrade_server.log" -D > "/var/lib/postgresql/9.4/main" -o "-p 9400 -b -c synchronous_commit=off -c > fsync=off -c full_page_writes=off -c listen_addresses='' -c > unix_socket_permissions=0700 -c > unix_socket_directories='/var/lib/postgresql'" start > > postgres cannot access the server configuration file > "/var/lib/postgresql/9.4/main/postgresql.conf": No such file or directory > > The issue is Debian/Ubuntu etc... don't have a postgresql.conf in the > cluster. They keep it separately in /etc/postgresql. > > Could we get a flag that allows us to specifically point to where the conf > filesare? > > JD > -- > Command Prompt, Inc. - http://www.commandprompt.com/ 503-667-4564 > PostgreSQL Centered full stack support, consulting and development. > Announcing "I'm offended" is basically telling the world you can't > control your own emotions, so everyone else should do it for you. > > > -- > 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] Problems with question marks in operators (JDBC, ECPG, ...)
Ah. I see. Thanks for the clarification. __ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * * On Tue, May 19, 2015 at 1:44 PM, Bruno Harbulot wrote: > > > On Tue, May 19, 2015 at 7:22 PM, Tom Lane wrote: > >> Mike Blackwell writes: >> > See for example >> > http://docs.oracle.com/cd/B19306_01/text.102/b14218/cqoper.htm#i997330, >> > Table 3-1, third row, showing the precedence of '?'. Further down the >> > page, under "Fuzzy" see "Backward Compatibility Syntax". >> >> If I'm reading that right, that isn't a SQL-level operator but an operator >> in their text search query language, which would only appear in SQL >> queries within string literals (compare tsquery's query operators in PG). >> So it wouldn't be a hazard for ?-substitution, as long as the substituter >> was bright enough to not change string literals. >> >> regards, tom lane >> > > That's how I read it too. I've tried this little test: > http://sqlfiddle.com/#!4/7436b/4/0 > > CREATE TABLE test_table ( > id INTEGER PRIMARY KEY, > name VARCHAR(100) > ); > > INSERT INTO test_table (id, name) VALUES (1, 'Nicole'); > INSERT INTO test_table (id, name) VALUES (2, 'Nicholas'); > INSERT INTO test_table (id, name) VALUES (3, 'Robert'); > INSERT INTO test_table (id, name) VALUES (4, 'Michael'); > INSERT INTO test_table (id, name) VALUES (5, 'Nicola'); > > CREATE INDEX idx_test_table_name ON test_table(name) INDEXTYPE IS > CTXSYS.CONTEXT; > > SELECT * FROM test_table WHERE CONTAINS(name, '?Nicolas', 1) > 0; > > > Fuzzy matching works indeed, but the question mark is part of the literal > (similarly to % when using LIKE). > > Best wishes, > > Bruno. > >
Re: [HACKERS] Problems with question marks in operators (JDBC, ECPG, ...)
See for example http://docs.oracle.com/cd/B19306_01/text.102/b14218/cqoper.htm#i997330, Table 3-1, third row, showing the precedence of '?'. Further down the page, under "Fuzzy" see "Backward Compatibility Syntax". __ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * * On Tue, May 19, 2015 at 12:45 PM, Bruno Harbulot < br...@distributedmatter.net> wrote: > > > On Tue, May 19, 2015 at 6:15 PM, Mike Blackwell > wrote: > >> A Google search suggests Oracle 9.x supports a unary '?' operator (fuzzy >> match), so the use of '?' in an operator name is not without precedent. >> >> > Interesting. Do you have any specific link? I'm probably not using the > right Google search, but the nearest reference I've found is for Oracle 10, > and it seems to use the tilde (~) operator for fuzzy matching: > http://www.oracle.com/technetwork/search/oses/overview/new-query-features-in-10-1-8-2-1-132287.pdf > > Best wishes, > > Bruno. >
Re: [HACKERS] Problems with question marks in operators (JDBC, ECPG, ...)
A Google search suggests Oracle 9.x supports a unary '?' operator (fuzzy match), so the use of '?' in an operator name is not without precedent. __ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * * On Tue, May 19, 2015 at 10:03 AM, Bruno Harbulot < br...@distributedmatter.net> wrote: > > > On Tue, May 19, 2015 at 3:23 PM, Kevin Grittner wrote: > >> David G. Johnston wrote: >> > On Mon, May 18, 2015 at 3:31 PM, Bruno Harbulot < >> br...@distributedmatter.net>wrote: >> >> >> In the discussion on the OpenJDK JDBC list two years ago >> >> ( >> http://mail.openjdk.java.net/pipermail/jdbc-spec-discuss/2013-February/50.html >> ), >> >> Lance Andersen said "There is nothing in the SQL standard that >> >> would support the use of an '?' as anything but a parameter >> >> marker.". >> >> > "CREATE OPERATOR is a PostgreSQL extension. There are no >> > provisions for user-defined operators in the SQL standard." >> >> Exactly. The standard specifies the characters to use for the >> predicates that it defines, and provides no mechanism for adding >> additional predicates; but who in the world would want to exclude >> all extensions to the standard? >> > > I was certainly not suggesting custom operators should be excluded. I was > suggesting using something that was actually not incompatible with the SQL > standards (and, even with standards aside, the expectations implementors > have regarding the question mark, since it affects other tools too). > > > >> > And by extension if indeed the standard does require the use of >> > "?" for parameters we are in violation there because the backend >> > protocol deals with $# placeholders and not "?" >> >> We're talking about a different specification that has question >> marks as parameter placeholders. That's in the Java Database >> Connector (JDBC) specification. (It is apparently also specified >> in other documents, although I'm not familiar enough with those to >> comment.) Note that it would create all sorts of pain if both the >> SQL statements and a connector issuing them used the same >> convention for substituting parameters; it is a *good* thing that >> plpgsql and SQL function definitions use a different convention >> than JDBC! >> > > Actually, we were not just talking about JDBC. I don't know the > specifications in details, but the SQL:201x (preliminary) documents linked > from > https://wiki.postgresql.org/wiki/Developer_FAQ#Where_can_I_get_a_copy_of_the_SQL_standards.3F > seem to have some information. The Foundation document (Section 4.25 > Dynamic SQL concepts) says that dynamic parameters are represented by a > question mark. > > In addition, the BNF grammar available at > http://www.savage.net.au/SQL/sql-2003-2.bnf.html#dynamic%20parameter%20specification > also says: > ::= > > I'm not familiar enough with these documents to know whether I'm missing > some context, but it would seem that the question mark is a reserved > character, beyond the scope of JDBC (at the very least, it seems > incompatible with Dynamic SQL and its implementation in ECPG). > > Best wishes, > > Bruno. > >
Re: [HACKERS] xpath changes in the recent back branches
On Thu, Feb 19, 2015 at 5:53 AM, Marko Tiikkaja wrote:
> Hi,
>
> Commit 79af9a1d2668c9edc8171f03c39e7fed571eeb98 changed xpath handling
> with regard to namespaces, and it seems to be fixing an actual issue.
> However, it was also backpatched to all branches despite it breaking for
> example code like this:
>
> do $$
> declare
> _x xml;
> begin
> _x := (xpath('/x:Foo/x:Bar', xml ' xmlns="teh:urn">12',
> array[['x','teh:urn']]))[1];
> raise notice '%', xpath('/Bar/Baz/text()', _x);
> raise notice '%', xpath('/Bar/Bat/text()', _x);
> end
> $$;
>
> The problem is that there's no way to write the code like this in such a
> way that it would work on both versions. If I add the namespace, it's
> broken on 9.1.14. Without it it's broken on 9.1.15.
>
> I'm now thinking of adding a workaround which strips namespaces, but that
> doesn't seem to be easy to do, even with PL/Perl. Is there a better
> workaround here that I'm not seeing?
>
>
FWIW, I've been working around the bug fixed in that commit for ages by
spelling my xpath like this:
xpath('/*[local-name()="Bar"]/*[local-name()="Baz"]/text()', data)
I've modularized my XML handling functions so the source of 'data' is
immaterial -- maybe it's a full document, maybe it's a fragment from a
previous xpath() call -- and the referenced commit is going to make correct
XPATH much more sane, readable, and maintainable. I, for one, welcome it
wholeheartedly.
HTH,
--Mike
Re: [HACKERS] RangeType internal use
On Fri, Feb 13, 2015 at 3:13 PM, Jim Nasby wrote: > > If we exclude the issue of needing one or two oddball partitions for +/- > infinity, I expect that fixed sized partitions would actually cover 80-90% > of cases. That would not be true in our case. The data is not at all evenly distributed over the partitioning key. We would need something more like: values a, b, and c get their own partitions and everything else goes in partition d.
Re: [HACKERS] Proposal: knowing detail of config files via SQL
This would default to being available to superusers only, right? Details of the file system shouldn't be available to any random user. ______ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * * On Fri, Jan 30, 2015 at 9:50 AM, Sawada Masahiko wrote: > On Sat, Jan 31, 2015 at 12:24 AM, David Fetter wrote: > > On Fri, Jan 30, 2015 at 09:38:10PM +0900, Sawada Masahiko wrote: > >> On Tue, Jan 27, 2015 at 3:34 AM, Robert Haas > wrote: > >> > On Thu, Jan 22, 2015 at 5:19 PM, Tom Lane wrote: > >> >> David Johnston writes: > >> >>> On Thu, Jan 22, 2015 at 3:04 PM, Tom Lane > wrote: > >> >>>> Is that a requirement, and if so why? Because this proposal > doesn't > >> >>>> guarantee any such knowledge AFAICS. > >> >> > >> >>> The proposal provides for SQL access to all possible sources of > variable > >> >>> value setting and, ideally, a means of ordering them in priority > order, so > >> >>> that a search for TimeZone would return two records, one for > >> >>> postgresql.auto.conf and one for postgresql.conf - which are > numbered 1 and > >> >>> 2 respectively - so that in looking at that result if the > >> >>> postgresql.auto.conf entry were to be removed the user would know > that what > >> >>> the value is in postgresql.conf that would become active. > Furthermore, if > >> >>> postgresql.conf has a setting AND there is a mapping in an > #included file > >> >>> that information would be accessible via SQL as well. > >> >> > >> >> I know what the proposal is. What I am questioning is the use-case > that > >> >> justifies having us build and support all this extra mechanism. How > often > >> >> does anyone need to know what the "next down" variable value would > be? > >> > > >> > I believe I've run into situations where this would be useful. I > >> > wouldn't be willing to put in the effort to do this myself, but I > >> > wouldn't be prepared to vote against a high-quality patch that someone > >> > else felt motivated to write. > >> > > >> > >> Attached patch adds new view pg_file_settings (WIP version). > >> This view behaves like followings, > >> [postgres][5432](1)=# select * from pg_file_settings ; > >> name| setting | > >> sourcefile > >> > ++ > >> max_connections| 100| > >> /home/masahiko/pgsql/master/3data/postgresql.conf > >> shared_buffers | 128MB | > >> /home/masahiko/pgsql/master/3data/postgresql.conf > > > > This looks great! > > > > Is there a reason not to have the sourcefile as a column in > > pg_settings? > > > > pg_file_settings view also has source file column same as pg_settings. > It might was hard to confirm that column in previous mail. > I put example of pg_file_settings again as follows. > > [postgres][5432](1)=# select * from pg_file_settings where name = > 'work_mem'; > -[ RECORD 1 ]-- > name | work_mem > setting| 128MB > sourcefile | /home/masahiko/pgsql/master/3data/hoge.conf > -[ RECORD 2 ]-- > name | work_mem > setting| 64MB > sourcefile | /home/masahiko/pgsql/master/3data/postgresql.auto.conf > > Regards, > > --- > Sawada Masahiko > > > -- > 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] explain sortorder
Tom, Thanks for the comments on what you ended up changing. It helps point out the kind of things I should be looking for. I'll try to let less slip through in the future. Mike ______ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * * On Mon, Jan 19, 2015 at 10:09 AM, Tom Lane wrote: > "Timmer, Marius" writes: > > We think, you wanted to switch to DESC behavior > > (print out NULLS FIRST) in cases, where > > „USING“ uses an operator which is considered to be > > a DESC operator. > > Right, because that's how addTargetToSortList() would parse it. > > > But get_equality_op_for_ordering_op is called in > > cases, where reverse is false, but > > the part > > if (reverse) > > *reverse = (strategy == BTGreaterStrategyNumber); > > never changes this to true? > > Sorry, not following? It's true that what I added to explain.c doesn't > worry too much about the possibility of get_ordering_op_properties() > failing --- that really shouldn't happen for something that was previously > accepted as a sorting operator. But if it does, "reverse" will just be > left as false, so the behavior will anyway be unsurprising I think. > We could alternatively make it throw a "cache lookup failed" error but > I'm not sure how that makes anyone's life better. > > 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] [PATCH] explain sortorder (fwd)
> > > From: "Timmer, Marius" > > Hi, > > attached is version 8, fixing remaining issues, adding docs and tests as > requested/agreed. > > > Marius & Arne > > This looks good to me. Test coverage seems complete. Doc updates are included. Output format looks like it should be acceptable to Heikki. I'll mark this as ready for committer. Thanks for the patch! Mike
Re: [HACKERS] [PATCH] explain sortorder
V6 of this patch applies, builds and checks against the current HEAD. The areas below could use some attention. In explain.c: malloc() should not be called directly here. palloc() would be the correct call, I believe, but the functions in stringinfo.h are probably your best choice as they remove the necessity for dealing with buffer size and overflow. There is leftover commented out code from the previous patch version in the T_Sort case. In show_sort_group_keys(), the splitting of the existing declaration and initialization of the keyresno and target seems unnecessary and against the style of surrounding code. Multi-line comments should follow the existing format. There are no tests for the "... is LC_COLLATE" and "COLLATE..." cases. Section 14.1 of the documentation may need to be updated. Mike. __ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * * On Wed, Jan 7, 2015 at 10:17 AM, Timmer, Marius < marius.tim...@uni-muenster.de> wrote: > Hi, > > we have spent the last days to realize your suggestions in the patch. > It affects the result of a EXPLAIN-Statement (even in non-verbose-mode). > Now you will get the order-information for every single sort-key which is > not ordered by the defaults. > > > best regards, > > Marius > > > > > --- > Marius Timmer > Zentrum für Informationsverarbeitung > Westfälische Wilhelms-Universität Münster > Einsteinstraße 60 > > mtimm...@uni-muenster.de >
Re: [HACKERS] [PATCH] explain sortorder
Looking forward to the new patch. I'll give it a more complete testing when you post it. Mike ______ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * * On Tue, Dec 23, 2014 at 5:11 AM, Arne Scheffer < arne.schef...@uni-muenster.de> wrote: > >Heikki Linnakangas writes: > >> I would suggest just adding the information to the Sort Key line. As > >> long as you don't print the modifiers when they are defaults (ASC and > >> NULLS LAST), we could print the information even in non-VERBOSE mode. > > >+1. I had assumed without looking that that was what it did already, > >else I'd have complained too. > > > regards, tom lane > > We will change the patch according to Heikkis suggestions. > > A nice Christmas & all the best in the New Year > > Arne Scheffer > > http://www.uni-muenster.de/ZIV/Mitarbeiter/ArneScheffer.shtml >
Re: [HACKERS] Commitfest problems
On Tue, Dec 16, 2014 at 10:15 AM, Mark Cave-Ayland < mark.cave-ayl...@ilande.co.uk> wrote: > > Well as I mentioned in my last email, practically all developers will > rebase and run "make check" on their patched tree before submitting to > the list. Even when this is true, and with people new to the project submitting patches I'm not sure it can be assumed, time passes and things can change between submission and review. I've seen a fair number of "Needs rebase" comments on patches, through no fault of the original submitter.
Re: [HACKERS] [PATCH] explain sortorder
Initial review:
Patch applies cleanly to current head, although it appears to have
soft/hard tab and trailing space issues.
make check fails with the output below. The expected collation clause is
not present.
--
-- Test explain feature: sort order
--
CREATE TABLE sortordertest (n1 char(1), n2 int4);
-- Insert values by which should be ordered
INSERT INTO sortordertest(n1, n2) VALUES ('d', 5), ('b', 3), ('a', 1),
('e', 2), ('c', 4);
-- Display sort order when explain analyze and verbose are true.
EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM sortordertest ORDER BY n1
COLLATE "C" DESC, n2;
QUERY PLAN
Sort
Output: n1, n2, ((n1)::character(1))
Sort Key: sortordertest.n1, sortordertest.n2
Sort Order: ASC NULLS LAST, ASC NULLS LAST
-> Seq Scan on public.sortordertest
Output: n1, n2, n1
(6 rows)
DROP TABLE sortordertest;
__
*Mike Blackwell | Technical Analyst, Distribution Services/Rollout
Management | RR Donnelley*
1750 Wallace Ave | St Charles, IL 60174-3401
Office: 630.313.7818
mike.blackw...@rrd.com
http://www.rrdonnelley.com
Re: [HACKERS] proposal: plpgsql - Assert statement
> > >> On 18 November 2014 21:19, Petr Jelinek wrote: >> >> Personally, I see this as natural extension of the conditional block >>> control >>> which we already have for loops with CONTINUE WHEN and EXIT WHEN. This >>> basically extends it to any block and it seems quite natural to have it >>> for >>> me... >>> >> This seems to me like a step in the direction of APL, where every statement is a conditional. Perl has the option of putting the conditional on the end of a statement as suggested here for ASSERT. My experience has been that while it may "look prettier" to some, the conditional is overlooked in reviews, etc., more often than one would expect, giving a net loss in the overall risk/productivity analysis. As a code maintainer, I would be opposed to adding something like this for no other reason than perceived aesthetics. Your mileage may, of course, vary.
Re: [HACKERS] Proposed changing the definition of decade for date_trunc and extract
On Fri, 2014-08-01 at 22:28 -0700, Mike Swanson wrote:
> I'd also argue that the current function basing the logic from
> definition #2 has limited use even when you want to use it for such.
> If you want to generate text for '(decades)s' you'd have to do:
> SELECT extract('year' from date_trunc('decade', now())) || 's';
> Or with my patch:
> SELECT floor(extract('year' from now()) / 10) || '0s';
> It's different, for sure, but I would actually think the second one is
> a bit less awkward. Plus it's shorter :)
I'm responding to myself because I realized that what I wrote was a bit
silly. The first and current example (which is invalidated by my patch)
should really be:
SELECT extract('decade' from now()) || '0s';
which makes it the shorter and simpler version of the two. I'll still
stand by my opinion that it's not an extremely useful function as it is.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Proposed changing the definition of decade for date_trunc and extract
On Sat, 2014-08-02 at 15:15 +1200, Gavin Flower wrote:
> Since there was no year zero: then it follows that the first decade
> comprises years 1 to 10, and the current Millennium started in 2001 - or
> am I being too logical??? :-)
This is pretty much the reason I'm sending this patch, because it makes
mathematical sense, plus my OCD-sense tingles when Postgres handles
centuries and millenniums correctly, whereas decades are not.
I will concede if the compatibility breaks are too great, but I don't
know how many people depend on the output of this. I didn't do any
market research :) Besides, it seemed to me that if the other two were
able to be fixed (albeit ~10 years ago), there's little reason to avoid
fixing decade too.
There's a few definitions of a decade:
* Spans of ten years that start from year 1.
* Spans of ten years defined by the second-to-the-right digit (years
1-9 would be in decade 0?) -- this is one of the colloquial
versions when people refer to "the (19)90s."
* The other version tends to be less well-defined. "The 1960s"
usually conjures up images of counterculture and the British
Invasion and such; debatably occurring around 1964-1972 (this
version used by culture can never be derived mathematically by a
database, but it might be worth putting out here).
* Any span of approximately 10 years (the interval type is fine
enough for this).
I lack significant research but it's rare to hear people refer to
1990-1999 as the "199th century" in the same way they might refer to
1900-1999 (or 1901-2000) as the "20th century" -- and it's worth noting
that common usage for determining 20th/21st centuries generally follow
the mathematical logic of them, even if some people are off-by-one when
determining when they start and end.
I'd also argue that the current function basing the logic from
definition #2 has limited use even when you want to use it for such.
If you want to generate text for '(decades)s' you'd have to do:
SELECT extract('year' from date_trunc('decade', now())) || 's';
Or with my patch:
SELECT floor(extract('year' from now()) / 10) || '0s';
It's different, for sure, but I would actually think the second one is
a bit less awkward. Plus it's shorter :)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Proposed changing the definition of decade for date_trunc and extract
For a long time (since version 8.0), PostgreSQL has adopted the logical barriers for centuries and millenniums in these functions. The calendar starts millennium and century 1 on year 1, directly after 1 BC. Unfortunately decades are still reported rather simplistically by dividing the year by 10. Years 1-10 are logically the first decade and working up from there, year 2014 should be counted as 202nd decade. I've pushed code and documentation changes to reflect this, based on the master branch (9.5devel), it's on the branch new_decade_def at https://github.com/chungy/postgres.git -- In both the commit message and docs, I made note of the backwards compatibility change. I don't know how much of an impact this would have but I suspect not many applications are really going to be affected by how decades are counted (should be simple to fix on their part, if any are...). -- Mike Swanson -- 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] Congrats Andres Freund, the newest PostgreSQL Commiter!
Congrats Andres! Mike __ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * * On Thu, May 22, 2014 at 9:24 PM, Fabrízio de Royes Mello < fabriziome...@gmail.com> wrote: > Hi All, > > At the Developer Meeting that occurred 21th May 2014 was announced a new > PostgreSQL commiter [1], Mr. Andres Freund. > > I had the opportunity to work and be mentored by him. He deserves very > much this confidence, for the excellent work that has been doing for the > community. > > Thank you and Congrats Andres! > > > [1] > https://wiki.postgresql.org/wiki/PgCon_2014_Developer_Meeting#New_Committer > > -- > Fabrízio de Royes Mello > Consultoria/Coaching PostgreSQL > >> Timbira: http://www.timbira.com.br > >> Blog sobre TI: http://fabriziomello.blogspot.com > >> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello > >> Twitter: http://twitter.com/fabriziomello >
Re: [HACKERS] tds_fdw for Sybase and MS SQL Server
Excellent! I have an application for this. I'll give it a look. Thanks! Mike ______ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * * On Sun, Apr 6, 2014 at 9:03 AM, Geoff Montee wrote: > If anyone is interested, I've developed a foreign data wrapper that can be > used to connect to Sybase databases and Microsoft SQL Server. You can get > it on GitHub here: > > https://github.com/GeoffMontee/tds_fdw > > I did my testing with FreeTDS, an open source TDS library. > > I've talked to Greg Smith about this in the past. I don't really have the > expertise or resources to develop and test this much more than I already > have. If anyone likes it and would like to take over "ownership" of the > code, feel free to do so. > > I actually developed this over a year ago, so it doesn't support write > operations added in PostgreSQL 9.3. > > By the way, thanks to everyone who spoke at PGConf NYC 2014. It was very > informative. > > Geoff Montee > >
[HACKERS] 9.3.2 Client-only installation per docs fails creating directory.
Per the docs (15.4 Installation Procedure, Client-only installation), after running make, the following should work: gmake -C src/bin installgmake -C src/include installgmake -C src/interfaces installgmake -C doc install However, it fails on the first command: [postgresql-9.3.2]$ sudo gmake -C src/bin install gmake: Entering directory `/opt/postgresql-9.3.2/src/bin' gmake -C initdb install gmake[1]: Entering directory `/opt/postgresql-9.3.2/src/bin/initdb' gmake -C ../../../src/port all [successful stuff cut here] /bin/mkdir -p '/usr/local/pgsql-9.3.2/bin' /usr/bin/install -c psql '/usr/local/pgsql-9.3.2/bin/psql' /usr/bin/install -c -m 644 ./psqlrc.sample '/usr/local/pgsql-9.3.2/share/psqlrc.sample' /usr/bin/install: cannot create regular file `/usr/local/pgsql-9.3.2/share/psqlrc.sample': No such file or directory gmake[1]: *** [install] Error 1 gmake[1]: Leaving directory `/opt/postgresql-9.3.2/src/bin/psql' gmake: *** [install-psql-recurse] Error 2 gmake: Leaving directory `/opt/postgresql-9.3.2/src/bin' The directory 'share' does not exist, which seem to be the issue. Is there a missing dependency somewhere? It appears the doc install correctly creates 'share', so installing src/bin last works. __ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * *
Re: [HACKERS] why semicolon after begin is not allowed in postgresql?
I believe the section you are reading refers to the BEGIN keyword in the procedural language plpgsql, not the SQL 'BEGIN' command. The issue stems from confusing two distinct languages both of which, along with several more procedural languages, are documented in the same manual. __ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * * On Fri, Nov 22, 2013 at 4:24 PM, AK wrote: > I am reading the following in the documentation: "Tip: A common mistake is > to > write a semicolon immediately after BEGIN. This is incorrect and will > result > in a syntax error." > > So, "common mistake" means semicolons after BEGIN seem consistent to many > people - it seems consistent to me as well. If PostgreSql allowed them, we > would have one less rule to memorize, shorter documentation, less mistakes > and so on. In other words, without this limitation PostgreSql would be > slightly more useful, right? > > What am I missing? Why do we need this rule? How is it making PostgreSql > better? > > > > -- > View this message in context: > http://postgresql.1045698.n5.nabble.com/why-semicolon-after-begin-is-not-allowed-in-postgresql-tp5779905.html > Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers >
Re: [HACKERS] stats for network traffic WIP
This patch looks good to me. It applies, builds, and runs the regression
tests. Documentation is included and it seems to do what it says. I don't
consider myself a code expert, but as far as I can see it looks fine. This
is a pretty straightforward enhancement to the existing pg_stat_* code.
If no one has any objections, I'll mark it ready for committer.
Mike
__
*Mike Blackwell | Technical Analyst, Distribution Services/Rollout
Management | RR Donnelley*
1750 Wallace Ave | St Charles, IL 60174-3401
Office: 630.313.7818
mike.blackw...@rrd.com
http://www.rrdonnelley.com
<http://www.rrdonnelley.com/>
* *
On Thu, Nov 14, 2013 at 11:29 PM, Nigel Heron wrote:
> On Wed, Nov 13, 2013 at 11:27 PM, Peter Eisentraut
> wrote:
> > On Fri, 2013-11-08 at 10:01 -0500, Nigel Heron wrote:
> >> here's v4 of the patch. I added documentation and a new global view
> >> called "pg_stat_socket" (includes bytes_sent, bytes_received and
> >> stats_reset time)
> >
> > Your patch needs to be rebased:
> >
> > CONFLICT (content): Merge conflict in src/test/regress/expected/rules.out
> >
>
> Hi,
> here's a rebased patch with some additions.
>
> an overview of it's current state...
>
> a new pg_stat_socket global view:
> - total bytes sent and received
> - bytes sent and received for user backends
> - bytes sent and received for wal senders
> - total connection attempts
> - successful connections to user backends
> - successful connections to wal senders
> - stats reset time
> pg_stat_reset_shared('socket') resets the counters
>
> added to pg_stat_database view:
> - bytes sent and received per db
> - successful connections per db
> pg_stat_reset() resets the counters
>
> added to pg_stat_activity view:
> - bytes sent and received per backend
>
> added to pg_stat_replication view:
> - bytes sent and received per wal sender
>
> using the existing track_counts guc to enable/disable these stats.
> -nigel.
>
Re: [HACKERS] additional json functionality
On Wed, Nov 13, 2013 at 4:16 PM, Josh Berkus wrote:
>
> > Putting it all together, I'd consider:
> > *) dropping json_object (although maybe there is a case I'm not
> thinking about)
> > *) changing json_build function names to get the json prefix
> > *) adding a json object constructor that takes two parallel arrays as
> > arguments.
>
> I was with you until the third idea. Huh?
>
>
I actually had a use case for this today, though with hstore, importing a
fixed length record with something along the lines of:
hstore(
ARRAY['field 1', 'field 2', 'field 3'],
regexp_matches(fixed_field,'(.{4})(.{10})(.{5})')
)
__
*Mike Blackwell | Technical Analyst, Distribution Services/Rollout
Management | RR Donnelley*
1750 Wallace Ave | St Charles, IL 60174-3401
Office: 630.313.7818
mike.blackw...@rrd.com
http://www.rrdonnelley.com
<http://www.rrdonnelley.com/>
* *
Re: [HACKERS] stats for network traffic WIP
Also still to be tested: performance impact. __ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * * On Fri, Nov 8, 2013 at 9:33 AM, Mike Blackwell wrote: > Patch applies and builds against git HEAD (as of 6790e738031089d5). "make > check" runs cleanly as well. > > The new features appear to work as advertised as far as I've been able to > check. > > The code looks good as far as I can see. Documentation patches are > included for the new features. > > Still to be tested: > the counts for streaming replication (no replication setup here to test > against yet). > > > __ > *Mike Blackwell | Technical Analyst, Distribution Services/Rollout > Management | RR Donnelley* > 1750 Wallace Ave | St Charles, IL 60174-3401 > Office: 630.313.7818 > mike.blackw...@rrd.com > http://www.rrdonnelley.com > > > <http://www.rrdonnelley.com/> > * * > > > On Fri, Nov 8, 2013 at 9:01 AM, Nigel Heron wrote: > >> On Tue, Oct 29, 2013 at 11:26 AM, Nigel Heron >> wrote: >> >> >> >> So, for now, the counters only track sockets created from an inbound >> >> (client to server) connection. >> > >> > here's v3 of the patch (rebase and cleanup). >> > >> >> Hi, >> here's v4 of the patch. I added documentation and a new global view >> called "pg_stat_socket" (includes bytes_sent, bytes_received and >> stats_reset time) >> >> thanks, >> -nigel. >> > >
Re: [HACKERS] stats for network traffic WIP
Patch applies and builds against git HEAD (as of 6790e738031089d5). "make check" runs cleanly as well. The new features appear to work as advertised as far as I've been able to check. The code looks good as far as I can see. Documentation patches are included for the new features. Still to be tested: the counts for streaming replication (no replication setup here to test against yet). __ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * * On Fri, Nov 8, 2013 at 9:01 AM, Nigel Heron wrote: > On Tue, Oct 29, 2013 at 11:26 AM, Nigel Heron > wrote: > >> > >> So, for now, the counters only track sockets created from an inbound > >> (client to server) connection. > > > > here's v3 of the patch (rebase and cleanup). > > > > Hi, > here's v4 of the patch. I added documentation and a new global view > called "pg_stat_socket" (includes bytes_sent, bytes_received and > stats_reset time) > > thanks, > -nigel. >
Re: [HACKERS] stats for network traffic WIP
On Wed, Oct 23, 2013 at 2:10 PM, Atri Sharma wrote: > > Adding myself as the co reviewer specifically for the testing > purposes, if its ok with you. > It's perfectly fine with me. Please do! ______ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * *
Re: [HACKERS] stats for network traffic WIP
On Wed, Oct 23, 2013 at 1:58 PM, Atri Sharma wrote: > > IMO, the idea is pretty good. Its just that we need to do some wide > spectrum performance testing. Thats only my thought though. I'm looking at trying to do some performance testing on this. Any suggestions on test scenarios, etc? __ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * *
Re: [HACKERS] stats for network traffic WIP
Sounds good. I personally don't have any interest in log file i/o counters, but that's just me. I wonder if stats collector counters might be useful... I seem to recall an effort to improve that area. Maybe not enough use to take the performance hit on a regular basis, though. __ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * * On Wed, Oct 23, 2013 at 1:44 PM, Nigel Heron wrote: > Hi, thanks, I'm still actively working on this patch. I've gotten the > traffic counters working when using SSL enabled clients (includes the > ssl overhead now) but I still have the walsender transfers under SSL > to work on. > I'll post an updated patch when i have it figured out. > Since the patch changes some views in pg_catalog, a regression test > fails .. i'm not sure what to do next. Change the regression test in > the patch, or wait until the review phase? > > I was also thinking of adding global counters for the stats collector > (pg_stat* file read/write bytes + packets lost) and also log file io > (bytes written for txt and csv formats) .. any interest? > > -nigel. > > On Wed, Oct 23, 2013 at 12:50 PM, Mike Blackwell > wrote: > > I added this to the current CF, and am starting to review it as I have > time. > > > > > __ > > Mike Blackwell | Technical Analyst, Distribution Services/Rollout > Management > > | RR Donnelley > > 1750 Wallace Ave | St Charles, IL 60174-3401 > > Office: 630.313.7818 > > mike.blackw...@rrd.com > > http://www.rrdonnelley.com > > > > > > > > > > On Mon, Oct 21, 2013 at 11:32 AM, Stephen Frost > wrote: > >> > >> Nigel, > >> > >> * Nigel Heron (nhe...@querymetrics.com) wrote: > >> > Hi, I've been using postgres for many years but never took the time to > >> > play > >> > with the code until now. As a learning experience i came up with this > >> > WIP > >> > patch to keep track of the # of bytes sent and received by the server > >> > over > >> > it's communication sockets. Counters are kept per database, per > >> > connection > >> > and globally/shared. > >> > >> Very neat idea. Please add it to the current commitfest > >> (http://commitfest.postgresql.org) and, ideally, someone will get in > and > >> review it during the next CM. > >> > >> Thanks! > >> > >> Stephen > > > > >
Re: [HACKERS] stats for network traffic WIP
I added this to the current CF, and am starting to review it as I have time. __ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * * On Mon, Oct 21, 2013 at 11:32 AM, Stephen Frost wrote: > Nigel, > > * Nigel Heron (nhe...@querymetrics.com) wrote: > > Hi, I've been using postgres for many years but never took the time to > play > > with the code until now. As a learning experience i came up with this WIP > > patch to keep track of the # of bytes sent and received by the server > over > > it's communication sockets. Counters are kept per database, per > connection > > and globally/shared. > > Very neat idea. Please add it to the current commitfest > (http://commitfest.postgresql.org) and, ideally, someone will get in and > review it during the next CM. > > Thanks! > > Stephen >
[HACKERS] RULE regression test fragility?
While reviewing the Network Stats Traffic patch I discovered the current regression test for rules depends on the system view definitions not changing: -- -- Check that ruleutils are working -- SELECT viewname, definition FROM pg_views WHERE schemaname <> 'information_schema' ORDER BY viewname; In this particular case new fields have been added to the view, breaking this apparently unrelated test. Is checking the definition of all views necessary for this test? Would it possibly be better to create a temporary view for this check, or is something else going on here? __ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * *
Re: [HACKERS] Commitfest II CLosed
Actually, I did call them out in the thread announcing the CF Wrap Up ( http://www.postgresql.org/message-id/CAESHdJonURj3i9HR2w4e=ohep5hx7snqyydsgyweqqa+a3d...@mail.gmail.com). Looking back, it may have been better to post it as a separate thread, but I'm not confident that would have made much difference. ______ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * * On Mon, Oct 21, 2013 at 9:45 AM, Hannu Krosing wrote: > On 10/21/2013 03:56 PM, Heikki Linnakangas wrote: > > > > I feel guilty to complain, while not actually volunteering to be a > > commitfest manager myself, but I wish the commitfest manager would be > > more aggressive in nagging, pinging and threatening people to review > > stuff. If nothing else, always feel free to nag me :-). Josh tried > > that with the infamous Slacker List, but that backfired. Rather than > > posting a public list of shame, I think it would work better to send > > short off-list nag emails, or chat via IM. Something like "Hey, you've > > signed up to review this. Any progress?". Or "Hey, could you take a > > look at X please? No-one else seems to care about it." > Or maybe even nag publicly with "list of orphans" - hey people, do you > *really* think that this patch is not needed ? > > > > -- > 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] CF 2013-09 Wrap Up
On Tue, Oct 15, 2013 at 1:39 AM, Noah Misch wrote: > On Mon, Oct 14, 2013 at 01:56:42PM -0500, Mike Blackwell wrote:> Any > patches marked Needs Review will be automatically moved to the next CF. > > We will try to make sure that all patches in the current CF have > received > > at least one review. > > The combined effect of those two statements is not clear to me. Does that > mean you'll retain never-reviewed patches and automatically move patches > that > have received at least one review? > Yes on the latter part. We will try to get a quick review for not-yet-reviewed patches and move or return them based on the result of that review. If we fail to find a reviewer, the patches will get moved to the next CF. For those following along, here are the patches still needing a first look. They are for the most part performance or internals patches and could use the eye of someone more experienced. Please consider a quick review of one of them if you fit that description. We'd like everyone to get a fair shake here. ^_^ HStore Gin Speedup<https://commitfest.postgresql.org/action/patch_view?id=1203> Performance Improvement by reducing WAL for Update Operation<https://commitfest.postgresql.org/action/patch_view?id=1209> [PoC] pgstattuple2: block sampling to reduce physical read<https://commitfest.postgresql.org/action/patch_view?id=1226> ECPG cursor readahead<https://commitfest.postgresql.org/action/patch_view?id=1195>
[HACKERS] CF 2013-09 Wrap Up
CF 2013-09 will be wrapping up this week. As a reminder, beginning on the official CF end date (11/15), patches Waiting for Author will be Returned with Feedback. Authors are welcome to add their patch to the next CF (2013-11). Any patches marked Needs Review will be automatically moved to the next CF. We will try to make sure that all patches in the current CF have received at least one review. If you have any questions or comments about the CF process, feel free to send a note to me or David Fetter (CFM). ^_^ Thanks to all who have submitted or reviewed patches this time around! Mike Blackwell
Re: [HACKERS] Patch for reserved connections for replication users
I'd received an email from Gibheer suggesting it be move due to lack of time to
work on it. I can certainly move it back if that's no longer the case.
On Oct 9, 2013, at 23:25, Amit Kapila wrote:
> On Thu, Oct 10, 2013 at 3:17 AM, Gibheer wrote:
> On Mon, 7 Oct 2013 11:39:55 +0530
> Amit Kapila wrote:
>> Robert Haas wrote:
>> On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund
>> wrote:
> Hmm. It seems like this match is making MaxConnections no longer
> mean the maximum number of connections, but rather the maximum
> number of non-replication connections. I don't think I support
> that definitional change, and I'm kinda surprised if this is
> sufficient to implement it anyway (e.g. see InitProcGlobal()).
>>>
I don't think the implementation is correct, but why don't you
like the definitional change? The set of things you can do from
replication connections are completely different from a normal
connection. So using separate "pools" for them seems to make sense.
That they end up allocating similar internal data seems to be an
implementation detail to me.
>>
>>> Because replication connections are still "connections". If I tell
>>> the system I want to allow 100 connections to the server, it should
>>> allow 100 connections, not 110 or 95 or any other number.
>>
>> I think that to reserve connections for replication, mechanism similar
>> to superuser_reserved_connections be used rather than auto vacuum
>> workers or background workers.
>> This won't change the definition of MaxConnections. Another thing is
>> that rather than introducing new parameter for replication reserved
>> connections, it is better to use max_wal_senders as it can serve the
>> purpose.
>>
>> Review for replication_reserved_connections-v2.patch, considering we
>> are going to use mechanism similar to superuser_reserved_connections
>> and won't allow definition of MaxConnections to change.
>>
>> 1. /* the extra unit accounts for the autovacuum launcher */
>> MaxBackends = MaxConnections + autovacuum_max_workers + 1 +
>> - + max_worker_processes;
>> + + max_worker_processes + max_wal_senders;
>>
>> Above changes are not required.
>>
>> 2.
>> + if ((!am_superuser && !am_walsender) &&
>> ReservedBackends > 0 &&
>> !HaveNFreeProcs(ReservedBackends))
>>
>> Change the check as you have in patch-1 for
>> ReserveReplicationConnections
>>
>> if (!am_superuser &&
>> (max_wal_senders > 0 || ReservedBackends > 0) &&
>> !HaveNFreeProcs(max_wal_senders + ReservedBackends))
>> ereport(FATAL,
>> (errcode(ERRCODE_TOO_MANY_CONNECTIONS),
>> errmsg("remaining connection slots are reserved for replication and
>> superuser connections")));
>>
>> 3. In guc.c, change description of max_wal_senders similar to
>> superuser_reserved_connections at below place:
>> {"max_wal_senders", PGC_POSTMASTER, REPLICATION_SENDING,
>> gettext_noop("Sets the maximum number of simultaneously running WAL
>> sender processes."),
>>
>> 4. With this approach, there is no need to change InitProcGlobal(), as
>> it is used to keep track bgworkerFreeProcs and autovacFreeProcs, for
>> others it use freeProcs.
>>
>> 5. Below description in config.sgml needs to be changed for
>> superuser_reserved_connections to include the effect of max_wal_enders
>> in reserved connections.
>> Whenever the number of active concurrent connections is at least
>> max_connections minus superuser_reserved_connections, new connections
>> will be accepted only for superusers, and no new replication
>> connections will be accepted.
>>
>> 6. Also similar description should be added to max_wal_senders
>> configuration parameter.
>>
>> 7. Below comment needs to be updated, as it assumes only superuser
>> reserved connections as part of MaxConnections limit.
>> /*
>> * ReservedBackends is the number of backends reserved for superuser
>> use.
>> * This number is taken out of the pool size given by MaxBackends so
>> * number of backend slots available to non-superusers is
>> * (MaxBackends - ReservedBackends). Note what this really means is
>> * "if there are <= ReservedBackends connections available, only
>> superusers
>> * can make new connections" --- pre-existing superuser connections
>> don't
>> * count against the limit.
>> */
>> int ReservedBackends;
>>
>> 8. Also we can add comment on top of definition for max_wal_senders
>> similar to ReservedBackends.
>>
>>
>> With Regards,
>> Amit Kapila.
>> EnterpriseDB: http://www.enterprisedb.com
>
> Hi,
>
> I took the time and reworked the patch with the feedback till now.
> Thank you very much Amit!
Is there any specific reason why you moved this patch to next CommitFest?
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] File_fdw documentation patch to clarify OPTIONS clause
Robert, Thanks for the reply. I have no objections to clarifying the note. Attached is a patch with the text you suggested. Mike __ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * * On Mon, Sep 23, 2013 at 11:42 AM, Robert Haas wrote: > On Fri, Sep 20, 2013 at 4:26 PM, Mike Blackwell > wrote: > > The file_fdw documentation for the OPTIONS clause references the COPY > > command's documentation. In the case of COPY, the value for some options > > (e.g. HEADER, OIDS, ...) is optional. While the file_fdw documentation > > makes no mention of it, the values for all options are required. > > > > Attached is a patch to add a note to the docs mentioning this fact. > > I think this would be a good thing to document, but it took me a > minute to properly understand what you were saying. So I'd like to > suggest slightly different wording: "Note that while COPY allows > options such as OIDS and HEADER to be specified without a > corresponding value, the foreign data wrapper syntax requires a value > to be present in all cases. To activate COPY options normally > supplied without a value, you can instead pass the value TRUE." > > Other suggestions welcome if you don't like that text... > > -- > 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 > file-fdw-doc.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
[HACKERS] File_fdw documentation patch to clarify OPTIONS clause
The file_fdw documentation for the OPTIONS clause references the COPY command's documentation. In the case of COPY, the value for some options (e.g. HEADER, OIDS, ...) is optional. While the file_fdw documentation makes no mention of it, the values for all options are required. Attached is a patch to add a note to the docs mentioning this fact. ______ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * * file-fdw-doc.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Performance Improvement by reducing WAL for Update Operation
The only environment I have available at the moment is a virtual box. That's probably not going to be very helpful for performance testing. ______ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * * On Mon, Jul 8, 2013 at 11:09 PM, Amit Kapila wrote: > > On Tuesday, July 09, 2013 2:52 AM Mike Blackwell wrote: > > > I can't comment on further direction for the patch, but since it was > marked as Needs Review in the CF app I took a quick look at it. > Thanks for looking into it. > > Last time Heikki has found test scenario's where the original patch was > not performing good. > He has also proposed a different approach for WAL encoding and sent the > modified patch which has comparatively less negative performance impact and > asked to check if the patch can reduce the performance impact for the > scenario's mentioned by him. > After that I found that with some modification's (use new tuple data for > encoding) in his approach, it eliminates the negative performance impact > and > have WAL reduction for more number of cases. > > I think the first thing to verify is whether the results posted can be > validated in some other environment setup by another person. > The testcase used is posted at below link: > http://www.postgresql.org/message-id/51366323.8070...@vmware.com > > > > > It patches and compiles clean against the current Git HEAD, and 'make > check' runs successfully. > > > Does it need documentation for the GUC variable > 'wal_update_compression_ratio'? > > This variable has been added to test the patch for different > compression_ratio during development test. > It was not decided to have this variable permanently as part of this > patch, so currently there is no documentation for it. > However if the decision comes out to be that it needs to be part of > patch, then documentation for same can be updated. > > With Regards, > Amit Kapila. > > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers >
[HACKERS] [REVIEW] row level security (v3)
The most recent patch (v3) applies and builds cleanly and passes make check. Documentation on the new SQL syntax and catalog changes is included with the patch and looks good to me. The regression tests look pretty complete. In addition to the included tests, dropping and altering the data type on a column referenced in the security clause work as expected, rejecting the change with a dependency error. Renaming a column succeeds as expected. pg_dump and restore properly was also successful. I noticed that the security clause is visible to any user via psql \dt+, as well as in the pg_rowsecurity view. Perhaps this should be mentioned in the relevant section of user-manag.sgml so users realize any sensitive information in the security clause isn't secure. What I've checked looks good. I don't feel qualified to do a code review so that's still outstanding. I believe Atri will be looking at that. __ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * *
Re: [HACKERS] Performance Improvement by reducing WAL for Update Operation
I can't comment on further direction for the patch, but since it was marked as Needs Review in the CF app I took a quick look at it. It patches and compiles clean against the current Git HEAD, and 'make check' runs successfully. Does it need documentation for the GUC variable 'wal_update_compression_ratio'? ______ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * * On Tue, Jul 2, 2013 at 2:26 AM, Hari Babu wrote: > On Friday, June 07, 2013 5:07 PM Amit Kapila wrote: > >On Wednesday, March 06, 2013 2:57 AM Heikki Linnakangas wrote: > >> On 04.03.2013 06:39, Amit Kapila wrote: > >> > On Sunday, March 03, 2013 8:19 PM Craig Ringer wrote: > >> >> On 02/05/2013 11:53 PM, Amit Kapila wrote: > >> >>>> Performance data for the patch is attached with this mail. > >> >>>> Conclusions from the readings (these are same as my previous > >> patch): > >> >>>> > >> > >> The attached patch also just adds overhead in most cases, but the > >> overhead is much smaller in the worst case. I think that's the right > >> tradeoff here - we want to avoid scenarios where performance falls off > >> the cliff. That said, if you usually just get a slowdown, we certainly > >> can't make this the default, and if we can't turn it on by default, > >> this probably just isn't worth it. > >> > >> The attached patch contains the variable-hash-size changes I posted in > >> the "Optimizing pglz compressor". But in the delta encoding function, > >> it goes further than that, and contains some further micro- > >> optimizations: > >> the hash is calculated in a rolling fashion, and it uses a specialized > >> version of the pglz_hist_add macro that knows that the input can't > >> exceed 4096 bytes. Those changes shaved off some cycles, but you could > >> probably do more. One idea is to only add every 10 bytes or so to the > >> history lookup table; that would sacrifice some compressibility for > >> speed. > >> > >> If you could squeeze pglz_delta_encode function to be cheap enough > >> that we could enable this by default, this would be pretty cool patch. > >> Or at least, the overhead in the cases that you get no compression > >> needs to be brought down, to about 2-5 % at most I think. If it can't > >> be done easily, I feel that this probably needs to be dropped. > > >After trying some more on optimizing pglz_delta_encode(), I found that if > we use new data also in history, then the results of compression and cpu > utilization >are much better. > > >In addition to the pg lz micro optimization changes, following changes are > done in modified patch > > >1. The unmatched new data is also added to the history which can be > referenced later. > >2. To incorporate this change in the lZ algorithm, 1 extra control bit is > needed to indicate if data is from old or new tuple > > The patch is rebased to use the new PG LZ algorithm optimization changes > which got committed recently. > > Performance Data > - > > Head code: > > testname | wal_generated | duration > > -+---+-- > > two short fields, no change |1232911016 | 35.1784930229187 > two short fields, one changed |1240322016 | 35.0436308383942 > two short fields, both changed |1235318352 | 35.4989421367645 > one short and one long field, no change |1042332336 | 23.4457180500031 > ten tiny fields, all changed|1395194136 | 41.9023628234863 > hundred tiny fields, first 10 changed | 626725984 | 21.2999589443207 > hundred tiny fields, all changed| 621899224 | 21.6676609516144 > hundred tiny fields, half changed | 623998272 | 21.2745981216431 > hundred tiny fields, half nulled| 557714088 | 19.5902800559998 > > > pglz-with-micro-optimization-compress-using-newdata-2: > > testname | wal_generated | duration > > -+---+-- > > two short fields, no change |1232903384 | 35.0115969181061 > two short fields, one changed |12329
Re: [HACKERS] [v9.4] row level security
With the current HEAD and v3 patch I'm seeing the following error with "make check". -- == creating temporary installation== == initializing database system == pg_regress: initdb failed Examine /usr/local/src/postgres.patched.v3/src/test/regress/log/initdb.log for the reason. Command was: "/usr/local/src/postgres.patched.v3/src/test/regress/./tmp_check/install//usr/local/pgsql/bin/initdb" -D "/usr/local/src/postgres.patched.v3/src/test/regress/./tmp_check/data" -L "/usr/local/src/postgres.patched.v3/src/test/regress/./tmp_check/install//usr/local/pgsql/share" --noclean --nosync > "/usr/local/src/postgres.patched.v3/src/test/regress/log/initdb.log" 2>&1 make[1]: *** [check] Error 2 make[1]: Leaving directory `/usr/local/src/postgres.patched.v3/src/test/regress' make: *** [check] Error 2 __ *Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley* 1750 Wallace Ave | St Charles, IL 60174-3401 Office: 630.313.7818 mike.blackw...@rrd.com http://www.rrdonnelley.com <http://www.rrdonnelley.com/> * *
Re: [HACKERS] [ADMIN] after 9.2.4 patch vacuumdb -avz not analyzing all tables
Looks like psql> vacuum (verbose, analyze) is not reflecting in pg_stat_user_tables as well in some cases. In this scenario I run the command, it outputs all the deleted pages etc (unlike the vacuumdb -avz analyze that seemed to be skipped in the log), but it does not update pg_stat_user_tables. Thats probably expected based on the description previously reported, but I wanted to confirm what I was seeing. On Fri, Apr 12, 2013 at 10:36 AM, Kevin Grittner wrote: > Scott Marlowe wrote: > > > Does this behavior only affect the 9.2 branch? Or was it ported > > to 9.1 or 9.0 or 8.4 as well? > > After leaving it on master for a while to see if anyone reported > problems in development, I back-patched as far as 9.0 in time for > the 9.2.3 (and related) patches. Prior to that the code was too > different for it to be the same patch, and (perhaps not entirely > coincidentally) I had not seen the problems before 9.0. From 9.0 > on I have seen multiple sites (all using queuing from Slony or a > JMS implementation) with recurring problems when the queue > temporarily got large, shrank again, and then wrapped around to the > beginning of the table's file space. In some cases performance was > so impaired that when such an event was triggered they would shut > down their application until a manual VACUUM could be run. > > -- > Kevin Grittner > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company >
Re: [HACKERS] [ADMIN] after 9.2.4 patch vacuumdb -avz not analyzing all tables
On further review this particular server skipped from 9.2.2 to 9.2.4. This is my most busy and downtime sensitive server and I was waiting on a maintenance window to patch to 9.2.3 when 9.2.4 dropped and bumped up the urgency. However, I have 3 other less busy production servers that were all running 9.2.3 for a while, didnt exhibit the problem, and still dont on 9.2.4. psql> analyze seems to work ok in the meantime, I'll report back if I notice any problems with that. Thanks very much for the response and investigation, it is much appreciated! On Thu, Apr 11, 2013 at 8:48 PM, Kevin Grittner wrote: > Tom Lane wrote: > > > However I've got to say that both of those side-effects of > > exclusive-lock abandonment seem absolutely brain dead now that I > > see them. Why would we not bother to tell the stats collector > > what we've done? Why would we think we should not do ANALYZE > > when we were told to? > > > > Would someone care to step forward and defend this behavior? > > Because it's not going to be there very long otherwise. > > I'm pretty sure that nobody involved noticed the impact on VACUUM > ANALYZE command; all discussion was around autovacuum impact; and > Jan argued that this was leaving things in a status quo for that, > so I conceded the point and left it for a follow-on patch if > someone felt the behavior needed to change. Sorry for the miss. > > http://www.postgresql.org/message-id/50bb700e.8060...@yahoo.com > > As far as I'm concerned all effects on the explicit command were > unintended and should be reverted. > > -- > Kevin Grittner > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company >
Re: [HACKERS] Show type in psql SELECT
On 25 February 2013 12:48, Craig Ringer wrote: > However, the thing I want most couldn't be provided by this patch > because it seems to be a deeper server limitation: the ability to get > typmod data from calculation results like > > NUMERIC(8,3) '11.131' + NUMERIC(8,3) '12.123' But is the derived typmod always available? For example, with PostGIS: postgis=# SELECT g, ST_Centroid(g) INTO TEMP t postgis-# FROM (SELECT 'POLYGON((0 0, 1 1, 0 1, 0 0))'::geometry(Polygon) g) p; SELECT 1 postgis=# \d t Table "pg_temp_15.t" Column| Type| Modifiers -+---+--- g | geometry(Polygon) | st_centroid | geometry | -Mike
[HACKERS] Show type in psql SELECT
Hi hackers, Type info can be viewed with "\d mytable", however often I'd like to see the type (and typmod) info in SELECT queries with psql, similar to pgAdmin III. For example: my_db=# \pset type my_db=# SELECT * FROM my_table; gid | description| width integer | character varying(255) | numeric(6,3) -++-- 1 | Hello | 3.220 (1 row) or in expanded form: my_db=# \x my_db=# SELECT * FROM my_table; -[ RECORD 1 ]-- gid : integer| 1 description : character varying(255) | Hello width : numeric(6,3) | 3.220 Has anyone else thought this was a missing feature? -Mike
Re: [HACKERS] Re: [GENERAL] pg_dump incredibly slow dumping a single schema from a large db
:) yah that makes sense no big deal. i'll probably just push this head buiild of pg_dump onto the production machines till it comes out. Thanks again! On Sat, Mar 31, 2012 at 3:44 PM, Tom Lane wrote: > Mike Roest writes: > > Any idea when 9.1.4 with this change will be out so we can pull the > cluster > > up. > > Well, we just did some releases last month, so unless somebody finds a > really nasty security or data-loss issue, I'd think it will be a couple > of months before the next set. > >regards, tom lane >
[HACKERS] Re: [GENERAL] pg_dump incredibly slow dumping a single schema from a large db
> > > I'm just pulling another backup using the stock 9.1.1 pg_dump to ensure > the backups are equivalent. > > Schema & data are identical between the 2 backups. the new backup passes all our tests for validating a tenant. Thank you again for the quick response! --Mike
[HACKERS] Re: [GENERAL] pg_dump incredibly slow dumping a single schema from a large db
> > I've committed fixes for both these issues. If you are in a position to > test with 9.1 branch tip from git, it'd be nice to have confirmation > that these patches actually cure your problem. For both of them, the > issue seems to only show up in a subset of cases, which may explain why > we'd not identified the problem before. > >regards, tom lane > Cool, I've grabbed your changes and it seems to have completely cured the issue. reading user-defined tables has gone down to 9 seconds from 2.5 minutes reading dependency data has gone down to 20 seconds from 5.5 minutes. Thanks so much Tom this is perfect. I'm just pulling another backup using the stock 9.1.1 pg_dump to ensure the backups are equivalent. Any idea when 9.1.4 with this change will be out so we can pull the cluster up. With regards to your previous question about sequences there are 61K in the DB, looks like our schema currently has about 115 sequences per tenant. --Mike
Re: [HACKERS] JSON for PG 9.2
I am very interested in experimenting with functional indexes into JSON structures. I think this could be very powerful combined with full text search as well as constraints. It would allow for using postgres as an unstructured data store without sacrificing the powerful indexing features, durability, and transactional semantics that comes with using postgres or RDBMSes in general. One use case in particular I have been trying to solve for lately is persisting and synchronizing client-side state (in a mobile application) with a server. It would be nice to have a flat, schemaless table (maybe a table that's like (id, type, owner, data) where data would be a JSON blob). I could do this now without JSON support, but I think indexing inside that JSON blob and having validation database side is valuable as well. And as I mentioned before, i'd rather not throw out the baby with the bathwater by using a different type of database because ACID, replication, and constraints are also very important to this. As is being consistent with the rest of our technology stack. (I'd essentially be using a relational database to persist an object database) I'm also not too concerned about storage consumption with this (even though columnar compression would help a lot in the future) since it's easily partitionable by user ID. For my case the equivalent of postgres's XPath would work. Also having it as a maintained contrib module would be sufficient, although it being part of core as XPath is would be even better. Just my $0.02... even if I'm a bit late to the conversation. Thanks! Mike
[HACKERS] Function argument names in pg_catalog
Hi hackers,
I'm curios why argument names (argname) are not used in the DDL for
functions in pg_catalog, while they are are used throughout the
documentation. For example, the documentation for pg_read_file in
Table 9-60[1] has an "SQL prototype":
pg_read_file(filename text, offset bigint, length bigint)
then why isn't the DDL for the function instead something like:
CREATE OR REPLACE FUNCTION
public.pg_read_file(filename text, "offset" bigint, length bigint)
RETURNS text AS 'pg_read_file'
LANGUAGE internal VOLATILE STRICT COST 1;
There are two advantages for using argument names for function
definitions: to add extra documentation for the parameters, and allow
named notation (where applicable).
For the "extra documentation"[2] point, the "SQL prototype" is visible
in PgAdmin or psql. For example, with the above example try "\df
public.pg_read_file", the fourth column shows 'filename text, "offset"
bigint, length bigint' in the fourth column. The existing "\df
pg_catalog.pg_read_file" returns "text, bigint, bigint", which sends
the user to look up the function in the documentation to determine
which bigint parameter is for "length" or "offset". Having built-in
extra documentation saves this trip.
For the named notation[3] rational, a user can rearrange the arguments:
select public.pg_read_file("offset" := 200, length := 10, filename := 'myfile')
or more practically, if parameters in the function were defined with
default_expr, then the named parameters can be used while omitting
default_expr parameters to accept defaults.
Are there any drawbacks? Performance/bloat? Technical limitations?
Apologies for my ignorance on how the DDL for functions in pg_catalog
are defined. I can only assume they are generated from their internal
C functions, as I can't find a pg_catalog.sql file in the source.
Thanks for your thoughts,
-Mike
[1] http://www.postgresql.org/docs/current/static/functions-admin.html
[2] http://www.postgresql.org/docs/current/static/sql-createfunction.html
[3] http://www.postgresql.org/docs/current/static/sql-syntax-calling-funcs.html
--
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] [GENERAL] Creating temp tables inside read only transactions
I like Darren's proposal. It is elegant. > Date: Fri, 8 Jul 2011 18:38:59 +1200 > From: gavinflo...@archidevsys.co.nz > To: dar...@darrenduncan.net > CC: pg...@j-davis.com; guilla...@lelarge.info; mbee...@hotmail.com; > pgsql-gene...@postgresql.org; pgsql-hackers@postgresql.org > Subject: Re: [HACKERS] [GENERAL] Creating temp tables inside read only > transactions > > On 08/07/11 18:21, Darren Duncan wrote: > > Jeff Davis wrote: > >> On Thu, 2011-07-07 at 20:56 -0700, Darren Duncan wrote: > When you create a temporary table, PostgreSQL needs to add rows in > pg_class, pg_attribute, and probably other system catalogs. So > there are > writes, which aren't possible in a read-only transaction. Hence the > error. And no, there is no workaround. > >>> That sounds like a deficiency to overcome. > >>> > >>> It should be possible for those system catalogs to be virtual, > >>> defined like union views over similar immutable tables for the > >>> read-only database plus mutable in-memory ones for the temporary > >>> tables. > >> > >> Ideally, yes, from a logical standpoint there are catalog entries that > >> are only interesting to one backend. > >> > >> But that doesn't mean it's easy to do. Remember that catalog lookups > >> (even though most go through a cache) are a path that is important to > >> performance. Also, more complex catalog interpretations may introduce > >> some extra bootstrapping challenges. > >> > >>> Are there any plans in the works to do this? > >> > >> I don't think so. It sounds like some fairly major work for a > >> comparatively minor benefit. > >> > >> Suggestions welcome, of course, to either make the work look more minor > >> or the benefits look more major ;) > > > > What I said before was a simplification; below I present my real > > proposal. > > > > I think an even better way to support this is would be based on > > Postgres having support for directly using multiple databases within > > the same SQL session at once, as if namespaces were another level > > deep, the first level being the databases, the second level the > > schemas, and the third level the schema objects. > > > > Kind of like what the SQL standard defines its catalog/schema/object > > namespaces. > > > > This instead of needing to use federating or that contrib module to > > use multiple Pg databases of the same cluster at once. > > > > Under this scenario, we make the property of a database being > > read-only or read-write for the current SQL session associated with a > > database rather than the whole SQL session. A given transaction can > > read from any database but can only make changes to the ones not > > read-only. > > > > Also, the proper way to do temporary tables would be to put them in > > another database than the main one, where the whole other database has > > the property of being temporary. > > > > Under this scenario, there would be separate system catalogs for each > > database, and so the ones for read-only databases are read-only, and > > the ones for other databases aren't. > > > > Then the system catalog itself fundamentally isn't more complicated, > > per database, and anything extra to handle cross-database queries or > > whatever, if anything, is a separate layer. Code that only deals with > > a single database at once would be an optimized situation and perform > > no worse than it does now. > > > > Furthermore, federating databases is done with the same interface, by > > adding remote/foreign databases as extra databases at the top level > > namespace. > > > > Fundamentally, a SQL session would be associated with a Pg server, not > > a database managed by such. When one starts a SQL session, there are > > initially no databases visible to them, and the top-level namespace is > > empty. > > > > They then "mount" a database, similarly to how one mounts an OS > > filesystem, by providing appropriate connection info, either just the > > database name or also user/pass or also remote host etc as is > > applicable, these details being the difference between using a > > local/same-Pg-cluster db or a remote/federated one, and the details > > also say whether it is temporary or initially read-only etc. > > > > See also how SQLite works; this "mount" being analogous to their > > "attach". > > > > Such a paradigm is also how my Muldis D language interfaces databases; > > this is the most flexible, portable, extensible, optimizable, and > > elegant approach I can think of. > > > > -- Darren Duncan > > > I would suggest that the default action for psql would be as now, > associate the session with a database in the name of the current O/S user. > > However, use a new psql flag, such as '-unattached' or '-N', to indicate > that no database is to be attached when psql starts up. > > While I don't have a current need for what you propose, it does look > interesting and potentially useful to me. >
Re: [HACKERS] smallserial / serial2
Yup- it's attached. Mike From: Brar Piening [mailto:b...@gmx.de] Sent: Tuesday, June 07, 2011 6:58 PM To: Mike Pultz Cc: pgsql-hackers@postgresql.org Subject: Re: [HACKERS] smallserial / serial2 On Wed, 20 Apr 2011 21:27:27 -0400, Mike Pultz <mailto:m...@mikepultz.com> wrote: Can this be added? Probably not - since it's not a complete patch ;-) I tried to test this one but was unable to find a complete version of the patch in my local mail archives and in the official archives (http://archives.postgresql.org/message-id/023001cbffc3$46f77840$d4e668c0$@m ikepultz.com) Could you please repost it for testing? Regards, Brar 20110607_serial2.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] smallserial / serial2
Sorry, forgot the documentation- I guess that stuff doesn't magically happen! New patch attached. Mike From: Brar Piening [mailto:b...@gmx.de] Sent: Tuesday, June 07, 2011 6:58 PM To: Mike Pultz Cc: pgsql-hackers@postgresql.org Subject: Re: [HACKERS] smallserial / serial2 On Wed, 20 Apr 2011 21:27:27 -0400, Mike Pultz <mailto:m...@mikepultz.com> wrote: Can this be added? Probably not - since it's not a complete patch ;-) I tried to test this one but was unable to find a complete version of the patch in my local mail archives and in the official archives (http://archives.postgresql.org/message-id/023001cbffc3$46f77840$d4e668c0$@m ikepultz.com) Could you please repost it for testing? Regards, Brar 20110607_serial2_v2.diff Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] smallserial / serial2
Hey Tom, I'm sure there are plenty of useful tables with <= 32k rows in them? I have a table for customers that uses a smallint (since the customer id is referenced all over the place)- due to the nature of our product, we're never going to have more than 32k customers, but I still want the benefit of the sequence. And since serial4 and serial8 are simply pseudo-types- effectively there for convenience, I'd argue that it should simply be there for completeness- just because it may be less used, doesn't mean it shouldn't be convenient? Also, in another case, I'm using it in a small table used to constrain a bigger table- eg: create table stuff ( id serial2 unique ); create table data ( id serial8 unique, stuff smallint not null, foreign key(stuff) references stuff(id) on update cascade on delete restrict ); Where our "data" table has ~700 million rows right now. And yes- I guess there's nothing to stop me from using a smallint in the data table (thus getting the size savings), and reference a int in the stuff table, but it seems like bad form to me to have a foreign key constraint between two different types. Mike -Original Message- From: Tom Lane [mailto:t...@sss.pgh.pa.us] Sent: Thursday, April 21, 2011 10:26 AM To: Mike Pultz Cc: pgsql-hackers@postgresql.org Subject: Re: [HACKERS] smallserial / serial2 "Mike Pultz" < <mailto:m...@mikepultz.com> m...@mikepultz.com> writes: > I use tables all the time that have sequences on smallint's; I'd like > to simplify my create files by not having to create the sequence > first, but I also don't want to give up those 2 bytes per column! A sequence that can only go to 32K doesn't seem all that generally useful ... Are you certain that you're really saving anything? More likely than not, the "saved" 2 bytes are going to disappear into alignment padding of a later column or of the whole tuple. Even if it really does help for your case, that's another reason to doubt that it's generally useful. regards, tom lane
[HACKERS] smallserial / serial2
I use tables all the time that have sequences on smallint's;
I'd like to simplify my create files by not having to create the sequence
first, but I also don't want to give up those 2 bytes per column!
Can this be added?
Mike
--- postgresql-9.0.4/src/backend/parser/parse_utilcmd.c 2011-04-14
23:15:53.0 -0400
+++ postgresql-9.0.4.new/src/backend/parser/parse_utilcmd.c 2011-04-20
21:10:26.0 -0400
@@ -280,8 +280,15 @@
{
char *typname =
strVal(linitial(column->typeName->names));
- if (strcmp(typname, "serial") == 0 ||
- strcmp(typname, "serial4") == 0)
+ if (strcmp(typname, "smallserial") == 0 ||
+ strcmp(typname, "serial2") == 0)
+ {
+ is_serial = true;
+ column->typeName->names = NIL;
+ column->typeName->typeOid = INT2OID;
+ }
+ else if (strcmp(typname, "serial") == 0 ||
+strcmp(typname, "serial4") == 0)
{
is_serial = true;
column->typeName->names = NIL;
Re: [JDBC] [HACKERS] JDBC connections to 9.1
On 18/04/11 17:12, Tom Lane wrote:
Dave Cramer writes:
Well initially my concern was that people would have a challenge in
the case where they had to re-certify their application if we made
this change, however I realize they will have to do this anyway since
upgrading to 9.1 is what necessitates it.
I don't see any backwards compatibility risk, if that's what you mean.
Every backend release since 7.3 has treated client_encoding 'UTF8' and
'UNICODE' the same, and earlier releases didn't accept either one.
regards, tom lane
As there seems to be a consensus forming for fixing the JDBC driver,
I've taken the liberty do so at the risk of being shot down. The patch
is fairly straightforward, just changing UNICODE to UTF8 in a number of
files including the translation files. I've tested this against 9.1devel
(HEAD) and 8.4.7. For each database version I build and the tests
running JDKs 1.4.2_19, 1.5.0_22 and 1.6.0_2. All on 32-bit.
Regards,
--
Mike Fowler
Registered Linux user: 379787
Index: doc/pgjdbc.xml
===
RCS file: /cvsroot/jdbc/pgjdbc/doc/pgjdbc.xml,v
retrieving revision 1.40
diff -c -r1.40 pgjdbc.xml
*** doc/pgjdbc.xml 25 Dec 2010 07:07:44 - 1.40
--- doc/pgjdbc.xml 18 Apr 2011 16:32:49 -
***
*** 179,185
encoding and you will have problems the moment you store data in it that
does not fit in the seven bit ASCII character set.
If you do not know what your encoding will be or are otherwise unsure
! about what you will be storing the UNICODE encoding
is a reasonable default to use.
--- 179,185
encoding and you will have problems the moment you store data in it that
does not fit in the seven bit ASCII character set.
If you do not know what your encoding will be or are otherwise unsure
! about what you will be storing the UTF8 encoding
is a reasonable default to use.
Index: org/postgresql/core/BaseConnection.java
===
RCS file: /cvsroot/jdbc/pgjdbc/org/postgresql/core/BaseConnection.java,v
retrieving revision 1.23
diff -c -r1.23 BaseConnection.java
*** org/postgresql/core/BaseConnection.java 1 May 2010 14:40:51 - 1.23
--- org/postgresql/core/BaseConnection.java 18 Apr 2011 16:32:49 -
***
*** 96,102
/**
* Encode a string using the database's client_encoding
! * (usually UNICODE, but can vary on older server versions).
* This is used when constructing synthetic resultsets (for
* example, in metadata methods).
*
--- 96,102
/**
* Encode a string using the database's client_encoding
! * (usually UTF8, but can vary on older server versions).
* This is used when constructing synthetic resultsets (for
* example, in metadata methods).
*
Index: org/postgresql/core/v2/ConnectionFactoryImpl.java
===
RCS file: /cvsroot/jdbc/pgjdbc/org/postgresql/core/v2/ConnectionFactoryImpl.java,v
retrieving revision 1.21
diff -c -r1.21 ConnectionFactoryImpl.java
*** org/postgresql/core/v2/ConnectionFactoryImpl.java 31 Mar 2011 03:06:38 - 1.21
--- org/postgresql/core/v2/ConnectionFactoryImpl.java 18 Apr 2011 16:32:49 -
***
*** 380,388
// 7.3 server that defaults to autocommit = off.
if (logger.logDebug())
! logger.debug("Switching to UNICODE client_encoding");
! String sql = "begin; set autocommit = on; set client_encoding = 'UNICODE'; ";
if (dbVersion.compareTo("9.0") >= 0) {
sql += "SET extra_float_digits=3; ";
} else if (dbVersion.compareTo("7.4") >= 0) {
--- 380,388
// 7.3 server that defaults to autocommit = off.
if (logger.logDebug())
! logger.debug("Switching to UTF8 client_encoding");
! String sql = "begin; set autocommit = on; set client_encoding = 'UTF8'; ";
if (dbVersion.compareTo("9.0") >= 0) {
sql += "SET extra_float_digits=3; ";
} else if (dbVersion.compareTo("7.4") >= 0) {
***
*** 391,397
sql += "commit";
SetupQueryRunner.run(protoConnection, sql, false);
! protoConnection.setEncoding(Encoding.getDatabaseEncoding("UNICODE"));
}
else
{
--- 391,397
sql += "commit";
SetupQueryRunner.run(protoConnection, sql, false);
! protoConnection.setEncoding(Encoding.getDatabaseEncoding("UTF8
Re: [JDBC] [HACKERS] JDBC connections to 9.1
On 18/04/11 17:35, Andrew Dunstan wrote: On 04/18/2011 11:25 AM, Tom Lane wrote: What concerns me most is that (assuming my dates are right) the JDBC driver has been broken for 11 days and no one noticed. This would lead me to believe that there is no JDBC build server. What would it take to set one up? +1 for doing something along that line. All you'd need to do is write a step for a buildfarm animal to fetch the JDBC driver and run some tests, and run it in a buildfarm client somewhere. The server code is quite agnostic about the steps that are reported on. IOW in addition to a running buildfarm member you need to write a small amount (< 100 lines, possibly much less) of perl code. cheers andrew I've found the entry on the Developer Wiki (http://wiki.postgresql.org/wiki/PostgreSQL_Buildfarm_Howto). What I'll do is set-up three "farms" on my machine - one for 1.4, one for 1.5 and one for 1.6. It's been a while since I've had an excuse to write some Perl! I can't guarantee when I'll have it done as I'm away for a little over a week from Wednesday and I'm not allowed internet access! Regards, -- Mike Fowler Registered Linux user: 379787 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [JDBC] [HACKERS] JDBC connections to 9.1
On 18/04/11 15:57, Tom Lane wrote: Bernd Helmle writes: If i am reading it correct, it reads "UTF8" from the backend, while expecting "UNICODE" only. Not sure what change has caused this, though. I am --- when I redid the GUC assign_hook logic a few weeks ago, I changed the client_encoding code so that it shows the normalized (official) name of the encoding, not whatever random string the client sent over. For instance, previous versions: regression=# set client_encoding = 'UnIcOdE'; SET regression=# show client_encoding ; client_encoding - UnIcOdE (1 row) versus HEAD: regression=# set client_encoding = 'UnIcOdE'; SET regression=# show client_encoding ; client_encoding - UTF8 (1 row) I wasn't aware that JDBC would fail on that. It's pretty annoying that it does, but maybe we should grin and bear it, ie revert the change to canonicalize the GUC's value? regards, tom lane Am I right in thinking that would be that change committed on the 7th (http://archives.postgresql.org/pgsql-committers/2011-04/msg00039.php) ? I've just run the JDBC test build on my machine and it fails dismally with this very message repeated over and over again. What concerns me most is that (assuming my dates are right) the JDBC driver has been broken for 11 days and no one noticed. This would lead me to believe that there is no JDBC build server. What would it take to set one up? If someone can point me to a test machine I'd happily assist in setting one up. As for the breakage itself I'm OK with a new driver version for a new database version and from my experience people expect that. I recall a number of people asking me if an 8.4 driver would be OK to use against 9 before the 9 version was stable. Regards, -- Mike Fowler Registered Linux user: 379787 -- 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] PG 9.0.3. How to select rows from xml
Hi,
On 02/04/11 20:47, emanov wrote:
Hi all!
What i need is transform xml document to table like that:
insert into tmp("Name", "Value")
select t.Name, t.Value from myxml.Nodes("/doc/person") as
t('Name:varchar|Value:int')
or similar. In fact I have many rows with many columns.
How I can do it with PG 9.0.3 where I can't find xpath_table?
Though deprecated, the xml2 contrib module with xpath_table is still
present. Installation instructions can be found on
http://www.postgresql.org/docs/9.0/static/contrib.html and the module is
documented at http://www.postgresql.org/docs/9.0/static/xml2.html.
Regards,
--
Mike Fowler
Registered Linux user: 379787
--
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] new keywords in 9.1
On 12/03/11 05:18, Robert Haas wrote: XMLEXISTS is pretty horrible in that the syntax apparently requires three new keywords (XMLEXISTS, PASSING, REF) which is pretty lame but I guess it's specified by the standard so I'm not sure there's much we can do about it. The rest look reasonable and necessary AFAICT I can confirm that I added the three keywords as described in the SQL/XML standard (section 8.4). Apologies for the delayed confirmation, I missed the thread when it was started and only noticed when your commit message arrived. Regards, -- Mike Fowler Registered Linux user: 379787 -- 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] Native XML
On 27/02/11 19:37, David E. Wheeler wrote: On Feb 27, 2011, at 11:23 AM, Tom Lane wrote: Well, that's why I asked --- if it's going to be a huge chunk of code, then I agree this is the wrong path to pursue. However, I do feel that libxml pretty well sucks, so if we could replace it with a relatively small amount of code, that might be the right thing to do. I think that XML parsers must be hard to get really right, because of all those I've used in Perl, XML::LibXML is far and away the best. Its docs suck, but it does the work really well. No, because the xpath stuff is fundamentally broken, and nobody seems to know how to make libxslt do what we actually need. See the open bugs on the TODO list. XPath is broken? I use it heavily in the Perl module Test::XPath and now, in PostgreSQL, with my explanation extension. http://github.com/theory/explanation/ Is this something I need to worry about I don't believe that XPath is "fundamentally broken", but I think Tom may have meant xslt. When reviewing a recent patch to xml2/xslt I found a few bugs in the way were using libxslt, as well as a bug in the library itself (see http://archives.postgresql.org/pgsql-hackers/2011-02/msg01878.php). However if Tom does mean that xpath is the culprit, it may be with the way the libxml2 library works. It's a very messy singleton. If I'm wrong, I'm sure I'll be corrected! Regards, -- Mike Fowler Registered Linux user: 379787 -- 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] First patch proposal
On 14/10/10 15:53, Alastair Turner wrote: It isn't a TODO item, or related to any previous thread I could find. It's certainly something I can see a use for. When I'm having a bad typing day I get annoyed that I find I've made a mistake after I've typed the password. To me this is a feature that will just make life a little more pleasant for command line junkies like me. Regards, -- Mike Fowler Registered Linux user: 379787 -- 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] First patch proposal
On 14/10/10 15:53, Alastair Turner wrote: It isn't a TODO item, or related to any previous thread I could find. It's certainly something I can see a use for. When I'm having a bad typing day I get annoyed that I find I've made a mistake after I've typed the password. To me this is a feature that will just make life a little more pleasant for command line junkies like me. Regards, -- Mike Fowler Registered Linux user: 379787 NB: Post attmpt two, yesterday's was never delievered to hackers so apologies if Alastair and Hitoshi have seen this message once already. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: [HACKERS] XML schema validation
On 15/10/10 15:08, Tomáš Pospíšil wrote: Hi Robert, I would like to contribute for community. Under licence used by PostgreSQL. So is (or was) there anybody with the same interest as me? Yes, I've been looking at improving the XML support overall, or at least working to finish the implementation of the SQL 2008 XML specification. Currently I've been looking at implementing XQuery, however I have recently started to come to the conclusion that Schema validation should be implemented first as there is some overlap of functionality. What have you looked at? Do you have an outline design you could share on how you would go about adding schema validation? Regrads, -- Mike Fowler Registered Linux user: 379787 -- 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] standby registration (was: is sync rep stalled?)
On Mon, Oct 4, 2010 at 3:25 PM, David Christensen wrote:
>
> On Oct 4, 2010, at 2:02 PM, Robert Haas wrote:
>
>> On Mon, Oct 4, 2010 at 1:57 PM, Markus Wanner wrote:
>>> On 10/04/2010 05:20 PM, Robert Haas wrote:
>>>> Quorum commit, even with configurable vote weights, can't handle a
>>>> requirement that a particular commit be replicated to (A || B) && (C
>>>> || D).
>>>
>>> Good point.
>>>
>>> Can the proposed standby registration configuration format cover such a
>>> requirement?
>>
>> Well, if you can name the standbys, there's no reason there couldn't
>> be a parameter that takes a string that looks pretty much like the
>> above. There are, of course, some situations that could be handled
>> more elegantly by quorum commit ("any 3 of 5 available standbys") but
>> the above is more general and not unreasonably longwinded for
>> reasonable numbers of standbys.
>
>
> Is there any benefit to be had from having standby roles instead of
> individual names? For instance, you could integrate this into quorum commit
> to express 3 of 5 "reporting" standbys, 1 "berlin" standby and 1 "tokyo"
> standby from a group of multiple per data center, or even just utilize role
> sizes of 1 if you wanted individual standbys to be "named" in this fashion.
> This role could be provided on connect of the standby is more-or-less
> tangential to the specific registration issue.
>
Big +1 FWIW.
--
Mike Rylander
--
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: General purpose utility functions used by the JSON data type
On 13/08/10 10:45, Joseph Adams wrote: This patch doesn't include tests . How would I go about writing them? I have made the JSON data type built-in, and I will post that patch shortly (it depends on this one). The built-in JSON data type uses all of these utility functions, and the tests for the JSON data type pass. Joseph, Most existing data types have a regression SQL script in src/test/regress/sql. Using one of them as an example to draw some inspiration from, you should be able to write a 'json.sql' script that exercises the data type and it's supporting functions. Full instructions can be found at http://wiki.postgresql.org/wiki/Regression_test_authoring Regards, -- Mike Fowler Registered Linux user: 379787 -- 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: xml_is_well_formed
On 11/08/10 21:27, Tom Lane wrote:
Robert Haas writes:
On Mon, Aug 9, 2010 at 10:41 AM, Robert Haas wrote:
There's also the fact that it would probably end up parsing the data
twice. Given xmloption, I'm inclined to think Tom has it right:
provided xml_is_well_formed() that follows xmloption, plus a specific
version for each of content and document.
Another reasonable option here would be to forget about having
xml_is_well_formed() per se and ONLY offer
xml_is_well_formed_content() and xml_is_well_formed_document().
We already have xml_is_well_formed(); just dropping it doesn't seem like
a helpful choice.
As a project management note, this CommitFest is over in 4 days, so
unless we have a new version of this patch real soon now we need to
defer it to the September 15th CommitFest
Yes. Mike, are you expecting to submit a new version before the end of
the week?
Yes and here it is, apologies for the delay. I have re-implemented
xml_is_well_formed such that it is sensitive to the XMLOPTION. The
additional _document and _content methods are now present. Tests and
documentation adjusted to suit.
Regards,
--
Mike Fowler
Registered Linux user: 379787
*** a/contrib/xml2/pgxml.sql.in
--- b/contrib/xml2/pgxml.sql.in
***
*** 5,18 SET search_path = public;
--SQL for XML parser
- CREATE OR REPLACE FUNCTION xml_is_well_formed(text) RETURNS bool
- AS 'MODULE_PATHNAME'
- LANGUAGE C STRICT IMMUTABLE;
-
-- deprecated old name for xml_is_well_formed
CREATE OR REPLACE FUNCTION xml_valid(text) RETURNS bool
! AS 'MODULE_PATHNAME', 'xml_is_well_formed'
! LANGUAGE C STRICT IMMUTABLE;
CREATE OR REPLACE FUNCTION xml_encode_special_chars(text) RETURNS text
AS 'MODULE_PATHNAME'
--- 5,14
--SQL for XML parser
-- deprecated old name for xml_is_well_formed
CREATE OR REPLACE FUNCTION xml_valid(text) RETURNS bool
! AS 'xml_is_well_formed'
! LANGUAGE INTERNAL STRICT IMMUTABLE;
CREATE OR REPLACE FUNCTION xml_encode_special_chars(text) RETURNS text
AS 'MODULE_PATHNAME'
*** a/contrib/xml2/uninstall_pgxml.sql
--- b/contrib/xml2/uninstall_pgxml.sql
***
*** 29,33 DROP FUNCTION xml_encode_special_chars(text);
-- deprecated old name for xml_is_well_formed
DROP FUNCTION xml_valid(text);
-
- DROP FUNCTION xml_is_well_formed(text);
--- 29,31
*** a/contrib/xml2/xpath.c
--- b/contrib/xml2/xpath.c
***
*** 27,33 PG_MODULE_MAGIC;
/* externally accessible functions */
- Datum xml_is_well_formed(PG_FUNCTION_ARGS);
Datum xml_encode_special_chars(PG_FUNCTION_ARGS);
Datum xpath_nodeset(PG_FUNCTION_ARGS);
Datum xpath_string(PG_FUNCTION_ARGS);
--- 27,32
***
*** 71,97 pgxml_parser_init(void)
}
- /* Returns true if document is well-formed */
-
- PG_FUNCTION_INFO_V1(xml_is_well_formed);
-
- Datum
- xml_is_well_formed(PG_FUNCTION_ARGS)
- {
- text *t = PG_GETARG_TEXT_P(0); /* document buffer */
- int32 docsize = VARSIZE(t) - VARHDRSZ;
- xmlDocPtr doctree;
-
- pgxml_parser_init();
-
- doctree = xmlParseMemory((char *) VARDATA(t), docsize);
- if (doctree == NULL)
- PG_RETURN_BOOL(false); /* i.e. not well-formed */
- xmlFreeDoc(doctree);
- PG_RETURN_BOOL(true);
- }
-
-
/* Encodes special characters (<, >, &, " and \r) as XML entities */
PG_FUNCTION_INFO_V1(xml_encode_special_chars);
--- 70,75
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***
*** 8625,8630 SELECT xmlexists('//town[text() = ''Toronto'']' PASSING BY REF 'Tor
--- 8625,8736
supports XPath, which is a subset of XQuery.
+
+
+ xml_is_well_formed
+
+
+ xml_is_well_formed
+ well formed
+
+
+
+ xml_is_well_formed(text)
+
+
+
+ The function xml_is_well_formed evaluates whether
+ the text is well formed XML content, returning
+ a boolean. It is useful for predetermining whether a cast to the XML type
+ will succeed, and therefore honours the current value of the
+ XMLOPTION session configuration parameter.
+
+
+ Example:
+
+
+
+ In addition to the structure checks, the function ensures that namespaces are correcty matched.
+
+
+
+
+
+ xml_is_well_formed_document
+
+
+ xml_is_well_formed_document
+ well formed
+
+
+
+ xml_is_well_formed_document(text)
+
+
+
+ This function is similar to xml_is_well_formed,
+ differing only in the handling of the XMLOPTION. The
+ xml_is_well_formed_document ignores XMLOPTION
+ and assumes that it is currently set to DOCUMENT. Therfore
+ it assists in predetermining whether a cast to the XML document type
+ will succeed.
+
+
+
+
+ xml_is_well_formed_content
+
+
+ xml_is_well_formed_content
+
Re: [HACKERS] Initial review of xslt with no limits patch
On 09/08/10 04:07, Tom Lane wrote: Robert Haas writes: On Sun, Aug 8, 2010 at 11:36 AM, Mike Fowler wrote: 1) XML2 is largely undocumented, giving rise to the problems encountered. Since the module is deprecated anyways, does it make more sense to get xslt handling moved into core and get it fully documented? Yes, I think that would be better. I'm hesitant to consider pulling this into core when there's so little consensus on how it ought to act. It'd be better to have a solid, widely used contrib module *first*, rather than imagine that pulling it into core is somehow a cure for its problems. Perhaps the first step forward is to pull xslt_process out of xml2 and create a standalone xslt contrib module. Then at least it can lose the stigma of being in a deprecated module and perhaps make it more visible to users. 2) Pavel's regression test exposes a bug in libxslt. The stylesheet declares 5 parameters, but uses 12. Simplifying, take the stylesheet: I'm not sure whether there's anything we can do about this. We should file a bug report with the libxslt authors, obviously. Turns out the bug was filed in 2005 (see https://bugzilla.gnome.org/show_bug.cgi?id=307061). They are currently taking a fairly loose interpretation of the XSLT spec. However that was only one aspect of the concern. The other was that no errors were being reported back in psql when the libxslt is generating errors. Is this desirable? Regards, -- Mike Fowler Registered Linux user: 379787 -- 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] Initial review of xslt with no limits patch
On 06/08/10 17:50, Pavel Stehule wrote: attached updated patch with regression test Bravely ignoring the quotation/varidic/ conversations, I've taken a look at the patch as is. Thanks to Tom's input I can now correctly drive the function. I can also report that code is now behaving in the expected way. I have two other observations, more directed at the community than Pavel: 1) XML2 is largely undocumented, giving rise to the problems encountered. Since the module is deprecated anyways, does it make more sense to get xslt handling moved into core and get it fully documented? 2) Pavel's regression test exposes a bug in libxslt. The stylesheet declares 5 parameters, but uses 12. Simplifying, take the stylesheet: http://www.w3.org/1999/XSL/Transform"; version="1.0"> and run the command: ~/temp$ xsltproc --stringparam n2 "v2" Untitled2.xsl Untitled1.xml v2 All looks good. However if you run: ~/temp$ xsltproc --stringparam n1 "v1" Untitled2.xsl Untitled1.xml runtime error: file Untitled2.xsl line 28 element value-of Variable 'n2' has not been declared. xmlXPathCompiledEval: evaluation failed runtime error: file Untitled2.xsl line 28 element value-of XPath evaluation returned no result. v1 The xslt_process function ignores these errors and returns cleanly. To summarize, the bug in libxslt is that it allows the processing of unnamed parameters - most other parsers would reject this stylesheet. Secondly, the xslt_process does not return the errors reported when running without passing the unnamed parameter. Personally I would like to see this documented and moved to core so that the whole of xml2 can be dropped. I also think that the errors should be reported, even if libxslt doesn't behave properly in all scenarios. Of course there's that whole other issue around how you pass the parameters in the first place that needs resolving too... Regards, -- Mike Fowler Registered Linux user: 379787 -- 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: xml_is_well_formed
On 06/08/10 21:55, Peter Eisentraut wrote: On fre, 2010-08-06 at 14:43 +0100, Mike Fowler wrote: Or perhaps it could return a string instead of a boolean: content, document, or NULL if it's neither. I like the sound of that. In fact this helps workaround the IS DOCUMENT and IS CONTENT limitations such that you can you can select only content, only documents or both is you use IS NOT NULL. Unless anyone sees a reason that this function needs to remain a boolean function, I'll rework the patch over the weekend. What is the actual use case for this function? Is the above behavior actually useful? The idea is to be able to filter a table that contains XML in TEXT that might not be well formed. Knowing that you're only dealing with well formed XML prevents you blowing up when you attempt the cast. One reason to stick with boolean is backward compatibility. To be honest I'm happiest with returning a boolean, even if there is some confusion over content only being valid. Though changing the return value to DOCUMENT/CONTENT/NULL makes things a touch more explicit, the same results can be achieved by simply running: SELECT data::xml FROM mixed WHERE xml_is_well_formed(data) AND data::xml IS DOCUMENT; Regards, -- Mike Fowler Registered Linux user: 379787 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: Review: Re: [PATCH] Re: [HACKERS] Adding xpath_exists function
On 06/08/10 20:55, Peter Eisentraut wrote:
On fre, 2010-08-06 at 09:04 +0100, Mike Fowler wrote:
If the patch is to be committed, does it make sense for me to refine
it such that it uses the new xpath internal function you extracted in
the xmlexists patch?
Yes, you can probably shrink this patch down to about 20 lines.
Updated the patch so that it will apply to head and re-worked the
function to use the new xpath internal function.
Regards,
--
Mike Fowler
Registered Linux user: 379787
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***
*** 8693,8698 SELECT xpath('//mydefns:b/text()', 'http://example.com";>test
+
+
+ xpath_exists
+
+
+ xpath_exists
+
+
+
+ xpath_exists(xpath, xml, nsarray)
+
+
+
+ The function xpath_exists is a specialised form
+ of the xpath function. Though the functions are
+ syntactically the same the xpath expressions are evaluated in differing
+ contexts. Instead of returning the XML values that satisfy the xpath, this
+ function returns a boolean indicating whether the query was satisfied or not.
+
+
+
+ Example:
+
+
+
*** a/src/backend/utils/adt/xml.c
--- b/src/backend/utils/adt/xml.c
***
*** 3541,3543 Datum xmlexists(PG_FUNCTION_ARGS)
--- 3541,3567
return 0;
#endif
}
+
+ /*
+ * Determines if the node specified by the supplied XPath exists
+ * in a given XML document, returning a boolean. Differs from
+ * xmlexists as it supports namespaces and is not defined in SQL/XML.
+ */
+ Datum
+ xpath_exists(PG_FUNCTION_ARGS)
+ {
+ #ifdef USE_LIBXML
+ text *xpath_expr_text = PG_GETARG_TEXT_P(0);
+ xmltype*data = PG_GETARG_XML_P(1);
+ ArrayType *namespaces = PG_GETARG_ARRAYTYPE_P(2);
+ int res_nitems;
+
+ xpath_internal(xpath_expr_text, data, namespaces,
+ &res_nitems, NULL);
+
+ PG_RETURN_BOOL(res_nitems > 0);
+ #else
+ NO_XML_SUPPORT();
+ return 0;
+ #endif
+ }
*** a/src/include/catalog/pg_proc.h
--- b/src/include/catalog/pg_proc.h
***
*** 4390,4395 DESCR("evaluate XPath expression");
--- 4390,4400
DATA(insert OID = 2614 ( xmlexists PGNSP PGUID 12 1 0 0 f f f t f i 2 0 16 "25 142" _null_ _null_ _null_ _null_ xmlexists _null_ _null_ _null_ ));
DESCR("test XML value against XPath expression");
+ DATA(insert OID = 3037 ( xpath_exists PGNSP PGUID 12 1 0 0 f f f t f i 3 0 16 "25 142 1009" _null_ _null_ _null_ _null_ xpath_exists _null_ _null_ _null_ ));
+ DESCR("evaluate XPath expression in a boolean context, with namespaces support");
+ DATA(insert OID = 3038 ( xpath_exists PGNSP PGUID 14 1 0 0 f f f t f i 2 0 16 "25 142" _null_ _null_ _null_ _null_ "select pg_catalog.xpath_exists($1, $2, ''{}''::pg_catalog.text[])" _null_ _null_ _null_ ));
+ DESCR("evaluate XPath expression in a boolean context");
+
/* uuid */
DATA(insert OID = 2952 ( uuid_in PGNSP PGUID 12 1 0 0 f f f t f i 1 0 2950 "2275" _null_ _null_ _null_ _null_ uuid_in _null_ _null_ _null_ ));
DESCR("I/O");
*** a/src/include/utils/xml.h
--- b/src/include/utils/xml.h
***
*** 37,42 extern Datum texttoxml(PG_FUNCTION_ARGS);
--- 37,43
extern Datum xmltotext(PG_FUNCTION_ARGS);
extern Datum xmlvalidate(PG_FUNCTION_ARGS);
extern Datum xpath(PG_FUNCTION_ARGS);
+ extern Datum xpath_exists(PG_FUNCTION_ARGS);
extern Datum xmlexists(PG_FUNCTION_ARGS);
extern Datum table_to_xml(PG_FUNCTION_ARGS);
*** a/src/test/regress/expected/xml.out
--- b/src/test/regress/expected/xml.out
***
*** 502,507 SELECT xpath('//b', 'one two three etc');
--- 502,560
{two,etc}
(1 row)
+ -- Test xpath_exists evaluation
+ SELECT xpath_exists('//town[text() = ''Toronto'']','Bidford-on-AvonCwmbranBristol'::xml);
+ xpath_exists
+ --
+ f
+ (1 row)
+
+ SELECT xpath_exists('//town[text() = ''Cwmbran'']','Bidford-on-AvonCwmbranBristol'::xml);
+ xpath_exists
+ --
+ t
+ (1 row)
+
+ INSERT INTO xmltest VALUES (4, 'BudvarfreeCarlinglots'::xml);
+ INSERT INTO xmltest VALUES (5, 'MolsonfreeCarlinglots'::xml);
+ INSERT INTO xmltest VALUES (6, 'http://myns.com";>BudvarfreeCarlinglots'::xml);
+ INSERT INTO xmltest VALUES (7, 'http://myns.com";>MolsonfreeCarlinglots'::xml);
+ SELECT COUNT(id) FROM xmltest WHERE xpath_exists('/menu/beer',data);
+ count
+ ---
+ 0
+ (1 row)
+
+ SELECT COUNT(id) FROM xmltest WHERE xpath_exists('/menu/beers',data);
+ count
+ ---
+ 2
+ (1 row)
+
+ SELECT COUNT(id) FROM xmltest WHERE xpath_exists('/menu/beers/name[text() = ''Molson
Re: [HACKERS] Initial review of xslt with no limits patch
On 06/08/10 15:08, Andrew Dunstan wrote:
On 08/06/2010 02:29 AM, Pavel Stehule wrote:
2010/8/6 David Fetter:
On Fri, Aug 06, 2010 at 05:57:37AM +0200, Pavel Stehule wrote:
2010/8/6 Andrew Dunstan:
On 08/05/2010 06:56 PM, Mike Fowler wrote:
SELECT
xslt_process('cim30400'::text,
$$http://www.w3.org/1999/XSL/Transform";
version="1.0">
[snip]
$$::text, 'n1=v1,n2=v2,n3=v3,n4=v4,n5=v5'::text)
I haven't been paying attention to this, so sorry if this has been
discussed
before, but it just caught my eye. Why are we passing these params
as a
comma-separated list rather than as an array or as a variadic list of
params? This looks rather ugly. What if you want to have a param that
includes a comma?
There is probably problem in pairs - label = value. Can be nice, if we
can use a variadic functions for this, but I am afraid, ...
using a variadic function isn't too much nice now
some xslt_process(xmlsrc, 'n1=v1','n2=v2','n3=v3'
This sounds like the perfect case for pulling hstore into core code. :)
I afraid so integration of hstore can break and block work on real
hash support. I would to have hash tables in core, but with usual
features and usual syntax - like Perl or PHP
Can we just keep this discussion within reasonable bounds? The issue
is not hstore or other hashes, but how to do the param list for xslt
sanely given what we have now. A variadic list will be much nicer than
what is currently proposed.
cheers
andrew
+1
Variadic seems the most sensible to me anyways.
However the more urgent problem is, pending someone spotting an error in
my ways, neither the existing code or the patched code appear able to
evaluate the parameters. Note than in my example I supplied a default
value for the fifth parameter and not even that value is appearing in
the outputted XML.
Regards,
--
Mike Fowler
Registered Linux user: 379787
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
