date:20150426

On 2015-04-25 20:47:04 -0400, Tom Lane wrote:
 Since default_with_oids is really only meant as a backwards-compatibility
 hack, I don't personally have a problem with restricting it to act only on
 plain tables.

FWIW, I think we're getting pretty close to the point, or are there
even, where we can remove default_with_oids. So not adding complications
because of it sounds good to me.

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] PL/pgSQL, RAISE and error context

2015-04-26 Thread Pavel Stehule

Hi

I reduced this patch, little bit cleaned - now it is based on plpgsql GUC
display_context_min_messages - like client_min_messages, log_min_messages.

Documentation added.

Regards

Pavel

2015-04-25 22:23 GMT+02:00 Pavel Stehule pavel.steh...@gmail.com:

 Hi

 2015-04-24 19:16 GMT+02:00 Joel Jacobson j...@trustly.com:

 On Fri, Apr 24, 2015 at 6:07 PM, Pavel Stehule pavel.steh...@gmail.com
 wrote:
  Example:
 
  context_messages = -warning, -error, +notice
 
 
  I prefer your first proposal - and there is a precedent for plpgsql -
  plpgsql_extra_checks
 
  It is clean for anybody. +-identifiers looks like horrible httpd
 config. :)

 I have to agree on that :) Just thought this is the best we can do if
 we want to reduce the number of GUCs to a minimum.


 I played with some prototype and I am thinking so we need only one GUC

 plpgsql.display_context_messages = 'none'; -- compatible with current
 plpgsql.display_context_messages = 'all';
 plpgsql.display_context_messages = 'exception, log'; -- what I prefer

 I implemented [ (WITH|WITHOUT) CONTEXT ] clause for RAISE statement

 RAISE NOTICE WITH CONTEXT 'some message';
 RAISE NOTICE WITH CONTEXT USING message = 'some message';
 RAISE EXCEPTION WITHOUT CONTEXT 'other message';

 The patch is very small with full functionality (without documentation) -
 I am thinking so it can work. This patch is back compatible - and allow to
 change default behave simply.

 plpgsql.display_context_messages can be simplified to some like
 plpgsql.display_context_min_messages

 What do you think about it?

 Regards

 Pavel


commit 33951bc23365029ee94af5ec43e90893dcd737a8
Author: Pavel Stehule pavel.steh...@gooddata.com
Date:   Sat Apr 25 22:09:28 2015 +0200

initial implementation of (WITH|WITHOUT) CONTEXT clause to plpgsql RAISE statement.

initial implementation of plpgsql GUC plpgsql.display_context_messages

diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index d36acf6..8aebb87 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -3406,10 +3406,10 @@ END LOOP optional replaceablelabel/replaceable /optional;
 raise errors.
 
 synopsis
-RAISE optional replaceable class=parameterlevel/replaceable /optional 'replaceable class=parameterformat/replaceable' optional, replaceable class=parameterexpression/replaceable optional, ... /optional/optional optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional /optional;
-RAISE optional replaceable class=parameterlevel/replaceable /optional replaceable class=parametercondition_name/ optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional /optional;
-RAISE optional replaceable class=parameterlevel/replaceable /optional SQLSTATE 'replaceable class=parametersqlstate/' optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional /optional;
-RAISE optional replaceable class=parameterlevel/replaceable /optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional;
+RAISE optional replaceable class=parameterlevel/replaceable /optional optional ( WITH | WITHOUT ) CONTEXT /optional 'replaceable class=parameterformat/replaceable' optional, replaceable class=parameterexpression/replaceable optional, ... /optional/optional optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional /optional;
+RAISE optional replaceable class=parameterlevel/replaceable /optional optional ( WITH | WITHOUT ) CONTEXT /optional replaceable class=parametercondition_name/ optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional /optional;
+RAISE optional replaceable class=parameterlevel/replaceable /optional optional ( WITH | WITHOUT ) CONTEXT /optional SQLSTATE 'replaceable class=parametersqlstate/' optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional /optional;
+RAISE optional replaceable class=parameterlevel/replaceable /optional optional ( WITH | WITHOUT ) CONTEXT /optional USING replaceable class=parameteroption/replaceable = replaceable class=parameterexpression/replaceable optional, ... /optional;
 RAISE ;
 /synopsis
 
@@ -3431,6 +3431,18 @@ RAISE ;
/para
 
para
+The options literalWITH CONTEXT/literal or literalWITHOUT CONTEXT/literal
+can enforce or suppress context information related to error or notice. This possibility
+can be forced by settings of configuration parameter literalplpgsql.display_context_min_messages/.
+This allows same values like replaceable class=parameterlevel/replaceable option plus
+value

Re: [HACKERS] INSERT ... ON CONFLICT syntax issues

2015-04-26 Thread Heikki Linnakangas


On 04/25/2015 12:01 PM, Andres Freund wrote:

INSERT ... ON CONFLICT (cola, colb [WHERE predicate_for_partial]) UPDATE|IGNORE

My problem with the WHERE being inside the parens in the above is that
it's
a) different from CREATE INDEX
b) unclear whether the WHERE belongs to colb or the whole index
expression. The equivalent for aggregates, which I bet is going to be
used less often, caused a fair amount of confusing.

That's why I wanted the WHERE outside the (), which requires either
adding DO between the index inference clause, and the action, to avoid
ambiguities in the grammar.


Yeah, having the WHERE outside the parens seems much nicer. What is the 
ambiguity?



But I'm generally having some doubts about the syntax.

Right now it's
INSERT ... ON CONFLICT opt_on_conf_clause UPDATE|IGNORE.

A couple things:

a) Why is is 'CONFLICT? We're talking about a uniquness violation. What
if we, at some later point, also want to handle other kind of
violations? Shouldn't it be ON UNIQUE CONFLICT/ERROR/VIOLATION ...


As Peter said, it's also for exclusion constraints. Perhaps ON 
CONSTRAINT VIOLATION? It doesn't apply to foreign key constraints, 
though. I think ON CONFLICT is fine.



b) For me there's a WITH before the index inference clause missing, to
have it read in 'SQL' style.


Agreed. ON would sound more natural than WITH though:

INSERT INTO mytable ON CONFLICT ON (keycol) UPDATE ...

The ability to specify a constraint by name hasn't been implemented, but 
that would read quite naturally as:


INSERT INTO mytable ON CONFLICT ON CONSTRAINT my_constraint UPDATE ...

- Heikki



--
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] Fwd: [GENERAL] 4B row limit for CLOB tables

2015-04-26 Thread Álvaro Hernández Tortosa



On 25/04/15 06:39, Jim Nasby wrote:

On 4/24/15 7:11 PM, Álvaro Hernández Tortosa wrote:

On 24/04/15 05:24, Tom Lane wrote:

...

TBH, I've got very little enthusiasm for fixing this given the number
of reports of trouble from the field, which so far as I recall is zero.
Álvaro's case came up through intentionally trying to create an
unreasonable number of tables, not from real usage.  This thread 
likewise

appears to contain lots of speculation and no reports of anyone hitting
a problem in practice.


 It is certainly true that this was a very synthetic case. I
envision, however, certain use cases where we may hit a very large
number of tables:


The original case has NOTHING to do with the number of tables and 
everything to do with the number of toasted values a table can have. 
If you have to toast 4B attributes in a single relation it will fail. 
In reality, if you get anywhere close to that things will fall apart 
due to OID conflicts.


This case isn't nearly as insane as 4B tables. A table storing 10 text 
fields each of which is 2K would hit this limit with only 400M rows. 
If my math is right that's only 8TB; certainly not anything insane 
space-wise or rowcount-wise.


Perhaps it's still not fixing, but I think it's definitely worth 
documenting.


They are definitely different problems, but caused by similar 
symptoms: an oid wrapping around, or not even there: just trying to find 
an unused one. If fixed, we should probably look at both at the same time.


It's worth document but also, as I said, maybe also fixing them, so 
that if three years from now they really show up, solution is already in 
production (rather than in patching state).


Regards,

Álvaro


--
Álvaro Hernández Tortosa


---
8Kdata



--
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] fix typos in comments

2015-04-26 Thread Tom Lane

Andres Freund and...@anarazel.de writes:
 On 2015-04-26 19:13:42 +0400, Dmitriy Olshevskiy wrote:
 - *therefor it is up to the calling routine to
 + *therefore it is up to the calling routine to

 I think both are actually legal? Yes therefore is more common, but
 still.

Hm.  My dictionary says that therefor is archaic, but to my eye it
looks just wrong.  Certainly no modern writer would spell it like that.

 Nope. Iff means if and only if.

Right, iff is intentional here (and in many other places).  We've
discussed that before.

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] forward vs backward slashes in msvc build code

2015-04-26 Thread Andrew Dunstan



On 04/26/2015 03:13 AM, Michael Paquier wrote:


Oops, attached is a patch that should make currawong happier..


OK, pushed.

Argh. This is not enough:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=currawongdt=2015-04-26%2006%3A00%3A01
The build is done in Debug mode, but it is failing to find some files
under the label Release, which is incorrect. I guess that this is
caused by the file detection in WriteFiles... TBH I don't have an
environment at hand to reproduce the issue and do a proper fix. Hence
I think that we should revert the patch for now, and come back to this
stuff later on.





Well, currawong is mine. so I can try anything you want to suggest.

But I don't think it's set up to build Debug.

Setting up a VS2008 environment is hard these days, as MS seems to have 
removed the download :-(


cheers

andrew


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

[HACKERS] fix typos in comments

2015-04-26 Thread Dmitriy Olshevskiy


Hello!

Please see this patch with several typos and 
mistakes in comments.


There are also typos in sgml files (duplicate to):
1. doc/src/sgml/logicaldecoding.sgml, ln 619
 Logical decoding can be used to to build
2. doc/src/sgml/ref/pg_dumpall.sgml, ln 457
 Specifies the name of the database to connect 
to to dump global


--
Dmitriy Olshevskiy

From 2558366552ce00bff5c57a98dea72f959e3c72e7 Mon Sep 17 00:00:00 2001
From: olshevskiy87 olshevski...@bk.ru
Date: Sun, 26 Apr 2015 18:40:07 +0400
Subject: [PATCH] fix typos in comments

---
 src/backend/access/brin/brin_tuple.c  | 2 +-
 src/backend/access/nbtree/nbtree.c| 2 +-
 src/backend/access/transam/twophase.c | 2 +-
 src/backend/catalog/objectaddress.c   | 4 ++--
 src/backend/commands/indexcmds.c  | 2 +-
 src/backend/executor/nodeModifyTable.c| 2 +-
 src/backend/libpq/pqcomm.c| 2 +-
 src/backend/optimizer/geqo/geqo_erx.c | 2 +-
 src/backend/postmaster/bgworker.c | 2 +-
 src/backend/replication/logical/snapbuild.c   | 2 +-
 src/backend/storage/lmgr/lwlock.c | 4 ++--
 src/backend/utils/cache/inval.c   | 2 +-
 src/bin/pg_archivecleanup/pg_archivecleanup.c | 2 +-
 src/bin/pg_basebackup/pg_recvlogical.c| 4 ++--
 src/bin/pg_upgrade/parallel.c | 2 +-
 src/bin/pg_upgrade/relfilenode.c  | 2 +-
 src/include/access/attnum.h   | 6 +++---
 src/include/access/xact.h | 2 +-
 src/include/mb/pg_wchar.h | 2 +-
 src/include/storage/s_lock.h  | 4 ++--
 src/interfaces/ecpg/pgtypeslib/datetime.c | 2 +-
 src/interfaces/ecpg/pgtypeslib/numeric.c  | 2 +-
 src/port/pgmkdirp.c   | 2 +-
 23 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/src/backend/access/brin/brin_tuple.c b/src/backend/access/brin/brin_tuple.c
index 93f00f6..08fa998 100644
--- a/src/backend/access/brin/brin_tuple.c
+++ b/src/backend/access/brin/brin_tuple.c
@@ -304,7 +304,7 @@ brin_free_tuple(BrinTuple *tuple)
 }
 
 /*
- * Create an palloc'd copy of a BrinTuple.
+ * Create a palloc'd copy of a BrinTuple.
  */
 BrinTuple *
 brin_copy_tuple(BrinTuple *tuple, Size len)
diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index 9a6dcdd..c2d52fa 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -40,7 +40,7 @@ typedef struct
 	BTSpool*spool;
 
 	/*
-	 * spool2 is needed only when the index is an unique index. Dead tuples
+	 * spool2 is needed only when the index is a unique index. Dead tuples
 	 * are put into spool2 instead of spool in order to avoid uniqueness
 	 * check.
 	 */
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 3ac339b..d9a3fab 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -291,7 +291,7 @@ AtAbort_Twophase(void)
 }
 
 /*
- * This is called after we have finished transfering state to the prepared
+ * This is called after we have finished transferring state to the prepared
  * PGXACT entry.
  */
 void
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index 30cb699..816ab50 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -99,7 +99,7 @@ typedef struct
 	AclObjectKind acl_kind;		/* ACL_KIND_* of this object type */
 	bool		is_nsp_name_unique;		/* can the nsp/name combination (or
 		 * name alone, if there's no
-		 * namespace) be considered an unique
+		 * namespace) be considered a unique
 		 * identifier for an object of this
 		 * class? */
 } ObjectPropertyType;
@@ -3200,7 +3200,7 @@ pg_identify_object(PG_FUNCTION_ARGS)
 
 			/*
 			 * We only return the object name if it can be used (together with
-			 * the schema name, if any) as an unique identifier.
+			 * the schema name, if any) as a unique identifier.
 			 */
 			if (get_object_namensp_unique(address.classId))
 			{
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 99acd4a..351d48e 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -1051,7 +1051,7 @@ ComputeIndexAttrs(IndexInfo *indexInfo,
  */
 
 /*
- * A expression using mutable functions is probably wrong,
+ * An expression using mutable functions is probably wrong,
  * since if you aren't going to get the same result for the
  * same data every time, it's not clear what the index entries
  * mean at all.
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 06ec82e..31666ed 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -304,7 +304,7 @@ ExecInsert(TupleTableSlot *slot,
 	 * inserting the record into the heap and all indexes.
 	 *

Re: [HACKERS] fix typos in comments

Hi,

Man, whoever invented these an vs. a rules... But then this patch made
me lookup the rules ;)

On 2015-04-26 19:13:42 +0400, Dmitriy Olshevskiy wrote:

 diff --git a/src/backend/optimizer/geqo/geqo_erx.c 
 b/src/backend/optimizer/geqo/geqo_erx.c
 index 69ac077..1a43ab7 100644
 --- a/src/backend/optimizer/geqo/geqo_erx.c
 +++ b/src/backend/optimizer/geqo/geqo_erx.c
 @@ -138,7 +138,7 @@ gimme_edge_table(PlannerInfo *root, Gene *tour1, Gene 
 *tour2,
   * registers edge from city1 to city2 in input edge table
   *
   * no assumptions about directionality are made;
 - * therefor it is up to the calling routine to
 + * therefore it is up to the calling routine to
   * call gimme_edge twice to make a bi-directional edge
   * between city1 and city2;
   * uni-directional edges are possible as well (just call
 gimme_edge

I think both are actually legal? Yes therefore is more common, but
still.

I left this out.

 diff --git a/src/include/access/attnum.h b/src/include/access/attnum.h
 index 82e811d..300b682 100644
 --- a/src/include/access/attnum.h
 +++ b/src/include/access/attnum.h
 @@ -29,14 +29,14 @@ typedef int16 AttrNumber;
   */
  /*
   * AttributeNumberIsValid
 - *   True iff the attribute number is valid.
 + *   True if the attribute number is valid.
   */
  #define AttributeNumberIsValid(attributeNumber) \
   ((bool) ((attributeNumber) != InvalidAttrNumber))
  
  /*
   * AttrNumberIsForUserDefinedAttr
 - *   True iff the attribute number corresponds to an user defined 
 attribute.
 + *   True if the attribute number corresponds to a user defined 
 attribute.
   */

Nope. Iff means if and only if.

 diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h
 index f4dc0db..b131412 100644
 --- a/src/include/storage/s_lock.h
 +++ b/src/include/storage/s_lock.h
 @@ -356,8 +356,8 @@ tas(volatile slock_t *lock)
  /*
   * Solaris has always run sparc processors in TSO (total store) mode, but
   * linux didn't use to and the *BSDs still don't. So, be careful about
 - * acquire/release semantics. The CPU will treat superflous membars as NOPs,
 - * so it's just code space.
 + * acquire/release semantics. The CPU will treat superfluous membars as 
 + * NOPs, so it's just code space.
   */
  #define HAS_TEST_AND_SET

superflous, err superfluous, trailing space removed.

I've pushed the rest. Thanks!

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] inherit support for foreign tables

2015-04-26 Thread Tom Lane

Andres Freund and...@anarazel.de writes:
 FWIW, I think we're getting pretty close to the point, or are there
 even, where we can remove default_with_oids. So not adding complications
 because of it sounds good to me.

Well, pg_dump uses it --- so the argument about not breaking old dump
scripts would apply to any attempt to remove it entirely.  But I don't
have a problem with saying that its semantics are frozen.

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] INSERT ... ON CONFLICT syntax issues

2015-04-26 Thread Petr Jelinek


On 26/04/15 12:08, Andres Freund wrote:

On April 26, 2015 11:22:01 AM GMT+02:00, Heikki Linnakangas hlinn...@iki.fi 
wrote:

On 04/25/2015 12:01 PM, Andres Freund wrote:


That's why I wanted the WHERE outside the (), which requires either
adding DO between the index inference clause, and the action, to

avoid

ambiguities in the grammar.


Yeah, having the WHERE outside the parens seems much nicer. What is the

ambiguity?



With a full keyword in between (like DO), there's none. But without it its 
ambiguous where a trailing UPDATE belongs to. At least from the point of a LALR 
grammar. WHERE UPDATE; is legal. I don't see the DO as much of a problem though.



The DO variant with WHERE outside of parenthesis sounds fine to me. Or 
at least better than the alternatives I've seen or can come up with.



A couple things:

a) Why is is 'CONFLICT? We're talking about a uniquness violation.

What

 if we, at some later point, also want to handle other kind of
 violations? Shouldn't it be ON UNIQUE CONFLICT/ERROR/VIOLATION

...

As Peter said, it's also for exclusion constraints. Perhaps ON
CONSTRAINT VIOLATION? It doesn't apply to foreign key constraints,
though. I think ON CONFLICT is fine.


What if we, as at least I have previously wished for, want to allow handling 
other types of constraints? It'd be quite cool to be able to insert the 
referenced key on a fkey violation for some use cases.


b) For me there's a WITH before the index inference clause missing,

to

 have it read in 'SQL' style.


Agreed. ON would sound more natural than WITH though:

INSERT INTO mytable ON CONFLICT ON (keycol) UPDATE ...


I chose WITh because of the repeated DO; that's all ;)



The ON CONFLICT ON sounds really weird to me. Either ON CONSTRAINT 
VIOLATION (foo) or ON CONFLICT [WITH] (foo) both seem acceptable.


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


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

Re: [HACKERS] extend pgbench expressions with functions

2015-04-26 Thread Fabien COELHO




v3, just a rebase.


Thanks for working on this.  I see it's already registered in the
2015-06 CF, and will have a look at when we get there.


v4, rebase (after pgbench moved to src) and use of the recently added 
syntax_error function.


--
Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index a808546..d09b2bf 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -762,7 +762,7 @@ pgbench optional replaceableoptions/ /optional replaceabledbname/
   references to variables literal:/replaceablevariablename/,
   and expressions composed of unary (literal-/) or binary operators
   (literal+/, literal-/, literal*/, literal//, literal%/)
-  with their usual associativity, and parentheses.
+  with their usual associativity, function literalabs/ and parentheses.
  /para
 
  para
diff --git a/src/bin/pgbench/exprparse.y b/src/bin/pgbench/exprparse.y
index e68631e..79240ab 100644
--- a/src/bin/pgbench/exprparse.y
+++ b/src/bin/pgbench/exprparse.y
@@ -20,6 +20,7 @@ static PgBenchExpr *make_integer_constant(int64 ival);
 static PgBenchExpr *make_variable(char *varname);
 static PgBenchExpr *make_op(char operator, PgBenchExpr *lexpr,
 		PgBenchExpr *rexpr);
+static PgBenchExpr *make_func(const char *fname, PgBenchExpr *arg1);
 
 %}
 
@@ -35,9 +36,9 @@ static PgBenchExpr *make_op(char operator, PgBenchExpr *lexpr,
 
 %type expr expr
 %type ival INTEGER
-%type str VARIABLE
+%type str VARIABLE FUNCTION
 
-%token INTEGER VARIABLE
+%token INTEGER VARIABLE FUNCTION
 %token CHAR_ERROR /* never used, will raise a syntax error */
 
 /* Precedence: lowest to highest */
@@ -59,6 +60,7 @@ expr: '(' expr ')'			{ $$ = $2; }
 	| expr '%' expr			{ $$ = make_op('%', $1, $3); }
 	| INTEGER{ $$ = make_integer_constant($1); }
 	| VARIABLE { $$ = make_variable($1); }
+	| FUNCTION '(' expr ')' { $$ = make_func($1, $3); pg_free($1); }
 	;
 
 %%
@@ -95,4 +97,33 @@ make_op(char operator, PgBenchExpr *lexpr, PgBenchExpr *rexpr)
 	return expr;
 }
 
+static struct {
+	char * fname;
+	int nargs;
+	PgBenchFunction tag;
+} PGBENCH_FUNCTIONS[] = {
+	{ abs, 1, PGBENCH_ABS }
+};
+
+static PgBenchExpr *
+make_func(const char * fname, PgBenchExpr *arg1)
+{
+	PgBenchExpr *expr = pg_malloc(sizeof(PgBenchExpr));
+	int nfunctions = sizeof(PGBENCH_FUNCTIONS) / sizeof(PGBENCH_FUNCTIONS[0]);
+	int i;
+
+	expr-etype = ENODE_FUNCTION;
+	expr-u.function.function = PGBENCH_NONE;
+
+	for (i = 0; i  nfunctions; i++)
+		if (pg_strcasecmp(fname, PGBENCH_FUNCTIONS[i].fname) == 0)
+			expr-u.function.function = PGBENCH_FUNCTIONS[i].tag;
+
+	if (expr-u.function.function == PGBENCH_NONE)
+		yyerror_more(unexpected function name, fname);
+
+	expr-u.function.arg1 = arg1;
+	return expr;
+}
+
 #include exprscan.c
diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l
index 5331ab7..d517dc1 100644
--- a/src/bin/pgbench/exprscan.l
+++ b/src/bin/pgbench/exprscan.l
@@ -57,6 +57,11 @@ space			[ \t\r\f]
 	yylval.ival = strtoint64(yytext);
 	return INTEGER;
 }
+[a-zA-Z]+   {
+	yycol += yyleng;
+	yylval.str = pg_strdup(yytext);
+	return FUNCTION;
+}
 
 [\n]			{ yycol = 0; yyline++; }
 {space}+		{ yycol += yyleng; /* ignore */ }
@@ -71,10 +76,16 @@ space			[ \t\r\f]
 %%
 
 void
-yyerror(const char *message)
+yyerror_more(const char *message, const char *more)
 {
 	syntax_error(expr_source, expr_lineno, expr_full_line, expr_command,
- message, NULL, expr_col + yycol);
+ message, more, expr_col + yycol);
+}
+
+void
+yyerror(const char *message)
+{
+	yyerror_more(message, NULL);
 }
 
 /*
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 06a4dfd..5084381 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -956,6 +956,26 @@ evaluateExpr(CState *st, PgBenchExpr *expr, int64 *retval)
 return false;
 			}
 
+		case ENODE_FUNCTION:
+			{
+switch (expr-u.function.function)
+{
+	case PGBENCH_ABS:
+	{
+		int64 arg1;
+		if (!evaluateExpr(st, expr-u.function.arg1, arg1))
+			return false;
+
+		*retval = arg1  0? arg1: -arg1;
+		return true;
+	}
+	default:
+		fprintf(stderr, bad function (%d)\n,
+expr-u.function.function);
+		return false;
+}
+			}
+
 		default:
 			break;
 	}
diff --git a/src/bin/pgbench/pgbench.h b/src/bin/pgbench/pgbench.h
index a3db6b9..f7e72f8 100644
--- a/src/bin/pgbench/pgbench.h
+++ b/src/bin/pgbench/pgbench.h
@@ -15,11 +15,18 @@ typedef enum PgBenchExprType
 {
 	ENODE_INTEGER_CONSTANT,
 	ENODE_VARIABLE,
-	ENODE_OPERATOR
+	ENODE_OPERATOR,
+	ENODE_FUNCTION
 } PgBenchExprType;
 
 typedef struct PgBenchExpr PgBenchExpr;
 
+typedef enum PgBenchFunction
+{
+	PGBENCH_NONE,
+	PGBENCH_ABS
+} PgBenchFunction;
+
 struct PgBenchExpr
 {
 	PgBenchExprType	etype;
@@ -39,6 +46,11 @@ struct PgBenchExpr
 			PgBenchExpr	*lexpr;
 			PgBenchExpr *rexpr;
 		} operator;
+		struct
+		{
+			PgBenchFunction function;
+			PgBenchExpr

Re: [HACKERS] INSERT ... ON CONFLICT syntax issues

* Heikki Linnakangas (hlinn...@iki.fi) wrote:
 On 04/25/2015 12:01 PM, Andres Freund wrote:
 INSERT ... ON CONFLICT (cola, colb [WHERE predicate_for_partial]) 
 UPDATE|IGNORE
 
 My problem with the WHERE being inside the parens in the above is that
 it's
 a) different from CREATE INDEX
 b) unclear whether the WHERE belongs to colb or the whole index
 expression. The equivalent for aggregates, which I bet is going to be
 used less often, caused a fair amount of confusing.
 
 That's why I wanted the WHERE outside the (), which requires either
 adding DO between the index inference clause, and the action, to avoid
 ambiguities in the grammar.
 
 Yeah, having the WHERE outside the parens seems much nicer. What is
 the ambiguity?

I like having it outside the parens also.

 But I'm generally having some doubts about the syntax.
 
 Right now it's
 INSERT ... ON CONFLICT opt_on_conf_clause UPDATE|IGNORE.
 
 A couple things:
 
 a) Why is is 'CONFLICT? We're talking about a uniquness violation. What
 if we, at some later point, also want to handle other kind of
 violations? Shouldn't it be ON UNIQUE CONFLICT/ERROR/VIOLATION ...
 
 As Peter said, it's also for exclusion constraints. Perhaps ON
 CONSTRAINT VIOLATION? It doesn't apply to foreign key constraints,
 though. I think ON CONFLICT is fine.

I don't mind using CONFLICT here, seems to make sense to me.

 b) For me there's a WITH before the index inference clause missing, to
 have it read in 'SQL' style.
 
 Agreed. ON would sound more natural than WITH though:
 
 INSERT INTO mytable ON CONFLICT ON (keycol) UPDATE ...
 
 The ability to specify a constraint by name hasn't been implemented,
 but that would read quite naturally as:
 
 INSERT INTO mytable ON CONFLICT ON CONSTRAINT my_constraint UPDATE ...

I don't particularly like the double-ON in this..

I've not tried, but is the first ON required to be a full keyword?
Seems like it probably is, but just to finish the thought I had, what
about:

INSERT INTO mytable .. IF CONFLICT ON (a,b) WHERE .. THEN UPDATE

IF is currently just an unreserved keyword though.

We could use FOR though:

INSERT INTO mytable .. FOR CONFLICT ON (a,b) WHERE .. THEN UPDATE

Though that'd probably sound better as:

INSERT INTO mytable .. FOR CONFLICT ON (a,b) WHERE .. DO UPDATE

Another option is:

INSERT INTO mytable .. WHEN CONFLICT ON (a,b) WHERE .. DO UPDATE

Which could also be:

INSERT INTO mytable .. WHEN CONFLICT ON (a,b) WHERE .. THEN UPDATE

of course..

What's important, in my view, is to keep the simple case simple and so
I'm not particularly wedded to any of these approaches, just trying to
help with other suggestions.

INSERT INTO mytable VALUES ('key1','key2','val1','val2')
ON CONFLICT UPDATE SET val1 = 'val1', val2 = 'val2';

strikes me as a the 99% use-case here that we need to keep sane, and
it'd be really nice if we didn't have to include the SET clause and
duplicate those values at all..  That could be something we add later
though, I don't think it needs to be done now.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] INSERT ... ON CONFLICT syntax issues

On April 26, 2015 11:22:01 AM GMT+02:00, Heikki Linnakangas hlinn...@iki.fi 
wrote:
On 04/25/2015 12:01 PM, Andres Freund wrote:
 INSERT ... ON CONFLICT (cola, colb [WHERE predicate_for_partial])
UPDATE|IGNORE

 My problem with the WHERE being inside the parens in the above is
that
 it's
 a) different from CREATE INDEX
 b) unclear whether the WHERE belongs to colb or the whole index
 expression. The equivalent for aggregates, which I bet is going
to be
 used less often, caused a fair amount of confusing.

 That's why I wanted the WHERE outside the (), which requires either
 adding DO between the index inference clause, and the action, to
avoid
 ambiguities in the grammar.

Yeah, having the WHERE outside the parens seems much nicer. What is the

ambiguity?


With a full keyword in between (like DO), there's none. But without it its 
ambiguous where a trailing UPDATE belongs to. At least from the point of a LALR 
grammar. WHERE UPDATE; is legal. I don't see the DO as much of a problem though.


 But I'm generally having some doubts about the syntax.

 Right now it's
 INSERT ... ON CONFLICT opt_on_conf_clause UPDATE|IGNORE.

 A couple things:

 a) Why is is 'CONFLICT? We're talking about a uniquness violation.
What
 if we, at some later point, also want to handle other kind of
 violations? Shouldn't it be ON UNIQUE CONFLICT/ERROR/VIOLATION
...

As Peter said, it's also for exclusion constraints. Perhaps ON 
CONSTRAINT VIOLATION? It doesn't apply to foreign key constraints, 
though. I think ON CONFLICT is fine.

What if we, as at least I have previously wished for, want to allow handling 
other types of constraints? It'd be quite cool to be able to insert the 
referenced key on a fkey violation for some use cases.

 b) For me there's a WITH before the index inference clause missing,
to
 have it read in 'SQL' style.

Agreed. ON would sound more natural than WITH though:

INSERT INTO mytable ON CONFLICT ON (keycol) UPDATE ...

I chose WITh because of the repeated DO; that's all ;)


--- 
Please excuse brevity and formatting - I am writing this on my mobile phone.


-- 
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 pgbench syntax error messages

2015-04-26 Thread Fabien COELHO




Here is v6, just a rebase.


Committed with minor stylistic fixes.


Thanks!

--
Fabien.


--
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] Fix pgbench --progress report under (very) low rate

2015-04-26 Thread Fabien COELHO




I haven't had time to really review the code here (except to notice
that you have a typo: nedded) but the idea of it seems good.


v3 rebase (after pgbench moved to src/bin) and minor style tweaking.

--
Fabien.diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 06a4dfd..38dc4a5 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -3607,6 +3607,28 @@ threadRun(void *arg)
 maxsock = sock;
 		}
 
+		/* also meet the next progress report time if needed */
+		if (progress  min_usec  0
+#if !defined(PTHREAD_FORK_EMULATION)
+			 thread-tid == 0
+#endif /* !PTHREAD_FORK_EMULATION */
+			)
+		{
+			/* get current time if needed */
+			if (now_usec == 0)
+			{
+instr_time	now;
+
+INSTR_TIME_SET_CURRENT(now);
+now_usec = INSTR_TIME_GET_MICROSEC(now);
+			}
+
+			if (now_usec = next_report)
+min_usec = 0;
+			else if ((next_report - now_usec)  min_usec)
+min_usec = next_report - now_usec;
+		}
+
 		if (min_usec  0  maxsock != -1)
 		{
 			int			nsocks; /* return from select(2) */

-- 
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] forward vs backward slashes in msvc build code

2015-04-26 Thread Andrew Dunstan



On 04/26/2015 04:08 PM, Noah Misch wrote:

On Sun, Apr 26, 2015 at 10:23:58AM -0400, Andrew Dunstan wrote:

Setting up a VS2008 environment is hard these days, as MS seems to have
removed the download :-(

Visual C++ 2008 Express Edition:
http://go.microsoft.com/?linkid=7729279



Oh, cool. Thanks.

cheers

andrew




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

Re: [HACKERS] forward vs backward slashes in msvc build code

2015-04-26 Thread Michael Paquier

On Mon, Apr 27, 2015 at 8:53 AM, Andrew Dunstan and...@dunslane.net wrote:

 On 04/26/2015 04:08 PM, Noah Misch wrote:

 On Sun, Apr 26, 2015 at 10:23:58AM -0400, Andrew Dunstan wrote:

 Setting up a VS2008 environment is hard these days, as MS seems to have
 removed the download :-(

 Visual C++ 2008 Express Edition:
 http://go.microsoft.com/?linkid=7729279


 Oh, cool. Thanks.

Thanks, I'll try to set up an environment and come up with a real
patch tested this time.

What I suspect is that we need to correctly replace all the references
to backslaches like '\\' or $obj =~ s/\\/_/g; to make things work
correctly.
Regards,
-- 
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] forward vs backward slashes in msvc build code

2015-04-26 Thread Noah Misch

On Sun, Apr 26, 2015 at 10:23:58AM -0400, Andrew Dunstan wrote:
 Setting up a VS2008 environment is hard these days, as MS seems to have
 removed the download :-(

Visual C++ 2008 Express Edition:
http://go.microsoft.com/?linkid=7729279


-- 
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] INSERT ... ON CONFLICT syntax issues

On Sun, Apr 26, 2015 at 6:34 AM, Stephen Frost sfr...@snowman.net wrote:
 What's important, in my view, is to keep the simple case simple and so
 I'm not particularly wedded to any of these approaches, just trying to
 help with other suggestions.

 INSERT INTO mytable VALUES ('key1','key2','val1','val2')
 ON CONFLICT UPDATE SET val1 = 'val1', val2 = 'val2';

 strikes me as a the 99% use-case here that we need to keep sane, and
 it'd be really nice if we didn't have to include the SET clause and
 duplicate those values at all..  That could be something we add later
 though, I don't think it needs to be done now.

You can do that already. That's what the EXCLUDED.* alias that is
automatically added is for (the thing that Andres disliked the
spelling of - or the other thing). This is legal, for example:

INSERT INTO mytable (foo, bar, baz, bat) VALUES ('key1','key2','val1','val2')
ON CONFLICT (foo) UPDATE SET (foo, bar, baz, bat) = (EXCLUDED.foo,
EXCLUDED.bar, EXCLUDED.baz, EXCLUDED.bat)';

I don't want to accept something that automatically merges the
excluded tuple (e.g., SET (*) = EXLCUDED.*), for reasons outlined
here: https://wiki.postgresql.org/wiki/UPSERT#VoltDB.27s_UPSERT
-- 
Peter Geoghegan


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

Re: [HACKERS] parallel mode and parallel contexts

2015-04-26 Thread Euler Taveira

On 19-03-2015 15:13, Robert Haas wrote:
 On Wed, Mar 18, 2015 at 5:36 PM, Andres Freund and...@2ndquadrant.com wrote:
 Reading the README first, the rest later. So you can comment on my
 comments, while I actually look at the code. Parallelism, yay!
 
I'm also looking at this piece of code and found some low-hanging fruit.


-- 
   Euler Taveira   Timbira - http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
From 3ce5376868f61a67540915b83a15c59a31fc895a Mon Sep 17 00:00:00 2001
From: Euler Taveira eu...@timbira.com
Date: Sun, 26 Apr 2015 13:49:37 -0300
Subject: [PATCH 1/3] fix some typos.

---
 src/backend/access/heap/heapam.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index da0b70e..16d8c59 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -2250,7 +2250,7 @@ heap_prepare_insert(Relation relation, HeapTuple tup, TransactionId xid,
 	/*
 	 * For now, parallel operations are required to be strictly read-only.
 	 * Unlike heap_update() and heap_delete(), an insert should never create
-	 * a combo CID, so it might be possible to relax this restrction, but
+	 * a combo CID, so it might be possible to relax this restriction, but
 	 * not without more thought and testing.
 	 */
 	if (IsInParallelMode())
@@ -2666,7 +2666,7 @@ heap_delete(Relation relation, ItemPointer tid,
 	Assert(ItemPointerIsValid(tid));
 
 	/*
-	 * Forbid this during a parallel operation, lest it allocate a combocid.
+	 * Forbid this during a parallel operation, lets it allocate a combocid.
 	 * Other workers might need that combocid for visibility checks, and we
 	 * have no provision for broadcasting it to them.
 	 */
@@ -3124,7 +3124,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
 	Assert(ItemPointerIsValid(otid));
 
 	/*
-	 * Forbid this during a parallel operation, lest it allocate a combocid.
+	 * Forbid this during a parallel operation, lets it allocate a combocid.
 	 * Other workers might need that combocid for visibility checks, and we
 	 * have no provision for broadcasting it to them.
 	 */
@@ -5435,7 +5435,7 @@ heap_inplace_update(Relation relation, HeapTuple tuple)
 	/*
 	 * For now, parallel operations are required to be strictly read-only.
 	 * Unlike a regular update, this should never create a combo CID, so it
-	 * might be possible to relax this restrction, but not without more
+	 * might be possible to relax this restriction, but not without more
 	 * thought and testing.  It's not clear that it would be useful, anyway.
 	 */
 	if (IsInParallelMode())
-- 
2.1.4

From cf25445d9d21496b6927e9ef45e6c3815fef8ad5 Mon Sep 17 00:00:00 2001
From: Euler Taveira eu...@timbira.com
Date: Sun, 26 Apr 2015 14:24:26 -0300
Subject: [PATCH 2/3] Avoid compiler warnings about unused variables in
 assert-disabled builds.

---
 src/backend/utils/fmgr/funcapi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backend/utils/fmgr/funcapi.c b/src/backend/utils/fmgr/funcapi.c
index ebd7ddd..b9f2b06 100644
--- a/src/backend/utils/fmgr/funcapi.c
+++ b/src/backend/utils/fmgr/funcapi.c
@@ -887,7 +887,7 @@ get_func_trftypes(HeapTuple procTup,
   Oid **p_trftypes)
 {
 
-	Form_pg_proc procStruct = (Form_pg_proc) GETSTRUCT(procTup);
+	Form_pg_proc procStruct PG_USED_FOR_ASSERTS_ONLY = (Form_pg_proc) GETSTRUCT(procTup);
 	Datum		protrftypes;
 	ArrayType  *arr;
 	int			nelems;
-- 
2.1.4

From 7d1716fdf84f24a1dddfa02db27d532e06c92c3d Mon Sep 17 00:00:00 2001
From: Euler Taveira eu...@timbira.com
Date: Sun, 26 Apr 2015 14:52:39 -0300
Subject: [PATCH 3/3] Fix some more typos.

---
 src/backend/access/transam/README.parallel | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/backend/access/transam/README.parallel b/src/backend/access/transam/README.parallel
index c066fff..2257e4c 100644
--- a/src/backend/access/transam/README.parallel
+++ b/src/backend/access/transam/README.parallel
@@ -76,7 +76,7 @@ Instead, we take a more pragmatic approach. First, we try to make as many of
 the operations that are safe outside of parallel mode work correctly in
 parallel mode as well.  Second, we try to prohibit common unsafe operations
 via suitable error checks.  These checks are intended to catch 100% of
-unsafe things that a user might do from the SQL interface, but code writen
+unsafe things that a user might do from the SQL interface, but code written
 in C can do unsafe things that won't trigger these checks.  The error checks
 are engaged via EnterParallelMode(), which should be called before creating
 a parallel context, and disarmed via ExitParallelMode(), which should be
@@ -108,7 +108,7 @@ worker.  This includes:
   - The combo CID mappings.  This is needed to ensure consistent answers to
 tuple visibility checks.  The need to synchronize this data structure is
 a major reason why

Re: [HACKERS] INSERT ... ON CONFLICT syntax issues

* Peter Geoghegan (p...@heroku.com) wrote:
 On Sun, Apr 26, 2015 at 11:08 AM, Stephen Frost sfr...@snowman.net wrote:
  I don't want to accept something that automatically merges the
  excluded tuple (e.g., SET (*) = EXLCUDED.*), for reasons outlined
  here: https://wiki.postgresql.org/wiki/UPSERT#VoltDB.27s_UPSERT
 
  Perhaps I'm missing it, but the reasons that I see there appear to be:
 
  It'd be like SELECT * and we'd have to decide what to do about the
  value for unspecified columns.  As for the latter- we have to do that
  *anyway*, no?  What happens if you do:
 
  INSERT INTO mytable (foo, bar, baz, bat) VALUES 
  ('key1','key2','val1','val2')
  ON CONFLICT (foo) UPDATE SET (baz) = (EXCLUDED.baz);
 
  ?
 
 It's like any other UPDATE - the values of columns not appearing in
 the targetlist are unchanged from the original row version now
 superseded. It doesn't matter that you had some other values in the
 INSERT. You only get what you ask for.

Ok, that makes sense..  So is the concern that an INSERT would end up
getting default values while an UPDATE would preserve whatever's there?

I don't see that as an issue.

Are you still against having a way to say go forth and update whatever
non-conflicting columns I've specified in the INSERT, if there is a
conflict..?

Again, not saying it has to be done now, but it'd certainly be nice if
we had it initially because otherwise the ORMs and frameworks of the
world will be stuck supporting the more verbose approach for as long as
we support it (~5 years..).

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] INSERT ... ON CONFLICT syntax issues

* Peter Geoghegan (p...@heroku.com) wrote:
 On Sun, Apr 26, 2015 at 11:35 AM, Stephen Frost sfr...@snowman.net wrote:
  Ok, that makes sense..  So is the concern that an INSERT would end up
  getting default values while an UPDATE would preserve whatever's there?
 
  I don't see that as an issue.
 
 I think it easily could be.

Ok..  Can you elaborate on that?  Would it be an issue that's different
from the same thing done as independent commands?

Perhaps it'd be an issue for individuals who attempt to combine some
more complicated INSERT/UPDATE logic and don't realize that they'd get
whatever the existing value is for the non-specified columns rather than
the default value, but I'm sure they'd realize it on testing it and,
well, there's lots of ways users can misuse SQL and PG and get what they
expect 99% of the time (JOIN would be a great example..) only to have
things break one day.

  Are you still against having a way to say go forth and update whatever
  non-conflicting columns I've specified in the INSERT, if there is a
  conflict..?
 
  Again, not saying it has to be done now, but it'd certainly be nice if
  we had it initially because otherwise the ORMs and frameworks of the
  world will be stuck supporting the more verbose approach for as long as
  we support it (~5 years..).
 
 The more verbose approach is entirely necessary much of the time. For example:
 
 insert into upsert_race_test (index, count)
 values ('541','-1') on conflict update set count=TARGET.count + 
 EXCLUDED.count;
 
 Merging like this will be a very common requirement.

I was just about to reply to myself that I didn't intend to say that we
would remove the more verbose syntax but rather that they'd have to
use the more verbose syntax as long as we supported a release which 
*didn't* have the simpler syntax, which would be ~5 years.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] fix typos in comments

On 2015-04-26 12:53:30 -0400, Tom Lane wrote:
 Andres Freund and...@anarazel.de writes:
  On 2015-04-26 19:13:42 +0400, Dmitriy Olshevskiy wrote:
  - *  therefor it is up to the calling routine to
  + *  therefore it is up to the calling routine to
 
  I think both are actually legal? Yes therefore is more common, but
  still.
 
 Hm.  My dictionary says that therefor is archaic, but to my eye it
 looks just wrong.  Certainly no modern writer would spell it like that.

Mine said that it's still common in some circles, particularly the law,
so I thought I'd leave it alone.  I don't have that much of a 'feeling'
for english, strangely enough.

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] fix typos in comments

2015-04-26 Thread Tom Lane

Andres Freund and...@anarazel.de writes:
 On 2015-04-26 12:53:30 -0400, Tom Lane wrote:
 Hm.  My dictionary says that therefor is archaic, but to my eye it
 looks just wrong.  Certainly no modern writer would spell it like that.

 Mine said that it's still common in some circles, particularly the law,
 so I thought I'd leave it alone.  I don't have that much of a 'feeling'
 for english, strangely enough.

Well, a quick grep says that our source tree contains 2 occurrences of
therefor (in pqcomm.c and geqo_erx.c), versus 700+ occurrences of
therefore.  So I'd be inclined to standardize on the latter.

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] Fwd: [GENERAL] 4B row limit for CLOB tables

* Álvaro Hernández Tortosa (a...@8kdata.com) wrote:
 It's worth document but also, as I said, maybe also fixing them,
 so that if three years from now they really show up, solution is
 already in production (rather than in patching state).

With the proliferation of JSON usage in PG thanks to jsonb, I'd count us
lucky if we don't get complaints about this in the next three years.

I don't expect to have time to work on it in the near future,
unfortunately, but Robert's thoughts on supporting a new TOAST pointer
structure (with a way to support what's currently there, to avoid an
on-disk break) seems like a good starting point to me.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] INSERT ... ON CONFLICT syntax issues

On Sun, Apr 26, 2015 at 11:35 AM, Stephen Frost sfr...@snowman.net wrote:
 Ok, that makes sense..  So is the concern that an INSERT would end up
 getting default values while an UPDATE would preserve whatever's there?

 I don't see that as an issue.

I think it easily could be.

 Are you still against having a way to say go forth and update whatever
 non-conflicting columns I've specified in the INSERT, if there is a
 conflict..?

 Again, not saying it has to be done now, but it'd certainly be nice if
 we had it initially because otherwise the ORMs and frameworks of the
 world will be stuck supporting the more verbose approach for as long as
 we support it (~5 years..).

The more verbose approach is entirely necessary much of the time. For example:

insert into upsert_race_test (index, count)
values ('541','-1') on conflict update set count=TARGET.count + EXCLUDED.count;

Merging like this will be a very common requirement.
-- 
Peter Geoghegan


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

Re: [HACKERS] fix typos in comments

On 2015-04-26 13:03:52 -0400, Tom Lane wrote:
 Andres Freund and...@anarazel.de writes:
  On 2015-04-26 12:53:30 -0400, Tom Lane wrote:
  Hm.  My dictionary says that therefor is archaic, but to my eye it
  looks just wrong.  Certainly no modern writer would spell it like that.
 
  Mine said that it's still common in some circles, particularly the law,
  so I thought I'd leave it alone.  I don't have that much of a 'feeling'
  for english, strangely enough.
 
 Well, a quick grep says that our source tree contains 2 occurrences of
 therefor (in pqcomm.c and geqo_erx.c), versus 700+ occurrences of
 therefore.  So I'd be inclined to standardize on the latter.

Done.

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] INSERT ... ON CONFLICT syntax issues

Peter,

* Peter Geoghegan (p...@heroku.com) wrote:
 On Sun, Apr 26, 2015 at 6:34 AM, Stephen Frost sfr...@snowman.net wrote:
  What's important, in my view, is to keep the simple case simple and so
  I'm not particularly wedded to any of these approaches, just trying to
  help with other suggestions.
 
  INSERT INTO mytable VALUES ('key1','key2','val1','val2')
  ON CONFLICT UPDATE SET val1 = 'val1', val2 = 'val2';
 
  strikes me as a the 99% use-case here that we need to keep sane, and
  it'd be really nice if we didn't have to include the SET clause and
  duplicate those values at all..  That could be something we add later
  though, I don't think it needs to be done now.
 
 You can do that already. That's what the EXCLUDED.* alias that is
 automatically added is for (the thing that Andres disliked the
 spelling of - or the other thing). This is legal, for example:
 
 INSERT INTO mytable (foo, bar, baz, bat) VALUES ('key1','key2','val1','val2')
 ON CONFLICT (foo) UPDATE SET (foo, bar, baz, bat) = (EXCLUDED.foo,
 EXCLUDED.bar, EXCLUDED.baz, EXCLUDED.bat)';

Yeah, that's not exactly simpler and I don't expect to see it used very
often (as in, less than 1%) because of that.

 I don't want to accept something that automatically merges the
 excluded tuple (e.g., SET (*) = EXLCUDED.*), for reasons outlined
 here: https://wiki.postgresql.org/wiki/UPSERT#VoltDB.27s_UPSERT

Perhaps I'm missing it, but the reasons that I see there appear to be:

It'd be like SELECT * and we'd have to decide what to do about the
value for unspecified columns.  As for the latter- we have to do that
*anyway*, no?  What happens if you do:

INSERT INTO mytable (foo, bar, baz, bat) VALUES ('key1','key2','val1','val2')
ON CONFLICT (foo) UPDATE SET (baz) = (EXCLUDED.baz);

?

As for the SELECT * concern, I fail to see how it's any different from
the exact same currently-encouraged usage of INSERT + UPDATE:

INSERT INTO mytable (foo, bar, baz, bat) VALUES ('key1','key2','val1','val2');

... catch the exception

UPDATE mytable SET baz = 'val1', bat = 'val2' WHERE foo = 'key1' and bar = 
'key2';

Clearly there are issues with the above if someone is running around
adding columns to tables and PG has to figure out if we should be
setting the non-mentioned columns to NULL or to the default for the
column, but we're all quite happy to do so and trust that whomever is
adding the column has set a sane default and that PG will use it when
the column isn't included in either the INSERT or the UPDATE.

Note that I wasn't suggesting your SET (*) = EXLCUDED.* syntax and if
that would expand to something different than what I've outlined above
then it would make sense to not include it (... or fix it to act the
same, and then it's just a more verbose approach).

Further, this is *very* different from how the SELECT * concern can
cause things to break unexpectedly- new columns end up getting returned
which the application is unlikely to be prepared for.  That doesn't
happen here and so I don't believe it makes any sense to try and compare
the two.

Happy to discuss, of course, and apologies if I missed some other issue-
I was just reading what I found at the link provided.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] INSERT ... ON CONFLICT syntax issues

On Sun, Apr 26, 2015 at 11:08 AM, Stephen Frost sfr...@snowman.net wrote:
 I don't want to accept something that automatically merges the
 excluded tuple (e.g., SET (*) = EXLCUDED.*), for reasons outlined
 here: https://wiki.postgresql.org/wiki/UPSERT#VoltDB.27s_UPSERT

 Perhaps I'm missing it, but the reasons that I see there appear to be:

 It'd be like SELECT * and we'd have to decide what to do about the
 value for unspecified columns.  As for the latter- we have to do that
 *anyway*, no?  What happens if you do:

 INSERT INTO mytable (foo, bar, baz, bat) VALUES ('key1','key2','val1','val2')
 ON CONFLICT (foo) UPDATE SET (baz) = (EXCLUDED.baz);

 ?

It's like any other UPDATE - the values of columns not appearing in
the targetlist are unchanged from the original row version now
superseded. It doesn't matter that you had some other values in the
INSERT. You only get what you ask for.

-- 
Peter Geoghegan


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

Re: [HACKERS] PATCH: default_index_tablespace

J.L.,

* jltal...@adv-solutions.net (jltal...@adv-solutions.net) wrote:
 Any suggestions on how to do it properly?
 Does Greg Stark's suggestion (at
 CAM-w4HPOASwsQMdGZqjyFHNubbUnWrUAo8ibci-97UKU=po...@mail.gmail.com)
 make sense to you?
 This approach might suffer from the same problem as mine, though.

Well, Greg's suggestion was intended to specifically avoid breaking
pg_dump by having two new GUCs and having default_tablespace take
precedence, if set.

 It seems to me, IMVHO, a limitation in pg_dump ---since 8.0 when
 tablespace support for CREATE INDEX was implemented--- that we
 should fix.
 Keeping backwards compatibility is indeed required; I just did not
 think about pg_dump at all :(

Limitation strikes me as not quite the right term, but I certainly agree
that it's unfortunate that pg_dump uses that GUC instead of adding the
TABLESPACE clause to the CREATE INDEX, then again, there are likely to
be historical reasons for that.

Unfortunately, not break existing pg_dump-generated files is pretty
tough.

 I don't mind reworking the patch or redoing it completely once there
 is a viable solution.

Having three GUCs in the end might work but it seems kind of grotty to
have the more-specific GUCs be overridden by the less-specific GUC.
We could throw a warning if the more-specific GUC is attempted to be set
while the less-specific GUC is set, and vis-versa, and essentially make
them either/or.  That'd cause additional warnings to be thrown when
restoring an older dump, but if pg_dump was modified to use the
TABLESPACE clause for CREATE INDEX for new dump files then that's only a
temporary situation.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] INSERT ... ON CONFLICT syntax issues

On Sun, Apr 26, 2015 at 11:43 AM, Stephen Frost sfr...@snowman.net wrote:
 I think it easily could be.

 Ok..  Can you elaborate on that?  Would it be an issue that's different
 from the same thing done as independent commands?

I think that the stuff I linked to describes my concerns exhaustively.
In any case, it's a discussion for another day.


-- 
Peter Geoghegan


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

[HACKERS] INSERT ... ON CONFLICT UPDATE/IGNORE 4.0

I have pushed my patch, newly rebased, to a new branch on my personal
Github account (branch: insert_conflict_4):

https://github.com/petergeoghegan/postgres/commits/insert_conflict_4

I'm not going to attach a patch here at all. Andres and Heikki should
now push their changes to that branch (or alternatively, Andres can
ask me to merge his stuff into it).

It's make-or-break time for this patch. Please help me get it over the
line in time. Heikki is in Northern California this week, and I think
we'll have time to talk about the patch, which I expect will help (an
in-person chat with Andres in NYC certainly helped *a lot*). But that
also means that he's going to be travelling a long distance, and we
can assume will have reduced availability for the next couple of days.

Notable changes
=

* Work of Heikki, myself and Andres from the last week or so rebased
to be cumulative (as before, ON CONFLICT IGNORE - RTE changes - ON
CONFLICT UPDATE). Would apply cleanly to today's git master branch.

* Improved INSERT documentation [1].

* Minor style tweaks to RTE change commit.

* Improved commit messages. Importantly, these have attribution that I
think fairly reflects everyone's individual contribution. Please let
me know if I missed something.

* Most importantly, RLS changes.

The RLS patch is now significantly simpler than before. In general,
I'm very happy with how the new approach to RLS enforcement, and the
new WCO kind field has resulted in a far simpler RLS patch. This
needs the scrutiny of a subject matter expert like Stephen or Dean. I
would be happy to grant either git access to my personal branch, to
push out code changes or tests as they see fit. Just e-mail me
privately with the relevant details.

Remaining challenges
=

* As discussed in a dedicated thread, we're probably going to have to
tweak the syntax a bit. No need to hold what I have here up for that,
though (provisionally, this version still puts inference WHERE clause
to infer partial indexes in parens). Let's confine the discussion of
that to its dedicated thread. These issues probably apply equally to
the IGNORE variant, and so can be considered a blocker to its commit
(and not just ON CONFLICT UPDATE).

* So far, there has been a lack of scrutiny about what the patch does
in the rewriter (in particular, to support the EXCLUDED.* pseudo-alias
expression) and optimizer (the whole concept of an auxiliary
query/plan that share a target RTE, and later target ResultRelation).
If someone took a close look at that, it would be most helpful.
ruleutils.c is also modified for the benefit of EXPLAIN output. This
all applies only to the ON CONFLICT UPDATE patch. A committer could
push out the IGNORE patch before this was 100% firm.

* I privately pointed out to Heikki what I'd said publicly about 6
weeks ago: that there is still a *very* small chance of exclusion
constraints exhibiting unprincipled deadlocks (he missed it at the
time). I think that this risk is likely to be acceptable, since it
takes so much to see it happen (and ON CONFLICT UPDATE/nbtree is
unaffected). But let's better characterize the risks, particularly in
light of the changes to store speculative tokens in the c_ctid field
on newly inserted (speculative) tuples. I think that that probably
made the problem significantly less severe, and perhaps it's now
entirely theoretical, but I want to make sure. I'm going to try and
characterize the risks with the patch here today.

Thanks

[1]
http://postgres-benchmarks.s3-website-us-east-1.amazonaws.com/on-conflict-docs/sql-insert.html
--
Peter Geoghegan

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

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

2015-04-26 Thread Kouhei Kaigai

The attached patch v13 is revised one according to the suggestion
by Robert.

- eliminated useless change in allpaths.c
- eliminated an extra space in FdwRoutine definition
- prohibited to have scanrelid==0 by other than ForeignScan
  or CustomScan, using Assert()
- definition of currentRelation in ExecInitForeignScan() and
  ExecInitCustomScan() were moved inside of the if-block on
  scanrelid  0
- GetForeignJoinPaths() was redefined and moved to
  add_paths_to_joinrel(), like set_join_pathlist_hook.

As suggested, FDW driver can skip to add additional paths if
equivalent paths are already added to a certain joinrel by
checking fdw_private. So, we can achieve the purpose when we
once moved the entrypoint to make_join_rel() - no to populate
redundant paths for each potential join combinations, even
though remote RDBMS handles it correctly. It also makes sense
if remote RDBMS handles tables join according to the order of
relations appear.

Its definition is below:
  void GetForeignJoinPaths(PlannerInfo *root,
   RelOptInfo *joinrel,
   RelOptInfo *outerrel,
   RelOptInfo *innerrel,
   List *restrictlist,
   JoinType jointype,
   SpecialJoinInfo *sjinfo,
   SemiAntiJoinFactors *semifactors,
   Relids param_source_rels,
   Relids extra_lateral_rels);

In addition to the arguments in the previous version, we added
some parameters computed during add_paths_to_joinrel().
Right now, I'm not certain whether we should include mergeclause_list
here, because it depends on enable_mergejoin even though extra join
logic based on merge-join may not want to be controlled by this GUC.

Hanada-san, could you adjust your postgres_fdw patch according to
the above new (previous?) definition.

Thanks,
--
NEC Business Creation Division / PG-Strom Project
KaiGai Kohei kai...@ak.jp.nec.com


 -Original Message-
 From: Kaigai Kouhei(海外 浩平)
 Sent: Friday, April 24, 2015 11:23 PM
 To: 'Robert Haas'
 Cc: Tom Lane; Thom Brown; Shigeru Hanada; pgsql-hackers@postgreSQL.org
 Subject: Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)
 
  On Wed, Apr 22, 2015 at 10:48 PM, Kouhei Kaigai kai...@ak.jp.nec.com 
  wrote:
   +   else if (scan-scanrelid == 0 
   +(IsA(scan, ForeignScan) || IsA(scan,
 CustomScan)))
   +   varno = INDEX_VAR;
  
   Suppose scan-scanrelid == 0 but the scan type is something else?  Is
   that legal?  Is varno == 0 the correct outcome in that case?
  
   Right now, no other scan type has capability to return a tuples
   with flexible type/attributes more than static definition.
   I think it is a valid restriction that only foreign/custom-scan
   can have scanrelid == 0.
 
  But the code as you've written it doesn't enforce any such
  restriction.  It just spends CPU cycles testing for a condition which,
  to the best of your knowledge, will never happen.
 
  If it's really a can't happen condition, how about checking it via an 
  Assert()?
 
  else if (scan-scanrelid == 0)
  {
  Assert(IsA(scan, ForeignScan) || IsA(scan, CustomScan));
  varno = INDEX_VAR;
  }
 
 Thanks for your suggestion. I'd like to use this idea on the next patch.
 
 --
 NEC Business Creation Division / PG-Strom Project
 KaiGai Kohei kai...@ak.jp.nec.com


pgsql-v9.5-custom-join.v13.patch
Description: pgsql-v9.5-custom-join.v13.patch

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

Re: [HACKERS] Typo in a comment in set_rel_size()

2015-04-26 Thread Amit Langote

On 2015-04-25 AM 04:20, Tom Lane wrote:  *
 
 It's not a typo; the comment was correct when written.  But I evidently
 missed updating it when set_append_rel_pathlist() got split into two
 functions.  Applied, thanks for noticing!
 

Ah, thanks!

Amit



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

Re: [HACKERS] INSERT ... ON CONFLICT UPDATE/IGNORE 4.0