date:20160126

2016-01-26 11:42 GMT+01:00 Marko Tiikkaja :

> On 25/01/16 19:57, Pavel Stehule wrote:
>
>> Marco is a author of this patch, so - Marco, please, send final version of
>> this patch
>>
>
> I don't really care about the tests.  Can we not use the v5 patch already
> in the thread?  As far as I could tell there were no reviewer's comments on
> it anymore.
>

It was not my request, but I counted Jim as second reviewer.

So, I'll return back original regress tests. If I remember well, there are
no any other objection, so I'll mark this version as ready for commiter.

1. the patch is rebased against master
2. now warning or errors due compilation
3. all tests are passed
4. the code is simple without side effects and possible negative
performance impacts
6. there was not objections against the design
7. the iteration over null bitmap is used more times in our code, but this
is new special case. We don't need iterate over array elements, so we
should not to use existing array iterators.

I'll mark this patch as ready for commiter

Regards

Pavel


>
> .m
>
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
new file mode 100644
index 9c143b2..23c933f
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***
*** 182,188 

  

!Comparison Operators
  
 
  comparison
--- 182,188 

  

!Comparison Functions and Operators
  
 
  comparison
***
*** 191,200 
  
 
  The usual comparison operators are available, shown in .
 
  
!
  Comparison Operators
  
   
--- 191,200 
  
 
  The usual comparison operators are available, shown in .
 
  
!
  Comparison Operators
  
   
***
*** 437,442 
--- 437,479 
 
  -->
  
+   
+ Comparison Functions
+ 
+  
+   
+Function
+Description
+Example
+Example Result
+   
+  
+  
+   
+
+  
+   num_notnulls
+  
+  num_notnulls(VARIADIC "any")
+
+Returns the number of not NULL input arguments
+num_nulls(1, NULL, 2)
+2
+   
+   
+
+  
+   num_nulls
+  
+  num_nulls(VARIADIC "any")
+
+Returns the number of NULL input arguments
+num_nulls(1, NULL, 2)
+1
+   
+  
+ 
+

  

*** table2-mapping
*** 10389,10395 


 The standard comparison operators shown in   are available for
 jsonb, but not for json. They follow the
 ordering rules for B-tree operations outlined at .
--- 10426,10432 


 The standard comparison operators shown in   are available for
 jsonb, but not for json. They follow the
 ordering rules for B-tree operations outlined at .
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
new file mode 100644
index 6a306f3..35810d1
*** a/src/backend/utils/adt/misc.c
--- b/src/backend/utils/adt/misc.c
***
*** 43,48 
--- 43,160 
  
  #define atooid(x)  ((Oid) strtoul((x), NULL, 10))
  
+ /*
+  * Collect info about NULL arguments. Returns true when result values
+  * are valid.
+  */
+ static bool
+ count_nulls(FunctionCallInfo fcinfo,
+ int32 *nargs, int32 *nulls)
+ {
+ 	int32 count = 0;
+ 	int i;
+ 
+ 	if (get_fn_expr_variadic(fcinfo->flinfo))
+ 	{
+ 		ArrayType  *arr;
+ 		int ndims, nitems, *dims;
+ 		bits8 *bitmap;
+ 		int bitmask;
+ 
+ 		/*
+ 		 * When parameter with packed variadic arguments is NULL, we
+ 		 * cannot to identify number of variadic argumens (NULL
+ 		 * or not NULL), then the correct result is NULL. This behave
+ 		 * is consistent with other variadic functions - see concat_internal.
+ 		 */
+ 		if (PG_ARGISNULL(0))
+ 			return false;
+ 
+ 		/*
+ 		 * Non-null argument had better be an array.  We assume that any call
+ 		 * context that could let get_fn_expr_variadic return true will have
+ 		 * checked that a VARIADIC-labeled parameter actually is an array.  So
+ 		 * it should be okay to just Assert that it's an array rather than
+ 		 * doing a full-fledged error check.
+ 		 */
+ 		Assert(OidIsValid(get_base_element_type(get_fn_expr_argtype(fcinfo->flinfo, 0;
+ 
+ 		/* OK, safe to fetch the array value */
+ 		arr = PG_GETARG_ARRAYTYPE_P(0);
+ 
+ 		ndims = ARR_NDIM(arr);
+ 		dims = ARR_DIMS(arr);
+ 		nitems = ArrayGetNItems(ndims, dims);
+ 
+ 		bitmap = ARR_NULLBITMAP(arr);
+ 		if (bitmap)
+ 		{
+ 			bitmask = 1;
+ 
+ 			for (i = 0; i < nitems; i++)
+ 			{
+ if ((*bitmap & bitmask) == 0)
+ 	count++;
+ 
+ bitmask <<= 1;
+ if (bitmask == 0x100)
+ {
+ 	bitmap++;
+ 	bitmask = 1;
+ }
+ 			}
+ 		}
+ 
+ 		*nargs = nitems;
+ 		*nulls = count;
+ 	}
+ 	else
+ 	{
+ 		for (i = 0; i < PG_NARGS(); i++)
+ 		{
+ 			if (PG_ARGISNULL(i))
+ count++;
+ 		}
+ 
+ 		*nargs = PG_NARGS();
+ 		*nulls = count;
+ 	}
+ 
+ 	return true;
+ }

Re: [HACKERS] [PoC] Asynchronous execution again (which is not parallel)

2016-01-26 Thread Kyotaro HORIGUCHI

Hi.

At Thu, 21 Jan 2016 19:09:19 +0900, Amit Langote 
 wrote in <56a0ae4f.9000...@lab.ntt.co.jp>
> 
> Hi!
> 
> On 2016/01/21 18:26, Kyotaro HORIGUCHI wrote:
> >>> Then, suppose we add a function bool ExecStartAsync(PlanState *target,
> >>> ExecCallback callback, PlanState *cb_planstate, void *cb_context).
> >>> For non-async-aware plan nodes, this just returns false.  async-aware
> >>> plan nodes should initiate some work, register some callbacks, and
> >>> return.  The callback that get registered should arrange in turn to
> >>> register the callback passed as an argument when a tuple becomes
> >>> available, passing the planstate and context provided by
> >>> ExecStartAsync's caller, plus the TupleTableSlot containing the tuple.
> >>
> >> Although I don't imagine clearly about the case of
> >> async-aware-nodes under non-aware-nodes, it seems to have a high
> >> affinity with (true) parallel execution framework.
> > 
> > The ExecStartAsync is similar to ExecStartNode of my old
> > patch. One of the most annoying things of that is that it needs
> > to walk down to their descendents and in turn it needs garbageous
> > corresponding additional codes for all type of nodes which can
> > have children.
> 
> Unless I am missing something, I wonder if this is where
> planstate_tree_walker() introduced by commit 8dd401aa is useful. For
> example, I see that it's used in ExecShutdownNode() in a manner that looks
> interesting for this discussion.

Oh, that's a part of parallel execution sutff. Thanks for letting
me know of that. The walker approach also fits to kick functions
that most types of node is unrelated. Only one (or two, including
ForeignScan) types of nodes are involved.

The attached patches have the same functionality but using
planstate_tree_walker instead of callbacks. This seems further
simpler the callbacks.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

>From a7f0f1f9077b474dd212db1fb690413dd7c4ef79 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi 
Date: Thu, 21 Jan 2016 09:37:25 +0900
Subject: [PATCH 1/2] PoC: Async start callback for executor using
 planstate_tree_walker

This patch allows async-capable nodes to run the node before
ExecProcNode(). eflags has new bit EXEC_FLAG_ASYNC to request
asynchronous execution to children on ExecInit phase.

As an example, nodeSeqscan registers dummy callback if requested, and
nodeAppend unconditionally requests to its children. So a plan
Append(SeqScan, SeqScan) runs the callback and yields LOG messages.
---
 src/backend/executor/execMain.c|   2 +
 src/backend/executor/execProcnode.c|  24 +
 src/backend/executor/nodeAppend.c  |   2 +
 src/backend/executor/nodeGather.c  | 167 -
 src/backend/executor/nodeMergeAppend.c |   3 +
 src/backend/executor/nodeNestloop.c|  13 +++
 src/backend/executor/nodeSeqscan.c |  16 
 src/include/executor/executor.h|   2 +
 src/include/executor/nodeGather.h  |   1 +
 src/include/executor/nodeSeqscan.h |   1 +
 src/include/nodes/execnodes.h  |   2 +
 11 files changed, 167 insertions(+), 66 deletions(-)

diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 76f7297..32b7bc3 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1552,6 +1552,8 @@ ExecutePlan(EState *estate,
 	if (use_parallel_mode)
 		EnterParallelMode();
 
+	ExecStartNode(planstate);
+
 	/*
 	 * Loop until we've processed the proper number of tuples from the plan.
 	 */
diff --git a/src/backend/executor/execProcnode.c b/src/backend/executor/execProcnode.c
index a31dbc9..2107ced 100644
--- a/src/backend/executor/execProcnode.c
+++ b/src/backend/executor/execProcnode.c
@@ -786,6 +786,30 @@ ExecEndNode(PlanState *node)
 }
 
 /*
+ * ExecStartNode - execute registered early-startup callbacks
+ */
+bool
+ExecStartNode(PlanState *node)
+{
+	if (node == NULL)
+		return false;
+
+	switch (nodeTag(node))
+	{
+	case T_GatherState:
+		return ExecStartGather((GatherState *)node);
+		break;
+	case T_SeqScanState:
+		return ExecStartSeqScan((SeqScanState *)node);
+		break;
+	default:
+		break;	
+	}
+
+	return planstate_tree_walker(node, ExecStartNode, NULL);
+}
+
+/*
  * ExecShutdownNode
  *
  * Give execution nodes a chance to stop asynchronous resource consumption
diff --git a/src/backend/executor/nodeAppend.c b/src/backend/executor/nodeAppend.c
index a26bd63..d10364c 100644
--- a/src/backend/executor/nodeAppend.c
+++ b/src/backend/executor/nodeAppend.c
@@ -165,6 +165,8 @@ ExecInitAppend(Append *node, EState *estate, int eflags)
 	{
 		Plan	   *initNode = (Plan *) lfirst(lc);
 
+		/* always request async-execition for children */
+		eflags |= EXEC_FLAG_ASYNC;
 		appendplanstates[i] = ExecInitNode(initNode, estate, eflags);
 		i++;
 	}
diff --git a/src/backend/executor/nodeGather.c b/src/backend/executor/nodeGather.c
index 16c981b..097f4bb

Re: [HACKERS] pglogical most basic setup for logical replication

2016-01-26 Thread Sebastien Diemer

>
> It sounds like you must be running on PostgreSQL 9.4.

Indeed I am on PostgreSQL 9.4, I omitted this important point.

You'll need to drop the subscriber database and re-create it. Use a new
> node name.

Seems to work but I still do not really understand what was wrong in the
first place though.
Thanks for your help Craig !

*Sébastien DIEMER*
*Polyconseil* | 26 rue de Berri, 75008 Paris

2016-01-26 11:22 GMT+01:00 Craig Ringer :

> On 26 January 2016 at 18:14, Sebastien Diemer <
> sebastien.die...@polyconseil.fr> wrote:
>
>> Hello,
>>
>> I did not manage to make the simplest logical replication scheme work
>> with pglogical.
>> My setup is the following: two postgresql nodes (one provider and one
>> subscriber) with one database and one simple table:
>> `CREATE TABLE t (c1 integer, PRIMARY KEY (c1));`
>>
>> I followed the README provided with pglogical source code without any
>> success.
>> I created the provider and the subscriber nodes without problem but then,
>> after sending the sql query `SELECT
>> pglogical.create_subscription('subscription1', 'host=localhost port=55432
>> dbname=postgres');`  I got:
>>
>> "ERROR:  pglogical_origin extension not found"
>>
>
> It sounds like you must be running on PostgreSQL 9.4.
>
> 9.5 was the primary target for pglogical. Replicating from 9.4 to 9.4 was
> added quite late in the process. It seems we left out some key information
> from the documentation there.
>
>
>> "ERROR:  subscriber subscription1 initialization failed during
>> nonrecoverable step (s), please try the setup again"
>>
>> I tried to replay the setup again without any success.
>>
>
> You'll need to drop the subscriber database and re-create it. Use a new
> node name.
>
> --
>  Craig Ringer   http://www.2ndQuadrant.com/
>  PostgreSQL Development, 24x7 Support, Training & Services
>

[HACKERS] pglogical most basic setup for logical replication

2016-01-26 Thread Sebastien Diemer

Hello,

I did not manage to make the simplest logical replication scheme work with
pglogical.
My setup is the following: two postgresql nodes (one provider and one
subscriber) with one database and one simple table:
`CREATE TABLE t (c1 integer, PRIMARY KEY (c1));`

I followed the README provided with pglogical source code without any
success.
I created the provider and the subscriber nodes without problem but then,
after sending the sql query `SELECT
pglogical.create_subscription('subscription1', 'host=localhost port=55432
dbname=postgres');`  I got:

"ERROR:  pglogical_origin extension not found"

So I `CREATE EXTENSION pglogical_origin;` on the subscriber node (this step
is not mentioned in the README).

But then, I have the following error on the worker running on the
subscriber node:

"ERROR:  subscriber subscription1 initialization failed during
nonrecoverable step (s), please try the setup again"

I tried to replay the setup again without any success.
What am I missing here ?

Thanks for your help


*Sébastien DIEMER*

Re: [HACKERS] pglogical most basic setup for logical replication

On 26 January 2016 at 18:14, Sebastien Diemer <
sebastien.die...@polyconseil.fr> wrote:

> Hello,
>
> I did not manage to make the simplest logical replication scheme work with
> pglogical.
> My setup is the following: two postgresql nodes (one provider and one
> subscriber) with one database and one simple table:
> `CREATE TABLE t (c1 integer, PRIMARY KEY (c1));`
>
> I followed the README provided with pglogical source code without any
> success.
> I created the provider and the subscriber nodes without problem but then,
> after sending the sql query `SELECT
> pglogical.create_subscription('subscription1', 'host=localhost port=55432
> dbname=postgres');`  I got:
>
> "ERROR:  pglogical_origin extension not found"
>

It sounds like you must be running on PostgreSQL 9.4.

9.5 was the primary target for pglogical. Replicating from 9.4 to 9.4 was
added quite late in the process. It seems we left out some key information
from the documentation there.


> "ERROR:  subscriber subscription1 initialization failed during
> nonrecoverable step (s), please try the setup again"
>
> I tried to replay the setup again without any success.
>

You'll need to drop the subscriber database and re-create it. Use a new
node name.

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

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-01-26 Thread Etsuro Fujita


On 2016/01/25 17:03, Rushabh Lathia wrote:

Here are couple of comments:



1)

int
IsForeignRelUpdatable (Relation rel);



Documentation for IsForeignUpdatable() need to change as it says:

If the IsForeignRelUpdatable pointer is set to NULL, foreign tables are
assumed
to be insertable, updatable, or deletable if the FDW provides
ExecForeignInsert,
ExecForeignUpdate or ExecForeignDelete respectively.

With introduce of DMLPushdown API now this is no more correct, as even if
FDW don't provide ExecForeignInsert, ExecForeignUpdate or
ExecForeignDelete API
still foreign tables are assumed to be updatable or deletable with
DMLPushdown
API's, right ?


That's what I'd like to discuss.

I intentionally leave that as-is, because I think we should determine 
the updatability of a foreign table in the current manner.  As you 
pointed out, even if the FDW doesn't provide eg, ExecForeignUpdate, an 
UPDATE on a foreign table could be done using the DML pushdown APIs if 
the UPDATE is *pushdown-safe*.  However, since all UPDATEs on the 
foreign table are not necessarily pushdown-safe, I'm not sure it's a 
good idea to assume the table-level updatability if the FDW provides the 
DML pushdown callback routines.  To keep the existing updatability 
decision, I think the FDW should provide the DML pushdown callback 
routines together with ExecForeignInsert, ExecForeignUpdate, or 
ExecForeignDelete.  What do you think about that?



2)

+ /* SQL statement to execute remotely (as a String node) */
+ FdwDmlPushdownPrivateUpdateSql,

FdwDmlPushdownPrivateUpdateSql holds the UPDATE/DELETE query, so name should
be something like FdwDmlPushdownPrivateQuery os FdwDmlPushdownPrivateSql ?



Later I realized that for FdwModifyPrivateIndex too the index name is
FdwModifyPrivateUpdateSql even though its holding any DML query. Not sure
whether we should consider to change this or not ?


To tell the truth, I imitated FdwModifyPrivateIndex when adding 
FdwDmlPushdownPrivateIndex, because I think "UpdateSql" means INSERT, 
UPDATE, or DELETE, not just UPDATE.  (IsForeignRelUpdatable discussed 
above reports not only the updatability but the insertability and 
deletability of a foreign table!).  So, +1 for leaving that as-is.



Apart from this perform sanity testing on the new patch and things working
as expected.


Thanks for the review!

Best regards,
Etsuro Fujita




--
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] proposal: PL/Pythonu - function ereport

2016-01-26 Thread Chapman Flack

On recent occasions, Pavel Stehule and Catalin Iacob have written:
...
>> But *inside* PLPython what I wrote is true, see this example for what I
>> mean:
>>
>> CREATE FUNCTION test()
>>   RETURNS int
>> AS $$
>>   def a_func():
>> raise plpy.Error('an error')
>>
>>   try:
>> a_func()
>>   except plpy.Error as exc:
>> plpy.info('have exc {} of type {}'.format(exc, type(exc)))
>> plpy.info('have spidata {}'.format(hasattr(exc, 'spidata')))
>> $$ LANGUAGE plpython3u;
>>
>> Running this produces:
>>
>> DROP FUNCTION
>> CREATE FUNCTION
>> psql:plpy_demo:16: INFO:  have exc an error of type 
>> psql:plpy_demo:16: INFO:  have spidata False
>>
>>> Currently If I raise plpy.Error, then it is immediately trasformed to
>>> PostgreSQL, and and then to SPIError and successors.
>>
>> Depends how you define immediately. I would say it's really not
>> immediately, it's only at the Postgres boundary. Imagine in the
>> example above that a_func could call lots of other Python code and
>> somewhere deep down raise Error would be used. Within that whole
>> execution the error stays Error and can be caught as such, it will
>> have nothing to do with SPIError.
...
> I would to reduce this patch and don't try to touch on buildin exceptions.
> I hope, so there isn't any disagreement?
> 
> I though about it lot of, and I see a  the core of problems in orthogonal
> constructed exceptions in Python and Postgres. We working with statements
> elog, ereport, RAISE EXCEPTION - and these statements are much more like
> templates - can generate any possible exception. Python is based on working
> with instances of predefined exceptions. And it is core of problem.
> Template like class cannot be ancestor of instance oriented classes. This
> is task for somebody who knows well OOP and meta OOP in Python - total

I've been following this discussion with great interest, because PL/Java
also is rather overdue for tackling the same issues: it has some partial
ability to catch things from PostgreSQL and even examine them in proper
detail, but very limited ability to throw things as information-rich as
is possible from C with ereport. And that work is not as far along as
you are with PL/Python, there is just a preliminary design/discussion
wiki document at
  https://github.com/tada/pljava/wiki/Thoughts-on-logging

I was unaware of this project in PL/Pythonu when I began it, then added
the reference when I saw this discussion.

In some ways designing for PL/Java might be easier because it is not
complete freedom to invent the whole design; for example in Java there
is already an SQLException hierarchy specified in JDBC 4, with
SQLNonTransientException, SQLTransientException, SQLRecoverable exception
and their several subclasses, with a defined mapping from the leading
two-character SQLSTATE category codes. So for Java at least one part
of the bikeshed is already painted. :)

But a lot of the rest of the design clearly involves asking the same
questions, such as what happens to an event as it bubbles up from a
PL function called by a PG query called by a PL function ... maybe all
the way out to an exception handler in front-end code. (That could be
a thinkable thought for Java because the standards specify JDBC as its
common database API both client-side and for server PL code, so it's
natural to think about the exception handling being similar both places.
It would be analogous to having the plpy database access functions designed
to present an interface consistent with a client-side API like psycopg.
Forgive me, I'm not familiar enough with PL/Pythonu to know if it *is*
like that or not.)

>From following this thread, I have the impression that what is
currently under discussion is low-hanging fruit that may be committed
soon, and other questions remaining for further design and discussion.
I'll continue to be interested in how the deeper design issues continue
to be shaped. Maybe ideas can sort of cross-pollinate 

-Chap


-- 
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] [PROPOSAL] VACUUM Progress Checker.

2016-01-26 Thread Vinayak Pokale

Hi,

Please find attached updated patch with an updated interface.

On Jan 26, 2016 11:22 AM, "Vinayak Pokale"  wrote:
>
> Hi Amit,
>
> Thank you for reviewing the patch.
>
> On Jan 26, 2016 9:51 AM, "Amit Langote" 
wrote:
> >
> >
> > Hi Vinayak,
> >
> > On 2016/01/25 20:58, Vinayak Pokale wrote:
> > > Hi,
> > >
> > > Please find attached updated patch with an updated interface.
> > >
> >
> > Thanks for updating the patch.
> >
> > > I added the below interface to update the
> > > scanned_heap_pages,scanned_index_pages and index_scan_count only.
> > > void pgstat_report_progress_scanned_pages(int num_of_int, uint32
> > > *progress_scanned_pages_param)
> >
> > I think it's still the same interface with the names changed. IIRC, what
> > was suggested was to provide a way to not have to pass the entire array
> > for the update of a single member of it. Just pass the index of the
> > updated member and its new value. Maybe, something like:
> >
> > void pgstat_progress_update_counter(int index, uint32 newval);
> >
> > The above function would presumably update the value of
> > beentry.st_progress_counter[index] or something like that.

Following interface functions are added:

/*
 * index: in the array of uint32 counters in the beentry
 * counter: new value of the (index)th counter
 */
void
pgstat_report_progress_update_counter(int index, uint32 counter)

/*
called to updatet the VACUUM progress phase.
msg: new value of (index)th message
*/
void
pgstat_report_progress_update_message(int index, char
msg[N_PROGRESS_PARAM][PROGRESS_MESSAGE_LENGTH])

Regards,
Vinayak


Vacuum_progress_checker_v10.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] CustomScan under the Gather node?

2016-01-26 Thread Kouhei Kaigai

> -Original Message-
> From: Amit Kapila [mailto:amit.kapil...@gmail.com]
> Sent: Wednesday, January 27, 2016 2:30 PM
> To: Kaigai Kouhei(海外 浩平)
> Cc: pgsql-hackers@postgresql.org
> Subject: ##freemail## Re: [HACKERS] CustomScan under the Gather node?
> 
> On Tue, Jan 26, 2016 at 12:00 PM, Kouhei Kaigai  wrote:
> >
> > Hello,
> >
> > What enhancement will be necessary to implement similar feature of
> > partial seq-scan using custom-scan interface?
> >
> > It seems to me callbacks on the three points below are needed.
> > * ExecParallelEstimate
> > * ExecParallelInitializeDSM
> > * ExecParallelInitializeWorker
> >
> > Anything else?
> 
> I don't think so.
> 
> > Does ForeignScan also need equivalent enhancement?
> 
> I think this depends on the way ForeignScan is supposed to be
> parallelized, basically if it needs to coordinate any information
> with other set of workers, then it will require such an enhancement.
>
After the post yesterday, I reminded an possible scenario around FDW
if it manages own private storage, like cstore_fdw.

Probably, ForeignScan node performing on columnar store (for example)
will need a coordination information like as partial seq-scan doing.
It is a case very similar to the implementation on local storage.

On the other hands, if we try postgres_fdw (or others) to get parallelized
with background worker, I doubt whether we need this coordination information
on local side. Remote query will have an additional qualifier to skip blocks
already fetched for this purpose.
At least, it does not needs something special enhancement.

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] Why format() adds double quote?

2016-01-27 6:24 GMT+01:00 Tatsuo Ishii :

> >> > I can agree, so current behave can be useful in some cases, but still
> it
> >> is
> >> > bug (inconsistency) between PostgreSQL parser and PostgreSQL escaping
> >> > functions.
> >> >
> >> > Currently, any multibyte char can be unescaped identifier (only
> >> apostrophes
> >> > are tested). We should to test white chars too.
> >>
> >> Really? I thought we do that test.
> >>
> >
> > what you are expecting from this test? UTF single quotes are tested only
> in
> > quote functions probably.
>
> I just wanted to demonstrate multibyte chars including ASCII white
> spaces can be an identifier.
>

I understand now.


>
> > We should to test white chars too.
>
> What do you exactly propose regarding white chars and multibyte chars
> here? Maybe you propose to consider non ASCII white spaces (treate
> them as ASCII white spaces)?
>

I propose the work with UTF white chars should be same like ASCII white
chars. The current design is too simple - with possible pretty bad issues.
Daniel's example is good - there is big gap in design.

Regards

Pavel


>
> Best regards,
> --
> Tatsuo Ishii
> SRA OSS, Inc. Japan
> English: http://www.sraoss.co.jp/index_en.php
> Japanese:http://www.sraoss.co.jp
>
> > Pavel
> >
> >
> >>
> >> test=# create table t6("あいう えお" int);
> >> CREATE TABLE
> >> test=# \d t6
> >>  Table "public.t6"
> >>Column|  Type   | Modifiers
> >> -+-+---
> >>  あいう えお | integer |
> >> --
> >> Tatsuo Ishii
> >> SRA OSS, Inc. Japan
> >> English: http://www.sraoss.co.jp/index_en.php
> >> Japanese:http://www.sraoss.co.jp
> >>
>

Re: [HACKERS] Why format() adds double quote?

>> What do you exactly propose regarding white chars and multibyte chars
>> here? Maybe you propose to consider non ASCII white spaces (treate
>> them as ASCII white spaces)?
>>
> 
> I propose the work with UTF white chars should be same like ASCII white
> chars. The current design is too simple - with possible pretty bad issues.
> Daniel's example is good - there is big gap in design.

I think we should consider followings before going forward:

1) Does PostgreSQL treat non ASCII white spaces same as ASCII white
   spaces anyware in the system? If not, there's no reason we should
   think format() and quote_indent() are exception.

2) What does the SQL standard say? Do they say that non ASCII white
   spaces should be treated as ASCII white spaces?

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp


-- 
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] Existence check for suitable index in advance when concurrently refreshing.

On Tue, Jan 26, 2016 at 9:33 PM, Masahiko Sawada  wrote:
> Hi all,
>
> In concurrently refreshing materialized view, we check whether that
> materialized view has suitable index(unique and not having WHERE
> condition), after filling data to new snapshot
> (refresh_matview_datafill()).
> This logic leads to taking a lot of time until postgres returns ERROR
> log if that table doesn't has suitable index and table is large. it
> wastes time.
> I think we should check whether that materialized view can use
> concurrently refreshing or not in advance.

+1

> The patch is attached.
>
> Please give me feedbacks.

+indexRel = index_open(indexoid, RowExclusiveLock);

Can we use AccessShareLock here, instead?

+if (indexStruct->indisunique &&
+IndexIsValid(indexStruct) &&
+RelationGetIndexExpressions(indexRel) == NIL &&
+RelationGetIndexPredicate(indexRel) == NIL)
+hasUniqueIndex = true;
+
+index_close(indexRel, RowExclusiveLock);

In the case where hasUniqueIndex = true, ISTM that we can get out of
the loop immediately just after calling index_close(). No?

+/* Must have at least one unique index */
+Assert(foundUniqueIndex);

Can we guarantee that there is at least one valid unique index here?
If yes, it's better to write the comment about that.

Regards,

-- 
Fujii Masao


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

Re: [HACKERS] pglogical - logical replication contrib module

2016-01-26 Thread Joshua D. Drake


On 12/31/2015 03:34 PM, Petr Jelinek wrote:

Hi,

I'd like to submit the replication solution which is based on the
pglogical_output [1] module (which is obviously needed for this to
compile).


This is fantastic! However, history presents itself here and PostgreSQL 
in the past has not "blessed" a single solution for Replication. 
Obviously that changed a bit with streaming replication but this is a 
bit different than that. As I understand it, PgLogical is Logical 
Replication (similar to Slony and Londiste). I wouldn't be surprised 
(although I don't know) if Slony were to start using some of the 
pglogical_output module features in the future.


If we were to accept PgLogical into core, it will become the default 
blessed solution for PostgreSQL. While that is great in some ways  it is 
a different direction than the project has taken in the past. Is this 
what we want to do?


Sincerely,

Joshua D. Drake

--
Command Prompt, Inc.  http://the.postgres.company/
 +1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.


--
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 9.5 release note

> Tatsuo Ishii  writes:
>> I saw following item in release-9.5.sgml:
>> 
>>
>> Support comments on domain
>> constraints (lvaro Herrera)
>>
>>   
> 
>> It seems the release note has nothing to do with the commit. Also,
>> commenting on DOMAIN constraints is not new in 9.5, I think.
> 
> Yeah, it is.  See 7eca575d1c28f6eee2bf4564f3d458d10c4a8f47, which is
> the commit that should have been listed here, likely.

Are you willing to fix it?

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp


-- 
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 format() adds double quote?

> 2016-01-26 21:00 GMT+01:00 Daniel Verite :
> 
>> Tatsuo Ishii wrote:
>>
>> > IMO, it's a bug or at least an inconsistency
>>
>> Personally I don't see this change being good for everything.
>>
>> Let's play devil's advocate:
>>
>> create table abc(U&"foo\2003" int);
>>
>> U+2003 is 'EM SPACE', in Unicode's General Punctuation block.
>>
>> With the current version, format('%I', attname) on this column is:
>> "foo "
>>
>> With the patched version, it produces this:
>> foo
>>
>> So the visual hint that there are more characters at the end is lost.
>>
> 
> I can agree, so current behave can be useful in some cases, but still it is
> bug (inconsistency) between PostgreSQL parser and PostgreSQL escaping
> functions.
> 
> Currently, any multibyte char can be unescaped identifier (only apostrophes
> are tested). We should to test white chars too.

Really? I thought we do that test.

test=# create table t6("あいう えお" int);
CREATE TABLE
test=# \d t6
 Table "public.t6"
   Column|  Type   | Modifiers 
-+-+---
 あいう えお | integer | 
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp


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

[HACKERS] Code of Conduct plan

2016-01-26 Thread Josh Berkus

Community members:

A number of people have contacted the Core Team about taking action
regarding a Code of Conduct (CoC) for the project. After some
discussion, the plan we have come up with is below.

**Please do not reply-all to this email, as we do not wish to generate
additional list traffic regarding CoCs**

1. The Core Team will appoint an exploration committee which will look
at various proposals (including the one drafted on pgsql-general) for
CoCs and discuss them. This committee will include both major community
members and less central folks who have hands-on experience with CoCs
and community management issues.  If you know of PostgreSQL community
members who have relevant experience, please nominate them by emailing
the core team: pgsql-c...@postgresql.org.

2. We will also hire a professional consultant to advise the committee
on CoC development, adoption, training, and enforcement.  Again, if
community members have a consultant to recommend, please email the core
team.

3. This committee will post a draft CoC or possibly a selection of draft
CoCs by or before late April for community comment.  Likely the
committee will be publishing drafts more frequently, but that will be up
to them to work out.

4. At the pgCon Community Unconference, and again at pgconf.EU, we will
have sessions where people can discuss and provide feedback about
proposed (or adopted) CoCs.  Possibly we will have CoC-related trainings
as well.

5. Once a draft is agreed upon, it will be circulated to our various
sub-communities for comment.

6. A "final" CoC will be endorsed by the committee and the Core Team
shortly after pgConf.EU, unless there is sufficently strong consensus to
adopt one before then.

Yes, we realize this is a long timeline.  The PostgreSQL Project has
never been about implementing things in a hurry; our practice has always
been to take all of the time required to develop the right feature the
right way.  Adopting a CoC is no different; if anything, we need to take
*more* time in order to get input from community members who do not
speak up frequently or assertively.

In the meantime, our policy remains: if you have experienced harassment
or feel that you are being treated unfairly by other project members,
email the Core Team and we will investigate your complaint and take
appropriate action.

Also, we want to thank Josh Drake for raising the CoC issue and getting
it off the TODO list and into process, and devising an initial "seed"
CoC.  Such things are all too easy to keep postponing.

Again, Please DO NOT comment on this plan on-list; one of the pieces of
feedback we have received loud and clear is that many community members
are unhappy with the amount of list traffic devoted to the subject of
CoCs.  As such, if you have comments on the plan above, please email the
core team instead of replying on-list, or wait for the committee and
address comments to them.

--Josh Berkus
  PostgreSQL Core Team


-- 
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 format() adds double quote?

>> > I can agree, so current behave can be useful in some cases, but still it
>> is
>> > bug (inconsistency) between PostgreSQL parser and PostgreSQL escaping
>> > functions.
>> >
>> > Currently, any multibyte char can be unescaped identifier (only
>> apostrophes
>> > are tested). We should to test white chars too.
>>
>> Really? I thought we do that test.
>>
> 
> what you are expecting from this test? UTF single quotes are tested only in
> quote functions probably.

I just wanted to demonstrate multibyte chars including ASCII white
spaces can be an identifier.

> We should to test white chars too.

What do you exactly propose regarding white chars and multibyte chars
here? Maybe you propose to consider non ASCII white spaces (treate
them as ASCII white spaces)?

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

> Pavel
> 
> 
>>
>> test=# create table t6("あいう えお" int);
>> CREATE TABLE
>> test=# \d t6
>>  Table "public.t6"
>>Column|  Type   | Modifiers
>> -+-+---
>>  あいう えお | integer |
>> --
>> Tatsuo Ishii
>> SRA OSS, Inc. Japan
>> English: http://www.sraoss.co.jp/index_en.php
>> Japanese:http://www.sraoss.co.jp
>>


-- 
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] RFC: replace pg_stat_activity.waiting with something more descriptive

2016-01-26 Thread Amit Kapila

On Tue, Jan 26, 2016 at 1:40 PM, and...@anarazel.de 
wrote:
>
> On 2016-01-26 13:22:09 +0530, Amit Kapila wrote:
> > @@ -633,9 +633,11 @@ postgres   27093  0.0  0.0  30096  2752 ?
 Ss   11:34   0:00 postgres: ser
> >   Time when the state was last
changed
> >  
> >  
> > - waiting
> > - boolean
> > - True if this backend is currently waiting on a lock
> > + wait_event
> > + text
> > + Wait event name if backend is currently waiting, otherwise
> > +  process not waiting
> > + 
> >  
> >  
>
> I still think this is a considerable regression in pg_stat_activity
> usability. There are lots of people out there that have queries that
> automatically monitor pg_stat_activity.waiting, and automatically go to
> pg_locks to understand what's going on, if there's one. With the above
> definition, that got much harder. Not only do I have to write
> WHERE wait_event <> 'process not waiting', but then also parse the wait
> event name, to know whether the process is waiting on a heavyweight
> lock, or something else!
>
> I do think there's a considerable benefit in improving the
> instrumentation here, but his strikes me as making live more complex for
> more users than it makes it easier.
>

Here, we have two ways to expose this functionality to user, first is
that we expose this new set of information (wait_type, wait_event)
separately either in new view or in pg_stat_activity and ask users
to migrate to this new information and mark pg_stat_activity.waiting as
deprecated and then remove it in future versions and second is remove
pg_stat_activity.waiting and expose new set of information which will
make users to forcibly move to this new set of information.  I think both
the ways have it's pros and cons and they are discussed upthread and
based on that I have decided to move forward with second way.

> At the very least this should be
> split into two fields (type & what we're actually waiting on).
>

makes sense to me, so we can repersent wait_type as:
wait_type text, values can be Lock (or HWLock), LWLock, Network, etc.

Let me know if that is okay or you have something else in mind?

> I also
> strongly suspect we shouldn't use in band signaling ("process not
> waiting"), but rather make the field NULL if we're not waiting on
> anything.
>

Agree, will change in next version of patch.

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

Re: [HACKERS] Why format() adds double quote?

2016-01-26 21:00 GMT+01:00 Daniel Verite :

> Tatsuo Ishii wrote:
>
> > IMO, it's a bug or at least an inconsistency
>
> Personally I don't see this change being good for everything.
>
> Let's play devil's advocate:
>
> create table abc(U&"foo\2003" int);
>
> U+2003 is 'EM SPACE', in Unicode's General Punctuation block.
>
> With the current version, format('%I', attname) on this column is:
> "foo "
>
> With the patched version, it produces this:
> foo
>
> So the visual hint that there are more characters at the end is lost.
>

I can agree, so current behave can be useful in some cases, but still it is
bug (inconsistency) between PostgreSQL parser and PostgreSQL escaping
functions.

Currently, any multibyte char can be unescaped identifier (only apostrophes
are tested). We should to test white chars too.

Regards

Pavel


>
> Best regards,
> --
> Daniel Vérité
> PostgreSQL-powered mailer: http://www.manitou-mail.org
> Twitter: @DanielVerite
>

Re: [HACKERS] CustomScan under the Gather node?

2016-01-26 Thread Amit Kapila

On Tue, Jan 26, 2016 at 12:00 PM, Kouhei Kaigai 
wrote:
>
> Hello,
>
> What enhancement will be necessary to implement similar feature of
> partial seq-scan using custom-scan interface?
>
> It seems to me callbacks on the three points below are needed.
> * ExecParallelEstimate
> * ExecParallelInitializeDSM
> * ExecParallelInitializeWorker
>
> Anything else?

I don't think so.

> Does ForeignScan also need equivalent enhancement?

I think this depends on the way ForeignScan is supposed to be
parallelized, basically if it needs to coordinate any information
with other set of workers, then it will require such an enhancement.

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

Re: [HACKERS] Why format() adds double quote?

2016-01-27 6:13 GMT+01:00 Tatsuo Ishii :

> > 2016-01-26 21:00 GMT+01:00 Daniel Verite :
> >
> >> Tatsuo Ishii wrote:
> >>
> >> > IMO, it's a bug or at least an inconsistency
> >>
> >> Personally I don't see this change being good for everything.
> >>
> >> Let's play devil's advocate:
> >>
> >> create table abc(U&"foo\2003" int);
> >>
> >> U+2003 is 'EM SPACE', in Unicode's General Punctuation block.
> >>
> >> With the current version, format('%I', attname) on this column is:
> >> "foo "
> >>
> >> With the patched version, it produces this:
> >> foo
> >>
> >> So the visual hint that there are more characters at the end is lost.
> >>
> >
> > I can agree, so current behave can be useful in some cases, but still it
> is
> > bug (inconsistency) between PostgreSQL parser and PostgreSQL escaping
> > functions.
> >
> > Currently, any multibyte char can be unescaped identifier (only
> apostrophes
> > are tested). We should to test white chars too.
>
> Really? I thought we do that test.
>

what you are expecting from this test? UTF single quotes are tested only in
quote functions probably.

Pavel


>
> test=# create table t6("あいう えお" int);
> CREATE TABLE
> test=# \d t6
>  Table "public.t6"
>Column|  Type   | Modifiers
> -+-+---
>  あいう えお | integer |
> --
> Tatsuo Ishii
> SRA OSS, Inc. Japan
> English: http://www.sraoss.co.jp/index_en.php
> Japanese:http://www.sraoss.co.jp
>

Re: [HACKERS] insert/update performance

2016-01-26 Thread Jinhua Luo

Ok, I found the vaccum output is correct.

I check the codes of lazy_scan_heap(), the rows to be removed are
reported in two parts, one is return of heap_page_prune(), the other
is ItemIdIsDead() when scanning the page.

After scanning all pages of the relation, those rows would be clean up in:

if (vacrelstats->num_dead_tuples > 0) {
...
lazy_vacuum_heap()
...
}

It would then output
> INFO:  "test": removed 6 row versions in 1 pages

The number of rows is correct.

But what kind of rows would satisfy heap_page_prune() and what would not?

In my case all updates are doing the same thing (there is no HOT
updates, obviously), but why some updated rows are reported by
heap_page_prune() but the others are not? And it's also a random
issue. That means sometimes heap_page_prune() would report all
removable rows, and sometimes it reports no rows.


Regards,
Jinhua Luo


-- 
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 format() adds double quote?

2016-01-27 8:25 GMT+01:00 Tatsuo Ishii :

> >> What do you exactly propose regarding white chars and multibyte chars
> >> here? Maybe you propose to consider non ASCII white spaces (treate
> >> them as ASCII white spaces)?
> >>
> >
> > I propose the work with UTF white chars should be same like ASCII white
> > chars. The current design is too simple - with possible pretty bad
> issues.
> > Daniel's example is good - there is big gap in design.
>
> I think we should consider followings before going forward:
>
> 1) Does PostgreSQL treat non ASCII white spaces same as ASCII white
>spaces anyware in the system? If not, there's no reason we should
>think format() and quote_indent() are exception.
>

+1


>
> 2) What does the SQL standard say? Do they say that non ASCII white
>spaces should be treated as ASCII white spaces?
>

I am not sure, if SQL standard say some about it. But I am sure, so using
unescaped or unclosed UTF8 spaces is bug, dangerous, wrong

Pavel


> Best regards,
> --
> Tatsuo Ishii
> SRA OSS, Inc. Japan
> English: http://www.sraoss.co.jp/index_en.php
> Japanese:http://www.sraoss.co.jp
>

Re: [HACKERS] RFC: replace pg_stat_activity.waiting with something more descriptive

2016-01-26 Thread and...@anarazel.de

On 2016-01-26 13:22:09 +0530, Amit Kapila wrote:
> @@ -633,9 +633,11 @@ postgres   27093  0.0  0.0  30096  2752 ?Ss   
> 11:34   0:00 postgres: ser
>   Time when the state was last changed
>  
>  
> - waiting
> - boolean
> - True if this backend is currently waiting on a lock
> + wait_event
> + text
> + Wait event name if backend is currently waiting, otherwise
> +  process not waiting
> + 
>  
>  

I still think this is a considerable regression in pg_stat_activity
usability. There are lots of people out there that have queries that
automatically monitor pg_stat_activity.waiting, and automatically go to
pg_locks to understand what's going on, if there's one. With the above
definition, that got much harder. Not only do I have to write
WHERE wait_event <> 'process not waiting', but then also parse the wait
event name, to know whether the process is waiting on a heavyweight
lock, or something else!

I do think there's a considerable benefit in improving the
instrumentation here, but his strikes me as making live more complex for
more users than it makes it easier. At the very least this should be
split into two fields (type & what we're actually waiting on). I also
strongly suspect we shouldn't use in band signaling ("process not
waiting"), but rather make the field NULL if we're not waiting on
anything.

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] pgbench - allow backslash-continuations in custom scripts

On Tue, Jan 26, 2016 at 6:51 PM, Kyotaro HORIGUCHI
 wrote:
> Mmm. I believed that this is on CF app..
>
> At Tue, 19 Jan 2016 15:41:54 +0900, Michael Paquier 
>  wrote in 
>

Re: [HACKERS] Improve tab completion for REFRESH MATERIALIZED VIEW

On Tue, Jan 26, 2016 at 9:52 PM, Masahiko Sawada  wrote:
> Tab completion for REFRESH command is oddly with following scenario.
>
> =# REFRESH MATERIALIZED VIEW CONCURRENTLY hoge_mv [Tab]
>
> It shows only WITH DATA option without WITH NO DATA option.
> Attached patch improves tab completion for WITH DATA/NO DATA option of
> REFRESH MATERIALIZED VIEW.

Correct. Nice catch and good patch.
-- 
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] pg_lsn cast to/from int8

2016-01-26 Thread Magnus Hagander

Is there a reason we don't have casts between int8 and pg_lsn? AFAICT it
works fine if I create the cast manually... Is it because of
signed/unsigned if people have really really many transactions?

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

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-01-26 Thread Rushabh Lathia

On Tue, Jan 26, 2016 at 4:15 PM, Etsuro Fujita 
wrote:

> On 2016/01/25 17:03, Rushabh Lathia wrote:
>
>> Here are couple of comments:
>>
>
> 1)
>>
>> int
>> IsForeignRelUpdatable (Relation rel);
>>
>
> Documentation for IsForeignUpdatable() need to change as it says:
>>
>> If the IsForeignRelUpdatable pointer is set to NULL, foreign tables are
>> assumed
>> to be insertable, updatable, or deletable if the FDW provides
>> ExecForeignInsert,
>> ExecForeignUpdate or ExecForeignDelete respectively.
>>
>> With introduce of DMLPushdown API now this is no more correct, as even if
>> FDW don't provide ExecForeignInsert, ExecForeignUpdate or
>> ExecForeignDelete API
>> still foreign tables are assumed to be updatable or deletable with
>> DMLPushdown
>> API's, right ?
>>
>
> That's what I'd like to discuss.
>
> I intentionally leave that as-is, because I think we should determine the
> updatability of a foreign table in the current manner.  As you pointed out,
> even if the FDW doesn't provide eg, ExecForeignUpdate, an UPDATE on a
> foreign table could be done using the DML pushdown APIs if the UPDATE is
> *pushdown-safe*.  However, since all UPDATEs on the foreign table are not
> necessarily pushdown-safe, I'm not sure it's a good idea to assume the
> table-level updatability if the FDW provides the DML pushdown callback
> routines.  To keep the existing updatability decision, I think the FDW
> should provide the DML pushdown callback routines together with
> ExecForeignInsert, ExecForeignUpdate, or ExecForeignDelete.  What do you
> think about that?
>
>
Sorry but I am not in favour of adding compulsion that FDW should provide
the DML pushdown callback routines together with existing ExecForeignInsert,
ExecForeignUpdate or ExecForeignDelete APIs.

May be we should change the documentation in such way, that explains

a) If FDW PlanDMLPushdown is NULL, then check for ExecForeignInsert,
ExecForeignUpdate or ExecForeignDelete APIs
b) If FDW PlanDMLPushdown is non-NULL and plan is not pushable
check for ExecForeignInsert, ExecForeignUpdate or ExecForeignDelete APIs
c) If FDW PlanDMLPushdown is non-NULL and plan is pushable
check for DMLPushdown APIs.

Does this sounds wired ?



> 2)
>>
>> + /* SQL statement to execute remotely (as a String node) */
>> + FdwDmlPushdownPrivateUpdateSql,
>>
>> FdwDmlPushdownPrivateUpdateSql holds the UPDATE/DELETE query, so name
>> should
>> be something like FdwDmlPushdownPrivateQuery os FdwDmlPushdownPrivateSql ?
>>
>
> Later I realized that for FdwModifyPrivateIndex too the index name is
>> FdwModifyPrivateUpdateSql even though its holding any DML query. Not sure
>> whether we should consider to change this or not ?
>>
>
> To tell the truth, I imitated FdwModifyPrivateIndex when adding
> FdwDmlPushdownPrivateIndex, because I think "UpdateSql" means INSERT,
> UPDATE, or DELETE, not just UPDATE.  (IsForeignRelUpdatable discussed above
> reports not only the updatability but the insertability and deletability of
> a foreign table!).  So, +1 for leaving that as-is.
>
>
Make sense for now.


> Apart from this perform sanity testing on the new patch and things working
>> as expected.
>>
>
> Thanks for the review!
>
> Best regards,
> Etsuro Fujita
>
>
>


-- 
Rushabh Lathia

Re: [HACKERS] pg_lsn cast to/from int8

2016-01-26 Thread Andres Freund

On 2016-01-26 14:56:21 +0100, Magnus Hagander wrote:
> Is there a reason we don't have casts between int8 and pg_lsn? AFAICT it
> works fine if I create the cast manually... Is it because of
> signed/unsigned if people have really really many transactions?

What for do you want that cast? Yes, the internally mostly share the
representation, but other than that, I don't really see why it's
interesting?


-- 
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_lsn cast to/from int8

2016-01-26 Thread Magnus Hagander

On Tue, Jan 26, 2016 at 3:00 PM, Andres Freund  wrote:

> On 2016-01-26 14:56:21 +0100, Magnus Hagander wrote:
> > Is there a reason we don't have casts between int8 and pg_lsn? AFAICT it
> > works fine if I create the cast manually... Is it because of
> > signed/unsigned if people have really really many transactions?
>
> What for do you want that cast? Yes, the internally mostly share the
> representation, but other than that, I don't really see why it's
> interesting?
>

In this case, mostly legacy compatibility. Making an app that works with
versions that don't have pg_lsn have a nice path forward to the modern
world. Being able to cast from pg_lsn to int8 can also make it easier to
work with the values in the client application, though I don't need that
for this particular one.

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

Re: [HACKERS] Improve tab completion for REFRESH MATERIALIZED VIEW

On Tue, Jan 26, 2016 at 10:25 PM, Michael Paquier
 wrote:
> On Tue, Jan 26, 2016 at 9:52 PM, Masahiko Sawada  
> wrote:
>> Tab completion for REFRESH command is oddly with following scenario.
>>
>> =# REFRESH MATERIALIZED VIEW CONCURRENTLY hoge_mv [Tab]
>>
>> It shows only WITH DATA option without WITH NO DATA option.
>> Attached patch improves tab completion for WITH DATA/NO DATA option of
>> REFRESH MATERIALIZED VIEW.
>
> Correct. Nice catch and good patch.

The patch looks good to me.

While testing the patch, I found that
REFRESH MATERIALIZED VIEW  doesn't list the materialized views.
I added the following change to the patch to fix that problem. Patch attached.

-   {"MATERIALIZED VIEW", NULL, NULL},
+   {"MATERIALIZED VIEW", NULL, _for_list_of_matviews},

Regards,

-- 
Fujii Masao


improve_tab_completion_for_refresh_v2.patch
Description: Binary data

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

Re: [HACKERS] Improve tab completion for REFRESH MATERIALIZED VIEW

On Tue, Jan 26, 2016 at 11:08 PM, Fujii Masao  wrote:
> On Tue, Jan 26, 2016 at 10:25 PM, Michael Paquier
>  wrote:
>> On Tue, Jan 26, 2016 at 9:52 PM, Masahiko Sawada  
>> wrote:
>>> Tab completion for REFRESH command is oddly with following scenario.
>>>
>>> =# REFRESH MATERIALIZED VIEW CONCURRENTLY hoge_mv [Tab]
>>>
>>> It shows only WITH DATA option without WITH NO DATA option.
>>> Attached patch improves tab completion for WITH DATA/NO DATA option of
>>> REFRESH MATERIALIZED VIEW.
>>
>> Correct. Nice catch and good patch.
>
> The patch looks good to me.
>
> While testing the patch, I found that
> REFRESH MATERIALIZED VIEW  doesn't list the materialized views.

This is not true. Sorry for the noise...

Regards,

-- 
Fujii Masao


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

Re: [HACKERS]WIP: Covering + unique indexes.

2016-01-26 Thread Anastasia Lubennikova


25.01.2016 03:32, Jeff Janes:

On Fri, Jan 22, 2016 at 7:19 AM, Anastasia Lubennikova
 wrote:

Done. I hope that my patch is close to the commit too.


Thanks for the update.

I've run into this problem:

create table foobar (x text, w text);
create unique index foobar_pkey on foobar (x) including (w);
alter table foobar add constraint foobar_pkey primary key using index
foobar_pkey;

ERROR:  index "foobar_pkey" does not have default sorting behavior
LINE 1: alter table foobar add constraint foobar_pkey primary key us...
^
DETAIL:  Cannot create a primary key or unique constraint using such an index.
Time: 1.577 ms


If I instead define the table as
create table foobar (x int, w xml);

Then I can create the index and then the primary key the first time I
do this in a session.  But then if I drop the table and repeat the
process, I get "does not have default sorting behavior" error even for
this index that previously succeeded, so I think there is some kind of
problem with the backend syscache or catcache.

create table foobar (x int, w xml);
create unique index foobar_pkey on foobar (x) including (w);
alter table foobar add constraint foobar_pkey primary key using index
foobar_pkey;
drop table foobar ;
create table foobar (x int, w xml);
create unique index foobar_pkey on foobar (x) including (w);
alter table foobar add constraint foobar_pkey primary key using index
foobar_pkey;
ERROR:  index "foobar_pkey" does not have default sorting behavior
LINE 1: alter table foobar add constraint foobar_pkey primary key us...
^
DETAIL:  Cannot create a primary key or unique constraint using such an index.


Great, I've fixed that. Thank you for the tip about cache.

I've also found and fixed related bug in copying tables with indexes:
create table tbl2 (like tbl including all);
And there's one more tiny fix in get_pkey_attnames in dblink module.

including_columns_3.0 is the latest version of patch.
And changes regarding the previous version are attached in a separate 
patch. Just to ease the review and debug.


I've changed size of pg_index.indclass array. It contains indnkeyatts 
elements now.
While pg_index.indkey still contains all attributes. And this query 
Retrieve primary key columns 
 provides 
pretty non-obvious result. Is it a normal behavior here or some changes 
are required? Do you know any similar queries?


--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index 9c8e308..ef6aee5 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -2058,7 +2058,7 @@ get_pkey_attnames(Relation rel, int16 *numatts)
 		/* we're only interested if it is the primary key */
 		if (index->indisprimary)
 		{
-			*numatts = index->indnatts;
+			*numatts = index->indnkeyatts;
 			if (*numatts > 0)
 			{
 result = (char **) palloc(*numatts * sizeof(char *));
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 412c845..c201f8b 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -3515,6 +3515,14 @@
   pg_class.relnatts)
  
 
+  
+  indnkeyatts
+  int2
+  
+  The number of key columns in the index. "Key columns" are ordinary
+  index columns in contrast with "included" columns.
+ 
+
  
   indisunique
   bool
diff --git a/doc/src/sgml/indexam.sgml b/doc/src/sgml/indexam.sgml
index 5f7befb..aaed5a7 100644
--- a/doc/src/sgml/indexam.sgml
+++ b/doc/src/sgml/indexam.sgml
@@ -111,6 +111,8 @@ typedef struct IndexAmRoutine
 boolamclusterable;
 /* does AM handle predicate locks? */
 boolampredlocks;
+/* does AM support columns included with clause INCLUDING? */
+boolamcaninclude;
 /* type of data stored in index, or InvalidOid if variable */
 Oid amkeytype;
 
@@ -852,7 +854,8 @@ amrestrpos (IndexScanDesc scan);
using unique indexes, which are indexes that disallow
multiple entries with identical keys.  An access method that supports this
feature sets amcanunique true.
-   (At present, only b-tree supports it.)
+   (At present, only b-tree supports it.) Columns which are present in the
+   INCLUDING clause are not used to enforce uniqueness.
   
 
   
diff --git a/doc/src/sgml/indices.sgml b/doc/src/sgml/indices.sgml
index 23bbec6..09d4e6b 100644
--- a/doc/src/sgml/indices.sgml
+++ b/doc/src/sgml/indices.sgml
@@ -633,7 +633,8 @@ CREATE INDEX test3_desc_index ON test3 (id DESC NULLS LAST);
Indexes can also be used to enforce uniqueness of a column's value,
or the uniqueness of the combined values of more than one column.
 
-CREATE UNIQUE INDEX name ON table (column , ...);
+CREATE UNIQUE INDEX name ON table (column , ...)
+INCLUDING (column , ...);
 
Currently, only B-tree

Re: [HACKERS] Releasing in September

2016-01-26 Thread Amit Kapila

On Tue, Jan 26, 2016 at 1:01 PM, Andres Freund  wrote:
> On 2016-01-26 00:26:18 -0600, Joshua Berkus wrote:
>
> > The alternative to this is an aggressive recruitment and mentorship
> > program to create more major contributors who can do deep review of
> > patches.  But that doesn't seem to have happened in the last 5 years,
> > and even if we started it now, it would be 2 years before it paid off.
>
> I think the amount of review and maintenance time is the largest part of
> the problem here, and is mostly unaffected by the manner we actually
> schedule releases.  The discussions around dropping patches on the floor
> are partially a question of that, and partially a question of not
> acceppting to maintain things that are of low interest.
>
> But I don't really see "aggressive recruitment and mentorship" really
> fixing this, even though there are other good reasons to work more on
> that side.  I think more fundamentally the issue is that that doing a
> "deep" review of a bigger patches takes so much time and knowledge, that
> it's not realistic to do so in ones own time anymore. You can if you're
> very dedicated over a long time, but realistically in most cases it
> requires your employer having an interest in you doing that.  Quite
> obviously there's an increasing number of employers paying people to
> submit things to postgres, whereas there seems no corresponding uptick
> on the review side.
>

Yeah, I also share the same feeling, but I think there is more to it, even
if
the employer is ready to support for the review of a patch which takes more
amount of time, the credit to reviewers is always on much lower side as
compare to the original Author even though the effort put by reviewer is
comparable to Author especially for somewhat complex patches.



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

Re: [HACKERS] pglogical - logical replication contrib module

On 26 January 2016 at 20:33, leo  wrote:

> Hi Steve Singer,
>
>I find the pglogical package has updated, I reinstall the new RPM
> package
> and test again. But I find the same error in subscription node after I run
> pglogical.create_subscription command:
>
>
Please don't side-track threads about patch review and development with
requests for support of a released version.

This thread is about getting pglogical into PostgreSQL 9.6 core, rather
than the separately released product that's been released to support 9.4
and 9.5.


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

Re: [HACKERS] Improve tab completion for REFRESH MATERIALIZED VIEW

2016-01-26 Thread Kevin Grittner

On Tue, Jan 26, 2016 at 8:24 AM, Fujii Masao  wrote:
> On Tue, Jan 26, 2016 at 11:08 PM, Fujii Masao  wrote:

>> While testing the patch, I found that
>> REFRESH MATERIALIZED VIEW  doesn't list the materialized views.
>
> This is not true. Sorry for the noise...

But CREATE MATERIALIZED VIEW  doesn't list existing matviews.
I'm not clear on why it's a good idea to do so, but most kinds of
objects (including tables, views, and indexes) do so; so it seems
best to follow suit here.  I will push something shortly with the
improvements from both of you, plus a couple other MV tab
completion issues I found in testing these patches.

-- 
Kevin Grittner
EDB: 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] pglogical most basic setup for logical replication

On 26 January 2016 at 19:18, Sebastien Diemer <
sebastien.die...@polyconseil.fr> wrote:

> It sounds like you must be running on PostgreSQL 9.4.
>
> Indeed I am on PostgreSQL 9.4, I omitted this important point.
>
>
I'll update the docs to mention the extra step on 9.4.

> You'll need to drop the subscriber database and re-create it. Use a new
>> node name.
>
> Seems to work but I still do not really understand what was wrong in the
> first place though.
> Thanks for your help Craig !
>

It's because internally pglogical relies on pg_dump and pg_restore to set
up the initial database state. If it fails partway through a pg_restore
there's no sensible way to undo those changes, especially since we have to
do it in a number of steps to achieve a partial restore.

A cleaner handling of this really requires the ability to use pg_dump's
functionality over a normal protocol connection from SQL, or at least as a
library.

The error should probably get a HINT to say that an incomplete restore
means the DB may have to be dropped and recreated.

It's all a bit messy, since we support multiple upstreams and support
writes directly into the downstream. So you actually can't always just drop
the downstream. A really good solution to this needs the ability to compare
various objects (type definitions, tables, etc), the ability to extract
just a required subset of dependencies from a database and other
complexities.

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

Re: [HACKERS] Improve tab completion for REFRESH MATERIALIZED VIEW

2016-01-26 Thread Kevin Grittner

On Tue, Jan 26, 2016 at 8:43 AM, Kevin Grittner  wrote:

> I will push something shortly with the
> improvements from both of you, plus a couple other MV tab
> completion issues I found in testing these patches.

Done.

-- 
Kevin Grittner
EDB: 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] Proposal:Use PGDLLEXPORT for libpq

2016-01-26 Thread Yury Zhuravlev


Hello hackers.
Why we do not use PGDLLEXPORT (__declspec (dllexport)) for the libpq (and 
other dlls)?
Now we are using black magic with gendef.pl for get def (aka export.txt) 
from .obj files.

I have set for some functions and it works.
Perhaps there is a circumstance that I do not know but my version seems to 
me simplify build under Windows.


Thanks. 
--

Yury Zhuravlev
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


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

Re: [HACKERS] Batch update of indexes

2016-01-26 Thread Konstantin Knizhnik

Hi hackers,

I have implemented "ALTER INDEX ... WHERE ..." clause allowing to change
condition for partial index.

Actually it allows us to append index without fully rebuilding it.
As I explained in the previous mails, partial indexes can be used to
increase insert speed.

Right now I get the following results (with one insert stream):

Insert with 1 index (primary key, monotonically ascending):
324275 TPS
Insert with 9 indexes (primary key + 8 indexes with random keys):
52495 TPS
Insert with primary key and 8 concurrently updated partial indexes:
194458 TPS
Insert with primary key and 8 "frozen" partial
indexes: 278446 TPS

So, as you can see insert with indexes is about 6 times slower than
insert without indexes.

And partial indexes allows to eliminate this gap.
When partial indexes are not affected (assuming that them will be
reconstructed "at night"),

performance is almost the same, as without indexes.
And if "ALTER INDEX" is done concurrently with inserts, it certainly
decrease insert speed,

but still it is 4 times faster than with normal indexes.

Such high TPS values were obtained using "insert from select" to bypass
libpq overhead.
With libpq (when each insert is sent as independent statement) results
are less impressive:

Insert with 1 index (primary key, monotonically ascending):
37892 TPS
Insert with 9 indexes (primary key + 8 indexes with random keys):
20231 TPS
Insert with primary key and 8 concurrently updated partial indexes:
26934 TPS
Insert with primary key and 8 "frozen" partial
indexes: 28863 TPS

But still partial indexes allows to almost eliminate two times
differences...

This results can be reproduced using our public repository:
https://github.com/postgrespro/postgres_cluster

Most of the code related with support of "ALTER INDEX .. WHERE" is in
AlterIndex function in

postgres_cluster/src/backend/commands/indexcmds.c
I have also added insbench utility for measuring insert performance,
using which this results were obtained.

It is located in postgres_cluster/src/bin/insbench directory.

Known issues:
1. I do not handle case when new condition for partial index is more
restricted than original.
There is no way in Postgres to exclude records from index (except
VACUUM), so in this case index has to be reconstructed from scratch.
2. Currently I am using SPI to locate records which should be included
in index.
3. I am not completely sure that there are no
synchronization/isolation problems in AlterIndex function

If this approach is considered to be interesting by community, I will
try to address these issues.

On 20.01.2016 12:28, Konstantin Knizhnik wrote:

Hi hackers,

I want to know opinion of community about possible ways of solving
quite common problem: increasing insert speed while still providing
indexes for efficient execution of queries.

Many applications have to deal with high input stream of data. Most of
the time while record inserting in the database is taken for update of
indexes. And without indexes we are not able to efficiently execute
queries.
So in many cases it is desirable to have "batch or concurrent" index
update. And it is acceptable that an index is slightly behind current
state of the table.

One interesting approach of solving this problem is discussed in this
article:

https://mark.zealey.org/2016/01/08/how-we-tweaked-postgres-upsert-performance-to-be-2-3-faster-than-mongodb

Them are using materialized views to build indexes in background.
Interesting idea, but copying content of the whole table just to be
able to build index concurrently seems to be overkill.

I thought about more straightforward ways of solving this problem. It
will be nice if we can preserve of of them main postulates of Postgres
and other RDBMSes: indexes are just optimization and result of query
should not depend on presence of indexes.

First idea is to use inheritance. I have investigated different ways
of splitting table into "archival" and "operational" parts, but all of
them requiring physical copying of data from one table to another.

Another idea is to use partial indexes
(http://www.postgresql.org/docs/current/static/indexes-partial.html)
Assume that we have stream of input data where each record have
increased timestamp:

create table t(
ts timestamp primary key,
c1 real,
c2 integer,
c3 varchar,
...
cN char(5)
);

We want to provide the highest insert speed for "t" but provide
indexes for c1..cN fields.

We can declared partial indexes:

create index idx1 on t(c1) where ts < '20/01/2016';
create index idx2 on t(c2) where ts < '20/01/2016';
...
create index idxN on t(cN) where ts < '20/01/2016';

As far as this indexes do not cover current date, them will not be
affected during insert operations.

But we can still efficiently run queries like

select * from t where c1>100 and ts <

Re: [HACKERS] pglogical - logical replication contrib module

On 23 January 2016 at 11:17, Steve Singer  wrote:

>
> 2) Does this patch provide a set of logical replication features that meet
> many popular use-cases
>
> Below I will review some use-cases and try to assess how pglogical meets
> them.
>
>  ** Streaming Postgresql Upgrade
>
> pg_upgrade is great for many situations but sometimes you don't want an in
> place upgrade but you want a streaming upgrade.  Possibly because you don't
> want application downtime but instead you just want to point your
> applications at the upgraded database server in a controlled manner.
>  Othertimes you
> might want an option of upgrading to a newer version of PG but maintain
> the option of having to rollback to the older version if things go badly.
>
> I think pglogical should be able to handle this use case pretty well
> (assuming the source version of PG is actually new enough to include
> pglogical).
>

Yep, it's designed significantly for that case.  That's also why support
for 9.4 and 9.5 is maintained as a standalone extension, so you can get
data out of 9.4 and 9.5 easily (and for that matter, upgrade 9.4 to 9.5).

> Support for replicating sequences would need to be added before this is as
> smooth but once sequence support was added I think this would work well.
>

This will unfortunately have to be 9.6 only. We can work around it with
some limitations in a pglogical downstream in older versions, but I really
want to get time to write a v2 of the sequence decoding patch so I can get
that into 9.6.

> ** Query only replicas (with temp tables or additional indexes)
>
> Sometimes you want a replica for long running or heavy queries.
> Requirements for temp tables, additional indexes or maybe the effect on
> vacuum means that our existing WAL based replicas are unsuitable.
>
> I think pglogical should be able to handle this use case pretty well with
> the caveat being that your replica is an asynchronous replica and will
> always lag the origin by some amount.
>

You can actually run it as a synchronous replica too, with the usual
limitations that you can have only one synchronous standby at a time, etc.
Or should be able to - I haven't had a chance to write proper tests for
sync rep using pglogical yet.

Performance will currently hurt if you do big xacts. That's why we need
interleaved xact streaming support down the track.

> Pglogical doesn't have any facilities to rename the tables between the
> origin and replica but they could be added later.
>

Yep, we could do that with a hook. You couldn't use initial schema sync if
you did that, of course.

> ** Sharding
>
> Systems that do application level sharding (or even sharding with a fdw)
> often have non-sharded tables that need to be available on all shards for
> relational integrity or joins.   Logical replication is one way to make
> sure that the replicated data gets to all the shards.  Sharding systems
> also sometimes want
> to take the data from individual shards and replicate it to a
> consolidation server for reporting purposes.
>
> Pglogical seems to meet this use case, I guess you would have a designated
> origin for the shared data/global data that all shards would subscribe to
> with a set containing the designated data.  For the consolidation use case
> you would have the consolidation server subscribe to all shards
>
> I am less clear about how someone would want DDL changes to work for these
> cases.  The DDL support in the patch is pretty limited so I am not going to
> think much now about how we would want DDL to work.
>

DDL support is "version 2" material, basically.

9.5 has hooks that allow DDL deparsing to be implemented as an extension.
That extension needs to be finished off (there's some work-in-progress code
floating around from 9.5 dev) and needs to expose an API for other
extensions. Then pglogical can register hooks with the ddl deparse
extension and use that for DDL replication.

As we learned with BDR, though, DDL replication is *hard*.

For one thing PostgreSQL has global objects like users that we can't
currently capture DDL for, and then creates db-local objects that have
dependences on them. So you have to manually replicate the global objects
still. I can see some possible solutions for this, but nothing's really on
the horizon.

Additionally there a some operations that are a bit problematic for logical
replication. Full table rewrites being the main one - they clobber
replication origin information among other issues. We really need a way to
decode

ALTER TABLE blah ADD COLUMN fred integer NOT NULL DEFAULT 42;

as

BEGIN;
ALTER TABLE blah ADD COLUMN fred integer;
ALTER TABLE blah ALTER COLUMN fred DEFAULT 42;
UPDATE blah SET fred = 42;
ALTER TABLE blah ALTER COLUMN fred NOT NULL;
COMMIT;

which involves some "interesting" co-operation between DDL deparse and
logical replication. The mapping of the decoded full table rewrite to the
underlying table is a bit interesting; we just get a decode stream for a

Re: [HACKERS] Proposal:Use PGDLLEXPORT for libpq

On 26 January 2016 at 23:04, Yury Zhuravlev 
wrote:

> Hello hackers.
> Why we do not use PGDLLEXPORT (__declspec (dllexport)) for the libpq (and
> other dlls)?
> Now we are using black magic with gendef.pl for get def (aka export.txt)
> from .obj files.
> I have set for some functions and it works.
> Perhaps there is a circumstance that I do not know but my version seems to
> me simplify build under Windows.
>

See

http://www.postgresql.org/message-id/1389762012.24046.2.ca...@vanquo.pezone.net

and

http://www.postgresql.org/message-id/0lix1g-1vpi6o3ros-00d...@mrelayeu.kundenserver.de

(sorry for HTML horror)

for history.

TL;DR:  PGDLLEXPORT is considered ugly Windows-droppings in the code and so
a less visible, albeit more convoluted, solution is used.


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

Re: [HACKERS] remove wal_level archive

2016-01-26 Thread Simon Riggs

On 1 September 2015 at 03:39, Peter Eisentraut  wrote:

> - The distinction between wal_level settings "archive" and "hot_standby"
> is in the way of automation or better intelligence, because the primary
> cannot tell what the receiver intends to do with the WAL.
>
> So here is a patch to get rid of the distinction.
>
> Bike-shedding:  In this patch, I removed "archive" and kept
> "hot_standby", because that's what the previous discussions suggested.
> Historically and semantically, it would be more correct the other way
> around.  On the other hand, keeping "hot_standby" would probably require
> fewer configuration files to be changed.  Or we could keep both, but
> that would be confusing (for users and in the code).
>

I've reviewed this and have a few comments.

Removing one of "archive" or "hot standby" will just cause confusion and
breakage, so neither is a good choice for removal.

What we should do is
1. Map "archive" and "hot_standby" to one level with a new name that
indicates that it can be used for both/either backup or replication.
  (My suggested name for the new level is "replica"...)
2. Deprecate "archive" and "hot_standby" so that those will be removed in a
later release.

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

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

Re: [HACKERS] pg_lsn cast to/from int8

On 26 January 2016 at 22:07, Magnus Hagander  wrote:


> In this case, mostly legacy compatibility. Making an app that works with
> versions that don't have pg_lsn have a nice path forward to the modern
> world. Being able to cast from pg_lsn to int8 can also make it easier to
> work with the values in the client application, though I don't need that
> for this particular one.
>
>
Wouldn't we need a uint8 type for that?

I guess we could just show people negative LSNs if the high bit is set
(that being rather unlikely) but still...

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

Re: [HACKERS] WIP: Failover Slots

Hi all

Here's v2 of the failover slots patch. It replicates a logical slot to a
physical streaming replica downstream, keeping the slots in sync. After the
downstream is promoted a client can replay from the logical slot.

UI to allow creation of non-failover slots is pending.

There's more testing to do to cover all the corners: drop slots, drop and
re-create, name conflicts between downstream !failover slots and upstream
failover slots, etc.

There's also a known bug where WAL isn't correctly retained for a slot
where that slot was created before a basebackup which I'll fix in a
revision shortly.

I'm interested in ideas on how to better test this.

-- 
 Craig Ringer   http://www.2ndQuadrant.com/

 PostgreSQL Development, 24x7 Support, Training & Services
From c2535eb27c6efc516a6aa7142fd0f59e85d3 Mon Sep 17 00:00:00 2001
From: Craig Ringer 
Date: Wed, 20 Jan 2016 17:16:29 +0800
Subject: [PATCH 1/2] Implement failover slots

Originally replication slots were unique to a single node and weren't
recorded in WAL or replicated. A logical decoding client couldn't follow
a physical standby failover and promotion because the promoted replica
didn't have the original master's slots. The replica may not have
retained all required WAL and there was no way to create a new logical
slot and rewind it back to the point the logical client had replayed to
anyway.

Failover slots lift this limitation by replicating slots consistently to
physical standbys, keeping them up to date and using them in WAL
retention calculations. This allows a logical decoding client to follow
a physical failover and promotion without losing its place in the change
stream.

Simon Riggs and Craig Ringer

WIP. Open items:

* Testing
* Implement !failover slots and UI for marking slots as failover slots
* Fix WAL retention for slots created before a basebackup
---
 src/backend/access/rmgrdesc/Makefile   |   2 +-
 src/backend/access/rmgrdesc/replslotdesc.c |  63 +
 src/backend/access/transam/rmgr.c  |   1 +
 src/backend/commands/dbcommands.c  |   3 +
 src/backend/replication/basebackup.c   |  12 -
 src/backend/replication/logical/decode.c   |   1 +
 src/backend/replication/logical/logical.c  |  19 +-
 src/backend/replication/slot.c | 433 -
 src/backend/replication/slotfuncs.c|   1 +
 src/bin/pg_xlogdump/replslotdesc.c |   1 +
 src/bin/pg_xlogdump/rmgrdesc.c |   1 +
 src/include/access/rmgrlist.h  |   1 +
 src/include/replication/slot.h |  61 +---
 src/include/replication/slot_xlog.h| 103 +++
 14 files changed, 610 insertions(+), 92 deletions(-)
 create mode 100644 src/backend/access/rmgrdesc/replslotdesc.c
 create mode 12 src/bin/pg_xlogdump/replslotdesc.c
 create mode 100644 src/include/replication/slot_xlog.h

diff --git a/src/backend/access/rmgrdesc/Makefile b/src/backend/access/rmgrdesc/Makefile
index c72a1f2..600b544 100644
--- a/src/backend/access/rmgrdesc/Makefile
+++ b/src/backend/access/rmgrdesc/Makefile
@@ -10,7 +10,7 @@ include $(top_builddir)/src/Makefile.global
 
 OBJS = brindesc.o clogdesc.o committsdesc.o dbasedesc.o gindesc.o gistdesc.o \
 	   hashdesc.o heapdesc.o mxactdesc.o nbtdesc.o relmapdesc.o \
-	   replorigindesc.o seqdesc.o smgrdesc.o spgdesc.o \
+	   replorigindesc.o replslotdesc.o seqdesc.o smgrdesc.o spgdesc.o \
 	   standbydesc.o tblspcdesc.o xactdesc.o xlogdesc.o
 
 include $(top_srcdir)/src/backend/common.mk
diff --git a/src/backend/access/rmgrdesc/replslotdesc.c b/src/backend/access/rmgrdesc/replslotdesc.c
new file mode 100644
index 000..b882846
--- /dev/null
+++ b/src/backend/access/rmgrdesc/replslotdesc.c
@@ -0,0 +1,63 @@
+/*-
+ *
+ * replslotdesc.c
+ *	  rmgr descriptor routines for replication/slot.c
+ *
+ * Portions Copyright (c) 2015, PostgreSQL Global Development Group
+ *
+ *
+ * IDENTIFICATION
+ *	  src/backend/access/rmgrdesc/replslotdesc.c
+ *
+ *-
+ */
+#include "postgres.h"
+
+#include "replication/slot_xlog.h"
+
+void
+replslot_desc(StringInfo buf, XLogReaderState *record)
+{
+	char	   *rec = XLogRecGetData(record);
+	uint8		info = XLogRecGetInfo(record) & ~XLR_INFO_MASK;
+
+	switch (info)
+	{
+		case XLOG_REPLSLOT_UPDATE:
+			{
+ReplicationSlotInWAL xlrec;
+
+xlrec = (ReplicationSlotInWAL) rec;
+
+appendStringInfo(buf, "slot %s to xmin=%u, catmin=%u, restart_lsn="UINT64_FORMAT"@%u",
+		NameStr(xlrec->name), xlrec->xmin, xlrec->catalog_xmin,
+		xlrec->restart_lsn, xlrec->restart_tli);
+
+break;
+			}
+		case XLOG_REPLSLOT_DROP:
+			{
+xl_replslot_drop *xlrec;
+
+xlrec = (xl_replslot_drop *) rec;
+
+appendStringInfo(buf, "slot %s", NameStr(xlrec->name));
+
+break;
+			}
+	}
+}
+
+const char *

Re: [HACKERS] pgbench stats per script & other stuff

Fabien COELHO wrote:

> You know how delighted I am to split patches...

Yes, of course, it's the most interesting task in the world.  I'm fully
aware of that.

FWIW I'm going to apply a preliminary commit to pgindent-clean the file
before your patches, then apply each patch as pgindent-clean.  Otherwise
your whitespace style was getting too much on my nerves.

> b) refactor statistics collections (per thread, per command, per whatever)
>so as to use the same structure everywhere, reducing the CLOC by 115.
>this enables the next small patch which can reuse the new functions.

I'm not really sure about the fact that we operate on those Stats
structs without locking.  I see upthread you convinced Michael that it
was okay, but is it really?  How severe is the damage if two threads
happen to collide?

Why is this function defined like this?

/*
 * Initialize a StatsData struct to all zeroes, but the given
 * start_time, except that if it's exactly zero don't change it.
 */
static void
initStats(StatsData *sd, double start_time)
{
sd->cnt = 0;
sd->skipped = 0;
initSimpleStats(>latency);
initSimpleStats(>lag);

/* not necessarily overriden? */
if (start_time)
sd->start_time = start_time;
}

It seems a bit funny to have the start_time not be reset when 0.0 is
passed, which is almost all the callers.  Using a float as a boolean
looks pretty odd; is that kosher?  Maybe it'd be a good idea to have a
separate boolean flag instead?  Something like this

/*
 * Initialize a StatsData struct to all zeroes.  Use the given
 * start_time only if reset_start_time, otherwise keep the original
 * value.
 */
static void
initStats(StatsData *sd, double start_time, bool reset_start_time)
{
sd->cnt = 0;
sd->skipped = 0;
initSimpleStats(>latency);
initSimpleStats(>lag);

/* not necessarily overriden? */
if (reset_start_time)
sd->start_time = start_time;
}

I renamed a couple of your functionettes, for instance doSimpleStats to
addToSimpleStats and appendSimpleStats to mergeSimpleStats.

Haven't looked at patches c or d yet.  I'm tempted to thrown patch e in
with the initial pgindent run.

-- 
Á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] pgbench stats per script & other stuff

Fabien COELHO wrote:

> a) add -b option for cumulating builtins and rework internal script
>management so that builtin and external scripts are managed the
>same way.

I tweaked this a bit.  I found a bug in threadRun: it was reading the
commands first, and setting st->use_file later.  This led to the wrong
commands being read.

Some other less interesting changes:

* made chooseScript have the logic to react to single existing script;
no need to inject ternary operators in each caller to check for that
condition.

* Added a debug line every time a script is chosen,
+   if (debug)
+   fprintf(stderr, "client %d executing script \"%s\"\n", st->id,
+   sql_script[st->use_file].name);
  (I'd have liked to have chooseScript itself do it, but it doesn't
  have the script name handy.  Maybe this indicates that the data
  structures are slightly wrong.)

* Added a separate routine to list available scripts; originally that
was duplicated in "-b list" and when -b got an invalid script name.

* In usage(), I split out the options to select a script instead of
mixing them within "Benchmarking options"; also changed wording of
parenthical comment, no longer carrying the full list of scripts (a
choice which also omitted "-b list" itself):
+  "\nOptions to select what to run:\n"
+  "  -b, --builtin=NAME   add buitin script (use \"-b list\" to 
display\n"
+  "   available scripts)\n"
+  "  -f, --file=FILENAME  add transaction script from FILENAME\n"
+  "  -N, --skip-some-updates  skip updates of pgbench_tellers and 
pgbench_branches\n"
+  "   (same as \"-b simple-update\")\n"
+  "  -S, --select-onlyperform SELECT-only transactions\n"
+  "   (same as \"-b select-only\")\n"

I couldn't find a better heading to use there, so that'll have to do
unless someone has a better idea.

Some other trivial changes.  Patch attached.  I plan to push this as
soon as I'm able.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 541d17b..fdd3331 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -261,6 +261,23 @@ pgbench  options  dbname
 benchmarking arguments:
 
 
+ 
+  -b scriptname
+  --builtin scriptname
+  
+   
+Add the specified builtin script to the list of executed scripts.
+Available builtin scripts are: tpcb-like,
+simple-update and select-only.
+The provided scriptname needs only to be a prefix
+of the builtin name, hence simp would be enough to select
+simple-update.
+With special name list, show the list of builtin scripts
+and exit immediately.
+   
+  
+ 
+
 
  
   -c clients
@@ -307,14 +324,13 @@ pgbench  options  dbname
  
 
  
-  -f filename
-  --file=filename
+  -f filename
+  --file=filename
   

-Read transaction script from filename.
+Add a transaction script read from filename to
+the list of executed scripts.
 See below for details.
--N, -S, and -f
-are mutually exclusive.

   
  
@@ -404,10 +420,8 @@ pgbench  options  dbname
   --skip-some-updates
   

-Do not update pgbench_tellers and
-pgbench_branches.
-This will avoid update contention on these tables, but
-it makes the test case even less like TPC-B.
+Run builtin simple-update script.
+Shorthand for -b simple-update.

   
  
@@ -512,9 +526,9 @@ pgbench  options  dbname
 Report the specified scale factor in pgbench's
 output.  With the built-in tests, this is not necessary; the
 correct scale factor will be detected by counting the number of
-rows in the pgbench_branches table.  However, when testing
-custom benchmarks (-f option), the scale factor
-will be reported as 1 unless this option is used.
+rows in the pgbench_branches table.
+However, when testing only custom benchmarks (-f option),
+the scale factor will be reported as 1 unless this option is used.

   
  
@@ -524,7 +538,8 @@ pgbench  options  dbname
   --select-only
   

-Perform select-only transactions instead of TPC-B-like test.
+Run built-in select-only script.
+Shorthand for -b select-only.

   
  
@@ -674,7 +689,17 @@ pgbench  options  dbname
   What is the Transaction Actually Performed in pgbench?
 
   
-   The default transaction script issues seven commands per transaction:
+   Pgbench executes test scripts chosen randomly from a specified list.
+   They include built-in scripts

Re: [HACKERS] Proposal:Use PGDLLEXPORT for libpq

2016-01-26 Thread Yury Zhuravlev


Craig Ringer wrote:
TL;DR:  PGDLLEXPORT is considered ugly Windows-droppings in the 
code and so a less visible, albeit more convoluted, solution is 
used.


It says more about the modules, and not about libpq. Using gendef.pl for 
this library in the light of the development of my CMake build seems silly.


--
Yury Zhuravlev
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


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

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

Hi


>>> But in my opinion this discussion shouldn't really even be about
>>> catching these things, most of the times you won't catch them and
>>> instead you'll let them go to Postgres. The discussion should be
>>> whether raise plpy.Error(...), plpy.raise_error, plpy.raise_info(,,,)
>>> etc. all with keyword argument support are a good PLPython interface
>>> to Postgres' ereport. I think they are.
>>>
>>
I removed from previous patch all OOP related changes. New patch contains
raise_ functions only. This interface is new generation of previous
functions: info, notice, warning, error with keyword parameters interface.
I didn't changed older functions due keeping compatibility.

I spent lot of time with experiments how to merge PostgreSQL exception
system with Python exception system. But I didn't find a solution without
some design issues. These concepts(s) are too different (note: there are
two concepts: PostgreSQL levels and ANSI SQL SQLSTATE (class, subclass
codes).

I hope so raise_ has benefit for PLPythonu users too. Currently users
has to use ugly workaround to raise rich PostgreSQL exception from Python.
With new function all available functionality related to PostgreSQL
exception can be used simply.

plpy.raise_warning("some doesn't work", detail = "user registration ... ")

This patch is reduced previous patch (no new features, no different
solution, no different code), so I use same entry in commitfest.

Regards

Pavel
diff --git a/doc/src/sgml/plpython.sgml b/doc/src/sgml/plpython.sgml
new file mode 100644
index 015bbad..705413e
*** a/doc/src/sgml/plpython.sgml
--- b/doc/src/sgml/plpython.sgml
*** $$ LANGUAGE plpythonu;
*** 1367,1372 
--- 1367,1401 

  

+The plpy module also provides the functions
+plpy.raise_debug(args),
+plpy.raise_log(args),
+plpy.raise_info(args),
+plpy.raise_notice(args),
+plpy.raise_warning(args),
+plpy.raise_error(args), and
+plpy.raise_fatal(args).elogin PL/Python
+plpy.raise_error and
+plpy.raise_fatal actually raise a Python exception
+which, if uncaught, propagates out to the calling query, causing
+the current transaction or subtransaction to be aborted.
+raise plpy.Error(msg) and
+raise plpy.Fatal(msg) are
+equivalent to calling
+plpy.raise_error and
+plpy.raise_fatal, respectively.
+The other functions only generate messages of different
+priority levels.
+Whether messages of a particular priority are reported to the client,
+written to the server log, or both is controlled by the
+ and
+ configuration
+variables. See  for more information.
+These functions allows to using keyword parameters:
+[ message [, detail [, hint [, sqlstate  [, schema  [, table  [, column  [, datatype  [, constraint ].
+   
+ 
+   
 Another set of utility functions are
 plpy.quote_literal(string),
 plpy.quote_nullable(string), and
diff --git a/src/pl/plpython/expected/plpython_test.out b/src/pl/plpython/expected/plpython_test.out
new file mode 100644
index 7b76faf..834b14a
*** a/src/pl/plpython/expected/plpython_test.out
--- b/src/pl/plpython/expected/plpython_test.out
*** contents.sort()
*** 43,51 
  return ", ".join(contents)
  $$ LANGUAGE plpythonu;
  select module_contents();
!module_contents
! --
!  Error, Fatal, SPIError, cursor, debug, error, execute, fatal, info, log, notice, prepare, quote_ident, quote_literal, quote_nullable, spiexceptions, subtransaction, warning
  (1 row)
  
  CREATE FUNCTION elog_test() RETURNS void
--- 43,51 
  return ", ".join(contents)
  $$ LANGUAGE plpythonu;
  select module_contents();
!  module_contents 
! -
!  Error, Fatal, SPIError, cursor, debug, error, execute, fatal, info, log, notice, prepare, quote_ident, quote_literal, quote_nullable, raise_debug, raise_error, raise_fatal, raise_info, raise_log, raise_notice, raise_warning, spiexceptions, subtransaction, warning
  (1 row)
  
  CREATE FUNCTION elog_test() RETURNS void
*** CONTEXT:  Traceback (most recent call la
*** 72,74 
--- 72,111 
PL/Python function

Re: [HACKERS] Proposal:Use PGDLLEXPORT for libpq

On 27 January 2016 at 00:16, Yury Zhuravlev 
wrote:


> It says more about the modules, and not about libpq. Using gendef.pl for
> this library in the light of the development of my CMake build seems silly.
>
>
For what it's worth I personally agree. I'd rather have PGDLLEXPORT used
directly, not least because it'd let us built with -fvisibility=hidden
under *nix. But I'm in the minority and not inclined to push the point.

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

Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-01-26 Thread Corey Huinker

On Tue, Jan 26, 2016 at 7:53 AM, Michael Paquier 
wrote:

> On Tue, Jan 26, 2016 at 7:00 PM, Torsten Zuehlsdorff
>  wrote:
> >
> > On 26.01.2016 07:52, Simon Riggs wrote:
> >
> >>> Imagine for example a script that in some rare cases passes happens to
> >>> pass infinity into generate_series() - in that case I'd much rather
> error
> >>> out than wait till the end of the universe.
> >>>
> >>> So +1 from me to checking for infinity.
> >>>
> >>
> >> +1
> >>
> >> ERROR infinite result sets are not supported, yet
> >
> >
> > Maybe we should skip the "yet". Or do we really plan to support them in
> > (infinite) future? ;)
> >
> > +1 from me to check infinity also.
>
> Something like the patch attached would be fine? This wins a backpatch
> because the query continuously running eats memory, no?
> --
> Michael
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>
I'll modify my generate_series_date to give similar errors.

I have no opinion on backpatch.

+1 for flippant references to infinity.

Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-01-26 Thread David Fetter

On Tue, Jan 26, 2016 at 09:53:26PM +0900, Michael Paquier wrote:
> On Tue, Jan 26, 2016 at 7:00 PM, Torsten Zuehlsdorff
>  wrote:
> >
> > On 26.01.2016 07:52, Simon Riggs wrote:
> >
> >>> Imagine for example a script that in some rare cases passes
> >>> happens to pass infinity into generate_series() - in that case
> >>> I'd much rather error out than wait till the end of the
> >>> universe.
> >>>
> >>> So +1 from me to checking for infinity.
> >>>
> >>
> >> +1
> >>
> >> ERROR infinite result sets are not supported, yet
> >
> >
> > Maybe we should skip the "yet". Or do we really plan to support
> > them in (infinite) future? ;)
> >
> > +1 from me to check infinity also.
> 
> Something like the patch attached would be fine? This wins a
> backpatch because the query continuously running eats memory, no?

+1 for back-patching.  There's literally no case where an infinite
input could be correct as the start or end of an interval for
generate_series.

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


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

Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-01-26 Thread Euler Taveira

On 26-01-2016 09:53, Michael Paquier wrote:
> Something like the patch attached would be fine? This wins a backpatch
> because the query continuously running eats memory, no?
> 
+1. Although it breaks compatibility, a function that just eats
resources is not correct.


-- 
   Euler Taveira   Timbira - http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento


-- 
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] remove wal_level archive

2016-01-26 Thread Euler Taveira

On 26-01-2016 12:56, Simon Riggs wrote:
> Removing one of "archive" or "hot standby" will just cause confusion and
> breakage, so neither is a good choice for removal.
> 
Agree.

> What we should do is 
> 1. Map "archive" and "hot_standby" to one level with a new name that
> indicates that it can be used for both/either backup or replication.
>   (My suggested name for the new level is "replica"...)
> 2. Deprecate "archive" and "hot_standby" so that those will be removed
> in a later release.
> 
3. Add a WARNING at startup (until removal of values) saying something
like "'archive' value is deprecated and will be removed in a future
version. Use 'replica' value instead."


-- 
   Euler Taveira   Timbira - http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento


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

Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)

2016-01-26 Thread Corey Huinker

Revised patch:

Differences in this patch vs my first one:
- infinite bounds generate errors identical to Michael's timestamp patch
(though I did like Simon's highly optimistic error message).
- Bounds checking moved before memory context allocation
- arg variable "finish" renamed "stop" to match parameter name in
documentation and error message

On Tue, Jan 26, 2016 at 12:47 PM, Corey Huinker 
wrote:

> On Tue, Jan 26, 2016 at 7:53 AM, Michael Paquier <
> michael.paqu...@gmail.com> wrote:
>
>> On Tue, Jan 26, 2016 at 7:00 PM, Torsten Zuehlsdorff
>>  wrote:
>> >
>> > On 26.01.2016 07:52, Simon Riggs wrote:
>> >
>> >>> Imagine for example a script that in some rare cases passes happens to
>> >>> pass infinity into generate_series() - in that case I'd much rather
>> error
>> >>> out than wait till the end of the universe.
>> >>>
>> >>> So +1 from me to checking for infinity.
>> >>>
>> >>
>> >> +1
>> >>
>> >> ERROR infinite result sets are not supported, yet
>> >
>> >
>> > Maybe we should skip the "yet". Or do we really plan to support them in
>> > (infinite) future? ;)
>> >
>> > +1 from me to check infinity also.
>>
>> Something like the patch attached would be fine? This wins a backpatch
>> because the query continuously running eats memory, no?
>> --
>> Michael
>>
>>
>> --
>> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgsql-hackers
>>
>>
> I'll modify my generate_series_date to give similar errors.
>
> I have no opinion on backpatch.
>
> +1 for flippant references to infinity.
>
>


v2.generate_series_date.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] Why format() adds double quote?

2016-01-26 Thread Daniel Verite

Tatsuo Ishii wrote:

> IMO, it's a bug or at least an inconsistency

Personally I don't see this change being good for everything.

Let's play devil's advocate:

create table abc(U&"foo\2003" int);

U+2003 is 'EM SPACE', in Unicode's General Punctuation block.

With the current version, format('%I', attname) on this column is:
"foo "

With the patched version, it produces this:
foo 

So the visual hint that there are more characters at the end is lost.

Best regards,
-- 
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite


-- 
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] Speedup twophase transactions

Stas Kelvich wrote:

> While this patch touches quite sensible part of postgres replay and there is 
> some rarely used code paths, I wrote shell script to setup master/slave 
> replication and test different failure scenarios that can happened with 
> instances. Attaching this file to show test scenarios that I have tested and 
> more importantly to show what I didn’t tested. Particularly I failed to 
> reproduce situation where StandbyTransactionIdIsPrepared() is called, may be 
> somebody can suggest way how to force it’s usage. Also I’m not too sure about 
> necessity of calling cache invalidation callbacks during 
> XlogRedoFinishPrepared(), I’ve marked this place in patch with 2REVIEWER 
> comment.

I think this is the third thread in which I say this: We need to push
Michael Paquier's recovery test framework, then convert your test script
to that.  That way we can put your tests in core.

-- 
Á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] Add generate_series(date,date) and generate_series(date,date,integer)

Simon Riggs wrote:
> On 25 January 2016 at 09:55, Tomas Vondra 
> wrote:
> 
> 
> > Imagine for example a script that in some rare cases passes happens to
> > pass infinity into generate_series() - in that case I'd much rather error
> > out than wait till the end of the universe.
> >
> > So +1 from me to checking for infinity.
> 
> +1
> 
> ERROR infinite result sets are not supported, yet

In the future we may get lazy evaluation of functions.  When and if that
happens we may want to remove this limitation so that you can get a
streaming result producing timestamp values continuously.

(I don't necessarily think you'd use generate_series in such cases.
Maybe you'd want clock_timestamp to generate the present value at each
call, for example.  But there may be uses for synthetic values as well.)

-- 
Á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] Patch: ResourceOwner optimization for tables with many partitions

2016-01-26 Thread Tom Lane

Aleksander Alekseev  writes:
>> Um, that's not too surprising, because they're exactly the same patch?

> Wrong diff. Here is correct one. 

This still had quite a few bugs, but I fixed them (hope I caught
everything) and pushed it.

I did some performance testing of the ResourceArray code in isolation.
It appears that you need 100 or so elements before the hash code path
is competitive with the array path, so I increased the cutover point
quite a bit from what you had.

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] Speedup twophase transactions

2016-01-26 Thread Stas Kelvich

Agree, I had the same idea in my mind when was writing that script.

I will migrate it to TAP suite and write a review for Michael Paquier's patch.

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


> On 26 Jan 2016, at 20:20, Alvaro Herrera  wrote:
> 
> Stas Kelvich wrote:
> 
>> While this patch touches quite sensible part of postgres replay and there is 
>> some rarely used code paths, I wrote shell script to setup master/slave 
>> replication and test different failure scenarios that can happened with 
>> instances. Attaching this file to show test scenarios that I have tested and 
>> more importantly to show what I didn’t tested. Particularly I failed to 
>> reproduce situation where StandbyTransactionIdIsPrepared() is called, may be 
>> somebody can suggest way how to force it’s usage. Also I’m not too sure 
>> about necessity of calling cache invalidation callbacks during 
>> XlogRedoFinishPrepared(), I’ve marked this place in patch with 2REVIEWER 
>> comment.
> 
> I think this is the third thread in which I say this: We need to push
> Michael Paquier's recovery test framework, then convert your test script
> to that.  That way we can put your tests in core.
> 
> -- 
> Á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] Why format() adds double quote?

2016-01-26 Thread Dickson S. Guedes

2016-01-26 5:29 GMT-02:00 Tatsuo Ishii :
>
> I assume you used UTF-8 encoding database.

Yes, I do.


-- 
Dickson S. Guedes
mail/xmpp: gue...@guedesoft.net - skype: guediz
http://github.com/guedes - http://guedesoft.net
http://www.postgresql.org.br


-- 
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 format() adds double quote?

2016-01-26 Thread Dickson S. Guedes

2016-01-26 18:00 GMT-02:00 Daniel Verite :
> ...
> create table abc(U&"foo\2003" int);
>
> U+2003 is 'EM SPACE', in Unicode's General Punctuation block.
>
> With the current version, format('%I', attname) on this column is:
> "foo "
>
> With the patched version, it produces this:
> foo
>
> So the visual hint that there are more characters at the end is lost.


Thanks for advocate, I see here that it even produces that output with
simple spaces.

postgres=# create table x ("aí  " text);
CREATE TABLE
postgres=# \d x
Tabela "public.x"
  Coluna  | Tipo | Modificadores
--+--+---
 aí   | text |


This will break copy user actions and scripts that parses that output.

Maybe the patch should consider left/right non-printable chars to
choose whether to show or not the " ?

[]s
-- 
Dickson S. Guedes
mail/xmpp: gue...@guedesoft.net - skype: guediz
http://github.com/guedes - http://guedesoft.net
http://www.postgresql.org.br


-- 
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] pgbench stats per script & other stuff

Fabien COELHO wrote:

> a) add -b option for cumulating builtins and rework internal script
>management so that builtin and external scripts are managed the
>same way.

I'm uncomfortable with the prefix-matching aspect of -b.  It makes
"-b s" ambiguous -- whether it stands for select-only or simple-update
is merely a matter of what comes earlier in the table, which doesn't
seem reasonable to me.  I'd rather have a real way to reject ambiguous
cases, or simply accept only complete spellings.  This is the guilty
party:

> +static char *
> +find_builtin(const char *name, char **desc)
> +{
> + int len = strlen(name), i;
> +
> + for (i = 0; i < N_BUILTIN; i++)
> + {
> + if (strncmp(builtin_script[i].name, name, len) == 0)
> + {
> + *desc = builtin_script[i].desc;
> + return builtin_script[i].script;
> + }
> + }

I'm going to change this to use strlen(builtin_script[i].name) instead
of "len" here.

If you want to implement real non-ambiguous-prefix code (i.e. have "se"
for "select-only", but reject "s" as ambiguous) be my guest.

-- 
Á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] Speedup twophase transactions

On Wed, Jan 27, 2016 at 5:39 AM, Stas Kelvich  wrote:
> Agree, I had the same idea in my mind when was writing that script.
> I will migrate it to TAP suite and write a review for Michael Paquier's patch.

Yeah, please! And you have won a free-hug coupon that I can give in
person next week.
-- 
Michael


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

Re: [HACKERS] Why format() adds double quote?

> Thanks for advocate, I see here that it even produces that output with
> simple spaces.
> 
> postgres=# create table x ("aí  " text);
> CREATE TABLE
> postgres=# \d x
> Tabela "public.x"
>   Coluna  | Tipo | Modificadores
> --+--+---
>  aí   | text |
> 
> 
> This will break copy user actions and scripts that parses that output.
> 
> Maybe the patch should consider left/right non-printable chars to
> choose whether to show or not the " ?

This is a totally different story from the topic discussed in this
thread. psql never adds double quotations to column name even with
upper case col names.

test=# create table t6("ABC" int);
CREATE TABLE
test=# \d t6
  Table "public.t6"
 Column |  Type   | Modifiers 
+-+---
 ABC| integer | 

If you want to change the existing psql's behavior, propose it
yourself.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp


-- 
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 format() adds double quote?

>> IMO, it's a bug or at least an inconsistency
> 
> Personally I don't see this change being good for everything.
> 
> Let's play devil's advocate:
> 
> create table abc(U&"foo\2003" int);
> 
> U+2003 is 'EM SPACE', in Unicode's General Punctuation block.
> 
> With the current version, format('%I', attname) on this column is:
> "foo "
> 
> With the patched version, it produces this:
> foo 
> 
> So the visual hint that there are more characters at the end is lost.

What is the "visual hint"? If you are talking about psql's output, it
never adds "visual hint" (double quotations).

If you are talking about the string handling in a program, what kind
of program cares about "visiual"?

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

-- 
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 pg_size_pretty is volatile?

On Tue, Jan 26, 2016 at 3:00 PM, Pavel Stehule  wrote:
>
>
> 2016-01-26 2:00 GMT+01:00 Michael Paquier :
>>
>> On Tue, Jan 26, 2016 at 5:35 AM, Pavel Stehule 
>> wrote:
>> > Vitaly Burovoy pointed on bug in my patch - a pg_size_bytes was VOLATILE
>> > function. It is copy/paste bug - I used pg_size_pretty definition, so
>> > the
>> > question is: why pg_size_pretty is volatile? It should be immutable too.
>>
>> +1. This function relies only on the input of its argument to generate a
>> result.
>
>
> attached patch
>
> all tests passed

Pushed. Thanks!

Regards,

-- 
Fujii Masao


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

[HACKERS] Does pglogical support PG 9.4.5?

2016-01-26 Thread leo

Hi all,

I am test pglogical on PG 9.4.5, but I am not successfully even if I
install the latest version "postgresql94-pglogical-1.0.1-2".
Is there any one running pglogical on PG 9.4.5 successfully?
I describe my operation step, please help me to trouble shooting?

provider node: 10.10.10.11
subscriber node: 10.10.10.103
  
Config pg_hba.conf on provider node:

hostall postgres0.0.0.0/0 trust 
hostreplication postgres10.10.10.103/0  trust  
 
Config pg_hba.conf on subscriber node:
 
hostall postgres0.0.0.0/0 trust
hostreplication postgres10.10.10.11/0  trust  

Config the postgres.conf according to README


1. Create subscribe node: 
Provider: 10.10.10.11
 SELECT pglogical.create_node(node_name := 'subscriber_test_3',dsn :=
'hostaddr= 10.10.10.103 port=5509 dbname=testdb user=postgres');
 create_node 
-
  2393536845

2. Create replication ser 
 select pglogical.create_replication_set('replicate_gis_data', true, true,
true, true);
 create_replication_set 

 2904604760

3. Add table into replication set

select
pglogical.replication_set_add_table('replicate_gis_data','test_table',
true);


===Executing on subscriber node
Enable the network connection on:
  enbale network connection and replication connection without password 


1. Create provider: 
SELECT pglogical.create_node(node_name := 'provider_test_2',dsn :=
'hostaddr= 10.10.10.11 port=5508 dbname=testdb user=postgres');
create_node 
-
  3082486013


2. Create subscription to provider noede, executing on subscribe node

select pglogical.create_subscription('replicate_gis_data_from_11',
'hostaddr= 10.10.10.11 port=5508 dbname=testdb
user=postgres',E'{"replicate_gis_data"}');



Error Message on subscriber node
===

< 2016-01-12 19:45:41.396 UTC >DEBUG:  shmem_exit(1): 2 before_shmem_exit
callbacks to make
< 2016-01-12 19:45:41.396 UTC >DEBUG:  shmem_exit(1): 6 on_shmem_exit
callbacks to make
< 2016-01-12 19:45:41.396 UTC >DEBUG:  proc_exit(1): 2 callbacks to make
< 2016-01-12 19:45:41.396 UTC >DEBUG:  exit(1)
< 2016-01-12 19:45:41.396 UTC >DEBUG:  shmem_exit(-1): 0 before_shmem_exit
callbacks to make
< 2016-01-12 19:45:41.396 UTC >DEBUG:  shmem_exit(-1): 0 on_shmem_exit
callbacks to make
< 2016-01-12 19:45:41.396 UTC >DEBUG:  proc_exit(-1): 0 callbacks to make
< 2016-01-12 19:45:41.397 UTC >DEBUG:  reaping dead processes
< 2016-01-12 19:45:41.397 UTC >LOG:  worker process: pglogical apply
26429:2377587811 (PID 967) exited with exit code 1
< 2016-01-12 19:45:41.397 UTC >DEBUG:  StartTransaction
< 2016-01-12 19:45:41.397 UTC >LOG:  unregistering background worker
"pglogical apply 26429:2377587811"
< 2016-01-12 19:45:41.397 UTC >DEBUG:  name: unnamed; blockState:  
DEFAULT; state: INPROGR, xid/subid/cid: 0/1/0, nestlvl: 1, children: 
< 2016-01-12 19:45:41.397 UTC >LOG:  registering background worker
"pglogical apply 26429:2377587811"
< 2016-01-12 19:45:41.397 UTC >LOG:  starting background worker process
"pglogical apply 26429:2377587811"
< 2016-01-12 19:45:41.397 UTC >DEBUG:  CommitTransaction
< 2016-01-12 19:45:41.397 UTC >DEBUG:  name: unnamed; blockState:  
STARTED; state: INPROGR, xid/subid/cid: 0/1/0, nestlvl: 1, children: 
< 2016-01-12 19:45:41.397 UTC >DEBUG:  find_in_dynamic_libpath: trying
"/usr/pgsql-9.4/lib/pglogical"
< 2016-01-12 19:45:41.397 UTC >DEBUG:  find_in_dynamic_libpath: trying
"/usr/pgsql-9.4/lib/pglogical.so"
< 2016-01-12 19:45:41.397 UTC >DEBUG:  InitPostgres
< 2016-01-12 19:45:41.397 UTC >DEBUG:  my backend ID is 8
< 2016-01-12 19:45:41.397 UTC >DEBUG:  StartTransaction
< 2016-01-12 19:45:41.397 UTC >DEBUG:  name: unnamed; blockState:  
DEFAULT; state: INPROGR, xid/subid/cid: 0/1/0, nestlvl: 1, children: 
< 2016-01-12 19:45:41.397 UTC >DEBUG:  CommitTransaction
< 2016-01-12 19:45:41.397 UTC >DEBUG:  name: unnamed; blockState:  
STARTED; state: INPROGR, xid/subid/cid: 0/1/0, nestlvl: 1, children: 
< 2016-01-12 19:45:41.397 UTC >DEBUG:  StartTransaction
< 2016-01-12 19:45:41.398 UTC >DEBUG:  name: unnamed; blockState:  
DEFAULT; state: INPROGR, xid/subid/cid: 0/1/0, nestlvl: 1, children: 
< 2016-01-12 19:45:41.398 UTC >DEBUG:  CommitTransaction
< 2016-01-12 19:45:41.398 UTC >DEBUG:  name: unnamed; blockState:  
STARTED; state: INPROGR, xid/subid/cid: 0/1/0, nestlvl: 1, children: 
< 2016-01-12 19:45:41.398 UTC >DEBUG:  StartTransaction
< 2016-01-12 19:45:41.398 UTC >DEBUG:  name: unnamed; blockState:  
DEFAULT; state: INPROGR, xid/subid/cid: 0/1/0, nestlvl: 1, children: 
< 2016-01-12 19:45:41.398 UTC >DEBUG:  CommitTransaction
< 2016-01-12 19:45:41.398 UTC >DEBUG:  name: unnamed; blockState:  
STARTED; state: INPROGR, xid/subid/cid: 0/1/0, nestlvl: 1, children: 
< 2016-01-12 19:45:41.398 UTC >ERROR:  subscriber replicate_gis_data_from_11
initialization failed during nonrecoverable step

Re: [HACKERS] Improve tab completion for REFRESH MATERIALIZED VIEW

On Tue, Jan 26, 2016 at 11:49 PM, Kevin Grittner  wrote:
> On Tue, Jan 26, 2016 at 8:43 AM, Kevin Grittner  wrote:
>
>> I will push something shortly with the
>> improvements from both of you, plus a couple other MV tab
>> completion issues I found in testing these patches.
>
> Done.

But ISTM that CREATE MATERIALIZED VIEW  doesn't list
existing matviews yet. What's worse it lists existing *views*.

This happens because words_after_create mechanism doesn't support
the case where the keyword has more than one words like "MATERIALIZED VIEW".
Probably we should improve that mechanism so that even multiple words can be
handled. Or we should just add something like the following.

   /* Complete CREATE MATERIALIZED VIEW with  */
+   else if (Matches3("CREATE", "MATERIALIZED", "VIEW"))
+   COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_matviews, NULL);

Regards,

-- 
Fujii Masao

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

Re: [HACKERS] Improve tab completion for REFRESH MATERIALIZED VIEW

On Wed, Jan 27, 2016 at 11:41 AM, Fujii Masao  wrote:
> But ISTM that CREATE MATERIALIZED VIEW  doesn't list
> existing matviews yet. What's worse it lists existing *views*.

Yep.

> This happens because words_after_create mechanism doesn't support
> the case where the keyword has more than one words like "MATERIALIZED VIEW".

Having worked on that, I am not convinced this is worth the complication.

> Probably we should improve that mechanism so that even multiple words can be
> handled. Or we should just add something like the following.
>
>/* Complete CREATE MATERIALIZED VIEW with  */
> +   else if (Matches3("CREATE", "MATERIALIZED", "VIEW"))
> +   COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_matviews, NULL);

Indeed, we do the same for tables and views as well. That would be fine.
-- 
Michael


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

Re: [HACKERS] brin_summarize_new_values error checking

On Mon, Jan 25, 2016 at 4:03 PM, Jeff Janes  wrote:
> In reviewing one of my patches[1], Fujii-san has pointed out that I
> didn't include checks for being in recovery, or for working on another
> backend's temporary index.
>
> I think that brin_summarize_new_values in 9.5.0 commits those same
> sins. In its case, I don't think those are critical, as they just
> result in getting less specific error messages that one might hope
> for, rather than something worse like a panic.
>
> But still, we might want to address them.

Agreed to add those checks. Also, I think we should document that
index maintenance functions cannot be executed during recovery.

Regards,

-- 
Fujii Masao


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

Re: [HACKERS] log_checkpoint's "0 transaction log file(s) added" is extremely misleading

On Fri, Jan 22, 2016 at 11:33 AM, Andres Freund  wrote:
> On January 22, 2016 3:29:44 AM GMT+01:00, Simon Riggs  
> wrote:
>>On 22 January 2016 at 01:12, Andres Freund  wrote:
>>
>>> Hi,
>>>
>>> While in theory correct, I think $subject is basically meaningless

What about just changing "added" to "preallocated" to avoid the confusion?

>>> because other backends may have added thousands of new segments. Yes,
>>it
>>> wasn't the checkpointer, but that's not particularly relevant
>>> imo. Additionally, afaics, it will only ever be 0 or 1.
>>>
>>
>>Even better, we could make it add >1
>
> That'd indeed be good, but I don't think it really will address my complaint: 
> We'd still potentially create new segments outside the prealloc call. 
> Including from within the checkpointer, when flushing WAL to be able to write 
> out a page.

IMO it's more helpful to display such information in something like
pg_stat_walwriter view rather than checkpoint log message.

Regards,

-- 
Fujii Masao


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

Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW

2016-01-26 Thread Etsuro Fujita


On 2016/01/26 22:57, Rushabh Lathia wrote:

On Tue, Jan 26, 2016 at 4:15 PM, Etsuro Fujita
> wrote:

On 2016/01/25 17:03, Rushabh Lathia wrote:



int
IsForeignRelUpdatable (Relation rel);



Documentation for IsForeignUpdatable() need to change as it says:

If the IsForeignRelUpdatable pointer is set to NULL, foreign
tables are
assumed
to be insertable, updatable, or deletable if the FDW provides
ExecForeignInsert,
ExecForeignUpdate or ExecForeignDelete respectively.

With introduce of DMLPushdown API now this is no more correct,
as even if
FDW don't provide ExecForeignInsert, ExecForeignUpdate or
ExecForeignDelete API
still foreign tables are assumed to be updatable or deletable with
DMLPushdown
API's, right ?



That's what I'd like to discuss.

I intentionally leave that as-is, because I think we should
determine the updatability of a foreign table in the current
manner.  As you pointed out, even if the FDW doesn't provide eg,
ExecForeignUpdate, an UPDATE on a foreign table could be done using
the DML pushdown APIs if the UPDATE is *pushdown-safe*.  However,
since all UPDATEs on the foreign table are not necessarily
pushdown-safe, I'm not sure it's a good idea to assume the
table-level updatability if the FDW provides the DML pushdown
callback routines.  To keep the existing updatability decision, I
think the FDW should provide the DML pushdown callback routines
together with ExecForeignInsert, ExecForeignUpdate, or
ExecForeignDelete.  What do you think about that?



Sorry but I am not in favour of adding compulsion that FDW should provide
the DML pushdown callback routines together with existing ExecForeignInsert,
ExecForeignUpdate or ExecForeignDelete APIs.

May be we should change the documentation in such way, that explains

a) If FDW PlanDMLPushdown is NULL, then check for ExecForeignInsert,
ExecForeignUpdate or ExecForeignDelete APIs
b) If FDW PlanDMLPushdown is non-NULL and plan is not pushable
check for ExecForeignInsert, ExecForeignUpdate or ExecForeignDelete APIs
c) If FDW PlanDMLPushdown is non-NULL and plan is pushable
check for DMLPushdown APIs.

Does this sounds wired ?


Yeah, but I think that that would be what is done during executor 
startup (see CheckValidResultRel()), while what the documentation is 
saying is about relation_is_updatable(); that is, how to decide the 
updatability of a given foreign table, not how the executor processes an 
individual INSERT/UPDATE/DELETE on a updatable foreign table.  So, I'm 
not sure it's a good idea to modify the documentation in such a way.


BTW, I have added the description about that check partially.  I added 
to the PlanDMLPushdown documentation:


+  If this function succeeds, PlanForeignModify
+  won't be executed, and BeginDMLPushdown,
+  IterateDMLPushdown and EndDMLPushdown will
+  be called at the execution stage, instead.

And for example, I added to the BeginDMLPushdown documentation:

+  If the BeginDMLPushdown pointer is set to
+  NULL, attempts to execute the table update directly on
+  the remote server will fail with an error message.

However, I agree that we should add a documentation note about the 
compulsion somewhere.  Maybe something like this:


The FDW should provide DML pushdown callback routines together with 
table-updating callback routines described above.  Even if the callback 
routines are provided, the updatability of a foreign table is determined 
based on the presence of ExecForeignInsert, ExecForeignUpdate or 
ExecForeignDelete if the IsForeignRelUpdatable pointer is set to NULL.


What's your opinion?

Best regards,
Etsuro Fujita




--
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] Releasing in September

2016-01-26 Thread Torsten Zuehlsdorff


On 26.01.2016 02:09, Peter Eisentraut wrote:

On 1/25/16 2:48 AM, Torsten Zühlsdorff wrote:

Nobody, but there are different solutions. And the same solutions works
different in quality and quantity in the different projects.
In FreeBSD for example there is an online tool for review
(http://review.freebsd.org) which was opened to public. There you can
review any code, left comments in the parts you wanted, submit different
users to it etc.
It is not perfect, but a huge step forward for the project. And
something i misses here often.
But as stated earlier in another thread: for a not-so-deep-involved
volunteer, it is often unclear *what* to review. The threads are long
and often there is no final description about how the patch is supposed
to work. That make testing quite hard and time consuming.


I agree better code review tooling could help a bit.  The URL you post
above doesn't work at the moment (for me), though.


I'm sorry, the url contains a typo. The correct one is:
https://reviews.freebsd.org/

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] pgbench - allow backslash-continuations in custom scripts

2016-01-26 Thread Kyotaro HORIGUCHI

Mmm. I believed that this is on CF app..

At Tue, 19 Jan 2016 15:41:54 +0900, Michael Paquier  
wrote in

Re: [HACKERS] custom function for converting human readable sizes to bytes

Hi

2016-01-26 13:48 GMT+01:00 Vitaly Burovoy :

> Hello, Pavel!
>
> That letter was not a complain against you. I'm sorry if it seems like
> that for you.
>

ok.

It was an intermediate review with several points to be clear for _me_
> from experienced hackers, mostly about a code design.
>
> 26.01.2016 07:05, Pavel Stehule пишет:
> >> pg_proc.h has changed, so the CATALOG_VERSION_NO must be also changed.
> > this is not a part of patch - only a commiter knows CATALOG_VERSION_NO
> >
> CATALOG_VERSION_NO is mentioned for a committer, it is not your fault.
>
> >> III. There is no support of 'bytes' unit, it seems such behavior got
> >> majority approval[2].
> >
> > No, because I have to use the supported units by configuration. The
> configuration supports only three chars long units. Support for "bytes" was
> removed, when I removed proprietary unit table.
> >
> Point "III" is the only for the question "d". Also to collect any
> possible features from the thread in one place.
>
> >> V. The documentation lacks a note that the base of the "pg_size_bytes"
> >> is 1024 whereas the base of the "pg_size_pretty" is 1000.
> >
> > It isn't true, base for both are 1024. It was designed when special
> table was used for pg_size_bytes. But when we share same control table,
> then is wrong to use different table. The result can be optically
> different, but semantically same.
> >
> Oops, I was wrong about a base of pg_size_pretty. It was a morning
> after a hard night...


> > negative values is fully supported.
> You have mentioned it at least three times before. It is not my new
> requirement or a point to your fault, it is an argument for
> symmetry/asymmetry of the function.
>
> > support of "bytes" depends on support "bytes" unit by GUC. When "bytes"
> unit will be supported, then it can be used in pg_size_bytes immediately.
> By the way there can be a comparison for a special size unit before
> calling parse_memory_unit.
>

there was more requirements based on original proposal (based in  separated
control tables). When I leaved this design, I dropped more requirements and
now is little bit hard to distinguish what is related and with. Some
features can be added later without any possible compatibility issues in
next commit fest or later, and I am searching some good borders for this
patch. Now, this border is a shared memory unit control table. And I am
trying to hold this border.

I am grateful for review - now this patch is better, and I hope near final
stage.

Regards

Pavel




>
> > Regards
> > Pavel
> >
> >> [2]
> http://www.postgresql.org/message-id/cacaco5qw7ffsffhkstjtycp4qf3oh9za14sc6z3dxx2yssj...@mail.gmail.com
>

[HACKERS] Existence check for suitable index in advance when concurrently refreshing.

2016-01-26 Thread Masahiko Sawada

Hi all,

In concurrently refreshing materialized view, we check whether that
materialized view has suitable index(unique and not having WHERE
condition), after filling data to new snapshot
(refresh_matview_datafill()).
This logic leads to taking a lot of time until postgres returns ERROR
log if that table doesn't has suitable index and table is large. it
wastes time.
I think we should check whether that materialized view can use
concurrently refreshing or not in advance.
The patch is attached.

Please give me feedbacks.

-- 
Regards,

--
Masahiko Sawada


matview_concurrently_refresh_check_index_v1.patch
Description: binary/octet-stream

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

Re: [HACKERS] Add generate_series(date,date) and generate_series(date,date,integer)