Re: [HACKERS] Parallel Seq Scan

[Amit Kapila]

On Sat, Sep 26, 2015 at 5:54 AM, Robert Haas  wrote:
>
> On Fri, Sep 25, 2015 at 12:55 AM, Amit Kapila 
wrote:
>
> > Yes, the patch needs more work in terms of dealing with
parallel-restricted
> > expressions/functions.  One idea which I have explored previously is
> > push down only safe clauses to workers (via partialseqscan node) and
> > execute restricted clauses in master (via Funnel node).  My analysis
> > is as follows:
> >
> > Usage of restricted functions in quals-
> > During create_plan() phase, separate out the quals that needs to be
> > executed at funnel node versus quals that needs to be executed on
> > partial seq scan node (do something similar to what is done in
> > create_indexscan_plan for index and non-index quals).
> >
> > Basically PartialSeqScan node can contain two different list of quals,
> > one for non-restrictive quals and other for restrictive quals and then
> > Funnel node can retrieve restrictive quals from partialseqscan node,
> > assuming partialseqscan node is its left child.
> >
> > Now, I think the above can only be possible under the assumption that
> > partialseqscan node is always a left child of funnel node, however that
is
> > not true because a gating node (Result node) can be added between the
> > two and tomorrow there could be more cases when other nodes will be
> > added between the two, if we consider the case of aggregation, the
> > situation will be more complex as before partial aggregation, all the
> > quals should be executed.
>
> What's the situation where the gating Result node sneaks in there?
>

The plan node structure will be something like
Funnel->Result->PartialSeqScan.  Both Result and PartialSeqScan are left
children in this structure.  Assuming at PartialSeqScan node, we have
identified the separate lists of Restrictive and Safe clauses (while forming
PartialSeqScan node, we can go through each of the clause and separate
out the safe and restrictive clause list with logic similar to what we
follow in
create_indexscan_plan().  here we need to be careful that if there is an OR
condition between restrictive and safe expression, both the expressions
should
be executed at funnel node.), we need to propagate that information to
Funnel
node while creating funnel plan.  In this case, while forming Funnel plan
node,
we need to traverse the left children and if there is any node (in this case
PartialSeqScan node) which has restrictive clauses, we need to pull those to
Funnel node.  So, the situation is that we need to traverse the whole plan
tree
and pull up the restrictive clauses from the nodes (which contain
them) beneath
Funnel node.


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

Re: [HACKERS] cluster_name and update_process_title documentation

[Tom Lane]

Peter Eisentraut  writes:
> The related settings cluster_name and update_process_title have somehow
> ended up at opposite corners of the documentation and sample files.  I
> propose to group them together in a new "Process Title" section, as in
> the attached patch.

I think you missed some places that need to be updated to add a new
config_group value; at least guc.c's config_group_names[] array, and
there might be other places.  But +1 for the concept.

(I wonder if we couldn't add a static assert to catch this class of
error in the future.)

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] [COMMITTERS] pgsql: Fix an O(N^2) problem in foreign key references.

[Tom Lane]

Jan Wieck  writes:
> On 09/18/2015 10:47 AM, Tom Lane wrote:
>> Attached is something closer to what I was envisioning; can you do
>> performance testing on it?

> Yes, that patch also has the desired performance for restoring a schema 
> with hundreds of thousands of foreign key constraints.

Great, thanks for checking.  I'll push it in a moment.

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] No Issue Tracker - Say it Ain't So!

[Joshua D. Drake]

Hello,

I accidently sent this directly to TGL so here we go:

Hello,

I am pretty sure RT can do what I am about to suggest but I know Redmine 
can do it. Consider the following situation:


Robert Haas posts: Parallelized Joins patch
New issue is created (via email)
Patch is automatically attached to issue
TGL reponds: Looks good but fix XYZ
Issue updated
Haas posts new patch
Issue updated
New patch, with date automatically appended to issue
Grittner responds: Tested this, looks good, committing
In body of email Grittner writes:
status: committed
Issue updated
Status set to committed
Issue closed

Now, take the same thing and apply it to a bug. We even have the ability 
to say who can assign something committed. For example, we could set it 
so that only the email address of the git-hook can assign a ticket 
committed. Other things we can do:


* Assign to specific releases
* Move issues to different trackers (not a bug, but a feature request)
* Assign issues to different committers/reviewers/users
* Relatively easy to make work as an issue/bug/commit/discussion tracker
* Easy to move patches/feature requests/bugs between trackers/releases
* East to have different trackers for different releases
* Full role and rights customization
* Interfaces with GIT
* Built in WIKI
* Runs on PostgreSQL
* Has an API
* Already works with PostgreSQL community accounts
* Hugely active community that isn't centric
* Manageable via email (requirement for most hackers)

In short, although we are talking about an issue tracker this is 
something that can be integrated into our existing workflow, habits of 
the current hackers would have to adapt not completely change.


Joshua D. Drake

EDIT: Note that I am not trying to start an argument about a bunch of 
different issues here. I am trying to illustrate how a properly managed 
system could not only integrate into but enhance our current workflow.


--
Command Prompt, Inc. - http://www.commandprompt.com/  503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Announcing "I'm offended" is basically telling the world you can't
control your own emotions, so everyone else should do it for you.


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

Re: [HACKERS] WIP: Rework access method interface

[Alvaro Herrera]

Teodor Sigaev wrote:
> >I'm OK about continuing work on amvalidate if we can build consuensus on its 
> >design.
> >Could you give some feedback on amvalidate version of patch please?
> >http://www.postgresql.org/message-id/capphfds8zywenz9vw6te5rzxbol1vu_wsw181veq+mu+v1d...@mail.gmail.com
> 
> In attach a bit modified patch based on 4-th version, and if there are no
> strong objections, I will commit it. Waiting this patch stops Alexander's
> development on CREATE ACCESS METHOD and he needs to move forward.

I think the messages in ereport() need some work -- at the bare minimum,
do not uppercase the initial.  Also things such as whether operation
names such as "order by" ought to be uppercase or in quotes, for example
here:

> + ereport(ERROR,
> + 
> (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
> +  errmsg("BRIN doesn't support order by 
> operators")));

I think the API of getOpFamilyInfo is a bit odd; is the caller expected
to fill intype and keytype always, and then it only sets the procs/opers
lists?  I wonder if it would be more sensible to have that routine
receive the pg_opclass tuple (or even the opclass OID) instead of the
opfamily OID.

I think "amapi.h" is not a great file name.  What about am_api.h?

I'm unsure about BRIN_NPROC.  Why did you set it to 15?  It's not
unthinkable that future opclass frameworks will have different numbers
of support procs.  For BRIN I'm thinking that we could add another
support proc which validates the opclass definition using the specific
framework; that way we will be able to check that the set of operators
defined are correct, etc (something that the current approach cannot
do).

I think another pgindent run would be good -- there's some broken
whitespace here and there.

-- 
Á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] Less than ideal error reporting in pg_stat_statements

[Marti Raudsepp]

On Wed, Sep 23, 2015 at 3:01 AM, Peter Geoghegan  wrote:
> I think that the real problem here is that garbage collection needs to
> deal with OOM more appropriately.

+1

I've also been seeing lots of log messages saying "LOG:  out of
memory" on a server that's hosting development databases. I put off
debugging this until now because it didn't seem to have any adverse
effects on the system.

The file on my system is currently 5.1GB (!). I don't know how it got
there -- under normal circumstances we don't have any enormous
queries, but perhaps our application bugs during development triggered
that.

The configuration on this system is pg_stat_statements.max = 1 and
pg_stat_statements.track = all.


The comment near gc_qtexts says:
* This won't be called often in the typical case, since it's likely that
* there won't be too much churn, and besides, a similar compaction process
* occurs when serializing to disk at shutdown or as part of resetting.
* Despite this, it seems prudent to plan for the edge case where the file
* becomes unreasonably large, with no other method of compaction likely to
* occur in the foreseeable future.
[...]
* Load the old texts file.  If we fail (out of memory, for instance) just
* skip the garbage collection.

So, as I understand it: if the system runs low on memory for an
extended period, and/or the file grows beyond 1GB (MaxAlloc), garbage
collection stops entirely, meaning it starts leaking disk space until
a manual intervention.

It's very frustrating when debugging aides cause further problems on a
system. If the in-line compaction doesn't materialize (or it's decided
not to backport it), I would propose instead to add a test to
pgss_store() to avoid growing the file beyond MaxAlloc (or perhaps
even a lower limit). Surely dropping some statistics is better than
this pathology.

Regards,
Marti


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

Re: [HACKERS] WIP: Rework access method interface

[Alexander Korotkov]

On Fri, Sep 25, 2015 at 6:25 PM, Tom Lane  wrote:

> Petr Jelinek  writes:
> > On 2015-09-25 16:11, Teodor Sigaev wrote:
> >> In attach a bit modified patch based on 4-th version, and if there are
> >> no strong objections, I will commit it. Waiting this patch stops
> >> Alexander's development on CREATE ACCESS METHOD and he needs to move
> >> forward.
>
> > The code itself looks ok to me, but before we can think about committing
> > this the documentation has to be updated.
>
> Yes.  Also, for a major change like this, it would be a good thing if
> the documentation got a review from a native English speaker.  I will
> volunteer to handle it if someone else does the first draft.
>

Great! I'll write this documentation.

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

Re: [HACKERS] WIP: Rework access method interface

[Petr Jelinek]

On 2015-09-25 16:11, Teodor Sigaev wrote:

I'm OK about continuing work on amvalidate if we can build consuensus
on its design.
Could you give some feedback on amvalidate version of patch please?
http://www.postgresql.org/message-id/capphfds8zywenz9vw6te5rzxbol1vu_wsw181veq+mu+v1d...@mail.gmail.com



In attach a bit modified patch based on 4-th version, and if there are
no strong objections, I will commit it. Waiting this patch stops
Alexander's development on CREATE ACCESS METHOD and he needs to move
forward.



The code itself looks ok to me, but before we can think about committing 
this the documentation has to be updated.


--
 Petr Jelinek  http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


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

Re: [HACKERS] WIP: Rework access method interface

[Tom Lane]

Petr Jelinek  writes:
> On 2015-09-25 16:11, Teodor Sigaev wrote:
>> In attach a bit modified patch based on 4-th version, and if there are
>> no strong objections, I will commit it. Waiting this patch stops
>> Alexander's development on CREATE ACCESS METHOD and he needs to move
>> forward.

> The code itself looks ok to me, but before we can think about committing 
> this the documentation has to be updated.

Yes.  Also, for a major change like this, it would be a good thing if
the documentation got a review from a native English speaker.  I will
volunteer to handle it if someone else does the first draft.

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] No Issue Tracker - Say it Ain't So!

[Simon Riggs]

On 24 September 2015 at 12:16, Tom Lane  wrote:

> I promised myself I'd stay out of this discussion, but ...
>
> Josh Berkus  writes:
> > I know we're big on reinventing the wheel here, but it would really be a
> > better idea to use an established product than starting over from
> > scratch. Writing a bug tracker is a lot of work and maintenance.
>
> Yeah; "let's write our own bug tracker" is a good way to make sure nothing
> comes of this.  On the other hand, "let's get all the Postgres hackers to
> change their habits" is an equally good way to make sure nothing comes of
> this.  (We tried that once already, with I-forget-which tracker; the
> results were, um, forgettable.)  I think the only approach that has any
> chance of success is to connect up some existing tracker software to more
> or less our existing work patterns.  So somebody who wants to make this
> happen needs to sit down and do that.
>
> FWIW, I concur with the opinions that it needs to be an OSS tracker.
> This project has been around for twenty years and I have every expectation
> that it will survive for another twenty.  I have no confidence in any
> closed-source product still being available (and free) in 2035.  We need
> something with an active development/support community, too, so it doesn't
> end up being us supporting it on our ownsome ten years out.  Other than
> that, I'm agnostic as to what gets picked.
>

Every application comes with its own presumed workflow. All implementations
of third party software include tailoring to the specific workflow to meet
the business requirement. (Which is...???)

I'm not at all concerned about what software we use for things, but I am
not in favour of adopting a new workflow, especially one that carries with
it certain presumptions that are not in fact true or even close, for us.

When a bug gets identified, the "assign to" function isn't going to work
well here. Who has the right to assign? Who has the duty to be assigned?
Especially when somebody starts talking about SLAs because then we'll be
talking about clocking-in etc.. That looks like a long and unpleasant
discussion to me, one that involves people that don't write code laying
down the rules for people that do, driven by rules implicit within a
particular package. Will we change our process every time the package
version changes? What happens if our process changes and the package does
not?

Writing or heavily adapting software that follows our way of working is
actually the only way this can ever succeed, like it has for CF app.

I have frequently been the agent of change in matters of process, but I see
no useful change here, just lots of wasted time. But then why are we even
talking about change? What thing is broken that needs to be fixed? Why is
adopting a new package a fix for that?

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

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

Re: [HACKERS] pageinspect patch, for showing tuple data

[Nikolay Shaplov]

В письме от 11 сентября 2015 15:12:04 пользователь Michael Paquier написал:

> > Ok.Let's come to the final decision with tuple_data_parse, and i will add
> > this switch there and to pure sql heap_page_item_attrs
> 
> Fine for me.

Here is final version with documentation.

Hope it will be the last one. :-)

-- 
Nikolay Shaplov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Companydiff --git a/contrib/pageinspect/Makefile b/contrib/pageinspect/Makefile
index aec5258..e4bc1af 100644
--- a/contrib/pageinspect/Makefile
+++ b/contrib/pageinspect/Makefile
@@ -5,7 +5,7 @@ OBJS		= rawpage.o heapfuncs.o btreefuncs.o fsmfuncs.o \
 		  brinfuncs.o ginfuncs.o $(WIN32RES)
 
 EXTENSION = pageinspect
-DATA = pageinspect--1.3.sql pageinspect--1.2--1.3.sql \
+DATA = pageinspect--1.4.sql pageinspect--1.3--1.4.sql pageinspect--1.2--1.3.sql \
 	pageinspect--1.1--1.2.sql pageinspect--1.0--1.1.sql \
 	pageinspect--unpackaged--1.0.sql
 PGFILEDESC = "pageinspect - functions to inspect contents of database pages"
diff --git a/contrib/pageinspect/heapfuncs.c b/contrib/pageinspect/heapfuncs.c
index 8d1666c..d42ca48 100644
--- a/contrib/pageinspect/heapfuncs.c
+++ b/contrib/pageinspect/heapfuncs.c
@@ -29,7 +29,11 @@
 #include "funcapi.h"
 #include "utils/builtins.h"
 #include "miscadmin.h"
+#include "utils/array.h"
+#include "utils/rel.h"
+#include "catalog/pg_type.h"
 
+Datum split_tuple_data(char *tuple_data, uint16 tuple_data_len, TupleDesc tuple_desc, uint16 t_infomask, uint16 t_infomask2, bits8 *t_bits, bool do_detoast, int error_level);
 
 /*
  * bits_to_text
@@ -122,8 +126,8 @@ heap_page_items(PG_FUNCTION_ARGS)
 		HeapTuple	resultTuple;
 		Datum		result;
 		ItemId		id;
-		Datum		values[13];
-		bool		nulls[13];
+		Datum		values[14];
+		bool		nulls[14];
 		uint16		lp_offset;
 		uint16		lp_flags;
 		uint16		lp_len;
@@ -155,6 +159,8 @@ heap_page_items(PG_FUNCTION_ARGS)
 		{
 			HeapTupleHeader tuphdr;
 			int			bits_len;
+			bytea		*tuple_data_bytea;
+			int			tuple_data_len;
 
 			/* Extract information from the tuple header */
 
@@ -168,6 +174,13 @@ heap_page_items(PG_FUNCTION_ARGS)
 			values[9] = UInt32GetDatum(tuphdr->t_infomask);
 			values[10] = UInt8GetDatum(tuphdr->t_hoff);
 
+			/* Copy raw tuple data into bytea attribute */
+			tuple_data_len = lp_len - tuphdr->t_hoff;
+			tuple_data_bytea = (bytea *) palloc(tuple_data_len + VARHDRSZ);
+			SET_VARSIZE(tuple_data_bytea, tuple_data_len + VARHDRSZ);
+			memcpy(VARDATA(tuple_data_bytea), (char *) tuphdr + tuphdr->t_hoff, tuple_data_len);
+			values[13] = PointerGetDatum(tuple_data_bytea);
+			nulls[13] = false;
 			/*
 			 * We already checked that the item is completely within the raw
 			 * page passed to us, with the length given in the line pointer.
@@ -208,7 +221,7 @@ heap_page_items(PG_FUNCTION_ARGS)
 			 */
 			int			i;
 
-			for (i = 4; i <= 12; i++)
+			for (i = 4; i <= 13; i++)
 nulls[i] = true;
 		}
 
@@ -223,3 +236,210 @@ heap_page_items(PG_FUNCTION_ARGS)
 	else
 		SRF_RETURN_DONE(fctx);
 }
+
+PG_FUNCTION_INFO_V1(tuple_data_split);
+Datum
+tuple_data_split(PG_FUNCTION_ARGS)
+{
+	Oidrel_oid;
+	bytea			*raw_data;
+	uint16			t_infomask;
+	uint16			t_infomask2;
+	text			*t_bits_str;
+	RelationData	*rel;
+	TupleDesc		tuple_desc;
+	bool			do_detoast = false;
+	interror_level = ERROR;
+
+	bits8			*t_bits = NULL;
+	Datum			res;
+
+	rel_oid		= PG_GETARG_OID(0);
+	raw_data	= PG_GETARG_BYTEA_P(1);
+	t_infomask	= PG_GETARG_INT16(2);
+	t_infomask2	= PG_GETARG_INT16(3);
+	t_bits_str	= PG_ARGISNULL(4) ? NULL : PG_GETARG_TEXT_P(4);
+	if (PG_NARGS()>=6) do_detoast = PG_GETARG_BOOL(5);
+	if (PG_NARGS()>=7) error_level = PG_GETARG_BOOL(6)?WARNING:ERROR;
+
+	if (!superuser())
+		ereport(ERROR,
+(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+	(errmsg("must be superuser to use raw page functions";
+
+	/*
+	 * Here we converting t_bits string back to the bits8 array,
+	 * as it really is in the tuple header
+	 */
+	if (t_infomask & HEAP_HASNULL)
+	{
+		int bits_str_len;
+		int bits_len;
+		char * p;
+		int off;
+		char byte = 0;
+		bits_len = (t_infomask2 & HEAP_NATTS_MASK) / 8 + 1;
+		if (! t_bits_str)
+		{
+			ereport(ERROR,
+	(errcode(ERRCODE_DATA_CORRUPTED),
+		errmsg("t_bits argument is NULL, though we expect it to be NOT NULL and %i character long", bits_len * 8)));
+		}
+		bits_str_len =  VARSIZE(t_bits_str) - VARHDRSZ;
+		if (bits_str_len % 8)
+		{
+			ereport(ERROR,
+	(errcode(ERRCODE_DATA_CORRUPTED),
+		errmsg("t_bits argument length should be multiple of eight")));
+		}
+		if (bits_len * 8 != bits_str_len)
+		{
+			ereport(ERROR,
+	(errcode(ERRCODE_DATA_CORRUPTED),
+		errmsg("wrong t_bits argument length. Expected %i, actual is %i", bits_len * 8, bits_str_len)));
+		}
+		t_bits = palloc(bits_len + 1);
+
+		p = (char *) t_bits_str + VARHDRSZ;
+		off = 0;
+		while (off

Re: [HACKERS] No Issue Tracker - Say it Ain't So!

[Joshua D. Drake]

On 09/25/2015 09:55 AM, Joe Conway wrote:

On 09/25/2015 09:32 AM, Tom Lane wrote:

2. There's no visibility for outsiders as to what issues are open or
recently fixed.  Not being outsiders, I'm not sure that we are terribly
well qualified to describe this problem precisely or identify a good
solution --- but I grant that there's a problem there.

I do not know how much emphasis the project should place on point #2.
By definition, fixing that will not return any direct benefit to us.


I would argue that there is some benefit for us in terms of advocacy. We
certainly get criticized for our lack of a bug tracker. I don't know of
anyone who specifically rejected Postgres because of that fact, but
would not be surprised if has happened.


I don't know if it has happened but I know that they have at least 
considered it. I also know that when standing in front of a room full of 
PostgreSQL users (Professional developers) and they find out we don't 
have a tracker? The only word for the response is: mortified.


JD



--
Command Prompt, Inc. - http://www.commandprompt.com/  503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Announcing "I'm offended" is basically telling the world you can't
control your own emotions, so everyone else should do it for you.


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

Re: [HACKERS] On-demand running query plans using auto_explain and signals

[Shulgin, Oleksandr]

On Fri, Sep 18, 2015 at 7:04 PM, Robert Haas  wrote:

>
> Frankly, I think you guys are making this out to be way more
> complicated than it really is.  Basically, I think the process being
> queried should publish a DSM via a slot it owns.  The recipient is
> responsible for reading it and then notifying the sender.  If a second
> process requests data before the first process reads its data, the
> second process can either (1) become an additional reader of the
> already-published data or (2) wait for the first process to finish,
> and then send its own inquiry.
>
> There are some problems to solve here, but they hardly seem impossible.


Thank you Robert, for your invaluable input on this patch!

I now believe that use of ProcessInterrupts() in the recently proposed
design as well as manipulation of proc latch due to use of shared memory
queue are major blockers.

In order to simplify things up, I've taken a step back and looked again at
the auto_explain contrib module.  I now propose the most simple thing
possible in my opinion: a new GUC option for auto_explain.  It will make
every backend, in which the module is loaded via *_preload_libraries
mechanism or using CREATE EXTENSION/LOAD commands, to actively publish the
plans of queries in dynamic shared memory as long as
auto_explain.publish_plans is turned on.

The greatest part for me, is that this approach doesn't require handling of
signals and is isolated in an external module, so it can be readily used
with the current server versions, no need to wait for >= 9.6.

Some implementation details:

For every backend that might be running (MaxBackends) we reserve a
dsm_handle slot in the addins shared memory.  When the new option is turned
on, the ExecutorRun hook will produce a plan in whatever format is
specified by the auto_explain.log_format, allocate a DSM segment, copy the
plan into the segment and finally store the DSM handle into its own slot.
No locking is required around this because every backend writes to its slot
exclusively, no other backend can be writing into it concurrently.  In the
ExecutorFinish hook we invalidate the backend's slot by setting the handle
to 0 and deallocate the DSM segment.

Reading of the plan is performed by a newly added function
pg_explain_backend(PID).  Since it can determine the target process'
backendId, it simply reads the DSM handle from that backend's slot and
tries to attach it (there's not much point in checking the handle for being
non-0, because the other backend could release the segment the moment after
we've checked it, so we rely on dsm_attach returning non-NULL).  If
attached successfully, we parse the contents and detach.  At this point the
backend to detach the last is actually releasing the segment, due to
reference counting.

Handling of the nested statements plans is an open question.  It can be
really useful when the top-level plan is simply displaying a "SELECT
my_stored_procedure()" and all the interesting stuff is happening behind
the scenes, but I didn't start to think about how this could be implemented
yet.

Pavel was really interested in retrieving the complete query text/plans
which could be over a megabyte in his case (and pg_stat_activity.query is
capped by 10240 bytes I believe).  This is now possible with the patch, but
some others might still want to put a threshold on the allocation,
especially given this is shared memory.  I can envision another GUC, but in
our experience the extremely long queries (and plans) are most of the time
due to use of VALUES() or IN() clauses with a huge list of literals.

I think we could fold the VALUES/IN into "?" if the query/plan text exceeds
the specified threshold, or unconditionally (yes, pg_stat_statements, I'm
looking at you).  This should help in the cases when the most interesting
part is in the plan nodes near the end, but there's such a huge list of
literals before it.

Future plans:

I believe this approach can be extended to enable instrumentation once
again.  The backend executing the query could update the published plan
every once in a while (for example, every N ms or 1% of rows processed in a
node), and any other process interested in this data, can simply read it
without the need for signals and complex and fragile communication.  This
obviously requires a core patch.

Some problems:

There is a potential problem with the limited total number of DSM segments:
it is reserved in a way to only allow 2 per backend (on average) and 64
additional per server, so if you run with the new option enabled at all
times, you're down to only 1 additional DSM per backend (again, on
average).  Not sure how critical this can be, but no one is forced to run
with this option enabled for all backends.

If you don't want to run it enabled at all times, then enabling the GUC
per-backend can be problematic.  It's still possible to update the conf
file and send SIGHUP to a single backend, but it's harder to accomplish
over psql, 

Re: [HACKERS] How to get value of 'Param' of the WHERE clause in the FDW?

[Tom Lane]

Dmitry Chichkov  writes:
> Evaluate via ExecEvalExpr, right?

Yeah.

> And sorry for a beginner question,
> what do I need to do to get that Expr from ForeignScanState?Is it
> accessible at all in old 9.1 API?

I think you're out of luck before 9.2.  There's no provision for
expressions to be executed by the FDW itself in 9.1.  And you can't
really work around that behind the planner's back, because if you put
an expression into your private fdw state, it won't get adjusted properly
during setrefs.c cleanup.

regards, tom lane


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

[HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

[Jeff Janes]

This needs a rebase, there are several conflicts in 
src/backend/executor/nodeAgg.c

Thanks,

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] No Issue Tracker - Say it Ain't So!

[Joe Conway]

On 09/25/2015 09:32 AM, Tom Lane wrote:
> 2. There's no visibility for outsiders as to what issues are open or
> recently fixed.  Not being outsiders, I'm not sure that we are terribly
> well qualified to describe this problem precisely or identify a good
> solution --- but I grant that there's a problem there.
> 
> I do not know how much emphasis the project should place on point #2.
> By definition, fixing that will not return any direct benefit to us.

I would argue that there is some benefit for us in terms of advocacy. We
certainly get criticized for our lack of a bug tracker. I don't know of
anyone who specifically rejected Postgres because of that fact, but
would not be surprised if has happened.

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] No Issue Tracker - Say it Ain't So!

[Joshua D. Drake]

On 09/25/2015 10:27 AM, Simon Riggs wrote:


2. There's no visibility for outsiders as to what issues are open or
recently fixed.  Not being outsiders, I'm not sure that we are terribly
well qualified to describe this problem precisely or identify a good
solution --- but I grant that there's a problem there.


If they can perform "git log" they can view what has happened recently.
Tracking what might happen is much harder for active contributors.




I've never had a user ask me for such a list. All I here is compliments
that our software is incredibly robust.

The only time this info is required is for people that provide a Support
service based upon PostgreSQL, yet are not themselves sufficiently
involved to know what bugs have been reported and are as yet unfixed. I
expect such people are extremely interested in getting other people to
do things that will help their business.

I don't see a sustainable mechanism that can support the manpower
resources required to provide that information to those people, apart
from become an active contributor, or pay someone who is.


I think you are thinking of this from a very small view point. In my 
mind this goes way beyond a bug tracker (I assume there is a reason the 
individual posted and said "issue tracker").


JD



--
Command Prompt, Inc. - http://www.commandprompt.com/  503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Announcing "I'm offended" is basically telling the world you can't
control your own emotions, so everyone else should do it for you.


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

Re: [HACKERS] upcoming infrastructure changes/OS upgrades on *.postgresql.org

[Tom Lane]

Stefan Kaltenbrunner  writes:
> yeah the point about 9.0.x is a very good one - so I think we will
> target mid/end of october for borka so we get a bit of time to deal with
> any fallout from the release (if needed).

Sounds like a plan.

> We could target the same timeframe for gemulon, unless people think that
> doing it this(!) weekend would work as well (as in would be far enough
> out from the upcoming releases).

Personally I would have no objection to upgrading gitmaster this weekend,
if you really wanna do it that soon.  Not next weekend though, that's
too close to the release date.

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] How to get value of 'Param' of the WHERE clause in the FDW?

[Tom Lane]

Dmitry Chichkov  writes:
> Thank you for the reply!  I'm trying to get the correct value and I need it
> at the execution stage.  I just don't see how to get baserestrictinfo in
> the execution stage or if the 'abc123' value would be there at all...

Hm?  At execution, you'd just evaluate whatever the expression is.
The planner constructs don't have much to do with that, and certainly
a Param should not be a special case in any way.

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] No Issue Tracker - Say it Ain't So!

[Merlin Moncure]

On Fri, Sep 25, 2015 at 11:32 AM, Tom Lane  wrote:
> Simon Riggs  writes:
>> I have frequently been the agent of change in matters of process, but I see
>> no useful change here, just lots of wasted time. But then why are we even
>> talking about change? What thing is broken that needs to be fixed? Why is
>> adopting a new package a fix for that?
>
> Fair questions indeed.  I think the core points here are:
>
> 1. We don't have a good process for making sure things don't "slip through
> the cracks".  I think everyone more or less relies on Bruce to run through
> his mailbox periodically and nag them about threads that don't seem to
> have been closed off.  The deficiencies of that are obvious.
>
> 2. There's no visibility for outsiders as to what issues are open or
> recently fixed.  Not being outsiders, I'm not sure that we are terribly
> well qualified to describe this problem precisely or identify a good
> solution --- but I grant that there's a problem there.

This maybe understates the ability of google to match up problem
scenarios with the relevant fix and commentary.  I'm somewhat
skeptical that issue trackers are much of an improvement upon informal
processes.  Bruce will simply have to run through a different systems
to do the same amount of nagging he's always done.

merlin


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

Re: [HACKERS] Less than ideal error reporting in pg_stat_statements

[Peter Geoghegan]

On Fri, Sep 25, 2015 at 11:37 AM, Peter Geoghegan  wrote:
>> So, as I understand it: if the system runs low on memory for an
>> extended period, and/or the file grows beyond 1GB (MaxAlloc), garbage
>> collection stops entirely, meaning it starts leaking disk space until
>> a manual intervention.
>
> I don't think that there is much more to discuss here: this is a bug.
> I will try and write a patch to fix it shortly.

I should add that it only leaks disk space at the rate at which new
queries are observed that are not stored within pg_stat_statements
(due to an error originating in the planner or something -- they
remain "sticky" entries). The reason we've not heard far more problem
reports is that it usually never gets out of hand in the first place.

Come to think of it, you'd have to repeatedly have new queries that
are never "unstickied"; if you have substantively the same query as an
error-during-planning "sticky" entry, it will still probably be able
to use that existing entry (it will become "unstickied" by this second
execution of what the fingerprinting logic considers to be the same
query).

In short, you have to have just the right workload to hit the bug.

-- 
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] Less than ideal error reporting in pg_stat_statements

[Peter Geoghegan]

On Tue, Sep 22, 2015 at 6:01 PM, Peter Geoghegan  wrote:
> I'm doubtful that this had anything to do with MaxAllocSize. You'd
> certainly need a lot of bloat to be affected by that in any way. I
> wonder how high pg_stat_statements.max was set to on this system, and
> how long each query text was on average.

To clarify: I think it probably starts off not having much to do with
MaxAllocSize. However, it might well be the case that transient memory
pressure results in the problematic code path hitting the MaxAllocSize
imitation. So it starts with malloc() returning NULL, which
temporarily blocks garbage collection, but in bad cases the
MaxAllocSize limitation becomes a permanent barrier to performing a
garbage collection (without a manual intervention).

-- 
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] upcoming infrastructure changes/OS upgrades on *.postgresql.org

[Stefan Kaltenbrunner]

On 09/25/2015 08:53 PM, Andres Freund wrote:
> On 2015-09-25 20:47:21 +0200, Stefan Kaltenbrunner wrote:
>> yeah the point about 9.0.x is a very good one - so I think we will
>> target mid/end of october for borka so we get a bit of time to deal with
>> any fallout from the release (if needed).
>>
>> We could target the same timeframe for gemulon, unless people think that
>> doing it this(!) weekend would work as well (as in would be far enough
>> out from the upcoming releases).
> 
> Seems better to do both after the release. It's not like wheezy support
> is running out tomorrow.

yeah - Thinking about it I agree on doing both afterwards, and while
wheezy still has a lot of life left we also have almost 20 boxes still
left to upgrade which takes time (~40 are already done but the ones left
are more complex and have more dependencies and/or complexity) so we
need to keep up the pace ;)




Stefan


-- 
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] How to get value of 'Param' of the WHERE clause in the FDW?

[Dmitry Chichkov]

Evaluate via ExecEvalExpr, right? And sorry for a beginner question,
what do I need to do to get that Expr from ForeignScanState?Is it
accessible at all in old 9.1 API?

I see code that is getting exec_exprs  from ForeignScan *node:
  ForeignScan *fsplan = (ForeignScan *)node->ss.ps.plan;
  List *exec_exprs = (List *)ExecInitExpr((Expr *)fsplan->fdw_exprs,
(PlanState *)node);

  then it goes through the list, initializes paramDesc/ExprState and
executes it via ExecEvalExpr.   Is that what I should do to get that
'abc123' value?

And I'm getting  "‘ForeignScan’ has no member named ‘fdw_exprs’" in the 9.1
API.  Is it possible to do in 9.1?




Is there some alternative way to flatten these subexpressions into consts,
before they are passed to FDW?

Kind Regards,
Dmitry





On Fri, Sep 25, 2015 at 11:54 AM, Tom Lane  wrote:

> ith that, and certainly
> a Param should not be a special case in any w
>

Re: [HACKERS] How to get value of 'Param' of the WHERE clause in the FDW?

[Dmitry Chichkov]

Thank you for the reply!  I'm trying to get the correct value and I need it
at the execution stage.  I just don't see how to get baserestrictinfo in
the execution stage or if the 'abc123' value would be there at all...

Kind regards,
Dmitry

On Fri, Sep 25, 2015 at 11:44 AM, Tom Lane  wrote:

> Dmitry Chichkov  writes:
> > It seems like during fdwPlan(..., RelOptInfo *baserel) stage I'm getting
> > baserel->baserestrictinfo, in which I see a Node *x  of  IsA(x, Param).
> > But it looks like the value 'abc123' is not yet available in the planning
> > stage, right?   And I don't see how can I get baserestrictinfo in the
> > execution stage or if the 'abc123' value would be there...
>
> If you are trying to get an estimated value for some subexpression at plan
> time, estimate_expression_value() is what to use; see for example the uses
> of that function in selfuncs.c.  Keep in mind that it *is* an estimate and
> cannot be guaranteed to still be correct at execution time, since the plan
> might be re-used with another parameter value.
>
> regards, tom lane
>

Re: [HACKERS] upcoming infrastructure changes/OS upgrades on *.postgresql.org

[Andres Freund]

On 2015-09-25 20:47:21 +0200, Stefan Kaltenbrunner wrote:
> yeah the point about 9.0.x is a very good one - so I think we will
> target mid/end of october for borka so we get a bit of time to deal with
> any fallout from the release (if needed).
> 
> We could target the same timeframe for gemulon, unless people think that
> doing it this(!) weekend would work as well (as in would be far enough
> out from the upcoming releases).

Seems better to do both after the release. It's not like wheezy support
is running out tomorrow.

- 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] [PATCH] postgres_fdw extension support

[Paul Ramsey]

Back from summer and conferencing, and finally responding, sorry for
the delay...

On Thu, Aug 20, 2015 at 6:01 PM, Michael Paquier
 wrote:
>
>
> if (needlabel)
> appendStringInfo(buf, "::%s",
> -
> format_type_with_typemod(node->consttype,
> -
> node->consttypmod));
> +
> format_type_be_qualified(node->consttype));
> Pondering more about this one, I think that we are going to need a new
> routine in format_type.c to be able to call format_type_internal as
> format_type_internal(type_oid, typemod, true/false, false, true). If typemod
> is -1, then typemod_given (the third argument) is false, otherwise
> typemod_given is true. That's close to what the C function format_type at
> the SQL level can do except that we want it to be qualified. Regression
> tests will need an update as well.

I ended up switching on whether the type being formatted was an
extension type or not. Extension types need to be fully qualified or
they won't get found by the remote. Conversely if you fully qualify
the built-in types, the regression test for postgres_fdw isn't happy.
This still isn't quite the best/final solution, perhaps something as
simple as this would work, but I'm not sure:

src/backend/utils/adt/format_type.c
+/*
+ * This version allows a nondefault typemod to be specified and fully
qualified.
+ */
+char *
+format_type_with_typemod_qualified(Oid type_oid, int32 typemod)
+{
+   return format_type_internal(type_oid, typemod, true, false, true);
+}

> Comment format is incorrect, this should be written like that:

Comments fixed.

> +   if (!extension_list)
> +   return false;
> I would rather mark that as == NIL instead, NIL is what an empty list is.

Done

> +extern bool is_shippable(Oid procnumber, PgFdwRelationInfo *fpinfo);
> There is no need to pass PgFdwRelationInfo to is_shippable. Only the list of
> extension OIDs is fine.

Done

> That's nitpicking, but normally we avoid more than 80 characters per line of
> code.

Done.

> When attempting to create a server when an extension is not installed we get
> an appropriate error:
> =# CREATE SERVER postgres_server
>  FOREIGN DATA WRAPPER postgres_fdw
> OPTIONS (host 'localhost', port '5432', dbname 'postgres', extensions
> 'seg');
> ERROR:  42601: the "seg" extension must be installed locally before it can
> be used on a remote server
> LOCATION:  extractExtensionList, shippable.c:245
> However, it is possible to drop an extension listed in a postgres_fdw
> server. Shouldn't we invalidate cache as well when pg_extension is updated?
> Thoughts?

For extensions with types, it's pretty hard to pull the extension out
from under the FDW, because the tables using the type will depend on
it. For simpler function-only extensions, it's possible, but as soon
as you pull the extension out and run a query, your FDW will notice
the extension is missing and complain. And then you'll have to update
the foreign server or foreign table entries and the cache will get
flushed. So there's no way to get a stale cache.

> +   list_free(extlist);
> It does not matter much to call list_free here. The list is going to be
> freed in the error context with ERROR.

Done.

> +   foreach(ext, extension_list)
> +   {
> +   Oid extension_oid = (Oid) lfirst(ext);
> +   if (foundDep->refobjid == extension_oid)
> +   {
> +   nresults++;
> +   }
> +   }
> You could just use list_member_oid here. And why not just breaking the loop
> once there is a match? This is doing unnecessary work visibly

Done.

> +By default only built-in operators and functions will be sent from the
> +local to the foreign server. This may be overridden using the following
> option:
> Missing a mention to "data types" here?

Actually extension data types traverse the postgres_fdw without
trouble without this patch, as long as both sides have the extension
installed. So not strictly needed to mention data types.

> It would be good to get some regression tests for this feature, which is
> something doable with the flag EXTRA_INSTALL and some tests with EXPLAIN
> VERBOSE. Note that you will need to use CREATE EXTENSION to make the
> extension available in the new test, which should be I imagine a new file
> like shippable.sql.

I've put a very very small regression file in that tests the shippable
feature, which can be fleshed out further as needed.

Thanks so much!

P.

> Regards,
> --
> Michael
diff --git a/contrib/postgres_fdw/Makefile b/contrib/postgres_fdw/Makefile
index d2b98e1..63576c4 100644
--- a/contrib/postgres_fdw/Makefile
+++ b/contrib/postgres_fdw/Makefile
@@ -1,7 +1,7 @@
 # contrib/postgres_fdw/Makefile
 
 MODULE_big = postgres_fdw
-OBJS = postgres_fdw.o option.o deparse.o connection.o $(WIN32RES)
+OBJS = 

Re: [HACKERS] No Issue Tracker - Say it Ain't So!

[Tom Lane]

Simon Riggs  writes:
> I have frequently been the agent of change in matters of process, but I see
> no useful change here, just lots of wasted time. But then why are we even
> talking about change? What thing is broken that needs to be fixed? Why is
> adopting a new package a fix for that?

Fair questions indeed.  I think the core points here are:

1. We don't have a good process for making sure things don't "slip through
the cracks".  I think everyone more or less relies on Bruce to run through
his mailbox periodically and nag them about threads that don't seem to
have been closed off.  The deficiencies of that are obvious.

2. There's no visibility for outsiders as to what issues are open or
recently fixed.  Not being outsiders, I'm not sure that we are terribly
well qualified to describe this problem precisely or identify a good
solution --- but I grant that there's a problem there.

I do not know how much emphasis the project should place on point #2.
By definition, fixing that will not return any direct benefit to us.
However, point #1 is clearly an issue and I think a low-overhead fix
for it would be a good thing.  If we can get some answer for #2 out
of it at the same time, even better.

regards, tom lane


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

Re: [HACKERS] pageinspect patch, for showing tuple data

[Nikolay Shaplov]

В письме от 25 сентября 2015 20:59:29 пользователь Michael Paquier написал:

> > Here is final version with documentation.
> 
> Thanks! I just had a short look at it:
> - I am not convinced that it is worth declaring 3 versions of
> tuple_data_split.
How which of them should we leave? 

> - The patch does not respect the project code style,
> particularly one-line "if condition {foo;}" are not adapted, code line is 
limited
> to 80 characters, etc. The list is basically here:
> http://www.postgresql.org/docs/current/static/source.html
I did my best. Results are attached.

> - Be aware of typos: s/whitch/which is one.
I've run spellchecker on all comments. Hope that I removed most of the 
mistakes. But as I am not native speaker, I will not be able to eliminate them 
all. I will need help here.
 
> +t_infomask2
> +integer
> +stores number of attributes (11 bits of the word). The
> rest are used for flag bits:
> +
> +0x2000 - tuple was updated and key cols modified, or tuple deleted
> +0x4000 - tuple was HOT-updated
> +0x8000 - this is heap-only tuple
> +0xE000 - visibility-related bits (so called "hint bits")
> +
> This large chunk of documentation is a duplicate of storage.sgml. If
> that's really necessary, it looks adapted to me to have more detailed
> comments at code level directly in heapfuncs.c.
Hm... There is no explanation of t_infomask/t_infomask2 bits in storage.sgml. 

If there is no source of information other then source code, then the 
documentation is not good.

If there were information about t_infomask/t_infomask2 in storage.sgml, then I 
would add "See storage.sgml for more info" into pageinspect doc, and thats 
all. But since there is no such information there, I  think that the best 
thing is to quote comments from source code there, so you can get all 
information from documentation, not looking for it in the code.

So I would consider two options: Either move t_infomask/t_infomask2 details 
into storage.sgml or leave as it is.

I am lazy, and does not feel confidence about touching main documentation, so I 
would prefer to leave as it is.


> The example of tuple_data_split in the docs would be more interesting
> if embedded with a call to heap_page_items.

This example would be almost not readable. May be I should add one more praise 
explaining where did we take arguments for tuple_data_split


-- 
Nikolay Shaplov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Companydiff --git a/contrib/pageinspect/Makefile b/contrib/pageinspect/Makefile
index aec5258..e4bc1af 100644
--- a/contrib/pageinspect/Makefile
+++ b/contrib/pageinspect/Makefile
@@ -5,7 +5,7 @@ OBJS		= rawpage.o heapfuncs.o btreefuncs.o fsmfuncs.o \
 		  brinfuncs.o ginfuncs.o $(WIN32RES)
 
 EXTENSION = pageinspect
-DATA = pageinspect--1.3.sql pageinspect--1.2--1.3.sql \
+DATA = pageinspect--1.4.sql pageinspect--1.3--1.4.sql pageinspect--1.2--1.3.sql \
 	pageinspect--1.1--1.2.sql pageinspect--1.0--1.1.sql \
 	pageinspect--unpackaged--1.0.sql
 PGFILEDESC = "pageinspect - functions to inspect contents of database pages"
diff --git a/contrib/pageinspect/heapfuncs.c b/contrib/pageinspect/heapfuncs.c
index 8d1666c..52a0663 100644
--- a/contrib/pageinspect/heapfuncs.c
+++ b/contrib/pageinspect/heapfuncs.c
@@ -29,7 +29,14 @@
 #include "funcapi.h"
 #include "utils/builtins.h"
 #include "miscadmin.h"
+#include "utils/array.h"
+#include "utils/rel.h"
+#include "catalog/pg_type.h"
 
+Datum split_tuple_data(char *tuple_data, uint16 tuple_data_len,
+			TupleDesc tuple_desc, uint16 t_infomask,
+			uint16 t_infomask2, bits8 *t_bits, bool
+			do_detoast, int error_level);
 
 /*
  * bits_to_text
@@ -91,7 +98,7 @@ heap_page_items(PG_FUNCTION_ARGS)
 		if (raw_page_size < SizeOfPageHeaderData)
 			ereport(ERROR,
 	(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-  errmsg("input page too small (%d bytes)", raw_page_size)));
+	 errmsg("input page too small (%d bytes)", raw_page_size)));
 
 		fctx = SRF_FIRSTCALL_INIT();
 		mctx = MemoryContextSwitchTo(fctx->multi_call_memory_ctx);
@@ -122,8 +129,8 @@ heap_page_items(PG_FUNCTION_ARGS)
 		HeapTuple	resultTuple;
 		Datum		result;
 		ItemId		id;
-		Datum		values[13];
-		bool		nulls[13];
+		Datum		values[14];
+		bool		nulls[14];
 		uint16		lp_offset;
 		uint16		lp_flags;
 		uint16		lp_len;
@@ -155,6 +162,8 @@ heap_page_items(PG_FUNCTION_ARGS)
 		{
 			HeapTupleHeader tuphdr;
 			int			bits_len;
+			bytea		*tuple_data_bytea;
+			int			tuple_data_len;
 
 			/* Extract information from the tuple header */
 
@@ -168,6 +177,14 @@ heap_page_items(PG_FUNCTION_ARGS)
 			values[9] = UInt32GetDatum(tuphdr->t_infomask);
 			values[10] = UInt8GetDatum(tuphdr->t_hoff);
 
+			/* Copy raw tuple data into bytea attribute */
+			tuple_data_len = lp_len - tuphdr->t_hoff;
+			tuple_data_bytea = (bytea *) palloc(tuple_data_len + VARHDRSZ);
+			SET_VARSIZE(tuple_data_bytea, tuple_data_len + VARHDRSZ);
+			memcpy(VARDATA(tuple_data_bytea), (char *) tuphdr + 

Re: [HACKERS] [COMMITTERS] pgsql: Fix an O(N^2) problem in foreign key references.

[Alvaro Herrera]

Tom Lane wrote:
> Jan Wieck  writes:
> > Attached is a complete rework of the fix from scratch, based on Tom's 
> > suggestion.
> 
> > The code now maintains a double linked list as suggested, but only uses 
> > it to mark all currently valid entries as invalid when hashvalue == 0. 
> > If a specific entry is invalidated it performs a hash lookup for maximum 
> > efficiency in that case.
> 
> That does not work; you're ignoring the possibility of hashvalue
> collisions, and for that matter not considering that the hash value
> is not equal to the hash key.  I don't think your code is invalidating
> the right entry at all during a foreign key constraint update, and
> it certainly cannot do the right thing if there's a hash value collision.

Would it make sense to remove the only the few oldest entries, instead
of all of them?  As is, I think this causes a storm of reloads every
once in a while, if the number of FKs in the system is large enough.
Maybe on a cache hit we could push the entry to the head of the list,
and then remove N entries from the back of the list when the threshold
is reached.

I think the assumption here is that most sessions will not reach the
threshold at all, except for those that access all tables such as
pg_dump -- that is, most sessions are short-lived.  But in cases
involved connection poolers, eventually all sessions would access all
tables, and thus be subject to the storm issue.

(Of course, this is all hypothetical.)

-- 
Á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] No Issue Tracker - Say it Ain't So!

[Tom Lane]

Joe Conway  writes:
> On 09/25/2015 09:32 AM, Tom Lane wrote:
>> I do not know how much emphasis the project should place on point #2.
>> By definition, fixing that will not return any direct benefit to us.

> I would argue that there is some benefit for us in terms of advocacy. We
> certainly get criticized for our lack of a bug tracker. I don't know of
> anyone who specifically rejected Postgres because of that fact, but
> would not be surprised if has happened.

Yeah.  I should have put in something to the effect that there may be
indirect benefits, but they're pretty hard to quantify.

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] upcoming infrastructure changes/OS upgrades on *.postgresql.org

[Tom Lane]

Stefan Kaltenbrunner  writes:
> * borka.postgresql.org - official tarball and docs build server, the
> upgrade will upgrade the toolchain on that box to the respective
> versions of what is in jessie. If people think it is necessarily to
> (double)check the effects beforehand we would like to know so we can
> coordinate some testing.

I think it's probably sufficient to run a test tarball build after
the upgrade; I can do so if you notify me when the upgrade's done.

As Andres says nearby, it would be good if this did *not* happen
during the first week of October, since we have releases scheduled
then.  Actually, since that will be the final 9.0.x release, I'd
vote for postponing the borka upgrade till after that.  That's
one less old branch to worry about new-toolchain compatibility for.
And if there is some subtle problem in the tarballs that we find later,
not having to reissue the last 9.0.x release would be a good thing.

regards, tom lane


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

Re: [HACKERS] Doubt in pgbench TPS number

[Fabien COELHO]

Hello Tatsuo,


Hmmm... I never use -C. The formula seems ok:

   tps_exclude = normal_xacts / (time_include -
   (INSTR_TIME_GET_DOUBLE(conn_total_time) / nthreads));


Hmmm... it is not:-)

I think that the degree of parallelism to consider is nclients, not 
nthreads: while connection time is accumulated in conn_time, other clients 
are possibly doing their transactions, in parallel, even if it is in the 
same thread, so it is not "stopped time" for all clients. It starts to 
matter with "-j 1 -c 30" and slow transactions, the cumulated conn_time in 
each thread may be arbitrary close to the whole time if there are many 
clients.


Now, I do not think that this tps computation makes that much sense. If 
you want to know the tps without reconnect, run without reconnecting... It 
is clear that I do not get this figure when running without -C, so maybe

the tps with/without reconnection should be dropped?

Anyway, here is a patch, which:
 - fixes the "exclude" computation (s/nthreads/nclients/)
 - fixes the count total for skipped (s/threads/thread/)
 - removes a useless parameter to doCustom
   (the conn_time is available through the thread parameter).

--
Fabien.diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 6ae1b86..9a7dd6a 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -1126,7 +1126,7 @@ agg_vals_init(AggVals *aggs, instr_time start)
 
 /* return false iff client should be disconnected */
 static bool
-doCustom(TState *thread, CState *st, instr_time *conn_time, FILE *logfile, AggVals *agg)
+doCustom(TState *thread, CState *st, FILE *logfile, AggVals *agg)
 {
 	PGresult   *res;
 	Command   **commands;
@@ -1357,7 +1357,7 @@ top:
 			return clientDone(st, false);
 		}
 		INSTR_TIME_SET_CURRENT(end);
-		INSTR_TIME_ACCUM_DIFF(*conn_time, end, start);
+		INSTR_TIME_ACCUM_DIFF(thread->conn_time, end, start);
 	}
 
 	/*
@@ -2616,7 +2616,7 @@ printResults(int ttype, int64 normal_xacts, int nclients,
 	time_include = INSTR_TIME_GET_DOUBLE(total_time);
 	tps_include = normal_xacts / time_include;
 	tps_exclude = normal_xacts / (time_include -
-		(INSTR_TIME_GET_DOUBLE(conn_total_time) / nthreads));
+		(INSTR_TIME_GET_DOUBLE(conn_total_time) / nclients));
 
 	if (ttype == 0)
 		s = "TPC-B (sort of)";
@@ -3462,7 +3462,7 @@ main(int argc, char **argv)
 
 		/* thread level stats */
 		throttle_lag += thread->throttle_lag;
-		throttle_latency_skipped += threads->throttle_latency_skipped;
+		throttle_latency_skipped += thread->throttle_latency_skipped;
 		latency_late += thread->latency_late;
 		if (throttle_lag_max > thread->throttle_lag_max)
 			throttle_lag_max = thread->throttle_lag_max;
@@ -3578,7 +3578,7 @@ threadRun(void *arg)
 		int			prev_ecnt = st->ecnt;
 
 		st->use_file = getrand(thread, 0, num_files - 1);
-		if (!doCustom(thread, st, >conn_time, logfile, ))
+		if (!doCustom(thread, st, logfile, ))
 			remains--;			/* I've aborted */
 
 		if (st->ecnt > prev_ecnt && commands[st->state]->type == META_COMMAND)
@@ -3717,7 +3717,7 @@ threadRun(void *arg)
 			if (st->con && (FD_ISSET(PQsocket(st->con), _mask)
 			|| commands[st->state]->type == META_COMMAND))
 			{
-if (!doCustom(thread, st, >conn_time, logfile, ))
+if (!doCustom(thread, st, logfile, ))
 	remains--;	/* I've aborted */
 			}
 

-- 
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] No Issue Tracker - Say it Ain't So!

[Simon Riggs]

On 25 September 2015 at 11:55, Joe Conway  wrote:

> On 09/25/2015 09:32 AM, Tom Lane wrote:
> > 2. There's no visibility for outsiders as to what issues are open or
> > recently fixed.  Not being outsiders, I'm not sure that we are terribly
> > well qualified to describe this problem precisely or identify a good
> > solution --- but I grant that there's a problem there.
> >
> > I do not know how much emphasis the project should place on point #2.
> > By definition, fixing that will not return any direct benefit to us.
>
> I would argue that there is some benefit for us in terms of advocacy.


There are also some negatives in terms of advocacy.

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

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

Re: [HACKERS] No Issue Tracker - Say it Ain't So!

[Simon Riggs]

On 25 September 2015 at 11:32, Tom Lane  wrote:

> Simon Riggs  writes:
> > I have frequently been the agent of change in matters of process, but I
> see
> > no useful change here, just lots of wasted time. But then why are we even
> > talking about change? What thing is broken that needs to be fixed? Why is
> > adopting a new package a fix for that?
>
> Fair questions indeed.  I think the core points here are:
>
> 1. We don't have a good process for making sure things don't "slip through
> the cracks".  I think everyone more or less relies on Bruce to run through
> his mailbox periodically and nag them about threads that don't seem to
> have been closed off.  The deficiencies of that are obvious.
>

I don't rely on that myself. That sounds like a personal viewpoint only. I
welcome more discussion amongst Committers with regard to coordination, but
formal systems aren't what I think will help there. That situation has
recently improved anyway, so no further change needed at present, IMHO.


> 2. There's no visibility for outsiders as to what issues are open or
> recently fixed.  Not being outsiders, I'm not sure that we are terribly
> well qualified to describe this problem precisely or identify a good
> solution --- but I grant that there's a problem there.
>

If they can perform "git log" they can view what has happened recently.
Tracking what might happen is much harder for active contributors.

I've never had a user ask me for such a list. All I here is compliments
that our software is incredibly robust.

The only time this info is required is for people that provide a Support
service based upon PostgreSQL, yet are not themselves sufficiently involved
to know what bugs have been reported and are as yet unfixed. I expect such
people are extremely interested in getting other people to do things that
will help their business.

I don't see a sustainable mechanism that can support the manpower resources
required to provide that information to those people, apart from become an
active contributor, or pay someone who is.


> I do not know how much emphasis the project should place on point #2.
> By definition, fixing that will not return any direct benefit to us.
> However, point #1 is clearly an issue and I think a low-overhead fix
> for it would be a good thing.  If we can get some answer for #2 out
> of it at the same time, even better.


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

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

Re: [HACKERS] No Issue Tracker - Say it Ain't So!

[Jeff Janes]

On Wed, Sep 23, 2015 at 2:14 PM, Alvaro Herrera 
wrote:

> Jeff Janes wrote:
>
>
> > I'd rather, say, put some more work into cleaning the kruft out of the
> > To-Do list, then put that effort into migrating the kruft to a fancier
> > filing cabinet.
>
> Casual users would need a community account in order to file bugs in the
> Todo wiki page.


Sorry, I changed lanes without signalling there, from an outward facing bug
tracker to an inward facing issue tracker.

But bug trackers usually do incorporate a to-do list as well.  I think they
are better separate anyway.  Someone who does have an community account
could (and sometimes does now) respond saying "that isn't really a bug, but
a feature request, and I added it to the To Do list".

I don't think a wiki page qualifies (by a long mile) as
> a bug tracker, anyway.
>

Right, they are quite different.  But I don't think the differences make a
difference, as far as making it stop being a place where ideas go to die
without ever realizing they are dead.  It would be nice to know the date on
which any given item was added to the TODO page (and general a wiki version
of "git blame" would be nice), but beyond that I don't see it making a
difference.  Since we want to have the wiki anyway (I assume) then using it
for open-items and todo list is basically free, while an issue tracker is
another piece of software to learn (for its users) and maintain (for the
infrastructure team).

Cheers,

Jeff

Re: [HACKERS] [COMMITTERS] pgsql: Fix an O(N^2) problem in foreign key references.

[Tom Lane]

Alvaro Herrera  writes:
> Would it make sense to remove the only the few oldest entries, instead
> of all of them?  As is, I think this causes a storm of reloads every
> once in a while, if the number of FKs in the system is large enough.
> Maybe on a cache hit we could push the entry to the head of the list,
> and then remove N entries from the back of the list when the threshold
> is reached.

Sure, there's room for optimization of that sort, although I think we
could do with some evidence that it's actually helpful.  I believe that
under "production" workloads InvalidateConstraintCacheCallBack won't
get called much at all, so the problem's moot.

(FWIW, it might take less code to put the recently-used entries at the
back of the list.  Then the loop in InvalidateConstraintCacheCallBack
could just invalidate/delete entries if either they're targets, or
the current list length exceeds the threshold.)

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] [COMMITTERS] pgsql: Fix an O(N^2) problem in foreign key references.

[Tom Lane]

I wrote:
> Alvaro Herrera  writes:
>> Would it make sense to remove the only the few oldest entries, instead
>> of all of them?  As is, I think this causes a storm of reloads every
>> once in a while, if the number of FKs in the system is large enough.
>> Maybe on a cache hit we could push the entry to the head of the list,
>> and then remove N entries from the back of the list when the threshold
>> is reached.

> (FWIW, it might take less code to put the recently-used entries at the
> back of the list.  Then the loop in InvalidateConstraintCacheCallBack
> could just invalidate/delete entries if either they're targets, or
> the current list length exceeds the threshold.)

Specifically, we could change it as per attached.  But this adds some
cycles to the mainline usage of fetching a valid cache entry, so I'd
want to see some evidence that there are use-cases it helps before
applying it.

regards, tom lane

diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 018cb99..54376ed 100644
*** a/src/backend/utils/adt/ri_triggers.c
--- b/src/backend/utils/adt/ri_triggers.c
*** ri_LoadConstraintInfo(Oid constraintOid)
*** 2814,2820 
  		ri_InitHashTables();
  
  	/*
! 	 * Find or create a hash entry.  If we find a valid one, just return it.
  	 */
  	riinfo = (RI_ConstraintInfo *) hash_search(ri_constraint_cache,
  			   (void *) ,
--- 2814,2821 
  		ri_InitHashTables();
  
  	/*
! 	 * Find or create a hash entry.  If we find a valid one, just return it,
! 	 * after pushing it to the end of the list to mark it recently used.
  	 */
  	riinfo = (RI_ConstraintInfo *) hash_search(ri_constraint_cache,
  			   (void *) ,
*** ri_LoadConstraintInfo(Oid constraintOid)
*** 2822,2828 
--- 2823,2833 
  	if (!found)
  		riinfo->valid = false;
  	else if (riinfo->valid)
+ 	{
+ 		dlist_delete(>valid_link);
+ 		dlist_push_tail(_constraint_cache_valid_list, >valid_link);
  		return riinfo;
+ 	}
  
  	/*
  	 * Fetch the pg_constraint row so we can fill in the entry.
*** ri_LoadConstraintInfo(Oid constraintOid)
*** 2931,2936 
--- 2936,2942 
  	/*
  	 * For efficient processing of invalidation messages below, we keep a
  	 * doubly-linked list, and a count, of all currently valid entries.
+ 	 * The most recently used entries are at the *end* of the list.
  	 */
  	dlist_push_tail(_constraint_cache_valid_list, >valid_link);
  	ri_constraint_cache_valid_count++;
*** ri_LoadConstraintInfo(Oid constraintOid)
*** 2948,2953 
--- 2954,2965 
   * Invalidate any ri_constraint_cache entry associated with the syscache
   * entry with the specified hash value, or all entries if hashvalue == 0.
   *
+  * We also invalidate entries if there are too many valid entries.  This rule
+  * avoids O(N^2) behavior in situations where a session touches many foreign
+  * keys and also does many ALTER TABLEs, such as a restore from pg_dump.  When
+  * we do kill entries for that reason, they'll be the least recently used
+  * ones, since they're at the front of the list.
+  *
   * Note: at the time a cache invalidation message is processed there may be
   * active references to the cache.  Because of this we never remove entries
   * from the cache, but only mark them invalid, which is harmless to active
*** InvalidateConstraintCacheCallBack(Datum 
*** 2961,2981 
  
  	Assert(ri_constraint_cache != NULL);
  
- 	/*
- 	 * If the list of currently valid entries gets excessively large, we mark
- 	 * them all invalid so we can empty the list.  This arrangement avoids
- 	 * O(N^2) behavior in situations where a session touches many foreign keys
- 	 * and also does many ALTER TABLEs, such as a restore from pg_dump.
- 	 */
- 	if (ri_constraint_cache_valid_count > 1000)
- 		hashvalue = 0;			/* pretend it's a cache reset */
- 
  	dlist_foreach_modify(iter, _constraint_cache_valid_list)
  	{
  		RI_ConstraintInfo *riinfo = dlist_container(RI_ConstraintInfo,
  	valid_link, iter.cur);
  
! 		if (hashvalue == 0 || riinfo->oidHashValue == hashvalue)
  		{
  			riinfo->valid = false;
  			/* Remove invalidated entries from the list, too */
--- 2973,2985 
  
  	Assert(ri_constraint_cache != NULL);
  
  	dlist_foreach_modify(iter, _constraint_cache_valid_list)
  	{
  		RI_ConstraintInfo *riinfo = dlist_container(RI_ConstraintInfo,
  	valid_link, iter.cur);
  
! 		if (hashvalue == 0 || riinfo->oidHashValue == hashvalue ||
! 			ri_constraint_cache_valid_count > 1000)
  		{
  			riinfo->valid = false;
  			/* Remove invalidated entries from the list, too */

-- 
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] upcoming infrastructure changes/OS upgrades on *.postgresql.org

[Andres Freund]

On 2015-09-25 20:11:32 +0200, Stefan Kaltenbrunner wrote:
> no date set yet though

> * gemulon.postgresql.org - gitmaster.postgresql, master git repository
> (we will likely coordinate this internally so that somebody on the team
> with a commit bit will test after the upgrade)

I'm pretty sure you're already thinking of this - but let's make sure
this doesn't coincide with the upcoming releases.

Greetings,

Andres Freund


-- 
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] Less than ideal error reporting in pg_stat_statements

[Peter Geoghegan]

On Fri, Sep 25, 2015 at 8:51 AM, Marti Raudsepp  wrote:
> I've also been seeing lots of log messages saying "LOG:  out of
> memory" on a server that's hosting development databases. I put off
> debugging this until now because it didn't seem to have any adverse
> effects on the system.
>
> The file on my system is currently 5.1GB (!). I don't know how it got
> there -- under normal circumstances we don't have any enormous
> queries, but perhaps our application bugs during development triggered
> that.

It could be explained by legitimate errors during planning, for
example. The query text is still stored.

> So, as I understand it: if the system runs low on memory for an
> extended period, and/or the file grows beyond 1GB (MaxAlloc), garbage
> collection stops entirely, meaning it starts leaking disk space until
> a manual intervention.

I don't think that there is much more to discuss here: this is a bug.
I will try and write a patch to fix it shortly. It will be
non-trivial, and I'm quite busy right now, so it might take a while. A
short-term remediation is to call pg_stat_statements_reset() on
systems affected like this.

-- 
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] How to get value of 'Param' of the WHERE clause in the FDW?

[Tom Lane]

Dmitry Chichkov  writes:
> It seems like during fdwPlan(..., RelOptInfo *baserel) stage I'm getting
> baserel->baserestrictinfo, in which I see a Node *x  of  IsA(x, Param).
> But it looks like the value 'abc123' is not yet available in the planning
> stage, right?   And I don't see how can I get baserestrictinfo in the
> execution stage or if the 'abc123' value would be there...

If you are trying to get an estimated value for some subexpression at plan
time, estimate_expression_value() is what to use; see for example the uses
of that function in selfuncs.c.  Keep in mind that it *is* an estimate and
cannot be guaranteed to still be correct at execution time, since the plan
might be re-used with another parameter value.

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] No Issue Tracker - Say it Ain't So!

[Joshua D. Drake]

On 09/25/2015 10:11 AM, Simon Riggs wrote:
>

> I do not know how much emphasis the project should place on point #2.
> By definition, fixing that will not return any direct benefit to us.

I would argue that there is some benefit for us in terms of advocacy.


There are also some negatives in terms of advocacy.


I know we are just being thorough here but I think the idea that there 
are negatives in the transparency of our project is simply untrue. The 
only argument I can think of is, "Well then people are going to know 
everything that is broken." To which I respond, "Good, that is a net 
positive".


JD



--
Command Prompt, Inc. - http://www.commandprompt.com/  503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Announcing "I'm offended" is basically telling the world you can't
control your own emotions, so everyone else should do it for you.


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

[HACKERS] upcoming infrastructure changes/OS upgrades on *.postgresql.org

[Stefan Kaltenbrunner]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Hi all!

As part of our regular maintenance schedule and in our continous effort
to run our systems on current and fully supported operating systems the
postgresql sysadmin has started upgrading the OS on our infrastructure
hosts from Debian Wheezy(aka Debian 7.x) to Debian Jessie (Debian 8.x).

This mail is mostly to announce our intend to upgrade the following two
systems in the forseeable future(no date set yet though) that might
directly affect -hackers/-commiters or a subset of those (release team)
and whether people want us to take special measures ahead of time to
ensure minimal disruption to the workflow process:


* gemulon.postgresql.org - gitmaster.postgresql, master git repository
(we will likely coordinate this internally so that somebody on the team
with a commit bit will test after the upgrade)

* borka.postgresql.org - official tarball and docs build server, the
upgrade will upgrade the toolchain on that box to the respective
versions of what is in jessie. If people think it is necessarily to
(double)check the effects beforehand we would like to know so we can
coordinate some
testing.




Stefan

-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJWBY5TAAoJEO1GOCPAcHIudgwQAMHCRkDy9KWWTN3gKsObwWkO
hQ+bc1CBPys5lKohI5Sv0KfunEk0DsbxzyTk3+eYmJRplzD2l8WB/dlYV7Kb0idY
Xi1CZ26/0Jc+YaQ5ImNurGEPFhhZpPw8eNJAxl6Kd7ux0ObQ8DWPKLGeZO3Jj1Mc
Ni+L8PHreg7PlE1j9MU0iSOwASCPNlS0/yf60QPHCZbCrE+7+WSsmFd0Cr8gG9oB
ClG2JzA/tylM/clsyRowAEg9NLj5l57SL/J6dlqbHZEGLy6dgGpkMhucCY8qJ87V
jgEd8jPmY9//Nvs0kIIGuvAhNty8DF0B9styXOBeNjSIbEJWaUr8VO5cRvehE8PN
KKlaFh79QvnxfnWhCWBR5LUFtFUcWBUiIyb6f7xoFY9vwS6surpxFsqiDveJyEZq
BN2z29OAtUxyRo7ZZaAwJiAoxLJ3mbZwIFIcfmXyMS/XE9SfzyuUX1uQZealRXHM
wlAR8NNmVJVOAlx6dvYawtoxJ1aAi63gjU8oQicASze+syS0vD6sKUe3BRp3vC54
st/Fv7ZEkko7Fc+2dpJKp78k+1Rz/M9YGW1XBcKqVuxUy/u6Njo90x9E0Su2cst/
0Jz/P3mckq2T/nf7AzV3Bx3TSnV5TUFPKLzm7+OwIA1lPRfsXXJ3CNRXP2hjmN1A
0Bsx1uUk1k8+bT+xhGT5
=8nHl
-END PGP SIGNATURE-


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

[HACKERS] How to get value of 'Param' of the WHERE clause in the FDW?

[Dmitry Chichkov]

Please help I'm doing a following query to a foreign wrapper:
   FUNCTION fwcall(text)    SELECT * FROM fwtable WHERE col=$1  ;
   ...
   SELECT * from fdwcall('abc123');


I'm looking for a way to get that parameter 'abc123' value in the FDW
wrapper code...


It seems like during fdwPlan(..., RelOptInfo *baserel) stage I'm getting
baserel->baserestrictinfo, in which I see a Node *x  of  IsA(x, Param).
But it looks like the value 'abc123' is not yet available in the planning
stage, right?   And I don't see how can I get baserestrictinfo in the
execution stage or if the 'abc123' value would be there...

Can somebody kick me to the right direction?  Please?

Thanks,
Dmitry

Re: [HACKERS] upcoming infrastructure changes/OS upgrades on *.postgresql.org

[Stefan Kaltenbrunner]

On 09/25/2015 08:30 PM, Tom Lane wrote:
> Stefan Kaltenbrunner  writes:
>> * borka.postgresql.org - official tarball and docs build server, the
>> upgrade will upgrade the toolchain on that box to the respective
>> versions of what is in jessie. If people think it is necessarily to
>> (double)check the effects beforehand we would like to know so we can
>> coordinate some testing.
> 
> I think it's probably sufficient to run a test tarball build after
> the upgrade; I can do so if you notify me when the upgrade's done.
> 
> As Andres says nearby, it would be good if this did *not* happen
> during the first week of October, since we have releases scheduled
> then.  Actually, since that will be the final 9.0.x release, I'd
> vote for postponing the borka upgrade till after that.  That's
> one less old branch to worry about new-toolchain compatibility for.
> And if there is some subtle problem in the tarballs that we find later,
> not having to reissue the last 9.0.x release would be a good thing.

yeah the point about 9.0.x is a very good one - so I think we will
target mid/end of october for borka so we get a bit of time to deal with
any fallout from the release (if needed).

We could target the same timeframe for gemulon, unless people think that
doing it this(!) weekend would work as well (as in would be far enough
out from the upcoming releases).


Stefan


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

[HACKERS] Tab completion for ALTER COLUMN SET STATISTICS

[Jeff Janes]

If I have "alter table foo alter COLUMN bar SET STATISTICS" in the line
buffer,
it tab completes to add " TO", which is not legal.

The attached patch makes it not tab complete anything at all, which is at
least not actively misleading.

I thought of having it complete "-1", "" so that it gives a clue
about what is needed, but I didn't see any precedence for non-literal
clue-giving and I did not want to try to create new precedence.


alter_column_tabcomplete_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] pg_ctl/pg_rewind tests vs. slow AIX buildfarm members

[Tom Lane]

I wrote:
> Attached is a draft patch for this.  I think it's fine for Unix (unless
> someone wants to object to relying on "/bin/sh -c"), but I have no idea
> whether it works for Windows.  The main risk is that if CMD.EXE runs
> the postmaster as a subprocess rather than overlaying itself a la shell
> "exec", the PID we'd get back would apply only to CMD.EXE not to the
> eventual postmaster.  If so, I do not know how to fix that, or whether
> it's fixable at all.

> Note that this makes the test case in question fail reliably, which is
> reasonable behavior IMO so I just changed the test.

> If this does (or can be made to) work on Windows, I'd propose applying
> it back to 9.2, where the current coding came in.

Nobody seems to have stepped up to fix the Windows side of this, but
after some more thought it occurred to me that maybe it doesn't need
(much) fixing.  There are two things that pg_ctl wants the PID for:

1. To check if the postmaster.pid file contains the expected child process
PID.  Well, that's a nice-to-have, but we never had it before and got
along well enough, because the start-timestamp comparison check covers
most real-world cases.  We can omit the PID-match check on Windows.

2. To see whether the child postmaster process is still running, via a
"kill(pid, 0)" test.  But if we've launched the postmaster through a
shell, and not used "&" semantics, presumably that shell will wait for its
child process to exit and then exit itself.  So *testing whether the shell
is still there is just about as good as testing the real postmaster PID*.

Therefore, we don't really need to worry about rewriting the Windows
subprocess-launch logic.  If someone would like to do it, that would
be nice, but we don't have to wait for that to happen before we can
get rid of the 5-second-timeout issues.

So the attached modified patch adjusts the PID-match logic and some
comments, but is otherwise what I posted before.  I believe that this
might actually work on Windows, but I have no way to test it.  Someone
please try that?  (Don't forget to test the service-start path, too.)

regards, tom lane

diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 6a36d29..62db328 100644
*** a/src/bin/pg_ctl/pg_ctl.c
--- b/src/bin/pg_ctl/pg_ctl.c
***
*** 28,33 
--- 28,34 
  #include 
  #include 
  #include 
+ #include 
  #include 
  
  #ifdef HAVE_SYS_RESOURCE_H
*** static int	CreateRestrictedProcess(char 
*** 153,162 
  static pgpid_t get_pgpid(bool is_status_request);
  static char **readfile(const char *path);
  static void free_readfile(char **optlines);
! static int	start_postmaster(void);
  static void read_post_opts(void);
  
! static PGPing test_postmaster_connection(bool);
  static bool postmaster_is_alive(pid_t pid);
  
  #if defined(HAVE_GETRLIMIT) && defined(RLIMIT_CORE)
--- 154,163 
  static pgpid_t get_pgpid(bool is_status_request);
  static char **readfile(const char *path);
  static void free_readfile(char **optlines);
! static pgpid_t start_postmaster(void);
  static void read_post_opts(void);
  
! static PGPing test_postmaster_connection(pgpid_t pm_pid, bool do_checkpoint);
  static bool postmaster_is_alive(pid_t pid);
  
  #if defined(HAVE_GETRLIMIT) && defined(RLIMIT_CORE)
*** free_readfile(char **optlines)
*** 419,454 
   * start/test/stop routines
   */
  
! static int
  start_postmaster(void)
  {
  	char		cmd[MAXPGPATH];
  
  #ifndef WIN32
  
  	/*
  	 * Since there might be quotes to handle here, it is easier simply to pass
! 	 * everything to a shell to process them.
! 	 *
! 	 * XXX it would be better to fork and exec so that we would know the child
! 	 * postmaster's PID directly; then test_postmaster_connection could use
! 	 * the PID without having to rely on reading it back from the pidfile.
  	 */
  	if (log_file != NULL)
! 		snprintf(cmd, MAXPGPATH, "\"%s\" %s%s < \"%s\" >> \"%s\" 2>&1 &",
   exec_path, pgdata_opt, post_opts,
   DEVNULL, log_file);
  	else
! 		snprintf(cmd, MAXPGPATH, "\"%s\" %s%s < \"%s\" 2>&1 &",
   exec_path, pgdata_opt, post_opts, DEVNULL);
  
! 	return system(cmd);
  #else			/* WIN32 */
  
  	/*
! 	 * On win32 we don't use system(). So we don't need to use & (which would
! 	 * be START /B on win32). However, we still call the shell (CMD.EXE) with
! 	 * it to handle redirection etc.
  	 */
  	PROCESS_INFORMATION pi;
  
--- 420,489 
   * start/test/stop routines
   */
  
! /*
!  * Start the postmaster and return its PID.
!  *
!  * Currently, on Windows what we return is the PID of the shell process
!  * that launched the postmaster (and, we trust, is waiting for it to exit).
!  * So the PID is usable for "is the postmaster still running" checks,
!  * but cannot be compared directly to postmaster.pid.
!  */
! static pgpid_t
  start_postmaster(void)
  {
  	char		cmd[MAXPGPATH];
  
  #ifndef WIN32
+ 	pgpid_t		pm_pid;
+ 
+ 	/* Flush stdio channels just before fork, to 

Re: [HACKERS] Parallel Seq Scan

[Robert Haas]

On Fri, Sep 25, 2015 at 12:55 AM, Amit Kapila  wrote:
> In the latest patch (parallel_seqscan_partialseqscan_v18.patch) posted by
> me yesterday, this was fixed.  Am I missing something or by any chance
> you are referring to wrong version of patch

You're right, I'm wrong.  Sorry.

> Yes, the patch needs more work in terms of dealing with parallel-restricted
> expressions/functions.  One idea which I have explored previously is
> push down only safe clauses to workers (via partialseqscan node) and
> execute restricted clauses in master (via Funnel node).  My analysis
> is as follows:
>
> Usage of restricted functions in quals-
> During create_plan() phase, separate out the quals that needs to be
> executed at funnel node versus quals that needs to be executed on
> partial seq scan node (do something similar to what is done in
> create_indexscan_plan for index and non-index quals).
>
> Basically PartialSeqScan node can contain two different list of quals,
> one for non-restrictive quals and other for restrictive quals and then
> Funnel node can retrieve restrictive quals from partialseqscan node,
> assuming partialseqscan node is its left child.
>
> Now, I think the above can only be possible under the assumption that
> partialseqscan node is always a left child of funnel node, however that is
> not true because a gating node (Result node) can be added between the
> two and tomorrow there could be more cases when other nodes will be
> added between the two, if we consider the case of aggregation, the
> situation will be more complex as before partial aggregation, all the
> quals should be executed.

What's the situation where the gating Result node sneaks in there?

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

[Robert Haas]

On Fri, Sep 25, 2015 at 7:46 AM, Amit Kapila  wrote:
> I have a question here which is why this format doesn't have a similar
> problem
> as the current version, basically in current patch the second read of
> SerializedParamExternData can be misaligned and for same reason in your
> patch the second read of Oid could by misaligned?

memcpy() can cope with unaligned data; structure member assignment can't.

I've worked some of this code over fairly heavily today and I'm pretty
happy with how my copy of execParallel.c looks now, but I've run into
one issue where I wanted to check with you. There are three places
where Instrumentation can be attached to a query: a ResultRelInfo's
ri_TrigInstrument (which doesn't matter for us because we don't
support parallel write queries, and triggers don't run on reads), a
PlanState's instrument, and a QueryDesc's total time.

Your patch makes provision to copy ONE Instrumentation structure per
worker back to the parallel leader.  I assumed this must be the
QueryDesc's totaltime, but it looks like it's actually the PlanState
of the top node passed to the worker.  That's of course no good if we
ever push more than one node down to the worker, which we may very
well want to do in the initial version, and surely want to do
eventually.  We can't just deal with the top node and forget all the
others.  Is that really what's happening here, or am I confused?

Assuming I'm not confused, I'm planning to see about fixing this...

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


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

Re: [HACKERS] No Issue Tracker - Say it Ain't So!

[Josh Berkus]

On 09/25/2015 10:27 AM, Simon Riggs wrote:
> On 25 September 2015 at 11:32, Tom Lane  1. We don't have a good process for making sure things don't "slip
> through
> the cracks".  I think everyone more or less relies on Bruce to run
> through
> his mailbox periodically and nag them about threads that don't seem to
> have been closed off.  The deficiencies of that are obvious.
> 
> 
> I don't rely on that myself. That sounds like a personal viewpoint only.
> I welcome more discussion amongst Committers with regard to
> coordination, but formal systems aren't what I think will help there.
> That situation has recently improved anyway, so no further change needed
> at present, IMHO.

??? Improved how?

> 2. There's no visibility for outsiders as to what issues are open or
> recently fixed.  Not being outsiders, I'm not sure that we are terribly
> well qualified to describe this problem precisely or identify a good
> solution --- but I grant that there's a problem there.
> 
> 
> If they can perform "git log" they can view what has happened recently.
> Tracking what might happen is much harder for active contributors.

It takes a lot of technical knowledge of PostgreSQL to relate a commit
message to a bug report, given that the bug report may not be
referenced, and the report and the commit often use completely different
terminology.  Also, users are often wanting to look for bug fixes from
months or even years ago, and git log has crappy searchability.

I can't say how many times I've had a conversation like this:

"Oh, that's a known issue.  It was fixed later in the 9.3 series."

"Really?  In which release specificially?"

"Ummm lemme search the release notes ... I know it's here somewhere ..."

> 
> I've never had a user ask me for such a list. All I here is compliments
> that our software is incredibly robust.

I have. I've had users ask for it, I've had customers ask for it, I've
had companies thinking of adopting PostgreSQL ask for it ... and for a
few of them, our lack of an issue tracker was a deciding factor in
saying that PostgreSQL "wasn't mature enough".  Certainly it was major
points off when Forrester rated us.

Also, members of downstream projects would really like us to get a bug
tracker they can kick up bugs to if they're determined to be in
Postgres.   There are bugs we're not even hearing about because it's too
confusing for someone who has a lot of projects to cover to figure out
our idiosyncratic system.

Today, having an issue tracker is considered "normal" for any
significant OSS project. The fact that we don't have one is regarded as
abberant, and not in a good way ... we're at the point where if we're
not going to adopt one, we'd better have a darned good reason which we
can explain clearly to the public.

> The only time this info is required is for people that provide a Support
> service based upon PostgreSQL, yet are not themselves sufficiently
> involved to know what bugs have been reported and are as yet unfixed. I
> expect such people are extremely interested in getting other people to
> do things that will help their business.

That has absolutely nothing to do with any reason for the PostgreSQL
project to have a bug tracker.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


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

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

[Amir Rohan]

On 09/25/2015 01:47 PM, Michael Paquier wrote:
> On Fri, Sep 25, 2015 at 5:57 PM, Amir Rohan  wrote:
>>

> That's the kind of thing that each serious developer on this mailing
> list already has in a rather different shape but with the same final
> result: 

Oh, I guess I'll have to write one then. :)


> offer a way to set up a cluster ready for hacking in a single
> command, and being able to switch among them easily. I am not sure we
> would really find a cross-platform script generic enough to satisfy
> all the needs of folks here and integrate it directly in the tree :)
> 

Yes, perl/bash seems to be the standard and both have their shorcomings,
in either convenience or familiarity... a static binary +
scripts/configuration would be least resistance,and it doesn't actually
have to live in the tree, but it needs to be good enough out of the box
that someone new will prefer to use/improve it over rolling their own
from scratch, and people need to know it's there.

Coming back to the recovery testing package...
I was investigating some behaviour having to do with recovery
and tried your new library to write a repro case. This uncovers some
implicit assumptions in the package that can make things diffficult
when violated. I had to rewrite nearly every function I used.

Major pain points:

1) Lib stores metadata (ports,paths, etc') using ports as keys.
-> Assumes ports aren't reused.
-> Because assumes server keep running until the tear down.

And

2) Behaviour (paths in particular) is hardwired rather then overridable
defaults.

This is exactly what I needed to test, problems:
3) Can't stop server without clearing its testing data (the maps holding
paths and things). But that data might be specifically
needed, in particular the backup shouldn't disappear when the
server melts down or we have a very low-grade DBA on our hands.
4) Port assignment relies on liveness checks on running servers.
If a server is shut down and a new instantiated, the port will get
reused, data will get trashed, and various confusing things can happen.
5) Servers are shutdown with -m 'immediate', which can lead to races
in the script when archiving is turned on. That may be good for some
tests, but there's no control over it.

Other issues:
6. Directory structure, used one directory per thing but more logical
to place all things related to an instance under a single directory,
and name them according to role (57333_backup, and so on).
7. enable_restoring() uses "cp -i" 'archive_command', not a good fit
for an automated test.

Aside from running the tests, the convenience of writing them
needs to be considered. My perl is very weak, it's been at least
a decade, but It was difficult to make progress because everything
is geared toward a batch "pass/fail" run . Stdout is redirected,
and the log files can't be 'tail --retry -f' in another terminal,
because they're clobbered at every run. Also:
8. No canned way to output a pprinted overview of the running system
(paths, ports, for manual checking).
9. Finding things is difficult, See 6.
10. If a test passes/fails or dies due to a bug, everything is cleaned.
Great for testing, bad for postmortem.
11. a canned "server is responding to queries" helper would be convenient.

It might be a good idea to:
1) Never reuse ports during a test. Liveness checking is used
to avoid collisions, but should not determine order of assignment.
2) Decouple cleanup from server shutdown. Do the cleanup as the end of
test only, and allow the user to keep things around.
3) Adjust the directory structure to one top directory per server with
(PGDATA, backup, archive) subdirs.
4) Instead of passing ports around as keys, have _explicit functions
which can be called directly by the user (I'd like the backup *HERE*
please), with the current functions refactored to merely invoke them
by interpolating in the values associated with the port they were given.
4b) server shutdown should perheps be "smart" by default, or segmented
into calmly_bring_to_a_close(), pull_electric_plug() and
drop_down_the_stairs_into_swimming_pool().

Regards,
Amir







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

[Robert Haas]

On Fri, Sep 25, 2015 at 12:00 AM, Amit Kapila  wrote:
> I think initPlan will work with the existing patches as we are always
> executing it in master and then sending the result to workers. Refer
> below code in funnel patch:

Sure, *if* that's what we're doing, then it will work.  But if an
initPlan actually attaches below a funnel, then it will break.  Maybe
that can't happen; I'm just sayin' ...

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

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

[Amir Rohan]

On 08/14/2015 06:32 AM, Michael Paquier wrote:
> On Fri, Aug 14, 2015 at 12:54 AM, Michael Paquier
>  wrote:
>> On Mon, Jun 29, 2015 at 10:11 PM, Michael Paquier
>>  wrote:
>>> On Wed, Mar 18, 2015 at 1:59 PM, Michael Paquier
>>>  wrote:
 Feedback is of course welcome, but note that I am not seriously
 expecting any until we get into 9.6 development cycle and I am adding
 this patch to the next CF.
>>> I have moved this patch to CF 2015-09, as I have enough patches to
>>> take care of for now... Let's focus on Windows support and improvement
>>> of logging for TAP in the first round. That will be already a good
>>> step forward.
>> OK, attached is a new version of this patch, that I have largely
>> reworked to have more user-friendly routines for the tests. The number
>> of tests is still limited still it shows what this facility can do:
>> that's on purpose as it does not make much sense to code a complete
>> and complicated set of tests as long as the core routines are not
>> stable, hence let's focus on that first.
>> I have not done yet tests on Windows, I am expecting some tricks
>> needed for the archive and recovery commands generated for the tests.
> Attached is v3. I have tested and fixed the tests such as they can run
> on Windows. archive_command and restore_command are using Windows'
> copy when needed. There was also a bug with the use of a hot standby
> instead of a warm one, causing test 002 to fail.
> I am rather happy with the shape of this patch now, so feel free to review 
> it...
> Regards,

Michael, I've ran these and it worked fine for me.
See attached patch with a couple of minor fixes.

Amir
diff --git a/src/test/recovery/RecoveryTest.pm b/src/test/recovery/RecoveryTest.pm
index c015f3b..5cc977d 100644
--- a/src/test/recovery/RecoveryTest.pm
+++ b/src/test/recovery/RecoveryTest.pm
@@ -11,7 +11,7 @@ package RecoveryTest;
 # It is then up to the test to decide what to do with the newly-created
 # node.
 #
-# Environmenment configuration of each node is available through a set
+# Environment configuration of each node is available through a set
 # of global variables provided by this package, hashed depending on the
 # port number of a node:
 # - connstr_nodes connection string to connect to this node
diff --git a/src/test/recovery/t/001_stream_rep.pl b/src/test/recovery/t/001_stream_rep.pl
index 757a639..2c59b73 100644
--- a/src/test/recovery/t/001_stream_rep.pl
+++ b/src/test/recovery/t/001_stream_rep.pl
@@ -31,9 +31,9 @@ psql $connstr_nodes{ $port_master },
 my $current_lsn = psql $connstr_nodes{ $port_master },
 	"SELECT pg_current_xlog_location();";
 my $caughtup_query = "SELECT '$current_lsn'::pg_lsn <= replay_location FROM pg_stat_replication";
-poll_query_until($caughtup_query, $connstr_nodes{ $port_master })
-	or die "Timed out while waiting for standby 1 to catch up";
 poll_query_until($caughtup_query, $connstr_nodes{ $port_standby_1 })
+	or die "Timed out while waiting for standby 1 to catch up";
+poll_query_until($caughtup_query, $connstr_nodes{ $port_standby_2 })
 	or die "Timed out while waiting for standby 2 to catch up";

 my $result = psql $connstr_nodes{ $port_standby_1 },
--
2.4.3


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

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

[Michael Paquier]

On Fri, Sep 25, 2015 at 3:11 PM, Amir Rohan  wrote:

> On 08/14/2015 06:32 AM, Michael Paquier wrote:
> > On Fri, Aug 14, 2015 at 12:54 AM, Michael Paquier
> >  wrote:
> >> On Mon, Jun 29, 2015 at 10:11 PM, Michael Paquier
> >>  wrote:
> >>> On Wed, Mar 18, 2015 at 1:59 PM, Michael Paquier
> >>>  wrote:
>  Feedback is of course welcome, but note that I am not seriously
>  expecting any until we get into 9.6 development cycle and I am adding
>  this patch to the next CF.
> >>> I have moved this patch to CF 2015-09, as I have enough patches to
> >>> take care of for now... Let's focus on Windows support and improvement
> >>> of logging for TAP in the first round. That will be already a good
> >>> step forward.
> >> OK, attached is a new version of this patch, that I have largely
> >> reworked to have more user-friendly routines for the tests. The number
> >> of tests is still limited still it shows what this facility can do:
> >> that's on purpose as it does not make much sense to code a complete
> >> and complicated set of tests as long as the core routines are not
> >> stable, hence let's focus on that first.
> >> I have not done yet tests on Windows, I am expecting some tricks
> >> needed for the archive and recovery commands generated for the tests.
> > Attached is v3. I have tested and fixed the tests such as they can run
> > on Windows. archive_command and restore_command are using Windows'
> > copy when needed. There was also a bug with the use of a hot standby
> > instead of a warm one, causing test 002 to fail.
> > I am rather happy with the shape of this patch now, so feel free to
> review it...
> > Regards,
>
> Michael, I've ran these and it worked fine for me.
> See attached patch with a couple of minor fixes.
>

Thanks! I still think that we could improve a bit more the way
parametrization is done in postgresql.conf when a node is initialized by
appending a list of parameters or have a set of hardcoded behaviors
including a set of default parameters and their values... But well feedback
is welcome regarding that. I also arrived at the conclusion that it would
be better to place the new package file in src/test/perl instead of
src/test/recovery to allow any users of the TAP tests to have it in their
PERL5LIB path and to be able to call the new routines to create and
manipulate nodes.
-- 
Michael

Re: [HACKERS] No Issue Tracker - Say it Ain't So!

[Albert Cervera i Areny]

2015-09-25 9:57 GMT+02:00 Torsten Zuehlsdorff :
>
>
> On 24.09.2015 20:23, David Fetter wrote:
>>
>> On Thu, Sep 24, 2015 at 12:10:07PM -0600, Ryan Pedela wrote:
>>>
>>> Kam Lasater wrote:

 I'd suggest: Github Issues, Pivotal Tracker or Redmine (probably in
 that order). There are tens to hundreds of other great ones out there,
 I'm sure one of them would also work.
>>>
>>>
>>> Why not just use Github issues?
>>
>>
>> Is Github issues something we can run ourselves?
>>
>> If not, it's a proprietary system which has a proprietor whose
>> existence even next month is not guaranteed, and whose interests are
>> not guaranteed to align with ours into an indefinite future.
>>
>> In some very important sense, it does not matter what features a
>> system has if it isn't one we can control.  At a minimum, this means
>> we need to run it in its entirety on resources we control, and we need
>> to be able to patch any piece of it on our own say-so.
>
>
> + 1
>
> Instead of Github maybe GitLab is a good choice. There is an open source
> community edition you have the full control over. And it is used by more
> than 100.000 organizations:
>
> https://en.wikipedia.org/wiki/GitLab
>

Of course everyone has their own preferences. Just wanted to point you
to roundup [1].

It's:

- Simple
- Python
- Very easy to extend
- Integrates well with e-mail
- Complete web interface

We use it for the Tryton [2] project and the source code we use is
available here [3] so you can take a look how easy it is to add new
fields and other stuff.

We use mercurial but commits automatically close the bug reports and
we have links with the codereview.

[1] http://roundup.sourceforge.net/docs/features.html
[2] https://bugs.tryton.org
[3] http://hg.tryton.org/bugs.tryton.org/

-- 
Albert Cervera i Areny
Tel. 93 553 18 03
@albertnan
www.NaN-tic.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] 9.3.9 and pg_multixact corruption

[Bjorn Munch]

On 25/09 09.37, Andreas Seltenreich wrote:
> [ adding Bjorn Munch to Cc ]

Oh. I am on the -hackers list but usually just scan for any subject
mentioning Solaris and this one did not. :-)

> Jim Nasby writes:
> > On 9/20/15 9:23 AM, Christoph Berg wrote:
> >> a short update here: the customer updated the compiler to a newer
> >> version, is now compiling using -O2 instead of -O3, and the code
> >> generated now looks sane, so this turned out to be a compiler issue.
> >> (Though it's unclear if the upgrade fixed it, or the different -O
> >> level.)
> >
> > Do we officially not support anything > -O2? If so it'd be nice if
> > configure threw at least a warning (if not an error that you had to
> > explicitly over-ride).
> 
> At least the solaris binaries distributed via postgresql.org[1] have
> been compiled with -xO3 according to pg_config.  And their code for
> multixact.c looks inconspicuous.  To recap the data points:
> 
> | compiler  | flags | multixact.o |
> |---+---+-|
> | Sun C 5.12 SunOS_sparc Patch 148917-07 2013/10/18 | -xO3  | bad |
> | Sun C 5.13 SunOS_Sparc 2014/10/20 | -xO2  | good|
> | Sun C 5.8 Patch 121015-04 2007/01/10  | -xO3  | good|

All the binaries I have compiled for distribution via postgresql.org
have been built with Sun Studio 11, aka Sun C 5.8 as listed on the
bottom here. Except the 9.5 Alpha where I finally had to upgrade to
version 12.1 (aka Sun C 5.10).

I am also pretty sure they have always been compiled with -O3. At
least that's what the build script is set to and I don't think that
has even been changed.

- Bjorn Munch


-- 
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] No Issue Tracker - Say it Ain't So!

[Torsten Zuehlsdorff]

On 25.09.2015 10:04, Albert Cervera i Areny wrote:

2015-09-25 9:57 GMT+02:00 Torsten Zuehlsdorff :

On 24.09.2015 20:23, David Fetter wrote:


On Thu, Sep 24, 2015 at 12:10:07PM -0600, Ryan Pedela wrote:


Kam Lasater wrote:


I'd suggest: Github Issues, Pivotal Tracker or Redmine (probably in
that order). There are tens to hundreds of other great ones out there,
I'm sure one of them would also work.



Why not just use Github issues?



Is Github issues something we can run ourselves?

If not, it's a proprietary system which has a proprietor whose
existence even next month is not guaranteed, and whose interests are
not guaranteed to align with ours into an indefinite future.

In some very important sense, it does not matter what features a
system has if it isn't one we can control.  At a minimum, this means
we need to run it in its entirety on resources we control, and we need
to be able to patch any piece of it on our own say-so.


+ 1

Instead of Github maybe GitLab is a good choice. There is an open source
community edition you have the full control over. And it is used by more
than 100.000 organizations:

https://en.wikipedia.org/wiki/GitLab


Of course everyone has their own preferences. Just wanted to point you
to roundup [1].


You're right. Though GitLab is not my own preference, but i'm currently 
porting it to FreeBSD. ;) But it is a good choice if you like Github.


There is a greater list of tools to for source code hosting:
https://en.wikipedia.org/wiki/Comparison_of_source_code_hosting_facilities

Greetings,
Torsten


--
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] CustomScan support on readfuncs.c

[Kouhei Kaigai]

> On Fri, Sep 25, 2015 at 6:49 AM, Kouhei Kaigai  wrote:
> 
> 
>   Hi,
> 
>   I tried to define two additional callbacks to support CustomScan
>   on readfuncs.c.
> 
>   First of all, we need to pay attention how to treat output of
>   TextOutCustomScan when additional text output is generated.
>   Only custom-scan provider knows what shall be displayed, so
>   all we can do is invoke a new callback: TextReadCustomScan().
>   Because private fields shall be displayed next to the common
>   field of CustomScan, this callback is invoked once pg_strtok
>   pointer reached to the last field of CustomScan. Then, custom
>   scan provider allocates CustomScan or a structure embedding
>   CustomScan; only CSP knows exact size to be allocated.
>   It can fetch some tokens using pg_strtok and reconstruct its
>   private fields, but no need to restore the common field because
>   it shall be done by _readCustomScan.
> 
> 
> 
> So will the specification of TextReadCustomScan() and
> TextOutCustomScan() ensures that they don't access the common
> fields (like custom_private or others) of CustomScan?
> If they change/overwrite them in some way, then I think _readCustomScan()
> won't work because it doesn't take into account that common fields could
> have been updated by TextReadCustomScan().
>
Yes. Even if callback of TextReadCustomScan() wants to change or
overwrite, then it is eventually overwritten by the core.
If we allow custom-scan provide to adjust common fields of CustomScan,
it is a change of interface role because the relevant callbacks (TextOut,
TextRead and NodeCopy) are expected to perform faithfully according to
the supplied node.
If extension really wants to adjust plan-node, probably, something like
plan_tree_mutator() should be the place to do.

Thanks,
--
NEC Business Creation Division / PG-Strom Project
KaiGai Kohei 

-- 
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] 9.3.9 and pg_multixact corruption

[Andreas Seltenreich]

[ adding Bjorn Munch to Cc ]

Jim Nasby writes:
> On 9/20/15 9:23 AM, Christoph Berg wrote:
>> a short update here: the customer updated the compiler to a newer
>> version, is now compiling using -O2 instead of -O3, and the code
>> generated now looks sane, so this turned out to be a compiler issue.
>> (Though it's unclear if the upgrade fixed it, or the different -O
>> level.)
>
> Do we officially not support anything > -O2? If so it'd be nice if
> configure threw at least a warning (if not an error that you had to
> explicitly over-ride).

At least the solaris binaries distributed via postgresql.org[1] have
been compiled with -xO3 according to pg_config.  And their code for
multixact.c looks inconspicuous.  To recap the data points:

| compiler  | flags | multixact.o |
|---+---+-|
| Sun C 5.12 SunOS_sparc Patch 148917-07 2013/10/18 | -xO3  | bad |
| Sun C 5.13 SunOS_Sparc 2014/10/20 | -xO2  | good|
| Sun C 5.8 Patch 121015-04 2007/01/10  | -xO3  | good|

regards,
Andreas

Footnotes: 
[1]  http://www.postgresql.org/download/solaris/


-- 
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] No Issue Tracker - Say it Ain't So!

[Torsten Zuehlsdorff]

On 24.09.2015 20:23, David Fetter wrote:

On Thu, Sep 24, 2015 at 12:10:07PM -0600, Ryan Pedela wrote:

Kam Lasater wrote:

I'd suggest: Github Issues, Pivotal Tracker or Redmine (probably in
that order). There are tens to hundreds of other great ones out there,
I'm sure one of them would also work.


Why not just use Github issues?


Is Github issues something we can run ourselves?

If not, it's a proprietary system which has a proprietor whose
existence even next month is not guaranteed, and whose interests are
not guaranteed to align with ours into an indefinite future.

In some very important sense, it does not matter what features a
system has if it isn't one we can control.  At a minimum, this means
we need to run it in its entirety on resources we control, and we need
to be able to patch any piece of it on our own say-so.


+ 1

Instead of Github maybe GitLab is a good choice. There is an open source 
community edition you have the full control over. And it is used by more 
than 100.000 organizations:


https://en.wikipedia.org/wiki/GitLab

Greetings,
Torsten


--
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] ON CONFLICT issues around whole row vars,

[Peter Geoghegan]

On Thu, Sep 24, 2015 at 8:25 AM, Andres Freund  wrote:
> Stuff I want to fix by tomorrow:
> * Whole row var references to exclude
> * wrong offsets for columns after dropped ones
> * INSTEAD DO UPDATE for tables with oids
>
> Do you know of anything else?

You said something in Dallas about the test case developed by Amit
Langote touching on a different bug to the regression test I came up
with. If that is the case, then you didn't list that one separately.
Otherwise, no.

-- 
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] 9.3.9 and pg_multixact corruption

[Andreas Seltenreich]

Alvaro Herrera writes:

> Jim Nasby wrote:
>> Do we officially not support anything > -O2? If so it'd be nice if configure
>> threw at least a warning (if not an error that you had to explicitly
>> over-ride).
>
> Keep in mind this is Sun OS C -- not one of the most popular compilers
> in the world.  I don't know what you suggest: have a test program that
> configure runs and detects whether the compiler does the wrong thing?
> It doesn't seem a sane idea to maintain test cases for all known
> compiler bugs ...

I think the intention was to make configure complain if there's a -O > 2
in CFLAGS.

OTOH, a unit test for multixact.c that exercises the code including
wraparounds sounds like a desirable thing regardless of the fact that it
could have caught this miscompilation earlier than 6 months into
production.

regards,
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] No Issue Tracker - Say it Ain't So!

[Magnus Hagander]

On Fri, Sep 25, 2015 at 12:40 AM, Tom Lane  wrote:

> Josh Berkus  writes:
> > On 09/24/2015 12:55 PM, Tom Lane wrote:
> >> I agree with the idea that we don't yet want to give the impression that
> >> this is the official bug tracker.  However, "beta-bugs" could give the
> >> impression that it was specifically for bugs about 9.5beta, without
> >> dispelling the idea that it is official.  Maybe "bugs-test.p.o"?
>
> > I'd suggest instead just having a big banner up in the page header which
> > says "this system is currently beta and not yet the canonical source for
> > postgres bug information".  That way, if it does become the canonical
> > source, we won't go breaking everyone's links when we change the domain
> > name.
>
> Works for me, if it's not too hard to do.
>

If that's hard to do, then we're using the wrong system

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

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

[Amir Rohan]

On 09/25/2015 09:29 AM, Michael Paquier wrote:
> 
> 
> On Fri, Sep 25, 2015 at 3:11 PM, Amir Rohan  > wrote:
> 
> On 08/14/2015 06:32 AM, Michael Paquier wrote:

> > I am rather happy with the shape of this patch now, so feel free
> to review it...
> > Regards,
> 
> Michael, I've ran these and it worked fine for me.
> See attached patch with a couple of minor fixes.
> 
> 
> Thanks! I still think that we could improve a bit more the way
> parametrization is done in postgresql.conf when a node is initialized by
> appending a list of parameters or have a set of hardcoded behaviors
> including a set of default parameters and their values... But well
> feedback is welcome regarding that. I also arrived at the conclusion
> that it would be better to place the new package file in src/test/perl
> instead of src/test/recovery to allow any users of the TAP tests to have
> it in their PERL5LIB path and to be able to call the new routines to
> create and manipulate nodes.
> -- 
> Michael 

Having a subcommand in Greg's PEG (http://github.com/gregs1104/peg),
that allows you to create one of several "canned" clusters would be
convenient as well, for manual testing and folling around with features.

Amir


-- 
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 can't we used CAPITAL LETTERS into replication slot_name?

[Andres Freund]

Hi,

On 2015-09-25 17:32:39 +0530, Rushabh Lathia wrote:
> I am just wondering why pg_create_physical_replication_slot() can't take
> CAPITAL LETTERS into slot_name ?

Windows. And OSX. Specifically case-insensitive filenames.

> If its by design then was should atleast change the hint into
> ReplicationSlotValidateName() which says: "Replication slot names may only
> contain letters, numbers, and the underscore character."

We could add a 'lower case' in there.

Greetings,

Andres Freund


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

[Amit Kapila]

On Thu, Sep 24, 2015 at 8:03 AM, Robert Haas  wrote:
>
> On Thu, Sep 3, 2015 at 6:21 AM, Amit Kapila 
wrote:
> > [ new patches ]
>
> Still more review comments:
>
> +   /* Allow space for terminating zero-byte
*/
> +   size = add_size(size, 1);
>
> This is pointless.  The length is already stored separately, and if it
> weren't, this wouldn't be adequate anyway because a varlena can
> contain NUL bytes.  It won't if it's text, but it could be bytea or
> numeric or whatever.
>

Agreed and I think we can do without that as well.

> RestoreBoundParams is broken, because it can do unaligned reads, which
> will core dump on some architectures (and merely be slow on others).
> If there are two or more parameters, and the first one is a varlena
> with a length that is not a multiple of MAXIMUM_ALIGNOF, the second
> SerializedParamExternData will be misaligned.
>

Agreed, but can't we fix that problem even in the current patch by using
*ALIGN (TYPEALIGN) macro?

> Also, it's pretty lame that we send the useless pointer even for a
> pass-by-reference data type and then overwrite the bad pointer with a
> good one a few lines later.  It would be better to design the
> serialization format so that we don't send the bogus pointer over the
> wire in the first place.
>
> It's also problematic in my view that there is so much duplicated code
> here. SerializedParamExternData and SerializedParamExecData are very
> similar and there are large swaths of very similar code to handle each
> case.  Both structures contain Datum value, Size length, bool isnull,
> and Oid ptype, albeit not in the same order for some reason.  The only
> difference is that SerializedParamExternData contains uint16 pflags
> and SerializedParamExecData contains int paramid.  I think we need to
> refactor this code to get rid of all this duplication.

The reason of having different structures is that both the structures are
derived from their non-serialized versions (ParamExternData and
ParamExecData).  I agree that we can avoid code duplication incase of
serialization and restoration of both structures, however doing it with
structure compatible to non-serialized version seems to have better code
readability and easier to enhance (think if we need to add a new parameter
in non-serialized version).  This can somewhat vary from each individual's
perspective and I won't argue much here if you say that having native format
as you are suggesting is better in terms of code readability and
maintenance.


>  I suggest that
> we decide to represent a datum here in a uniform fashion, perhaps like
> this:
>
> First, store a 4-byte header word.  If this is -2, the value is NULL
> and no data follows.  If it's -1, the value is pass-by-value and
> sizeof(Datum) bytes follow.  If it's >0, the value is
> pass-by-reference and the value gives the number of following bytes
> that should be copied into a brand-new palloc'd chunk.
>
> Using a format like this, we can serialize and restore datums in
> various contexts - including bind and exec params - without having to
> rewrite the code each time.  For example, for param extern data, you
> can dump an array of all the ptypes and paramids and then follow it
> with all of the params one after another.  For param exec data, you
> can dump an array of all the ptypes and paramids and then follow it
> with the values one after another.  The code that reads and writes the
> datums in both cases can be the same.  If we need to send datums in
> other contexts, we can use the same code for it.
>
> The attached patch - which I even tested! - shows what I have in mind.
> It can save and restore the ParamListInfo (bind parameters).  I
> haven't tried to adapt it to the exec parameters because I don't quite
> understand what you are doing there yet, but you can see that the
> datum-serialization logic is separated from the stuff that knows about
> the details of ParamListInfo, so datumSerialize() should be reusable
> for other purposes.

I think we can adapt this for Param exec data parameters.  I can take
care of integrating this with funnel patch and changing the param exec data
parameters related code if we decide to go via this way.

>  This also doesn't have the other problems
> mentioned above.
>

I have a question here which is why this format doesn't have a similar
problem
as the current version, basically in current patch the second read of
SerializedParamExternData can be misaligned and for same reason in your
patch the second read of Oid could by misaligned?


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

Re: [HACKERS] pageinspect patch, for showing tuple data

[Michael Paquier]

On Fri, Sep 25, 2015 at 8:30 PM, Nikolay Shaplov wrote:
> Here is final version with documentation.

Thanks! I just had a short look at it:
- I am not convinced that it is worth declaring 3 versions of tuple_data_split.
- The patch does not respect the project code style, particularly
one-line "if condition {foo;}" are not adapted, code line is limited
to 80 characters, etc. The list is basically here:
http://www.postgresql.org/docs/current/static/source.html
- Be aware of typos: s/whitch/which is one.

+t_infomask2
+integer
+stores number of attributes (11 bits of the word). The
rest are used for flag bits:
+
+0x2000 - tuple was updated and key cols modified, or tuple deleted
+0x4000 - tuple was HOT-updated
+0x8000 - this is heap-only tuple
+0xE000 - visibility-related bits (so called "hint bits")
+
This large chunk of documentation is a duplicate of storage.sgml. If
that's really necessary, it looks adapted to me to have more detailed
comments at code level directly in heapfuncs.c.

The example of tuple_data_split in the docs would be more interesting
if embedded with a call to heap_page_items.

> Hope it will be the last one. :-)

Unfortunately not :) But this is definitely going in the right
direction thinking that this code is mainly targeted for educational
purposes.
-- 
Michael


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

[HACKERS] Why can't we used CAPITAL LETTERS into replication slot_name?

[Rushabh Lathia]

Hi,

I am just wondering why pg_create_physical_replication_slot() can't take
CAPITAL LETTERS into slot_name ?

Comment over ReplicationSlotValidateName() says that - Slot names may
consist out of [a-z0-9_]{1,NAMEDATALEN-1} which should allow the name to be
used as a directory name on every supported OS.

If its by design then was should atleast change the hint into
ReplicationSlotValidateName() which says: "Replication slot names may only
contain letters, numbers, and the underscore character."

Comments ?


Regards,
Rushabh Lathia
www.EnterpriseDB.com

Re: [HACKERS] Why can't we used CAPITAL LETTERS into replication slot_name?

[Rushabh Lathia]

Thanks Andres.

PFA patch to fix the hint message.

On Fri, Sep 25, 2015 at 5:34 PM, Andres Freund  wrote:

> Hi,
>
> On 2015-09-25 17:32:39 +0530, Rushabh Lathia wrote:
> > I am just wondering why pg_create_physical_replication_slot() can't take
> > CAPITAL LETTERS into slot_name ?
>
> Windows. And OSX. Specifically case-insensitive filenames.
>
> > If its by design then was should atleast change the hint into
> > ReplicationSlotValidateName() which says: "Replication slot names may
> only
> > contain letters, numbers, and the underscore character."
>
> We could add a 'lower case' in there.
>
> Greetings,
>
> Andres Freund
>



Regards,
Rushabh Lathia
www.EnterpriseDB.com


slot_name_hint_message_change.patch
Description: application/download

-- 
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] DBT-3 with SF=20 got failed

[Tomas Vondra]

On 09/25/2015 02:54 AM, Robert Haas wrote:

On Thu, Sep 24, 2015 at 1:58 PM, Tomas Vondra
 wrote:

Meh, you're right - I got the math wrong. It's 1.3% in both cases.

However the question still stands - why should we handle the
over-estimate in one case and not the other? We're wasting the
samefraction of memory in both cases.


Well, I think we're going around in circles here. It doesn't seem
likely that either of us will convince the other.


Let's agree we disagree ;-) That's perfectly OK, no hard feelings.


But for the record, I agree with you that in the scenario you lay
out, it's the about the same problem in both cases. I could argue
that it's slightly different because of [ tedious and somewhat
tenuous argument omitted ], but I'll spare you that.


OK, although that makes kinda prevents further discussion.


However, consider the alternative scenario where, on the same
machine, perhaps even in the same query, we perform two hash joins,
one of which involves hashing a small table (say, 2MB) and one of
which involves hashing a big table (say, 2GB). If the small query
uses twice the intended amount of memory, probably nothing bad will
happen. If the big query does the same thing, a bad outcome is much
more likely. Say the machine has 16GB of RAM. Well, a 2MB
over-allocation is not going to break the world. A 2GB
over-allocation very well might.


I've asked about case A. You've presented case B and shown that indeed, 
the limit seems to help here. I don't see how that makes any difference 
in case A, which I asked about?



I really don't see why this is a controversial proposition. It seems
clearly as daylight from here.


I wouldn't say controversial, but I do see the proposed solution as 
misguided because we're fixing A and claiming to also fix B. Not only 
we're not really fixing B, but may actually make it needlessly slower 
for people who don't have problems with B at all.


We've ran into problem with allocating more than MaxAllocSize. The 
proposed fix (imposing arbitrary limit) is also supposedly fixing 
over-estimation problems, but actually it not (IMNSHO).


And I think my view is supported by the fact that solutions that seem to 
actually fix the over-estimation properly emerged. I mean the "let's not 
build the buckets at all, until the very end" and "let's start with 
nbatches=0" discussed yesterday. (And I'm not saying that because I 
proposed those two things.)


Anyway, I think you're right we're going in circles here. I think we 
both presented all the arguments we had and we still disagree. I'm not 
going to continue with this - I'm unlikely to win an argument against 
two committers if that didn't happen until now. Thanks for the 
discussion though.



regards

--
Tomas Vondra  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] CustomScan support on readfuncs.c

[Amit Kapila]

On Fri, Sep 25, 2015 at 6:49 AM, Kouhei Kaigai  wrote:

> Hi,
>
> I tried to define two additional callbacks to support CustomScan
> on readfuncs.c.
>
> First of all, we need to pay attention how to treat output of
> TextOutCustomScan when additional text output is generated.
> Only custom-scan provider knows what shall be displayed, so
> all we can do is invoke a new callback: TextReadCustomScan().
> Because private fields shall be displayed next to the common
> field of CustomScan, this callback is invoked once pg_strtok
> pointer reached to the last field of CustomScan. Then, custom
> scan provider allocates CustomScan or a structure embedding
> CustomScan; only CSP knows exact size to be allocated.
> It can fetch some tokens using pg_strtok and reconstruct its
> private fields, but no need to restore the common field because
> it shall be done by _readCustomScan.
>

So will the specification of TextReadCustomScan() and
TextOutCustomScan() ensures that they don't access the common
fields (like custom_private or others) of CustomScan?
If they change/overwrite them in some way, then I think _readCustomScan()
won't work because it doesn't take into account that common fields could
have been updated by TextReadCustomScan().



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

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

[Michael Paquier]

On Fri, Sep 25, 2015 at 5:57 PM, Amir Rohan  wrote:
>
> Having a subcommand in Greg's PEG (http://github.com/gregs1104/peg),
> that allows you to create one of several "canned" clusters would be
> convenient as well, for manual testing and folling around with features.


That's the kind of thing that each serious developer on this mailing
list already has in a rather different shape but with the same final
result: offer a way to set up a cluster ready for hacking in a single
command, and being able to switch among them easily. I am not sure we
would really find a cross-platform script generic enough to satisfy
all the needs of folks here and integrate it directly in the tree :)
-- 
Michael


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

[HACKERS] cluster_name and update_process_title documentation

[Peter Eisentraut]

The related settings cluster_name and update_process_title have somehow
ended up at opposite corners of the documentation and sample files.  I
propose to group them together in a new "Process Title" section, as in
the attached patch.
From 159e0db4d7da647ca6d9153250c60a744517ab3e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Fri, 25 Sep 2015 09:14:50 -0400
Subject: [PATCH] Group cluster_name and update_process_title settings together

---
 doc/src/sgml/config.sgml  | 97 +++
 src/backend/utils/misc/guc.c  |  4 +-
 src/backend/utils/misc/postgresql.conf.sample |  6 +-
 src/include/utils/guc_tables.h|  1 +
 4 files changed, 62 insertions(+), 46 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index c8ec219..593b8fd 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -4345,30 +4345,6 @@ What To Log
   
  
 
- 
-  cluster_name (string)
-  
-   cluster_name configuration parameter
-  
-  
-  
-   
-Sets the cluster name that appears in the process title for all
-processes in this cluster. The name can be any string of less than
-NAMEDATALEN characters (64 characters in a standard
-build). Only printable ASCII characters may be used in the
-cluster_name value. Other characters will be
-replaced with question marks (?).  No name is shown
-if this parameter is set to the empty string '' (which is
-the default). This parameter can only be set at server start.
-   
-   
-The process title is typically viewed using programs like
-ps or, on Windows, Process Explorer.
-   
-  
- 
-
  
   debug_print_parse (boolean)
   
@@ -4956,9 +4932,61 @@ Using CSV-Format Log Output
 
 
   
-
 
-   
+
+   
+Process Title
+
+
+ These settings control how the process title as seen
+ by ps is modified.  See 
+ for details.
+
+
+
+ 
+  cluster_name (string)
+  
+   cluster_name configuration parameter
+  
+  
+  
+   
+Sets the cluster name that appears in the process title for all
+processes in this cluster. The name can be any string of less than
+NAMEDATALEN characters (64 characters in a standard
+build). Only printable ASCII characters may be used in the
+cluster_name value. Other characters will be
+replaced with question marks (?).  No name is shown
+if this parameter is set to the empty string '' (which is
+the default). This parameter can only be set at server start.
+   
+   
+The process title is typically viewed using programs like
+ps or, on Windows, Process Explorer.
+   
+  
+ 
+
+ 
+  update_process_title (boolean)
+  
+   update_process_title configuration parameter
+  
+  
+  
+   
+Enables updating of the process title every time a new SQL command
+is received by the server.  The process title is typically viewed
+by the ps command,
+or in Windows by using the Process Explorer.
+Only superusers can change this setting.
+   
+  
+ 
+
+   
+  
 

 Run-time Statistics
@@ -5076,23 +5104,6 @@ Query and Index Statistics Collector
   
  
 
- 
-  update_process_title (boolean)
-  
-   update_process_title configuration parameter
-  
-  
-  
-   
-Enables updating of the process title every time a new SQL command
-is received by the server.  The process title is typically viewed
-by the ps command,
-or in Windows by using the Process Explorer.
-Only superusers can change this setting.
-   
-  
- 
-
  
   stats_temp_directory (string)
   
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 3d0eb2d..b0554fb 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1181,7 +1181,7 @@ static struct config_bool ConfigureNamesBool[] =
 	},
 
 	{
-		{"update_process_title", PGC_SUSET, STATS_COLLECTOR,
+		{"update_process_title", PGC_SUSET, PROCESS_TITLE,
 			gettext_noop("Updates the process title to show the active SQL command."),
 			gettext_noop("Enables updating of the process title every time a new SQL command is received by the server.")
 		},
@@ -3366,7 +3366,7 @@ static struct config_string ConfigureNamesString[] =
 	},
 
 	{
-		{"cluster_name", PGC_POSTMASTER, LOGGING_WHAT,
+		{"cluster_name", PGC_POSTMASTER, PROCESS_TITLE,
 			gettext_noop("Sets the name of the cluster which is included in the process title."),
 			NULL,
 			GUC_IS_NAME
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 412e5c2..18c433b 100644
--- 

Re: [HACKERS] PATCH: use foreign keys to improve join estimates v1

[Tomas Vondra]

Hi,

On 09/25/2015 03:39 AM, David Rowley wrote:

On 24 September 2015 at 23:57, Tomas Vondra
> wrote:


2) find_best_match_foreign_key
--

I think the comment before the function needs rephrasing (seems a
bit broken to me). I do like the approach in general, although it
changes the semantics a bit. The original code only considered
"fully matching" fkeys, while the new code simply takes the longest
match.



Oops, I did not code this at all the way I had originally pictured it. I
guess the evidence of that is in the function comment, which I wrote
before coding the function.
I of course intended to only allow full matches.
A full patch which fixes this is attached. This also should fix the
clause duplication trick that I talked about.


OK, thanks. Let's leave partial FK matches for the future.




The comment before the function mentions it's possible to confuse
the code with duplicate quals. Can you give an example? >


Something like: SELECT * FROM a LEFT JOIN b ON a.id a.id=b.id b.id
and a.id a.id=b.id b.id AND a.id a.id=b.id b.id AND a.id2 = b.id2 AND
a.id3 = b.id3;

Note a.id a.id = b.id b.id repeated 3 times.

Where a foreign key exists on (a.id a.id) ref (b.id b.id), and also
(a.id2, a.id3) ref (b.id2, b.id3). In this case we match 3 quals for
the FK with 1 key, and 2 quals with the FK with 2 keys. But this is
now fixed in the attached.

I used an outer join here as they won't be transformed into eclass
members and back to quals again. INNER JOIN wouldn't allow the
duplicates to materialise again.


Ah, OK. Didn't think about outer joins.



...

I looked at this again, and I'm not all that sure it's possible to
assume that 1.0 /  is valid when there's more than one
relation at either side of the join.

>

My reasoning for this is that the whole basis for the patch is that a
if we find a foreign key match, then we can be sure enough, as far as
row estimations go, that exactly 1 tuple will exist matching that
condition. This assumption of the 1 tuple no longer holds when 2
relations have already been joined, as this can either cause tuple
duplication, or elimination.


I don't see why that would be the case. Of course, you need to take the 
right , i.e. the "target" of the foreign key (the table with 
UNIQUE constraint) so that the selectivity matches the fact that exactly 
1 tuple (on the PK side) matches.


Let's say we have 3 tables

A (100 rows)
B (33 rows)
D (22 rows)

and let's assume A references the other two tables

   A (b_id) REFERENCES B(id)
   A (c_id) REFERENCES C(id)

Now, let's estimate the join

  SELECT * FROM A JOIN B ON (a.b_id = b.id)
  JOIN C ON (a.c_id = c.id)

Which will do this

  card(join) = card(A) * card(B) * card(C) * sel(b_id=id) * sel(c_id=id)

where card(T) is a cardinality of a table, and sel(C) is selectivity of 
the conditions. We do know that


  card(A) = 1000
  card(B) = 333
  card(C) = 222

and we do know that selectivity of each condition is 1/card(T) of the 
table with the UNIQUE constraint, referenced by the FK. So


  sel(b_id=id) = 1/card(B) = 1/333
  sel(c_id=id) = 1/card(C) = 1/222

The fact is that these selectivities perfectly eliminate the impact of 
cardinality of the table. So


  card(join) = 1000 * (333 * (1/333)) * (222 * (1/222))

so the expected cardinality is 1000.

Of course, this estimation effectively happens in steps, i.e. we first 
join 2 tables - say A and B, then (A,B) and C. So in the first step we 
do this:


  card(A,B) = card(A) * card(B) * sel(b_id=id) = 1000

  card((A,B),C) = card(A,B) * card(C) * sel(a_id=id) = 1000

The join order does not matter - we could easily join B,C first, and 
then join A.


  card(B,C) = card(B) * card(C) * sel(NULL) = 73926

and then

  card((B,C),A) = card(B,C) * card(A) * sel(a_id=id) * sel(b_id=id)
= 1000

Notice that in this case, the singleton table (A) actually references 
primary keys within the join relation - obviously the whole join 
relation does not have unique keys or anything, but that does not matter.


The crucial part here is that selectivity of the condition needs to use 
the number of tuples of the base relation, because that's the right 
selectivity for the join clause.




It's a little hard force the join order so that it happens this way, but
say the join order in the following example was, from left to right:

a CROSS JOIN b JOIN c on a.id a.id=c.id

Of course, the planner would perform the cross join last in reality, but
it's possible to trick it too not.

Let's say a foreign key exists on c (id) references a(id). If we
assume that a.id a.id = c.id  produces 1 tuple in the (a,b)
rel, thenwe'd be very wrong, as it's not 1, it's the number of
rows in b! Which could be millions.


I think this is the part where you're wrong. What needs to happen in the 
estimation is this:


  card(A,B) = card(A) * card(B)  /* 

Re: [HACKERS] [COMMITTERS] pgsql: Fix an O(N^2) problem in foreign key references.

[Jan Wieck]

On 09/18/2015 10:47 AM, Tom Lane wrote:

Jan Wieck  writes:

Attached is a complete rework of the fix from scratch, based on Tom's
suggestion.



The code now maintains a double linked list as suggested, but only uses
it to mark all currently valid entries as invalid when hashvalue == 0.
If a specific entry is invalidated it performs a hash lookup for maximum
efficiency in that case.


That does not work; you're ignoring the possibility of hashvalue
collisions, and for that matter not considering that the hash value
is not equal to the hash key.  I don't think your code is invalidating
the right entry at all during a foreign key constraint update, and
it certainly cannot do the right thing if there's a hash value collision.

Attached is something closer to what I was envisioning; can you do
performance testing on it?


Sorry for the delay.

Yes, that patch also has the desired performance for restoring a schema 
with hundreds of thousands of foreign key constraints.



Regards, Jan

--
Jan Wieck
Senior Software Engineer
http://slony.info


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