Re: [HACKERS] [ADMIN] Replication slots and isolation levels

[Andres Freund]

On 2015-11-02 15:37:57 -0500, Robert Haas wrote:
> On Fri, Oct 30, 2015 at 9:49 AM, Vladimir Borodin  wrote:
> > I’ve tried two ways - bare SELECT in autocommit mode and BEGIN; SELECT;
> > ROLLBACK. I first described the problem in thread on pgsql-admin@ [0], there
> > is copy-paste from psql there, but during conversation initial description
> > was lost.
> >
> > [0]
> > http://www.postgresql.org/message-id/7f74c5ea-6741-44fc-b6c6-e96f18d76...@simply.name
> 
> Hmm.  That behavior seems unexpected to me, but I might be missing something.

The conflict is because of a relation lock, not because of
visibility. Hot-Standby feedback changes nothing about that.

I presume all the other conflicts are all because of relation level
locks? Check pg_stat_database_conflicts and the server logs to verify.

Andres


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

Re: [HACKERS] pglogical_output - a general purpose logical decoding output plugin

[Craig Ringer]

On 3 November 2015 at 16:41, Craig Ringer  wrote:
> On 3 November 2015 at 02:58, Jim Nasby  wrote:
>> On 11/2/15 8:36 AM, Craig Ringer wrote:
>>>
>>> Here's the protocol documentation discussed in the README. It's
>>> asciidoc at the moment, so it can be formatted into something with
>>> readable tables.
>>
>>
>> Is this by chance up on github? It'd be easier to read the final output
>> there than the raw asciidoctor. ;)
>
> Not yet, no. Should be able to format it to PDF, HTML, etc if needed though.

In fact, I'll just put a PDF up shortly.

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


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

Re: [HACKERS] NOTIFY in Background Worker

[Andres Freund]

On 2015-11-03 09:52:34 +0100, Simon Riggs wrote:
> On 3 November 2015 at 09:35, Andres Freund  wrote:
> 
> 
> > > With this commit - bde39eed0cafb82bc94c40e95d96b5cf47b6f719, it is not
> > possible
> > > to execute Notify commands inside a parallel worker. Can't we change
> > > it as disable both listen and notify commands inside a background worker?
> >
> > Well, parallel workers are something different from general background
> > workers. I don't see why it'd make sense to allow listen/notify there,
> > given the rest of the restrictions?
> >
> 
> What are the restrictions and/or where are they documented? Thanks

There's a section in README.parallel:
> Instead, we take a more pragmatic approach. First, we try to make as many of
> the operations that are safe outside of parallel mode work correctly in
> parallel mode as well.  Second, we try to prohibit common unsafe operations
> via suitable error checks.  These checks are intended to catch 100% of
> unsafe things that a user might do from the SQL interface, but code written
> in C can do unsafe things that won't trigger these checks.  The error checks
> are engaged via EnterParallelMode(), which should be called before creating
> a parallel context, and disarmed via ExitParallelMode(), which should be
> called after all parallel contexts have been destroyed.  The most
> significant restriction imposed by parallel mode is that all operations must
> be strictly read-only; we allow no writes to the database and no DDL.  We
> might try to relax these restrictions in the future.

Basically the restriction is that, for now, while a parallelized
statement is in progress you can't write data and no DDL is possible.

Andres


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

Re: [HACKERS] NOTIFY in Background Worker

[Pavel Stehule]

2015-11-03 9:54 GMT+01:00 Andres Freund :

> On 2015-11-03 09:52:26 +0100, Pavel Stehule wrote:
> > 2015-11-03 9:35 GMT+01:00 Andres Freund :
> >
> > > On 2015-11-03 17:19:43 +1100, Haribabu Kommi wrote:
> > > > With this commit - bde39eed0cafb82bc94c40e95d96b5cf47b6f719, it is
> not
> > > possible
> > > > to execute Notify commands inside a parallel worker. Can't we change
> > > > it as disable both listen and notify commands inside a background
> worker?
> > >
> > > Well, parallel workers are something different from general background
> > > workers. I don't see why it'd make sense to allow listen/notify there,
> > > given the rest of the restrictions?
> > >
> >
> > I though about this possibility and I am thinking, so NOTIFY can be
> pretty
> > useful there.
> >
> > The background workers can be used for running AT TIME tasks - run import
> > every 10 minutes. The notification can be useful for starting AFTER tasks
> > where the required previous steps should be committed.
> >
> > If we use workers more for execution custom code (PLpgSQL, PLPython, ...)
> > then notification mechanism can be interesting (both directions).
>
> Did you actually read what I wrote above? paralell workers != general
> background workers.
>

I miss it, I am sorry. My notice is related to background workers

Regards

Pavel


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

Re: [HACKERS] NOTIFY in Background Worker

[Andres Freund]

On 2015-11-03 09:52:26 +0100, Pavel Stehule wrote:
> 2015-11-03 9:35 GMT+01:00 Andres Freund :
> 
> > On 2015-11-03 17:19:43 +1100, Haribabu Kommi wrote:
> > > With this commit - bde39eed0cafb82bc94c40e95d96b5cf47b6f719, it is not
> > possible
> > > to execute Notify commands inside a parallel worker. Can't we change
> > > it as disable both listen and notify commands inside a background worker?
> >
> > Well, parallel workers are something different from general background
> > workers. I don't see why it'd make sense to allow listen/notify there,
> > given the rest of the restrictions?
> >
> 
> I though about this possibility and I am thinking, so NOTIFY can be pretty
> useful there.
> 
> The background workers can be used for running AT TIME tasks - run import
> every 10 minutes. The notification can be useful for starting AFTER tasks
> where the required previous steps should be committed.
> 
> If we use workers more for execution custom code (PLpgSQL, PLPython, ...)
> then notification mechanism can be interesting (both directions).

Did you actually read what I wrote above? paralell workers != general
background workers.


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

Re: [HACKERS] NOTIFY in Background Worker

[Andres Freund]

On 2015-11-03 17:19:43 +1100, Haribabu Kommi wrote:
> On Sat, Aug 29, 2015 at 12:55 PM, Thomas Munro
>  wrote:
> > On Sat, Aug 29, 2015 at 9:03 AM, Thomas Munro
> >  wrote:
> > This made me wonder what happens if a background worker calls LISTEN.
> > NotifyMyFrontEnd simply logs the notifications, since there is no remote
> > libpq to sent a message to.  Perhaps a way of delivering to background
> > workers could be developed, though of course there are plenty of other kinds
> > of IPC available already.
> 
> With this commit - bde39eed0cafb82bc94c40e95d96b5cf47b6f719, it is not 
> possible
> to execute Notify commands inside a parallel worker. Can't we change
> it as disable both listen and notify commands inside a background worker?

Well, parallel workers are something different from general background
workers. I don't see why it'd make sense to allow listen/notify there,
given the rest of the restrictions?

Andres


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

Re: [HACKERS] pglogical_output - a general purpose logical decoding output plugin

[Craig Ringer]

On 3 November 2015 at 02:58, Jim Nasby  wrote:
> On 11/2/15 8:36 AM, Craig Ringer wrote:
>>
>> Here's the protocol documentation discussed in the README. It's
>> asciidoc at the moment, so it can be formatted into something with
>> readable tables.
>
>
> Is this by chance up on github? It'd be easier to read the final output
> there than the raw asciidoctor. ;)

Not yet, no. Should be able to format it to PDF, HTML, etc if needed though.

Depending on the consensus here, I'm expecting the protocol docs will
likely get turned into a plaintext formatted README in the source
tree, or into SGML docs.

The rest are in rather more readable Markdown form at this point,
again pending opinions on where they should live - in the public SGML
docs or in-tree READMEs.

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


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

Re: [HACKERS] NOTIFY in Background Worker

[Pavel Stehule]

2015-11-03 9:35 GMT+01:00 Andres Freund :

> On 2015-11-03 17:19:43 +1100, Haribabu Kommi wrote:
> > On Sat, Aug 29, 2015 at 12:55 PM, Thomas Munro
> >  wrote:
> > > On Sat, Aug 29, 2015 at 9:03 AM, Thomas Munro
> > >  wrote:
> > > This made me wonder what happens if a background worker calls LISTEN.
> > > NotifyMyFrontEnd simply logs the notifications, since there is no
> remote
> > > libpq to sent a message to.  Perhaps a way of delivering to background
> > > workers could be developed, though of course there are plenty of other
> kinds
> > > of IPC available already.
> >
> > With this commit - bde39eed0cafb82bc94c40e95d96b5cf47b6f719, it is not
> possible
> > to execute Notify commands inside a parallel worker. Can't we change
> > it as disable both listen and notify commands inside a background worker?
>
> Well, parallel workers are something different from general background
> workers. I don't see why it'd make sense to allow listen/notify there,
> given the rest of the restrictions?
>

I though about this possibility and I am thinking, so NOTIFY can be pretty
useful there.

The background workers can be used for running AT TIME tasks - run import
every 10 minutes. The notification can be useful for starting AFTER tasks
where the required previous steps should be committed.

If we use workers more for execution custom code (PLpgSQL, PLPython, ...)
then notification mechanism can be interesting (both directions).

Regards

Pavel


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

Re: [HACKERS] NOTIFY in Background Worker

[Simon Riggs]

On 3 November 2015 at 09:35, Andres Freund  wrote:


> > With this commit - bde39eed0cafb82bc94c40e95d96b5cf47b6f719, it is not
> possible
> > to execute Notify commands inside a parallel worker. Can't we change
> > it as disable both listen and notify commands inside a background worker?
>
> Well, parallel workers are something different from general background
> workers. I don't see why it'd make sense to allow listen/notify there,
> given the rest of the restrictions?
>

What are the restrictions and/or where are they documented? Thanks

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

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

[HACKERS] RFC/WIP: adding new configuration options to TOAST

[Bill Moran]

Looking for feedback to see if anyone sees any issues or has any
suggestions on what I'm doing. The attached patch alters 3 things
with regard to TOAST behavior:

1) Add a GUC target_compression_ratio: When attempting to
   compress a datum in the TOAST code, only stored the compressed
   version if it saves at least target_compression_ratio% space.

2) Add a constant COMPRESSION_TEST_SIZE: If a datum is larger
   than this size, initially COMPRESSION_TEST_SIZE bytes are
   compressed, and the entire datum is only compressed if the
   test compression indicates that it might be worth it. The
   goal is to avoid the CPU of compressing a large value that
   isn't going to save space anyway.

3) Add a GUC target_tuple_size: which exposes the "fit at least
   4 tuples on a page" logic as a configurable value. The value
   is exposed as a maximum desired size (instead of the target
   tuples to fit on a page) because that seems like a more
   intuitive configuration option to me.

If this seems to be on track, then my next step is to make these
values configurable on a per-table basis.

I'm tracking my work on github, if that's easier to review than
the patch for anyone: https://github.com/williammoran/postgres

--
Bill Moran
diff -r --exclude=.git --exclude=src/test --exclude=README postgres_head/doc/src/sgml/config.sgml postgres_new/doc/src/sgml/config.sgml
6708a6709,6773
>
> TOAST
> 
> 
> 
> 
>  target_tuple_size (integer)
>  
>   target_tuple_size configuration parameter
>  
>  
>  
>   
>The maximum size (in bytes) that a tuple may be without incurring the TOAST
>code. Tuples smaller than this size will always be stored as-is, without
>compression or external storage.
>   
> 
>   
>Lowering this value will cause smaller tuples to be considered for compression
>and external storage, possibly improving on-disk storage efficiency, but at
>the cost of additional CPU load during data modification.
>   
> 
>   
>Setting this value too high may cause inefficient on-disk storage by causing
>large tuples to fill most of a page without leaving enough space for
>additional tuples on the same page (thus resulting in significant wasted
>space on each database page) This value must be smaller than the system
>wide page size (normally 8k).
>   
> 
>   
>Columns will be compressed and/or moved external to the table until the
>size is less than target_tuple_size or no additional gains can be made.
>   
>  
>  
> 
> 
> 
>  target_compression_savings (integer)
>  
>   target_compression_savings configuration parameter
>  
>  
>  
>   
>The minimum amount of space that must be saved in order for compression to
>be used on an eligible column, as a percent of the original size.
>   
> 
>   
>Setting this value properly avoids the CPU overhead of compressing columns
>where very little space is saved. Valid values are between 1 and 99. Setting
>this to a high value (such as 90) will only use compression if it results in
>a large amount of space savings, where as a low value will result in compression
>being used frequently.
>   
>  
> 
> 
> 
>
> 
diff -r --exclude=.git --exclude=src/test --exclude=README postgres_head/src/backend/access/heap/heapam.c postgres_new/src/backend/access/heap/heapam.c
68a69
> #include "utils/guc.h"
2616c2617
< 	else if (HeapTupleHasExternal(tup) || tup->t_len > TOAST_TUPLE_THRESHOLD)
---
> 	else if (HeapTupleHasExternal(tup) || tup->t_len > target_tuple_size)
3918c3919
< 	  newtup->t_len > TOAST_TUPLE_THRESHOLD);
---
> 	  newtup->t_len > target_tuple_size);
diff -r --exclude=.git --exclude=src/test --exclude=README postgres_head/src/backend/access/heap/rewriteheap.c postgres_new/src/backend/access/heap/rewriteheap.c
128a129
> #include "utils/guc.h"
650c651
< 	else if (HeapTupleHasExternal(tup) || tup->t_len > TOAST_TUPLE_THRESHOLD)
---
> 	else if (HeapTupleHasExternal(tup) || tup->t_len > target_tuple_size)
diff -r --exclude=.git --exclude=src/test --exclude=README postgres_head/src/backend/access/heap/tuptoaster.c postgres_new/src/backend/access/heap/tuptoaster.c
41a42
> #include "utils/guc.h"
731c732,736
< 	maxDataLen = TOAST_TUPLE_TARGET - hoff;
---
> 	maxDataLen = target_tuple_size;
> 	if (hoff >= maxDataLen)
> 		maxDataLen = 1;
> 	else
> 		maxDataLen = maxDataLen - hoff;
1324,1332c1329,1348
< 	 * We recheck the actual size even if pglz_compress() reports success,
< 	 * because it might be satisfied with having saved as little as one byte
< 	 * in the compressed data --- which could turn into a net loss once you
< 	 * consider header and alignment padding.  Worst case, the compressed
< 	 * format might require three padding bytes (plus header, which is
< 	 * included in VARSIZE(tmp)), whereas 

Re: [HACKERS] proposal: multiple psql option -c

[Pavel Stehule]

2015-11-03 4:16 GMT+01:00 Robert Haas :

> On Sat, Oct 31, 2015 at 2:50 PM, Pavel Stehule 
> wrote:
> > fixed patch attached
>
> The documentation included in this patch doesn't really make it clear
> why -g is different from or better than -c.
>

I wrote some text. But needs some work of native speaker.

Regards

Pavel


>
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
new file mode 100644
index 212dbfa..18ff8e5
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
*** EOF
*** 223,228 
--- 223,268 
  
  
  
+   -g command(s)
+   --group-command=command(s)
+   
+   
+   Specifies that psql is to execute one or
+   more command strings, commands,
+   and then exit. This is useful in shell scripts. Start-up files
+   (psqlrc and ~/.psqlrc) are
+   ignored with this option.
+   
+ 
+   
+   There are a few differences between -c and
+   -g options. The option -c can be used
+   only once. The option -g can be used more times.
+   This can simplify writing some non trivial SQL commands. With the
+   -g option it is possible to call several psql
+   parametrized backslash commands. When you execute multiple SQL
+   commands via -c option, only result of last command
+   is returned. The execution started by -g option shows
+   result of all commands.
+   
+ 
+   
+   Another difference is in wrapping the transaction. The -c
+   option runs commands in one transaction. The -g option
+   uses autocommit mode by default. This allows running multiple commads
+   which would otherwise not be allowed to execute within one transaction.
+   This is typical for VACUUM command.
+ 
+ psql -Atq -g "VACUUM FULL foo; SELECT pg_relation_size('foo')"
+ psql -Atq -g "VACUUM FULL foo" -g "SELECT pg_relation_size('foo')"
+ 
+ 
+   
+ 
+   
+ 
+ 
+ 
-h hostname
--host=hostname

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
new file mode 100644
index 50d3ff5..73ed5c1
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
*** process_file(char *filename, bool single
*** 2293,2299 
  	int			result;
  	char	   *oldfilename;
  	char		relpath[MAXPGPATH];
- 	PGresult   *res;
  
  	if (!filename)
  	{
--- 2293,2298 
*** process_file(char *filename, bool single
*** 2338,2374 
  	oldfilename = pset.inputfile;
  	pset.inputfile = filename;
  
! 	if (single_txn)
! 	{
! 		if ((res = PSQLexec("BEGIN")) == NULL)
! 		{
! 			if (pset.on_error_stop)
! 			{
! result = EXIT_USER;
! goto error;
! 			}
! 		}
! 		else
! 			PQclear(res);
! 	}
! 
! 	result = MainLoop(fd);
! 
! 	if (single_txn)
! 	{
! 		if ((res = PSQLexec("COMMIT")) == NULL)
! 		{
! 			if (pset.on_error_stop)
! 			{
! result = EXIT_USER;
! goto error;
! 			}
! 		}
! 		else
! 			PQclear(res);
! 	}
  
- error:
  	if (fd != stdin)
  		fclose(fd);
  
--- 2337,2344 
  	oldfilename = pset.inputfile;
  	pset.inputfile = filename;
  
! 	result = MainLoop(fd, single_txn);
  
  	if (fd != stdin)
  		fclose(fd);
  
*** error:
*** 2376,2383 
  	return result;
  }
  
- 
- 
  static const char *
  _align2string(enum printFormat in)
  {
--- 2346,2351 
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
new file mode 100644
index 5b63e76..2ef4ea6
*** a/src/bin/psql/help.c
--- b/src/bin/psql/help.c
*** usage(unsigned short int pager)
*** 83,88 
--- 83,90 
  	fprintf(output, _("  -c, --command=COMMANDrun only single command (SQL or internal) and exit\n"));
  	fprintf(output, _("  -d, --dbname=DBNAME  database name to connect to (default: \"%s\")\n"), env);
  	fprintf(output, _("  -f, --file=FILENAME  execute commands from file, then exit\n"));
+ 	fprintf(output, _("  -g, --group-command=COMMAND\n"
+ 	  "   run more groups of commands (SQL or internal) and exit\n"));
  	fprintf(output, _("  -l, --list   list available databases, then exit\n"));
  	fprintf(output, _("  -v, --set=, --variable=NAME=VALUE\n"
  	  "   set psql variable NAME to VALUE\n"
diff --git a/src/bin/psql/mainloop.c b/src/bin/psql/mainloop.c
new file mode 100644
index b6cef94..4147238
*** a/src/bin/psql/mainloop.c
--- b/src/bin/psql/mainloop.c
***
*** 24,31 
   * This loop is re-entrant. May be called by \i command
   *	which reads input from a file.
   */
! int
! MainLoop(FILE *source)
  {
  	PsqlScanState scan_state;	/* lexer working state */
  	volatile PQExpBuffer query_buf;		/* buffer for query being accumulated */
--- 24,31 
   * This loop is re-entrant. May be called by \i command
   *	which reads input from a file.
   */
! static int
! 

Re: [HACKERS] Parallel Seq Scan

[Amit Kapila]

On Fri, Oct 23, 2015 at 4:41 PM, Amit Kapila 
wrote:
>
> On Fri, Oct 23, 2015 at 10:33 AM, Robert Haas 
wrote:

Please find the rebased partial seq scan patch attached with this
mail.

Robert suggested me off list that we should once try to see if we
can use Seq Scan node instead of introducing a new Partial Seq Scan
node. I have analyzed to see if we can use the SeqScan node (containing
parallel flag) instead of introducing new partial seq scan and found that
we primarily need to change most of the functions in nodeSeqScan.c to
have a parallel flag check and do something special for Partial Seq Scan
and apart from that we need special handling in function
ExecSupportsBackwardScan().  In general, I think we can make
SeqScan node parallel-aware by having some special paths without
introducing much complexity and that can save us code-duplication
between nodeSeqScan.c and nodePartialSeqScan.c.  One thing that makes
me slightly uncomfortable with this approach is that for partial seq scan,
currently the plan looks like:

QUERY PLAN
--
 Gather  (cost=0.00..2588194.25 rows=9990667 width=4)
   Number of Workers: 1
   ->  Partial Seq Scan on t1  (cost=0.00..89527.51 rows=9990667 width=4)
 Filter: (c1 > 1)
(4 rows)

Now instead of displaying Partial Seq Scan, if we just display Seq Scan,
then it might confuse user, so it is better to add some thing indicating
parallel node if we want to go this route.

Thoughts?



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


parallel_seqscan_partialseqscan_v24.patch
Description: Binary data

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

Re: [HACKERS] September 2015 Commitfest

[Michael Paquier]

On Mon, Nov 2, 2015 at 9:35 PM, Michael Paquier wrote:
> And now CF begins officially. The axe has fallen as promised 26 hours after.

Seeing no volunteers around, I can take the CFM hat for November's CF.
Any objections/complaints/remarks?
-- 
Michael


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

Re: [HACKERS] Foreign join pushdown vs EvalPlanQual

[Kouhei Kaigai]

> On Thu, Oct 29, 2015 at 6:05 AM, Kouhei Kaigai  wrote:
> > In this case, the EPQ slot to store the joined tuple is still
> > a challenge to be solved.
> >
> > Is it possible to use one or any of EPQ slots that are setup for
> > base relations but represented by ForeignScan/CustomScan?
> 
> Yes, I proposed that exact thing upthread.
> 
> > In case when ForeignScan run a remote join that involves three
> > base foreign tables (relid=2, 3, 5 for example), for example,
> > no other code touches this slot. So, it is safe even if we put
> > a joined tuple on EPQ slots of underlying base relations.
> >
> > In this case, EPQ slots are initialized as below:
> >
> >   es_epqTuple[0] ... EPQ tuple of base relation (relid=1)
> >   es_epqTuple[1] ... EPQ of the joined tuple (for relis=2, 3 5)
> >   es_epqTuple[2] ... EPQ of the joined tuple (for relis=2, 3 5), copy of 
> > above
> >   es_epqTuple[3] ... EPQ tuple of base relation (relid=4)
> >   es_epqTuple[4] ... EPQ of the joined tuple (for relis=2, 3 5), copy of 
> > above
> >   es_epqTuple[5] ... EPQ tuple of base relation (relid=6)
> 
> You don't really need to initialize them all.  You can just initialize
> es_epqTuple[1] and leave 2 and 4 unused.
> 
> > Then, if FDW/CSP is designed to utilize the preliminary joined
> > tuples rather than local join, it can just raise the tuple kept
> > in one of the EPQ slots for underlying base relations.
> > If FDW/CSP prefers local join, it can perform as like local join
> > doing; check join condition and construct a joined tuple by itself
> > or by alternative plan.
> 
> Right.
>
A challenge is that junk wholerow references on behalf of ROW_MARK_COPY
are injected by preprocess_targetlist(). It is earlier than the main path
consideration by query_planner(), thus, it is not predictable how remote
query shall be executed at this point.
If ROW_MARK_COPY, base tuple image is fetched using this junk attribute.
So, here is two options if we allow to put joined tuple on either of
es_epqTuple[].

options-1) We ignore record type definition. FDW returns a joined tuple
towards the whole-row reference of either of the base relations in this
join. The junk attribute shall be filtered out eventually and only FDW
driver shall see, so it is harmless to do (probably).
This option takes no big changes, however, we need a little brave to adopt.

options-2) We allow FDW/CSP to adjust target-list of the relevant nodes
after these paths get chosen by planner. It enables to remove whole-row
reference of base relations and add alternative whole-row reference instead
if FDW/CSP can support it.
This feature can be relevant to target-list push-down to the remote side,
not only EPQ rechecks, because adjustment of target-list means we allows
FDW/CSP to determine which expression shall be executed locally, or shall
not be.
I think, this option is more straightforward, however, needs a little bit
deeper consideration, because we have to design the best hook point and
need to ensure how path-ification will perform.

Therefore, I think we need two steps towards the entire solution.
Step-1) FDW/CSP will recheck base EPQ tuples and support local
reconstruction on the fly. It does not need something special
enhancement on the planner - so we can fix up by v9.5 release.
Step-2) FDW/CSP will support adjustment of target-list to add whole-row
reference of joined tuple instead of multiple base relations, then FDW/CSP
will be able to put a joined tuple on either of EPQ slot if it wants - it
takes a new feature enhancement, so v9.6 is a suitable timeline.

How about your opinion towards the direction?
I don't want to drop extra optimization opportunity, however, we are now in
November. I don't have enough brave to add none-obvious new feature here.

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


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

Re: [HACKERS] fortnight interval support

[Michael Paquier]

On Wed, Oct 28, 2015 at 4:17 AM, Gavin Flower wrote:
> You trying to get PostgreSQL banned in France???  :-)
>
> When I was learning French many years ago, I was told that the French
> consider their fortnight to be 15 days!!!

Confirmed. I would translate fornight as 'quinzaine' to French.
(please let's not add that btw).
-- 
Michael


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

Re: [HACKERS] WIP: Rework access method interface

[Alexander Korotkov]

On Tue, Nov 3, 2015 at 2:36 AM, Tom Lane  wrote:

> Alvaro Herrera  writes:
> > Tom Lane wrote:
> >> I'm kind of inclined to just let the verifiers read the catalogs for
> >> themselves.  AFAICS, a loop around the results of SearchSysCacheList
> >> is not going to be significantly more code than what this patch does,
> >> and it avoids presuming that we know what the verifiers will wish to
> >> look at.
>
> > Hmm, so this amounts to saying the verifier can only run after the
> > catalog rows are written.  Is that okay?
>
> Why not?  Surely we are not interested in performance-optimizing for
> the case of a detectably incorrect CREATE OP CLASS command.
>
> I don't actually care that much about running the verifiers during
> CREATE/etc OP CLASS at all.  It's at least as important to be able
> to run them across the built-in opclasses, considering all the chances
> for human error in constructing those things.  Even in the ALTER OP CLASS
> case, the verifier would likely need to look at existing catalog rows as
> well as the new ones.  So basically, I see zero value in exposing CREATE/
> ALTER OP CLASS's internal working representation to the verifiers.
>

I'm OK with validating opclass directly by system catalog, i.e. looping
over SearchSysCacheList results. Teodor was telling me something similar
personally.
I'll also try to rearrange header according to the comments upthread.

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

Re: [HACKERS] Re: Why not to use 'pg_ctl start -D ../data' to register posgtresql windows service

[Michael Paquier]

On Tue, Nov 3, 2015 at 10:46 AM, YuanyuanLiu wrote:
>I really learned a lot from you, and thank you!

By the way, in the future you may want to ask general questions to
pgsql-general, pgsql-hackers is where new features are being discussed
and where technical discussions happen :)
-- 
Michael


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

Re: [HACKERS] ALTER SYSTEM vs symlink

[Robert Haas]

On Mon, Nov 2, 2015 at 10:40 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Mon, Nov 2, 2015 at 10:13 PM, Amit Kapila  wrote:
>>> I think that is the sensible way to deal with this and any other such
>>> parameters.  We already have a way to disallow setting of individual
>>> parameters (GUC_DISALLOW_IN_AUTO_FILE) via Alter System.
>>> Currently we disallow to set data_directory via this mechanism and I think
>>> we can do the same for other parameters if required.  Do you think we
>>> should do some investigation/analysis about un-safe parameters rather
>>> then doing it in retail fashion?
>
>> -1.
>
>> This was discussed before, and I feel about it now the same way I felt
>> about it then: disallowing all GUCs that could potentially cause the
>> server not to start would make ALTER SYSTEM a whole lot less useful.
>
> I definitely agree that unconditionally disallowing "unsafe" parameters
> in ALTER SYSTEM would be counterproductive.  data_directory is disallowed
> there only because it's nonsensical: you can't even find the auto.conf
> file until you've settled on the data_directory.
>
> However, where I thought this discussion was going was to allow admins
> to selectively disable particular parameters from being set that way.
> Whether a particular installation finds locking down
> shared_preload_libraries to be more useful than allowing it to be set
> doesn't seem to me to be a one-size-fits-all question.  So at least in
> principle I'd be okay with a feature to control that.  But maybe it's
> sufficient to say "you can use sepgsql to restrict that".  Not every
> feature needs to be in core.

I really think this is a fine use of a custom contrib module.  If it's
important, we can even include that contrib module in the core
distribution as an example of how to restrict access to specific
commands or subcommands that a user may want to prevent in their
environment.

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


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

Re: [HACKERS] OS X El Capitan and DYLD_LIBRARY_PATH

[Andres Freund]

Hi,

On 2015-10-14 11:24:27 -0400, Peter Eisentraut wrote:
> The new OS X release 10.11 "El Capitan" has a "security" feature that
> prevents passing DYLD_LIBRARY_PATH to child processes.  Somehow, that
> variable is stripped from the environment.

Two colleagues of mine at Citus just hit that.

> The exact specifications of this new behavior aren't clear to me yet.
> For example, this C program works just fine:
> 
> ```
> #include 
> #include 
> 
> int
> main()
> {
> printf("DYLD_LIBRARY_PATH = %s\n", getenv("DYLD_LIBRARY_PATH"));
> return 0;
> }
> ```
> 
> but this shell script does not:
> 
> ```
> echo "DYLD_LIBRARY_PATH = $DYLD_LIBRARY_PATH"
> ```
> 
> There is breakage all over the Internet if you search for this, but the
> full details don't appear to be very clear.
> 
> One workaround is to disable System Integrity Protection, effectively
> restoring the behavior of the previous OS release.
> 
> Or don't upgrade quite yet if you don't want to deal with this at the
> moment.

Apparently the behaviour is that DYLD_LIBRARY_PATH is prevented from
being inherited into any system binaries. E.g. a shell. But specifying
it inside a shell script will allow it to be inherited to children of
that shell, unless the child is a protected process in turn.


I wonder if we could fix this by using install_name_tool during the
tempinstall to add an appropriate rpath.

Alternatively we could, apparently, specify a relative path to libraries
as explained here:
http://qin.laya.com/tech_coding_help/dylib_linking.html
 % install_name_tool -change libbz2.1.0.2.dylib  
@executable_path/../Frameworks/libbz2.1.0.2.dylib MyFunBinary

which ought to work independently from the tempinstall and normal
installation path.


Since I'm not a mac user I won't be able to check this out myself.


Andres


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

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

[Pavel Stehule]

2015-11-02 17:01 GMT+01:00 Catalin Iacob :

> Hello,
>
> Here's a detailed review:
>
> 1. in PLy_spi_error__init__ you need to check kw for NULL before doing
> PyDict_Size(kw) otherwise for plpy.SPIError() you get Bad internal
> call because PyDict_Size expects a real dictionary not NULL
>

PyDict_Size returns -1 when the dictionary is NULL

http://grokbase.com/t/python/python-dev/042ft6qaqq/pydict-size-error-return

done


>
> 2. a test with just plpy.SPIError() is still missing, this would have
> caught 1.
>

please, can you write some example - I am not able raise described error


>
> 3. the tests have "possibility to set all accessable fields in custom
> exception" above a test that doesn't set all fields, it's confusing
> (and accessible is spelled wrong)
>
>
done


> 4. in the docs, "The constructor of SPIError exception (class)
> supports keyword parameters." sounds better as "An SPIError instance
> can be constructed using keyword parameters."
>

done


>
> 5. there is conceptual code duplication between PLy_spi_exception_set
> in plpy_spi.c, since that code also constructs an SPIError but from C
> and with more info available (edata->internalquery and
> edata->internalpos). But making a tuple and assigning it to spidata is
> in both. Not sure how this can be improved.
>

I see it, but I don't think, so current code should be changed.
PLy_spi_exception_set is controlled directly by fully filled ErrorData
structure, __init__ is based only on keyword parameters with possibility
use inherited data. If I'll build ErrorData in __init__ function and call
PLy_spi_exception_set, then the code will be longer and more complex.


>
> 6. __init__ can use METH_VARARGS | METH_KEYWORDS in both Python2 and
> 3, no need for the #if
>
>
done


> 7. "could not create dictionary for SPI exception" would be more
> precise as "could not create dictionary for SPIError" right?
>

done


>
> 8. in PLy_add_methods_to_dictionary I would avoid import since it
> makes one think of importing modules; maybe "could not create
> functionwrapper for \"%s\"",  "could not create method wrapper for \"%s\""


done


>
> 9. also in PLy_add_methods_to_dictionary "could public method \"%s\"
> in dictionary" is not proper English and is missing not, maybe "could
> not add method \"%s\" to class dictionary"?
>

done


>
> 10. in PLy_add_methods_to_dictionary if PyCFunction_New succeeds but
> PyMethod_New fails, func will leak
>

done


>
> 11. it would be nice to have a test for the invalid SQLSTATE code part
>

done


>
> 12. similar to 10, in PLy_spi_error__init__ taking the "invalid
> SQLSTATE" branch leaks exc_args, in general early returns via PLy_elog
> will leave the intermediate Python objects leaking
>

dome


>
>
> Will mark the patch as Waiting for author.
>

attached new update

Regards

Pavel
diff --git a/doc/src/sgml/plpython.sgml b/doc/src/sgml/plpython.sgml
new file mode 100644
index 015bbad..9333fbe
*** a/doc/src/sgml/plpython.sgml
--- b/doc/src/sgml/plpython.sgml
*** $$ LANGUAGE plpythonu;
*** 1205,1210 
--- 1205,1235 
  approximately the same functionality
 

+ 
+   
+Raising Errors
+ 
+
+ The plpy module provides several possibilities to
+ to raise a exception:
+
+ 
+
+ 
+  SPIError([ message [, detail [, hint [, sqlstate  [, schema  [, table  [, column  [, datatype  [, constraint ])
+  
+
+An SPIError instance can be constructed using keyword parameters.
+ 
+ DO $$
+   raise plpy.SPIError('custom message', hint = 'It is test only');
+ $$ LANGUAGE plpythonu;
+ 
+
+  
+ 
+
+   
   
  
   
diff --git a/src/pl/plpython/expected/plpython_error.out b/src/pl/plpython/expected/plpython_error.out
new file mode 100644
index 1f52af7..435a5c2
*** a/src/pl/plpython/expected/plpython_error.out
--- b/src/pl/plpython/expected/plpython_error.out
*** EXCEPTION WHEN SQLSTATE 'SILLY' THEN
*** 422,424 
--- 422,486 
  	-- NOOP
  END
  $$ LANGUAGE plpgsql;
+ /* the possibility to set fields of custom exception
+  */
+ DO $$
+ raise plpy.SPIError('This is message text.',
+ detail = 'This is detail text',
+ hint = 'This is hint text.')
+ $$ LANGUAGE plpythonu;
+ ERROR:  plpy.SPIError: This is message text.
+ DETAIL:  This is detail text
+ HINT:  This is hint text.
+ CONTEXT:  Traceback (most recent call last):
+   PL/Python anonymous code block, line 4, in 
+ hint = 'This is hint text.')
+ PL/Python anonymous code block
+ \set VERBOSITY verbose
+ DO $$
+ raise plpy.SPIError('This is message text.',
+ detail = 'This is detail text',
+ hint = 'This is hint text.',
+ sqlstate = 'SILLY',
+ schema = 'any info about schema',
+ table = 'any info about table',
+ column = 'any info about column',
+ datatype = 'any 

Re: [HACKERS] [BUGS] BUG #12989: pg_size_pretty with negative values

[Julien Rouhaud]

On 03/11/2015 04:06, Robert Haas wrote:
> On Sat, Oct 31, 2015 at 2:25 PM, Julien Rouhaud
>  wrote:
>> I just reviewed your patch, everything looks fine for me. Maybe some
>> minor cosmetic changes could be made to avoid declaring too many vars,
>> but I think a committer would have a better idea on this, so I mark
>> this patch as ready for committer.
> 
> I don't think we should define Sign(x) as a macro in c.h. c.h is
> really only supposed to contain stuff that's pretty generic and
> universal, and the fact that we haven't needed it up until now
> suggests that Sign(x) isn't.  I'd suggest instead defining something
> like:
> 
> #define half_rounded(x)   (((x) + (x) < 0 ? 0 : 1) / 2)
> 
> Maybe rename numeric_divide_by_two to numeric_half_rounded.
> Generally, let's try to make the numeric and int64 code paths look as
> similar as possible.
> 
> Recomputing numeric_absolute(x) multiple times should be avoided.
> Compute it once and save the answer.
> 

Thanks for these comments. I therefore change the status to waiting on
author.

Regards.

-- 
Julien Rouhaud
http://dalibo.com - http://dalibo.org


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

Re: [HACKERS] Patent warning about the Greenplum source code

[Simon Riggs]

On 3 November 2015 at 08:12, Bruce Momjian  wrote:


> I am posting this at the request of Josh Berkus, who wanted
> clarification on some issues.  FYI, I have been speaking in this thread
> as a community member, and not as a member of core, and made some
> mistakes in my handling of this --- my apologies.
>

Thank you, apology accepted.


> The crux of my concern is that a patent in close-source software is
> barely visible --- it might be mentioned in marketing material or
> documentation, but that is unlikely.  When something is released as open
> source, by definition, the patented idea is visible in that code.  In a
> strange twist of fate, open source actually allows more chances for
> seeing patented ideas than closed source.


I agree with this.


> I should have made this clear
> in my initial post, and there is nothing Greenplum-specific about any of
> this.


Good. Now we have addressed the issue of balance, the fundamental issue
raised in your original post is still important and does need to be
addressed, against any and all companies/patents.

Your vigilance on patent issues is useful. Thank you very much.

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

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

Re: [HACKERS] [ADMIN] Replication slots and isolation levels

[Vladimir Borodin]

> 3 нояб. 2015 г., в 11:38, Andres Freund  написал(а):
> 
> On 2015-11-02 15:37:57 -0500, Robert Haas wrote:
>> On Fri, Oct 30, 2015 at 9:49 AM, Vladimir Borodin  wrote:
>>> I’ve tried two ways - bare SELECT in autocommit mode and BEGIN; SELECT;
>>> ROLLBACK. I first described the problem in thread on pgsql-admin@ [0], there
>>> is copy-paste from psql there, but during conversation initial description
>>> was lost.
>>> 
>>> [0]
>>> http://www.postgresql.org/message-id/7f74c5ea-6741-44fc-b6c6-e96f18d76...@simply.name
>> 
>> Hmm.  That behavior seems unexpected to me, but I might be missing something.
> 
> The conflict is because of a relation lock, not because of
> visibility. Hot-Standby feedback changes nothing about that.
> 
> I presume all the other conflicts are all because of relation level
> locks? Check pg_stat_database_conflicts and the server logs to verify.

Oh, good point, thank you, it gives the answer. Actually I’ve already done a 
switchover in this cluster, so pg_stat_database_conflicts started from scratch 
:( But the logs haven’t been rotated yet:

root@rpopdb01e ~ # fgrep -e 562f9ef0.23df,6 -e 562fa107.451a -e 562fa1d9.5146 
-e 562f9ef0.23df,10 -e 562fa259.56d1 
/var/lib/pgsql/9.4/data/pg_log/postgresql-2015-10-27_185736.csv
2015-10-27 19:06:28.656 MSK,,,9183,,562f9ef0.23df,6,,2015-10-27 18:57:36 
MSK,,0,LOG,0,"parameter ""hot_standby_feedback"" changed to 
""off""",""
2015-10-27 19:10:05.039 
MSK,"postgres","rpopdb",17690,"[local]",562fa107.451a,1,"",2015-10-27 19:06:31 
MSK,12/54563,0,ERROR,40001,"canceling statement due to conflict with 
recovery","User query might have needed to see row versions that must be 
removed.","select count(*) from rpop.rpop_imap_uidls;",,,"psql"
2015-10-27 19:10:05.995 
MSK,"monitor","rpopdb",20806,"localhost:51794",562fa1d9.5146,1,"",2015-10-27 
19:10:01 MSK,15/24192,0,ERROR,40001,"canceling statement due to conflict with 
recovery","User was holding shared buffer pin for too long.""SQL function 
""to_timestamp"" statement 1","select cnt from monitor.bad_rpop_total",,,""
2015-10-27 19:12:06.878 MSK,,,9183,,562f9ef0.23df,10,,2015-10-27 18:57:36 
MSK,,0,LOG,0,"parameter ""hot_standby_feedback"" changed to 
""on""",""
2015-10-27 19:17:57.056 
MSK,"postgres","rpopdb",5,"[local]",562fa259.56d1,1,"",2015-10-27 19:12:09 
MSK,3/35442,0,FATAL,40001,"terminating connection due to conflict with 
recovery","User was holding a relation lock for too long.","select count(*) 
from rpop.rpop_imap_uidls;",,,"psql"
root@rpopdb01e ~ #

So FATAL is due to relation lock and one ERROR is due to pinned buffers (this 
is actually from another user) but there is also one ERROR due to old snapshots 
(first line). But I actually turned off hs_feedback before first ERROR and 
turned it on after it. So it seems to work expectedly.

Does it actually mean that I could get such conflicts (due to relation locks, 
for example) even in repeatable read or serializable? I mean, is there any 
dependency between transaction isolation level on standby and conflicts with 
recovery?

And am I right that the only way not to have confl_lock is to increase 
max_standby_streaming_delay?


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


--
May the force be with you…
https://simply.name

Re: [HACKERS] Re: BUG #13685: Archiving while idle every archive_timeout with wal_level hot_standby

[Robert Haas]

On Mon, Nov 2, 2015 at 12:58 AM, Jeff Janes  wrote:
> If a transaction holding locks aborts on an otherwise idle server, perhaps it 
> will take a very long time for a log-shipping standby to realize this.  But I 
> have hard time believing that anyone who cares about that would be using 
> log-shipping (rather than streaming) anyway.

I'm sure other people here understand this better than me, but I
wonder if it wouldn't make more sense to somehow log this data only if
something material has changed in the data being logged.  This seems
to be trying to log something only if something else has been written
to WAL, which I'm not sure is the right test.

Also, this check here:

+   if (last_snapshot_lsn != insert_lsn &&
+   checkpoint_lsn != insert_lsn &&
+   checkpoint_lsn != previous_lsn)

...seems like it will fire if there have been 0 or 1 WAL records since
the last checkpoint, regardless of what they are.  I'm not sure that's
the right test, and it'll break again the minute we have a third thing
we want to log only if the system is non-idle.

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


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

Re: [HACKERS] Re: BUG #13685: Archiving while idle every archive_timeout with wal_level hot_standby

[Andres Freund]

On 2015-11-03 10:23:35 -0500, Robert Haas wrote:
> On Mon, Nov 2, 2015 at 12:58 AM, Jeff Janes  wrote:
> > If a transaction holding locks aborts on an otherwise idle server, perhaps 
> > it will take a very long time for a log-shipping standby to realize this.  
> > But I have hard time believing that anyone who cares about that would be 
> > using log-shipping (rather than streaming) anyway.
> 
> I'm sure other people here understand this better than me, but I
> wonder if it wouldn't make more sense to somehow log this data only if
> something material has changed in the data being logged.

Phew. That doesn't seem easy to measure. I'm doubtful that it's worth
comparing the snapshot and such, especially in the back
branches.

We could maybe add something that we only log a snapshot if XXX
megabytes have been logged or something. But I don't know which number
to pick here - and if there's other write activity the price of a
snapshot record really isn't high.

Andres


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

Re: [HACKERS] Documentation fix for psql

[Tom Lane]

Albe Laurenz  writes:
> The psql documentation calls the \pset options unicode_*_style
> when in reality they are called unicode_*_linestyle.
> This should be backpatched to 9.5.

So I see.  Looks like there are a few other grammatical issues in that
patch too ... will fix.  Thanks for the report!

regards, tom lane


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

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

[Robert Haas]

On Mon, Nov 2, 2015 at 12:15 PM, Peter Eisentraut  wrote:
> On 10/30/15 9:26 AM, Robert Haas wrote:
>> That's true, but doesn't allowing every parameter to be multiply
>> specified greatly increase the implementation complexity for a pretty
>> marginal benefit?
>
> Well, the way I would have approached is that after all the parsing you
> end up with a list of PQconninfoOption arrays and you you loop over that
> list and call connectDBStart() until you have a connection that
> satisfies you.
>
> I see that the patch doesn't do that and it looks quite messy as a result.

Hmm, I see.  That approach certainly seems worth considering.

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


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

Re: [HACKERS] Freeze avoidance of very large table.

[Robert Haas]

On Mon, Nov 2, 2015 at 10:33 PM, Amit Kapila  wrote:
> On Tue, Nov 3, 2015 at 5:04 AM, Robert Haas  wrote:
>>
>> On Sat, Oct 31, 2015 at 1:32 AM, Amit Kapila 
>> wrote:
>> >
>> > What is your main worry about changing the name of this map, is it
>> > about more code churn or is it about that we might introduce new issues
>> > or is it about that people are already accustomed to call this map as
>> > visibility map?
>>
>> My concern is mostly that I think calling it the "visibility and
>> freeze map" is excessively long and wordy.
>>
>> One observation that someone made previously is that there is a
>> difference between "all-visible" and "index-only scan OK".  An
>> all-visible page that has a HOT update is no longer all-visible (it
>> needs vacuuming) but an index-only scan would still be OK (because
>> only the non-indexed values in the tuple have changed, and every scan
>> scan can see either the old or the new tuple but not both.  At
>> present, the index-only scan will consult the heap page anyway,
>> because all we know is that the page is not all-visible.  But maybe in
>> the future somebody will decide to add a bit for that.  Then we'd have
>> the "visibility, usable for index-only scans, and freeze map", but I
>> think "_vufiosfm" will not be a good choice for a file suffix.
>>
>
> I think in that case we can call it as page info map or page state map, but
> I find retaining visibility map name in this case or for future (if we want
> to
> add another bit) as confusing.  In-fact if you find "visibility and freeze
> map",
> as excessively long, then we can change it to "page info map" or "page state
> map" now as well.

Sure.  Or we could just keep calling it the visibility map, and then
everyone would know what we're talking about.

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


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

Re: [HACKERS] Dangling Client Backend Process

[Robert Haas]

On Fri, Oct 30, 2015 at 11:03 AM, Andres Freund  wrote:
> On 2015-10-30 10:57:45 -0400, Tom Lane wrote:
>> Andres Freund  writes:
>> > adding a parseInput(conn) into the loop yields the expected
>> > FATAL:  57P01: terminating connection due to unexpected postmaster exit
>> > Is there really any reason not to do that?
>>
>> Might work, but it probably needs some study:
>
> Yea, definitely. I was just at pgconf.eu's keynote catching up on a
> talk. No fully thought through proposal's to be expected ;)
>
>> (a) is it safe
>
> I don't immediately see why not.
>
>> (b) is this the right place / are there other places
>
> I think it's ok for the send failure case, in a quick lookthrough I
> didn't find anything else for writes - I'm not entirely sure all read
> cases are handled tho, but it seems less likely to be mishandles.

pqHandleSendFailure() has this comment:

 * Primarily, what we want to accomplish here is to process an async
 * NOTICE message that the backend might have sent just before it died.

And also this comment:

 * Accept any available input data, ignoring errors.  Note that if
 * pqReadData decides the backend has closed the channel, it will close
 * our side of the socket --- that's just what we want here.

And finally this comment:

 * Parse any available input messages.  Since we are in PGASYNC_IDLE
 * state, only NOTICE and NOTIFY messages will be eaten.

Now, taken together, these messages suggest two conclusions:

1. The decision to ignore errors here was deliberate.
2. Calling pqParseInput() wouldn't notice errors anyway because the
connection is PGASYNC_IDLE.

With respect to the first conclusion, I think it's fair to argue that,
while this may have seemed like a reasonable idea at the time, it's
probably not what we want any more.  Quite apart from the patch
proposed here, ProcessInterrupts() has assume for years (certainly
since 9.0, I think) that it's legitimate to signal a FATAL error to an
idle client and assume that the client will read that error as a
response to its next protocol message.  So it seems to me that this
function should be adjusted to notice errors, and not just notice and
notify messages.

The second conclusion does not appear to be correct.  parseInput()
will call pqParseInput3() or pqParseInput2(), either of which will
handle an error as if it were a notice - i.e. by printing it out.

Here's a patch based on that analysis, addressing just that one
function, not any of the other changes talked about on this thread.
Does this make sense?  Would we want to back-patch it, and if so how
far, or just adjust master?  My gut is just master, but I don't know
why this issue wouldn't also affect Hot Standby kills and maybe other
kinds of connection termination situations, so maybe there's an
argument for back-patching.  On the third hand, failing to read the
error message off of a just-terminated connection isn't exactly a
crisis of the first order either.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 828f18e..07c4335 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -1553,8 +1553,8 @@ sendFailed:
 /*
  * pqHandleSendFailure: try to clean up after failure to send command.
  *
- * Primarily, what we want to accomplish here is to process an async
- * NOTICE message that the backend might have sent just before it died.
+ * Primarily, what we want to accomplish here is to process any messages that
+ * the backend might have sent just before it died.
  *
  * NOTE: this routine should only be called in PGASYNC_IDLE state.
  */
@@ -1562,16 +1562,16 @@ void
 pqHandleSendFailure(PGconn *conn)
 {
 	/*
-	 * Accept any available input data, ignoring errors.  Note that if
-	 * pqReadData decides the backend has closed the channel, it will close
-	 * our side of the socket --- that's just what we want here.
+	 * Accept and parse any available input data.  Note that if pqReadData
+	 * decides the backend has closed the channel, it will close our side of
+	 * the socket --- that's just what we want here.
 	 */
 	while (pqReadData(conn) > 0)
-		 /* loop until no more data readable */ ;
+		parseInput(conn);
 
 	/*
-	 * Parse any available input messages.  Since we are in PGASYNC_IDLE
-	 * state, only NOTICE and NOTIFY messages will be eaten.
+	 * Make one attempt to parse available input messages even if we read no
+	 * data.
 	 */
 	parseInput(conn);
 }

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

Re: [HACKERS] September 2015 Commitfest

[Robert Haas]

On Tue, Nov 3, 2015 at 8:12 AM, Michael Paquier
 wrote:
> On Mon, Nov 2, 2015 at 9:35 PM, Michael Paquier wrote:
>> And now CF begins officially. The axe has fallen as promised 26 hours after.
>
> Seeing no volunteers around, I can take the CFM hat for November's CF.
> Any objections/complaints/remarks?

I think that's great.  What, in your opinion, would  be the most
helpful thing for me to do to move things along?

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


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

Re: [HACKERS] Minor clarifying changes to abbreviated key abort code comments

[Peter Geoghegan]

On Tue, Nov 3, 2015 at 5:47 AM, Robert Haas  wrote:
> This comment doesn't make sense to me:
>
> +* (TSS_BUILDRUNS state prevents control reaching here in any
> +* case).
>
> Unless I'm missing something, that's not actually true.

It is true.  consider_abort_common() only actually considers aborting
when state is TSS_INITIAL (we're still doing an internal sort). The
only other pertinent state here is TSS_BUILDRUNS. The point is that
TSS_BUILDRUNS is a generic "point of no return" past which
abbreviation cannot be aborted. That is a little arbitrary.

-- 
Peter Geoghegan


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

[HACKERS] Bitmap index scans use of filters on available columns

[Jeff Janes]

create table f as select (random()*100)::int as x, md5(random()::text)
as y from generate_series(1,100);

create index on f (x, y);
analyze verbose f; --dont vacuum

explain select * from f where x=5 and y like '%abc%';

  QUERY PLAN
--
 Bitmap Heap Scan on f  (cost=382.67..9314.72 rows=1 width=37)
   Recheck Cond: (x = 5)
   Filter: (y ~~ '%abc%'::text)
   ->  Bitmap Index Scan on f_x_y_idx  (cost=0.00..382.67 rows=10433 width=0)
 Index Cond: (x = 5)

Is there a fundamental reason the filter on y is not being applied to
the index scan, rather than the heap scan?  Should this be a to-do
item?  This could avoid a lot of heap block reads, and also might
prevent the bitmap from overflowing work_mem and turning lossy.

Cheers,

Jeff


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

Re: [HACKERS] [patch] to build docs on Mac OS X El Capitan with MacPorts

[Neil Tiffin]

I should add that this was required for a postgres git build using MacPorts to 
supply dependencies and not a build of postgres using MacPorts.

Neil

> On Nov 3, 2015, at 8:11 AM, Robert Haas  wrote:
> 
> On Sun, Nov 1, 2015 at 8:41 AM, Neil Tiffin  wrote:
>> The attached patch was required to get the docs to build on Mac OS X 10.11.1 
>> (15B42) with MacPorts 2.3.4.  After changing docbook.m4 ‘autoreconf’ has to 
>> be run.  This patch does not include the new version of ‘configure'
> 
> Can anyone else verify this?



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

Re: [HACKERS] fortnight interval support

[Robert Haas]

On Tue, Nov 3, 2015 at 8:31 AM, Alvaro Herrera  wrote:
> (WRT the reference to Jane Austen and "Se'ennight" for "week", it occurs
> to me that fortnight is a similar contraction for "forteen night".)

Well, clearly we also need enquië for the elves of Arda and tenday for
for Faerûnians.  Seems like a serious oversight, though making enquië
work in non-Unicode locales might be tricky.

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


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

Re: [HACKERS] Getting sorted data from foreign server

[Robert Haas]

On Fri, Oct 30, 2015 at 6:19 AM, Ashutosh Bapat
 wrote:
> If there is a collate clause in the ORDER BY, the server crashes with
> assertion
> +Assert(loc_cxt.state == FDW_COLLATE_NONE ||
> +loc_cxt.state == FDW_COLLATE_SAFE);
>
>
> The assertion is fine as long as is_foreign_expr() tests only boolean
> expressions (appearing in quals). This patch uses the function to test an
> expression appearing in ORDER BY clause, which need not be boolean. Attached
> patch removed the assertion and instead makes the function return false,
> when the walker deems collation of the expression unsafe. The walker can not
> return false when it encounter unsafe expression since the subtree it's
> examining might be part of an expression which does not use the collation
> ultimately.

I've committed this version.

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


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

Re: [HACKERS] Minor clarifying changes to abbreviated key abort code comments

[Peter Geoghegan]

On Tue, Nov 3, 2015 at 11:15 AM, Robert Haas  wrote:
> OK, I see.  Fixing comments in the back-branches is not always a
> productive use of time, and in general I might like it if you pushed
> for such things less frequently.  But I've done it anyway in this
> instance.

I guess I favor doing it where the comment is actually wrong, which
does apply here -- we don't *rely* on anything. We could very well
abort when state is TSS_BUILDRUNS, but we elect not too, since
TSS_BUILDRUNS is taken to mean that it's too late for aborting to be
worth it.

Thanks
-- 
Peter Geoghegan


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

Re: [HACKERS] Minor clarifying changes to abbreviated key abort code comments

[Robert Haas]

On Tue, Nov 3, 2015 at 12:36 PM, Peter Geoghegan  wrote:
> On Tue, Nov 3, 2015 at 5:47 AM, Robert Haas  wrote:
>> This comment doesn't make sense to me:
>>
>> +* (TSS_BUILDRUNS state prevents control reaching here in any
>> +* case).
>>
>> Unless I'm missing something, that's not actually true.
>
> It is true.  consider_abort_common() only actually considers aborting
> when state is TSS_INITIAL (we're still doing an internal sort). The
> only other pertinent state here is TSS_BUILDRUNS. The point is that
> TSS_BUILDRUNS is a generic "point of no return" past which
> abbreviation cannot be aborted. That is a little arbitrary.

OK, I see.  Fixing comments in the back-branches is not always a
productive use of time, and in general I might like it if you pushed
for such things less frequently.  But I've done it anyway in this
instance.

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


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

Re: [HACKERS] Minor clarifying changes to abbreviated key abort code comments

[Robert Haas]

On Tue, Nov 3, 2015 at 2:19 PM, Peter Geoghegan  wrote:
> On Tue, Nov 3, 2015 at 11:15 AM, Robert Haas  wrote:
>> OK, I see.  Fixing comments in the back-branches is not always a
>> productive use of time, and in general I might like it if you pushed
>> for such things less frequently.  But I've done it anyway in this
>> instance.
>
> I guess I favor doing it where the comment is actually wrong, which
> does apply here -- we don't *rely* on anything. We could very well
> abort when state is TSS_BUILDRUNS, but we elect not too, since
> TSS_BUILDRUNS is taken to mean that it's too late for aborting to be
> worth it.

Fair point.

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


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

Re: [HACKERS] Freeze avoidance of very large table.

[Masahiko Sawada]

On Tue, Nov 3, 2015 at 12:33 PM, Amit Kapila  wrote:
> On Tue, Nov 3, 2015 at 5:04 AM, Robert Haas  wrote:
>>
>> On Sat, Oct 31, 2015 at 1:32 AM, Amit Kapila 
>> wrote:
>> >
>> > What is your main worry about changing the name of this map, is it
>> > about more code churn or is it about that we might introduce new issues
>> > or is it about that people are already accustomed to call this map as
>> > visibility map?
>>
>> My concern is mostly that I think calling it the "visibility and
>> freeze map" is excessively long and wordy.
>>
>> One observation that someone made previously is that there is a
>> difference between "all-visible" and "index-only scan OK".  An
>> all-visible page that has a HOT update is no longer all-visible (it
>> needs vacuuming) but an index-only scan would still be OK (because
>> only the non-indexed values in the tuple have changed, and every scan
>> scan can see either the old or the new tuple but not both.  At
>> present, the index-only scan will consult the heap page anyway,
>> because all we know is that the page is not all-visible.  But maybe in
>> the future somebody will decide to add a bit for that.  Then we'd have
>> the "visibility, usable for index-only scans, and freeze map", but I
>> think "_vufiosfm" will not be a good choice for a file suffix.
>>
>
> I think in that case we can call it as page info map or page state map, but
> I find retaining visibility map name in this case or for future (if we want
> to
> add another bit) as confusing.  In-fact if you find "visibility and freeze
> map",
> as excessively long, then we can change it to "page info map" or "page state
> map" now as well.
>

In that case, file suffix would be "_pim" or "_psm"?
IMO, "page info map" would be better, because the bit doesn't indicate
the status of page in real time, it's just additional information.
Also we need to rewrite to new name in source code, and source file
name as well.

Regards,

--
Masahiko Sawada


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

Re: [HACKERS] [PATCH] postgres_fdw extension support

[Tom Lane]

Paul Ramsey  writes:
> [ 20151006b_postgres_fdw_extensions.patch ]

Starting to look through this now.  I'm dubious of the decision to have
ExtractExtensionList throw errors if there are un-installed extensions
mentioned in the FDW options.  Wouldn't it be a lot more convenient if
such extension names were silently ignored?  You cannot guarantee that the
list is always up to date anyway; consider creating a server, setting some
extension options, and then dropping some of those extensions.  Moreover,
the current semantics create a hazard for pg_dump, which can't reasonably
be expected to know that it must restore extensions X and Y before it can
create foreign server Z.

There might be a case for raising a WARNING during
postgres_fdw_validator(), but no more than that, IMO.  Certainly ERROR
during regular use of the server is right out.

Comments?

regards, tom lane


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

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

[Pavel Stehule]

2015-11-03 17:13 GMT+01:00 Catalin Iacob :

> On Tue, Nov 3, 2015 at 12:49 PM, Pavel Stehule 
> wrote:
> >> 1. in PLy_spi_error__init__ you need to check kw for NULL before doing
> >> PyDict_Size(kw) otherwise for plpy.SPIError() you get Bad internal
> >> call because PyDict_Size expects a real dictionary not NULL
> >
> >
> > PyDict_Size returns -1 when the dictionary is NULL
> >
> >
> http://grokbase.com/t/python/python-dev/042ft6qaqq/pydict-size-error-return
>
> Yes, but it also sets the error indicator to BadInternalCall. In 2.7
> the code is:
> Py_ssize_t
> PyDict_Size(PyObject *mp)
> {
> if (mp == NULL || !PyDict_Check(mp)) {
> PyErr_BadInternalCall();
> return -1;
> }
> return ((PyDictObject *)mp)->ma_used;
> }
>
> I had a PLy_elog right after the PyDict_Size call for debugging and
> that one was raising BadInternalCall since it checked the error
> indicator. So the NULL check is needed.
>

I did it in last patch - PyDict_Size is not called when kw is NULL


>
> >> 2. a test with just plpy.SPIError() is still missing, this would have
> >> caught 1.
>

one test contains "x = plpy.SPIError()". Is it, what you want?


> >
> >
> > please, can you write some example - I am not able raise described error
>
> The example was plpy.SPIError() but I now realize that, in order to
> see a failure, you need the extra PLy_elog which I had in there.
> But this basic form of the constructor is still an important thing to
> test so please add this as well to the regression test.
>
> >> 5. there is conceptual code duplication between PLy_spi_exception_set
> >> in plpy_spi.c, since that code also constructs an SPIError but from C
> >> and with more info available (edata->internalquery and
> >> edata->internalpos). But making a tuple and assigning it to spidata is
> >> in both. Not sure how this can be improved.
> >
> >
> > I see it, but I don't think, so current code should be changed.
> > PLy_spi_exception_set is controlled directly by fully filled ErrorData
> > structure, __init__ is based only on keyword parameters with possibility
> use
> > inherited data. If I'll build ErrorData in __init__ function and call
> > PLy_spi_exception_set, then the code will be longer and more complex.
>
> Indeed, I don't really see how to improve this but it does bug me a bit.
>
> One more thing,
> +The plpy module provides several possibilities to
> +to raise a exception:
>
> This has "to" 2 times and is weird since it says it offers several
> possibilities but then shows only one (the SPIError constructor).
> And SPIError should be plpy.SPIError everywhere to
> be consistent.
>

I'll do it tomorrow

>
> Maybe something like (needs markup):
> A plpy.SPIError can be raised from PL/Python, the constructor accepts
> keyword parameters:
>   plpy.SPIError([ message [, detail [, hint [, sqlstate [, schema [,
> table [, column [, datatype [, constraint ])
> then the example
>
> If you fix the doc and add the plpy.SPIError() test I'm happy. I'll
> give it one more test on Python2.7 and 3.5 and mark it Ready for
> Committer.
>

Regards

Pavel

Re: [HACKERS] [PATCH] postgres_fdw extension support

[Robert Haas]

On Tue, Nov 3, 2015 at 2:57 PM, Tom Lane  wrote:
> Paul Ramsey  writes:
>> [ 20151006b_postgres_fdw_extensions.patch ]
>
> Starting to look through this now.  I'm dubious of the decision to have
> ExtractExtensionList throw errors if there are un-installed extensions
> mentioned in the FDW options.  Wouldn't it be a lot more convenient if
> such extension names were silently ignored?  You cannot guarantee that the
> list is always up to date anyway; consider creating a server, setting some
> extension options, and then dropping some of those extensions.  Moreover,
> the current semantics create a hazard for pg_dump, which can't reasonably
> be expected to know that it must restore extensions X and Y before it can
> create foreign server Z.
>
> There might be a case for raising a WARNING during
> postgres_fdw_validator(), but no more than that, IMO.  Certainly ERROR
> during regular use of the server is right out.

Agreed.  I don't know whether it's better to emit a WARNING or some
lower-level message (INFO, DEBUG), but I think an ERROR will suck due
to the pg_dump issues you mention.

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


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

Re: [HACKERS] fortnight interval support

[Michael Paquier]

On Wed, Nov 4, 2015 at 2:31 AM, Robert Haas  wrote:
> On Tue, Nov 3, 2015 at 8:31 AM, Alvaro Herrera  
> wrote:
>> (WRT the reference to Jane Austen and "Se'ennight" for "week", it occurs
>> to me that fortnight is a similar contraction for "forteen night".)
>
> Well, clearly we also need enquië for the elves of Arda and tenday for
> for Faerûnians.  Seems like a serious oversight, though making enquië
> work in non-Unicode locales might be tricky.

You are forgetting Klingon, parseltongue, dwarfic and the one of
Mordor. It's not worth angering them as well. But I'll stop here.
-- 
Michael


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

Re: [HACKERS] [PATCH] postgres_fdw extension support

[Tom Lane]

Robert Haas  writes:
> On Tue, Nov 3, 2015 at 2:57 PM, Tom Lane  wrote:
>> Paul Ramsey  writes:
>>> [ 20151006b_postgres_fdw_extensions.patch ]

>> There might be a case for raising a WARNING during
>> postgres_fdw_validator(), but no more than that, IMO.  Certainly ERROR
>> during regular use of the server is right out.

> Agreed.  I don't know whether it's better to emit a WARNING or some
> lower-level message (INFO, DEBUG), but I think an ERROR will suck due
> to the pg_dump issues you mention.

I've committed this with a WARNING during validation and no comment
otherwise.

I left out the proposed regression tests because they fail in "make
installcheck" mode, unless you've previously built and installed cube
and seg, which seems like an unacceptable requirement to me.  I don't
think that leaving the code untested is a good final answer, of course.
The idea I was toying with was to create a dummy extension for testing
purposes by means of doing a direct INSERT into pg_extension --- which
is ugly and would only work for superusers, but the latter is true of
"CREATE EXTENSION cube" too.  Anybody have a better idea?

regards, tom lane


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

Re: [HACKERS] [PATCH] postgres_fdw extension support

[Paul Ramsey]

Thanks everyone for the held and feedback on this patch!

-- 
Paul Ramsey
http://cleverelephant.ca
http://postgis.net

On November 3, 2015 at 3:47:37 PM, Tom Lane (t...@sss.pgh.pa.us) wrote:

Robert Haas  writes:  
> On Tue, Nov 3, 2015 at 2:57 PM, Tom Lane  wrote:  
>> Paul Ramsey  writes:  
>>> [ 20151006b_postgres_fdw_extensions.patch ]  

>> There might be a case for raising a WARNING during  
>> postgres_fdw_validator(), but no more than that, IMO. Certainly ERROR  
>> during regular use of the server is right out.  

> Agreed. I don't know whether it's better to emit a WARNING or some  
> lower-level message (INFO, DEBUG), but I think an ERROR will suck due  
> to the pg_dump issues you mention.  

I've committed this with a WARNING during validation and no comment  
otherwise.  

I left out the proposed regression tests because they fail in "make  
installcheck" mode, unless you've previously built and installed cube  
and seg, which seems like an unacceptable requirement to me. I don't  
think that leaving the code untested is a good final answer, of course.  
The idea I was toying with was to create a dummy extension for testing  
purposes by means of doing a direct INSERT into pg_extension --- which  
is ugly and would only work for superusers, but the latter is true of  
"CREATE EXTENSION cube" too. Anybody have a better idea?  

regards, tom lane  

Re: [HACKERS] pg_stat_statements query jumbling question

[Peter Geoghegan]

On Sat, Oct 31, 2015 at 10:03 AM, Julien Rouhaud
 wrote:
>> At least, I would like to give some options to be chosen by the
>> user. Is it possible and/or reasonable?
>>
>
> I'm also rather sceptical about this change.

Is anyone willing to argue for it, apart from Satoshi?


-- 
Peter Geoghegan


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

Re: [HACKERS] Re: BUG #13685: Archiving while idle every archive_timeout with wal_level hot_standby

[Andres Freund]

On November 4, 2015 12:37:02 AM GMT+01:00, Michael Paquier 
 wrote:
>On Wed, Nov 4, 2015 at 12:43 AM, Andres Freund 
>wrote:
>> On 2015-11-03 10:23:35 -0500, Robert Haas wrote:
>>> On Mon, Nov 2, 2015 at 12:58 AM, Jeff Janes 
>wrote:
>>> > If a transaction holding locks aborts on an otherwise idle server,
>perhaps it will take a very long time for a log-shipping standby to
>realize this.  But I have hard time believing that anyone who cares
>about that would be using log-shipping (rather than streaming) anyway.
>>>
>>> I'm sure other people here understand this better than me, but I
>>> wonder if it wouldn't make more sense to somehow log this data only
>if
>>> something material has changed in the data being logged.
>>
>> Phew. That doesn't seem easy to measure. I'm doubtful that it's worth
>> comparing the snapshot and such, especially in the back
>> branches.
>
>Well, I guess that's why I thought it would be more simple to check if
>we are at the beginning of a segment at first sight. This has no
>chance to break if anything else like that is being added in the
>future as it doesn't depend on the record types, though new similar
>records added on a timely manner would need a similar check. Perhaps
>this could be coupled by a check on the last XLOG_SWITCH_XLOG record
>instead of checkpoint activity though.
>
>> We could maybe add something that we only log a snapshot if XXX
>> megabytes have been logged or something. But I don't know which
>number
>> to pick here - and if there's other write activity the price of a
>> snapshot record really isn't high.
>
>On a completely idle system, I don't think we should log any standby
>records. This is what ~9.3 does.

Are you sure? I think it'll around checkpoints, no? I thought Heikki had fixed 
that, but looking sound that doesn't seem to be the case.

Andres

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


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

Re: [HACKERS] RFC/WIP: adding new configuration options to TOAST

[Craig Ringer]

On 3 November 2015 at 23:04, Bill Moran  wrote:
>
> Looking for feedback to see if anyone sees any issues or has any
> suggestions on what I'm doing. The attached patch alters 3 things
> with regard to TOAST behavior:

COMPRESSION_TEST_SIZE (2) seems useful.

The other two mostly seem like options nobody's going to know are
there, or know how to sensibly set if they do notice them. What's the
driving reason behind those, the problem you're trying to solve? Why
make them configurable per-table (or at all)?

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


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

Re: [HACKERS] Re: BUG #13685: Archiving while idle every archive_timeout with wal_level hot_standby

[Michael Paquier]

On Wed, Nov 4, 2015 at 12:43 AM, Andres Freund  wrote:
> On 2015-11-03 10:23:35 -0500, Robert Haas wrote:
>> On Mon, Nov 2, 2015 at 12:58 AM, Jeff Janes  wrote:
>> > If a transaction holding locks aborts on an otherwise idle server, perhaps 
>> > it will take a very long time for a log-shipping standby to realize this.  
>> > But I have hard time believing that anyone who cares about that would be 
>> > using log-shipping (rather than streaming) anyway.
>>
>> I'm sure other people here understand this better than me, but I
>> wonder if it wouldn't make more sense to somehow log this data only if
>> something material has changed in the data being logged.
>
> Phew. That doesn't seem easy to measure. I'm doubtful that it's worth
> comparing the snapshot and such, especially in the back
> branches.

Well, I guess that's why I thought it would be more simple to check if
we are at the beginning of a segment at first sight. This has no
chance to break if anything else like that is being added in the
future as it doesn't depend on the record types, though new similar
records added on a timely manner would need a similar check. Perhaps
this could be coupled by a check on the last XLOG_SWITCH_XLOG record
instead of checkpoint activity though.

> We could maybe add something that we only log a snapshot if XXX
> megabytes have been logged or something. But I don't know which number
> to pick here - and if there's other write activity the price of a
> snapshot record really isn't high.

On a completely idle system, I don't think we should log any standby
records. This is what ~9.3 does.
-- 
Michael


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

Re: [HACKERS] PoC: Partial sort

[Peter Geoghegan]

On Tue, Oct 20, 2015 at 4:17 AM, Alexander Korotkov
 wrote:
> Planner regression is fixed in the attached version of patch. It appears
> that get_cheapest_fractional_path_for_pathkeys() behaved wrong when no
> ordering is required.

I took a look at this. My remarks are not comprehensive, but are just
what I noticed having looked at this for the first time in well over a
year.

Before anything else, I would like to emphasize that I think that this
is pretty important work; it's not just a "nice to have". I'm very
glad you picked it up, because we need it. In the real world, there
will be *lots* of cases that this helps.

Explain output
---

A query like your original test query looks like this for me:

postgres=# explain analyze select * from test order by v1, v2 limit 100;
 QUERY
PLAN

 Limit  (cost=429.54..434.53 rows=100 width=16) (actual
time=15125.819..22414.038 rows=100 loops=1)
   ->  Partial sort  (cost=429.54..50295.52 rows=100 width=16)
(actual time=15125.799..22413.297 rows=100 loops=1)
 Sort Key: v1, v2
 Presorted Key: v1
 Sort Method: top-N heapsort  Memory: 27kB
 ->  Index Scan using test_v1_idx on test
(cost=0.42..47604.43 rows=100 width=16) (actual time=1.663..13.066
rows=151 loops=1)
 Planning time: 0.948 ms
 Execution time: 22414.895 ms
(8 rows)

I thought about it for a while, and decided that you have the basic
shape of the explain output right here. I see where you are going by
having the sort node drive things.

I don't think the node should be called "Partial sort", though. I
think that this is better presented as just a "Sort" node with a
presorted key.

I think it might be a good idea to also have a "Sort Groups: 2" field
above. That illustrates that you are in fact performing 2 small sorts
per group, which is the reality. As you said, it's good to have this
be high, because then the sort operations don't need to do too many
comparisons, which could be expensive.

Sort Method


Even thought the explain analyze above shows "top-N heapsort" as its
sort method, that isn't really true. I actually ran this through a
debugger, which is why the above plan took so long to execute, in case
you wondered. I saw that in practice the first sort executed for the
first group uses a quicksort, while only the second sort (needed for
the 2 and last group in this example) used a top-N heapsort.

Is it really worth using a top-N heapsort to avoid sorting the last
little bit of tuples in the last group? Maybe we should never push
down to an individual sort operation (we have one
tuplesort_performsort() per group) that it should be bounded. Our
quicksort falls back on insertion sort in the event of only having 7
elements (at that level of recursion), so having this almost always
use quicksort may be no bad thing.

Even if you don't like that, the "Sort Method" shown above is just
misleading. I wonder, also, if you need to be more careful about
whether or not "Memory" is really the high watermark, as opposed to
the memory used by the last sort operation of many. There could be
many large tuples in one grouping, for example. Note that the current
code will not show any "Memory" in explain analyze for cases that have
memory freed before execution is done, which this is not consistent
with. Maybe that's not so important. Unsure.

trace_sort output shows that these queries often use a large number of
tiny individual sorts. Maybe that's okay, or maybe we should make it
clearer that the sorts are related. I now use trace_sort a lot.

Abbreviated Keys
---

It could be very bad for performance that the first non-presorted key
uses abbreviated keys. There needs to be a way to tell tuplesort to
not waste its time with them, just as there currently is for bounded
(top-N heapsort) sorts. They're almost certainly the wrong way to go,
unless you have huge groups (where partial sorting is unlikely to win
in the first place).

Other issues in executor


This is sort of an optimizer issue, but code lives in execAmi.c.
Assert is redundant here:

+   case T_Sort:
+   /* We shouldn't reach here without having plan node */
+   Assert(node);
+   /* With skipCols sort node holds only last bucket */
+   if (node && ((Sort *)node)->skipCols == 0)
+   return true;
+   else
+   return false;

I don't like that you've added a Plan node argument to
ExecMaterializesOutput() in this function, too.

There is similar assert/pointer test code within
ExecSupportsBackwardScan() and ExecSupportsMarkRestore(). In general,
I have 

Re: [HACKERS] RFC/WIP: adding new configuration options to TOAST

[Jeff Janes]

On Tue, Nov 3, 2015 at 5:21 PM, Craig Ringer  wrote:
> On 3 November 2015 at 23:04, Bill Moran  wrote:
>>
>> Looking for feedback to see if anyone sees any issues or has any
>> suggestions on what I'm doing. The attached patch alters 3 things
>> with regard to TOAST behavior:
>
> COMPRESSION_TEST_SIZE (2) seems useful.
>
> The other two mostly seem like options nobody's going to know are
> there, or know how to sensibly set if they do notice them. What's the
> driving reason behind those, the problem you're trying to solve? Why
> make them configurable per-table (or at all)?

I currently have a table with one column which has a median width of
500 bytes, a 90th percentile of 650 bytes, and makes up 75% of the
table's size, and the column is rarely used, while the table itself is
frequently seq scanned.  I'd very much like to drive that column out
of main and into toast. I think target_tuple_size would let me do
that.

(Per-column control would be even nicer, but I'd take what I can get)

Cheers,

Jeff


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

Re: [HACKERS] RFC/WIP: adding new configuration options to TOAST

[Bill Moran]

On Tue, 3 Nov 2015 18:34:39 -0800
Jeff Janes  wrote:

> On Tue, Nov 3, 2015 at 5:21 PM, Craig Ringer  wrote:
> > On 3 November 2015 at 23:04, Bill Moran  wrote:
> >>
> >> Looking for feedback to see if anyone sees any issues or has any
> >> suggestions on what I'm doing. The attached patch alters 3 things
> >> with regard to TOAST behavior:
> >
> > COMPRESSION_TEST_SIZE (2) seems useful.
> >
> > The other two mostly seem like options nobody's going to know are
> > there, or know how to sensibly set if they do notice them. What's the
> > driving reason behind those, the problem you're trying to solve? Why
> > make them configurable per-table (or at all)?
> 
> I currently have a table with one column which has a median width of
> 500 bytes, a 90th percentile of 650 bytes, and makes up 75% of the
> table's size, and the column is rarely used, while the table itself is
> frequently seq scanned.  I'd very much like to drive that column out
> of main and into toast. I think target_tuple_size would let me do
> that.

That's exactly the use case. As it currently stands, any tuple smaller
than about 2K will never be toasted. So if you have 1900 bytes of
highly compressible text that is infrequently queried from the table
whilst other columns are frequently accessed, there's no way to force
it to be out of line from the main table, or be compressed.

The two new configurables allow the DBA to make tradeoff decisions on
CPU usage vs. storage efficiency. Since the TOAST code attempts to
process the column that will provide the largest gain first, in your
described use case you could calculate the size of the other columns,
and set the target_tuple_size to just a bit larger than that, and
that large column should get moved into the toast table in most or
all cases (depending on how predictable the other sizes are)

Compression is a similarly hard-coded value in current versions.
I feel that allowing the DBA to control how much savings is required
before incurring the overhead of compression is worthwhile, especially
when considered on a per-table basis. For example, the compression
on an archive table could be very aggressive, whereas compression on
a frequently accessed table might only be justified if it saves a lot
of space. How much space compression saves is highly dependent on the
data being stored.

I don't have anything remotely like statistical advice on how much
improvement can actually be gained yet. Once I have per-table values
implemented, it will be much, much easier to test the impact.

It does sound like I need to spend a little more time improving the
documentation to ensure that it's clear what these values achieve.

> (Per-column control would be even nicer, but I'd take what I can get)

Oddly, I hadn't considered getting as granualar as per-column, but
now that you've got me thinking about it, it seems like a logical
step to take.

-- 
Bill Moran


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

Re: [HACKERS] [PATCH] postgres_fdw extension support

[Tom Lane]

I wrote:
> I left out the proposed regression tests because they fail in "make
> installcheck" mode, unless you've previously built and installed cube
> and seg, which seems like an unacceptable requirement to me.  I don't
> think that leaving the code untested is a good final answer, of course.
> The idea I was toying with was to create a dummy extension for testing
> purposes by means of doing a direct INSERT into pg_extension --- which
> is ugly and would only work for superusers, but the latter is true of
> "CREATE EXTENSION cube" too.  Anybody have a better idea?

I had a possibly better idea: instead of manufacturing an empty extension
with a direct INSERT, hack on the one extension that we know for sure
will be installed, namely postgres_fdw itself.  So we could do, eg,

create function foo() ...
alter extension postgres_fdw add function foo();

and then test shippability of foo() with or without having listed
postgres_fdw as a shippable extension.

This is certainly pretty ugly in its own right, but it would avoid
the maintainability hazards of an explicit INSERT into pg_extension.
So on balance it seems a bit nicer than my first idea.

regards, tom lane


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

[HACKERS] Re: Why not to use 'pg_ctl start -D ../data' to register posgtresql windows service

[YuanyuanLiu]

Ok, Michael! Thanks for your kindly remind!

Regards!
Liu Yuanyuan



--
View this message in context: 
http://postgresql.nabble.com/Why-not-to-use-pg-ctl-start-D-data-to-register-posgtresql-windows-service-tp5872282p5872631.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.


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

Re: [HACKERS] [PATCH] postgres_fdw extension support

[Michael Paquier]

On Wed, Nov 4, 2015 at 2:23 PM, Tom Lane  wrote:
> Michael Paquier  writes:
>> That's still strange to have a dummy object in
>> postgres_fdw.so just for testing purposes.
>
> We could drop the extra functions at the end of the test, but I don't
> see the point exactly.  We'd just be leaving the regression test database
> with some odd contents of the extension --- there's not any wider effect
> than that.

Oh, I see. Then we rely just on the fact that postgres_fdw is
installed. Are you working on a patch? Do you need one?
-- 
Michael


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

Re: [HACKERS] Getting sorted data from foreign server

[Ashutosh Bapat]

On Tue, Nov 3, 2015 at 11:35 PM, Robert Haas  wrote:

> On Fri, Oct 30, 2015 at 6:19 AM, Ashutosh Bapat
>  wrote:
> > If there is a collate clause in the ORDER BY, the server crashes with
> > assertion
> > +Assert(loc_cxt.state == FDW_COLLATE_NONE ||
> > +loc_cxt.state == FDW_COLLATE_SAFE);
> >
> >
> > The assertion is fine as long as is_foreign_expr() tests only boolean
> > expressions (appearing in quals). This patch uses the function to test an
> > expression appearing in ORDER BY clause, which need not be boolean.
> Attached
> > patch removed the assertion and instead makes the function return false,
> > when the walker deems collation of the expression unsafe. The walker can
> not
> > return false when it encounter unsafe expression since the subtree it's
> > examining might be part of an expression which does not use the collation
> > ultimately.
>
> I've committed this version.
>
>
Thanks.


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



-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

Re: [HACKERS] Trivial heap_finish_speculative() error message inaccuracy

[Tom Lane]

Peter Geoghegan  writes:
> While auditing the access method code associated with ON CONFLICT DO
> UPDATE's speculative insertion infrastructure, I noticed an
> inaccuracy.

> Attached patch fixes the trivial inaccuracy in a defensive elog()
> call. Quite simply, this call site didn't get the memo when we renamed
> that function during the late stages of development.

This seems like a fine teaching moment in which to point out our
longstanding error message style guideline that says not to put
names of C functions into error messages in the first place.

regards, tom lane


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

Re: [HACKERS] Request: pg_cancel_backend variant that handles 'idle in transaction' sessions

[Amit Kapila]

On Tue, Nov 3, 2015 at 7:56 PM, Pavel Stehule 
wrote:

>
>
> 2015-11-03 3:42 GMT+01:00 Amit Kapila :
>
>> On Mon, Nov 2, 2015 at 10:45 PM, Pavel Stehule 
>> wrote:
>>>
>>>
>>> It is 100% true. But the users can do strange things. If we solve idle
>>> transactions and not idle session, then they are able to increase
>>> max_connections to thousands with happy smile in face.
>>>
>>> I have not strong idea about how to solve it well - maybe introduce
>>> transaction_idle_timeout and session_idle_timeout?
>>>
>>>
>> What exactly do we want to define session_idle_timeout?  Some
>> possibilities:
>> a. Reset the session related variables like transaction, prepared
>> statements, etc. and retain it for connection pool kind of stuff
>> b. Exit from the session
>>
>
> b is safe state - and currently it is only one state, that we can forward
> to client side (with keep_alive packets) - so I prefer b
>
>
Okay, I think one more point to consider is that it would be preferable to
have such an option for backend sessions and not for other processes
like WalSender.


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

Re: [HACKERS] RFC/WIP: adding new configuration options to TOAST

[Craig Ringer]

On 4 November 2015 at 10:58, Bill Moran  wrote:
> On Tue, 3 Nov 2015 18:34:39 -0800
> Jeff Janes  wrote:
>
>> On Tue, Nov 3, 2015 at 5:21 PM, Craig Ringer  wrote:
>> > On 3 November 2015 at 23:04, Bill Moran  wrote:
>> >>
>> >> Looking for feedback to see if anyone sees any issues or has any
>> >> suggestions on what I'm doing. The attached patch alters 3 things
>> >> with regard to TOAST behavior:
>> >
>> > COMPRESSION_TEST_SIZE (2) seems useful.
>> >
>> > The other two mostly seem like options nobody's going to know are
>> > there, or know how to sensibly set if they do notice them. What's the
>> > driving reason behind those, the problem you're trying to solve? Why
>> > make them configurable per-table (or at all)?
>>
>> I currently have a table with one column which has a median width of
>> 500 bytes, a 90th percentile of 650 bytes, and makes up 75% of the
>> table's size, and the column is rarely used, while the table itself is
>> frequently seq scanned.  I'd very much like to drive that column out
>> of main and into toast. I think target_tuple_size would let me do
>> that.
>
> That's exactly the use case. As it currently stands, any tuple smaller
> than about 2K will never be toasted. So if you have 1900 bytes of
> highly compressible text that is infrequently queried from the table
> whilst other columns are frequently accessed, there's no way to force
> it to be out of line from the main table, or be compressed.

Ok, so that's the underlying issue to solve. Make smaller tuples
TOASTable, especially when quite compressible.

Shouldn't that be column-level, really?

We have SET STORAGE at the moment. Would some sort of "FORCE" option
to SET STORAGE EXTERNAL meet your needs? It'd potentially force small
data out of line too, but that'd make sense for your rarely accessed
use case.

> The two new configurables allow the DBA to make tradeoff decisions on
> CPU usage vs. storage efficiency. Since the TOAST code attempts to
> process the column that will provide the largest gain first, in your
> described use case you could calculate the size of the other columns,
> and set the target_tuple_size to just a bit larger than that, and
> that large column should get moved into the toast table in most or
> all cases (depending on how predictable the other sizes are)

I'm just concerned that this is another knob that 0.001% of the user
base will know about and use correctly, 1% will use incorrectly based
on some cargo-culted nonsense they pick up somewhere, and the rest
will have no clue exists. We have more than a few of those already.

The way you describe using a GUC here makes it sound like what you
really want is just a column storage option, and that twiddling a GUC
like this is a workaround to try to make one column more aggressively
compressed and moved out of line without affecting the others.

> Compression is a similarly hard-coded value in current versions.
> I feel that allowing the DBA to control how much savings is required
> before incurring the overhead of compression is worthwhile, especially
> when considered on a per-table basis. For example, the compression
> on an archive table could be very aggressive, whereas compression on
> a frequently accessed table might only be justified if it saves a lot
> of space. How much space compression saves is highly dependent on the
> data being stored.

Yes, I can see value in making attempts at compression more (or less)
aggressive. In that regard it's a pity the pluggable compression
support didn't go anywhere really, because the current algorithm is
cpu-cheap at the cost of pretty poor compression ratios.

>> (Per-column control would be even nicer, but I'd take what I can get)
>
> Oddly, I hadn't considered getting as granualar as per-column, but
> now that you've got me thinking about it, it seems like a logical
> step to take.

I think per-column is really where it makes sense, if it's to be done
at all. At least based on the use cases given.

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


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

Re: [HACKERS] [PATCH] postgres_fdw extension support

[Michael Paquier]

On Wed, Nov 4, 2015 at 12:38 PM, Tom Lane wrote:
> I wrote:
>> I left out the proposed regression tests because they fail in "make
>> installcheck" mode, unless you've previously built and installed cube
>> and seg, which seems like an unacceptable requirement to me.  I don't
>> think that leaving the code untested is a good final answer, of course.
>> The idea I was toying with was to create a dummy extension for testing
>> purposes by means of doing a direct INSERT into pg_extension --- which
>> is ugly and would only work for superusers, but the latter is true of
>> "CREATE EXTENSION cube" too.  Anybody have a better idea?
>
> I had a possibly better idea: instead of manufacturing an empty extension
> with a direct INSERT, hack on the one extension that we know for sure
> will be installed, namely postgres_fdw itself.  So we could do, eg,
>
> create function foo() ...
> alter extension postgres_fdw add function foo();
> and then test shippability of foo() with or without having listed
> postgres_fdw as a shippable extension.

Yeah, I don't have a better idea than that. Could we consider shipping
that in a different library than postgres_fdw.so, like
postgres_fdw_test.so? That's still strange to have a dummy object in
postgres_fdw.so just for testing purposes.
-- 
Michael


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

[HACKERS] Trivial heap_finish_speculative() error message inaccuracy

[Peter Geoghegan]

While auditing the access method code associated with ON CONFLICT DO
UPDATE's speculative insertion infrastructure, I noticed an
inaccuracy.

Attached patch fixes the trivial inaccuracy in a defensive elog()
call. Quite simply, this call site didn't get the memo when we renamed
that function during the late stages of development.

-- 
Peter Geoghegan
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 35a2b05..72611fb 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -5809,7 +5809,7 @@ heap_finish_speculative(Relation relation, HeapTuple tuple)
 		lp = PageGetItemId(page, offnum);
 
 	if (PageGetMaxOffsetNumber(page) < offnum || !ItemIdIsNormal(lp))
-		elog(ERROR, "heap_confirm_insert: invalid lp");
+		elog(ERROR, "heap_finish_speculative: invalid lp");
 
 	htup = (HeapTupleHeader) PageGetItem(page, lp);
 

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

[HACKERS] Valgrind and shared_buffers (Was: Restore-reliability mode)

[Peter Geoghegan]

On Mon, Jul 27, 2015 at 7:12 AM, Alvaro Herrera
 wrote:
> I only tried a few tests, for lack of time, and it didn't produce any.
> (To verify that the whole thing was working properly, I reduced the
> range of memory made available during PinBuffer and that resulted in a
> crash immediately).  I am not really familiar with valgrind TBH and just
> copied a recipe to run postmaster under it, so if someone with more
> valgrind-fu could verify this, it would be great.
>
>
> This part:
>
>> > > > Under CLOBBER_FREED_MEMORY, wipe a shared buffer when its
>> > > > global pin count falls to zero.
>
> can be done without any valgrind, I think.  Any takers?

It seems like this didn't go anywhere. Any chance that you'll find the
time to work on this soon?

-- 
Peter Geoghegan


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

Re: [HACKERS] COPY FREEZE and PD_ALL_VISIBLE

[Amit Kapila]

On Tue, Nov 3, 2015 at 8:07 PM, Simon Riggs  wrote:
>
> On 3 November 2015 at 15:23, Amit Kapila  wrote:
>>
>> On Fri, Oct 23, 2015 at 6:29 AM, Simon Riggs 
wrote:
>>>
>>> Easy enough to do it at the end of the COPY FREEZE in one step.
>>
>>
>> Here, we might want to consider that setting bit in visibility map
>> will generate WAL log whereas Copy Freeze otherwise skip WAL
>> when wal_level is less than archive.  This can lead to extra disk
>> writes which can slow down Copy Freeze, but OTOH that might
>> be acceptable.
>
>
> I'm building the map as I go, in the latest version of this patch I'm
working on.
>

As, you are working on this patch, I have marked it as "Waiting on Author".

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

Re: [HACKERS] Freeze avoidance of very large table.

[Amit Kapila]

On Wed, Nov 4, 2015 at 4:45 AM, Masahiko Sawada 
wrote:
>
> On Tue, Nov 3, 2015 at 12:33 PM, Amit Kapila 
wrote:
> > On Tue, Nov 3, 2015 at 5:04 AM, Robert Haas 
wrote:
> >>
> >> On Sat, Oct 31, 2015 at 1:32 AM, Amit Kapila 
> >> wrote:
> >> >
> >> > What is your main worry about changing the name of this map, is it
> >> > about more code churn or is it about that we might introduce new
issues
> >> > or is it about that people are already accustomed to call this map as
> >> > visibility map?
> >>
> >> My concern is mostly that I think calling it the "visibility and
> >> freeze map" is excessively long and wordy.
> >>
> >> One observation that someone made previously is that there is a
> >> difference between "all-visible" and "index-only scan OK".  An
> >> all-visible page that has a HOT update is no longer all-visible (it
> >> needs vacuuming) but an index-only scan would still be OK (because
> >> only the non-indexed values in the tuple have changed, and every scan
> >> scan can see either the old or the new tuple but not both.  At
> >> present, the index-only scan will consult the heap page anyway,
> >> because all we know is that the page is not all-visible.  But maybe in
> >> the future somebody will decide to add a bit for that.  Then we'd have
> >> the "visibility, usable for index-only scans, and freeze map", but I
> >> think "_vufiosfm" will not be a good choice for a file suffix.
> >>
> >
> > I think in that case we can call it as page info map or page state map,
but
> > I find retaining visibility map name in this case or for future (if we
want
> > to
> > add another bit) as confusing.  In-fact if you find "visibility and
freeze
> > map",
> > as excessively long, then we can change it to "page info map" or "page
state
> > map" now as well.
> >
>
> In that case, file suffix would be "_pim" or "_psm"?

Right.

> IMO, "page info map" would be better, because the bit doesn't indicate
> the status of page in real time, it's just additional information.
> Also we need to rewrite to new name in source code, and source file
> name as well.
>

I think so.  Here I think the right thing to do is lets proceed with fixing
other issues of patch and work on this part later and in the mean time
we might get more feedback on this part of proposal.

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

Re: [HACKERS] [PATCH] postgres_fdw extension support

[Tom Lane]

Michael Paquier  writes:
> On Wed, Nov 4, 2015 at 12:38 PM, Tom Lane wrote:
>> I had a possibly better idea: instead of manufacturing an empty extension
>> with a direct INSERT, hack on the one extension that we know for sure
>> will be installed, namely postgres_fdw itself.  So we could do, eg,
>> 
>> create function foo() ...
>> alter extension postgres_fdw add function foo();
>> and then test shippability of foo() with or without having listed
>> postgres_fdw as a shippable extension.

> Yeah, I don't have a better idea than that. Could we consider shipping
> that in a different library than postgres_fdw.so, like
> postgres_fdw_test.so?

I'm envisioning the extra function(s) as just being SQL functions, so
they don't need any particular infrastructure.

> That's still strange to have a dummy object in
> postgres_fdw.so just for testing purposes.

We could drop the extra functions at the end of the test, but I don't
see the point exactly.  We'd just be leaving the regression test database
with some odd contents of the extension --- there's not any wider effect
than that.

regards, tom lane


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

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

[Catalin Iacob]

Sorry, you're right, I didn't notice the x = plpy.SPIError() test.

I did notice that you included the kw != NULL, I was explaining why it
really is needed even though it *seems* the code also works without
it.

There's just the doc part left then.


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

Re: [HACKERS] RFC/WIP: adding new configuration options to TOAST

[Jim Nasby]

On 11/3/15 8:34 PM, Jeff Janes wrote:

I currently have a table with one column which has a median width of
500 bytes, a 90th percentile of 650 bytes, and makes up 75% of the
table's size, and the column is rarely used, while the table itself is
frequently seq scanned.  I'd very much like to drive that column out
of main and into toast. I think target_tuple_size would let me do
that.


+1 on having a way to induce that behavior, as I've faced the same thing 
in the past.

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com


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

Re: [HACKERS] Trivial heap_finish_speculative() error message inaccuracy

[Peter Geoghegan]

On Tue, Nov 3, 2015 at 7:10 PM, Tom Lane  wrote:
> This seems like a fine teaching moment in which to point out our
> longstanding error message style guideline that says not to put
> names of C functions into error messages in the first place.

I don't ordinarily do that, of course, but I thought it was important
to be consistent with other such elog() calls within heapam.c. I think
that there at least 10 that look like this.

-- 
Peter Geoghegan


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

Re: [HACKERS] ParallelContexts can get confused about which worker is which

[Amit Kapila]

On Tue, Nov 3, 2015 at 8:25 PM, Robert Haas  wrote:
>
> On Mon, Nov 2, 2015 at 4:13 PM, Robert Haas  wrote:
> > On Sun, Nov 1, 2015 at 1:11 AM, Amit Kapila 
wrote:
> >> If we are going to add a new parameter to BackgroundWorker structure,
> >> then the same needs to be updated in docs [1] as well.
> >
> > Right, good point.
>
> Here's an updated patch with documentation added and two bugs fixed.
>

Patch looks good to me.


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

Re: [HACKERS] Request: pg_cancel_backend variant that handles 'idle in transaction' sessions

[Amit Kapila]

On Tue, Nov 3, 2015 at 7:53 PM, Merlin Moncure  wrote:
>
> On Mon, Nov 2, 2015 at 8:42 PM, Amit Kapila 
wrote:
> >
> > What exactly do we want to define session_idle_timeout?  Some
> > possibilities:
> > a. Reset the session related variables like transaction, prepared
> > statements, etc. and retain it for connection pool kind of stuff
> > b. Exit from the session
> >
> > If we want something on lines of option (a), then I think it is better
> > to have just a single time out (session_idle_timeout/idle_timeout)
>
> I'm not thrilled about the prefix 'session_': most .conf variables
> apply to the session (like statement_timeout) and we don't use the
> session prefix for any of those.
>
> "transaction_idle_timeout" is ok, if you want the timeout to apply as
> an expiration for a transaction going idle.
>
> "idle_timeout" doesn't make much sense to me.  It's the responsibility
> of the pooler to mange idle-but-not-in-transaction sessions and we
> already have machinery to support that (DISCARD).
>

I think if transaction is idle for long time, then the chances that someone
will use that session is less, so idle_timeout seems to me the right tool
for such sessions.  I have checked that databases like Oracle also has
such a variable to help out users for such situations.


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

Re: [HACKERS] September 2015 Commitfest

[Michael Paquier]

On Wed, Nov 4, 2015 at 2:38 AM, Robert Haas  wrote:
> On Tue, Nov 3, 2015 at 8:12 AM, Michael Paquier
>  wrote:
>> On Mon, Nov 2, 2015 at 9:35 PM, Michael Paquier wrote:
>>> And now CF begins officially. The axe has fallen as promised 26 hours after.
>>
>> Seeing no volunteers around, I can take the CFM hat for November's CF.
>> Any objections/complaints/remarks?
>
> I think that's great.  What, in your opinion, would be the most
> helpful thing for me to do to move things along?

Hm, I does not seem to me that you are the best fit for the patches
currently stated as "Ready for committer" as you did not much
participate in the threads related to them, except perhaps "SQL
function to report log message". I would think that Heikki is the one
who should take care of wrapping the patch for libpq and OOM, and the
stuff of pg_rewind. Andres would be suited for the bgwriter for
XLOG_RUNNING_XACTS. Finally the in-core regression test suite is a bit
more general... Perhaps Noah who worked on improving things in this
area in light of CVE-2014-0067, but I won't force any committer on it
either, I can understand that we're all busy.

So at this stage, I guess the main issue is to reduce the stack of
patches waiting for reviews with some first-level reviews. But this
applies to everybody here.
-- 
Michael


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

Re: [HACKERS] Re: BUG #13685: Archiving while idle every archive_timeout with wal_level hot_standby

[Michael Paquier]

On Wed, Nov 4, 2015 at 12:43 AM, Andres Freund  wrote:
> On 2015-11-03 10:23:35 -0500, Robert Haas wrote:
>> On Mon, Nov 2, 2015 at 12:58 AM, Jeff Janes  wrote:
>> > If a transaction holding locks aborts on an otherwise idle server, perhaps 
>> > it will take a very long time for a log-shipping standby to realize this.  
>> > But I have hard time believing that anyone who cares about that would be 
>> > using log-shipping (rather than streaming) anyway.
>>
>> I'm sure other people here understand this better than me, but I
>> wonder if it wouldn't make more sense to somehow log this data only if
>> something material has changed in the data being logged.
>
> Phew. That doesn't seem easy to measure. I'm doubtful that it's worth
> comparing the snapshot and such, especially in the back
> branches.
>
> We could maybe add something that we only log a snapshot if XXX
> megabytes have been logged or something. But I don't know which number
> to pick here - and if there's other write activity the price of a
> snapshot record really isn't high.

My first guess on the matter is that we would like to have an extra
condition that depends on max_wal_size with at least a minimum number
of segments generated since the last standby snapshot, perhaps
max_wal_size / 16, but this coefficient is clearly a rule of thumb.
With the default configuration of 1GB, that would be waiting for 4
segments to be generated before logging in a standby snapshot.
-- 
Michael


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

Re: [HACKERS] extend pgbench expressions with functions

[Kyotaro HORIGUCHI]

Hello, sorry for the silence.

At Fri, 18 Sep 2015 20:35:48 +0200 (CEST), Fabien COELHO  
wrote in 
> > -1.  double is an inexact type, whereas integer is an exact type.
> 
> Sure. I already argue on that very line.

Agreed.

> > The typical way to handle this sort of thing is to define a struct
> > whose first member is a type field and whose second field is a union
> > of all the types you need to care about.
> 
> Yep.
> 
> > Then that gets passed around everywhere.  This patch should be
> > designed in such a way that if we eventually end up with functions
> > that have 10 different return types instead of 2 different return
> > types, we don't need to add 8 more parameters to any functions.
> > Instead, those still return PgBench_Value (or whatever we call it)
> > which is the aforementioned struct, but there are more options for
> > what that can contain.
> 
> I just put the double type as a proof of concept, but for pgbench only
> integers really matters.
> 
> What you suggest would work, but it would also result in ugly and
> lengthy code, as I argued in another mail, because you have to decide
> for overloaded operators and functions which actual typed operator
> must be called, and then perform the necessary type conversions
> depending on the actual type of the operands. The implicit descendent
> typing used in the patch hides this, and is more than enough for
> pgbench, IMO.

I also agree this.

> If this is a blocker, I would rather remove the support for doubles
> than write verbose and inelegant code.

I understood the situation and agreed for current shape of the
code. I no longer object the calling-alternatively code. But I'd
like see the abbreviated discussion in the comment on the
function.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



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

Re: [HACKERS] Re: BUG #13685: Archiving while idle every archive_timeout with wal_level hot_standby

[Michael Paquier]

On Wed, Nov 4, 2015 at 8:39 AM, Andres Freund  wrote:
> On November 4, 2015 12:37:02 AM GMT+01:00, Michael Paquier wrote:
>>On a completely idle system, I don't think we should log any standby
>>records. This is what ~9.3 does.
>
> Are you sure? I think it'll around checkpoints, no? I thought Heikki had 
> fixed that, but looking sound that doesn't seem to be the case.

Er, yes, sorry. I should have used clearer words: I meant idle system
with something running nothing including internal checkpoints. An
instance indeed generates a XLOG_RUNNING_XACTS record before a
checkpoint record on an idle system.
-- 
Michael


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

Re: [HACKERS] ATT_FOREIGN_TABLE and ATWrongRelkindError()

[Etsuro Fujita]

On 2015/10/28 20:10, Robert Haas wrote:

On Fri, Oct 23, 2015 at 11:51 AM, Etsuro Fujita
 wrote:

BTW, I found an incorrect error message in ATWrongRelkindError. Attached is
a patch for fixing the message.



Committed and back-patched to 9.3.


Thanks!

Best regards,
Etsuro Fujita



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

Re: [HACKERS] Foreign join pushdown vs EvalPlanQual

[Etsuro Fujita]

On 2015/10/28 6:04, Robert Haas wrote:

On Tue, Oct 20, 2015 at 12:39 PM, Etsuro Fujita
 wrote:

Sorry, my explanation was not correct.  (Needed to take in caffeine.) What
I'm concerned about is the following:

SELECT * FROM localtab JOIN (ft1 LEFT JOIN ft2 ON ft1.x = ft2.x) ON
localtab.id = ft1.id FOR UPDATE OF ft1

LockRows
-> Nested Loop
  Join Filter: (localtab.id = ft1.id)
  -> Seq Scan on localtab
  -> Foreign Scan on 
   Remote SQL: SELECT * FROM ft1 LEFT JOIN ft2 WHERE ft1.x = ft2.x
FOR UPDATE OF ft1

Assume that ft1 performs late row locking.



If the SQL includes "FOR UPDATE of ft1", then it clearly performs
early row locking.  I assume you meant to omit that.


Right.  Sorry for my mistake.


If an EPQ recheck was invoked
due to a concurrent transaction on the remote server that changed only the
value x of the ft1 tuple previously retrieved, then we would have to
generate a fake ft1/ft2-join tuple with nulls for ft2. (Assume that the ft2
tuple previously retrieved was not a null tuple.) However, I'm not sure how
we can do that in ForeignRecheck; we can't know for example, which one is
outer and which one is inner, without an alternative local join execution
plan.  Maybe I'm missing something, though.



I would expect it to issue a new query like: SELECT * FROM ft1 LEFT
JOIN ft2 WHERE ft1.x = ft2.x AND ft1.tid = $0 AND ft2.tid = $1.


We assume here that ft1 uses late row locking, so I thought the above 
SQL should include "FOR UPDATE of ft1".  But I still don't think that 
that is right; the SQL with "FOR UPDATE of ft1" wouldn't generate the 
fake ft1/ft2-join tuple with nulls for ft2, as expected.  The reason for 
that is that the updated version of the ft1 tuple wouldn't satisfy the 
ft1.tid = $0 condition in an EPQ recheck, because the ctid for the 
updated version of the ft1 tuple has changed.  (IIUC, I think that if we 
use a TID scan for ft1, the SQL would generate the expected result, 
because I think that the TID condition would be ignored in the EPQ 
recheck, but I don't think it's guaranteed to use a TID scan for ft1.) 
Maybe I'm missing something, though.



This should be significantly more efficient than fetching the base
rows from each of two tables with two separate queries.


Maybe I think we could fix the SQL, so I have to admit that, but I'm 
just wondering (1) what would happen for the case when ft1 uses late row 
rocking and ft2 uses early row rocking and (2) that would be still more 
efficient than re-fetching only the base row from ft1.


What I thought to improve the efficiency in the secondary-plan approach 
that I proposed was that if we could parallelize re-fetching foreign 
rows in ExecLockRows and EvalPlanQualFetchRowMarks, we would be able to 
improve the efficiency not only for the case when performing a join of 
foreign tables remotely but for the case when performing the join locally.


Best regards,
Etsuro Fujita



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

Re: [HACKERS] COPY FREEZE and PD_ALL_VISIBLE

[Simon Riggs]

On 3 November 2015 at 15:23, Amit Kapila  wrote:

> On Fri, Oct 23, 2015 at 6:29 AM, Simon Riggs 
> wrote:
>
>> On 21 October 2015 at 13:31, Jeff Janes  wrote:
>>
>> Index-only scans will visit the heap for each tuple until the first
>>> VACUUM is done.
>>>
>>> The first vacuum will read the entire table, but not need to write it
>>> anymore.  And will create the _vm file.
>>>
>>> I think we really want to create _vm file as well as set PD_ALL_VISIBLE,
>>> but I don't know the best way to do that.  Set a flag somewhere and then
>>> create it in bulk at the end of the transaction?  Set it bit by bit as the
>>> pages are extended and initialized?
>>>
>>
>> Easy enough to do it at the end of the COPY FREEZE in one step.
>>
>
> Here, we might want to consider that setting bit in visibility map
> will generate WAL log whereas Copy Freeze otherwise skip WAL
> when wal_level is less than archive.  This can lead to extra disk
> writes which can slow down Copy Freeze, but OTOH that might
> be acceptable.
>

I'm building the map as I go, in the latest version of this patch I'm
working on.

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

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

Re: [HACKERS] Request: pg_cancel_backend variant that handles 'idle in transaction' sessions

[Merlin Moncure]

On Mon, Nov 2, 2015 at 1:23 PM, Jim Nasby  wrote:
> On 11/2/15 11:15 AM, Pavel Stehule wrote:
>>
>> I have not strong idea about how to solve it well - maybe introduce
>> transaction_idle_timeout and session_idle_timeout?
>
>
> Yes, please. This is a very common problem. I would love a better way to
> detect (or prevent) clients from being brain-dead about how they're using
> transactions, but short of that this is the next best thing.
>
> Actually, one other thing that would help is to have the ability to turn
> this into an ERROR:
>
> begin;
> WARNING:  there is already a transaction in progress

curious: does the SQL standard define this behavior?

Anyways, we've pretty studiously avoided (minus a couple of
anachronisms) .conf setting thats control behavior of SQL commands in
a non performance way.

IMO, this as yet another case for 'stored procedures' that can manage
transaction state: you could rig up your own procedure: CALL
begin_tx_safe(); which would test transaction state and fail if
already in one.  This doesn't help you if you're not in direct control
of application generated SQL but it's a start.  Barring that, at least
warnings tend to stand out in the database log.

merlin


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

Re: [HACKERS] ParallelContexts can get confused about which worker is which

[Robert Haas]

On Mon, Nov 2, 2015 at 4:13 PM, Robert Haas  wrote:
> On Sun, Nov 1, 2015 at 1:11 AM, Amit Kapila  wrote:
>> If we are going to add a new parameter to BackgroundWorker structure,
>> then the same needs to be updated in docs [1] as well.
>
> Right, good point.

Here's an updated patch with documentation added and two bugs fixed.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/doc/src/sgml/bgworker.sgml b/doc/src/sgml/bgworker.sgml
index c17d398..505e362 100644
--- a/doc/src/sgml/bgworker.sgml
+++ b/doc/src/sgml/bgworker.sgml
@@ -58,6 +58,7 @@ typedef struct BackgroundWorker
 charbgw_library_name[BGW_MAXLEN];   /* only if bgw_main is NULL */
 charbgw_function_name[BGW_MAXLEN];  /* only if bgw_main is NULL */
 Datum   bgw_main_arg;
+charbgw_extra[BGW_EXTRALEN];
 int bgw_notify_pid;
 } BackgroundWorker;
 
@@ -183,6 +184,13 @@ typedef struct BackgroundWorker
   
 
   
+   bgw_extra can contain extra data to be passed
+   to the background worker.  Unlike bgw_main_arg, this data
+   is not passed as an argument to the worker's main function, but it can be
+   accessed via MyBgworkerEntry, as discussed above.
+  
+
+  
bgw_notify_pid is the PID of a PostgreSQL
backend process to which the postmaster should send SIGUSR1
when the process is started or exits.  It should be 0 for workers registered
diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c
index 7cb53f2..ab00279 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -77,10 +77,6 @@ typedef struct FixedParallelState
 	/* Mutex protects remaining fields. */
 	slock_t		mutex;
 
-	/* Track whether workers have attached. */
-	int			workers_expected;
-	int			workers_attached;
-
 	/* Maximum XactLastRecEnd of any worker. */
 	XLogRecPtr	last_xlog_end;
 } FixedParallelState;
@@ -295,8 +291,6 @@ InitializeParallelDSM(ParallelContext *pcxt)
 	fps->parallel_master_backend_id = MyBackendId;
 	fps->entrypoint = pcxt->entrypoint;
 	SpinLockInit(>mutex);
-	fps->workers_expected = pcxt->nworkers;
-	fps->workers_attached = 0;
 	fps->last_xlog_end = 0;
 	shm_toc_insert(pcxt->toc, PARALLEL_KEY_FIXED, fps);
 
@@ -403,7 +397,6 @@ ReinitializeParallelDSM(ParallelContext *pcxt)
 
 	/* Reset a few bits of fixed parallel state to a clean state. */
 	fps = shm_toc_lookup(pcxt->toc, PARALLEL_KEY_FIXED);
-	fps->workers_attached = 0;
 	fps->last_xlog_end = 0;
 
 	/* Recreate error queues. */
@@ -455,6 +448,7 @@ LaunchParallelWorkers(ParallelContext *pcxt)
 	worker.bgw_main = ParallelWorkerMain;
 	worker.bgw_main_arg = UInt32GetDatum(dsm_segment_handle(pcxt->seg));
 	worker.bgw_notify_pid = MyProcPid;
+	memset(_extra, 0, BGW_EXTRALEN);
 
 	/*
 	 * Start workers.
@@ -466,6 +460,7 @@ LaunchParallelWorkers(ParallelContext *pcxt)
 	 */
 	for (i = 0; i < pcxt->nworkers; ++i)
 	{
+		memcpy(worker.bgw_extra, , sizeof(int));
 		if (!any_registrations_failed &&
 			RegisterDynamicBackgroundWorker(,
 			>worker[i].bgwhandle))
@@ -893,6 +888,10 @@ ParallelWorkerMain(Datum main_arg)
 	pqsignal(SIGTERM, die);
 	BackgroundWorkerUnblockSignals();
 
+	/* Determine and set our parallel worker number. */
+	Assert(ParallelWorkerNumber == -1);
+	memcpy(, MyBgworkerEntry->bgw_extra, sizeof(int));
+
 	/* Set up a memory context and resource owner. */
 	Assert(CurrentResourceOwner == NULL);
 	CurrentResourceOwner = ResourceOwnerCreate(NULL, "parallel toplevel");
@@ -917,18 +916,9 @@ ParallelWorkerMain(Datum main_arg)
 (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
 			   errmsg("bad magic number in dynamic shared memory segment")));
 
-	/* Determine and set our worker number. */
+	/* Look up fixed parallel state. */
 	fps = shm_toc_lookup(toc, PARALLEL_KEY_FIXED);
 	Assert(fps != NULL);
-	Assert(ParallelWorkerNumber == -1);
-	SpinLockAcquire(>mutex);
-	if (fps->workers_attached < fps->workers_expected)
-		ParallelWorkerNumber = fps->workers_attached++;
-	SpinLockRelease(>mutex);
-	if (ParallelWorkerNumber < 0)
-		ereport(ERROR,
-(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
- errmsg("too many parallel workers already attached")));
 	MyFixedParallelState = fps;
 
 	/*
diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index cf505d6..2a878eb 100644
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -314,6 +314,7 @@ BackgroundWorkerStateChange(void)
 		rw->rw_worker.bgw_restart_time = slot->worker.bgw_restart_time;
 		rw->rw_worker.bgw_main = slot->worker.bgw_main;
 		rw->rw_worker.bgw_main_arg = slot->worker.bgw_main_arg;
+		memcpy(rw->rw_worker.bgw_extra, slot->worker.bgw_extra, BGW_EXTRALEN);
 
 		/*
 		 * Copy the PID to be notified about state changes, but only if the
diff --git a/src/include/postmaster/bgworker.h b/src/include/postmaster/bgworker.h
index 

Re: [HACKERS] shm_mq fix for non-blocking mode

[Robert Haas]

On Thu, Oct 22, 2015 at 10:00 PM, Robert Haas  wrote:
>> ...and so I've committed it and back-patched to 9.4.
>
> Sigh.  This was buggy; I have no idea how it survived my earlier testing.
>
> I will go fix it.  Sorry.

Gah!  That, too, turned out to be buggy, although in a considerably
more subtle way.  I've pushed another fix with a detailed comment and
an explanatory commit message that hopefully squashes this problem for
good.  Combined with the fix at
http://www.postgresql.org/message-id/ca+tgmozzv3u9trsvcao+-otxbsz_u+a5q8x-_b+vzcehhtz...@mail.gmail.com
this seems to squash occasional complaints about workers "dying
unexpectedly" when they really had done no such thing.

The test code I used to find these problems is attached.  I compiled
and installed the parallel_dummy extension, did pgbench -i -s 100, and
then ran this:

while psql -c "select parallel_count('pgbench_accounts', 4)"; do sleep 1; done

Without these fixes, this can hang or error out, but with these fixes,
it works fine.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
commit 84fa9b17b8427721af23e38094a6cbfec8c8bb00
Author: Robert Haas 
Date:   Thu Oct 15 19:21:38 2015 -0400

parallel_dummy

diff --git a/contrib/parallel_dummy/Makefile b/contrib/parallel_dummy/Makefile
new file mode 100644
index 000..de00f50
--- /dev/null
+++ b/contrib/parallel_dummy/Makefile
@@ -0,0 +1,19 @@
+MODULE_big = parallel_dummy
+OBJS = parallel_dummy.o $(WIN32RES)
+PGFILEDESC = "parallel_dummy - dummy use of parallel infrastructure"
+
+EXTENSION = parallel_dummy
+DATA = parallel_dummy--1.0.sql
+
+REGRESS = parallel_dummy
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = contrib/parallel_dummy
+top_builddir = ../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/contrib/parallel_dummy/parallel_dummy--1.0.sql b/contrib/parallel_dummy/parallel_dummy--1.0.sql
new file mode 100644
index 000..d49bd0f
--- /dev/null
+++ b/contrib/parallel_dummy/parallel_dummy--1.0.sql
@@ -0,0 +1,7 @@
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION parallel_dummy" to load this file. \quit
+
+CREATE FUNCTION parallel_count(rel pg_catalog.regclass,
+			  nworkers pg_catalog.int4)
+RETURNS pg_catalog.int8 STRICT
+	AS 'MODULE_PATHNAME' LANGUAGE C;
diff --git a/contrib/parallel_dummy/parallel_dummy.c b/contrib/parallel_dummy/parallel_dummy.c
new file mode 100644
index 000..f63b63e
--- /dev/null
+++ b/contrib/parallel_dummy/parallel_dummy.c
@@ -0,0 +1,152 @@
+/*--
+ *
+ * parallel_dummy.c
+ *		Test harness code for parallel mode code.
+ *
+ * Copyright (C) 2013-2014, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ *		contrib/parallel_dummy/parallel_dummy.c
+ *
+ * -
+ */
+
+#include "postgres.h"
+
+#include "access/heapam.h"
+#include "access/parallel.h"
+#include "access/relscan.h"
+#include "access/xact.h"
+#include "fmgr.h"
+#include "miscadmin.h"
+#include "storage/spin.h"
+#include "utils/builtins.h"
+#include "utils/guc.h"
+#include "utils/snapmgr.h"
+
+PG_MODULE_MAGIC;
+
+PG_FUNCTION_INFO_V1(parallel_count);
+
+#define		TOC_SCAN_KEY			1
+#define		TOC_RESULT_KEY			2
+
+void		_PG_init(void);
+void		count_worker_main(dsm_segment *seg, shm_toc *toc);
+
+static void count_helper(HeapScanDesc pscan, int64 *resultp);
+
+Datum
+parallel_count(PG_FUNCTION_ARGS)
+{
+	Oid			relid = PG_GETARG_OID(0);
+	int32		nworkers = PG_GETARG_INT32(1);
+	int64	   *resultp;
+	int64		result;
+	ParallelContext *pcxt;
+	ParallelHeapScanDesc pscan;
+	HeapScanDesc	scan;
+	Relation	rel;
+	Size		sz;
+
+	if (nworkers < 0)
+		ereport(ERROR,
+(errmsg("number of parallel workers must be non-negative")));
+
+	rel = relation_open(relid, AccessShareLock);
+
+	EnterParallelMode();
+
+	pcxt = CreateParallelContextForExternalFunction("parallel_dummy",
+			 "count_worker_main",
+			 nworkers);
+	sz = heap_parallelscan_estimate(GetActiveSnapshot());
+	shm_toc_estimate_chunk(>estimator, sz);
+	shm_toc_estimate_chunk(>estimator, sizeof(int64));
+	shm_toc_estimate_keys(>estimator, 2);
+	InitializeParallelDSM(pcxt);
+	pscan = shm_toc_allocate(pcxt->toc, sz);
+	heap_parallelscan_initialize(pscan, rel, GetActiveSnapshot());
+	shm_toc_insert(pcxt->toc, TOC_SCAN_KEY, pscan);
+	resultp = shm_toc_allocate(pcxt->toc, sizeof(int64));
+	shm_toc_insert(pcxt->toc, TOC_RESULT_KEY, resultp);
+
+	LaunchParallelWorkers(pcxt);
+
+	scan = heap_beginscan_parallel(rel, pscan);
+
+	/* here's where we do the "real work" ... */
+	count_helper(scan, resultp);
+
+	WaitForParallelWorkersToFinish(pcxt);
+
+	heap_rescan(scan, NULL);
+	*resultp = 0;
+
+	ReinitializeParallelDSM(pcxt);
+	

Re: [HACKERS] fortnight interval support

[Alvaro Herrera]

Michael Paquier wrote:
> On Wed, Oct 28, 2015 at 4:17 AM, Gavin Flower wrote:
> > You trying to get PostgreSQL banned in France???  :-)
> >
> > When I was learning French many years ago, I was told that the French
> > consider their fortnight to be 15 days!!!
> 
> Confirmed. I would translate fornight as 'quinzaine' to French.
> (please let's not add that btw).

I don't know what's the origin of the word "fortnight", in particular
whether it's always supposed to be exactly fourteen days, but the
equivalent to quizaine in Spanish ("quincena") is ambiguous enough that
I would have trouble making it part of intervals in Postgres.  It is
sometimes interpreted as "two weeks", other times as exactly fifteen
days, other times as "half a month".

(WRT the reference to Jane Austen and "Se'ennight" for "week", it occurs
to me that fortnight is a similar contraction for "forteen night".)

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


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

Re: [HACKERS] Minor clarifying changes to abbreviated key abort code comments

[Robert Haas]

On Sat, Oct 31, 2015 at 3:42 PM, Peter Geoghegan  wrote:
> Attached are a couple of patches that only change code comments. The
> first (abort abbreviation) patch is recommended for backpatch to 9.5.
> The second is a tiny tweak.

This comment doesn't make sense to me:

+* (TSS_BUILDRUNS state prevents control reaching here in any
+* case).

Unless I'm missing something, that's not actually true.

I've committed 0002.

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


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

Re: [HACKERS] eXtensible Transaction Manager API

[Simon Riggs]

On 2 November 2015 at 13:24, Konstantin Knizhnik 
wrote:

> PostgreSQL assumes that top-level xid commit is atomic, along with all of
> its subtransactions. So the API having access to only Get/Set at the xid
> level would not work. We would need TransactionIdSetTreeStatus() rather
> than just SetStatus. GetStatus is not atomic.
>
>
> XTM API has function SetTransactionStatus but it actually encapsulate
> TransactionIdSetTreeStatus.
>

OK, thanks.


> At present, I can't tell whether you need subtrans and multixact calls in
> the API as well. I would imagine we probably do, though we might imagine an
> implementation that didn't support those concepts.
>
>
> Postgres-XL doesn't support subtransactions. Neither did we at this moment.
> Support of subtransactions will be our next step.
>

OK. I guess it depends where we want to put the patch; discussion here
means in-core, so for it to go in here, we do need subxacts etc.

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

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

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

[Catalin Iacob]

On Tue, Nov 3, 2015 at 12:49 PM, Pavel Stehule  wrote:
>> 1. in PLy_spi_error__init__ you need to check kw for NULL before doing
>> PyDict_Size(kw) otherwise for plpy.SPIError() you get Bad internal
>> call because PyDict_Size expects a real dictionary not NULL
>
>
> PyDict_Size returns -1 when the dictionary is NULL
>
> http://grokbase.com/t/python/python-dev/042ft6qaqq/pydict-size-error-return

Yes, but it also sets the error indicator to BadInternalCall. In 2.7
the code is:
Py_ssize_t
PyDict_Size(PyObject *mp)
{
if (mp == NULL || !PyDict_Check(mp)) {
PyErr_BadInternalCall();
return -1;
}
return ((PyDictObject *)mp)->ma_used;
}

I had a PLy_elog right after the PyDict_Size call for debugging and
that one was raising BadInternalCall since it checked the error
indicator. So the NULL check is needed.

>> 2. a test with just plpy.SPIError() is still missing, this would have
>> caught 1.
>
>
> please, can you write some example - I am not able raise described error

The example was plpy.SPIError() but I now realize that, in order to
see a failure, you need the extra PLy_elog which I had in there.
But this basic form of the constructor is still an important thing to
test so please add this as well to the regression test.

>> 5. there is conceptual code duplication between PLy_spi_exception_set
>> in plpy_spi.c, since that code also constructs an SPIError but from C
>> and with more info available (edata->internalquery and
>> edata->internalpos). But making a tuple and assigning it to spidata is
>> in both. Not sure how this can be improved.
>
>
> I see it, but I don't think, so current code should be changed.
> PLy_spi_exception_set is controlled directly by fully filled ErrorData
> structure, __init__ is based only on keyword parameters with possibility use
> inherited data. If I'll build ErrorData in __init__ function and call
> PLy_spi_exception_set, then the code will be longer and more complex.

Indeed, I don't really see how to improve this but it does bug me a bit.

One more thing,
+The plpy module provides several possibilities to
+to raise a exception:

This has "to" 2 times and is weird since it says it offers several
possibilities but then shows only one (the SPIError constructor).
And SPIError should be plpy.SPIError everywhere to
be consistent.

Maybe something like (needs markup):
A plpy.SPIError can be raised from PL/Python, the constructor accepts
keyword parameters:
  plpy.SPIError([ message [, detail [, hint [, sqlstate [, schema [,
table [, column [, datatype [, constraint ])
then the example

If you fix the doc and add the plpy.SPIError() test I'm happy. I'll
give it one more test on Python2.7 and 3.5 and mark it Ready for
Committer.


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

Re: [HACKERS] COPY FREEZE and PD_ALL_VISIBLE

[Amit Kapila]

On Fri, Oct 23, 2015 at 6:29 AM, Simon Riggs  wrote:

> On 21 October 2015 at 13:31, Jeff Janes  wrote:
>
> Index-only scans will visit the heap for each tuple until the first VACUUM
>> is done.
>>
>> The first vacuum will read the entire table, but not need to write it
>> anymore.  And will create the _vm file.
>>
>> I think we really want to create _vm file as well as set PD_ALL_VISIBLE,
>> but I don't know the best way to do that.  Set a flag somewhere and then
>> create it in bulk at the end of the transaction?  Set it bit by bit as the
>> pages are extended and initialized?
>>
>
> Easy enough to do it at the end of the COPY FREEZE in one step.
>

Here, we might want to consider that setting bit in visibility map
will generate WAL log whereas Copy Freeze otherwise skip WAL
when wal_level is less than archive.  This can lead to extra disk
writes which can slow down Copy Freeze, but OTOH that might
be acceptable.


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

Re: [HACKERS] Request: pg_cancel_backend variant that handles 'idle in transaction' sessions

[Merlin Moncure]

On Mon, Nov 2, 2015 at 8:42 PM, Amit Kapila  wrote:
> On Mon, Nov 2, 2015 at 10:45 PM, Pavel Stehule 
> wrote:
>>
>>
>> It is 100% true. But the users can do strange things. If we solve idle
>> transactions and not idle session, then they are able to increase
>> max_connections to thousands with happy smile in face.
>>
>> I have not strong idea about how to solve it well - maybe introduce
>> transaction_idle_timeout and session_idle_timeout?
>>
>
> What exactly do we want to define session_idle_timeout?  Some
> possibilities:
> a. Reset the session related variables like transaction, prepared
> statements, etc. and retain it for connection pool kind of stuff
> b. Exit from the session
>
> If we want something on lines of option (a), then I think it is better
> to have just a single time out (session_idle_timeout/idle_timeout)

I'm not thrilled about the prefix 'session_': most .conf variables
apply to the session (like statement_timeout) and we don't use the
session prefix for any of those.

"transaction_idle_timeout" is ok, if you want the timeout to apply as
an expiration for a transaction going idle.

"idle_timeout" doesn't make much sense to me.  It's the responsibility
of the pooler to mange idle-but-not-in-transaction sessions and we
already have machinery to support that (DISCARD).

"transaction_timeout" is the best, and simplest, hypothetical setting
IMNSHO.   It gives you a well defined upper bound guarantee of
transaction time regardless of application behavior, which neither
statement_timeout or transaction_idle_timeout give, even when used in
conjunction as I understand them.  It would completely displace
statement_timeout in all servers I manage.

merlin


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

Re: [HACKERS] COPY FREEZE and PD_ALL_VISIBLE

[Amit Kapila]

On Fri, Oct 23, 2015 at 2:46 AM, Robert Haas  wrote:
>
> On Wed, Oct 21, 2015 at 1:31 PM, Jeff Janes  wrote:
> > It turns out it was pretty easy to set PD_ALL_VISIBLE on the new pages,
> > since the code in hio that requests the relation to be extended already
has
> > info on the tuple's intended freeze status.
> >
> > Then you just need to refrain from clearing PD_ALL_VISIBLE when that
tuple
> > is actually written into the page.  Not only because clearing would
defeat
> > the purpose, but also because it will cause an error--apparently the
> > incipient page is not yet in a state where visibilitymap_clear is
willing to
> > deal with it.
>
> Wouldn't it be better to instead fill the page with tuples first and
> THEN set PD_ALL_VISIBLE?
>

Ideally that would be better and if we want to do that way, then I think it
needs to be done at end of Copy Freeze operation, also for that we
need to mark the buffer as dirty again (do you see any other way
of achieving the same?).  OTOH, I don't see any harm even if we do it
in the way as done in patch.


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

Re: [HACKERS] Request: pg_cancel_backend variant that handles 'idle in transaction' sessions

[Pavel Stehule]

2015-11-03 3:42 GMT+01:00 Amit Kapila :

> On Mon, Nov 2, 2015 at 10:45 PM, Pavel Stehule 
> wrote:
>>
>>
>> It is 100% true. But the users can do strange things. If we solve idle
>> transactions and not idle session, then they are able to increase
>> max_connections to thousands with happy smile in face.
>>
>> I have not strong idea about how to solve it well - maybe introduce
>> transaction_idle_timeout and session_idle_timeout?
>>
>>
> What exactly do we want to define session_idle_timeout?  Some
> possibilities:
> a. Reset the session related variables like transaction, prepared
> statements, etc. and retain it for connection pool kind of stuff
> b. Exit from the session
>

b is safe state - and currently it is only one state, that we can forward
to client side (with keep_alive packets) - so I prefer b

Regards

Pavel


>
> If we want something on lines of option (a), then I think it is better
> to have just a single time out (session_idle_timeout/idle_timeout)
>
>
>
> With Regards,
> Amit Kapila.
> EnterpriseDB: http://www.enterprisedb.com
>

[HACKERS] Documentation fix for psql

[Albe Laurenz]

The psql documentation calls the \pset options unicode_*_style
when in reality they are called unicode_*_linestyle.

This should be backpatched to 9.5.

Yours,
Laurenz Albe


0001-Fix-documentation-for-pset-unicode_-_linestyle.patch
Description: 0001-Fix-documentation-for-pset-unicode_-_linestyle.patch

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

Re: [HACKERS] [patch] to build docs on Mac OS X El Capitan with MacPorts

[Robert Haas]

On Sun, Nov 1, 2015 at 8:41 AM, Neil Tiffin  wrote:
> The attached patch was required to get the docs to build on Mac OS X 10.11.1 
> (15B42) with MacPorts 2.3.4.  After changing docbook.m4 ‘autoreconf’ has to 
> be run.  This patch does not include the new version of ‘configure'

Can anyone else verify this?

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


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