Re: [HACKERS] proposal: PL/Pythonu - function ereport

[Catalin Iacob]

On Thu, Dec 3, 2015 at 6:54 PM, Pavel Stehule  wrote:
> Don't understand - if Fatal has same behave as Error, then why it cannot be
> inherited from Error?
>
> What can be broken?

Existing code that did "except plpy.Error as exc" will now also be
called if plpy.Fatal is raised. That wasn't the case so this changes
meaning of existing code, therefore it introduces an incompatibility.

>>
>> 5. all the reporting functions: plpy.debug...plpy.fatal functions
>> should also be changed to allow more arguments than the message and
>> allow them as keyword arguments
>
>
> this is maybe bigger source of broken compatibility
>
> lot of people uses plpy.debug(var1, var2, var3)
>
> rich constructor use $1 for message, $2 for detail, $3 for hint. So it was a
> reason, why didn't touch these functions

No, if you use PyArg_ParseTupleAndKeywords you'll have Python accept
all of these so previous ways are still accepted:

plpy.error('a_msg', 'a_detail', 'a_hint')
plpy.error'a_msg', 'a_detail', hint='a_hint')
plpy.error('a_msg', detail='a_detail', hint='a_hint')
plpy.error(msg='a_msg', detail='a_detail', hint='a_hint')
plpy.error('a_msg')
plpy.error(msg='a_msg')
etc.

But I now see that even though the docs say plpy.error and friends
take a single msg argument, they actually allow any number of
arguments. If multiple arguments are passed they are converted to a
tuple and the string representation of that tuple goes into
ereport/plpy.Error:

CREATE FUNCTION test()
  RETURNS int
AS $$
  try:
plpy.error('an error message', 'detail for error', 'hint for error')
  except plpy.Error as exc:
plpy.info('have exc {}'.format(exc))
plpy.info('have exc.args |{}| of type {}'.format(exc.args, type(exc.args)))
$$ LANGUAGE plpython3u;
SELECT test();
[catalin@archie pg]$ psql -f plpy test
CREATE FUNCTION
psql:plpy:13: INFO:  have exc ('an error message', 'detail for error',
'hint for error')
psql:plpy:13: INFO:  have exc.args |("('an error message', 'detail for
error', 'hint for error')",)| of type 

In the last line note that exc.args (the args tuple passed in the
constructor of plpy.Error) is a tuple with a single element which is a
string which is a representation of the tuple passed into plpy.error.
Don't know why this was thought useful, it was certainly surprising to
me. Anyway, the separate $2, $3 etc are currently not detail and hint,
they're just more stuff that gets appended to this tuple. They don't
get passed to clients as hints. And you can pass lots of them not just
detail and hint.

My proposal would change the meaning of this to actually interpret the
second argument as detail, third as hint and to only allow a limited
number (the ones with meaning to ereport). The hint would go to
ereport so it would be a real hint: it would go to clients as HINT and
so on. I think that's way more useful that what is done now. But I now
see my proposal is also backward incompatible.

> I am not against to plpy.ereport function - it can live together with rich
> exceptions class, and users can use what they prefer.

I wasn't suggesting introducing ereport, I was saying the existing
functions and exceptions are very similar to your proposed ereport.
Enhancing them to take keyword arguments would make them a bit more
powerful. Adding ereport would be another way of doing the same thing
so that's more confusing than useful.

All in all it's hard to improve this cleanly. I'm still not sure the
latest patch is a good idea but now I'm also not sure what I proposed
is better than leaving it as it is.


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

[HACKERS]

[Michael Paquier]

On Mon, Dec 7, 2015 at 11:37 PM, Fujii Masao  wrote:

> The latest patch has another problem; pg_receivexlog trying to connect to
> the PostgreSQL >= 9.4 always reports the following message unexpectedly.
>
> could not identify system: got 1 rows and 4 fields, expected 1 rows
> and 4 or more fields
>
> This problem happens because the patch incorrectly treats the case where
> IDENTIFY_SYSTEM command returns NULL as database name, as an error case.
>
> Attached is the updated version of the patch, which fixes the problem.
> Comments?
>

The patch looks good. The top comment of RunIdentifySystem is incorrect
though. It should mention that a database name is returned and not a plugin
name.
-- 
Michael

Re: [HACKERS] [sqlsmith] Failed to generate plan on lateral subqueries

[Andreas Seltenreich]

I wrote:

> Tom Lane writes:
>> Andreas Seltenreich  writes:
>>> I've added new grammar rules to sqlsmith and improved some older ones.
>>> This was rewarded with a return of "failed to generate plan" errors.
>>
>> I believe I've dealt with these cases now.  Thanks for the report!
>
> I no longer see "failed to build any n-way joins" after pulling, but
> there are still instances of "could not devise a query plan". Samples below.

sorry, I spoke too soon: nine of the former have been logged through the
night.  I'm attaching a larger set of sample queries this time in case
that there are still multiple causes for the observed errors.

regards,
Andreas

-- select query||';' from error e
--  where (msg ~~ 'ERROR:  failed to build any%')
--  and e.t > now() - interval '8 hours'
--  order by length(query) asc;

select  
  ref_0.collation_schema as c0
from 
  (select  
  sample_0.is_supported as c0
from 
  information_schema.sql_packages as sample_0 tablesample system (0.8) 
where 25 > 15
fetch first 60 rows only) as subq_0
right join information_schema.collations as ref_0
on (subq_0.c0 = ref_0.collation_catalog ),
  lateral (select  
ref_1.f2 as c0, 
ref_1.f1 as c1, 
ref_0.collation_name as c2, 
ref_0.pad_attribute as c3, 
subq_0.c0 as c4, 
subq_0.c0 as c5, 
subq_0.c0 as c6
  from 
public.func_index_heap as ref_1
  where ref_1.f2 ~ ref_1.f1
  fetch first 170 rows only) as subq_1,
  lateral (select  
sample_8.b as c0, 
subq_1.c1 as c1, 
sample_8.a as c2
  from 
public.clstr_tst as sample_8 tablesample system (8.8) 
left join public.f_star as sample_9 tablesample bernoulli (7.5) 
on (sample_8.a = sample_9.aa )
  left join public.rules_log as sample_10 tablesample bernoulli (7) 
  on (sample_8.b = sample_10.f1 )
  where sample_10.tag <> sample_8.c
  fetch first 126 rows only) as subq_2
where (ref_0.collation_name is NULL) 
  or (subq_2.c1 !~ subq_2.c1)
fetch first 56 rows only;
select  
  subq_16.c4 as c0
from 
  public.rule_and_refint_t1 as sample_18 tablesample system (9.7) 
left join (select  
sample_20.xx as c0, 
sample_20.xx as c1
  from 
public.inhf as sample_20 tablesample bernoulli (1) 
  where sample_20.xx is not NULL) as subq_11
  inner join public.main_table as sample_25 tablesample system (1.2) 
inner join (select  
  sample_26.b as c0, 
  subq_15.c1 as c1, 
  subq_15.c0 as c2, 
  32 as c3, 
  subq_15.c0 as c4
from 
  public.dropcolumn as sample_26 tablesample system (7.6) ,
  lateral (select  
sample_27.t as c0, 
sample_26.b as c1
  from 
public.radix_text_tbl as sample_27 tablesample system (4.6) 
  where (sample_26.b is not NULL) 
and (sample_27.t ~~ sample_27.t)) as subq_15
where subq_15.c0 !~ subq_15.c0) as subq_16
on (sample_25.b = subq_16.c0 )
  on (subq_11.c0 = subq_16.c2 )
on (sample_18.id1a = sample_25.a ),
  lateral (select  
subq_16.c3 as c0, 
subq_17.c1 as c1, 
sample_18.id1a as c2, 
coalesce(subq_16.c0, subq_16.c1) as c3
  from 
public.rtest_vcomp as ref_21,
lateral (select  
  sample_28.a as c0, 
  ref_21.size_in_cm as c1, 
  subq_11.c0 as c2, 
  sample_18.id1a as c3
from 
  public.tab1 as sample_28 tablesample system (8.2) 
where subq_11.c1 <= sample_28.b
fetch first 111 rows only) as subq_17
  where ref_21.size_in_cm is NULL
  fetch first 101 rows only) as subq_18
where subq_11.c0 ~>=~ subq_11.c1
fetch first 94 rows only;
select  
  subq_33.c0 as c0, 
  subq_33.c0 as c1
from 
  (select  
sample_95.y as c0, 
sample_95.z as c1
  from 
public.check2_tbl as sample_95 tablesample system (5.1) 
  where sample_95.y = sample_95.y) as subq_28
  right join pg_catalog.pg_user as ref_101
  on (subq_28.c0 = ref_101.passwd )
left join pg_catalog.pg_opfamily as sample_96 tablesample bernoulli (9.8) 
on (ref_101.usesysid = sample_96.opfmethod ),
  lateral (select  
subq_31.c0 as c0, 
sample_97.e as c1, 
sample_97.e as c2
  from 
public.dropcolumnchild as sample_97 tablesample system (2.4) ,
lateral (select  
  subq_30.c0 as c0
from 
  public.random_tbl as sample_98 tablesample system (8.4) ,
  lateral (select  
subq_28.c1 as c0, 
subq_28.c0 as c1, 
subq_28.c1 as c2, 
sample_97.e as c3
  

Re: [HACKERS]

[Michael Paquier]

On Tue, Dec 8, 2015 at 4:37 PM, Michael Paquier 
wrote:

> The patch looks good. The top comment of RunIdentifySystem is incorrect
> though. It should mention that a database name is returned and not a plugin
> name.
>

I am not sure what happened here, gmail has suddendly removed the subject
of the email... Sorry for the noise and breaking the thread.
-- 
Michael

Re: [HACKERS] pg_receivexlog: spurious error message connecting to 9.3

[Michael Paquier]

On Mon, Dec 7, 2015 at 11:37 PM, Fujii Masao  wrote:

> The latest patch has another problem; pg_receivexlog trying to connect to
> the PostgreSQL >= 9.4 always reports the following message unexpectedly.
>
> could not identify system: got 1 rows and 4 fields, expected 1 rows
> and 4 or more fields
>
> This problem happens because the patch incorrectly treats the case where
> IDENTIFY_SYSTEM command returns NULL as database name, as an error case.
>
> Attached is the updated version of the patch, which fixes the problem.
> Comments?
>

The patch looks good. The top comment of RunIdentifySystem is incorrect
though. It should mention that a database name is returned and not a plugin
name.
(not sure what happened with the last message, gmail went crazy for a
second removing the subject).
-- 
Michael

Re: [HACKERS] [PATCH] Equivalence Class Filters

[Tom Lane]

Jim Nasby  writes:
> On 12/6/15 10:38 AM, Tom Lane wrote:
>> I said "in most cases".  You can find example cases to support almost any
>> weird planner optimization no matter how expensive and single-purpose;
>> but that is the wrong way to think about it.  What you have to think about
>> is average cases, and in particular, not putting a drag on planning time
>> in cases where no benefit ensues.  We're not committing any patches that
>> give one uncommon case an 1100X speedup by penalizing every other query 10%,
>> or even 1%; especially not when there may be other ways to fix it.

> This is a problem that seriously hurts Postgres in data warehousing 
> applications.

Please provide some specific examples.  I remain skeptical that this
would make a useful difference all that often in the real world ...
and handwaving like that does nothing to change my opinion.  What do
the queries look like, and why would deducing an extra inequality
condition help them?

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] pg_receivexlog: spurious error message connecting to 9.3

[Fujii Masao]

On Wed, Nov 25, 2015 at 11:34 AM, Robert Haas  wrote:
> On Tue, Nov 24, 2015 at 8:32 AM, Fujii Masao  wrote:
>> On Tue, Nov 24, 2015 at 7:00 PM, Marco Nenciarini
>>  wrote:
>>> Hi Robert,
>>>
>>> On 17/11/15 20:10, Robert Haas wrote:
 On Tue, Nov 10, 2015 at 1:35 AM, Craig Ringer  
 wrote:
> On 10 November 2015 at 01:47, Marco Nenciarini
>  wrote:
>
>> I've attached a little patch that removes the errors when connected to 
>> 9.3.
>
> Looks good to me. No point confusing users.
>
> The other callers of RunIdentifySystem are pg_basebackup and
> pg_receivelogical.
>
> pg_basebackup doesn't ask for the db_name (passes null).
>
> pg_receivelogical handles it being null already (and if it didn't,
> it'd die with or without this patch).
>
> pg_receivexlog expects it to be null and fails gracefully if it isn't.
>
> So this change just removes some pointless noise.

 The fprintf(stderr, ...) does not cause a non-local exit, so the
 "else" just after it should be deleted.  Otherwise, when that branch
 is taken, *db_name doesn't get initialized at all.

 Actually, I'd suggest doing it like the attached instead, which seems
 a bit tighter.

>>>
>>> I agree, your patch is better.
>>
>> +else if (PQserverVersion(conn) >= 90400)
>>  fprintf(stderr,
>>  _("%s: could not identify system: got %d rows and
>> %d fields, expected %d rows and %d or more fields\n"),
>>  progname, PQntuples(res), PQnfields(res), 1, 4);
>>  }
>>
>> In the above case, PQclear(res) should be called and FALSE should be 
>> returned?
>
> Hmm, yeah, it looks like that would be more consistent with what the
> other parts of this function do.

The latest patch has another problem; pg_receivexlog trying to connect to
the PostgreSQL >= 9.4 always reports the following message unexpectedly.

could not identify system: got 1 rows and 4 fields, expected 1 rows
and 4 or more fields

This problem happens because the patch incorrectly treats the case where
IDENTIFY_SYSTEM command returns NULL as database name, as an error case.

Attached is the updated version of the patch, which fixes the problem.
Comments?

Regards,

-- 
Fujii Masao
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index 2c963b6..39c616a 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -294,15 +294,21 @@ RunIdentifySystem(PGconn *conn, char **sysid, TimeLineID *starttli,
 	/* Get database name, only available in 9.4 and newer versions */
 	if (db_name != NULL)
 	{
-		if (PQnfields(res) < 4)
-			fprintf(stderr,
-	_("%s: could not identify system: got %d rows and %d fields, expected %d rows and %d or more fields\n"),
-	progname, PQntuples(res), PQnfields(res), 1, 4);
+		*db_name = NULL;
+		if (PQserverVersion(conn) >= 90400)
+		{
+			if (PQnfields(res) < 4)
+			{
+fprintf(stderr,
+		_("%s: could not identify system: got %d rows and %d fields, expected %d rows and %d or more fields\n"),
+		progname, PQntuples(res), PQnfields(res), 1, 4);
 
-		if (PQgetisnull(res, 0, 3))
-			*db_name = NULL;
-		else
-			*db_name = pg_strdup(PQgetvalue(res, 0, 3));
+PQclear(res);
+return false;
+			}
+			if (!PQgetisnull(res, 0, 3))
+*db_name = pg_strdup(PQgetvalue(res, 0, 3));
+		}
 	}
 
 	PQclear(res);

-- 
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] Cube extension kNN support

[Stas Kelvich]

Hello, fixed.



cube_distances.diff
Description: Binary data


---
Stas Kelvich
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company

> On 01 Dec 2015, at 17:52, Teodor Sigaev  wrote:
> 
> Patch looks good, but there ara some review notices:
> 1 gmake installcheck fails:
> *** /.../pgsql/contrib/cube/expected/cube_1.out   2015-12-01 
> 17:49:01.768764000 +0300
> --- /.../pgsql/contrib/cube/results/cube.out  2015-12-01 
> 17:49:12.190818000 +0300
> ***
> *** 1382,1388 
>  (1 row)
> 
>  -- Test of distances
> ! --
>  SELECT cube_distance('(1,1)'::cube, '(4,5)'::cube);
>   cube_distance
>  ---
> --- 1382,1388 
>  (1 row)
> 
>  -- Test of distances
> ! --
>  SELECT cube_distance('(1,1)'::cube, '(4,5)'::cube);
>   cube_distance
> 
> Seems, there a extra space at the end of string
> 
> 2 Pls, don use in C-code magick constants like 'case 16:'. Use macros to 
> define some human-readable name (g_cube_distance())
> 
> 3 Switch in g_cube_distance(): default switch path should generate a error. 
> It just simplifies a degbug process, may be in future.
> 
> 4 Docs: pls, don't use a strings with unlimited length.
> 
> 
> Stas Kelvich wrote:
>> Hello.
>> 
>> That is updated version of the patch with proper update scripts.
>> 
>> Also i’ve noted that documentation states the wrong thing:
>> 
>> “It does not matter which order the opposite corners of a cube are entered 
>> in. The cube functions automatically swap values if needed to create a 
>> uniform "lower left — upper right" internal representation."
>> 
>> But in practice cubes stored "as is" and that leads to problems with getting 
>> cubes sorted along specific dimension directly from index.
>> As a simplest workaround i’ve deleted that sentence from docs and 
>> implemented two coordinate getters -> and ~>. First one returns
>> coordinate of cube as it stored, and second returns coordinate of cube 
>> normalised to (LL,UR)-form.
>> 
>> Other way to fix thing is to force ’normalization’ while creating cube. But 
>> that can produce wrong sorts with already existing data.
>> 
>>> On 09 Jul 2015, at 16:40, Alexander Korotkov  wrote:
>>> 
>>> Hi!
>>> 
>>> On Sat, May 9, 2015 at 6:53 AM, Stas Kelvich  wrote:
>>> Patch is pretty ready, last issue was about changed extension interface, so 
>>> there should be migration script and version bump.
>>> Attaching a version with all migration stuff.
>>> 
>>> I can't see cube--1.0--1.1.sql in the patch. Did forget to include it?
>>> 
>>> --
>>> Alexander Korotkov
>>> Postgres Professional: http://www.postgrespro.com
>>> The Russian Postgres Company
>> 
>> Stas Kelvich
>> Postgres Professional: http://www.postgrespro.com
>> The Russian Postgres Company
>> 
>> 
>> 
>> 
> 
> -- 
> Teodor Sigaev   E-mail: teo...@sigaev.ru
>   WWW: http://www.sigaev.ru/
> 
> 
> -- 
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers


-- 
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 to generate plan on lateral subqueries

[Tom Lane]

Amit Langote  writes:
> On 2015/12/07 2:52, Andreas Seltenreich wrote:
>> I've added new grammar rules to sqlsmith and improved some older ones.
>> This was rewarded with a return of "failed to generate plan" errors.
>> The failing queries all contain a lateral subquery.

> * Removing the limit (fetch first...) in lateral sub-queries makes the
> errors go away for all above queries.

Yeah.  I've been able to reduce Andreas' first example to

select * from
  text_tbl tt1
left join int8_tbl i8 on i8.q1 = 42,
  lateral (select
i8.q2,
tt2.f1
  from
text_tbl tt2
limit 1
) as ss
where tt1.f1 = ss.f1;

ERROR:  failed to build any 3-way joins

The key features are (1) a subquery with a LATERAL reference to the inner
side of a left join, and (2) a degenerate join condition for the left
join, ie it only references the inner side.  (2) causes the planner to
see the left join as a clauseless join, so it prefers to postpone it
as long as possible.  In this case it will try to join tt1 to ss first,
because the WHERE condition is an inner-join clause linking those two,
and inner joins are allowed to associate into the lefthand sides of left
joins.  But now it's stuck: because of the lateral reference to i8, the
only way to generate a join between tt1+ss and i8 is for i8 to be on the
outside of a nestloop, and that doesn't work because nestloop can only
handle LEFT outer joins not RIGHT outer joins.  So that's a dead end;
but because it thought earlier that tt1 could be joined to ss, it did not
generate the tt1+i8 join at all, so it fails to find any way to build the
final join.

If you remove the LIMIT then the sub-select can be flattened, causing
the problem to go away because there's no longer a lateral ordering
constraint (there's not actually any need to evaluate i8.q2 while
scanning tt2).

I think the way to fix this is that join_is_legal() should be taught to
notice whether the proposed join would have unresolved lateral references
to other relations that it will need to be on the outside of any join to.
If join_is_legal were to reject tt1+ss then the has_legal_joinclause
tests at the bottom of have_join_order_restriction would fail, so
have_join_order_restriction would correctly report that there's a
constraint forcing tt1 and i8 to be joined directly despite the lack
of a join clause.

Andreas' second example is a similar situation, with the addition of a
PlaceHolderVar in the sub-select (since it has a non-strict output
variable that has to bubble up through an outer join).  In this case
we fail earlier, because although join_is_legal again lets us try to
make a join that can't be useful, the check_hazardous_phv() test that
I recently added to joinpath.c recognizes that there's no safe way
to make that join, so it rejects all the possible join paths and we
end up with a joinrel with no paths, leading to the different error
message.

I think that fixing join_is_legal() may be enough to take care of
this case too, but I need to think about whether there could still be
any cases in which check_hazardous_phv() would reject all paths for
a joinrel.  It might be that that logic is in the wrong place and
needs to be folded into join_is_legal(), or that join_is_legal()
*also* has to account for this (which would be annoying).

I've not traced through the third example in detail, but it looks
like it's just a variant of these problems.

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] [DOCS] max_worker_processes on the standby

[Fujii Masao]

On Sat, Dec 5, 2015 at 12:56 AM, Alvaro Herrera
 wrote:
> Fujii Masao wrote:
>
>> Sorry for not reviewing the patch before you push it...
>>
>> In HEAD, I ran very simple test case:
>>
>> 1. enable track_commit_timestamp
>> 2. start the server
>> 3. run some transactions
>> 4. execute pg_last_committed_xact() -- returns non-null values
>> 5. shutdown the server with immdiate mode
>> 6. restart the server -- crash recovery happens
>> 7. execute pg_last_committed_xact()
>>
>> The last call of pg_last_committed_xact() returns NULL values, which means
>> that the xid and timestamp information of the last committed transaction
>> disappeared by crash recovery. Isn't this a bug?
>
> Hm, not really, because the status of the "last" transaction is kept in
> shared memory as a cache and not expected to live across a restart.
> However, I tested the equivalent scenario:
>
> alvherre=# create table fg();
> CREATE TABLE
>
> alvherre=# select ts.* from pg_class,pg_xact_commit_timestamp(xmin) ts where 
> relname = 'fg';
>   ts
> ---
>  2015-12-04 12:41:48.017976-03
> (1 fila)
>
> then crash the server, and after recovery the data is gone:
>
> alvherre=# select ts.*, xmin, c.relname from pg_class 
> c,pg_xact_commit_timestamp(xmin) ts where relname = 'fg';
>  ts | xmin | relname
> +--+-
> |  630 | fg
> (1 fila)
>
> Not sure what is going on; my reading of the code certainly says that
> the data should be there.  I'm looking into it.
>
> I also noticed that I didn't actually push the whole of the patch
> yesterday -- I neglected to "git add" the latest changes, the ones that
> fix the promotion scenario :-( so the commit messages is misleading
> because it describes something that's not there.

So firstly you will push those "latest" changes soon?

Regards,

-- 
Fujii Masao


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

Re: [HACKERS] [PATCH] Equivalence Class Filters

[Jim Nasby]

On 12/6/15 10:38 AM, Tom Lane wrote:

I said "in most cases".  You can find example cases to support almost any
weird planner optimization no matter how expensive and single-purpose;
but that is the wrong way to think about it.  What you have to think about
is average cases, and in particular, not putting a drag on planning time
in cases where no benefit ensues.  We're not committing any patches that
give one uncommon case an 1100X speedup by penalizing every other query 10%,
or even 1%; especially not when there may be other ways to fix it.


This is a problem that seriously hurts Postgres in data warehousing 
applications. We can't keep ignoring optimizations that provide even as 
little as 10% execution improvements for 10x worse planner performance, 
because in a warehouse it's next to impossible for planning time to matter.


Obviously it'd be great if there was a fast, easy way to figure out 
whether a query would be expensive enough to go the whole 9 yards on 
planning it but at this point I suspect a simple GUC would be a big 
improvement.

--
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


--
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] Tsvector editing functions

[Stas Kelvich]

Hello.

Done with the list of suggestions. Also heavily edit delete function.



tsvector_ops.diff
Description: Binary data

---
Stas Kelvich
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company

> On 27 Nov 2015, at 15:13, Teodor Sigaev  wrote:
> 
> Hmm, seems, it will be useful to add two fuctions:
> 
> tsvector filter(tsvector, array_of_weigths) - returns tsvector contains 
> lexemes with given weights
> 
> tsvector setweight(tsvector, weigth, array_of_lexemes) - sets given weight 
> for given lexemes
> 
> Stas Kelvich wrote:
>> Hello.
>> 
>> There is patch that adds some editing routines for tsvector type.
>> 
>> tsvector delete(tsvector, text)
>>  removes entry from tsvector by lexeme name
>> set unnest(tsvector)
>>  expands a tsvector to a set of rows. Each row has following columns: 
>> lexeme, postings, weights.
>> text[] to_array(tsvector)
>>  converts tsvector to array of lexemes
>> tsvector to_tsvector(text[])
>>  converts array of lexemes to tsvector
>> 
>> 
>> 
>> 
>> 
>> 
>> Stas Kelvich
>> Postgres Professional: http://www.postgrespro.com
>> The Russian Postgres Company
>> 
>> 
>> 
>> 
>> 
> 
> -- 
> Teodor Sigaev   E-mail: teo...@sigaev.ru
>   WWW: http://www.sigaev.ru/


-- 
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] Using quicksort for every external sort run

[Jeff Janes]

On Wed, Dec 2, 2015 at 10:03 AM, Robert Haas  wrote:

>
> While large sorts are uncommon in queries, they are much more common
> in index builds.  Therefore, I think we ought to be worrying more
> about regressions at 64MB than at 4MB, because we ship with
> maintenance_work_mem = 64MB and a lot of people probably don't change
> it before trying to build an index.

You have more sympathy for people who don't tune their settings than I do.

Especially now that auovacuum_work_mem exists, there is much less
constraint on increasing maintance_work_mem than there is on work_mem.
Unless, perhaps, you have a lot of user-driven temp tables which get
indexes created on them.


> If we make those index builds go
> faster, users will be happy.  If we make them go slower, users will be
> sad.  So I think it's worth asking the question "are there any CREATE
> INDEX commands that someone might type on a system on which they've
> done no other configuration that will be slower with this patch"?

I found a regression on my 2nd attempt.  I am indexing random md5
hashes (so they should get the full benefit of key abbreviation), and
in this case 400,000,000 of them:

create table foobar as select md5(random()::text) as x, random() as y
from generate_series(1,1);
insert into foobar select * from foobar ;
insert into foobar select * from foobar ;

Gives a 29GB table.

with the index:

create index on foobar (x);


With 64MB maintenance_work_mem, I get (best time of 7 or 8):

unpatched 2,436,483.834 ms
allpatches 3,964,875.570 ms62% slower
not_0005   3,794,716.331 ms


The unpatched sort ends with a 118-way merge followed by a 233-way merge:

LOG:  finished 118-way merge step: CPU 98.65s/835.67u sec elapsed 1270.61 sec
LOG:  performsort done (except 233-way final merge): CPU
98.75s/835.88u sec elapsed 1276.14 sec
LOG:  external sort ended, 2541465 disk blocks used: CPU
194.02s/1635.12u sec elapsed 2435.46 sec

The patched one ends with a 2-way, two sequential 233-way merges, and
a final 233-way merge:

LOG:  finished 2-way merge step: CPU 62.08s/435.70u sec elapsed 587.52 sec
LOG:  finished 233-way merge step: CPU 77.94s/660.11u sec elapsed 897.51 sec
LOG:  a multi-pass external merge sort is required (234 tape maximum)
HINT:  Consider increasing the configuration parameter "maintenance_work_mem".
LOG:  finished 233-way merge step: CPU 94.55s/884.63u sec elapsed 1185.17 sec
LOG:  performsort done (except 233-way final merge): CPU
94.76s/884.69u sec elapsed 1192.01 sec
LOG:  external sort ended, 2541656 disk blocks used: CPU
202.65s/1771.50u sec elapsed 3963.90 sec


If you just look at the final merges of each, they should have the
same number of tuples going through them (i.e. all of the tuples) but
the patched one took well over twice as long, and all that time was IO
time, not CPU time.

I reversed out the memory pooling patch, and that shaved some time
off, but nowhere near bringing it back to parity.

I think what is going on here is that the different numbers of runs
with the patched code just makes it land in an anti-sweat spot in the
tape emulation and buffering algorithm.

Each tape gets 256kB of buffer.  But two tapes have one third of the
tuples each other third are spread over all the other tapes almost
equally (or maybe one tape has 2/3 of the tuples, if the output of one
233-way nonfinal merge was selected as the input of the other one).
Once the large tape(s) has depleted its buffer, the others have had
only slightly more than 1kB each depleted.  Yet when it goes to fill
the large tape, it also tops off every other tape while it is there,
which is not going to get much read-ahead performance on them, leading
to a lot of random IO.

Now, I'm not sure why this same logic wouldn't apply to the unpatched
code with 118-way merge too.  So maybe I am all wet here.  It seems
like that imbalance would be enough to also cause the problem.

I have seen this same type if things years ago, but was never able to
analyze it to my satisfaction (as I haven't been able to do now,
either).

So if this patch with this exact workload just happens to land on a
pre-existing infelicity, how big of a deal is that?  It wouldn't be
creating a regression, just shoving the region that experiences the
problem around in such a way that it affects a different group of use
cases.

And perhaps more importantly, can anyone else reproduce this, or understand it?

Cheers,

Jeff


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

Re: [HACKERS] Foreign join pushdown vs EvalPlanQual

[Robert Haas]

On Mon, Dec 7, 2015 at 12:25 AM, Etsuro Fujita
 wrote:
>> Instead, I think we should go the opposite direction and pass the
>> outerplan to GetForeignPlan after all.  I was lulled into a full sense
>> of security by the realization that every FDW that uses this feature
>> MUST want to do outerPlan(scan_plan) = fdw_outerplan.  That's true,
>> but irrelevant.  The point is that the FDW might want to do something
>> additional, like frob the outer plan's tlist, and it can't do that if
>> we don't pass it fdw_outerplan.  So we should do that, after all.
>
> As I proposed upthread, another idea would be to 1) to store an
> fdw_outerpath in the fdw_private list of a ForeignPath node, and then 2) to
> create an fdw_outerplan from *the fdw_outerpath stored into
> the fdw_private* in GetForeignPlan.  One good thing for this is that we keep
> the API of create_foreignscan_path as-is.  What do you think about that?

I don't think it's a good idea, per what I said in the first paragraph
of this email:

http://www.postgresql.org/message-id/ca+tgmoz5g+zgph3stmgm6cwgtoywz3n1pjsw6lvhz31ofgl...@mail.gmail.com

I think the core system likely needs visibility into where paths and
plans are present in node trees, and putting them somewhere inside
fdw_private would be going in the opposite direction.

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


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

Re: [HACKERS] jsonb_delete not documented

[Andrew Dunstan]

On 12/06/2015 10:49 PM, Tom Lane wrote:

Peter Eisentraut  writes:

I see.  The reference from pg_operator to pg_proc is by OID rather than
function name, so I didn't find them.  Is that because the function is
overloaded?

Yeah, I suppose so --- regproc can't resolve overloaded function names.


It's kind of odd that these are the only operators (at
first glance) that are set up like that.

I think the customary thing when creating functions meant as operator
support is to give them unique names.  These weren't done that way ...
I wasn't involved, but I wonder whether there was uncertainty as to
whether these should be documented as functions or operators.





If we want to require that then perhaps we should have a check for it? I 
don't recall the exact reasoning so many months later, but you're 
probably right about how it came about.



cheers

andrew


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

Re: [HACKERS] [PATCH] Equivalence Class Filters

[David G. Johnston]

On Mon, Dec 7, 2015 at 8:35 AM, Jim Nasby  wrote:

> On 12/6/15 10:38 AM, Tom Lane wrote:
>
>> I said "in most cases".  You can find example cases to support almost any
>> weird planner optimization no matter how expensive and single-purpose;
>> but that is the wrong way to think about it.  What you have to think about
>> is average cases, and in particular, not putting a drag on planning time
>> in cases where no benefit ensues.  We're not committing any patches that
>> give one uncommon case an 1100X speedup by penalizing every other query
>> 10%,
>> or even 1%; especially not when there may be other ways to fix it.
>>
>
> This is a problem that seriously hurts Postgres in data warehousing
> applications. We can't keep ignoring optimizations that provide even as
> little as 10% execution improvements for 10x worse planner performance,
> because in a warehouse it's next to impossible for planning time to matter.
>
> Obviously it'd be great if there was a fast, easy way to figure out
> whether a query would be expensive enough to go the whole 9 yards on
> planning it but at this point I suspect a simple GUC would be a big
> improvement.


Something like "enable_equivalencefilters" but that defaults to false
unlike every one existing "enable_*" GUC?

​It would be a lot more user-friendly to have something along the lines of
"planner_mode (text)" with labels like "star, transactional, bulk_load,
etc..." because I suspect there are other things we'd want to add if we
start identifying queries by their type/usage and optimize accordingly.
Having the knobs available is necessary but putting on a façade would make
the user more straight-forward for the common cases.

David J.

Re: [HACKERS] [PATCH] Equivalence Class Filters

[Jim Nasby]

On 12/7/15 9:54 AM, Tom Lane wrote:

Jim Nasby  writes:

>On 12/6/15 10:38 AM, Tom Lane wrote:

>>I said "in most cases".  You can find example cases to support almost any
>>weird planner optimization no matter how expensive and single-purpose;
>>but that is the wrong way to think about it.  What you have to think about
>>is average cases, and in particular, not putting a drag on planning time
>>in cases where no benefit ensues.  We're not committing any patches that
>>give one uncommon case an 1100X speedup by penalizing every other query 10%,
>>or even 1%; especially not when there may be other ways to fix it.

>This is a problem that seriously hurts Postgres in data warehousing
>applications.

Please provide some specific examples.  I remain skeptical that this
would make a useful difference all that often in the real world ...
and handwaving like that does nothing to change my opinion.  What do
the queries look like, and why would deducing an extra inequality
condition help them?


I was speaking more broadly than this particular case. There's a lot of 
planner improvements that get shot down because of the planning overhead 
they would add. That's great for cases when milliseconds count, but 
spending an extra 60 seconds (a planning eternity) to shave an hour off 
a warehouse/reporting query.


There needs to be some way to give the planner an idea of how much 
effort it should expend. GEQO and *_collapse_limit addresses this in the 
opposite direction (putting a cap on planner effort), but I think we 
need something that does the opposite "I know this query will take a 
long time, so expend extra effort on planning it."

--
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


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

Re: [HACKERS] [PATCH] Equivalence Class Filters

[Jim Nasby]

On 12/7/15 10:44 AM, Simon Riggs wrote:

There are many optimizations we might adopt, yet planning time is a
factor. It seems simple enough to ignore more complex optimizations if
we have already achieved a threshold cost (say 10). Such a test would
add nearly zero time for the common case. We can apply the optimizations
in some kind of ordering depending upon the cost, so we are careful to
balance the cost/benefit of trying certain optimizations.


Unfortunately I've seen a lot of millisecond queries that have 6 figure 
estimates, due to data being in cache. So I'm not sure how practical 
that would be.


Maybe a better starting point would be a planner timeout.

I definitely agree we need some method to limit planning time when 
necessary (ie: OLTP). Without that we'll never be able to start testing 
more complex optimizations.

--
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


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

Re: [HACKERS] [PATCH] Equivalence Class Filters

[Simon Riggs]

On 6 December 2015 at 16:38, Tom Lane  wrote:

>> Lastly, in most cases knowing that t2.id <= 10 is just not worth all
> >> that much; it's certainly far less useful than an equality condition.
>
> > I'd have thought that my link to a thread of a reported 1100 to 2200
> times
> > performance improvement might have made you think twice about claiming
> the
> > uselessness of this idea.
>

Personally, I think this optimization is a long way down the list of
important items because I don't see it as a typical use case. There are
already patches submitted for more important items, so this isn't the right
place to be arguing such things. Not every beneficial optimization has a
wide use case.

Since the discussion has become more general, I would add a few points.

I said "in most cases".  You can find example cases to support almost any
> weird planner optimization no matter how expensive and single-purpose;
> but that is the wrong way to think about it.  What you have to think about
> is average cases, and in particular, not putting a drag on planning time
> in cases where no benefit ensues.  We're not committing any patches that
> give one uncommon case an 1100X speedup by penalizing every other query
> 10%,
> or even 1%; especially not when there may be other ways to fix it.
>

I agree with this point also, I just don't see it as a blocker for
expensive general case optimizations.

There are many optimizations we might adopt, yet planning time is a factor.
It seems simple enough to ignore more complex optimizations if we have
already achieved a threshold cost (say 10). Such a test would add nearly
zero time for the common case. We can apply the optimizations in some kind
of ordering depending upon the cost, so we are careful to balance the
cost/benefit of trying certain optimizations.

Optimizer directives might be useful also, but we can do just as well with
a heuristic.

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: [HACKERS] Support of partial decompression for datums

[Ildus Kurbangaliev]

On Sat, 5 Dec 2015 06:14:07 +0900
Michael Paquier  wrote:

> On Sat, Dec 5, 2015 at 12:10 AM, Simon Riggs 
> wrote:
> > On 4 December 2015 at 13:47, Ildus Kurbangaliev
> >  wrote:
> >  
> >>
> >> Attached patch adds support of partial decompression for datums.
> >> It will be useful in many cases when extracting part of data is
> >> enough for big varlena structures.
> >>
> >> It is especially useful for expanded datums, because it provides
> >> storage for partial results.  
> >
> >
> > This isn't enough for anyone else to follow your thoughts and agree
> > enough to commit.
> >
> > Please explain the whole idea, starting from what problem you are
> > trying to solve and how well this does it, why you did it this way
> > and the other ways you tried/decided not to pursue. Thanks.  
> 
> Yeah, I would imagine that what Ildus is trying to achieve is
> something close to LZ4_decompress_safe_partial, by being able to stop
> compression after getting a certain amount of data decompressed, and
> continue working once again after.
> 
> And actually I think I get the idea. With his test case, what we get
> first is a size, and then we reuse this size to extract only what we
> need to fetch only a number of items from the tsvector. But that's
> actually linked to the length of the compressed chunk, and at the end
> we would still need to decompress the whole string perhaps, but it is
> not possible to be sure using the information provided.
> 
> Ildus, using your patch for tsvector, are you aiming at being able to
> complete an operation by only using a portion of the compressed data?
> Or are you planning to use that to improve the speed of detection of
> corrupted data in the chunk? If that's the latter, we would need to
> still decompress the whole string anyway, so having a routine able to
> decompress only until a given position is not necessary, and based on
> the example given upthread it is not possible to know what you are
> trying to achieve. Hence could you share your thoughts regarding your
> stuff with tsvector?
> 
> Changing pglz_decompress shape is still a bad idea anyway, I guess we
> had better have something new like pglz_decompress_partial instead.

Yes, you've got the idea. First we get a size of entries in tsvector, 
then with the size we can get WordEntry values. WordEntry 
contains offset of lexeme in the data and length of lexeme.
Information in these entries is enough to calculate an offset until we
need to decompress tsvector varlena.

So for example in binary search, we will decompress until half of
lexemes data (lexemes in tsvector are sorted), and then if search will
go left, then we just reuse that decompressed block, and we don't need
other part of tsvector. If search will go right, then we just
decompress a half of remaining part using the saved state and so on.

So in half of cases, we will decompress only a half of lexemes
in tsvector, and even if we need to decompress more, in most of cases we
will not decompress the whole tsvector.

-- 
Ildus Kurbangaliev
Postgres Professional: http://www.postgrespro.com
Russian Postgres 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] More stable query plans via more predictable column statistics

[Shulgin, Oleksandr]

On Fri, Dec 4, 2015 at 6:48 PM, Robert Haas  wrote:

> On Tue, Dec 1, 2015 at 10:21 AM, Shulgin, Oleksandr
>  wrote:
> >
> > What I have found is that in a significant percentage of instances, when
> a
> > duplicate sample value is *not* put into the MCV list, it does produce
> > duplicates in the histogram_bounds, so it looks like the MCV cut-off
> happens
> > too early, even though we have enough space for more values in the MCV
> list.
> >
> > In the extreme cases I've found completely empty MCV lists and histograms
> > full of duplicates at the same time, with only about 20% of distinct
> values
> > in the histogram (as it turns out, this happens due to high fraction of
> > NULLs in the sample).
>
> Wow, this is very interesting work.  Using values_cnt rather than
> samplerows to compute avgcount seems like a clear improvement.  It
> doesn't make any sense to raise the threshold for creating an MCV
> based on the presence of additional nulls or too-wide values in the
> table.  I bet compute_distinct_stats needs a similar fix.


Yes, and there's also the magic 1.25 multiplier in that code.  I think it
would make sense to agree first on how exactly the patch for
compute_scalar_stats() should look like, then port the relevant bits to
compute_distinct_stats().


> But for
> plan stability considerations, I'd say we should back-patch this all
> the way, but those considerations might mitigate for a more restrained
> approach.  Still, maybe we should try to sneak at least this much into
> 9.5 RSN, because I have to think this is going to help people with
> mostly-NULL (or mostly-really-wide) columns.
>

I'm not sure.  Likely people would complain or have found this out on their
own if they were seriously affected.

What I would be interested is people running the queries I've shown on
their data to see if there are any interesting/unexpected patterns.

As far as the rest of the fix, your code seems to remove the handling
> for ndistinct < 0.  That seems unlikely to be correct, although it's
> possible that I am missing something.


The difference here is that ndistinct at this scope in the original code
did hide a variable from an outer scope.  That one could be < 0, but in my
code there is no inner-scope ndistinct, we are referring to the outer scope
variable which cannot be < 0.


> Aside from that, the rest of
> this seems like a policy change, and I'm not totally sure off-hand
> whether it's the right policy.  Having more MCVs can increase planning
> time noticeably, and the point of the existing cutoff is to prevent us
> from choosing MCVs that aren't actually "C".  I think this change
> significantly undermines those protections.  It seems to me that it
> might be useful to evaluate the effects of this part of the patch
> separately from the samplerows -> values_cnt change.
>

Yes, that's why I was wondering if frequency cut-off approach might be
helpful here.  I'm going to have a deeper look at array's typanalyze
implementation at the least.

--
Alex

Re: [HACKERS] HINTing on UPDATE foo SET foo.bar = ..;

[Michael Paquier]

On Wed, Sep 2, 2015 at 6:19 AM, Marko Tiikkaja  wrote:

> Hopefully nobody minds if I slip this to the commit fest that started
> today?  The attached patch should address all the comments from the 9.5
> cycle.
>

All the previous comments have been addressed. Perhaps some regression
tests would have some value?
-- 
Michael

Re: [HACKERS] Some questions about the array.

[YUriy Zhuravlev]

On Friday 04 December 2015 16:52:48 Teodor Sigaev wrote:
> Seems, omitting boundaries in insert/update isn't a good idea. I suggest to 
> allow omitting only in select subscripting.

It was my last attempt to do so. So now I agree, the most simple is now 
disabled for insert and update. New patch in attach.
-- 
YUriy Zhuravlev
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Companydiff --git a/doc/src/sgml/array.sgml b/doc/src/sgml/array.sgml
index 4385a09..6ee71a5 100644
--- a/doc/src/sgml/array.sgml
+++ b/doc/src/sgml/array.sgml
@@ -257,6 +257,26 @@ SELECT schedule[1:2][1:1] FROM sal_emp WHERE name = 'Bill';
 (1 row)
 
 
+  Possible to skip the lower-bound or
+  upper-bound
+  for get first or last element in slice.
+
+
+SELECT schedule[:][:] FROM sal_emp WHERE name = 'Bill';
+
+schedule
+
+ {{meeting,lunch},{training,presentation}}
+(1 row)
+
+SELECT schedule[:2][2:] FROM sal_emp WHERE name = 'Bill';
+
+schedule
+
+ {{lunch},{presentation}}
+(1 row)
+
+
   If any dimension is written as a slice, i.e., contains a colon, then all
   dimensions are treated as slices.  Any dimension that has only a single
   number (no colon) is treated as being from 1
diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c
index 29f058c..d9bf977 100644
--- a/src/backend/executor/execQual.c
+++ b/src/backend/executor/execQual.c
@@ -268,10 +268,12 @@ ExecEvalArrayRef(ArrayRefExprState *astate,
 	bool		eisnull;
 	ListCell   *l;
 	int			i = 0,
-j = 0;
+j = 0,
+indexexpr;
 	IntArray	upper,
 lower;
 	int		   *lIndex;
+	AnyArrayType *arrays;
 
 	array_source = ExecEvalExpr(astate->refexpr,
 econtext,
@@ -293,6 +295,7 @@ ExecEvalArrayRef(ArrayRefExprState *astate,
 	foreach(l, astate->refupperindexpr)
 	{
 		ExprState  *eltstate = (ExprState *) lfirst(l);
+		eisnull = false;
 
 		if (i >= MAXDIM)
 			ereport(ERROR,
@@ -300,10 +303,23 @@ ExecEvalArrayRef(ArrayRefExprState *astate,
 	 errmsg("number of array dimensions (%d) exceeds the maximum allowed (%d)",
 			i + 1, MAXDIM)));
 
-		upper.indx[i++] = DatumGetInt32(ExecEvalExpr(eltstate,
-	 econtext,
-	 ,
-	 NULL));
+		if (eltstate == NULL && astate->refattrlength <= 0)
+		{
+			if (isAssignment)
+ereport(ERROR,
+	(errcode(ERRCODE_ARRAY_SUBSCRIPT_ERROR),
+	 errmsg("cannot determine upper index for empty array")));
+			arrays = (AnyArrayType *)DatumGetArrayTypeP(array_source);
+			indexexpr = AARR_LBOUND(arrays)[i] + AARR_DIMS(arrays)[i] - 1;
+		}
+		else
+			indexexpr = DatumGetInt32(ExecEvalExpr(eltstate,
+   econtext,
+   ,
+   NULL));
+
+		upper.indx[i++] = indexexpr;
+
 		/* If any index expr yields NULL, result is NULL or error */
 		if (eisnull)
 		{
@@ -321,6 +337,7 @@ ExecEvalArrayRef(ArrayRefExprState *astate,
 		foreach(l, astate->reflowerindexpr)
 		{
 			ExprState  *eltstate = (ExprState *) lfirst(l);
+			eisnull = false;
 
 			if (j >= MAXDIM)
 ereport(ERROR,
@@ -328,10 +345,19 @@ ExecEvalArrayRef(ArrayRefExprState *astate,
 		 errmsg("number of array dimensions (%d) exceeds the maximum allowed (%d)",
 j + 1, MAXDIM)));
 
-			lower.indx[j++] = DatumGetInt32(ExecEvalExpr(eltstate,
-		 econtext,
-		 ,
-		 NULL));
+			if (eltstate == NULL)
+			{
+arrays = (AnyArrayType *)DatumGetArrayTypeP(array_source);
+indexexpr = AARR_LBOUND(arrays)[j];
+			}
+			else
+indexexpr = DatumGetInt32(ExecEvalExpr(eltstate,
+	   econtext,
+	   ,
+	   NULL));
+
+			lower.indx[j++] = indexexpr;
+
 			/* If any index expr yields NULL, result is NULL or error */
 			if (eisnull)
 			{
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 26264cb..a761263 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -2417,6 +2417,8 @@ _copyAIndices(const A_Indices *from)
 
 	COPY_NODE_FIELD(lidx);
 	COPY_NODE_FIELD(uidx);
+	COPY_SCALAR_FIELD(lidx_default);
+	COPY_SCALAR_FIELD(uidx_default);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index aa6e102..e75b448 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2162,6 +2162,8 @@ _equalAIndices(const A_Indices *a, const A_Indices *b)
 {
 	COMPARE_NODE_FIELD(lidx);
 	COMPARE_NODE_FIELD(uidx);
+	COMPARE_SCALAR_FIELD(lidx_default);
+	COMPARE_SCALAR_FIELD(uidx_default);
 
 	return true;
 }
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 012c14b..ed77c75 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2773,6 +2773,8 @@ _outA_Indices(StringInfo str, const A_Indices *node)
 
 	WRITE_NODE_FIELD(lidx);
 	WRITE_NODE_FIELD(uidx);
+	WRITE_BOOL_FIELD(lidx_default);
+	WRITE_BOOL_FIELD(uidx_default);
 }
 
 static void
diff --git 

Re: [HACKERS] Using quicksort for every external sort run

[Peter Geoghegan]

On Mon, Dec 7, 2015 at 9:01 AM, Jeff Janes  wrote:
> So if this patch with this exact workload just happens to land on a
> pre-existing infelicity, how big of a deal is that?  It wouldn't be
> creating a regression, just shoving the region that experiences the
> problem around in such a way that it affects a different group of use
> cases.
>
> And perhaps more importantly, can anyone else reproduce this, or understand 
> it?

That's odd. I've never seen anything like that in the field myself,
but then I've never really been a professional DBA.

If possible, could you try using the ioreplay tool to correlate I/O
with a point in the trace_sort timeline? For both master, and the
patch, for comparison? The tool is available from here:

https://code.google.com/p/ioapps/

There is also a tool available to graph the recorded I/O requests over
time called ioprofiler.

This is the only way that I've been able to graph I/O over time
successfully before. Maybe there is a better way, using perf blockio
or something like that, but this is the way I know to work.

While I'm quite willing to believe that there are oddities about our
polyphase merge implementation that can result in what you call
anti-sweetspots (sourspots?), I have a much harder time imagining why
reverting my merge patch could make things better, unless the system
was experiencing some kind of memory pressure. I mean, it doesn't
change the algorithm at all, except to make more memory available from
the merge by avoiding palloc() fragmentation. How could that possibly
hurt?

-- 
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] [sqlsmith] Failed to generate plan on lateral subqueries

[Tom Lane]

Andreas Seltenreich  writes:
> I've added new grammar rules to sqlsmith and improved some older ones.
> This was rewarded with a return of "failed to generate plan" errors.

I believe I've dealt with these cases now.  Thanks for the report!

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] Using quicksort for every external sort run

[Greg Stark]

​So incidentally I've been running some benchmarks myself. Mostly to
understand the current scaling behaviour of sorting to better judge whether
Peter's analysis of where the pain points are and why we should not worry
about optimizing for the multiple merge pass case were on target. I haven't
actually benchmarked his patch at all, just stock head so far.

The really surprising result (for me) so far is that apparently merge
passes spent actually very little time doing I/O. I had always assumed most
of the time was spent waiting on I/O and that's why we spend so much effort
ensuring sequential I/O and trying to maximize run lengths. I was expecting
to see a huge step increase in the total time whenever there was an
increase in merge passes. However I see hardly any increase, sometimes even
a decrease despite the extra pass. The time generally increases as work_mem
decreases but the slope is pretty moderate and gradual with no big steps
due to extra passes.

On further analysis I'm less surprised by this than previously. The larger
benchmarks I'm running are on a 7GB table which only actually generates
2.6GB of sort data so even writing all that out and then reading it all
back in on a 100MB/s disk would only take an extra 50s. That won't make a
big dent when the whole sort takes about 30 minutes. Even if you assume
there's a substantial amount of random I/O it'll only be a 10% difference
or so which is more or less in line with what I'm seeing.

I haven't actually got to benchmarking Peter's patch at all but this is
reinforcing his argument dramatically. If the worst case for using
quicksort is that the shorter runs might push us into doing an extra merge
and that might add an extra 10% to the run-time that will be easily
counter-balanced by the faster quicksort and in any case it only affects
people who for some reason can't just increase work_mem to allow the single
merge mode.


Table SizeSort Size128MB64MB32MB16MB8MB4MB6914MB2672 MB3392.293102.133343.53
4081.234727.745620.773457MB1336 MB1669.161593.851444.221654.272076.742266.84
2765MB1069 MB1368.921250.441117.21293.451431.641772.181383MB535 MB716.48
625.06557.14575.67644.2721.68691MB267 MB301.08295.87266.84256.29283.82292.24
346MB134 MB145.48149.48133.23130.69127.67137.7435MB13 MB3.5816.7711.2311.93
13.973.17

The colours are to give an idea of the number of merge passes. Grey, is an
internal sort. White is a single merge. Yellow and red are successively
more merges (though the exact boundary between yellow and red may not be
exactly meaningful due to my misunderstanding polyphase merge).

The numbers here are seconds taken from the "elapsed" in the following log
statements when running queries like the following with trace_sort enabled:
LOG:  external sort ended, 342138 disk blocks used: CPU 276.04s/3173.04u
sec elapsed 5620.77 sec
STATEMENT:  select count(*) from (select * from n2 order by r
offset 999) AS x;

This was run on the smallest size VM on Google Compute Engine with 600MB of
virtual RAM and a 100GB virtual network block device.

Re: [HACKERS] [PATCH] Equivalence Class Filters

[Gavin Flower]

On 08/12/15 05:27, David G. Johnston wrote:
On Mon, Dec 7, 2015 at 8:35 AM, Jim Nasby >wrote:


On 12/6/15 10:38 AM, Tom Lane wrote:

I said "in most cases".  You can find example cases to support
almost any
weird planner optimization no matter how expensive and
single-purpose;
but that is the wrong way to think about it.  What you have to
think about
is average cases, and in particular, not putting a drag on
planning time
in cases where no benefit ensues.  We're not committing any
patches that
give one uncommon case an 1100X speedup by penalizing every
other query 10%,
or even 1%; especially not when there may be other ways to fix it.


This is a problem that seriously hurts Postgres in data
warehousing applications. We can't keep ignoring optimizations
that provide even as little as 10% execution improvements for 10x
worse planner performance, because in a warehouse it's next to
impossible for planning time to matter.

Obviously it'd be great if there was a fast, easy way to figure
out whether a query would be expensive enough to go the whole 9
yards on planning it but at this point I suspect a simple GUC
would be a big improvement.


Something like "enable_equivalencefilters" but that defaults to false 
unlike every one existing "enable_*" GUC?


​It would be a lot more user-friendly to have something along the 
lines of "planner_mode (text)" with labels like "star, transactional, 
bulk_load, etc..." because I suspect there are other things we'd want 
to add if we start identifying queries by their type/usage and 
optimize accordingly.  Having the knobs available is necessary but 
putting on a façade would make the user more straight-forward for the 
common cases.


David J.


How about:

planning_time_base 10  # Default effort, may be increased or decreased 
as required - must be at least 1
planning_time_  0  # By default, planner makes no (or minimal) 
effort to optimise for feature 


So for some people, adjusting planning_time_base may be sufficient - but 
for more specialised cases, people can tell the planner to consider 
expending more effort.



Cheers,
Gavin



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

Re: [HACKERS] [PATCH] Equivalence Class Filters

[Evgeniy Shishkin]

> On 07 Dec 2015, at 22:27, Gavin Flower  wrote:
> 
> On 08/12/15 05:27, David G. Johnston wrote:
>> On Mon, Dec 7, 2015 at 8:35 AM, Jim Nasby > >wrote:
>> 
>>On 12/6/15 10:38 AM, Tom Lane wrote:
>> 
>>I said "in most cases".  You can find example cases to support
>>almost any
>>weird planner optimization no matter how expensive and
>>single-purpose;
>>but that is the wrong way to think about it.  What you have to
>>think about
>>is average cases, and in particular, not putting a drag on
>>planning time
>>in cases where no benefit ensues.  We're not committing any
>>patches that
>>give one uncommon case an 1100X speedup by penalizing every
>>other query 10%,
>>or even 1%; especially not when there may be other ways to fix it.
>> 
>> 
>>This is a problem that seriously hurts Postgres in data
>>warehousing applications. We can't keep ignoring optimizations
>>that provide even as little as 10% execution improvements for 10x
>>worse planner performance, because in a warehouse it's next to
>>impossible for planning time to matter.
>> 
>>Obviously it'd be great if there was a fast, easy way to figure
>>out whether a query would be expensive enough to go the whole 9
>>yards on planning it but at this point I suspect a simple GUC
>>would be a big improvement.
>> 
>> 
>> Something like "enable_equivalencefilters" but that defaults to false unlike 
>> every one existing "enable_*" GUC?
>> 
>> ​It would be a lot more user-friendly to have something along the lines of 
>> "planner_mode (text)" with labels like "star, transactional, bulk_load, 
>> etc..." because I suspect there are other things we'd want to add if we 
>> start identifying queries by their type/usage and optimize accordingly. 
>> Having the knobs available is necessary but putting on a façade would make 
>> the user more straight-forward for the common cases.
>> 
>> David J.
>> 
> How about:
> 
> planning_time_base 10  # Default effort, may be increased or decreased as 
> required - must be at least 1
> planning_time_  0  # By default, planner makes no (or minimal) effort to 
> optimise for feature 
> 
> So for some people, adjusting planning_time_base may be sufficient - but for 
> more specialised cases, people can tell the planner to consider expending 
> more effort.
> 

Mysql have now 19 optimizer_switch parameters
https://dev.mysql.com/doc/refman/5.7/en/switchable-optimizations.html

Please don't do that.


I'd rather like some sort of pg_stat_statements, which would track execution 
and planning time.
On new query, we can lookup if query can benefit from more planning time.
But i don't know how costly this can be. 

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



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

Re: [HACKERS] [PATCH] Equivalence Class Filters

[Gavin Flower]

On 08/12/15 08:34, Evgeniy Shishkin wrote:

On 07 Dec 2015, at 22:27, Gavin Flower  wrote:

On 08/12/15 05:27, David G. Johnston wrote:

On Mon, Dec 7, 2015 at 8:35 AM, Jim Nasby >wrote:

On 12/6/15 10:38 AM, Tom Lane wrote:

I said "in most cases".  You can find example cases to support
almost any
weird planner optimization no matter how expensive and
single-purpose;
but that is the wrong way to think about it.  What you have to
think about
is average cases, and in particular, not putting a drag on
planning time
in cases where no benefit ensues.  We're not committing any
patches that
give one uncommon case an 1100X speedup by penalizing every
other query 10%,
or even 1%; especially not when there may be other ways to fix it.


This is a problem that seriously hurts Postgres in data
warehousing applications. We can't keep ignoring optimizations
that provide even as little as 10% execution improvements for 10x
worse planner performance, because in a warehouse it's next to
impossible for planning time to matter.

Obviously it'd be great if there was a fast, easy way to figure
out whether a query would be expensive enough to go the whole 9
yards on planning it but at this point I suspect a simple GUC
would be a big improvement.


Something like "enable_equivalencefilters" but that defaults to false unlike every one 
existing "enable_*" GUC?

​It would be a lot more user-friendly to have something along the lines of "planner_mode 
(text)" with labels like "star, transactional, bulk_load, etc..." because I suspect 
there are other things we'd want to add if we start identifying queries by their type/usage and 
optimize accordingly. Having the knobs available is necessary but putting on a façade would make 
the user more straight-forward for the common cases.

David J.


How about:

planning_time_base 10  # Default effort, may be increased or decreased as 
required - must be at least 1
planning_time_  0  # By default, planner makes no (or minimal) effort to 
optimise for feature 

So for some people, adjusting planning_time_base may be sufficient - but for 
more specialised cases, people can tell the planner to consider expending more 
effort.


Mysql have now 19 optimizer_switch parameters
https://dev.mysql.com/doc/refman/5.7/en/switchable-optimizations.html
I notice that they are either on or off, I suspect that it is better to 
have some sort of measure of how much extra effort the planner should make.




Please don't do that.
I think having some might be useful - though in most situations, having 
a general indicator to the planner might be sufficient.


From reading the thread, I have the impression that for some extreme 
workloads, them some extra twiddling would be useful even though for 
most people it simply be an unnecessary complication.


In over twenty years I've never needed such knobs, but I might get a 
project next year where they might be useful.  So I agree that for most 
situations, such extra stuff is not needed - but I'd like additional 
options available if I ever needed them.




I'd rather like some sort of pg_stat_statements, which would track execution 
and planning time.
On new query, we can lookup if query can benefit from more planning time.
But i don't know how costly this can be.


Cheers,
Gavin



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




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

Re: [HACKERS] Patch: Implement failover on libpq connect level.

[Korry Douglas]

I've tried to deal with some of these problems.

My patch have support for following things:

1. Check whether database instance is in the recovery/standby mode and
try to find another one if so.
2. Let cluster management software to have some time to promote one of
the standbys to master. I.e. there can be failover timeout specified to
allow retry after some time if no working master found.

Really there is room for some improvements in handling of connect
timeout (which became much more important thing when ability to try
next host appears). Now it is handled only by blocking-mode connect
functions, not by async state machine. But I decided to publish patch
without these improvements to get feedback from community.

A bit of testing on this turns up a problem.

Consider a connection string that specifies two hosts and a read/write 
connection:


  postgresql://korry@127.0.0.1:5301,127.0.0.1:5300/edb?readonly=0

If the first host is a healthy standby (meaning that I can connect to it 
but pg_is_in_recovery() returns 't'), the state machine will never move 
on to the second host.


The problem seems to be in PQconnectPoll() in the case for 
CONNECTION_AUTH_OK, specifically this code:


  /* We can release the address list now. */
  pg_freeaddrinfo_all(conn->addrlist_family, conn->addrlist);
  conn->addrlist = NULL;
  conn->addr_cur = NULL;

That frees up the list of alternative host addresses.  The state machine 
then progresses to CONNECTION_CHECK_RO (which invokes 
pg_is_in_recovery()), then CONNECTION_CHECK_RW (waiting for the response 
from the server).  Since we just connected to a standby replica, 
pg_is_in_recovery() returns 't' and the state changes to 
CONNECTION_NEEDED.  The next call to try_next_address() will fail to 
find a next address because we freed the list in the case for 
CONNECTION_AUTH_OK.


A related issue:  when the above sequence occurs, no error message is 
returned (because the case for CONNECTION_NEEDED thinks "an appropriate 
error message is already set up").


In short, if you successfully connect to standby replica (and specify 
readonly=0), the remaining hosts are ignored, even though one of those 
hosts is a master.


And one comment about the code itself - in connectDBStart(), you've 
added quite a bit of code to parse multiple hosts/ports. I would 
recommend adding a comment that shows the expected format, and then 
choosing better variable names (better than 'p', 'q', and 'r'); perhaps 
the variable names could refer to components of the connection string 
that you are parsing (like 'host', 'port', 'delimiter', ...).  That 
would make the code much easier to read/maintain.


Thanks.


-- Korry



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

Re: [HACKERS] Patch: Implement failover on libpq connect level.

[Korry Douglas]

I've tried to deal with some of these problems.

My patch have support for following things:

1. Check whether database instance is in the recovery/standby mode and
try to find another one if so.
2. Let cluster management software to have some time to promote one of
the standbys to master. I.e. there can be failover timeout specified to
allow retry after some time if no working master found.

Really there is room for some improvements in handling of connect
timeout (which became much more important thing when ability to try
next host appears). Now it is handled only by blocking-mode connect
functions, not by async state machine. But I decided to publish patch
without these improvements to get feedback from community.

A bit of testing on this turns up a problem.

Consider a connection string that specifies two hosts and a read/write 
connection:


  postgresql://korry@127.0.0.1:5301,127.0.0.1:5300/edb?readonly=0

If the first host is a healthy standby (meaning that I can connect to 
it but pg_is_in_recovery() returns 't'), the state machine will never 
move on to the second host.


The problem seems to be in PQconnectPoll() in the case for 
CONNECTION_AUTH_OK, specifically this code:


  /* We can release the address list now. */
  pg_freeaddrinfo_all(conn->addrlist_family, conn->addrlist);
  conn->addrlist = NULL;
  conn->addr_cur = NULL;

That frees up the list of alternative host addresses.  The state 
machine then progresses to CONNECTION_CHECK_RO (which invokes 
pg_is_in_recovery()), then CONNECTION_CHECK_RW (waiting for the 
response from the server).  Since we just connected to a standby 
replica, pg_is_in_recovery() returns 't' and the state changes to 
CONNECTION_NEEDED.  The next call to try_next_address() will fail to 
find a next address because we freed the list in the case for 
CONNECTION_AUTH_OK.


A related issue:  when the above sequence occurs, no error message is 
returned (because the case for CONNECTION_NEEDED thinks "an 
appropriate error message is already set up").


In short, if you successfully connect to standby replica (and specify 
readonly=0), the remaining hosts are ignored, even though one of those 
hosts is a master.


A follow-up - the conn->addrlist is also freed when the case for 
CONNECTION_CHECK_RW decides that conn->status != CONNECTION_OK and calls 
closePGConn().



-- Korry


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

Re: [HACKERS] Re: In-core regression tests for replication, cascading, archiving, PITR, etc.

[Alvaro Herrera]

Michael Paquier wrote:

> > If that's not a hard-coded PG version number then I don't know
> > what it is.  Maybe it would be better to use random() instead,
> > but surely this isn't good as-is.
> 
> We would definitely want something within the ephemeral port range, so
> we are up to that:
> rand() * 16384 + 49152;

Yes, this seems to produce the correct range.

Thanks Noah and Tom for the review, and thanks Michael for the patch.  I
pushed it.  A slight fix was to change the chomp() call; it was always
returning 1 (number of elements chomped) so it tried to kill init.

-- 
Á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] Re: In-core regression tests for replication, cascading, archiving, PITR, etc.

[Alvaro Herrera]

Michael Paquier wrote:
> On Thu, Dec 3, 2015 at 6:59 AM, Alvaro Herrera wrote:
> > I didn't push the changed for config_default you requested a few
> > messages upthread; it's not clear to me how setting it to undef affects
> > the whole thing.  If setting it to undef makes the MSVC toolchain run
> > the tap tests in the default config, then I can do it; let's be clear
> > about what branch to backpatch this to.  Also the "1;" at the end of
> > RewindTest.
> 
> Setting it to undef will prevent the tests to run, per vcregress.pl:
> die "Tap tests not enabled in configuration"
>   unless $config->{tap_tests};
> Also, setting it to undef will match the existing behavior on
> platforms where ./configure is used because the switch
> --enable-tap-tests needs to be used there. And I would believe that in
> most cases Windows environments are not going to have IPC::Run
> deployed.

But if I don't set it to anything, then it will be "initialized" as
undef, so it has the same effect.

-- 
Á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] Re: In-core regression tests for replication, cascading, archiving, PITR, etc.

[Tom Lane]

Alvaro Herrera  writes:
> Michael Paquier wrote:
>> We would definitely want something within the ephemeral port range, so
>> we are up to that:
>> rand() * 16384 + 49152;

> Yes, this seems to produce the correct range.

Speaking of ephemeral port range ... if get_new_node() were to run
past port 65535, which is certainly possible with this new code,
it would fail altogether.  Seems it needs to wrap around properly
within that port range.

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: In-core regression tests for replication, cascading, archiving, PITR, etc.

[Alvaro Herrera]

Tom Lane wrote:
> Alvaro Herrera  writes:
> > Michael Paquier wrote:
> >> We would definitely want something within the ephemeral port range, so
> >> we are up to that:
> >> rand() * 16384 + 49152;
> 
> > Yes, this seems to produce the correct range.
> 
> Speaking of ephemeral port range ... if get_new_node() were to run
> past port 65535, which is certainly possible with this new code,
> it would fail altogether.  Seems it needs to wrap around properly
> within that port range.

Oh, of course.  Pushed fix.

-- 
Á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] Foreign join pushdown vs EvalPlanQual

[Tom Lane]

Robert Haas  writes:
> I think the core system likely needs visibility into where paths and
> plans are present in node trees, and putting them somewhere inside
> fdw_private would be going in the opposite direction.

Absolutely.  You don't really want FDWs having to take responsibility
for setrefs.c processing of their node trees, for example.  This is why
e.g. ForeignScan has both fdw_exprs and fdw_private.

I'm not too concerned about whether we have to adjust FDW-related APIs
as we go along.  It's been clear from the beginning that we'd have to
do that, and we are nowhere near a point where we should promise that
we're done doing so.

regards, tom lane


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

Re: [HACKERS] [PATCH] Equivalence Class Filters

[Simon Riggs]

On 7 December 2015 at 16:55, Jim Nasby  wrote:


> Unfortunately I've seen a lot of millisecond queries that have 6 figure
> estimates, due to data being in cache. So I'm not sure how practical that
> would be.
>

We seem to be agreed that longer running queries may benefit from more
thorough optimization.

I like the idea of specifying a goal for the optimization, so the user can
explicitly ask to spend more time. But I would also rather that I didn't
have to do that at all, by using a heuristic that allows us to act sensibly
even when not explicitly instructed by the user. It Just Works.

We can decide what the heuristic limit is based upon the cost of the
technique. I don't think it matters how quick your CPU is, it matters how
long planning takes as a % of the whole expected execution time.

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: [HACKERS] pg_hba_lookup function to get all matching pg_hba.conf entries

[Haribabu Kommi]

On Sat, Dec 5, 2015 at 3:31 AM, Alvaro Herrera  wrote:
> Haribabu Kommi wrote:
>
>> How about as follows?
>>
>> postgres=# select * from pg_hba_lookup('all','all','::1');
>>  line_number | type  | database |  user   |  address  | hostname | method | 
>> options |  mode
>> -+---+--+-+---+--++-+-
>> 84   | local  | ["all"] | ["all"] |   |  | trust  | 
>> {}  | skipped
>> 86   | host   | ["all"] | ["all"] | 127.0.0.1 |  | trust  | 
>> {}  | skipped
>> 88   | host   | ["all"] | ["all"] | ::1   |  | trust  | 
>> {}  | matched
>> (3 rows)
>
> What did you do to the whitespace when posting that table?  I had to
> reformat it pretty heavily to understand what you had.
> Anyway, I think the "mode" column should be right after the line number.
> I assume the "reason" for skipped lines is going to be somewhere in the
> table too.

when i try to copy paste the output from psql, it didn't come properly, so
I adjusted the same to looks properly, but after sending mail, it look ugly.

Added reason column also as the last column of the table and moved the mode
as the second column.

> What happens if a "reject" line is matched?  I hope the lookup
> would terminate there.

whenever any line matches with the given arguments, the function stops
processing further lines.

> What does it mean to query for "all"?  Do you have database and user
> named "all"?  Because otherwise that seems wrong to me; you should be
> able to query for specific databases/users, but not for special
> keywords; maybe I am wrong and there is a use case for this, in which
> case please state what it is.

The 'all' is just passed as a database and user name. In my configuration
I just put every database to match. so just for a test i did that way. There is
no special handling for keywords.

> I see three problems in your code.  One is that the translation of
> auth_method enum to text should be a separate function, not the SQL
> function layer;

Moved into a different function.

>another is that the code to put keywords as JSON object
> values is way too repetitive; the other is that messing with the JSON
> API like that is not nice.  (I don't think we're closed to doing that,
> but that would be a separate discussion).  I think this patch should
> just use the "push value" interface rather than expose add_jsonb.
>
> (I assume the usage of JSON rather than a regular array was already
> discussed and JSON was chosen for some reason.)

Repetitive jsonb object code is moved into a function and used those functions.
Changed all jsonb calls into push value functions.

Regards,
Hari Babu
Fujitsu Australia


pg_hba_lookup_poc_v5.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] [sqlsmith] Failed to generate plan on lateral subqueries

[Andreas Seltenreich]

Tom Lane writes:

> Andreas Seltenreich  writes:
>> I've added new grammar rules to sqlsmith and improved some older ones.
>> This was rewarded with a return of "failed to generate plan" errors.
>
> I believe I've dealt with these cases now.  Thanks for the report!

I no longer see "failed to build any n-way joins" after pulling, but
there are still instances of "could not devise a query plan". Samples below.

regards,
Andreas

select
  ref_1.aa as c0,
  subq_1.c1 as c1,
  coalesce(ref_1.class, ref_1.class) as c2,
  subq_1.c0 as c3
from
  (select
  subq_0.c1 as c0,
  coalesce(sample_0.a, sample_1.i) as c1
from
  public.rtest_t9 as sample_0 tablesample bernoulli (5.6)
inner join public.iportaltest as sample_1 tablesample bernoulli 
(9.8)
on (sample_0.a = sample_1.i ),
  lateral (select
sample_1.d as c0,
ref_0.a as c1,
sample_1.p as c2,
ref_0.a as c3,
ref_0.a as c4,
sample_0.b as c5,
sample_1.i as c6
  from
public.rtest_view2 as ref_0
  where sample_0.b = sample_0.b
  fetch first 93 rows only) as subq_0
where sample_0.b ~<=~ sample_0.b) as subq_1
right join public.e_star as ref_1
on (subq_1.c0 = ref_1.aa )
where ref_1.cc < ref_1.cc
fetch first 59 rows only;

select
  sample_69.tmpllibrary as c0,
  coalesce(sample_69.tmplname, sample_69.tmplname) as c1,
  subq_33.c0 as c2
from
  (select
  coalesce(ref_53.provider, sample_68.typdefault) as c0
from
  pg_catalog.pg_type as sample_68 tablesample bernoulli (6.9)
inner join pg_catalog.pg_shseclabel as ref_53
on (sample_68.typowner = ref_53.objoid ),
  lateral (select
sample_68.typcategory as c0,
ref_54.speaker as c1,
ref_54.speaker as c2
  from
public.test_range_excl as ref_54
  where (ref_53.label >= ref_53.provider)
and (ref_53.label !~* ref_53.provider)
  fetch first 143 rows only) as subq_32
where ref_53.label ~>~ ref_53.label) as subq_33
right join pg_catalog.pg_pltemplate as sample_69 tablesample bernoulli (9.8)
on (subq_33.c0 = sample_69.tmplhandler )
where sample_69.tmplvalidator ~ subq_33.c0
fetch first 131 rows only;


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

Re: [HACKERS] Re: In-core regression tests for replication, cascading, archiving, PITR, etc.

[Michael Paquier]

On Tue, Dec 8, 2015 at 7:46 AM, Alvaro Herrera 
wrote:

> Michael Paquier wrote:
> > On Thu, Dec 3, 2015 at 6:59 AM, Alvaro Herrera wrote:
> > > I didn't push the changed for config_default you requested a few
> > > messages upthread; it's not clear to me how setting it to undef affects
> > > the whole thing.  If setting it to undef makes the MSVC toolchain run
> > > the tap tests in the default config, then I can do it; let's be clear
> > > about what branch to backpatch this to.  Also the "1;" at the end of
> > > RewindTest.
> >
> > Setting it to undef will prevent the tests to run, per vcregress.pl:
> > die "Tap tests not enabled in configuration"
> >   unless $config->{tap_tests};
> > Also, setting it to undef will match the existing behavior on
> > platforms where ./configure is used because the switch
> > --enable-tap-tests needs to be used there. And I would believe that in
> > most cases Windows environments are not going to have IPC::Run
> > deployed.
>
> But if I don't set it to anything, then it will be "initialized" as
> undef, so it has the same effect.
>

Yes, it will have the same effect. Still it is a problem to not list it in
default_config.pl as the other options, no? And that's as well the case
with GetFakeConfigure, which should list it I think for consistency with
the rest. See the attached for example.
-- 
Michael


20151208_tap_msvc_fix.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] [PATCH] Equivalence Class Filters

[David Rowley]

On 8 December 2015 at 04:35, Jim Nasby  wrote:

> On 12/6/15 10:38 AM, Tom Lane wrote:
>
>> I said "in most cases".  You can find example cases to support almost any
>> weird planner optimization no matter how expensive and single-purpose;
>> but that is the wrong way to think about it.  What you have to think about
>> is average cases, and in particular, not putting a drag on planning time
>> in cases where no benefit ensues.  We're not committing any patches that
>> give one uncommon case an 1100X speedup by penalizing every other query
>> 10%,
>> or even 1%; especially not when there may be other ways to fix it.
>>
>
> This is a problem that seriously hurts Postgres in data warehousing
> applications. We can't keep ignoring optimizations that provide even as
> little as 10% execution improvements for 10x worse planner performance,
> because in a warehouse it's next to impossible for planning time to matter.
>
> Obviously it'd be great if there was a fast, easy way to figure out
> whether a query would be expensive enough to go the whole 9 yards on
> planning it but at this point I suspect a simple GUC would be a big
> improvement.


I've certainly been here before [1], but the idea fell of deaf ears.

The biggest frustration for me is that the way Tom always seems to argue
his point it's as if planning time is roughly the same or more expensive
than execution time, and likely that's true in many cases, but I would
imagine in more normal cases that execution time is longer, although I've
never had the guts to stand up and argued this as I don't have any stats to
back me up.

I was talking to Thomas Munro yesterday about this, and he mentioned that
it would be quite nice to have some stats on how much time is spent in the
planner, vs executor. He came up with the idea of adding a column to
pg_stat_statements to record the planning time.

If we could get real statistics on real world cases and we discovered that,
for example on average that the total CPU time of planning was only 1% of
execution time, then it would certainly make adding 2% overhead onto the
planner a bit less of a worry, as this would just be %2 of 1% (0.02%). Such
information, if fed back into the community might be able to drive us in
the right direction when it comes to deciding what needs to be done to
resolve this constant issue with accepting planner improvement patches.

I believe that with parallel query on the horizon for 9.6 that we're now
aiming to support bigger OLAP type database than ever before. So if we
ignore patches like this one then it appears that we have some conflicting
goals in the community as it seems that we're willing to add the brawn, but
we're not willing to add the brain. If this is the case then it's a shame,
as I think we can have both. So I very much agree on the fact that we must
find a way to maintain support and high performance of small OLTP databases
too.

[1]
http://www.postgresql.org/message-id/caaphdvrjrz-0xinyiqtiws0mfx17gwd2y8vz+i92nuzsha8...@mail.gmail.com

--
 David Rowley   http://www.2ndQuadrant.com/

 PostgreSQL Development, 24x7 Support, Training & Services