Re: [HACKERS] Typos/Questions in bloom documentation

[Amit Langote]

On 2016/06/07 14:41, Michael Paquier wrote:
> On Fri, Apr 22, 2016 at 1:25 AM, David G. Johnston
>  wrote:
>> On Wed, Apr 20, 2016 at 9:18 PM, Amit Langote
>>  wrote:
>>> I agree it's unclear.  Does the following make it any better (updated
>>> patch attached):
> 
> I have sent a patch to rework the docs here:
> http://www.postgresql.org/message-id/cab7npqqb8dcfmy1uodmijoszdhbfox-us-uw6rfyrzhpeib...@mail.gmail.com
> This may interest people here.

Thanks, Michael.

Regards,
Amit




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

[Michael Paquier]

On Sun, Jun 5, 2016 at 11:01 PM, Andreas Seltenreich  wrote:
> Creating some foreign tables via postgres_fdw in the regression db of
> master as of de33af8, sqlsmith triggers the following assertion:
>
> TRAP: FailedAssertion("!(const Node*)(var))->type) == T_Var))", File: 
> "deparse.c", Line: 1116)
>
> gdb says var is holding a T_PlaceHolderVar instead.  In a build without
> assertions, it leads to an error later:
>
> ERROR:  cache lookup failed for type 0
>
> Recipe:
>
> --8<---cut here---start->8---
> create extension postgres_fdw;
> create server myself foreign data wrapper postgres_fdw;
> create schema fdw_postgres;
> create user mapping for public server myself options (user :'USER');
> import foreign schema public from server myself into fdw_postgres;
> select subq_0.c0 as c0 from
>(select 31 as c0 from fdw_postgres.a as ref_0
>   where 93 >= ref_0.aa) as subq_0
>right join fdw_postgres.rtest_vview5 as ref_1
>on (subq_0.c0 = ref_1.a )
>where 92 = subq_0.c0;
> --8<---cut here---end--->8---

Open item for 9.6 added.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Improve tab completion for USER MAPPING

[Kyotaro HORIGUCHI]

Hi,

At Tue, 7 Jun 2016 00:03:57 +0900, Masahiko Sawada  
wrote in 

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

[Amit Langote]

On 2016/06/05 23:01, Andreas Seltenreich wrote:
> Creating some foreign tables via postgres_fdw in the regression db of
> master as of de33af8, sqlsmith triggers the following assertion:
> 
> TRAP: FailedAssertion("!(const Node*)(var))->type) == T_Var))", File: 
> "deparse.c", Line: 1116)
> 
> gdb says var is holding a T_PlaceHolderVar instead.  In a build without
> assertions, it leads to an error later:
> 
> ERROR:  cache lookup failed for type 0
> 
> Recipe:
> 
> --8<---cut here---start->8---
> create extension postgres_fdw;
> create server myself foreign data wrapper postgres_fdw;
> create schema fdw_postgres;
> create user mapping for public server myself options (user :'USER');
> import foreign schema public from server myself into fdw_postgres;
> select subq_0.c0 as c0 from
>(select 31 as c0 from fdw_postgres.a as ref_0
> where 93 >= ref_0.aa) as subq_0
>right join fdw_postgres.rtest_vview5 as ref_1
>on (subq_0.c0 = ref_1.a )
>where 92 = subq_0.c0;
> --8<---cut here---end--->8---

Thanks for the example.  It seems that postgres_fdw join-pushdown logic
(within foreign_join_ok()?) should reject a join if any PlaceHolderVars in
its targetlist are required above it.  Tried to do that with the attached
patch which trivially fixes the reported assertion failure.

A guess from my reading of the patch's thread [1] is that to support such
a case, join deparse code should be able to construct subqueries which it
currently doesn't support.  I may be missing something though.

Thanks,
Amit

[1]
https://www.postgresql.org/message-id/CAFjFpRdHgeNOhM0AB6Gxz1eVx_yOqkYwuKddZeB5vPzfBaeCnQ%40mail.gmail.com
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 4d17272..ec86b9a 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -4036,6 +4036,20 @@ foreign_join_ok(PlannerInfo *root, RelOptInfo *joinrel, JoinType jointype,
 			return false;
 	}
 
+	/*
+	 * Cannot push down if any PlaceHolderVars in its result are needed above
+	 * the join.
+	 */
+	foreach(lc, root->placeholder_list)
+	{
+		PlaceHolderInfo	   *phinfo = lfirst(lc);
+		Relidsrelids = joinrel->relids;
+
+		if (bms_nonempty_difference(phinfo->ph_needed, relids) &&
+			bms_is_subset(phinfo->ph_eval_at, relids))
+			return false;
+	}
+
 	/* Save the join clauses, for later use. */
 	fpinfo->joinclauses = joinclauses;
 

-- 
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] Prepared statements and generic plans

[Albe Laurenz]

Bruce Momjian wrote:
>> !distinct column values, a generic plan assumes a column equality
>> !comparison will match 33% of processed rows.  Column statistics
>>
>> ... assumes *that* a column equality comparison will match 33% of *the* 
>> processed rows.
> 
> Uh, that seems overly wordy.  I think the rule is that if the sentence
> makes sense without the words, you should not use them, but it is
> clearly a judgement call in this case.  Do you agree?

My gut feeling is that at least the "the" should be retained, but mine
are the guts of a German speaker.
It is clearly a judgement call, so follow your instincts.

> Updated patch attached.
> 
> One more thing --- there was talk of moving some of this into chapter
> 66, but as someone already mentioned, there are no subsections there
> because it is a dedicated topic:
> 
>   66. How the Planner Uses Statistics.
> 
> I am not inclined to add a prepare-only section to that chapter.  On the
> other hand, the issues described apply to PREPARE and to protocol-level
> prepare, so having it in PREPARE also seems illogical.  However, I am
> inclined to leave it in PREPARE until we are ready to move all of this
> to chapter 66.

I think it would be ok to leave it where it is in your patch; while the
paragraph goes into technical detail, it is still alright in the general
documentation (but only just).

Yours,
Laurenz Albe

-- 
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] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

[Ashutosh Bapat]

On Tue, Jun 7, 2016 at 11:36 AM, Amit Langote  wrote:

> On 2016/06/05 23:01, Andreas Seltenreich wrote:
> > Creating some foreign tables via postgres_fdw in the regression db of
> > master as of de33af8, sqlsmith triggers the following assertion:
> >
> > TRAP: FailedAssertion("!(const Node*)(var))->type) == T_Var))",
> File: "deparse.c", Line: 1116)
> >
> > gdb says var is holding a T_PlaceHolderVar instead.  In a build without
> > assertions, it leads to an error later:
> >
> > ERROR:  cache lookup failed for type 0
> >
> > Recipe:
> >
> > --8<---cut here---start->8---
> > create extension postgres_fdw;
> > create server myself foreign data wrapper postgres_fdw;
> > create schema fdw_postgres;
> > create user mapping for public server myself options (user :'USER');
> > import foreign schema public from server myself into fdw_postgres;
> > select subq_0.c0 as c0 from
> >(select 31 as c0 from fdw_postgres.a as ref_0
> > where 93 >= ref_0.aa) as subq_0
> >right join fdw_postgres.rtest_vview5 as ref_1
> >on (subq_0.c0 = ref_1.a )
> >where 92 = subq_0.c0;
> > --8<---cut here---end--->8---
>

The repro assumes existence of certain tables/views e.g. rtest_vview5, a in
public schema. Their definition is not included here. Although I could
reproduce the issue by adding a similar query in the postgres_fdw
regression tests (see attached patch).


>
> Thanks for the example.  It seems that postgres_fdw join-pushdown logic
> (within foreign_join_ok()?) should reject a join if any PlaceHolderVars in
> its targetlist are required above it.  Tried to do that with the attached
> patch which trivially fixes the reported assertion failure.
>

Although the patch fixes the issue, it's restrictive. The placeholder Vars
can be evaluated locally after the required columns are fetched from the
foreign server. The right fix, therefore, is to build targetlist containing
only the Vars that belong to the foreign tables, which in this case would
contain "nothing". Attached patch does this and fixes the issue, while
pushing down the join. Although, I haven't tried the exact query given in
the report. Please let me know if the patch fixes issue with that query as
well.

The query generated by sqlsmith doesn't seem to return any result since it
assigns 31 to subq_0.c0 and the WHERE clause checks 92 =subq_0.c0. Is that
expected?

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index 35c27e7..f72cff6 100644
--- a/contrib/postgres_fdw/deparse.c
+++ b/contrib/postgres_fdw/deparse.c
@@ -724,21 +724,23 @@ deparse_type_name(Oid type_oid, int32 typemod)
 List *
 build_tlist_to_deparse(RelOptInfo *foreignrel)
 {
 	List	   *tlist = NIL;
 	PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo *) foreignrel->fdw_private;
 
 	/*
 	 * We require columns specified in foreignrel->reltarget->exprs and those
 	 * required for evaluating the local conditions.
 	 */
-	tlist = add_to_flat_tlist(tlist, foreignrel->reltarget->exprs);
+	tlist = add_to_flat_tlist(tlist,
+		 pull_var_clause((Node *) foreignrel->reltarget->exprs,
+		 PVC_RECURSE_PLACEHOLDERS));
 	tlist = add_to_flat_tlist(tlist,
 			  pull_var_clause((Node *) fpinfo->local_conds,
 			  PVC_RECURSE_PLACEHOLDERS));
 
 	return tlist;
 }
 
 /*
  * Deparse SELECT statement for given relation into buf.
  *
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 1de0bc4..50fb5c3 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -2046,20 +2046,37 @@ SELECT t1."C 1" FROM "S 1"."T 1" t1, LATERAL (SELECT DISTINCT t2.c1, t3.c1 FROM
1
1
1
1
1
1
1
1
 (10 rows)
 
+-- query which introduces placeholders in the targetlist
+EXPLAIN (COSTS false, VERBOSE)
+SELECT q.a FROM (SELECT 3 as a FROM ft1 WHERE 10 >= ft1.c1) q RIGHT JOIN ft2 ON (q.a = ft2.c1) WHERE 9 = q.a;
+  QUERY PLAN   
+---
+ Foreign Scan
+   Output: 3
+   Filter: (9 = 3)
+   Relations: (public.ft2) LEFT JOIN (public.ft1)
+   Remote SQL: SELECT NULL FROM ("S 1"."T 1" r2 LEFT JOIN "S 1"."T 1" r4 ON (((3 = r2."C 1")) AND ((10 >= r4."C 1"
+(5 rows)
+
+SELECT q.a FROM (SELECT 3 as a FROM ft1 WHERE 10 >= ft1.c1) q RIGHT JOIN ft2 ON (q.a = ft2.c1) WHERE 9 = q.a;
+ a 
+---
+(0 rows)
+
 -- create another user for permission, user mapping, effective user tests
 CREATE USER view_owner;
 -- grant privileges on ft4 and ft5 to view_owner
 GRANT ALL ON ft4 TO view_owner;
 GRANT ALL ON ft5 TO view_owner;
 

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

[Amit Langote]

Hi Ashutosh,

On 2016/06/07 17:02, Ashutosh Bapat wrote:
> On Tue, Jun 7, 2016 at 11:36 AM, Amit Langote wrote:
>> On 2016/06/05 23:01, Andreas Seltenreich wrote:

...

>>> --8<---cut here---start->8---
>>> create extension postgres_fdw;
>>> create server myself foreign data wrapper postgres_fdw;
>>> create schema fdw_postgres;
>>> create user mapping for public server myself options (user :'USER');
>>> import foreign schema public from server myself into fdw_postgres;
>>> select subq_0.c0 as c0 from
>>>(select 31 as c0 from fdw_postgres.a as ref_0
>>> where 93 >= ref_0.aa) as subq_0
>>>right join fdw_postgres.rtest_vview5 as ref_1
>>>on (subq_0.c0 = ref_1.a )
>>>where 92 = subq_0.c0;
>>> --8<---cut here---end--->8---
>>
> 
> The repro assumes existence of certain tables/views e.g. rtest_vview5, a in
> public schema. Their definition is not included here. Although I could
> reproduce the issue by adding a similar query in the postgres_fdw
> regression tests (see attached patch).

See below for the query I used (almost same as the regression test you added).

>> Thanks for the example.  It seems that postgres_fdw join-pushdown logic
>> (within foreign_join_ok()?) should reject a join if any PlaceHolderVars in
>> its targetlist are required above it.  Tried to do that with the attached
>> patch which trivially fixes the reported assertion failure.
>>
> 
> Although the patch fixes the issue, it's restrictive. The placeholder Vars
> can be evaluated locally after the required columns are fetched from the
> foreign server. The right fix, therefore, is to build targetlist containing
> only the Vars that belong to the foreign tables, which in this case would
> contain "nothing". Attached patch does this and fixes the issue, while
> pushing down the join. Although, I haven't tried the exact query given in
> the report. Please let me know if the patch fixes issue with that query as
> well.

That's the patch I came up with initially but it seemed to me to produce
the wrong result.  Correct me if that is not so:

CREATE EXTENSION IF NOT EXISTS postgres_fdw WITH SCHEMA public;

CREATE SERVER myserver FOREIGN DATA WRAPPER postgres_fdw OPTIONS (dbname
'test');

CREATE USER MAPPING FOR CURRENT_USER SERVER myserver;

CREATE TABLE base1 (a integer);
CREATE TABLE base2 (a integer);

CREATE FOREIGN TABLE fbase1 (a integer) SERVER myserver OPTIONS
(table_name 'base1');

INSERT INTO fbase1 VALUES (1);

CREATE FOREIGN TABLE fbase2 (a integer) SERVER myserver OPTIONS
(table_name 'base2');

INSERT INTO fbase2 VALUES (2);


explain verbose select subq.a, b2.a from (select 1 as a from fbase1 as b1)
as subq right join fbase2 as b2 on (subq.a = b2.a);
  QUERY PLAN

--
 Foreign Scan  (cost=100.00..22423.12 rows=42778 width=8)
   Output: 1, b2.a
   Relations: (public.fbase2 b2) LEFT JOIN (public.fbase1 b1)
   Remote SQL: SELECT r2.a FROM (public.base2 r2 LEFT JOIN public.base1 r4
ON (((1 = r2.a
(4 rows)

select subq.a, b2.a from (select 1 as a from fbase1 as b1) as subq right
join fbase2 as b2 on (subq.a = b2.a);
 a | a
---+---
 1 | 2
(1 row)


 to crosscheck - just using the local tables

explain verbose select subq.a, b2.a from (select 1 as a from base1 as b1)
as subq right join base2 as b2 on (subq.a = b2.a);
  QUERY PLAN

---
 Nested Loop Left Join  (cost=0.00..97614.88 rows=32512 width=8)
   Output: (1), b2.a
   Join Filter: (1 = b2.a)
   ->  Seq Scan on public.base2 b2  (cost=0.00..35.50 rows=2550 width=4)
 Output: b2.a
   ->  Materialize  (cost=0.00..48.25 rows=2550 width=4)
 Output: (1)
 ->  Seq Scan on public.base1 b1  (cost=0.00..35.50 rows=2550 width=4)
   Output: 1
(9 rows)

select subq.a, b2.a from (select 1 as a from base1 as b1) as subq right
join base2 as b2 on (subq.a = b2.a);
 a | a
---+---
   | 2
(1 row)

I thought both queries should produce the same result (the latter).

Which the non-push-down version does:

explain verbose select subq.a, b2.a from (select 1 as a from fbase1 as b1)
as subq right join fbase2 as b2 on (subq.a = b2.a);
  QUERY PLAN

---
 Nested Loop Left Join  (cost=200.00..128737.19 rows=42778 width=8)
   Output: (1), b2.a
   Join Filter: (1 = b2.a)
   ->  Foreign Scan on public.fbase2 b2  (cost=100.00..197.75 rows=2925
width=4)
 Output: b2.a
 Remote SQL: SELECT a FROM public.base2
   ->  Materialize  (cost=100.00..212.38 rows=2925 width=4)
 Output: (1)
 ->  Foreign Scan on public.fbase1 b1  (cost=100.00..197.75
rows=2925 width=4)
   Output: 1
 

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

[Ashutosh Bapat]

> That's the patch I came up with initially but it seemed to me to produce
> the wrong result.  Correct me if that is not so:
>
> CREATE EXTENSION IF NOT EXISTS postgres_fdw WITH SCHEMA public;
>
> CREATE SERVER myserver FOREIGN DATA WRAPPER postgres_fdw OPTIONS (dbname
> 'test');
>
> CREATE USER MAPPING FOR CURRENT_USER SERVER myserver;
>
> CREATE TABLE base1 (a integer);
> CREATE TABLE base2 (a integer);
>
> CREATE FOREIGN TABLE fbase1 (a integer) SERVER myserver OPTIONS
> (table_name 'base1');
>
> INSERT INTO fbase1 VALUES (1);
>
> CREATE FOREIGN TABLE fbase2 (a integer) SERVER myserver OPTIONS
> (table_name 'base2');
>
> INSERT INTO fbase2 VALUES (2);
>
>
> explain verbose select subq.a, b2.a from (select 1 as a from fbase1 as b1)
> as subq right join fbase2 as b2 on (subq.a = b2.a);
>   QUERY PLAN
>
>
> --
>  Foreign Scan  (cost=100.00..22423.12 rows=42778 width=8)
>Output: 1, b2.a
>Relations: (public.fbase2 b2) LEFT JOIN (public.fbase1 b1)
>Remote SQL: SELECT r2.a FROM (public.base2 r2 LEFT JOIN public.base1 r4
> ON (((1 = r2.a
> (4 rows)
>
> select subq.a, b2.a from (select 1 as a from fbase1 as b1) as subq right
> join fbase2 as b2 on (subq.a = b2.a);
>  a | a
> ---+---
>  1 | 2
> (1 row)
>
>
>  to crosscheck - just using the local tables
>
> explain verbose select subq.a, b2.a from (select 1 as a from base1 as b1)
> as subq right join base2 as b2 on (subq.a = b2.a);
>   QUERY PLAN
>
>
> ---
>  Nested Loop Left Join  (cost=0.00..97614.88 rows=32512 width=8)
>Output: (1), b2.a
>Join Filter: (1 = b2.a)
>->  Seq Scan on public.base2 b2  (cost=0.00..35.50 rows=2550 width=4)
>  Output: b2.a
>->  Materialize  (cost=0.00..48.25 rows=2550 width=4)
>  Output: (1)
>  ->  Seq Scan on public.base1 b1  (cost=0.00..35.50 rows=2550
> width=4)
>Output: 1
> (9 rows)
>
> select subq.a, b2.a from (select 1 as a from base1 as b1) as subq right
> join base2 as b2 on (subq.a = b2.a);
>  a | a
> ---+---
>| 2
> (1 row)
>
> I thought both queries should produce the same result (the latter).
>
> Which the non-push-down version does:
>
> explain verbose select subq.a, b2.a from (select 1 as a from fbase1 as b1)
> as subq right join fbase2 as b2 on (subq.a = b2.a);
>   QUERY PLAN
>
>
> ---
>  Nested Loop Left Join  (cost=200.00..128737.19 rows=42778 width=8)
>Output: (1), b2.a
>Join Filter: (1 = b2.a)
>->  Foreign Scan on public.fbase2 b2  (cost=100.00..197.75 rows=2925
> width=4)
>  Output: b2.a
>  Remote SQL: SELECT a FROM public.base2
>->  Materialize  (cost=100.00..212.38 rows=2925 width=4)
>  Output: (1)
>  ->  Foreign Scan on public.fbase1 b1  (cost=100.00..197.75
> rows=2925 width=4)
>Output: 1
>Remote SQL: SELECT NULL FROM public.base1
> (11 rows)
>
> select subq.a, b2.a from (select 1 as a from fbase1 as b1) as subq right
> join fbase2 as b2 on (subq.a = b2.a);
>  a | a
> ---+---
>| 2
> (1 row)
>
>
Thanks for that case.

I thought, columns of inner relation will be set to null during projection
from ForeignScan for joins. But I was wrong. If we want to push-down joins
in this case, we have two solutions
1. Build queries with subqueries at the time of deparsing. Thus a base
relation or join has to include placeholders while being deparsed as a
subquery. This means that the deparser should deparse expression
represented by the placeholder. This may not be possible always.
2. At the time of projection from ForeignScan recognize the nullable
placeholders and nullify them if the corresponding relation is nullified.
That looks like a major surgery.

So, your patch looks to be the correct approach (even after we support
deparsing subqueries). Can you please include a test in regression?

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

Re: [HACKERS] slower connect from hostnossl clients

[Pavel Stehule]

2016-06-07 12:18 GMT+02:00 Magnus Hagander :

>
>
> On Tue, Jun 7, 2016 at 11:31 AM, Pavel Stehule 
> wrote:
>
>>
>>
>> 2016-06-07 11:29 GMT+02:00 Magnus Hagander :
>>
>>>
>>>
>>> On Tue, Jun 7, 2016 at 11:24 AM, Pavel Stehule 
>>> wrote:
>>>
 Hi

 I am testing speed of connection to Postgres.

 The ssl connection is slower, and it is expected. But when I configure
 pg_hba.conf to disable ssl via hostnossl, then ssl is not used, but the
 speed is similar to ssl.

 Is it expected behave?


>>> That's definitely not expected behavior. hostnossl should turn off ssl
>>> which should turn off the overhead completely. Does it make a difference if
>>> you also disable it from the client side?
>>>
>>
>> When I explicitly disabled ssl, then I seen significantly less time
>>
>>
> Intersting. Can you check with a network trace that it actually turns off
> ssl, so nothing is broken there?
>

I tested it on local only. The difference is +/- 5-10 ms, but it is well
visible

My customer tested it on network, but on Windows, and there difference is
about 100ms

Pavel


>
> One thing that could be taking the time is an extra roundtrip -- e.g. it
> tries to connect with ssl fails and retries without. A network trace should
> also make this obvious, and can hopefully show you exactly where in the
> connection the time is spent.
>

See attached log

My pg_hba.conf

# TYPE  DATABASEUSERADDRESS METHOD

# "local" is for Unix domain socket connections only
local   all all trust
# IPv4 local connections:

hostnosslall all 10.151.1.41/32
trust
# IPv6 local connections:
hostall all ::1/128 trust

connection string
host=10.151.1.41   port=5432  dbname=postgres user=pavel

Regards

Pavel


>
>
> --
>  Magnus Hagander
>  Me: http://www.hagander.net/
>  Work: http://www.redpill-linpro.com/
>


log
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] slower connect from hostnossl clients

[Pavel Stehule]

Hi

I am testing speed of connection to Postgres.

The ssl connection is slower, and it is expected. But when I configure
pg_hba.conf to disable ssl via hostnossl, then ssl is not used, but the
speed is similar to ssl.

Is it expected behave?

Regards

Pavel

Re: [HACKERS] slower connect from hostnossl clients

[Pavel Stehule]

2016-06-07 11:29 GMT+02:00 Magnus Hagander :

>
>
> On Tue, Jun 7, 2016 at 11:24 AM, Pavel Stehule 
> wrote:
>
>> Hi
>>
>> I am testing speed of connection to Postgres.
>>
>> The ssl connection is slower, and it is expected. But when I configure
>> pg_hba.conf to disable ssl via hostnossl, then ssl is not used, but the
>> speed is similar to ssl.
>>
>> Is it expected behave?
>>
>>
> That's definitely not expected behavior. hostnossl should turn off ssl
> which should turn off the overhead completely. Does it make a difference if
> you also disable it from the client side?
>

When I explicitly disabled ssl, then I seen significantly less time

Regards

Pavel


>
> --
>  Magnus Hagander
>  Me: http://www.hagander.net/
>  Work: http://www.redpill-linpro.com/
>

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

[Ashutosh Bapat]

On Tue, Jun 7, 2016 at 4:07 PM, Amit Langote 
wrote:

> On 2016/06/07 19:13, Ashutosh Bapat wrote:
> > I thought, columns of inner relation will be set to null during
> projection
> > from ForeignScan for joins. But I was wrong. If we want to push-down
> joins
> > in this case, we have two solutions
> > 1. Build queries with subqueries at the time of deparsing. Thus a base
> > relation or join has to include placeholders while being deparsed as a
> > subquery. This means that the deparser should deparse expression
> > represented by the placeholder. This may not be possible always.
> > 2. At the time of projection from ForeignScan recognize the nullable
> > placeholders and nullify them if the corresponding relation is nullified.
> > That looks like a major surgery.
> > So, your patch looks to be the correct approach (even after we support
> > deparsing subqueries). Can you please include a test in regression?
>
> I added a slightly modified version of your test to the originally posted
> patch.
>
> Looks good to me. If we add a column from the outer relation, the
"NULL"ness of inner column would be more clear. May be we should tweak the
query to produce few more rows, some with non-NULL columns from both the
relations. Also add a note to the comment in the test mentioning that such
a join won't be pushed down for a reader to understand the EXPLAIN output.
Also, you might want to move that test, closer to other un-pushability
tests.


-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

Re: [HACKERS] Problem with dumping bloom extension

[Robert Haas]

On Mon, Jun 6, 2016 at 5:55 PM, Michael Paquier
 wrote:
>> It seems important to get this fixed.  I added it to the open items list.
>
> I added already it as " Access methods created with extensions are
> dumped individually ". That's not specific to bloom.

Oh, sorry, I didn't spot that.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] slower connect from hostnossl clients

[Magnus Hagander]

On Tue, Jun 7, 2016 at 12:41 PM, Andreas Karlsson  wrote:

> On 06/07/2016 12:18 PM, Magnus Hagander wrote:
> > Intersting. Can you check with a network trace that it actually turns
> > off ssl, so nothing is broken there?
> >
> > One thing that could be taking the time is an extra roundtrip -- e.g. it
> > tries to connect with ssl fails and retries without. A network trace
> > should also make this obvious, and can hopefully show you exactly where
> > in the connection the time is spent.
>
> I think this is to be expected given that the backend code initializes the
> TLS connection before it looks at anything in pg_hba.conf. The TLS
> connection setup is done when calling BackendInitialize() which happens
> very early in the life of a backend.
>
> I am not familiar enough with this part of the code to know if there is a
> reasonable way to fix this.


Hm. You're saying it's the actual
loading-of-certificate-and-setting-up-context that's slowing it down, not
the actual connection step?

Interesting, hadn't thought of that. I guess it can be - but it would
definitely be good to identify if that's really the case. If it is there is
definitely some optimization to be done there.


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Re: [HACKERS] slower connect from hostnossl clients

[Andreas Karlsson]

On 06/07/2016 12:18 PM, Magnus Hagander wrote:
> Intersting. Can you check with a network trace that it actually turns
> off ssl, so nothing is broken there?
>
> One thing that could be taking the time is an extra roundtrip -- e.g. it
> tries to connect with ssl fails and retries without. A network trace
> should also make this obvious, and can hopefully show you exactly where
> in the connection the time is spent.

I think this is to be expected given that the backend code initializes 
the TLS connection before it looks at anything in pg_hba.conf. The TLS 
connection setup is done when calling BackendInitialize() which happens 
very early in the life of a backend.


I am not familiar enough with this part of the code to know if there is 
a reasonable way to fix this.


Andreas



--
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] slower connect from hostnossl clients

[Magnus Hagander]

On Tue, Jun 7, 2016 at 11:31 AM, Pavel Stehule 
wrote:

>
>
> 2016-06-07 11:29 GMT+02:00 Magnus Hagander :
>
>>
>>
>> On Tue, Jun 7, 2016 at 11:24 AM, Pavel Stehule 
>> wrote:
>>
>>> Hi
>>>
>>> I am testing speed of connection to Postgres.
>>>
>>> The ssl connection is slower, and it is expected. But when I configure
>>> pg_hba.conf to disable ssl via hostnossl, then ssl is not used, but the
>>> speed is similar to ssl.
>>>
>>> Is it expected behave?
>>>
>>>
>> That's definitely not expected behavior. hostnossl should turn off ssl
>> which should turn off the overhead completely. Does it make a difference if
>> you also disable it from the client side?
>>
>
> When I explicitly disabled ssl, then I seen significantly less time
>
>
Intersting. Can you check with a network trace that it actually turns off
ssl, so nothing is broken there?

One thing that could be taking the time is an extra roundtrip -- e.g. it
tries to connect with ssl fails and retries without. A network trace should
also make this obvious, and can hopefully show you exactly where in the
connection the time is spent.


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Re: [HACKERS] slower connect from hostnossl clients

[Magnus Hagander]

On Tue, Jun 7, 2016 at 11:24 AM, Pavel Stehule 
wrote:

> Hi
>
> I am testing speed of connection to Postgres.
>
> The ssl connection is slower, and it is expected. But when I configure
> pg_hba.conf to disable ssl via hostnossl, then ssl is not used, but the
> speed is similar to ssl.
>
> Is it expected behave?
>
>
That's definitely not expected behavior. hostnossl should turn off ssl
which should turn off the overhead completely. Does it make a difference if
you also disable it from the client side?

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

[Amit Langote]

On 2016/06/07 19:13, Ashutosh Bapat wrote:
> I thought, columns of inner relation will be set to null during projection
> from ForeignScan for joins. But I was wrong. If we want to push-down joins
> in this case, we have two solutions
> 1. Build queries with subqueries at the time of deparsing. Thus a base
> relation or join has to include placeholders while being deparsed as a
> subquery. This means that the deparser should deparse expression
> represented by the placeholder. This may not be possible always.
> 2. At the time of projection from ForeignScan recognize the nullable
> placeholders and nullify them if the corresponding relation is nullified.
> That looks like a major surgery.
> So, your patch looks to be the correct approach (even after we support
> deparsing subqueries). Can you please include a test in regression?

I added a slightly modified version of your test to the originally posted
patch.

Thanks,
Amit.
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 1de0bc4..2dcce36 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -2053,6 +2053,28 @@ SELECT t1."C 1" FROM "S 1"."T 1" t1, LATERAL (SELECT DISTINCT t2.c1, t3.c1 FROM
1
 (10 rows)
 
+-- query which introduces placeholders in the targetlist
+EXPLAIN (COSTS false, VERBOSE)
+SELECT q.a FROM (SELECT 3 as a FROM ft1 WHERE 10 = ft1.c1) q RIGHT JOIN ft2 ON (q.a = ft2.c1) WHERE ft2.c1 = 10;
+  QUERY PLAN  
+--
+ Nested Loop Left Join
+   Output: (3)
+   Join Filter: (3 = ft2.c1)
+   ->  Foreign Scan on public.ft2
+ Output: ft2.c1
+ Remote SQL: SELECT "C 1" FROM "S 1"."T 1" WHERE (("C 1" = 10)) ORDER BY "C 1" ASC NULLS LAST
+   ->  Foreign Scan on public.ft1
+ Output: 3
+ Remote SQL: SELECT NULL FROM "S 1"."T 1" WHERE ((10 = "C 1"))
+(9 rows)
+
+SELECT q.a FROM (SELECT 3 as a FROM ft1 WHERE 10 = ft1.c1) q RIGHT JOIN ft2 ON (q.a = ft2.c1) WHERE ft2.c1 = 10;
+ a 
+---
+  
+(1 row)
+
 -- create another user for permission, user mapping, effective user tests
 CREATE USER view_owner;
 -- grant privileges on ft4 and ft5 to view_owner
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 4d17272..ec86b9a 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -4036,6 +4036,20 @@ foreign_join_ok(PlannerInfo *root, RelOptInfo *joinrel, JoinType jointype,
 			return false;
 	}
 
+	/*
+	 * Cannot push down if any PlaceHolderVars in its result are needed above
+	 * the join.
+	 */
+	foreach(lc, root->placeholder_list)
+	{
+		PlaceHolderInfo	   *phinfo = lfirst(lc);
+		Relidsrelids = joinrel->relids;
+
+		if (bms_nonempty_difference(phinfo->ph_needed, relids) &&
+			bms_is_subset(phinfo->ph_eval_at, relids))
+			return false;
+	}
+
 	/* Save the join clauses, for later use. */
 	fpinfo->joinclauses = joinclauses;
 
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 6c2b08c..8970fd6 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -484,6 +484,10 @@ SELECT t1c1, avg(t1c1 + t2c1) FROM (SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2
 EXPLAIN (COSTS false, VERBOSE)
 SELECT t1."C 1" FROM "S 1"."T 1" t1, LATERAL (SELECT DISTINCT t2.c1, t3.c1 FROM ft1 t2, ft2 t3 WHERE t2.c1 = t3.c1 AND t2.c2 = t1.c2) q ORDER BY t1."C 1" OFFSET 10 LIMIT 10;
 SELECT t1."C 1" FROM "S 1"."T 1" t1, LATERAL (SELECT DISTINCT t2.c1, t3.c1 FROM ft1 t2, ft2 t3 WHERE t2.c1 = t3.c1 AND t2.c2 = t1.c2) q ORDER BY t1."C 1" OFFSET 10 LIMIT 10;
+-- query which introduces placeholders in the targetlist
+EXPLAIN (COSTS false, VERBOSE)
+SELECT q.a FROM (SELECT 3 as a FROM ft1 WHERE 10 = ft1.c1) q RIGHT JOIN ft2 ON (q.a = ft2.c1) WHERE ft2.c1 = 10;
+SELECT q.a FROM (SELECT 3 as a FROM ft1 WHERE 10 = ft1.c1) q RIGHT JOIN ft2 ON (q.a = ft2.c1) WHERE ft2.c1 = 10;
 
 -- create another user for permission, user mapping, effective user tests
 CREATE USER view_owner;

-- 
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] IPv6 link-local addresses and init data type

[Peter Eisentraut]

On 6/7/16 1:19 AM, Haribabu Kommi wrote:

How about the following case, Do we treat them as same or different?

select 'fe80::%eth1'::inet = 'fe80::%ETH1'::inet;

fe80::%2/64 is only treated as the valid address but not other way as
fe80::/64%2.
Do we need to throw an error in this case or just ignore.


I suspect questions like these are already solved in someone else's IP 
address type/object/class implementation, and we could look there.


--
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [BUGS] Routine analyze of single column prevents standard autoanalyze from running at all

[Jim Nasby]

On 6/6/16 3:23 PM, Josh berkus wrote:

On 06/06/2016 01:38 PM, Tom Lane wrote:


Also, I'd be a bit inclined to disable the counter reset whenever a column
list is specified, disregarding the corner case where a list is given but
it includes all the table's analyzable columns.  It doesn't really seem
worth the effort to account for that case specially (especially after
you consider that index expressions should count as analyzable columns).

Thoughts?


+1.  Better to err on the side of duplicate analyzes than none at all.

Also, I'm not surprised this took so long to discover; I doubt most
users are aware that you *can* analyze individual columns.


Is there any significant advantage to not analyzing all columns? Only 
case I can think of is if you have a fair number of columns that have 
been toasted; otherwise I'd think IO would completely swamp any other 
considerations.

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)   mobile: 512-569-9461


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Typo in pg_visibility

[Amit Langote]

Attached fixes a typo:

s/PG_ALL_VISIBLE/PD_ALL_VISIBLE/g

Thanks,
Amit




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Typo in pg_visibility

[Amit Langote]

On 2016/06/08 9:38, Amit Langote wrote:
> Attached fixes a typo:
> 
> s/PG_ALL_VISIBLE/PD_ALL_VISIBLE/g

Oops.  Made a couple of mistakes there:

Subject: Typo in pg_visibility documentation
Patch: Really attached this time.

Thanks,
Amit
diff --git a/doc/src/sgml/pgvisibility.sgml b/doc/src/sgml/pgvisibility.sgml
index cdd6a6f..48b003d 100644
--- a/doc/src/sgml/pgvisibility.sgml
+++ b/doc/src/sgml/pgvisibility.sgml
@@ -29,7 +29,7 @@
  
 
  
-  Functions which display information about PG_ALL_VISIBLE
+  Functions which display information about PD_ALL_VISIBLE
   are much more costly than those which only consult the visibility map,
   because they must read the relation's data blocks rather than only the
   (much smaller) visibility map.

-- 
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] Reviewing freeze map code

[Jim Nasby]

On 6/6/16 3:57 PM, Peter Geoghegan wrote:

On Mon, Jun 6, 2016 at 11:35 AM, Andres Freund  wrote:

We need a read-only utility which checks that the system is in a correct
and valid state.  There are a few of those which have been built for
different pieces, I believe, and we really should have one for the
visibility map, but I don't think it makes sense to imply in any way
that VACUUM can or should be used for that.


Meh. This is vacuum behaviour that *has existed* up to this point. You
essentially removed it. Sure, I'm all for adding a verification
tool. But that's just pie in the skie at this point.  We have a complex,
data loss threatening feature, which just about nobody can verify at
this point. That's crazy.


FWIW, I agree with the general sentiment. Building a stress-testing
suite would have been a good idea. In general, testability is a design
goal that I'd be willing to give up other things for.


Related to that, I suspect it would be helpful if it was possible to 
test boundary cases in this kind of critical code by separating the 
logic from the underlying implementation. It becomes very hard to verify 
the system does the right thing in some of these scenarios, because it's 
so difficult to put the system into that state to begin with. Stuff that 
depends on burning through a large number of XIDs is an example of that. 
(To be clear, I'm talking about unit-test kind of stuff here, not 
validating an existing system.)

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)   mobile: 512-569-9461


--
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] Tracking wait event for latches

[Michael Paquier]

On Sat, Jun 4, 2016 at 2:41 AM, Robert Haas  wrote:
> On Thu, Jun 2, 2016 at 1:34 AM, Michael Paquier
>  wrote:
>> This patch is shaped this way intentionally based on the feedback I
>> received at PGCon (Robert and others). We could provide a routine that
>> extensions call in _PG_init to register a new latch event name in
>> shared memory, but I didn't see much use in doing so, take for example
>> the case of background worker, it is possible to register a custom
>> string for pg_stat_activity via pgstat_report_activity. One could take
>> advantage of having custom latch wait names in shared memory if an
>> extension has wait points with latches though... But I am still not
>> sure if that's worth the complexity.
>
> I can't see how you could ever guarantee that it wouldn't just fail.
> We allocate a certain amount of "slop" in the main shared memory
> segment, but it's not infinite and can certainly be exhausted.  It
> seems like it would suck if you tried to load your extension and it
> failed because there was no room left for more wait-point names.
> Maybe it would suck less than not having wait-point names, but I'm not
> really sure.  I think we'd do better to get something that handles the
> core stuff well and then consider extensions later or not at all.

Yeah, that's as well my line of thoughts on the matter since the
beginning: keep it simple and done. What is written just after those
words is purely hand-waving and I have no way to prove it, but my
instinctive guess is that more than 90% of the real use cases where we
need to track the latch waits in pgstat would be covered without the
need of this extra shared memory infrastructure for extensions.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] installcheck failing on psql_crosstab

[Michael Paquier]

On Wed, Jun 8, 2016 at 8:21 AM, Alvaro Herrera  wrote:
> Tom Lane wrote:
>> Michael Paquier  writes:
>> > On Tue, Jun 7, 2016 at 12:31 PM, Michael Paquier
>> >  wrote:
>> >> On Tue, Jun 7, 2016 at 12:28 AM, Alvaro Herrera
>> >>  wrote:
>> >>> Hmm, so we could solve the complaint by adding an ANALYZE.  I'm open to
>> >>> that; other opinions?
>>
>> >> We could just enforce work_mem to 64kB and then reset it.
>>
>> > Or just set up work_mem to a wanted value for the duration of the run
>> > of psql_crosstab. Attached is my proposal.
>>
>> I liked the ANALYZE idea better; this seems pretty ad-hoc.
>
> Done that way.

OK, thanks for fixing the issue!
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Parallel pg_dump's error reporting doesn't work worth squat

[Tom Lane]

Kyotaro HORIGUCHI  writes:
> At Mon, 06 Jun 2016 11:12:14 -0400, Tom Lane  wrote in 
> <17504.1465225...@sss.pgh.pa.us>
>> Uh, what?  PQcancel is very carefully coded so that it's safe to use
>> in a signal handler.  If it's doing mallocs someplace, that would be
>> surprising.

> PQcancel is disigned to run in a signal handler on *Linux*, but
> the discussion here is that the equivalent of send/recv and the
> similars on Windows can be blocked by the TerminateThread'ed
> thread via heap lock.

What's your evidence for that?  I'd expect those to be kernel calls,
or whatever the equivalent concept is on Windows.  If they are not,
and in particular if they do malloc's, I think we have got problems
in many other contexts besides parallel dump.

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] Problem with dumping bloom extension

[Stephen Frost]

* Peter Eisentraut (peter.eisentr...@2ndquadrant.com) wrote:
> On 6/7/16 11:16 AM, Stephen Frost wrote:
> >Moved to CLOSE_WAIT.
> 
> Could you add an explanation on the wiki page about what this section means?

I understood it to simply be a step on the way to being resolved- that
is, everything goes through CLOSE_WAIT for some period of time and then
is moved to resolved when it's clear that the consensus is that it's
closed.

That doesn't appear to be the consensus on the meaning though, and I
didn't add it, so I'm not the one to update the wiki page to explain it.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Problem with dumping bloom extension

[Robert Haas]

On Tue, Jun 7, 2016 at 2:40 PM, Peter Eisentraut
 wrote:
> On 6/7/16 11:16 AM, Stephen Frost wrote:
>>
>> Moved to CLOSE_WAIT.
>
> Could you add an explanation on the wiki page about what this section means?

Noah created that section.  My interpretation is that it's supposed to
be for things we think are fixed, but maybe there's a chance they
aren't - like a performance problem that we've patched but not
received confirmation from the original reporter that the patch fixes
it for them.  I'm inclined to think that most issues should just
become "resolved" right away.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Parallel query and temp_file_limit

[Peter Geoghegan]

On Tue, Jun 7, 2016 at 8:32 AM, Robert Haas  wrote:
> You previously offered to write a patch for this.  Are you still
> planning to do that?

OK, I'll get to that in the next few days.

I'm slightly concerned that I might have missed a real problem in the
code. I'll need to examine the issue more closely.

-- 
Peter Geoghegan


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Parallel pg_dump's error reporting doesn't work worth squat

[Kyotaro HORIGUCHI]

At Tue, 07 Jun 2016 15:38:04 -0400, Tom Lane  wrote in 
<24181.1465328...@sss.pgh.pa.us>
> Kyotaro HORIGUCHI  writes:
> > At Mon, 06 Jun 2016 11:12:14 -0400, Tom Lane  wrote in 
> > <17504.1465225...@sss.pgh.pa.us>
> >> Uh, what?  PQcancel is very carefully coded so that it's safe to use
> >> in a signal handler.  If it's doing mallocs someplace, that would be
> >> surprising.
> 
> > PQcancel is disigned to run in a signal handler on *Linux*, but
> > the discussion here is that the equivalent of send/recv and the
> > similars on Windows can be blocked by the TerminateThread'ed
> > thread via heap lock.
> 
> What's your evidence for that?  I'd expect those to be kernel calls,
> or whatever the equivalent concept is on Windows.  If they are not,
> and in particular if they do malloc's, I think we have got problems
> in many other contexts besides parallel dump.

Well, I found that I misunderstood your following sentence.

> Your original suggestion to use write_msg would end up going
> through fprintf, which might well use malloc internally.  (It's
> possible that Windows' version of write() could too, I suppose,
> but that's probably as low-level as we are going to get.)

I took the sentence enclosed by parentheses as that write() or
other syscalls may blocked by heap-lock on Windows. But it should
mean that "We have no way than to regard Windows' version of
write() as a kernel call, that is, heap-lock safe". It seems
quite natural and I totally agree to the judgment.

Consequently, I agree to just use write(), not write_msg() and
consider the combination of PQcancel and TerminateThread safe on
Windows.

Sorry for my confusion.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Reviewing freeze map code

[Amit Kapila]

On Wed, Jun 8, 2016 at 8:37 AM, Robert Haas  wrote:
>
> On Tue, Jun 7, 2016 at 10:19 AM, Amit Kapila 
wrote:
> > I have implemented the above function in attached patch.  Currently, it
> > returns SETOF tupleids, but if we want some variant of same, that should
> > also be possible.
>
> I think we'd want to bump the pg_visibility version to 1.1 and do the
> upgrade dance, since the existing thing was in beta1.
>

Okay, will do it in next version of patch.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Re: [HACKERS] Reviewing freeze map code

[Amit Kapila]

On Tue, Jun 7, 2016 at 11:01 PM, Andres Freund  wrote:>
> I think if we go with the pg_check_visibility approach, we should also
> copy the other consistency checks from vacuumlazy.c, given they can't
> easily be triggered.

Are you referring to checks that are done in lazy_scan_heap() for each
block?  I think the meaning full checks in this context could be (a) page
is marked as visible, but corresponding vm is not marked. (b) page is
marked as all visible and has dead tuples. (c) vm bit indicates frozen, but
page contains non-frozen tuples.

I think right now the design of pg_visibility is such that it returns the
required information at page level to user by means of various functions
like pg_visibility, pg_visibility_map, etc.  If we want to add page level
checks in this new routine as well, then we have to think what should be
the output if such checks fails, shall we issue warning, shall we return
information in some other way.  Also, I think there will be some duplicity
with the already provided information via other functions of this module.

>
> Wonder how we can report both block and tuple
> level issues. Kinda inclined to report everything as a block level
> issue?
>

The way currently this module provides information, it seems better to have
separate API's for block and tuple level inconsistency.  For block level, I
think most of the information can be retrieved by existing API's and for
tuple level, this new API can be used.



With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Re: [HACKERS] parallel.c is not marked as test covered

[Noah Misch]

On Tue, Jun 07, 2016 at 10:24:33PM +, Clément Prévost wrote:
> I also considered setting max_parallel_degree to 1 to make the test more
> futur-proof but there is a rather long discussion on the setting name (
> https://www.postgresql.org/message-id/20160424035859.gb29...@momjian.us) so
> I can't decide on my own if it's worth the explicit dependency.

It wouldn't be a problem to update this test when renaming the setting, but I
didn't see an impending need to use that setting.

> > As of today, "make installcheck" passes with
> > "default_transaction_isolation =
> > serializable" in postgresql.conf.  Let's preserve that property.  You could
> > wrap the parallel queries in "begin isolation level repeatable read;"
> > ... "commit;", or you could SET default_transaction_isolation itself.
> >
> 
> I did add the transaction, but I don't get why this specific test should
> use this specific transaction isolation level.

We disable all parallelism at serializable isolation.  Any other isolation
level would have worked; repeatable read was an arbitrary choice.  I added a
comment to that effect.

> -test: select_into select_distinct select_distinct_on select_implicit 
> select_having subselect union case join aggregates transactions random 
> portals arrays btree_index hash_index update namespace prepared_xacts delete
> +test: select_into select_distinct select_distinct_on select_implicit 
> select_having subselect union case join aggregates transactions random 
> portals arrays btree_index hash_index update namespace prepared_xacts delete 
> select_parallel

I moved the test to a different group, in light of this parallel_schedule
header comment:

# By convention, we put no more than twenty tests in any one parallel group;
# this limits the number of connections needed to run the tests.

> +  exception
> +-- raise custom exception, the original message contains
> +-- a worker PID that must be hidden in the test output
> +when others then raise exception 'Error in worker';

I changed this to keep the main message while overwriting the CONTEXT; a bug
in this area could very well produce some other error rather than no error.


Committed that way.

Thanks,
nm


-- 
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] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

[Noah Misch]

On Tue, Jun 07, 2016 at 09:49:23PM +0900, Amit Langote wrote:
> On Tue, Jun 7, 2016 at 7:47 PM, Ashutosh Bapat wrote:
> > On Tue, Jun 7, 2016 at 4:07 PM, Amit Langote wrote:
> >> On 2016/06/07 19:13, Ashutosh Bapat wrote:
> >> > So, your patch looks to be the correct approach (even after we support
> >> > deparsing subqueries). Can you please include a test in regression?
> >>
> >> I added a slightly modified version of your test to the originally posted
> >> patch.
> >>
> > Looks good to me. If we add a column from the outer relation, the "NULL"ness
> > of inner column would be more clear. May be we should tweak the query to
> > produce few more rows, some with non-NULL columns from both the relations.
> > Also add a note to the comment in the test mentioning that such a join won't
> > be pushed down for a reader to understand the EXPLAIN output. Also, you
> > might want to move that test, closer to other un-pushability tests.
> 
> Done in the attached.  Please check if my comment explains the reason
> of push-down failure correctly.

[Action required within 72 hours.  This is a generic notification.]

The above-described topic is currently a PostgreSQL 9.6 open item.  Robert,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
9.6 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within 72 hours of this
message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping 9.6rc1.  Consequently, I will appreciate your
efforts toward speedy resolution.  Thanks.

[1] 
http://www.postgresql.org/message-id/20160527025039.ga447...@tornado.leadboat.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] Problem with dumping bloom extension

[Noah Misch]

On Tue, Jun 07, 2016 at 03:23:46PM -0400, Robert Haas wrote:
> On Tue, Jun 7, 2016 at 2:40 PM, Peter Eisentraut
>  wrote:
> > On 6/7/16 11:16 AM, Stephen Frost wrote:
> >>
> >> Moved to CLOSE_WAIT.
> >
> > Could you add an explanation on the wiki page about what this section means?
> 
> Noah created that section.  My interpretation is that it's supposed to
> be for things we think are fixed, but maybe there's a chance they
> aren't - like a performance problem that we've patched but not
> received confirmation from the original reporter that the patch fixes
> it for them.  I'm inclined to think that most issues should just
> become "resolved" right away.

Yep, pretty much that.  CLOSE_WAIT is for performance defects, race
conditions, and other defects where a successful fix is difficult to verify
beyond reasonable doubt.  Other things can move directly to "resolved".  I
don't mind if practice diverges from that intent, and I don't really process
the two sections differently.


-- 
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 #14155: bloom index error with unlogged table

[Michael Paquier]

On Wed, Jun 8, 2016 at 1:23 AM, Tom Lane  wrote:
> "David G. Johnston"  writes:
>> On Tue, Jun 7, 2016 at 1:35 AM, Michael Paquier 
>> wrote:
>>> I have finally given a shot at improving the docs with the attached.
>>> Comments are welcome.
>
>> [ assorted comments ]
>
> I adopted most of David's suggestions, whacked it around a bit further
> myself, and committed.  See what you think.

That looks better, thanks.

>> It would be nice to give guidance on selecting a bit size for columns and
>> a signature length.  Yes, Wikipedia covers the topic but to get the reader
>> started some discussion of the relevant trade-offs when using larger
>> numbers than the default would be nice.  I don't suspect using smaller the
>> default values is apt to be worthwhile...
>
> Agreed, but I didn't want to write such text myself.  There's room for
> further improvement here.  I did add a note in the main example about
> what happens with a non-default signature length, but that hardly
> constitutes guidance.
>
> BTW, it seemed to me while generating the example that the planner's
> costing for bloom index searches was unduly pessimistic; maybe there's
> work to do there?

I wanted them to do so to prove that index rechecks are necessary as
false positives can be returned when scanning the index. We could add
an extra example with an index that has a longer signature size... I
am not sure that's worth the complication though.

I am marking this item as closed, in my view things are looking far better.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Rename max_parallel_degree?

[Robert Haas]

On Fri, Jun 3, 2016 at 9:39 AM, Tom Lane  wrote:
> Robert Haas  writes:
>> I think we should just go with max_parallel_workers for a limit on
>> total parallel workers within max_work_processes, and
>> max_parallel_workers_per_gather for a per-Gather limit.  It's slightly
>> annoying that we may end up renaming the latter GUC, but not as
>> annoying as spending another three weeks debating this and missing
>> beta2.
>
> +1.  I'm not as convinced as you are that we'll replace the GUC later,
> but in any case this is an accurate description of the current
> semantics.  And I'm really *not* in favor of fudging the name with
> the intent of changing the GUC's semantics later --- that would fail
> all sorts of compatibility expectations.

Here's a patch change max_parallel_degree to
max_parallel_workers_per_gather, and also changing parallel_degree to
parallel_workers.  I haven't tackled adding a separate
max_parallel_workers, at least not yet.  Are people OK with this?

Note that there is a dump/restore hazard if people have set the
parallel_degree reloption on a beta1 install, or used ALTER { USER |
DATABASE } .. SET parallel_degree.  Can everybody live with that?
Should I bump catversion when applying this?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


parallel-degree-to-workers-v1.patch
Description: binary/octet-stream

-- 
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] Reviewing freeze map code

[Robert Haas]

On Tue, Jun 7, 2016 at 10:19 AM, Amit Kapila  wrote:
> I have implemented the above function in attached patch.  Currently, it
> returns SETOF tupleids, but if we want some variant of same, that should
> also be possible.

I think we'd want to bump the pg_visibility version to 1.1 and do the
upgrade dance, since the existing thing was in beta1.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

[Ashutosh Bapat]

On further thought, I think we need to restrict the join pushdown only for
outer joins. Only those joins can produce NULL rows. If we go with that
change, we will need my changes as well and a testcase with inner join.

On Tue, Jun 7, 2016 at 6:19 PM, Amit Langote 
wrote:

> On Tue, Jun 7, 2016 at 7:47 PM, Ashutosh Bapat wrote:
> > On Tue, Jun 7, 2016 at 4:07 PM, Amit Langote wrote:
> >> On 2016/06/07 19:13, Ashutosh Bapat wrote:
> >> > So, your patch looks to be the correct approach (even after we support
> >> > deparsing subqueries). Can you please include a test in regression?
> >>
> >> I added a slightly modified version of your test to the originally
> posted
> >> patch.
> >>
> > Looks good to me. If we add a column from the outer relation, the
> "NULL"ness
> > of inner column would be more clear. May be we should tweak the query to
> > produce few more rows, some with non-NULL columns from both the
> relations.
> > Also add a note to the comment in the test mentioning that such a join
> won't
> > be pushed down for a reader to understand the EXPLAIN output. Also, you
> > might want to move that test, closer to other un-pushability tests.
>
> Done in the attached.  Please check if my comment explains the reason
> of push-down failure correctly.
>
> Thanks,
> Amit
>



-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

Re: [HACKERS] [BUGS] Routine analyze of single column prevents standard autoanalyze from running at all

[Tom Lane]

Jim Nasby  writes:
> Is there any significant advantage to not analyzing all columns? Only 
> case I can think of is if you have a fair number of columns that have 
> been toasted; otherwise I'd think IO would completely swamp any other 
> considerations.

Yeah, my guess is that the OP's example where analyzing just one column
was significantly cheaper boiled down to some of the other columns being
mostly toasted data.  Otherwise it's hard to see how there's much more
expense in analyzing them all.

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] Reviewing freeze map code

[Amit Kapila]

On Tue, Jun 7, 2016 at 10:10 PM, Masahiko Sawada 
wrote:
>
> Thank you for implementing the patch.
>
> I've not test it deeply but here are some comments.
> This check tool only checks if the frozen page has live-unfrozen tuple.
> That is, it doesn't care in case where the all-frozen page mistakenly
> has dead-frozen tuple.
>

Do you mean to say that we should have a check for ItemIdIsDead() and then
if item is found to be dead, then add it to array of non_frozen items?  If
so, earlier I thought we might not need this check as we are already using
heap_tuple_needs_eventual_freeze(), but now again looking at it, it seems
wise to check for dead items separately as those won't be covered by other
check.

>
> +   /* Clean up. */
> +   if (vmbuffer != InvalidBuffer)
> +   ReleaseBuffer(vmbuffer);
>
> I think that we should use BufferIsValid() here.
>

We can use BufferIsValid() as well, but I am trying to be consistent with
nearby code, refer collect_visibility_data().  We can change at all places
together if people prefer that way.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Re: [HACKERS] COMMENT ON, psql and access methods

[Alvaro Herrera]

Michael Paquier wrote:
> On Thu, Jun 2, 2016 at 3:42 PM, Michael Paquier
>  wrote:
> > On Thu, Jun 2, 2016 at 1:00 PM, Michael Paquier
> >  wrote:
> >> I have added an open item for 9.6 regarding this patch, that would be
> >> good to complete this work in this release for consistency with the
> >> other objects.
> >
> > Doh. I forgot to update psql --help.
> 
> And Query_for_list_of_access_methods was defined more or less twice,
> the one of my patch having an error...

Thanks, I have pushed this.  I only added a "translate_columns" option
to printQuery.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] installcheck failing on psql_crosstab

[Alvaro Herrera]

Tom Lane wrote:
> Michael Paquier  writes:
> > On Tue, Jun 7, 2016 at 12:31 PM, Michael Paquier
> >  wrote:
> >> On Tue, Jun 7, 2016 at 12:28 AM, Alvaro Herrera
> >>  wrote:
> >>> Hmm, so we could solve the complaint by adding an ANALYZE.  I'm open to
> >>> that; other opinions?
> 
> >> We could just enforce work_mem to 64kB and then reset it.
> 
> > Or just set up work_mem to a wanted value for the duration of the run
> > of psql_crosstab. Attached is my proposal.
> 
> I liked the ANALYZE idea better; this seems pretty ad-hoc.

Done that way.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Should phraseto_tsquery('simple', 'blue blue') @@ to_tsvector('simple', 'blue') be true ?

[Tom Lane]

Jean-Pierre Pelletier  writes:
> I wanted to test if phraseto_tsquery(), new with 9.6 could be used for
> matching consecutive words but it won't work for us if it cannot handle
> consecutive *duplicate* words.

> For example, the following returns true:select
> phraseto_tsquery('simple', 'blue blue') @@ to_tsvector('simple', 'blue');

> Is this expected ?

I concur that that seems like a rather useless behavior.  If we have
"x <-> y" it is not possible to match at distance zero, while if we
have "x <-> x" it seems unlikely that the user is expecting us to
treat that identically to "x".  So phrase search simply should not
consider distance-zero matches.

The attached one-liner patch seems to fix this problem, though I am
uncertain whether any other places need to be changed to match.
Also, there is a regression test case that changes:

*** /home/postgres/pgsql/src/test/regress/expected/tstypes.out  Thu May  5 
19:21:17 2016
--- /home/postgres/pgsql/src/test/regress/results/tstypes.out   Tue Jun  7 
17:55:41 2016
***
*** 897,903 
  SELECT ts_rank_cd(' a:1 sa:2A sb:2D g'::tsvector, 'a <-> s:* <-> sa:A');
   ts_rank_cd 
  
!   0.0714286
  (1 row)
  
  SELECT ts_rank_cd(' a:1 sa:2A sb:2D g'::tsvector, 'a <-> s:* <-> sa:B');
--- 897,903 
  SELECT ts_rank_cd(' a:1 sa:2A sb:2D g'::tsvector, 'a <-> s:* <-> sa:A');
   ts_rank_cd 
  
!   0
  (1 row)
  
  SELECT ts_rank_cd(' a:1 sa:2A sb:2D g'::tsvector, 'a <-> s:* <-> sa:B');


I'm not sure if this case is intentionally exhibiting the behavior that
both parts of "s:* <-> sa:A" can be matched to the same lexeme, or if the
result simply wasn't thought about carefully.

regards, tom lane

diff --git a/src/backend/utils/adt/tsvector_op.c b/src/backend/utils/adt/tsvector_op.c
index 591e59c..95ad69b 100644
*** a/src/backend/utils/adt/tsvector_op.c
--- b/src/backend/utils/adt/tsvector_op.c
*** TS_phrase_execute(QueryItem *curitem,
*** 1409,1415 
  		{
  			while (Lpos < Ldata.pos + Ldata.npos)
  			{
! if (WEP_GETPOS(*Lpos) <= WEP_GETPOS(*Rpos))
  {
  	/*
  	 * Lpos is behind the Rpos, so we have to check the
--- 1409,1415 
  		{
  			while (Lpos < Ldata.pos + Ldata.npos)
  			{
! if (WEP_GETPOS(*Lpos) < WEP_GETPOS(*Rpos))
  {
  	/*
  	 * Lpos is behind the Rpos, so we have to check the

-- 
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] parallel.c is not marked as test covered

[Clément Prévost]

>
> On Sun, May 15, 2016 at 12:53:13PM +, Clément Prévost wrote:
> > After some experiments, I found out that, for my setup (9b7bfc3a88ef7b),
> a
> > parallel seq scan is used given both parallel_setup_cost
> > and parallel_tuple_cost are set to 0 and given that the table is at
> least 3
> > times as large as the biggest test table tenk1.
>
> That worked because it pushed the table over the
> create_plain_partial_paths()
> 1000-block (8 MiB) threshold; see
> http://www.postgresql.org/message-id/26842.1462941...@sss.pgh.pa.us
>
> While the way you tested this is not wrong, I recommend instead querying an
> inheritance parent if that achieves no less test coverage.  "SELECT
> count(*)
> FROM a_star" gets a parallel plan under your cost parameters.  That avoids
> building and discarding a relatively-large table.


Nice, this works well and is faster than copying tenk1 aggressively.


> > The attached patch is a regression test using this method that is
> > reasonably small and fast to run. I also hid the workers count from the
> > explain output when costs are disabled as suggested by Tom Lane and
> Robert
> > Haas on this same thread (
> >
> http://www.postgresql.org/message-id/CA+TgmobBQS4ss3+CwoZOKgbsBqKfRndwc=hlialaep5axqc...@mail.gmail.com
> > ).
>
> It seems wrong to me for COSTS OFF to disable output that is not a cost
> estimate.  file_fdw did that, but I don't want to proliferate that loose
> interpretation of COSTS OFF.
>

Agreed, I reverted it.

Unlike the example in the linked-to message, this test won't experience
> variable worker count.  For the query you used and for the a_star query,
> low
> input size makes the planner request exactly one worker.  If later planner
> enhancements give this query a configuration-specific worker count, the
> test
> could capture EXPLAIN output with PL/pgSQL and compare it to a regex.


I also considered setting max_parallel_degree to 1 to make the test more
futur-proof but there is a rather long discussion on the setting name (
https://www.postgresql.org/message-id/20160424035859.gb29...@momjian.us) so
I can't decide on my own if it's worth the explicit dependency.


> > --- /dev/null
> > +++ b/src/test/regress/sql/select_parallel.sql
> > @@ -0,0 +1,22 @@
> > +--
> > +-- PARALLEL
> > +--
> > +
> > +-- setup parallel test
> > +create unlogged table tenk_parallel as table tenk1 with no data;
> > +set parallel_setup_cost=0;
> > +set parallel_tuple_cost=0;
> > +
> > +-- create a table with enough data to trigger parallel behavior
> > +insert into tenk_parallel
> > +  select tenk1.* from tenk1, generate_series(0,2);
> > +
> > +explain (costs off)
> > +  select count(*) from tenk_parallel;
> > +select count(*) from tenk_parallel;
>
> As of today, "make installcheck" passes with
> "default_transaction_isolation =
> serializable" in postgresql.conf.  Let's preserve that property.  You could
> wrap the parallel queries in "begin isolation level repeatable read;"
> ... "commit;", or you could SET default_transaction_isolation itself.
>

I did add the transaction, but I don't get why this specific test should
use this specific transaction isolation level.

On Tue, Jun 7, 2016 at 7:36 PM Peter Eisentraut <
peter.eisentr...@2ndquadrant.com> wrote:

> Please see also my message
> <
> https://www.postgresql.org/message-id/92bec2a5-6d5b-6630-ce4d-2ed76f357...@2ndquadrant.com
> >
> with a similar solution.  That test also contains a query that raises an
> error, which is another code path that we should cover.
>

Added.


select_parallel_regress_astar.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] COMMENT ON, psql and access methods

[Michael Paquier]

On Wed, Jun 8, 2016 at 7:06 AM, Alvaro Herrera  wrote:
> Michael Paquier wrote:
>> On Thu, Jun 2, 2016 at 3:42 PM, Michael Paquier
>>  wrote:
>> > On Thu, Jun 2, 2016 at 1:00 PM, Michael Paquier
>> >  wrote:
>> >> I have added an open item for 9.6 regarding this patch, that would be
>> >> good to complete this work in this release for consistency with the
>> >> other objects.
>> >
>> > Doh. I forgot to update psql --help.
>>
>> And Query_for_list_of_access_methods was defined more or less twice,
>> the one of my patch having an error...
>
> Thanks, I have pushed this.  I only added a "translate_columns" option
> to printQuery.

Thanks.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] slower connect from hostnossl clients

[Tom Lane]

Magnus Hagander  writes:
> One thing that could be taking the time is an extra roundtrip -- e.g. it
> tries to connect with ssl fails and retries without.

I'd assume a priori that that's it.  If so, the fix is to configure libpq
to try non-SSL first not SSL first.  There is an option for that, IIRC.

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] Problem with dumping bloom extension

[Michael Paquier]

On Tue, Jun 7, 2016 at 8:10 PM, Robert Haas  wrote:
> On Mon, Jun 6, 2016 at 5:55 PM, Michael Paquier
>  wrote:
>>> It seems important to get this fixed.  I added it to the open items list.
>>
>> I added already it as " Access methods created with extensions are
>> dumped individually ". That's not specific to bloom.
>
> Oh, sorry, I didn't spot that.

Never mind. I refreshed the wiki again after that, and kept your entry
which uses directly $subject.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [BUGS] BUG #14155: bloom index error with unlogged table

[David G. Johnston]

On Tue, Jun 7, 2016 at 1:35 AM, Michael Paquier 
wrote:

> On Fri, Jun 3, 2016 at 11:25 PM, Tom Lane  wrote:
> > Michael Paquier  writes:
> >> Actually, the docs could be more polished.
> >
> > I think the docs could stand to be rewritten from scratch ;-).  But
> > upthread there was an offer to work on them if we made the code behavior
> > saner.  I've done the latter part, I don't want to do the former.
>
> I have finally given a shot at improving the docs with the attached.
> Comments are welcome.


​It would be nice to give guidance on selecting a bit size for columns and
a signature length​.  Yes, Wikipedia covers the topic but to get the reader
started some discussion of the relevant trade-offs when using larger
numbers than the default would be nice.  I don't suspect using smaller the
default values is apt to be worthwhile...

David J.

Re: [HACKERS] [BUGS] BUG #14155: bloom index error with unlogged table

[David G. Johnston]

On Tue, Jun 7, 2016 at 1:35 AM, Michael Paquier 
wrote:

> On Fri, Jun 3, 2016 at 11:25 PM, Tom Lane  wrote:
> > Michael Paquier  writes:
> >> Actually, the docs could be more polished.
> >
> > I think the docs could stand to be rewritten from scratch ;-).  But
> > upthread there was an offer to work on them if we made the code behavior
> > saner.  I've done the latter part, I don't want to do the former.
>
> I have finally given a shot at improving the docs with the attached.
> Comments are welcome.
>

​Looks good.  Thanks!​

​Some minor word-smithing​ related stuff and one definitional concern:

​"of all indexed attributes and so it can report false positives" -> of all
indexed attributes and as such is prone to reporting false positives;

​"in the set, however" -> "in the set although"
​
​"one only needs a single bloom index (default 80, maximum 4096)" -> ​the
default seems like it would be better placed in the first paragraph of the
intro where "whose size in calculated in bits" is mentioned; or better yet
dropped altogether since the parameters section covers the defaults.

*** "to the number of the column for" - the examples imply that each
parameter refers to columns by name, not number.

"a bloom index  representing first the advantage to be more" - this intro
to the example needs some work.  maybe: "Here is a more complete example of
index definition and usage, as well as a comparison with the equivalent
btree index.  The bloom index is considerably smaller as well as performs
better than the btree index.

---As an aside, is a multi-column index really a fair comparison here?

---Leaving a sequential scan explain analyze in place should be considered.

​"The Bloom opclass interface" -> The Bloom opclass interface requires a
hash function for the indexing datatype and an equality operator for
searching.  The example...(drop the simple conclusion the word the equality
operator part better).

​"are implemented with the module" - are supplied by this module. (side
question, for 10.0 how about we call these extensions instead of modules?)

David J.

Re: [HACKERS] Problem with dumping bloom extension

[Stephen Frost]

Michael,

* Michael Paquier (michael.paqu...@gmail.com) wrote:
> Stephen, are you working on a patch or should I get one out of my
> pocket? That's something we should get fixed quickly. We need as well
> to be careful with the support for COMMENT with access methods, the
> current consensus on the matter is that it will be soon committed.

I'll fix it today.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] [sqlsmith] Failed assertion in postgres_fdw/deparse.c:1116

[Amit Langote]

On Tue, Jun 7, 2016 at 7:47 PM, Ashutosh Bapat wrote:
> On Tue, Jun 7, 2016 at 4:07 PM, Amit Langote wrote:
>> On 2016/06/07 19:13, Ashutosh Bapat wrote:
>> > So, your patch looks to be the correct approach (even after we support
>> > deparsing subqueries). Can you please include a test in regression?
>>
>> I added a slightly modified version of your test to the originally posted
>> patch.
>>
> Looks good to me. If we add a column from the outer relation, the "NULL"ness
> of inner column would be more clear. May be we should tweak the query to
> produce few more rows, some with non-NULL columns from both the relations.
> Also add a note to the comment in the test mentioning that such a join won't
> be pushed down for a reader to understand the EXPLAIN output. Also, you
> might want to move that test, closer to other un-pushability tests.

Done in the attached.  Please check if my comment explains the reason
of push-down failure correctly.

Thanks,
Amit


pgfdw-join-pd-bug-3.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] WIP: Data at rest encryption

[Ants Aasma]

Hi all,

I have been working on data-at-rest encryption support for PostgreSQL.
In my experience this is a common request that customers make. The
short of the feature is that all PostgreSQL data files are encrypted
with a single master key and are decrypted when read from the OS. It
does not provide column level encryption which is an almost orthogonal
feature, arguably better done client side.

Similar things can be achieved with filesystem level encryption.
However this is not always optimal for various reasons. One of the
better reasons is the desire for HSM based encryption in a storage
area network based setup.

Attached to this mail is a work in progress patch that adds an
extensible encryption mechanism. There are some loose ends left to tie
up, but the general concept and architecture is at a point where it's
ready for some feedback, fresh ideas and bikeshedding.

Usage
=

Set up database like so:

(read -sp "Postgres passphrase: " PGENCRYPTIONKEY; echo;
 export PGENCRYPTIONKEY
 initdb -k -K pgcrypto $PGDATA )

Start PostgreSQL:

(read -sp "Postgres passphrase: " PGENCRYPTIONKEY; echo;
 export PGENCRYPTIONKEY
 postgres $PGDATA )

Design
==

The patch adds a new GUC called encryption_library, when specified the
named library is loaded before shared_preload_libraries and is
expected to register its encryption routines. For now the API is
pretty narrow, one parameterless function that lets the extension do
key setup on its own terms, and two functions for
encrypting/decrypting an arbitrary sized block of data with tweak. The
tweak should alter the encryption function so that identical block
contents are encrypted differently based on their location. The GUC
needs to be set at bootstrap time, so it gets set by a new option for
initdb. During bootstrap an encryption sample gets stored in the
control file, enabling useful error messages.

The library name is not stored in controldata. I'm not quite sure
about this decision. On one hand it would be very useful to tell the
user what he needs to get at his data if the configuration somehow
goes missing and it would get rid of the extra GUC. On the other hand
I don't really want to bloat control data, and the same encryption
algorithm could be provided by different implementations.

For now the encryption is done for everything that goes through md,
xlog and slru. Based on a review of read/write/fread/fwrite calls this
list is missing:

* BufFile - needs refactoring
* Logical reorder buffer serialization - probably needs a stream mode
cipher API addition.
* logical_heap_rewrite - can be encrypted as one big block
* 2PC state data - ditto
* pg_stat_statements - query texts get appended so a stream mode
cipher might be needed here too.

copydir needed some changes too because tablespace and database oid
are included in the tweak and so copying also needs to decrypt and
encrypt with the new tweak value.

For demonstration purposes I imported Brian Gladman's AES-128-XTS mode
implementation into pgcrypto and used an environment variable for key
setup. This part is not really in any reviewable state, the XTS code
needs heavy cleanup to bring it up to PostgreSQL coding standards,
keysetup needs something secure, like PBKDF2 or scrypt.

Performance with current AES implementation is not great, but not
horrible either, I'm seeing around 2x slowdown for larger than
shared_buffers, smaller than free memory workloads. However the plan
is to fix this - I have a prototype AES-NI implementation that does
3GB/s per core on my Haswell based laptop (1.25 B/cycle).

Open questions
==

The main questions is what to do about BufFile? It currently provides
both unaligned random access and a block based interface. I wonder if
it would be a good idea to refactor it to be fully block based under
the covers.

I would also like to incorporate some database identifier as a salt in
key setup. However, system identifier stored in control file doesn't
fit this role well. It gets initialized somewhat too late in the
bootstrap process, and more importantly, gets changed on pg_upgrade.
This will make link mode upgrades impossible, which seems like a no
go. I'm torn whether to add a new value for this purpose (perhaps
stored outside the control file) or allow setting of system identifier
via initdb. The first seems like a better idea, the file could double
as a place to store additional encryption parameters, like key length
or different cipher primitive.

Regards,
Ants Aasma
diff --git a/contrib/pgcrypto/Makefile b/contrib/pgcrypto/Makefile
index 18bad1a..04ce887 100644
--- a/contrib/pgcrypto/Makefile
+++ b/contrib/pgcrypto/Makefile
@@ -20,7 +20,7 @@ SRCS		= pgcrypto.c px.c px-hmac.c px-crypt.c \
 		mbuf.c pgp.c pgp-armor.c pgp-cfb.c pgp-compress.c \
 		pgp-decrypt.c pgp-encrypt.c pgp-info.c pgp-mpi.c \
 		pgp-pubdec.c pgp-pubenc.c pgp-pubkey.c pgp-s2k.c \
-		pgp-pgsql.c
+		pgp-pgsql.c xts.c
 
 MODULE_big	= pgcrypto
 OBJS		= $(SRCS:.c=.o) 

Re: [HACKERS] Re: pg9.6 segfault using simple query (related to use fk for join estimates)

[Noah Misch]

On Tue, Jun 07, 2016 at 10:20:44AM -0400, Tom Lane wrote:
> Noah Misch  writes:
> > On Fri, Jun 03, 2016 at 10:32:24AM -0400, Noah Misch wrote:
> >> IMMEDIATE ATTENTION REQUIRED.  This PostgreSQL 9.6 open item is long past 
> >> due
> >> for your status update.  Please reacquaint yourself with the policy on open
> >> item ownership[1] and then reply immediately.  If I do not hear from you by
> >> 2016-06-04 15:00 UTC, I will transfer this item to release management team
> >> ownership without further notice.
> >> [1] 
> >> http://www.postgresql.org/message-id/20160527025039.ga447...@tornado.leadboat.com
> 
> > This PostgreSQL 9.6 open item now needs a permanent owner.  I want 
> > PostgreSQL
> > to have this planner functionality, but I cannot both give it the attention 
> > it
> > needs and meet commitments predating this open item.  Would any other
> > committer like to take ownership?  If this role interests you, please read
> > this thread and the policy linked above, then send an initial status update
> > bearing a date for your subsequent status update.  If the item does not 
> > have a
> > permanent owner by 2016-06-07 22:00 UTC, I will resolve the item by 
> > reverting
> > commits 68d704e and 137805f.
> 
> The state of play here seems to be that Tomas is willing to have a go at
> rewriting the patch per my suggestions, but Simon has not shown any
> indications of responding in a timely fashion; and time is now of the
> essence.
> 
> I am willing to take ownership of this item; but if I do, I will start
> by reverting the aforementioned commits and their followups.  I do not
> think that very much of what's there now will survive without significant
> changes, and to my taste it will be easier to review a rewritten patch
> de novo.  If Tomas is able to produce a rewritten patch within a week
> (by 6/14), I will undertake to review it with an eye to committing by
> the end of next week.  If we are unable to produce something satisfactory
> before beta2, the feature needs to be postponed into the next devel cycle.

Through the lens of open item procedure, I see no defects in your offer of
ownership.  I have updated the open items sheet to reflect you being the new
owner.  Thanks.

My personal opinion is that the community should not undertake a "rewrite" of
a nontrivial feature after freeze.  The fact that a progenitor was present in
the tree at freeze doesn't make the rewrite much less risky than a brand new
feature.  So, I suggest that you instead revert the patches and review that
rewrite for next CommitFest.  Even so, I am okay with your current plan.

Thanks,
nm


-- 
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] installcheck failing on psql_crosstab

[Tom Lane]

Michael Paquier  writes:
> On Tue, Jun 7, 2016 at 12:31 PM, Michael Paquier
>  wrote:
>> On Tue, Jun 7, 2016 at 12:28 AM, Alvaro Herrera
>>  wrote:
>>> Hmm, so we could solve the complaint by adding an ANALYZE.  I'm open to
>>> that; other opinions?

>> We could just enforce work_mem to 64kB and then reset it.

> Or just set up work_mem to a wanted value for the duration of the run
> of psql_crosstab. Attached is my proposal.

I liked the ANALYZE idea better; this seems pretty ad-hoc.

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] Re: pg9.6 segfault using simple query (related to use fk for join estimates)

[Robert Haas]

On Tue, Jun 7, 2016 at 10:20 AM, Tom Lane  wrote:
> Noah Misch  writes:
>> On Fri, Jun 03, 2016 at 10:32:24AM -0400, Noah Misch wrote:
>>> IMMEDIATE ATTENTION REQUIRED.  This PostgreSQL 9.6 open item is long past 
>>> due
>>> for your status update.  Please reacquaint yourself with the policy on open
>>> item ownership[1] and then reply immediately.  If I do not hear from you by
>>> 2016-06-04 15:00 UTC, I will transfer this item to release management team
>>> ownership without further notice.
>>> [1] 
>>> http://www.postgresql.org/message-id/20160527025039.ga447...@tornado.leadboat.com
>
>> This PostgreSQL 9.6 open item now needs a permanent owner.  I want PostgreSQL
>> to have this planner functionality, but I cannot both give it the attention 
>> it
>> needs and meet commitments predating this open item.  Would any other
>> committer like to take ownership?  If this role interests you, please read
>> this thread and the policy linked above, then send an initial status update
>> bearing a date for your subsequent status update.  If the item does not have 
>> a
>> permanent owner by 2016-06-07 22:00 UTC, I will resolve the item by reverting
>> commits 68d704e and 137805f.
>
> The state of play here seems to be that Tomas is willing to have a go at
> rewriting the patch per my suggestions, but Simon has not shown any
> indications of responding in a timely fashion; and time is now of the
> essence.
>
> I am willing to take ownership of this item; but if I do, I will start
> by reverting the aforementioned commits and their followups.  I do not
> think that very much of what's there now will survive without significant
> changes, and to my taste it will be easier to review a rewritten patch
> de novo.  If Tomas is able to produce a rewritten patch within a week
> (by 6/14), I will undertake to review it with an eye to committing by
> the end of next week.  If we are unable to produce something satisfactory
> before beta2, the feature needs to be postponed into the next devel cycle.

That sounds pretty fair to me.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Reviewing freeze map code

[Amit Kapila]

On Tue, Jun 7, 2016 at 2:52 AM, Robert Haas  wrote:
>
> On Mon, Jun 6, 2016 at 5:06 PM, Andres Freund  wrote:
>
>
> > I'd also be ok with adding & documenting (beta release notes)
> > CREATE EXTENSION pg_visibility;
> > SELECT relname FROM pg_class WHERE relkind IN ('r', 'm') AND NOT
pg_check_visibility(oid);
> > or something olong those lines.
>
> That wouldn't be too useful as-written in my book, because it gives
> you no detail on what exactly the problem was.  Maybe it could be
> "pg_check_visibility(regclass) RETURNS SETOF tid", where the returned
> TIDs are non-frozen TIDs on frozen pages.  Then I think something like
> this would work:
>
> SELECT c.oid, pg_check_visibility(c.oid) FROM pg_class WHERE relkind
> IN ('r', 't', 'm');
>

I have implemented the above function in attached patch.  Currently, it
returns SETOF tupleids, but if we want some variant of same, that should
also be possible.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


pg_check_visibility_func_v1.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Re: pg9.6 segfault using simple query (related to use fk for join estimates)

[Tom Lane]

Noah Misch  writes:
> On Fri, Jun 03, 2016 at 10:32:24AM -0400, Noah Misch wrote:
>> IMMEDIATE ATTENTION REQUIRED.  This PostgreSQL 9.6 open item is long past due
>> for your status update.  Please reacquaint yourself with the policy on open
>> item ownership[1] and then reply immediately.  If I do not hear from you by
>> 2016-06-04 15:00 UTC, I will transfer this item to release management team
>> ownership without further notice.
>> [1] 
>> http://www.postgresql.org/message-id/20160527025039.ga447...@tornado.leadboat.com

> This PostgreSQL 9.6 open item now needs a permanent owner.  I want PostgreSQL
> to have this planner functionality, but I cannot both give it the attention it
> needs and meet commitments predating this open item.  Would any other
> committer like to take ownership?  If this role interests you, please read
> this thread and the policy linked above, then send an initial status update
> bearing a date for your subsequent status update.  If the item does not have a
> permanent owner by 2016-06-07 22:00 UTC, I will resolve the item by reverting
> commits 68d704e and 137805f.

The state of play here seems to be that Tomas is willing to have a go at
rewriting the patch per my suggestions, but Simon has not shown any
indications of responding in a timely fashion; and time is now of the
essence.

I am willing to take ownership of this item; but if I do, I will start
by reverting the aforementioned commits and their followups.  I do not
think that very much of what's there now will survive without significant
changes, and to my taste it will be easier to review a rewritten patch
de novo.  If Tomas is able to produce a rewritten patch within a week
(by 6/14), I will undertake to review it with an eye to committing by
the end of next week.  If we are unable to produce something satisfactory
before beta2, the feature needs to be postponed into the next devel cycle.

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] Why we don't have checksums on clog files

[Amit Kapila]

On Mon, Jun 6, 2016 at 8:43 PM, Alex Ignatov 
wrote:

> Hello!
>
> Why we don't have checksums on clog files.
>
> We have checksum on pg_control, optional checksumming on data files, some
> form of checksumming on wal's. But why we don't have any checksumming on
> clogs. Corruptions on clogs lead to transaction visisbility problems and
> database consistency violation.
>
> Can anybody explain this situation with clogs?
>
>
I think it will be better if users can have an option to checksum clog
pages.  However, I think that will need a change in page (CLOG-page) format
which might not be a trivial work to accomplish.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Re: [HACKERS] Why we don't have checksums on clog files

[Alexander Korotkov]

On Tue, Jun 7, 2016 at 5:41 PM, Amit Kapila  wrote:

> On Mon, Jun 6, 2016 at 8:43 PM, Alex Ignatov 
> wrote:
>
>> Why we don't have checksums on clog files.
>>
>> We have checksum on pg_control, optional checksumming on data files, some
>> form of checksumming on wal's. But why we don't have any checksumming on
>> clogs. Corruptions on clogs lead to transaction visisbility problems and
>> database consistency violation.
>>
>> Can anybody explain this situation with clogs?
>>
>>
> I think it will be better if users can have an option to checksum clog
> pages.  However, I think that will need a change in page (CLOG-page) format
> which might not be a trivial work to accomplish.
>

I think we should think no only about CLOG, but about all SLRU pages.
For data pages we have to reinitialize database cluster to enable
checksums. I think we can also require reinitialization to enable checksums
of SLRU pages. Thus, major problem would be that number of items fitting to
page would depend on whether checksums are enabled.  However, it shouldn't
too hard.

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: [HACKERS] Problem with dumping bloom extension

[Stephen Frost]

* Noah Misch (n...@leadboat.com) wrote:
> The above-described topic is currently a PostgreSQL 9.6 open item.  Stephen,
> since you committed the patch believed to have created it, you own this open
> item.  If some other commit is more relevant or if this does not belong as a
> 9.6 open item, please let us know.  Otherwise, please observe the policy on
> open item ownership[1] and send a status update within 72 hours of this
> message.  Include a date for your subsequent status update.  Testers may
> discover new open items at any time, and I want to plan to get them all fixed
> well in advance of shipping 9.6rc1.  Consequently, I will appreciate your
> efforts toward speedy resolution.  Thanks.

Addressed with 562f06f.

Moved to CLOSE_WAIT.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold

[Robert Haas]

On Sat, Jun 4, 2016 at 4:21 PM, Kevin Grittner  wrote:
> On Fri, Jun 3, 2016 at 4:24 PM, Kevin Grittner  wrote:
>> Consequently, causing the index to be ignored in planning when
>> using the old index
>
> That last line should have read "using an old snapshot"
>
>> is not a nice optimization, but necessary for
>> correctness.  We already have logic to do this for other cases
>> (like HOT updates), so it is a matter of tying in to that existing
>> code correctly.  This won't be all that novel.
>
> Just to demonstrate that, the minimal patch to fix behavior in this
> area would be:
>
> diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
> index 31a1438..6c379da 100644
> --- a/src/backend/catalog/index.c
> +++ b/src/backend/catalog/index.c
> @@ -2268,6 +2268,9 @@ IndexBuildHeapRangeScan(Relation heapRelation,
> Assert(numblocks == InvalidBlockNumber);
> }
>
> +   if (old_snapshot_threshold >= 0)
> +   indexInfo->ii_BrokenHotChain = true;
> +
> reltuples = 0;
>
> /*
>
> Of course, ii_BrokenHotChain should be renamed to something like
> ii_UnsafeForOldSnapshots, and some comments need to be updated; but
> the above is the substance of it.

I don't know why we'd want to rename it like that...

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [BUGS] BUG #14155: bloom index error with unlogged table

[Tom Lane]

"David G. Johnston"  writes:
> On Tue, Jun 7, 2016 at 1:35 AM, Michael Paquier 
> wrote:
>> I have finally given a shot at improving the docs with the attached.
>> Comments are welcome.

> [ assorted comments ]

I adopted most of David's suggestions, whacked it around a bit further
myself, and committed.  See what you think.

> ​It would be nice to give guidance on selecting a bit size for columns and
> a signature length​.  Yes, Wikipedia covers the topic but to get the reader
> started some discussion of the relevant trade-offs when using larger
> numbers than the default would be nice.  I don't suspect using smaller the
> default values is apt to be worthwhile...

Agreed, but I didn't want to write such text myself.  There's room for
further improvement here.  I did add a note in the main example about
what happens with a non-default signature length, but that hardly
constitutes guidance.

BTW, it seemed to me while generating the example that the planner's
costing for bloom index searches was unduly pessimistic; maybe there's
work to do there?

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] Parallel safety tagging of extension functions

[Andreas Karlsson]

On 06/07/2016 05:44 PM, Robert Haas wrote:

cube: I think we need a new extension version.
hstore: Does not apply for me.
intarray: Does not apply for me.


Those three and ltree, pg_trgm, and seg depend on my patch with fixes 
for the GiST/GIN function signatures in 
https://www.postgresql.org/message-id/574f091a.3050...@proxel.se. The 
reason for the dependency is that I also mark the those functions with 
changed signatures as parallel safe.


If that patch is not going to be applied I can easily fix those 6 
patches to not depend on the function signature patch.


Sorry for not making this dependency clear.

Andreas


--
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] Parallel query and temp_file_limit

[Robert Haas]

On Sun, Jun 5, 2016 at 4:32 PM, Peter Geoghegan  wrote:
> On Wed, May 18, 2016 at 12:01 PM, Peter Geoghegan  wrote:
>>> I think for 9.6 we just have to document this issue.  In the next
>>> release, we could (and might well want to) try to do something more
>>> clever.
>>
>> Works for me. You may wish to update comments within fd.c at the same time.
>
> I've created a 9.6 open issue for this.

You previously offered to write a patch for this.  Are you still
planning to do that?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Parallel safety tagging of extension functions

[Robert Haas]

On Fri, Jun 3, 2016 at 8:37 PM, Andreas Karlsson  wrote:
> Here is the patch split into many small patches as you suggested. The
> current patches are based on top of the patch which fixes the signatures for
> gin and gist functions.

Generally, I think that there's no point in changing many of these
contrib modules in this way.  There's no use that I can see, for
example, in running a parallel query using any of the adminpack
functions, so why mark them anything other than the default of
parallel-unsafe?

adminpack: Doesn't seem useful.
bloom: As you say, functions are never called directly, so there's no point.
btree_gin: As you say, functions are never called directly, so there's no point.
btree_gist: As you say, functions are never called directly, so
there's no point.
chkpass: Doesn't seem useful.
citext: Committed.
cube: I think we need a new extension version.
dblink: Isn't changing dblink_fdw_validator pointless?  The others I get.
earthdistance: Committed.
file_fdw: As you say, functions are never called directly, so there's no point.
fuzzystrmatch: Committed.
hstore: Does not apply for me.
hstore_plperl: As you say, functions are never called directly, so
there's no point.
hstore_plpython: As you say, functions are never called directly, so
there's no point.
intagg: Committed.
intarray: Does not apply for me.
lo: Committed.

More later.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Re: pg9.6 segfault using simple query (related to use fk for join estimates)

[Tom Lane]

Noah Misch  writes:
> My personal opinion is that the community should not undertake a "rewrite" of
> a nontrivial feature after freeze.  The fact that a progenitor was present in
> the tree at freeze doesn't make the rewrite much less risky than a brand new
> feature.  So, I suggest that you instead revert the patches and review that
> rewrite for next CommitFest.  Even so, I am okay with your current plan.

TBH, I think the odds are very good that that's how it will end up being;
my standards for committing a large patch a few days before beta2 are
going to be quite high.  But I feel it's only fair to offer Tomas the
chance to get something in this year not next year.  Also, even though
this can be expected to be heavily-rewritten code, the fact that there
was a progenitor makes it less risky than a truly new patch would be.

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] [BUGS] BUG #14155: bloom index error with unlogged table

[David G. Johnston]

On Tue, Jun 7, 2016 at 12:23 PM, Tom Lane  wrote:

> "David G. Johnston"  writes:
> > On Tue, Jun 7, 2016 at 1:35 AM, Michael Paquier <
> michael.paqu...@gmail.com>
> > wrote:
> >> I have finally given a shot at improving the docs with the attached.
> >> Comments are welcome.
>
> > [ assorted comments ]
>
> I adopted most of David's suggestions, whacked it around a bit further
> myself, and committed.  See what you think.
>

Looks good though I'm waiting for the website to update.

Do I understand the process correctly?  The current 9.6 docs reflect what
was committed and branched as beta1.  Ongoing work is done against master
(devel docs).  When beta2 is released it is branched from the current
master; not beta1.  At that point the current devel docs will become the
new 9.6 docs.

​Thanks!​

​David J.
​

Re: [HACKERS] Reviewing freeze map code

[Masahiko Sawada]

On Tue, Jun 7, 2016 at 11:19 PM, Amit Kapila  wrote:
> On Tue, Jun 7, 2016 at 2:52 AM, Robert Haas  wrote:
>>
>> On Mon, Jun 6, 2016 at 5:06 PM, Andres Freund  wrote:
>>
>>
>> > I'd also be ok with adding & documenting (beta release notes)
>> > CREATE EXTENSION pg_visibility;
>> > SELECT relname FROM pg_class WHERE relkind IN ('r', 'm') AND NOT
>> > pg_check_visibility(oid);
>> > or something olong those lines.
>>
>> That wouldn't be too useful as-written in my book, because it gives
>> you no detail on what exactly the problem was.  Maybe it could be
>> "pg_check_visibility(regclass) RETURNS SETOF tid", where the returned
>> TIDs are non-frozen TIDs on frozen pages.  Then I think something like
>> this would work:
>>
>> SELECT c.oid, pg_check_visibility(c.oid) FROM pg_class WHERE relkind
>> IN ('r', 't', 'm');
>>
>
> I have implemented the above function in attached patch.  Currently, it
> returns SETOF tupleids, but if we want some variant of same, that should
> also be possible.
>
>
> With Regards,
> Amit Kapila.
> EnterpriseDB: http://www.enterprisedb.com

Thank you for implementing the patch.

I've not test it deeply but here are some comments.
This check tool only checks if the frozen page has live-unfrozen tuple.
That is, it doesn't care in case where the all-frozen page mistakenly
has dead-frozen tuple.
I think this tool should check such case, otherwise the function name
would need to be changed.

+   /* Clean up. */
+   if (vmbuffer != InvalidBuffer)
+   ReleaseBuffer(vmbuffer);

I think that we should use BufferIsValid() here.

Regards,

--
Masahiko Sawada


-- 
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] parallel.c is not marked as test covered

[Peter Eisentraut]

On 6/7/16 1:27 AM, Noah Misch wrote:

Testing under these conditions does not test the planner job at all but at
least some parallel code can be run on the build farm and the test suite
gets 2643 more lines and 188 more function covered.


Nice.


Please see also my message 
 
with a similar solution.  That test also contains a query that raises an 
error, which is another code path that we should cover.


--
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [BUGS] BUG #14155: bloom index error with unlogged table

[Tom Lane]

"David G. Johnston"  writes:
> Do I understand the process correctly?  The current 9.6 docs reflect what
> was committed and branched as beta1.  Ongoing work is done against master
> (devel docs).  When beta2 is released it is branched from the current
> master; not beta1.  At that point the current devel docs will become the
> new 9.6 docs.

There is no beta1 branch.  9.6 is still the master branch, and beta2 will
be stamped on master, as will at least the next couple of betas.  We will
branch off REL9_6_STABLE whenever we're ready to open the tree for
9.7^H^H^H10 development, which likely won't be till September.  (The
point here is to try to keep people focused on stabilizing 9.6 for
awhile longer yet.  Last year we opened new development in the summer,
and that had a detrimental effect on getting 9.5 out the door.)

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] Reviewing freeze map code

[Andres Freund]

On 2016-06-07 19:49:59 +0530, Amit Kapila wrote:
> On Tue, Jun 7, 2016 at 2:52 AM, Robert Haas  wrote:
> >
> > On Mon, Jun 6, 2016 at 5:06 PM, Andres Freund  wrote:
> >
> >
> > > I'd also be ok with adding & documenting (beta release notes)
> > > CREATE EXTENSION pg_visibility;
> > > SELECT relname FROM pg_class WHERE relkind IN ('r', 'm') AND NOT
> pg_check_visibility(oid);
> > > or something olong those lines.
> >
> > That wouldn't be too useful as-written in my book, because it gives
> > you no detail on what exactly the problem was.  Maybe it could be
> > "pg_check_visibility(regclass) RETURNS SETOF tid", where the returned
> > TIDs are non-frozen TIDs on frozen pages.  Then I think something like
> > this would work:
> >
> > SELECT c.oid, pg_check_visibility(c.oid) FROM pg_class WHERE relkind
> > IN ('r', 't', 'm');
> >
> 
> I have implemented the above function in attached patch.  Currently, it
> returns SETOF tupleids, but if we want some variant of same, that should
> also be possible.

Cool!

I think if we go with the pg_check_visibility approach, we should also
copy the other consistency checks from vacuumlazy.c, given they can't
easily be triggered.  Wonder how we can report both block and tuple
level issues. Kinda inclined to report everything as a block level
issue?

Regards,

Andres


-- 
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] Problem with dumping bloom extension

[Peter Eisentraut]

On 6/7/16 11:16 AM, Stephen Frost wrote:

Moved to CLOSE_WAIT.


Could you add an explanation on the wiki page about what this section means?

--
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Should phraseto_tsquery('simple', 'blue blue') @@ to_tsvector('simple', 'blue') be true ?

[Jean-Pierre Pelletier]

Hi,

I wanted to test if phraseto_tsquery(), new with 9.6 could be used for
matching consecutive words but it won't work for us if it cannot handle
consecutive *duplicate* words.

For example, the following returns true:select
phraseto_tsquery('simple', 'blue blue') @@ to_tsvector('simple', 'blue');

Is this expected ?

Thanks,
Jean-Pierre Pelletier


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers