Re: [PATCH] session_replication_role = replica with TRUNCATE

[Craig Ringer]

On 30 December 2017 at 03:32, Petr Jelinek 
wrote:

> On 29/12/17 16:53, Marco Nenciarini wrote:
> > Il 29/12/17 15:14, Petr Jelinek ha scritto:
> >>
> >> May be worth documenting that the session_replication_role also affects
> >> TRUNCATE's interaction with FKs in config.sgml.
> >>
> >
> > The current documentation of session_replication_role GUC is:
> >
> > Controls firing of replication-related triggers and rules for the
> > current session. Setting this variable requires superuser privilege
> > and results in discarding any previously cached query plans.
> > Possible values are origin (the default), replica and local.
> > See ALTER TABLE for more information.
> >
> > It doesn't speak about foreign keys or referential integrity, but only
> > about triggers and rules. I don't think that it's worth to add a special
> > case for truncate, unless we want to expand/rewrite the documentation to
> > specify all the effects in the details.
> >
>
> The effects on foreign keys is implied by the fact that for DML it's
> implemented using triggers, but that's not true for TRUNCATE. In any
> case it does not hurt to mention the FKs explicitly rather than
> implicitly here.


Yeah. I'd argue that's an oversight in the current docs that "can cause FK
violations" isn't mentioned. That's kind of important, and FKs being
triggers is implementation detail we shouldn't be relying on users to know.

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

CONSTANT/NOT NULL/initializer properties for plpgsql record variables

[Tom Lane]

I said a couple of times in recent threads that it wouldn't be too hard
to implement $SUBJECT given the other patches I've been working on.
Attached is proof of the pudding.  This needs to be applied on top of
the patches in
https://postgr.es/m/23537.1514589...@sss.pgh.pa.us
and
https://postgr.es/m/11986.1514407...@sss.pgh.pa.us

It's pretty straightforward really.  Worth noting is that this also
fixes the null-domain-value issues I mentioned as being a loose end
in the first of the above-referenced messages.  Also, I created a
new plpgsql test file for these features, and moved the one relevant
existing test case into that file.

I was a bit disappointed to find that no documentation changes seem
needed, because the SGML docs fail to acknowledge that these cases
didn't work ...

regards, tom lane

diff --git a/src/pl/plpgsql/src/Makefile b/src/pl/plpgsql/src/Makefile
index f9a3616..ad93e3e 100644
*** a/src/pl/plpgsql/src/Makefile
--- b/src/pl/plpgsql/src/Makefile
*** DATA = plpgsql.control plpgsql--1.0.sql 
*** 26,32 
  
  REGRESS_OPTS = --dbname=$(PL_TESTDB)
  
! REGRESS = plpgsql_call plpgsql_record
  
  all: all-lib
  
--- 26,32 
  
  REGRESS_OPTS = --dbname=$(PL_TESTDB)
  
! REGRESS = plpgsql_call plpgsql_record plpgsql_varprops
  
  all: all-lib
  
diff --git a/src/pl/plpgsql/src/expected/plpgsql_varprops.out b/src/pl/plpgsql/src/expected/plpgsql_varprops.out
index ...109056c .
*** a/src/pl/plpgsql/src/expected/plpgsql_varprops.out
--- b/src/pl/plpgsql/src/expected/plpgsql_varprops.out
***
*** 0 
--- 1,300 
+ --
+ -- Tests for PL/pgSQL variable properties: CONSTANT, NOT NULL, initializers
+ --
+ create type var_record as (f1 int4, f2 int4);
+ create domain int_nn as int not null;
+ create domain var_record_nn as var_record not null;
+ create domain var_record_colnn as var_record check((value).f2 is not null);
+ -- CONSTANT
+ do $$
+ declare x constant int := 42;
+ begin
+   raise notice 'x = %', x;
+ end$$;
+ NOTICE:  x = 42
+ do $$
+ declare x constant int;
+ begin
+   x := 42;  -- fail
+ end$$;
+ ERROR:  variable "x" is declared CONSTANT
+ LINE 4:   x := 42;  -- fail
+   ^
+ do $$
+ declare x constant int; y int;
+ begin
+   for x, y in select 1, 2 loop  -- fail
+   end loop;
+ end$$;
+ ERROR:  variable "x" is declared CONSTANT
+ LINE 4:   for x, y in select 1, 2 loop  -- fail
+   ^
+ do $$
+ declare x constant int[];
+ begin
+   x[1] := 42;  -- fail
+ end$$;
+ ERROR:  variable "x" is declared CONSTANT
+ LINE 4:   x[1] := 42;  -- fail
+   ^
+ do $$
+ declare x constant int[]; y int;
+ begin
+   for x[1], y in select 1, 2 loop  -- fail (currently, unsupported syntax)
+   end loop;
+ end$$;
+ ERROR:  syntax error at or near "["
+ LINE 4:   for x[1], y in select 1, 2 loop  -- fail (currently, unsup...
+^
+ do $$
+ declare x constant var_record;
+ begin
+   x.f1 := 42;  -- fail
+ end$$;
+ ERROR:  variable "x" is declared CONSTANT
+ LINE 4:   x.f1 := 42;  -- fail
+   ^
+ do $$
+ declare x constant var_record; y int;
+ begin
+   for x.f1, y in select 1, 2 loop  -- fail
+   end loop;
+ end$$;
+ ERROR:  variable "x" is declared CONSTANT
+ LINE 4:   for x.f1, y in select 1, 2 loop  -- fail
+   ^
+ -- initializer expressions
+ do $$
+ declare x int := sin(0);
+ begin
+   raise notice 'x = %', x;
+ end$$;
+ NOTICE:  x = 0
+ do $$
+ declare x int := 1/0;  -- fail
+ begin
+   raise notice 'x = %', x;
+ end$$;
+ ERROR:  division by zero
+ CONTEXT:  SQL statement "SELECT 1/0"
+ PL/pgSQL function inline_code_block line 3 during statement block local variable initialization
+ do $$
+ declare x bigint[] := array[1,3,5];
+ begin
+   raise notice 'x = %', x;
+ end$$;
+ NOTICE:  x = {1,3,5}
+ do $$
+ declare x record := row(1,2,3);
+ begin
+   raise notice 'x = %', x;
+ end$$;
+ NOTICE:  x = (1,2,3)
+ do $$
+ declare x var_record := row(1,2);
+ begin
+   raise notice 'x = %', x;
+ end$$;
+ NOTICE:  x = (1,2)
+ -- NOT NULL
+ do $$
+ declare x int not null;  -- fail
+ begin
+   raise notice 'x = %', x;
+ end$$;
+ ERROR:  variable "x" must have a default value, since it's declared NOT NULL
+ LINE 2: declare x int not null;  -- fail
+   ^
+ do $$
+ declare x int not null := 42;
+ begin
+   raise notice 'x = %', x;
+   x := null;  -- fail
+ end$$;
+ NOTICE:  x = 42
+ ERROR:  null value cannot be assigned to variable "x" declared NOT NULL
+ CONTEXT:  PL/pgSQL function inline_code_block line 5 at assignment
+ do $$
+ declare x int not null := null;  -- fail
+ begin
+   raise notice 'x = %', x;
+ end$$;
+ ERROR:  null value cannot be assigned to variable "x" declared NOT NULL
+ CONTEXT:  PL/pgSQL function inline_code_block line 3 during statement block local variable initialization
+ do $$
+ declare x record not null;  -- fail
+ begin
+   raise notice 'x = %', x;
+ end$$;
+ ERROR:  variable "x" must have a default value, since it's declared NOT NULL
+ LINE 2: declare x record not n

Re: Converting plpgsql to use DTYPE_REC for named composite types

[Tom Lane]

I wrote:
> I hacked on the domain-support problem a bit and it worked out well,
> so attached is a revised patch that incorporates that.  This caused
> me to revise some assumptions about what expandedrecord.c's internal
> invariants ought to be, so it's probably better to look at this as
> a new patch rather than a diff from v1.

Oh, I'd not looked at the documentation angle of this.  Here's a short
add-on patch which just adds the fact that "record" is now a valid
argument type, and removes a no-longer-correct claim that system columns
are always inaccessible in row variables.

The documentation draws a distinction between "record" and "row" variables
which is now rather artificial so far as the code is concerned.  But it's
not totally pointless either, since it's still true that a variable
declared "record" will mutate its type in a way that a
named-composite-type variable will not.  I'm inclined to leave that text
as is; anyone think differently?

regards, tom lane
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 7d23ed4..86d28fb 100644
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
***
*** 123,129 
   and they can return a result of any of these types.  They can also
   accept or return any composite type (row type) specified by name.
   It is also possible to declare a PL/pgSQL
!  function as returning record, which means that the result
   is a row type whose columns are determined by specification in the
   calling query, as discussed in .
  
--- 123,131 
   and they can return a result of any of these types.  They can also
   accept or return any composite type (row type) specified by name.
   It is also possible to declare a PL/pgSQL
!  function as accepting record, which means that any
!  composite type will do as input, or
!  as returning record, which means that the result
   is a row type whose columns are determined by specification in the
   calling query, as discussed in .
  
*** user_id users.user_id%TYPE;
*** 672,685 
 
  
 
- Only the user-defined columns of a table row are accessible in a
- row-type variable, not the OID or other system columns (because the
- row could be from a view).  The fields of the row type inherit the
- table's field size or precision for data types such as
- char(n).
-
- 
-
  Here is an example of using composite types.  table1
  and table2 are existing tables having at least the
  mentioned fields:
--- 674,679 

Re: Why standby restores some WALs many times from archive?

[Sergey Burladyan]

Jeff Janes  writes:

> On Thu, Dec 28, 2017 at 7:02 AM, Victor Yagofarov  wrote:
> > I have postgres 9.4 standby with archive-based replication (via
> > restore_command).

> Can you show us both your archive_command and your restore_command?

We use this scripts:
https://github.com/avito-tech/dba-utils/tree/master/pg_archive

But I can reproduce problem with simple cp & mv:
archive_command:
  test ! -f /var/lib/postgresql/wals/%f && \
  test ! -f /var/lib/postgresql/wals/%f.tmp && \
  cp %p /var/lib/postgresql/wals/%f.tmp && \
  mv /var/lib/postgresql/wals/%f.tmp /var/lib/postgresql/wals/%f

recovery.conf:
  restore_command = 'cp /var/lib/postgresql/wals/%f %p'
  archive_cleanup_command = 'pg_archivecleanup /var/lib/postgresql/wals/ %r'
  standby_mode = on

I use docker for tests and run standby with DEBUG3 (full log in attachments).

I run this commands in test:
PGVER=9.4 docker-compose up
docker-compose exec -u postgres master psql \
-c "create database test"
docker-compose exec -u postgres master psql -d test \
-c "create table xxx (id serial, v text)"
# we need some recycled WALs
docker-compose exec -u postgres master psql -d test \
-c "insert into xxx (v) select 'this is long line for fill space ' || n 
from generate_series(1, 500) n"
docker-compose exec -u postgres standby psql -c "checkpoint"
docker-compose exec -u postgres master psql -d test \
-c "insert into xxx (v) select 'this is long line for fill space ' || n 
from generate_series(1, 100) n; truncate xxx;"
docker-compose logs standby | grep 'restored log file' | sort | uniq -c | 
awk '$1 > 1 { print }'
docker-compose down -v

Log have two duplicated WALS:
4 | 00010024
3 | 0001002B


DEBUG:  switched WAL source from stream to archive after failure
DEBUG:  executing restore command "cp 
/var/lib/postgresql/wals/00010024 pg_xlog/RECOVERYXLOG"
LOG:  restored log file "00010024" from archive
DEBUG:  got WAL segment from archive
DEBUG:  skipping restartpoint, already performed at 0/22258058
DEBUG:  executing restore command "cp 
/var/lib/postgresql/wals/00010025 pg_xlog/RECOVERYXLOG"
cp: cannot stat ‘/var/lib/postgresql/wals/00010025’: No such 
file or directory
DEBUG:  could not restore file "00010025" from archive: child 
process exited with exit code 1
DEBUG:  unexpected pageaddr 0/1000 in log segment 00010025, 
offset 0
DEBUG:  switched WAL source from archive to stream after failure
DEBUG:  switched WAL source from stream to archive after failure
DEBUG:  executing restore command "cp 
/var/lib/postgresql/wals/00010024 pg_xlog/RECOVERYXLOG"
LOG:  restored log file "00010024" from archive
DEBUG:  got WAL segment from archive
DEBUG:  skipping restartpoint, already performed at 0/22258058
DEBUG:  executing restore command "cp 
/var/lib/postgresql/wals/00010025 pg_xlog/RECOVERYXLOG"
cp: cannot stat ‘/var/lib/postgresql/wals/00010025’: No such 
file or directory
DEBUG:  could not restore file "00010025" from archive: child 
process exited with exit code 1
DEBUG:  unexpected pageaddr 0/1000 in log segment 00010025, 
offset 0
DEBUG:  switched WAL source from archive to stream after failure
DEBUG:  switched WAL source from stream to archive after failure
DEBUG:  executing restore command "cp 
/var/lib/postgresql/wals/00010024 pg_xlog/RECOVERYXLOG"
LOG:  restored log file "00010024" from archive
DEBUG:  got WAL segment from archive
DEBUG:  skipping restartpoint, already performed at 0/22258058



LOG:  restartpoint starting: xlog
DEBUG:  performing replication slot checkpoint
DEBUG:  switched WAL source from stream to archive after failure
DEBUG:  executing restore command "cp 
/var/lib/postgresql/wals/0001002C pg_xlog/RECOVERYXLOG"
cp: cannot stat ‘/var/lib/postgresql/wals/0001002C’: No such 
file or directory
DEBUG:  could not restore file "0001002C" from archive: child 
process exited with exit code 1
LOG:  unexpected pageaddr 0/2100 in log segment 0001002C, 
offset 0
DEBUG:  switched WAL source from archive to stream after failure
DEBUG:  switched WAL source from stream to archive after failure
DEBUG:  executing restore command "cp 
/var/lib/postgresql/wals/0001002B pg_xlog/RECOVERYXLOG"
LOG:  restored log file "0001002B" from archive
DEBUG:  got WAL segment from archive
DEBUG:  executing restore command "cp 
/var/lib/postgresql/wals/0001002C pg_xlog/RECOVERYXLOG"
cp: cannot stat ‘/var/lib/postgresql/wals/0001002C’: No such 
file or directory
DEBUG:  could not restore file "00

Re: Fix a Oracle-compatible instr function in the documentation

[Tatsuo Ishii]

> Hi,
> 
> Attached is a patch to fix a very trivial issue of the documentation.
> 
> The documentation of PL/pgSQL provides sample codes of Oracle-compatible
> instr functions. However, the behaviour is a little differet.
> Oracle's instr raises an error when the forth argument value is less than
> zero, but the sample code returns zero. This patch fixes this.

Do we need treat this as a bug fix? If so, do we need to back patch as
well?

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] taking stdbool.h into use

[Michael Paquier]

On Fri, Dec 29, 2017 at 12:33:24PM -0500, Tom Lane wrote:
> Alvaro Herrera  writes:
> > Michael Paquier wrote:
> >> I have looked at 0002 and 0003. Those look good to ship for me.
> 
> > Yeah, I'd vote to push those right away to see what buildfarm has to
> > say.  That way you can push 0001 shortly after the dust settles (if
> > any), which will have an effect on the bootstrap data format patch.
> 
> Yeah, I think all of this is at the point where the next thing to do
> is see what the buildfarm has to say.  I could test it manually on
> prairiedog, but I'd just as soon let the buildfarm script do the work.
> 
> It does make sense, probably, to push 0001-0003 first and see if
> anything turns up from that, then 0004.

I have not looked at 0001 in details yet, which was going to be my next
step. If you could wait for at least two days that would be nice to give
me some room.

0002 and 0003 are independent on the rest, which is why I looked at them
first. Would we want to actually backpatch them at some point? Perhaps
not per the lack of complains in this area.
--
Michael


signature.asc
Description: PGP signature

Re: Converting plpgsql to use DTYPE_REC for named composite types

[Tom Lane]

I wrote:
> I'll stick this into the January commitfest, but I'd like to get it
> reviewed and committed pretty soon, because there are follow-on patches
> that need to get done in time for v11 --- in particular, we need to close
> out the lack of plpgsql support for domains-over-composite.

I hacked on the domain-support problem a bit and it worked out well,
so attached is a revised patch that incorporates that.  This caused
me to revise some assumptions about what expandedrecord.c's internal
invariants ought to be, so it's probably better to look at this as
a new patch rather than a diff from v1.

Mostly this is changes internal to expandedrecord.c, but I cleaned up
some code in plpgsql that tests for domain-ness, and I also added support
in ExecEvalFieldSelect for extracting a field directly from an expanded
record without flattening it into a tuple first.  It hadn't been clear
that that was worth the trouble before, but it definitely does come up
while applying domain constraints.  For example, having that fast path
makes about a 2X speed difference in a test like this:

create type two_ints as (f1 int, f2 int);
create domain ordered_ints as two_ints check((value).f1 <= (value).f2);

\timing on

do $$
declare d ordered_ints;
begin
  for i in 1..300 loop
d.f2 := i;
d.f1 := i;
  end loop;
end$$;


There are still a couple of soft spots having to do with enforcing
domain constraints against null composite values, e.g. if there's
a constraint that would reject a null value we don't notice that
at DECLARE time.  I think there's not much point in being strict
about that until we have support for initializers for composite
variables.  Which is definitely worth doing but it seems like it
could be a separate patch.

The patches in <11986.1514407...@sss.pgh.pa.us> still apply over this
with just some line-number offsets, so I won't post a new version
of those.

regards, tom lane



use-dtype-rec-for-all-composites-2.patch.gz
Description: use-dtype-rec-for-all-composites-2.patch.gz

FOR EACH ROW triggers on partitioned tables

[Alvaro Herrera]

This patch enables FOR EACH ROW triggers on partitioned tables.

As presented, this patch is sufficient to discuss the semantics that we
want for triggers on partitioned tables, which is the most pressing
question here ISTM.

However, this is incomplete: it doesn't create triggers when you do
ALTER TABLE ATTACH PARTITION or by CREATE TABLE PARTITION OF.  I'm using
this as a basis on which to try foreign keys for partitioned tables.
Getting this to committable status requires adding those features.

This is essentially the same patch I posted as 0003 in
https://postgr.es/m/20171220194937.pldcecyx7yrwmgkg@alvherre.pgsql

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 4caccf4dc0ab25de37a109170052f98273450468 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera 
Date: Tue, 21 Nov 2017 15:53:11 -0300
Subject: [PATCH v1] Allow FOR EACH ROW triggers on partitioned tables

---
 src/backend/commands/trigger.c | 86 +++---
 src/test/regress/expected/triggers.out | 83 +++-
 src/test/regress/sql/triggers.sql  | 63 -
 3 files changed, 202 insertions(+), 30 deletions(-)

diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 92ae3822d8..eb6b25b28c 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -133,6 +133,10 @@ static bool before_stmt_triggers_fired(Oid relid, CmdType 
cmdType);
  * relation, as well as ACL_EXECUTE on the trigger function.  For internal
  * triggers the caller must apply any required permission checks.
  *
+ * When called on partitioned tables, this function recurses to create the
+ * trigger on all the partitions, except if isInternal is true, in which
+ * case caller is expected to execute recursion on its own.
+ *
  * Note: can return InvalidObjectAddress if we decided to not create a trigger
  * at all, but a foreign-key constraint.  This is a kluge for backwards
  * compatibility.
@@ -179,8 +183,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
 * Triggers must be on tables or views, and there are additional
 * relation-type-specific restrictions.
 */
-   if (rel->rd_rel->relkind == RELKIND_RELATION ||
-   rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+   if (rel->rd_rel->relkind == RELKIND_RELATION)
{
/* Tables can't have INSTEAD OF triggers */
if (stmt->timing != TRIGGER_TYPE_BEFORE &&
@@ -190,13 +193,59 @@ CreateTrigger(CreateTrigStmt *stmt, const char 
*queryString,
 errmsg("\"%s\" is a table",

RelationGetRelationName(rel)),
 errdetail("Tables cannot have INSTEAD 
OF triggers.")));
-   /* Disallow ROW triggers on partitioned tables */
-   if (stmt->row && rel->rd_rel->relkind == 
RELKIND_PARTITIONED_TABLE)
+   }
+   else if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+   {
+   /* Partitioned tables can't have INSTEAD OF triggers */
+   if (stmt->timing != TRIGGER_TYPE_BEFORE &&
+   stmt->timing != TRIGGER_TYPE_AFTER)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-errmsg("\"%s\" is a partitioned table",
+errmsg("\"%s\" is a table",

RelationGetRelationName(rel)),
-errdetail("Partitioned tables cannot 
have ROW triggers.")));
+errdetail("Tables cannot have INSTEAD 
OF triggers.")));
+   /*
+* FOR EACH ROW triggers have further restrictions
+*/
+   if (stmt->row)
+   {
+   /*
+* Disallow WHEN clauses; I think it's okay, but 
disallow for now
+* to reduce testing surface.
+*/
+   if (stmt->whenClause)
+   ereport(ERROR,
+   
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+errmsg("\"%s\" is a 
partitioned table",
+   
RelationGetRelationName(rel)),
+errdetail("Triggers FOR EACH 
ROW on partitioned table cannot have WHEN clauses.")));
+
+   /*
+* BEFORE triggers FOR EACH ROW are forbidden, because 
they would
+* allow the user to direct the row to another 
partition, which
+* isn

Re: pgbench - add \if support

[Fabien COELHO]

Another rebase after the pow function commit.


Mostly a rebase after zipfian function commit.


--
Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 1519fe7..7068063 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -895,6 +895,21 @@ pgbench  options  d
   
 
   
+   
+\if expression
+\elif expression
+\else
+\endif
+
+  
+  This group of commands implements nestable conditional blocks,
+  similarly to psql's .
+  Conditional expressions are identical to those with \set,
+  with non-zero values interpreted as true.
+ 
+
+   
+

 
  \set varname expression
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index fce7e3a..e88f782 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -2148,7 +2148,7 @@ hello 10
   
 
 
-  
+  
 \if expression
 \elif expression
 \else
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index e065f7b..ea022ff 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -32,6 +32,7 @@
 #endif			/* ! WIN32 */
 
 #include "postgres_fe.h"
+#include "fe_utils/conditional.h"
 
 #include "getopt_long.h"
 #include "libpq-fe.h"
@@ -273,6 +274,9 @@ typedef enum
 	 * and we enter the CSTATE_SLEEP state to wait for it to expire. Other
 	 * meta-commands are executed immediately.
 	 *
+	 * CSTATE_SKIP_COMMAND for conditional branches which are not executed,
+	 * quickly skip commands that do not need any evaluation.
+	 *
 	 * CSTATE_WAIT_RESULT waits until we get a result set back from the server
 	 * for the current command.
 	 *
@@ -282,6 +286,7 @@ typedef enum
 	 * command counter, and loops back to CSTATE_START_COMMAND state.
 	 */
 	CSTATE_START_COMMAND,
+	CSTATE_SKIP_COMMAND,
 	CSTATE_WAIT_RESULT,
 	CSTATE_SLEEP,
 	CSTATE_END_COMMAND,
@@ -311,6 +316,7 @@ typedef struct
 	PGconn	   *con;			/* connection handle to DB */
 	int			id;/* client No. */
 	ConnectionStateEnum state;	/* state machine's current state. */
+	ConditionalStack cstack;	/* enclosing conditionals state */
 
 	int			use_file;		/* index in sql_script for this client */
 	int			command;		/* command number in script */
@@ -399,7 +405,11 @@ typedef enum MetaCommand
 	META_SET,	/* \set */
 	META_SETSHELL,/* \setshell */
 	META_SHELL,	/* \shell */
-	META_SLEEP	/* \sleep */
+	META_SLEEP,	/* \sleep */
+	META_IF,	/* \if */
+	META_ELIF,	/* \elif */
+	META_ELSE,	/* \else */
+	META_ENDIF	/* \endif */
 } MetaCommand;
 
 typedef enum QueryMode
@@ -1468,6 +1478,27 @@ coerceToDouble(PgBenchValue *pval, double *dval)
 		return true;
 	}
 }
+
+/*
+ * Return true or false for conditional purposes.
+ * Non zero numerical values are true.
+ */
+static bool
+valueTruth(PgBenchValue *pval)
+{
+	switch (pval->type)
+	{
+		case PGBT_INT:
+			return pval->u.ival != 0;
+		case PGBT_DOUBLE:
+			return pval->u.dval != 0.0;
+		default:
+			/* internal error, unexpected type */
+			Assert(false);
+			return false;
+	}
+}
+
 /* assign an integer value */
 static void
 setIntValue(PgBenchValue *pv, int64 ival)
@@ -1943,6 +1974,14 @@ getMetaCommand(const char *cmd)
 		mc = META_SHELL;
 	else if (pg_strcasecmp(cmd, "sleep") == 0)
 		mc = META_SLEEP;
+	else if (pg_strcasecmp(cmd, "if") == 0)
+		mc = META_IF;
+	else if (pg_strcasecmp(cmd, "elif") == 0)
+		mc = META_ELIF;
+	else if (pg_strcasecmp(cmd, "else") == 0)
+		mc = META_ELSE;
+	else if (pg_strcasecmp(cmd, "endif") == 0)
+		mc = META_ENDIF;
 	else
 		mc = META_NONE;
 	return mc;
@@ -2064,11 +2103,11 @@ preparedStatementName(char *buffer, int file, int state)
 }
 
 static void
-commandFailed(CState *st, const char *message)
+commandFailed(CState *st, const char *cmd, const char *message)
 {
 	fprintf(stderr,
-			"client %d aborted in command %d of script %d; %s\n",
-			st->id, st->command, st->use_file, message);
+			"client %d aborted in command %d (%s) of script %d; %s\n",
+			st->id, st->command, cmd, st->use_file, message);
 }
 
 /* return a script number with a weighted choice. */
@@ -2256,6 +2295,8 @@ doCustom(TState *thread, CState *st, StatsData *agg)
 	st->state = CSTATE_START_THROTTLE;
 else
 	st->state = CSTATE_START_TX;
+/* check consistency */
+Assert(conditional_stack_empty(st->cstack));
 break;
 
 /*
@@ -2421,7 +2462,7 @@ doCustom(TState *thread, CState *st, StatsData *agg)
 {
 	if (!sendCommand(st, command))
 	{
-		commandFailed(st, "SQL command send failed");
+		commandFailed(st, "SQL", "SQL command send failed");
 		st->state = CSTATE_ABORTED;
 	}
 	else
@@ -2454,7 +2495,7 @@ doCustom(TState *thread, CState *st, StatsData *agg)
 
 		if (!evaluateSleep(st, argc, argv, &usec))
 		{
-			commandFailed(st, "execution of meta-command 'sleep' failed");
+			commandFailed(st, "sleep", "execution of meta-command failed");
 

Re: [HACKERS] Proposal: Local indexes for partitioned table

[Alvaro Herrera]

I just realized there's a further problem in the area: when a partition
is detached from its parent, its indexes are not made independent of the
indexes on parent.  So they can't be dropped on their own (booh!); and
dropping the index on the former parent partitioned table drops the
index on the former partition also (hiss!).  I think the fix for this is
to delete all dependencies, and re-register normal ones.  Should be
straightforward, but I'm not doing it this year.

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

Re: [PATCH] Logical decoding of TRUNCATE

[Andres Freund]

Hi,

On 2017-12-29 14:15:22 +0100, Marco Nenciarini wrote:
> This patch implements support for TRUNCATE statements
> in logical replication. The work has mainly done by Simon Riggs then
> finished by me. Tests are written by me.
> 
> TRUNCATE is treated as a form of DELETE for the purpose of deciding
> whether to publish, or not.

It'd be good if you explained exactly what the chosen behaviour is, and
why that's the right behaviour in comparison to other alternatives.

- Andres

Re: Condition variable live lock

[Andres Freund]

On 2017-12-29 12:16:20 +1300, Thomas Munro wrote:
> Here is one way to fix it: track the wait queue size and use that
> number to limit the wakeup loop.  See attached.
>
> That's unbackpatchable though, because it changes the size of struct
> ConditionVariable, potentially breaking extensions compiled against an
> earlier point release.  Maybe this problem won't really cause problems
> in v10 anyway?  It requires a particular interaction pattern that
> barrier.c produces but more typical client code might not: the awoken
> backends keep re-adding themselves because they're waiting for
> everyone (including the waker) to do something, but the waker is stuck
> in that broadcast loop.

Hm, I'm not quite convinced by this approach. Partially because of the
backpatch issue you mention, partially because using the list length as
a limit doesn't seem quite nice.

Given that the proclist_contains() checks in condition_variable.c are
already racy, I think it might be feasible to collect all procnos to
signal while holding the spinlock, and then signal all of them in one
go.

Obviously it'd be nicer to not hold a spinlock while looping, but that
seems like something we can't fix in the back branches. [insert rant
about never using spinlocks unless there's very very clear convicing
reasons].

- Andres

Re: Deadlock in multiple CIC.

[Andres Freund]

On 2017-12-26 13:31:03 -0300, Alvaro Herrera wrote:
> It's strange that this has gone undetected for so long.  I wonder if
> there's an interaction with logical decoding and its historical
> snapshot stuff here.

I can't see how - did you have a vague theory you could share?

Greetings,

Andres Freund

Re: Should we nonblocking open FIFO files in COPY?

[Andres Freund]

On 2017-12-26 22:30:08 -0800, Robert Haas wrote:
> On Tue, Dec 26, 2017 at 7:51 PM, Michael Paquier
>  wrote:
> >> > Hmm.  What about the case where we try to open a plain file that's on
> >> > an inaccessible filesystem, e.g. due to a disk failure?  Allowing
> >> > cancel to work just for FIFOs would be OK, I guess, but allowing it
> >> > for other open() calls that hang would be better.  I'm not sure if we
> >> > can make it work that way, but it would be nice if we could.

I doubt it's realistic to make pg resilient in case of FS problems like
that. Partially because the OS level handling usually is very haphazard
and inconsistent, and partially because the amount of work to get there
seems quite significant with only a small payoff.


> >> That is doable, just stat() and check before open().
> >
> > I think TOCTOU when I read such things.. The data folder is a trusted
> > environment but any patches doing things like that ought to be careful.
> 
> Yeah.  I was more wondering whether an ostensibly non-blocking open()
> would nevertheless block on an inaccessible file.

It very likely would, depending on the type of error. Or stat() would
end up being stuck somewhere in the kernel while it's retrying IO for a
lengthy amount of time.

Greetings,

Andres Freund

Re: [PATCH] session_replication_role = replica with TRUNCATE

[Petr Jelinek]

On 29/12/17 16:53, Marco Nenciarini wrote:
> Il 29/12/17 15:14, Petr Jelinek ha scritto:
>>
>> May be worth documenting that the session_replication_role also affects
>> TRUNCATE's interaction with FKs in config.sgml.
>>
> 
> The current documentation of session_replication_role GUC is:
> 
> Controls firing of replication-related triggers and rules for the
> current session. Setting this variable requires superuser privilege
> and results in discarding any previously cached query plans.
> Possible values are origin (the default), replica and local.
> See ALTER TABLE for more information.
> 
> It doesn't speak about foreign keys or referential integrity, but only
> about triggers and rules. I don't think that it's worth to add a special
> case for truncate, unless we want to expand/rewrite the documentation to
> specify all the effects in the details.
> 

The effects on foreign keys is implied by the fact that for DML it's
implemented using triggers, but that's not true for TRUNCATE. In any
case it does not hurt to mention the FKs explicitly rather than
implicitly here.

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

Re: array_ndims never returns zero

[Vladimir Svedov]

Maybe if you decide not to touch the code, I should report to documentation
mail group, asking to add this special exception to docs?..

Thank you

On 29 December 2017 at 19:50, Pavel Stehule  wrote:

>
>
> 2017-12-29 17:52 GMT+01:00 Tom Lane :
>
>> Vladimir Svedov  writes:
>> > Reading
>> > https://stackoverflow.com/questions/48022753/why-does-array-
>> ndimsarray-produce-null#48022980
>> > confused me much - why array_ndims never returns zero indeed?..
>>
>> Yeah, it's not a very good choice that it returns null for a zero-D
>> array.  But it's been like that for 20-some years, so the question
>> is whether we are prepared to take the compatibility hit from
>> changing it.
>>
>> If we were willing to break things around zero-D arrays, I don't think
>> that's the only thing to change.  It's equally silly that array_dims()
>> returns NULL for such arrays, for instance; their dimensions are
>> certainly not unknown.  Perhaps an empty string is the right result,
>> though I've not thought about it hard.
>>
>> I'd also argue that an out-of-range AARR_NDIM result is grounds
>> for raising an error; returning NULL is a poor substitute for
>> reporting data corruption.
>>
>> In short, if we're to touch this, I'd want somebody to go through all
>> the array functions/operators and see if anything else is weird with
>> zero-D arrays.
>>
>
> Although I see a cost of compatibility break, I agree so NULL in this case
> is confusing.
>
> The empty array can be taken as possible unlimited dimensional with zero
> sized dimensions.
>
> The test on zero is more readable.
>
> Regards
>
> Pavel
>
>>
>> regards, tom lane
>>
>>
>

Rangejoin rebased

[Jeff Davis]

New rangejoin patch attached.

I had previously attempted to make this work well for multiple range
join keys, but this patch implements single rangejoin keys only, and
the rest of the rangejoin clauses are effectively just rechecks. I
believe it can be made effective for multiple rangejoin keys, but at
the cost of additional complexity which is neither justified nor
implemented at this point.

Regards,
 Jeff Davis
diff --git a/doc/src/sgml/rangetypes.sgml b/doc/src/sgml/rangetypes.sgml
index 3a034d9..e27150d 100644
--- a/doc/src/sgml/rangetypes.sgml
+++ b/doc/src/sgml/rangetypes.sgml
@@ -522,4 +522,74 @@ INSERT 0 1
 
   
  
+ 
+  Range Join
+
+  
+range type
+join
+  
+
+  
+A Range Join is a join where one of the join conditions is the range
+overlaps operator (see ). In other
+words, rather than returning rows where corresponding fields are equal, it
+returns rows where the corresponding fields overlap.
+  
+  
+For instance, to find users who were active on a website at the same time
+as they were using a mobile app, you might issue a query like:
+
+CREATE TABLE web_session(
+web_session_id text primary key,
+username text,
+during tstzrange);
+CREATE TABLE app_session(
+app_session_id text primary key,
+username text,
+during tstzrange);
+INSERT INTO web_session VALUES
+('0cc175b9c0f1b6a8', 'user1', '[2017-02-01 09:30, 2017-02-01 10:45)'),
+('92eb5ffee6ae2fec', 'user2', '[2017-02-01 09:30, 2017-02-01 10:45)'),
+('4a8a08f09d37b737', 'user3', '[2017-02-01 09:30, 2017-02-01 10:45)');
+INSERT INTO app_session VALUES
+('8277e0910d750195', 'user1', '[2017-02-01 10:30, 2017-02-01 11:45)'),
+('b448797616e091ad', 'user3', '[2017-02-01 09:00, 2017-02-01 11:00)'),
+('95649038408b5f33', 'user4', '[2017-02-01 09:30, 2017-02-01 10:45)');
+
+SELECT w.username,
+   w.during * a.during AS both_during
+FROM  web_session w, app_session a
+WHERE w.username = a.username
+AND w.during && a.during;
+ username | both_during 
+--+-
+ user1| ["2017-02-01 10:30:00-08","2017-02-01 10:45:00-08")
+ user3| ["2017-02-01 09:30:00-08","2017-02-01 10:45:00-08")
+(2 rows)
+
+  
+  
+In addition to the general execution strategies available, Postgres also
+has special support for a variant of Merge Join called Range Merge Join:
+
+ EXPLAIN (costs off) SELECT w.username,
+   w.during * a.during AS both_during
+FROM  web_session w, app_session a
+WHERE w.username = a.username
+AND w.during && a.during;
+  QUERY PLAN  
+--
+ Range Merge Join
+   Merge Cond: ((w.during && a.during) AND (w.username = a.username))
+   ->  Sort
+ Sort Key: w.during, w.username
+ ->  Seq Scan on web_session w
+   ->  Sort
+ Sort Key: a.during, a.username
+ ->  Seq Scan on app_session a
+(8 rows)
+
+  
+ 
 
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 7e4fbaf..1d9a2ad 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -917,7 +917,11 @@ ExplainNode(PlanState *planstate, List *ancestors,
pname = sname = "Nested Loop";
break;
case T_MergeJoin:
-   pname = "Merge";/* "Join" gets added by 
jointype switch */
+   if (((MergeJoin*)plan)->mergeRangeJoin)
+   pname = "Range Merge";  /* "Join" gets added by 
jointype switch */
+   else
+   pname = "Merge";/* "Join" gets added by 
jointype switch */
+
sname = "Merge Join";
break;
case T_HashJoin:
diff --git a/src/backend/executor/nodeMergejoin.c 
b/src/backend/executor/nodeMergejoin.c
index ef9e1ee..e18a294 100644
--- a/src/backend/executor/nodeMergejoin.c
+++ b/src/backend/executor/nodeMergejoin.c
@@ -89,15 +89,67 @@
  * proceed to another state.  This state is stored in the node's
  * execution state information and is preserved across calls to
  * ExecMergeJoin. -cim 10/31/89
+ *
+ * RANGE MERGE JOIN
+ *
+ * Range Merge Join is a generalization of merge join. When a join
+ * predicate involves the range overlaps operator (&&), a merge 
join
+ * becomes a range join.
+ *
+ * The input ranges must be ordered by (lower-bound, upper-bound), 
which
+ * is the ordinary range_ops operator class. MJCompare must not 
simply
+ * use the operator class's comparison semantics though; instead it
+ * returns:
+ *
+ *   - MJCMP_MATCHES if outer range overlaps inner range
+ *   

Re: array_ndims never returns zero

[Pavel Stehule]

2017-12-29 17:52 GMT+01:00 Tom Lane :

> Vladimir Svedov  writes:
> > Reading
> > https://stackoverflow.com/questions/48022753/why-does-
> array-ndimsarray-produce-null#48022980
> > confused me much - why array_ndims never returns zero indeed?..
>
> Yeah, it's not a very good choice that it returns null for a zero-D
> array.  But it's been like that for 20-some years, so the question
> is whether we are prepared to take the compatibility hit from
> changing it.
>
> If we were willing to break things around zero-D arrays, I don't think
> that's the only thing to change.  It's equally silly that array_dims()
> returns NULL for such arrays, for instance; their dimensions are
> certainly not unknown.  Perhaps an empty string is the right result,
> though I've not thought about it hard.
>
> I'd also argue that an out-of-range AARR_NDIM result is grounds
> for raising an error; returning NULL is a poor substitute for
> reporting data corruption.
>
> In short, if we're to touch this, I'd want somebody to go through all
> the array functions/operators and see if anything else is weird with
> zero-D arrays.
>

Although I see a cost of compatibility break, I agree so NULL in this case
is confusing.

The empty array can be taken as possible unlimited dimensional with zero
sized dimensions.

The test on zero is more readable.

Regards

Pavel

>
> regards, tom lane
>
>

Re: Converting plpgsql to use DTYPE_REC for named composite types

[Pavel Stehule]

2017-12-29 18:38 GMT+01:00 Tom Lane :

> Pavel Stehule  writes:
> > Interesting part from test:
>
> > alter table mutable drop column f1;
> > alter table mutable add column f1 float8;
> > -- currently, this fails due to cached plan for "r.f1 + 1" expression
> > select sillyaddone(42);
> > ERROR:  type of parameter 4 (double precision) does not match that when
> > preparing the plan (integer)
> > CONTEXT:  PL/pgSQL function sillyaddone(integer) line 1 at RETURN
>
> > In this case, can we invalidate plan cache?
>
> That's something I expect we can improve in followon patches, but
> it seems a bit outside the scope of this patch (which is mighty
> big already).
>
>
ok



> > Because PLPGSQL_NSTYPE_ROW is removed, then "switch" statement is maybe
> > useless
>
> I'd just as soon leave it as a switch, for possible future expansion.
>

I don't think so there will be some cases - but it is not a issue


> > why is in exec_assign_value still case for PLPGSQL_DTYPE_ROW ?
>
> Take it out and see ;-).  The whole point of having DTYPE_ROW
> at all is to support stuff like "SELECT ... INTO a,b,c".
>

ok

Regards

Pavel


> regards, tom lane
>

Re: Converting plpgsql to use DTYPE_REC for named composite types

[Tom Lane]

Pavel Stehule  writes:
> Interesting part from test:

> alter table mutable drop column f1;
> alter table mutable add column f1 float8;
> -- currently, this fails due to cached plan for "r.f1 + 1" expression
> select sillyaddone(42);
> ERROR:  type of parameter 4 (double precision) does not match that when
> preparing the plan (integer)
> CONTEXT:  PL/pgSQL function sillyaddone(integer) line 1 at RETURN

> In this case, can we invalidate plan cache?

That's something I expect we can improve in followon patches, but
it seems a bit outside the scope of this patch (which is mighty
big already).

> Because PLPGSQL_NSTYPE_ROW is removed, then "switch" statement is maybe
> useless

I'd just as soon leave it as a switch, for possible future expansion.

> why is in exec_assign_value still case for PLPGSQL_DTYPE_ROW ?

Take it out and see ;-).  The whole point of having DTYPE_ROW
at all is to support stuff like "SELECT ... INTO a,b,c".

regards, tom lane

Re: [HACKERS] taking stdbool.h into use

[Tom Lane]

Alvaro Herrera  writes:
> Michael Paquier wrote:
>> I have looked at 0002 and 0003. Those look good to ship for me.

> Yeah, I'd vote to push those right away to see what buildfarm has to
> say.  That way you can push 0001 shortly after the dust settles (if
> any), which will have an effect on the bootstrap data format patch.

Yeah, I think all of this is at the point where the next thing to do
is see what the buildfarm has to say.  I could test it manually on
prairiedog, but I'd just as soon let the buildfarm script do the work.

It does make sense, probably, to push 0001-0003 first and see if
anything turns up from that, then 0004.

regards, tom lane

Re: Converting plpgsql to use DTYPE_REC for named composite types

[Pavel Stehule]

Hi

2017-12-29 9:56 GMT+01:00 Pavel Stehule :

> Hi
>
> I'll stick this into the January commitfest, but I'd like to get it
>> reviewed and committed pretty soon, because there are follow-on patches
>> that need to get done in time for v11 --- in particular, we need to close
>> out the lack of plpgsql support for domains-over-composite.
>>
>>
> I didn't checked code - just I did some performance tests and I am
> thinking so performance is very good.
>
> Master's record type has 50% speed of row type in my test. Patched has +/-
> same speed.
>
> I see very small slowdown for row type .. about 3% but I think so it is
> acceptable - I tested some worst case.
>
> Unfortunately - it breaks and very breaks all plpgsql related extensions -
> pldebug, plprofiler, plpgsql_check. On second hand, there are only few
> extensions of this kind.
>
>
I checked the code:

Interesting part from test:

alter table mutable drop column f1;
alter table mutable add column f1 float8;
-- currently, this fails due to cached plan for "r.f1 + 1" expression
select sillyaddone(42);
ERROR:  type of parameter 4 (double precision) does not match that when
preparing the plan (integer)
CONTEXT:  PL/pgSQL function sillyaddone(integer) line 1 at RETURN

In this case, can we invalidate plan cache? It can decrease a risk of
runtime issues when tables are altered.

Because PLPGSQL_NSTYPE_ROW is removed, then "switch" statement is maybe
useless

if (ns != NULL && nnames == 2)
{
switch (ns->itemtype)
{
case PLPGSQL_NSTYPE_REC:
{
/*
 * words 1/2 are a record name, so third word could
be
 * a field in this record.
 */
PLpgSQL_rec *rec;
PLpgSQL_recfield *new;

rec = (PLpgSQL_rec *) (plpgsql_Datums[ns->itemno]);
new = plpgsql_build_recfield(rec, word3);

wdatum->datum = (PLpgSQL_datum *) new;
wdatum->ident = NULL;
wdatum->quoted = false; /* not used */
wdatum->idents = idents;
return true;
}

default:
break;
}
}
}

should be reduced

   if (ns != NULL && nnames == 2 && ns->itemtype ==
PLPGSQL_NSTYPE_REC)
{
/*
 * words 1/2 are a record name, so third word could
be
 * a field in this record.
 */
PLpgSQL_rec *rec;
PLpgSQL_recfield *new;

rec = (PLpgSQL_rec *) (plpgsql_Datums[ns->itemno]);
new = plpgsql_build_recfield(rec, word3);

wdatum->datum = (PLpgSQL_datum *) new;
wdatum->ident = NULL;
wdatum->quoted = false; /* not used */
wdatum->idents = idents;
return true;
}


why is in exec_assign_value still case for PLPGSQL_DTYPE_ROW ?


Regards

Pavel



> Regards
>
> Pavel
>
>
> regards, tom lane
>>
>>
>

Re: Possible hole in Windows directory restrictions?

[Tom Lane]

Jack Christensen  writes:
> On 12/29/2017 9:56 AM, Tom Lane wrote:
>> In https://postgr.es/m/1514541656508-0.p...@n3.nabble.com
>> it's reported that "SELECT pg_ls_dir('c:')" works to allow
>> display of the root directory on drive C.  If true, this
>> would be a violation of the principle that the core file
>> access functions only let you get at PG-related directories.
>> However, I looked at the code, and it sure looks like
>> path_is_relative_and_below_cwd() contains code to reject use
>> of Windows drive letters.  Am I missing something?  Anyone
>> want to check if they can reproduce this on a Windows build?

> Could not reproduce with a fresh install.

Thanks for checking.  Digging in the git history, I see that
path_is_relative_and_below_cwd() was introduced in 9.1
(commit 0de0cc150).  pg_ls_dir and friends were in core for
some time before that, so perhaps the answer is that the
OP was using some old PG version.  (Pre-9.1 also defaulted
to standard_conforming_strings = off, which might explain
some other odd things about his report.)

regards, tom lane

Re: [PROPOSAL] bracketed-paste support for psql

[Alvaro Herrera]

Peter Eisentraut wrote:
> On 12/15/17 11:22, Geoff Winkless wrote:
> > It occurred to me the other day while people were talking about
> > pasting blocks of text creating problems, especially with tabs, that
> > xterm bracketed-paste support (also works in at least putty and
> > probably others) that would block curses handling and just paste as-is
> > would be a useful (and I'm guessing relatively simple) thing to add.
> 
> You need to put
> 
> set enable-bracketed-paste on
> 
> into ~/.inputrc, then it works.

Thanks for mentioning this -- I didn't know if, and it's a nice feature.
I've been trying it for a few days and it seems to work pretty well.

There is a problem with backslash commands, though: the parser seems to
search for arguments to the commands all the way to the end of the
buffer rather than the end of the line, so if you paste this:

\d tab1
select * from tab1;

it fails in this seemingly very stupid way:

alvherre=# \d tab1
select * from tab1;
Table "public.tab1"
 Column │  Type   │ Collation │ Nullable │ Default 
┼─┼───┼──┼─
 a  │ integer │   │  │ 

\d: extra argument "select" ignored
\d: extra argument "*" ignored
\d: extra argument "from" ignored
\d: extra argument "tab1;" ignored

It would be useful to stop looking for arguments at EOL.  This behavior
is at odds with how backslash commands work elsewhere.


However, if you don't have a table named 'tab1', processing stops right
there:

alvherre=# \d tab1
select * from tab1;
Did not find any relation named "tab1".
alvherre=# 

I think this would be fine in ON_ERROR_STOP mode, but I have that turned
off here, so it looks like a bug to me.

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

Re: Possible hole in Windows directory restrictions?

[Jack Christensen]

On 12/29/2017 9:56 AM, Tom Lane wrote:


In https://postgr.es/m/1514541656508-0.p...@n3.nabble.com
it's reported that "SELECT pg_ls_dir('c:')" works to allow
display of the root directory on drive C.  If true, this
would be a violation of the principle that the core file
access functions only let you get at PG-related directories.
However, I looked at the code, and it sure looks like
path_is_relative_and_below_cwd() contains code to reject use
of Windows drive letters.  Am I missing something?  Anyone
want to check if they can reproduce this on a Windows build?

regards, tom lane


Could not reproduce with a fresh install.

C:\Program Files\PostgreSQL\10\bin>psql.exe
Password:
psql (10.1)
WARNING: Console code page (437) differs from Windows code page (1252)
 8-bit characters might not work correctly. See psql reference
 page "Notes for Windows users" for details.
Type "help" for help.

postgres=# select version();
  version

 PostgreSQL 10.1, compiled by Visual C++ build 1800, 64-bit
(1 row)


postgres=# SELECT pg_ls_dir('c:');
ERROR:  path must be in or below the current directory

Re: array_ndims never returns zero

[Tom Lane]

Vladimir Svedov  writes:
> Reading
> https://stackoverflow.com/questions/48022753/why-does-array-ndimsarray-produce-null#48022980
> confused me much - why array_ndims never returns zero indeed?..

Yeah, it's not a very good choice that it returns null for a zero-D
array.  But it's been like that for 20-some years, so the question
is whether we are prepared to take the compatibility hit from
changing it.

If we were willing to break things around zero-D arrays, I don't think
that's the only thing to change.  It's equally silly that array_dims()
returns NULL for such arrays, for instance; their dimensions are
certainly not unknown.  Perhaps an empty string is the right result,
though I've not thought about it hard.

I'd also argue that an out-of-range AARR_NDIM result is grounds
for raising an error; returning NULL is a poor substitute for
reporting data corruption.

In short, if we're to touch this, I'd want somebody to go through all
the array functions/operators and see if anything else is weird with
zero-D arrays.

regards, tom lane

Re: plpgsql function startup-time improvements

[Tom Lane]

"Tels"  writes:
> On Thu, December 28, 2017 5:43 pm, Tom Lane wrote:
>> Which field order were you checking?  Are you accounting for alignment
>> padding?
> ...
> Sounds logical, thanx for the detailed explanation. In my test the first 4
> padding bytes are probably not there, because I probably miss something
> that aligns ptrs on 8-byte boundaries. At least that would explain the
> sizes seen here.

Hm, yeah, you would get different results on a compiler that considered
pointers to only require 4-byte alignment.  Although that would be an
ABI break, so I'm surprised to hear there are any x86_64 compilers that
do that by default.

> So, if you moved "isnull" and "freeval" right behind "isconst" and
> "notnull", you could save another 2 byte, land at 64, and thus no extra
> padding would keep it at 64?

I thought about that idea while reading your earlier message,
but I don't favor it for two reasons:

* It'd be an illogical ordering of the fields: isnull and freeval
belong with the value, not somewhere else.  This is not simply an
academic point, because of the field-sharing conventions represented
by PLpgSQL_variable.  I expect that sometime soon we will get
around to implementing CONSTANT, NOT NULL, and initializer properties
for composite variables, and the most natural way to do that will be
to include the isconst, notnull, and default_val fields into
PLpgSQL_variable so they can be shared by PLpgSQL_var and PLpgSQL_rec.
Shoving isnull and freeval into the middle of that would be really ugly.

* The second patch I posted in this thread adds another enum field
at the end of PLpgSQL_var.  Right now, with either field ordering
under discussion, that's basically free space-wise because it's
just going into what would otherwise be trailing padding space.
But it destroys any space savings from moving isnull and freeval
somewhere else.

In the end, it's not wise to put too much emphasis on struct size
and padding considerations, as that just ends up being platform
dependent anyway.  None of what we've said in this back-and-forth
is quite right for 32-bit-pointer machines, and once Peter's stdbool
patch lands, the assumption that bool is 1 byte will be shaky as
well.  I think actually the point about maintaining a logical field
order is the most significant consideration here.  There's no great
harm in trying to avoid space wastage on today's most popular
machines, but we shouldn't let that consideration contort the code
very far.

regards, tom lane

Re: Basebackups reported as idle

[Magnus Hagander]

On Fri, Dec 29, 2017 at 3:11 PM, David Steele  wrote:

> On 12/29/17 6:49 AM, Michael Paquier wrote:
> > On Thu, Dec 28, 2017 at 06:21:46PM +0100, Magnus Hagander wrote:
> >> On Fri, Dec 22, 2017 at 2:31 AM, Michael Paquier <
> michael.paqu...@gmail.com>
> >> wrote:
> >>> Could you update the patch?
> >>
> >> I thought I had, but I can see now that email was a figment of my
> >> imagination :)
> >
> > I'll take that as a fragment instead. The patch as proposed looks good
> > to me. Thanks for sending an updated version.
>
> Looks good to me as well.
>

Thanks to both of you for the reviews. Backpatched and pushed!

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

Possible hole in Windows directory restrictions?

[Tom Lane]

In https://postgr.es/m/1514541656508-0.p...@n3.nabble.com
it's reported that "SELECT pg_ls_dir('c:')" works to allow
display of the root directory on drive C.  If true, this
would be a violation of the principle that the core file
access functions only let you get at PG-related directories.
However, I looked at the code, and it sure looks like
path_is_relative_and_below_cwd() contains code to reject use
of Windows drive letters.  Am I missing something?  Anyone
want to check if they can reproduce this on a Windows build?

regards, tom lane

Re: [PATCH] session_replication_role = replica with TRUNCATE

[Marco Nenciarini]

Il 29/12/17 15:14, Petr Jelinek ha scritto:
> 
> May be worth documenting that the session_replication_role also affects
> TRUNCATE's interaction with FKs in config.sgml.
> 

The current documentation of session_replication_role GUC is:

Controls firing of replication-related triggers and rules for the
current session. Setting this variable requires superuser privilege
and results in discarding any previously cached query plans.
Possible values are origin (the default), replica and local.
See ALTER TABLE for more information.

It doesn't speak about foreign keys or referential integrity, but only
about triggers and rules. I don't think that it's worth to add a special
case for truncate, unless we want to expand/rewrite the documentation to
specify all the effects in the details.

Regards,
Marco

-- 
Marco Nenciarini - 2ndQuadrant Italy
PostgreSQL Training, Services and Support
marco.nenciar...@2ndquadrant.it | www.2ndQuadrant.it



signature.asc
Description: OpenPGP digital signature

Re: Basebackups reported as idle

[Alvaro Herrera]

Michael Paquier wrote:
> On Thu, Dec 28, 2017 at 06:21:46PM +0100, Magnus Hagander wrote:

> > I thought I had, but I can see now that email was a figment of my
> > imagination :)
> 
> I'll take that as a fragment instead.

Not at all ... https://www.merriam-webster.com/dictionary/figment
"something made up or contrived"

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

Re: [HACKERS] Commits don't block for synchronous replication

[Simon Riggs]

On 1 December 2017 at 02:50, Michael Paquier  wrote:
> On Fri, Nov 24, 2017 at 11:38 PM, Simon Riggs  wrote:
>> On 23 November 2017 at 11:11, Michael Paquier  
>> wrote:
>>
>>> This is older than the bug report of this thread. All those
>>> indications point out that the patch has *not* been committed. So it
>>> seems to me that you perhaps committed it to your local repository,
>>> but forgot to push it to the remote. I am switching back the patch
>>> status to what looks correct to me "Ready for committer". Thanks.
>>
>> Yes, that looks like it's my mistake. Thanks for rechecking.
>>
>> Will commit and backpatch when I get home.
>
> Ping. Simon, are you planning to look at this patch, and potentially commit 
> it?
>
> I am moving this item to next CF for now.

Applied, thanks.

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: [HACKERS] [PATCH] Lockable views

[Tatsuo Ishii]

>> >> > 1) Leave as it is (ignore tables appearing in a subquery)
>> >> > 
>> >> > 2) Lock all tables including in a subquery
>> >> > 
>> >> > 3) Check subquery in the view 
>> > 
>> >> > So it seem #1 is the most reasonable way to deal with the problem
>> >> > assuming that it's user's responsibility to take appropriate locks on
>> >> > the tables in the subquery.
>> > 
>> > I adopted #1 and I didn't change anything about this.
>> 
>> Looks good to me except that the patch lacks documents and the
>> regression test needs more cases. For example, it needs a test for the
>> case #1 above: probably using pg_locks to make sure that the tables
>> appearing in the subquery do not hold locks.
> 
> Attached is the update patch, v3. I add some regression test and
> the documentation.

The patch produces a warning.

/home/t-ishii/lock_view-v3.patch:542: trailing whitespace.
-- Verify that we  can lock a auto-updatable views 
warning: 1 line adds whitespace errors.

Your addition to the doc:
+   Automatically updatable views (see )
+   that do not have INSTEAD OF triggers or INSTEAD
+   rules are also lockable. When a view is locked, its base relations are
+   also locked recursively with the same lock mode.

does not mention about the point:

>> >> > 1) Leave as it is (ignore tables appearing in a subquery)

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] why not parallel seq scan for slow functions

[Marina Polyakova]

Hello everyone in this thread!

On 29-11-2017 8:01, Michael Paquier wrote:

Moved to next CF for extra reviews.


Amit, I would like to ask some questions about your patch (and can you 
please rebase it on the top of the master?):


1)
+			path->total_cost -= (target->cost.per_tuple - oldcost.per_tuple) * 
path->rows;


Here we undo the changes that we make in this function earlier:

path->total_cost += target->cost.startup - oldcost.startup +
(target->cost.per_tuple - oldcost.per_tuple) * path->rows;

Perhaps we should not start these "reversible" changes in this case from 
the very beginning?


2)
gpath->subpath = (Path *)
create_projection_path(root,
   
gpath->subpath->parent,
   
gpath->subpath,
   target);
...
+   if (((ProjectionPath *) gpath->subpath)->dummypp)
+   {
...
+			path->total_cost += (target->cost.per_tuple - oldcost.per_tuple) * 
gpath->subpath->rows;

+   }
+   else
+   {
...
+			path->total_cost += (cpu_tuple_cost + target->cost.per_tuple) * 
gpath->subpath->rows;

+   }

As I understand it, here in the if-else block we change the run costs of 
gpath in the same way as they were changed for its subpath in the 
function create_projection_path earlier. But for the startup costs we 
always subtract the cost of the old target:


path->startup_cost += target->cost.startup - oldcost.startup;
path->total_cost += target->cost.startup - oldcost.startup +
(target->cost.per_tuple - oldcost.per_tuple) * path->rows;

Should we change the startup costs of gpath in this way if 
((ProjectionPath *) gpath->subpath)->dummypp is false?


3)
simple_gather_path = (Path *)
create_gather_path(root, rel, cheapest_partial_path, 
rel->reltarget,
   NULL, NULL);
+   /* Add projection step if needed */
+   if (target && simple_gather_path->pathtarget != target)
+		simple_gather_path = apply_projection_to_path(root, rel, 
simple_gather_path, target);

...
+		path = (Path *) create_gather_merge_path(root, rel, subpath, 
rel->reltarget,

+  
  subpath->pathkeys, NULL, NULL);
+   /* Add projection step if needed */
+   if (target && path->pathtarget != target)
+   path = apply_projection_to_path(root, rel, path, 
target);
...
@@ -2422,16 +2422,27 @@ apply_projection_to_path(PlannerInfo *root,
...
gpath->subpath = (Path *)
create_projection_path(root,
   
gpath->subpath->parent,
   
gpath->subpath,
   target);

The target is changing so we change it for the gather(merge) node and 
for its subpath. Do not we have to do this work (replace the subpath by 
calling the function create_projection_path if the target is different) 
in the functions create_gather(_merge)_path too? I suppose that the 
target of the subpath affects its costs => the costs of the 
gather(merge) node in the functions cost_gather(_merge) (=> the costs of 
the gather(merge) node in the function apply_projection_to_path).


4)
+* Consider ways to implement parallel paths.  We always skip
+* generating parallel path for top level scan/join nodes till 
the
+* pathtarget is computed.  This is to ensure that we can 
account for
+* the fact that most of the target evaluation work will be 
performed
+* in workers.
+*/
+   generate_gather_paths(root, current_rel, scanjoin_target);
+
+   /* Set or update cheapest_total_path and related fields */
+   set_cheapest(current_rel);

As I understand it (if correctly, thank the comments of Robert Haas and 
Tom Lane :), after that we cannot use the function 
apply_projection_to_path for paths in the current_rel->pathlist without 
risking that the cheapest path will change. And we have several calls to 
the function adjust_paths_for_srfs (which uses apply_projection_to_path 
for paths in the current_rel->pathlist) in grouping_planner after 
generating the gather paths:


/* Now fix things up if scan/join target contains SRFs */
if (parse->hasTargetSRFs)
adjust_paths_for_srfs(root, current_rel,
  
scanjoin_targets,
  
scanjoin_targets_contain_srfs);
...
 

Re: [PATCH] session_replication_role = replica with TRUNCATE

[Craig Ringer]

On 29 December 2017 at 22:14, Petr Jelinek 
wrote:

> Hi,
>
> On 29/12/17 13:01, Marco Nenciarini wrote:
> > Hi,
> >
> > The current behavior of session_replication_role = replica with TRUNCATE
> > is not the same of with the other commands.
> > It does not check FKs for INSERT/UPDATE/DELETE but it does for TRUNCATE,
> > so one cannot execute TRUNCATE on a table when it is possible to DELETE
> > from table without WHERE clause.
> >
>
> Yes please, I never understood why 'DELETE FROM foo;' works fine with
> 'session_replication_role = replica' and FKs while 'TRUNCATE foo;' would
> throw error.
>

I've spent ages scratching my head about various ways to handle TRUNCATE
and FKs on the downstream, and it never once occurred to me that
session_replication_role should ignore FK cascades for replicated truncate.
But it's really sensible to do just that. Sure, you can have dangling FKs,
but the same is true if you create FKs from non-replicated tables pointing
to replicated tables and do DELETEs from the replicated table, if your
replication tool doesn't have some trick to stop you creating the FK.

I'd still like to know if it was a cascade when applying it, so I can
possibly make some client-determined behaviour choice, but that's for the
other patch really.

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

Re: Add hint about replication slots when nearing wraparound

[Simon Riggs]

On 27 December 2017 at 11:39, Michael Paquier  wrote:
> On Wed, Dec 27, 2017 at 08:47:20AM +0100, Feike Steenbergen wrote:
>> Changed the block from a note to a caution,
>
> Thanks for the new version.
>
> - "You might also need to commit or roll back old prepared transactions.")));
> + "You might also need to commit or roll back old prepared transactions, or 
> drop stale replication slots.")));
> Would "or TO drop stale replication slots" be more correct English?
>
> ereport(WARNING,
> (errmsg("oldest xmin is far in the past"),
> -errhint("Close open transactions soon to avoid wraparound 
> problems.")));
> +errhint("Close open transactions soon to avoid wraparound 
> problems. You might also need to commit or roll back old prepared 
> transactions, or drop stale replication slots.")));
>
> I am not convinced that you need this bit. autovacuum_freeze_max_age can
> be set to lower to even lower values than the default.
>
> Still, those are minor comments, so I am marking this patch as ready for
> committer to get more input from higher-ups.

I left that in for consistency.

Applied, thanks.

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: [PATCH] session_replication_role = replica with TRUNCATE

[Petr Jelinek]

Hi,

On 29/12/17 13:01, Marco Nenciarini wrote:
> Hi,
> 
> The current behavior of session_replication_role = replica with TRUNCATE
> is not the same of with the other commands.
> It does not check FKs for INSERT/UPDATE/DELETE but it does for TRUNCATE,
> so one cannot execute TRUNCATE on a table when it is possible to DELETE
> from table without WHERE clause.
> 

Yes please, I never understood why 'DELETE FROM foo;' works fine with
'session_replication_role = replica' and FKs while 'TRUNCATE foo;' would
throw error.

> I'm attaching a simple patch to make TRUNCATE match behavior of DELETE
> for session_replication_role = replica.
> 

May be worth documenting that the session_replication_role also affects
TRUNCATE's interaction with FKs in config.sgml.

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

Re: Basebackups reported as idle

[David Steele]

On 12/29/17 6:49 AM, Michael Paquier wrote:
> On Thu, Dec 28, 2017 at 06:21:46PM +0100, Magnus Hagander wrote:
>> On Fri, Dec 22, 2017 at 2:31 AM, Michael Paquier 
>> wrote:
>>> Could you update the patch?
>>
>> I thought I had, but I can see now that email was a figment of my
>> imagination :)
> 
> I'll take that as a fragment instead. The patch as proposed looks good
> to me. Thanks for sending an updated version.

Looks good to me as well.

-- 
-David
da...@pgmasters.net



signature.asc
Description: OpenPGP digital signature

Re: MCV lists for highly skewed distributions

[John Naylor]

On 12/28/17, Jeff Janes  wrote:
> I want to revive a patch I sent  couple years ago to the performance list,
> as I have seen the issue pop up repeatedly since then.

> If we stored just a few more values, their inclusion in the MCV would mean
> they are depleted from the residual count, correctly lowering the estimate
> we would get for very rare values not included in the sample.
>
> So instead of having the threshold of 1.25x the average frequency over all
> values, I think we should use 1.25x the average frequency of only those
> values not already included in the MCV, as in the attached.

After reading the thread you mentioned, I think that's a good approach.

On master, the numerator of avg_count is nonnull_cnt, but you've
(indirectly) set it to values_cnt. You didn't mention it here, but I
think you're right and master is wrong, since in this function only
sortable values go through the MCV path. If I understand the code
correctly, if there are enough too-wide values in the sample, they
could bias the average and prevent normal values from being considered
for the MCV list. Am I off base here?

Anyway, since you're now overwriting ndistinct_table, I would rename
it to ndistinct_remaining for clarity's sake.

This part doesn't use any loop variables, so should happen before the loop:

+   if (num_mcv > track_cnt)
+   num_mcv = track_cnt;

I think this comment
/* estimate # of occurrences in sample of a typical nonnull value */
belongs just above the calculation of avg_count.

> I think that perhaps maxmincount should also use the dynamic
> values_cnt_remaining rather than the static one.  After all, things
> included in the MCV don' get represented in the histogram.  When I've seen
> planning problems due to skewed distributions I also usually see redundant
> values in the histogram boundary list which I think should be in the MCV
> list instead. But I have not changed that here, pending discussion.

I think this is also a good idea, but I haven't thought it through. If
you don't go this route, I would move this section back out of the
loop as well.

-John Naylor

array_ndims never returns zero

[Vladimir Svedov]

Hi,
Reading
https://stackoverflow.com/questions/48022753/why-does-array-ndimsarray-produce-null#48022980
confused me much - why array_ndims never returns zero indeed?..
select char_length('') returns zero and according to
https://www.postgresql.org/docs/current/static/functions-string.html it
shows the "Number of characters in string ",on the other hand
https://www.postgresql.org/docs/10/static/functions-array.html
array_ndims "returns
the number of dimensions of the array",but
https://github.com/postgres/postgres/blob/master/src/backend/utils/adt/arrayfuncs.c#L1635

if (AARR_NDIM(v) <= 0 || AARR_NDIM(v) > MAXDIM)
PG_RETURN_NULL(); so the question - do you think it makes sense?.. What is
the logic behind it?
Please advise where to address it if I chose the wrong channel, as It's
probably not a bug, but a feature?..

Regards

Re: TAP test module - PostgresClient

[Tels]

On Thu, December 28, 2017 10:14 pm, Tom Lane wrote:
> Craig Ringer  writes:
>> Another option might be to teach the TAP infrastructure and the
>> buildfarm
>> client how to fetch cpanminus and build DBD::Pg against our build-tree,
>> so
>> we could use Perl DBI.
>
> As a buildfarm owner, I'd take a *really* dim view of the buildfarm
> trying to auto-fetch code off the internet.  As a developer, the
> idea that "make check-world" would try to do that is even scarier.
> Buildfarm owners are likely to have taken at least some steps to
> sandbox their builds, but developers not so much.
>
> I do not think we should even think about going there.

Well, while I couldn't agree more on the "running code from the internet
is dangerous" thing, there are a few points for it, tho:

* if you use Perl modules on your system, you are likely doing already,
anyway, as the Perl modules come, you guessed it, from the internet :)
Just because a random $PackageMaintainer signed it does mean it is really
safe.

* And a lot of Perl modules are not in say, Debian repositories, so you
need to use CPAN (or re-invent everything). Unfortunately, the trend for
other languages seems to go into the same direction, with Ruby gems, the
python package manager, and almost everybody else re-inventing their own
packaging system, often poorly. So you might already have fallen in the
trap of "use random code from the internet". (Of course, that is not
really an argument for doing it, too...)

* the other option seems to be "re-invent the wheel, again, locally",
which isn't always the best, either.

I do agree tho that having "setup" or "make check" auto-fetching stuff
from the internet is not a good idea, however. Mostly because it becomes
suddenly much harder to run in closed networks w/o access and such
side-loading installations can bypass your systems packaging system, which
doesn't sound good, either.

OTOH, it is possible to setup local repositories, or maybe even
pre-bundling modules into some sort of "approved TAP bundle" hosted on an
official server.

The last resort would be to pre-bundle the wanted modules, but this has
the risk of outdating them fast. Plus, pre-bundled modules are not more
security vetted than the ones from the internet, so you might as well use
the CPAN version directly.

The best course seems to me to have dependencies on the OS packackes for
the  Perl modules you want to use. Not sure, however, if the build farm
client has "proper" Debian etc. packages and if it is even possible to add
these dependencies in this way.

Best wishes,

Tels

[PATCH] Logical decoding of TRUNCATE

[Marco Nenciarini]

Hi,

This patch implements support for TRUNCATE statements
in logical replication. The work has mainly done by Simon Riggs then
finished by me. Tests are written by me.

TRUNCATE is treated as a form of DELETE for the purpose of deciding
whether to publish, or not.

The "TRUNCATE behavior when session_replication_role = replica"[1] patch
is required from this patch to work correctly with tables referenced by
foreign keys.

[1] https://commitfest.postgresql.org/16/1447/

Regards,
Marco

-- 
Marco Nenciarini - 2ndQuadrant Italy
PostgreSQL Training, Services and Support
marco.nenciar...@2ndquadrant.it | www.2ndQuadrant.it
diff --git a/contrib/test_decoding/expected/ddl.out 
b/contrib/test_decoding/expected/ddl.out
index 1e22c1eefc..d1feea4909 100644
*** a/contrib/test_decoding/expected/ddl.out
--- b/contrib/test_decoding/expected/ddl.out
***
*** 543,548  UPDATE table_with_unique_not_null SET data = 3 WHERE data = 2;
--- 543,550 
  UPDATE table_with_unique_not_null SET id = -id;
  UPDATE table_with_unique_not_null SET id = -id;
  DELETE FROM table_with_unique_not_null WHERE data = 3;
+ TRUNCATE table_with_unique_not_null;
+ TRUNCATE table_with_unique_not_null, table_with_unique_not_null RESTART 
IDENTITY CASCADE;
  -- check toast support
  BEGIN;
  CREATE SEQUENCE toasttable_rand_seq START 79 INCREMENT 1499; -- portable 
"random"
***
*** 660,665  SELECT data FROM 
pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'inc
--- 662,673 
   table public.table_with_unique_not_null: DELETE: id[integer]:4
   COMMIT
   BEGIN
+  table public.table_with_unique_not_null: TRUNCATE: (no-flags)
+  COMMIT
+  BEGIN
+  table public.table_with_unique_not_null: TRUNCATE: restart_seqs cascade
+  COMMIT
+  BEGIN
   table public.toasttable: INSERT: id[integer]:1 
toasted_col1[text]:'1234567891011121314151617181920212223242526272829303132333435363738394041424344454647484950515253545556575859606162636465666768697071727374757677787980818283848586878889909192939495969798991001011021031041051061071081091101211311411511611711811912012112212312412512612712812913013113213313413513613713813914014114214314414514614714814915015115215315415515615715815916016116216316416516616716816917017117217317417517617717817918018118218318418518618718818919019119219319419519619719819920020120220320420520620720820921021121221321421521621721821922022123224225226227228229230231232233234235236237238239240241242243244245246247248249250251252253254255256257258259260261262263264265266267268269270271272273274275276277278279280281282283284285286287288289290291292293294295296297298299300301302303304305306307308309310311312313314315316317318319320321322323324325326327328329330331332343353363373383393403413423433443453463473483493503513523533543553563573583593603613623633643653663673683693703713723733743753763773783793803813823833843853863873883893903913923933943953963973983994004014024034044054064074084094104114124134144154164174184194204214224234244254264274284294304314324334344354364374384394404414424434544644744844945045145245345445545645745845946046146246346446546646746846947047147247347447547647747847948048148248348448548648748848949049149249349449549649749849950050150250350450550650750850951051151251351451551651751851952052152252352452552652752852953053153253353453553653753853954054154254354454554654754854955055155255355456557558559560561562563564565566567568569570571572573574575576577578579580581582583584585586587588589590591592593594595596597598599600601602603604605606607608609610611612613614615616617618619620621622623624625626627628629630631632633634635636637638639640641642643644645646647648649650651652653654655656657658659660661662663664665676686696706716726736746756766776786796806816826836846856866876886896906916926936946956966976986997007017027037047057067077087097107117127137147157167177187197207217227237247257267277287297307317327337347357367377387397407417427437447457467477487497507517527537547557567577587597607617627637647657667677687697707717727737747757767877978078178278378478578678778878979079179279379479579679779879980080180280380480580680780880981081181281381481581681781881982082182282382482582682782882983083183283383483583683783883984084184284384484584684784884985085185285385485585685785885986086186286386486586686786886987087187287387487587687787887988088188288388488588688789890891892893894895896897898899900901902903904905906907908909910911912913914915916917918919920921922923924925926927928929930931932933934935936937938939940941942943944945946947948949950951952953954955956957958959960961962963964965966967968969970971972973974975976977978979980981982983984985986987988989990991992993994995996997998999100010011002100310041005100610071008100910101011101210131014101510161017101810191020102110221023102410251026102710281029103010311032103310341035103610371038103910401041104210431044104510461047104810491050105110521053105410551056105710581059106010611062106310641065106610671068

Re: [HACKERS] path toward faster partition pruning

[Alvaro Herrera]

I happened to notice that Ashutosh's patch series at
https://www.postgresql.org/message-id/CAFjFpReJhFSoy6DqH0ipFSHd=sLNEkSzAtz4VWCaS-w2jZL=u...@mail.gmail.com
has a 0001 patch that modifies the partition_bound_cmp stuff too.
Are those conflicting?

Ashutosh's commit message:
Modify bound comparision functions to accept members of PartitionKey

Functions partition_bound_cmp(), partition_rbound_cmp() and
partition_rbound_datum_cmp() are required to merge partition bounds
from joining relations. While doing so, we do not have access to the
PartitionKey of either relations. So, modify these functions to accept
only required members of PartitionKey so that the functions can be
reused for merging bounds.

Amit's:
Some interface changes for partition_bound_{cmp/bsearch}

Introduces a notion of PartitionBoundCmpArg, which replaces the set
of arguments void *probe and bool probe_is_bound of both
partition_bound_cmp and partition_bound_bsearch.  It wasn't possible
before to specify the number of datums when a non-bound type of
probe is passed.  Slightly tweaking the existing interface to allow
specifying the same seems awkward.  So, instead encapsulate that
into PartitionBoundCmpArg.  Also, modify partition_rbound_datum_cmp
to compare caller-specifed number of datums, instead of
key->partnatts datums.


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

Re: plpgsql function startup-time improvements

[Tels]

Moin,

On Thu, December 28, 2017 5:43 pm, Tom Lane wrote:
> "Tels"  writes:
>> On Wed, December 27, 2017 3:38 pm, Tom Lane wrote:
>>> Also, I changed PLpgSQL_var.isconst and PLpgSQL_var.notnull from "int"
>>> to "bool", which is what they should have been all along, and relocated
>>> them in the PLpgSQL_var struct.
>
>> With a short test program printing out the size of PLpgSQL_var to check,
>> I
>> always saw 72 bytes, regardless of bool vs. int. So a bool would be 4
>> bytes here. Hmm.
>
> Seems fairly unlikely, especially given that we typedef bool as char ;-).

Hmn, yes, I can see my confusion. And a test shows, that sizeof(bool) is 1
here. and *char etc are 8.

> Which field order were you checking?  Are you accounting for alignment
> padding?
>
> By my count, with the existing field order on typical 64-bit hardware,
> we ought to have
>
> PLpgSQL_datum_type dtype;   -- 4 bytes [1]
> int dno;-- 4 bytes
> char   *refname;-- 8 bytes
> int lineno; -- 4 bytes
> -- 4 bytes wasted due to padding here
> PLpgSQL_type *datatype; -- 8 bytes
> int isconst;-- 4 bytes
> int notnull;-- 4 bytes
> PLpgSQL_expr *default_val;  -- 8 bytes
> PLpgSQL_expr *cursor_explicit_expr; -- 8 bytes
> int cursor_explicit_argrow; -- 4 bytes
> int cursor_options; -- 4 bytes
>
> Datum   value;  -- 8 bytes
> boolisnull; -- 1 byte
> boolfreeval;-- 1 byte
>
> so at this point we've consumed 74 bytes, but the whole struct has
> to be aligned on 8-byte boundaries because of the pointers, so
> sizeof(PLpgSQL_var) ought to be 80 --- and that is what I see here.
>
> With the proposed redesign,
>
> PLpgSQL_datum_type dtype;   -- 4 bytes [1]
> int dno;-- 4 bytes
> char   *refname;-- 8 bytes
> int lineno; -- 4 bytes
>
> boolisconst;-- 1 byte
> boolnotnull;-- 1 byte
> -- 2 bytes wasted due to padding here
> PLpgSQL_type *datatype; -- 8 bytes
> PLpgSQL_expr *default_val;  -- 8 bytes
> PLpgSQL_expr *cursor_explicit_expr; -- 8 bytes
> int cursor_explicit_argrow; -- 4 bytes
> int cursor_options; -- 4 bytes
>
> Datum   value;  -- 8 bytes
> boolisnull; -- 1 byte
> boolfreeval;-- 1 byte
>
> so we've consumed 66 bytes, which rounds up to 72 with the addition
> of trailing padding.

Sounds logical, thanx for the detailed explanation. In my test the first 4
padding bytes are probably not there, because I probably miss something
that aligns ptrs on 8-byte boundaries. At least that would explain the
sizes seen here.

So, if you moved "isnull" and "freeval" right behind "isconst" and
"notnull", you could save another 2 byte, land at 64, and thus no extra
padding would keep it at 64?

>> And maybe folding all four bool fields into an "int flags" field with
>> bits
>> would save space, and not much slower (depending on often how the
>> different flags are accessed due to the ANDing and ORing ops)?
>
> That'd be pretty messy/invasive in terms of the code changes needed,
> and I don't think it'd save any space once you account for alignment
> and the fact that my other patch proposes to add another enum at
> the end of the struct.  Also, I'm not exactly convinced that
> replacing byte sets and tests with bitflag operations would be
> cheap time-wise.  (It would particularly be a mess for isnull,
> since then there'd be an impedance mismatch with a whole lotta PG
> APIs that expect null flags to be bools.)

Already had a hunch the idea wouldn't be popular, and this are all pretty
solid arguments against it. Nevermind, then :)

Best wishes,

Tels

Re: [HACKERS] taking stdbool.h into use

[Alvaro Herrera]

Michael Paquier wrote:
> On Thu, Dec 28, 2017 at 01:45:54PM -0500, Peter Eisentraut wrote:

> > Here is a new patch set that picks up the best pieces of the recent
> > discussions:  We use stdbool.h if available, use bool8 in the system
> > catalogs, define GinTernaryValue to be the same size as bool, and
> > rewrite the GinNullCategory code to work without casting.
> 
> I have looked at 0002 and 0003. Those look good to ship for me.

Yeah, I'd vote to push those right away to see what buildfarm has to
say.  That way you can push 0001 shortly after the dust settles (if
any), which will have an effect on the bootstrap data format patch.

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

[PATCH] session_replication_role = replica with TRUNCATE

[Marco Nenciarini]

Hi,

The current behavior of session_replication_role = replica with TRUNCATE
is not the same of with the other commands.
It does not check FKs for INSERT/UPDATE/DELETE but it does for TRUNCATE,
so one cannot execute TRUNCATE on a table when it is possible to DELETE
from table without WHERE clause.

I'm attaching a simple patch to make TRUNCATE match behavior of DELETE
for session_replication_role = replica.

Regards,
Marco

-- 
Marco Nenciarini - 2ndQuadrant Italy
PostgreSQL Training, Services and Support
marco.nenciar...@2ndquadrant.it | www.2ndQuadrant.it
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index d979ce266d..296807849f 100644
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
***
*** 1341,1356  ExecuteTruncate(TruncateStmt *stmt)
}
  
/*
!* Check foreign key references.  In CASCADE mode, this should be
!* unnecessary since we just pulled in all the references; but as a
!* cross-check, do it anyway if in an Assert-enabled build.
 */
  #ifdef USE_ASSERT_CHECKING
-   heap_truncate_check_FKs(rels, false);
- #else
-   if (stmt->behavior == DROP_RESTRICT)
heap_truncate_check_FKs(rels, false);
  #endif
  
/*
 * If we are asked to restart sequences, find all the sequences, lock 
them
--- 1341,1364 
}
  
/*
!* Suppress foreign key references check if session replication role is
!* set to REPLICA.
 */
+   if (SessionReplicationRole != SESSION_REPLICATION_ROLE_REPLICA)
+   {
+ 
+   /*
+* Check foreign key references.  In CASCADE mode, this should 
be
+* unnecessary since we just pulled in all the references; but 
as a
+* cross-check, do it anyway if in an Assert-enabled build.
+*/
  #ifdef USE_ASSERT_CHECKING
heap_truncate_check_FKs(rels, false);
+ #else
+   if (stmt->behavior == DROP_RESTRICT)
+   heap_truncate_check_FKs(rels, false);
  #endif
+   }
  
/*
 * If we are asked to restart sequences, find all the sequences, lock 
them


signature.asc
Description: OpenPGP digital signature

Re: Basebackups reported as idle

[Michael Paquier]

On Thu, Dec 28, 2017 at 06:21:46PM +0100, Magnus Hagander wrote:
> On Fri, Dec 22, 2017 at 2:31 AM, Michael Paquier 
> wrote:
>> Could you update the patch?
> 
> I thought I had, but I can see now that email was a figment of my
> imagination :)

I'll take that as a fragment instead. The patch as proposed looks good
to me. Thanks for sending an updated version.
--
Michael


signature.asc
Description: PGP signature

Re: AS OF queries

[Konstantin Knizhnik]

On 28.12.2017 20:28, Peter Eisentraut wrote:

On 12/28/17 11:36, Konstantin Knizhnik wrote:

Attached please find new version of AS OF patch which allows to specify
time travel period.
Older versions outside this period may be reclaimed by autovacuum.
This behavior is controlled by "time_travel_period" parameter.

So where are we on using quasi SQL-standard syntax for a nonstandard
interpretation?  I think we could very well have a variety of standard
and nonstandard AS OF variants, including by commit timestamp, xid,
explicit range columns, etc.  But I'd like to see a discussion on that,
perhaps in a documentation update, which this patch is missing.

SQL:2011 ||defines rules for creation and querying of temporal tables.
I have not read this standard myself, I just take information about it 
from wikipedia:

https://en.wikipedia.org/wiki/SQL:2011
According to this standard time-sliced queries are specified using
|
AS OF SYSTEM TIME| |and| |VERSIONS BETWEEN SYSTEM TIME ... AND ...|clauses.

Looks like it is supported now only by Oracle. IBM DB, MS-SQL, are 
providing similar functionality in slightly different way.
I am not sure whether strict support of SQL:2011 standard is critical 
and which other functionality we need.



I have questions about corner cases.  What happens when multiple tables
are queried with different AS OF clauses?

It is possible.


   Can there be apparent RI
violations?
Right now AS OF is used only in selects, not in update statements. So I 
do not understand how integrity constraints can be violated.



  What happens when the time_travel_period is changed during
a session?
Right now it depends on autovacuum: how fast it will be able to reclaim 
old version.
Actually I I do not see much sense in changing time travel period during 
session.
In asof-4.patch time_travel_period is postmaster level GUC which can not 
be changed in session.
But I have changed policy for it for SIGHUP to make experiments with it 
more easier.




How can we check how much old data is available, and how can
we check how much space it uses?
Physical space used by the database/relation can be determined using 
standard functions, for example pg_total_relation_size.

I do not know any simple way to get total number of all stored versions.

  What happens if no old data for the
selected AS OF is available?

It will just return the version closest to the specified timestamp.


  How does this interact with catalog
changes, such as changes to row-level security settings?  (Do we apply
the current or the past settings?)

Catalog changes are not currently supported.
And I do not have good understanding how to support it if query involves 
two different timeslice with different versions of the table.
Too much places in parser/optimizer have to be change to support such 
"historical collisions".




This patch should probably include a bunch of tests to cover these and
other scenarios.

Right now I have added just one test: asof.sql.
It requires "track_commit_timestamp" option to be switched on and it is 
postmaster level GUC.

So I have added for it postgresql.asof.conf and asof_schedule.
This test should be launched using the following command:

make check EXTRA_REGRESS_OPTS="--schedule=asof_schedule 
--temp-config=postgresql.asof.config"


If there is some better way to include this test in standard regression 
tests, please let me know.

(Maybe "period" isn't the best name, because it implies a start and an
end.  How about something with "age"?)
Well I am not an English native speaker. So I can not conclude what is 
more natural.
"period" is widely used in topics related with temporal tables (just 
count occurrences of this word at https://en.wikipedia.org/wiki/SQL:2011)

Age is not used here at all.
From my point of view age is something applicable to person, building, 
monument,...
It is not possible to say about "ge of time travel". In science fiction 
"time machines" frequently have limitations: you can not got more than N 
years in the past.

How we can name this N? Is it "period", "age" or something else?

I attached yet another version of the patch which includes test for AS 
OF query.


--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index eb5bbb5..7acaf30 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -78,6 +78,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
 	ExprContext *econtext;
 	HeapScanDesc scan;
 	TIDBitmap  *tbm;
+	EState	   *estate;
 	TBMIterator *tbmiterator = NULL;
 	TBMSharedIterator *shared_tbmiterator = NULL;
 	TBMIterateResult *tbmres;
@@ -85,11 +86,13 @@ BitmapHeapNext(BitmapHeapScanState *node)
 	TupleTableSlot *slot;
 	ParallelBitmapHeapState *pstate = node->pstate;
 	dsa_area   *dsa = node->ss.ps.state->es_query_dsa;
+	TimestampTz outerAsofTimestamp = 0;
 
 	/*
 	 * 

Fix a Oracle-compatible instr function in the documentation

[Yugo Nagata]

Hi,

Attached is a patch to fix a very trivial issue of the documentation.

The documentation of PL/pgSQL provides sample codes of Oracle-compatible
instr functions. However, the behaviour is a little differet.
Oracle's instr raises an error when the forth argument value is less than
zero, but the sample code returns zero. This patch fixes this.

Regards,

-- 
Yugo Nagata 
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 7d23ed4..5a67c38 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -5676,6 +5676,10 @@ DECLARE
 length integer;
 ss_length integer;
 BEGIN
+IF occur_index <= 0 THEN
+RAISE 'argument ''%'' is out of range', occur_index USING ERRCODE = '22003';
+END IF;
+
 IF beg_index > 0 THEN
 temp_str := substring(string FROM beg_index);
 pos := position(string_to_search IN temp_str);

Re: Converting plpgsql to use DTYPE_REC for named composite types

[Pavel Stehule]

Hi

I'll stick this into the January commitfest, but I'd like to get it
> reviewed and committed pretty soon, because there are follow-on patches
> that need to get done in time for v11 --- in particular, we need to close
> out the lack of plpgsql support for domains-over-composite.
>
>
I didn't checked code - just I did some performance tests and I am thinking
so performance is very good.

Master's record type has 50% speed of row type in my test. Patched has +/-
same speed.

I see very small slowdown for row type .. about 3% but I think so it is
acceptable - I tested some worst case.

Unfortunately - it breaks and very breaks all plpgsql related extensions -
pldebug, plprofiler, plpgsql_check. On second hand, there are only few
extensions of this kind.

Regards

Pavel


regards, tom lane
>
>

Re: [HACKERS] taking stdbool.h into use

[Michael Paquier]

On Thu, Dec 28, 2017 at 01:45:54PM -0500, Peter Eisentraut wrote:
> An earlier patch I posted defines GinTernaryValue to be the same size as
> bool, accounting for different possible sizes of bool.

Doh. I forgot this one. Yes that approach is fine.

> Here is a new patch set that picks up the best pieces of the recent
> discussions:  We use stdbool.h if available, use bool8 in the system
> catalogs, define GinTernaryValue to be the same size as bool, and
> rewrite the GinNullCategory code to work without casting.

I have looked at 0002 and 0003. Those look good to ship for me.
--
Michael


signature.asc
Description: PGP signature