Re: [HACKERS] Parallel pg_dump's error reporting doesn't work worth squat

[Kyotaro HORIGUCHI]

> Sounds reasonable. I look into this further.

I looked into that and found one problem in the patch.

> Next, I got the following behavior for the following command,
> then freeze. Maybe stopping at the same point with the next
> paragraph but I'm not sure. The same thing occurs this patch on
> top of the current master but doesn't on Linux.

This occurs in the following steps.

1. One of the workers dies from some reason.
   (InitCompressorZlib immediately goes into exit_horribly in this case)

2. The main thread detects in ListenToWorkers that the worker is dead.

3. ListenToWorkers calls exit_horribly then exit_nicely

4. exit_nicely calls archive_close_connection as a callback then
   the callback calls ShutdownWorkersHard

5. ShutdownWorkersHard should close the write side of the pipe
   but the patch skips it for WIN32.

So, the attached patch on top the patch fixes that, that is,
pg_dump returns to command prompt even for the case.

By the way, the reason of the "invalid snapshot identifier" is
that some worker threads try to use it after the connection on
the first worker closed. Some of the workers succesfully did
before the connection closing and remained listening to their
master to inhibit termination of the entire process.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/bin/pg_dump/parallel.c b/src/bin/pg_dump/parallel.c
index f650d3f..6c08426 100644
--- a/src/bin/pg_dump/parallel.c
+++ b/src/bin/pg_dump/parallel.c
@@ -308,7 +308,6 @@ checkAborting(ArchiveHandle *AH)
 static void
 ShutdownWorkersHard(ParallelState *pstate)
 {
-#ifndef WIN32
 	int			i;
 
 	/*
@@ -318,6 +317,7 @@ ShutdownWorkersHard(ParallelState *pstate)
 	for (i = 0; i < pstate->numWorkers; i++)
 		closesocket(pstate->parallelSlot[i].pipeWrite);
 
+#ifndef WIN32
 	for (i = 0; i < pstate->numWorkers; i++)
 		kill(pstate->parallelSlot[i].pid, SIGTERM);
 #else

-- 
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] statistics for shared catalogs not updated when autovacuum is off

[Noah Misch]

On Wed, May 25, 2016 at 05:50:41PM -0400, Tom Lane wrote:
> I wrote:
> > OK.  I'm going to work on replacing the DBWriteRequest structure with
> > an OID list, and on improving the comments some more, but I don't
> > anticipate further functional changes.
> 
> Here's the end result of that.

You may want to compare your patch to this pending patch for the same bug:
http://www.postgresql.org/message-id/flat/24f09c2d-e5bf-1f73-db54-8255c1280...@2ndquadrant.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] ORDER/GROUP BY expression not found in targetlist

[Peter Geoghegan]

On Wed, May 25, 2016 at 7:12 PM, Andres Freund  wrote:
>
> =# CREATE TABLE twocol(col01 int, col02 int);
> =# SELECT DISTINCT col01, col02, col01 FROM twocol ;
> ERROR:  XX000: ORDER/GROUP BY expression not found in targetlist
> LOCATION:  get_sortgroupref_tle, tlist.c:341
>
> which appears to be a 9.6 regression, presumable fallout from the path
> restructuring.

It's surprising that SQL Smith didn't catch something with such simple
steps to reproduce.


-- 
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] ORDER/GROUP BY expression not found in targetlist

[Andres Freund]

Hi,

trying to reproduce a performance problem I just found:

=# CREATE TABLE twocol(col01 int, col02 int);
=# SELECT DISTINCT col01, col02, col01 FROM twocol ;
ERROR:  XX000: ORDER/GROUP BY expression not found in targetlist
LOCATION:  get_sortgroupref_tle, tlist.c:341

which appears to be a 9.6 regression, presumable fallout from the path
restructuring.

Greetings,

Andres Freund


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

Re: [HACKERS] Parallel pg_dump's error reporting doesn't work worth squat

[Kyotaro HORIGUCHI]

At Wed, 25 May 2016 10:11:28 -0400, Tom Lane  wrote in 
<24577.1464185...@sss.pgh.pa.us>
> Kyotaro HORIGUCHI  writes:
> > I tried it on Windows 7/64 but first of all, I'm surprised to see
> > that the following command line gets an error but it would be
> > fine because it is consistent with what is written in the manual.
> 
> > | >pg_dump postgres://horiguti:hoge@localhost/postgres --jobs=9 -Fd -f 
> > testdump
> > | pg_dump: too many command-line arguments (first is "--jobs=9")
> > | Try "pg_dump --help" for more information.
> 
> Where do you see an example like that?  We should fix it.

Sorry for the confusing sentence. The command line came from your
first mail starting this thread:p And "consistent with manual"
means that the command line results in an error (even only on
Windows) since it is difrerent from the document. No such example
has been seen in the documentation AFAICS.

https://www.postgresql.org/message-id/2458.1450894...@sss.pgh.pa.us
https://www.postgresql.org/docs/9.6/static/app-pgdump.html

>  The only case
> that is certain to work is switches before non-switch arguments, and so
> we should not give any documentation examples in the other order.  On a
> platform using GNU getopt(), getopt() will rearrange the argv[] array to
> make such cases work, but non-GNU getopt() doesn't do that (and I don't
> really trust the GNU version not to get confused, either).

Yeah, I knew it after reading port/getopt_long.c. But the error
message seems saying nothing about that (at least to me). And
those accumstomed to the GNU getopt's behavior will fail to get
the point of the message. The documentation also doesn't
explicitly saying about the restriction; it is only implicitly
shown in synopsis. How about something like the following
message? (The patch attached looks too dependent on the specific
behavior of our getopt_long.c, but doing it in getopt_long.c also
seems to be wrong..)

| >pg_dump hoge -f
| pg_dump: non-option arguments should not precede options.


> > I might be wrong with something, but pg_dump seems to have some
> > issues in thread handling.
> 
> Wouldn't surprise me ... there's a lot of code in there depending on
> static variables, and we've only tried to thread-proof a few.  Still,
> I think that's a separate issue from what this patch is addressing.

Sounds reasonable. I look into this further.
I see no other functional problem in this patch.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 1267afb..52e9094 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -546,9 +546,13 @@ main(int argc, char **argv)
 	/* Complain if any arguments remain */
 	if (optind < argc)
 	{
-		fprintf(stderr, _("%s: too many command-line arguments (first is \"%s\")\n"),
-progname, argv[optind]);
-		fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+		if (optind > 0 && argv[optind - 1][0] != '-')
+			fprintf(stderr, _("%s: non-option arguments should not precede options.\n"),
+	progname);
+		else
+			fprintf(stderr, _("%s: too many command-line arguments (first is \"%s\")\n"),
+	progname, argv[optind]);
+		fprintf(stderr, _("Non-Try \"%s --help\" for more information.\n"),
 progname);
 		exit_nicely(1);
 	}

-- 
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] Deleting prepared statements from libpq.

[Craig Ringer]

On 25 May 2016 at 18:05, Dmitry Igrishin  wrote:

> Hello,
>
> According to
>
> https://www.postgresql.org/docs/current/static/protocol-flow.html#PROTOCOL-FLOW-EXT-QUERY
> there are Close message for closing prepared statements or portals, but
> according to
>
> https://www.postgresql.org/docs/current/static/libpq-exec.html#LIBPQ-PQPREPARE
> "there is no libpq function for deleting a prepared statement".
>
> Could you tell me please, what is the reason for this?
>

Nobody's implemented it.

A patch to add PQclosePrepared and PQsendClosePrepared would be welcome. At
least, I think so...


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

Re: [HACKERS] Inheritance

[Craig Ringer]

On 26 May 2016 at 01:56, Jim Nasby  wrote:

> On 5/24/16 8:52 PM, Craig Ringer wrote:
> > Absolutely, and I use OO heavily. But a half-assed "object relational"
> > feature in the database that only kind-of works isn't OO, and it's
> > generally less useful than using existing relational-to-oo modelling
> > techniques like FK-related 1:1 child tables for specialisation.
>
> How is it less useful than that? To me, the FK "solution" is the absolute
> worst of everything: you still have all the separate child tables that you
> must explicitly query *and* you have to get all the joins correct as well.
> And hope it doesn't have horrible performance.
>
> Note that there was enough enthusiasm to adopt whole new database
>> engines, but not enough to use PostgreSQL's existing features for that.
>> Partly because they suck. In particular, people looking for this tend to
>> want to be able to create new subtypes without having to mess around
>> making schema changes and modelling everything.
>>
>
> Which is a decision people have come to regret, because then your codebase
> somehow has to deal with 38 different versions of what a "customer" is.


Oh, I totally agree there. It's almost as bad as people serialising Java
objects into the DB. Ugh. It's a bad, bad idea.

It's also what people seem to want to do, and I understand that somewhat
given the pain involved in full table rewrites under extended locks and the
hoop-jumping required to avoid them. It's particularly painful with
something app devs tend to need, but RDBMS designers prefer to ignore:
user-defined columns/attributes. Find me someone who *doesn't* want the
ability for their app users/deployers/etc to add arbitrary attributes to
customer records etc w/o code changes. jsonb helps a lot there, but you
lose Pg's type system and have to use a different query syntax etc.

Lower-pain ways to make schema changes and blend dynamic columns
(application-user-defined columns)  with normal columns would help a lot
there. A pseudo-column that can store a TOASTable record extension that's a
set of colname/coltype/coltypmod . Storage is the relatively easy bit
though, the problem is how to work with that though the planner and
executor and output useful results...

There's much more future in improving document-structured storage like
>> jsonb, and possibly extending in future toward hybrid storage with some
>> normal cols and some dynamic cols, than with Pg's
>> pseudo-object-relational inheritance feature.
>>
>
> I don't see why we can't do both. There's many cases where more
> flexibility in what output tuples look like would be very valuable. The
> JSON features are one aspect; crosstab is another.
>

Agreed. A real PIVOT, especially one we can use as a source of tuples for
subqueries etc, would be really useful and is the sort of thing we'd need
some degree of dynamic column handling for.


> Postgres is well past the point where our relational features are the big
> selling point. It's now about scale, an incredibly robust storage engine,
> and all the extensiblity opportunities. We've moved from being an RDBMS to
> being a "Data Platform". Improving our OO capabilities just continues that.


Right. I'm just not convinced table inheritance is a useful part of that
picture.


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

Re: [HACKERS] pg_bsd_indent - improvements around offsetof and sizeof

[Tom Lane]

Andres Freund  writes:
> Might be worthwhile to look into 'uncrustify'. It's fairly
> customizable.  A colleague at citus made all citus code be formatted by
> it; and while there's some minor details I dislike, it seems to work
> ok.

Hmm ... a quick look says that it's been around for awhile and it's
actively maintained, which seem like positive things.  Would be worth
seeing if it can be configured to match our style.

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] [BUGS] BUG #13473: VACUUM FREEZE mistakenly cancel standby sessions

[Alvaro Herrera]

Marco Nenciarini wrote:

> diff --git a/src/backend/access/heap/heapam.c 
> b/src/backend/access/heap/heapam.c
> index eb8eada..434880a 100644
> --- a/src/backend/access/heap/heapam.c
> +++ b/src/backend/access/heap/heapam.c
> @@ -4764,7 +4764,13 @@ heap_xlog_freeze(XLogRecPtr lsn, XLogRecord *record)
>* consider the frozen xids as running.
>*/
>   if (InHotStandby)
> - ResolveRecoveryConflictWithSnapshot(cutoff_xid, xlrec->node);
> + {
> + TransactionId latestRemovedXid = cutoff_xid;
> +
> + TransactionIdRetreat(latestRemovedXid);
> +
> + ResolveRecoveryConflictWithSnapshot(latestRemovedXid, 
> xlrec->node);
> + }
>  
>   /* If we have a full-page image, restore it and we're done */
>   if (record->xl_info & XLR_BKP_BLOCK(0))

Actually, in 9.3 there's a heap_xlog_freeze routine and a separate
heap_xlog_freeze_page routine; see commit 8e9a16ab8f7f.  Applying this
patch to that branch leaves heap_xlog_freeze itself unmodified, and only
heap_xlog_freeze_page is updated.  I think this is pretty much harmless,
as the older routine would only be called by a standby running a minor
version earlier than that commit, which has its own set of much more
serious bugs.  But I think the safest route is to patch them both, so
I'm going to do that.

-- 
Á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] pg_bsd_indent - improvements around offsetof and sizeof

[Tom Lane]

Alvaro Herrera  writes:
> Andres Freund wrote:
>> On 2016-05-25 22:01:53 +0200, Piotr Stefaniak wrote:
>>> On 2016-05-25 21:13, Tom Lane wrote:
 I'd love to see a fix for its brain damage around function pointer typedef 
 formatting, too.

>>> Show me a few examples and I'll look into it.

> See src/include/replication/logical.h; the problem there is pretty
> obvious.

More examples are in, eg, src/include/access/amapi.h.  It's aligning the
additional lines of a multiline function-pointer typedef to *something*,
but it's not very clear what exactly, and in any case it's indenting them
much too far to have any hope of readability.

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] pg_bsd_indent - improvements around offsetof and sizeof

[Andres Freund]

On 2016-05-25 18:17:51 -0400, Alvaro Herrera wrote:
> Andres Freund wrote:
> > On 2016-05-25 22:01:53 +0200, Piotr Stefaniak wrote:
> > FWIW, I looked at using clang-format at some point, and it looked like
> > it'd be a number of additional options to make it work for our case
> > without changing the code layout too much. There seemed limited
> > enthusiasm from the authors about accepting relevant options.
> 
> FWIW I looked at GNU indent a year ago or so, and there were a few
> things about our style that they simply did not have options for.  I
> don't recall the details but my conclusion was that it was a dead end.

Might be worthwhile to look into 'uncrustify'. It's fairly
customizable.  A colleague at citus made all citus code be formatted by
it; and while there's some minor details I dislike, it seems to work
ok.


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

Re: [HACKERS] pg_bsd_indent - improvements around offsetof and sizeof

[Alvaro Herrera]

Andres Freund wrote:
> On 2016-05-25 22:01:53 +0200, Piotr Stefaniak wrote:
> > On 2016-05-25 21:13, Tom Lane wrote:
> > > I'd love to see a fix for its brain damage around function pointer 
> > > typedef formatting, too.
> > 
> > Show me a few examples and I'll look into it.

See src/include/replication/logical.h; the problem there is pretty
obvious.

For another broken construct, see tupleLockExtraInfo in
src/backend/access/heap/heapam.c.

Also, see pre_indent and post_indent in src/tools/pgindent/pgindent.
A few bugs in pg_bsd_indent are worked around there.

> > > I'm excited about this too, not least because it suggests that maybe 
> > > bsdindent isn't quite as opaque as it appears.
> > 
> > It's old, hacked on many times over the past few decades and historically
> > just a band-aid rather than something designed from the ground up, so it's
> > not easy to work with. Which is why I think that a newer tool (like
> > ClangFormat) should be considered as a replacement for pg_bsd_indent.
> 
> FWIW, I looked at using clang-format at some point, and it looked like
> it'd be a number of additional options to make it work for our case
> without changing the code layout too much. There seemed limited
> enthusiasm from the authors about accepting relevant options.

FWIW I looked at GNU indent a year ago or so, and there were a few
things about our style that they simply did not have options for.  I
don't recall the details but my conclusion was that it was a dead end.

-- 
Á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] statistics for shared catalogs not updated when autovacuum is off

[Tom Lane]

I wrote:
> OK.  I'm going to work on replacing the DBWriteRequest structure with
> an OID list, and on improving the comments some more, but I don't
> anticipate further functional changes.

Here's the end result of that.  I went ahead and pushed the original
small patch, concluding that that was fixing a different bug and so
should be a different commit.

regards, tom lane

diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index a6db9cd..ef36193 100644
*** a/src/backend/postmaster/pgstat.c
--- b/src/backend/postmaster/pgstat.c
***
*** 38,44 
  #include "access/xact.h"
  #include "catalog/pg_database.h"
  #include "catalog/pg_proc.h"
- #include "lib/ilist.h"
  #include "libpq/ip.h"
  #include "libpq/libpq.h"
  #include "libpq/pqsignal.h"
--- 38,43 
*** static int	localNumBackends = 0;
*** 221,237 
  static PgStat_ArchiverStats archiverStats;
  static PgStat_GlobalStats globalStats;
  
! /* Write request info for each database */
! typedef struct DBWriteRequest
! {
! 	Oid			databaseid;		/* OID of the database to write */
! 	TimestampTz request_time;	/* timestamp of the last write request */
! 	slist_node	next;
! } DBWriteRequest;
! 
! /* Latest statistics request times from backends */
! static slist_head last_statrequests = SLIST_STATIC_INIT(last_statrequests);
  
  static volatile bool need_exit = false;
  static volatile bool got_SIGHUP = false;
  
--- 220,233 
  static PgStat_ArchiverStats archiverStats;
  static PgStat_GlobalStats globalStats;
  
! /*
!  * List of OIDs of databases we need to write out.  If an entry is InvalidOid,
!  * it means to write only the shared-catalog stats ("DB 0"); otherwise, we
!  * will write both that DB's data and the shared stats.
!  */
! static List *pending_write_requests = NIL;
  
+ /* Signal handler flags */
  static volatile bool need_exit = false;
  static volatile bool got_SIGHUP = false;
  
*** PgstatCollectorMain(int argc, char *argv
*** 3497,3504 
  	init_ps_display("stats collector process", "", "", "");
  
  	/*
! 	 * Read in an existing statistics stats file or initialize the stats to
! 	 * zero.
  	 */
  	pgStatRunningInCollector = true;
  	pgStatDBHash = pgstat_read_statsfiles(InvalidOid, true, true);
--- 3493,3499 
  	init_ps_display("stats collector process", "", "", "");
  
  	/*
! 	 * Read in existing stats files or initialize the stats to zero.
  	 */
  	pgStatRunningInCollector = true;
  	pgStatDBHash = pgstat_read_statsfiles(InvalidOid, true, true);
*** PgstatCollectorMain(int argc, char *argv
*** 3544,3551 
  			}
  
  			/*
! 			 * Write the stats file if a new request has arrived that is not
! 			 * satisfied by existing file.
  			 */
  			if (pgstat_write_statsfile_needed())
  pgstat_write_statsfiles(false, false);
--- 3539,3546 
  			}
  
  			/*
! 			 * Write the stats file(s) if a new request has arrived that is
! 			 * not satisfied by existing file(s).
  			 */
  			if (pgstat_write_statsfile_needed())
  pgstat_write_statsfiles(false, false);
*** pgstat_get_tab_entry(PgStat_StatDBEntry 
*** 3875,3888 
   * pgstat_write_statsfiles() -
   *		Write the global statistics file, as well as requested DB files.
   *
!  *	If writing to the permanent files (happens when the collector is
!  *	shutting down only), remove the temporary files so that backends
!  *	starting up under a new postmaster can't read the old data before
!  *	the new collector is ready.
   *
   *	When 'allDbs' is false, only the requested databases (listed in
!  *	last_statrequests) will be written; otherwise, all databases will be
!  *	written.
   * --
   */
  static void
--- 3870,3883 
   * pgstat_write_statsfiles() -
   *		Write the global statistics file, as well as requested DB files.
   *
!  *	'permanent' specifies writing to the permanent files not temporary ones.
!  *	When true (happens only when the collector is shutting down), also remove
!  *	the temporary files so that backends starting up under a new postmaster
!  *	can't read old data before the new collector is ready.
   *
   *	When 'allDbs' is false, only the requested databases (listed in
!  *	pending_write_requests) will be written; otherwise, all databases
!  *	will be written.
   * --
   */
  static void
*** pgstat_write_statsfiles(bool permanent, 
*** 3942,3956 
  	while ((dbentry = (PgStat_StatDBEntry *) hash_seq_search(&hstat)) != NULL)
  	{
  		/*
! 		 * Write out the tables and functions into the DB stat file, if
! 		 * required.
! 		 *
! 		 * We need to do this before the dbentry write, to ensure the
! 		 * timestamps written to both are consistent.
  		 */
  		if (allDbs || pgstat_db_requested(dbentry->databaseid))
  		{
  			dbentry->stats_timestamp = globalStats.stats_timestamp;
  			pgstat_write_db_statsfile(dbentry, permanent);
  		}
  
--- 3937,3950 
  	while ((dbentry = (PgStat_StatDBEntry *) hash_seq_

Re: [HACKERS] Is the unfair lwlock behavior intended?

[Andres Freund]

On 2016-05-25 17:24:22 -0400, Robert Haas wrote:
> On Wed, May 25, 2016 at 3:26 PM, Andres Freund  wrote:
> > On 2016-05-25 11:15:37 -0700, Andres Freund wrote:
> >> On 2016-05-25 14:09:43 -0400, Robert Haas wrote:
> >> I don't think anybody was doing that? The first questions on this thread
> >> were about upgrading and retesting...
> >
> > Something I've repeatedly wondered about around this topic is whether we
> > could split ProcArrayLock into one that governs entering or leaving the
> > procarray from the one that's for consistent snapshots.  I think there's
> > no need for 
> > ProcArrayAdd/ProcArrayRemove/CountDBBackends()/CancelDBBackends()/
> > CountUserBackends()/CountOtherDBBackends() (and potentially some more)
> > to conflict with GetSnapshotData()/ProcArrayEndTransaction()/
> > TransactionIdIsInProgress()/TransactionIdIsActive()/GetOldestXmin()/...
> > as long as we're careful to ensure that by the time a entry is removed
> > ProcArrayEndTransaction() has been called.
> 
> I'm doubtful about how much that would reduce contention,

Yea, I don't think it'll usually will make a huge difference.  It'd
likely have solved this specific complaint though; and it'd remove
nearly all other exclusive acquisitions of ProcArrayLock other than
ProcArrayEndTransaction().


> However, I think it might be worth doing anyway, because redesigning
> the whole mechanism might be easier if that lock weren't doing so many
> only-semi-related things.

Very much agreed upon this.  Having a distinct 'SnapshotLock', for
EndTransaction() and GetSnapshotData() would be a first step in making
the locking more granular.

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] Is the unfair lwlock behavior intended?

[Robert Haas]

On Wed, May 25, 2016 at 3:26 PM, Andres Freund  wrote:
> On 2016-05-25 11:15:37 -0700, Andres Freund wrote:
>> On 2016-05-25 14:09:43 -0400, Robert Haas wrote:
>> I don't think anybody was doing that? The first questions on this thread
>> were about upgrading and retesting...
>
> Something I've repeatedly wondered about around this topic is whether we
> could split ProcArrayLock into one that governs entering or leaving the
> procarray from the one that's for consistent snapshots.  I think there's
> no need for ProcArrayAdd/ProcArrayRemove/CountDBBackends()/CancelDBBackends()/
> CountUserBackends()/CountOtherDBBackends() (and potentially some more)
> to conflict with GetSnapshotData()/ProcArrayEndTransaction()/
> TransactionIdIsInProgress()/TransactionIdIsActive()/GetOldestXmin()/...
> as long as we're careful to ensure that by the time a entry is removed
> ProcArrayEndTransaction() has been called.

I'm doubtful about how much that would reduce contention, because when
I've used perf or inserted instrumentation to see which actual call
sides are the problem, it's always been entirely down to
GetSnapshotData() and ProcArrayEndTransaction().  However, I think it
might be worth doing anyway, because redesigning the whole mechanism
might be easier if that lock weren't doing so many only-semi-related
things.

-- 
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] Changed SRF in targetlist handling

[Tom Lane]

Andres Freund  writes:
> On 2016-05-25 15:20:03 -0400, Tom Lane wrote:
>> We could certainly make a variant behavior in nodeFunctionscan.c that
>> emulates that, if we feel that being exactly bug-compatible on the point
>> is actually what we want.  I'm dubious about that though, not least
>> because I don't think *anyone* actually believes that that behavior isn't
>> broken.  Did you read my upthread message suggesting assorted compromise
>> choices?

> You mean https://www.postgresql.org/message-id/21076.1464034...@sss.pgh.pa.us 
> ?
> If so, yes.

> If we go with rewriting this into LATERAL, I'd vote for 2.5 (trailed by
> option 1), that'd keep most of the functionality, and would break
> visibly rather than invisibly in the cases where not.

2.5 would be okay with me.

> I guess you're not planning to work on this?

Well, not right now, as it's clearly too late for 9.6.  I might hack on
it later if nobody beats me to it.

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] Changed SRF in targetlist handling

[Andres Freund]

On 2016-05-25 15:20:03 -0400, Tom Lane wrote:
> Andres Freund  writes:
> > On 2016-05-25 15:02:23 -0400, Tom Lane wrote:
> >> [ shrug... ]  That seems like it's morally equivalent to (but uglier than)
> >> what I wanted to do, which is to teach the planner to rewrite the query to
> >> put the SRFs into a lateral FROM item.  Splitting the tlist into two
> >> levels will work out to be exactly the same rewriting problem.
> 
> > I think that depends on how bug compatible we want to be. It seems
> > harder to get the (rather odd!) lockstep iteration behaviour between two
> > SRFS with the LATERAL approach?
> 
> We could certainly make a variant behavior in nodeFunctionscan.c that
> emulates that, if we feel that being exactly bug-compatible on the point
> is actually what we want.  I'm dubious about that though, not least
> because I don't think *anyone* actually believes that that behavior isn't
> broken.  Did you read my upthread message suggesting assorted compromise
> choices?

You mean https://www.postgresql.org/message-id/21076.1464034...@sss.pgh.pa.us ?
If so, yes.

If we go with rewriting this into LATERAL, I'd vote for 2.5 (trailed by
option 1), that'd keep most of the functionality, and would break
visibly rather than invisibly in the cases where not.

I guess you're not planning to work on this?

Greetings,

Andres Freund


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

Re: [HACKERS] pg_bsd_indent - improvements around offsetof and sizeof

[Andres Freund]

On 2016-05-25 22:01:53 +0200, Piotr Stefaniak wrote:
> On 2016-05-25 21:13, Tom Lane wrote:
> > I'd love to see a fix for its brain damage around function pointer typedef 
> > formatting, too.
> 
> Show me a few examples and I'll look into it.
> 
> > I'm excited about this too, not least because it suggests that maybe 
> > bsdindent isn't quite as opaque as it appears.
> 
> It's old, hacked on many times over the past few decades and historically
> just a band-aid rather than something designed from the ground up, so it's
> not easy to work with. Which is why I think that a newer tool (like
> ClangFormat) should be considered as a replacement for pg_bsd_indent.

FWIW, I looked at using clang-format at some point, and it looked like
it'd be a number of additional options to make it work for our case
without changing the code layout too much. There seemed limited
enthusiasm from the authors about accepting relevant options.

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] pg_bsd_indent - improvements around offsetof and sizeof

[Piotr Stefaniak]

On 2016-05-25 21:13, Tom Lane wrote:

I'd love to see a fix for its brain damage around function pointer typedef 
formatting, too.


Show me a few examples and I'll look into it.


I'm excited about this too, not least because it suggests that maybe bsdindent 
isn't quite as opaque as it appears.


It's old, hacked on many times over the past few decades and 
historically just a band-aid rather than something designed from the 
ground up, so it's not easy to work with. Which is why I think that a 
newer tool (like ClangFormat) should be considered as a replacement for 
pg_bsd_indent.




--
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] statistics for shared catalogs not updated when autovacuum is off

[Tom Lane]

Michael Paquier  writes:
> On Tue, May 24, 2016 at 3:52 PM, Tom Lane  wrote:
>> True, although I'm not sure if adding a dependency on IsSharedRelation()
>> here is a good thing.  In any case, you took the dependency too far ...

> OK. It seemed more simple to me to have the dependency on catalog.h to
> avoid two lookups. But I won't fight for it either.

Well, it might be worth doing, but I think it's a distinct patch.

> I am going to look at the second patch you sent.

OK.  I'm going to work on replacing the DBWriteRequest structure with
an OID list, and on improving the comments some more, but I don't
anticipate further functional changes.

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] statistics for shared catalogs not updated when autovacuum is off

[Michael Paquier]

On Tue, May 24, 2016 at 3:52 PM, Tom Lane  wrote:
> Michael Paquier  writes:
>> At the same time I am not getting why pgstat_fetch_stat_tabentry needs
>> to be that complicated. Based on the relation OID we can know if it is
>> a shared relation or not, there is no point in doing two times the
>> same lookup in the pgstat hash table.
>
> True, although I'm not sure if adding a dependency on IsSharedRelation()
> here is a good thing.  In any case, you took the dependency too far ...

OK. It seemed more simple to me to have the dependency on catalog.h to
avoid two lookups. But I won't fight for it either.

>> Attached is a patch that fixes the issue here:
>
> I have not tested it, but I would bet a lot that this patch is broken:
> what will happen if the first request in a transaction is for a shared
> catalog is that we'll read just the shared-catalog data, and then
> subsequent requests for stats for an unshared table will find nothing.
> Moreover, even if you undid the change to the pgstat_read_statsfiles()
> call so that we still did read the appropriate unshared stats, this
> would have the effect of reversing the problem: if the first request
> is for a shared catalog, then we'd check to ensure the shared stats
> are up-to-date, but fail to ensure that the unshared ones are.

Your bet is right. I forgot the transactional behavior of this code
path. The stats obtained at the first request would not have been
correct for the database of the backend.

> In short, I think the attached is enough to fix it.  There's some cosmetic
> cleanup that could be done here: a lot of these functions could use better
> comments, and I'm not happy about the autovac launcher depending on
> MyDatabaseId being zero.  But those are not functional problems.

I am going to look at the second patch you sent.
-- 
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] [PROPOSAL] Move all am-related reloption code into src/backend/access/[am-name] and get rid of relopt_kind

[Fabrízio de Royes Mello]

On Wed, May 25, 2016 at 3:03 PM, Alvaro Herrera 
wrote:
>
> Nikolay Shaplov wrote:
> > В письме от 25 мая 2016 13:25:38 Вы написали:
> > > Teodor Sigaev wrote:
> > > > >This all should me moved behind "access method" abstraction...
> > > >
> > > > +1 relopt_kind should be moved in am, at least. Or removed.
> > >
> > > Hm, but we have tablespace options too, so I'm not sure that using AM
as
> > > abstraction level is correct.
> > We will use am for all indexes, and keep all the rest in relopotion.c
for
> > non-indexes. May be divided options catalog into sections one section
for each
> > kind.
>
> That makes sense.  I can review the patch later.
>
> > And as I also would like to use this code for dynamic attoptions later,
I
> > would like to remove relopt_kind at all. Because it spoils live in that
case.
>
> As I remember, Fabrízio (in CC) had a patch for dynamic reloptions, but
> there was some problem with it and we dumped it; see
>
https://www.postgresql.org/message-id/CAFcNs+rqCq1H5eXW-cvdti6T-xo2STEkhjREx=odmakk5ti...@mail.gmail.com
> for previous discussion.
>

Yeah, and it was forked into other long discussion thread [1] that explain
the reasons to patch got rejected.

IMHO we need a way (maybe at SQL level too) to define and manipulate the
reloptions, and this way should cover all database objects. It isn't a
simple patch because we'll need introduce new catalogs, refactor and
rewrite a lot of code... but it should be a better way to do it. Anyway we
can start with your approach and grow it in small pieces. I have a lot of
ideas about it so I'm glad to discuss it if you want.

Regards,

[1]
https://www.postgresql.org/message-id/cafj8prcx_vdcsdbumknhhyr_-n_ctl84_7r+-bj17hckt_m...@mail.gmail.com
[2]
https://www.postgresql.org/message-id/ca+tgmoznfdqt2kotx38yjus3f_avisclxawbmpdwxhyxrg_...@mail.gmail.com

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello

Re: [HACKERS] Is the unfair lwlock behavior intended?

[Andres Freund]

Hi,

On 2016-05-24 06:03:07 +, Tsunakawa, Takayuki wrote:
> At that time, many backend processes (I forgot the number) were acquiring and 
> releasing share mode lock on ProcArrayLock, most of which were from 
> TransactionIsInProgress().

FWIW, I suspect that 9.6 might be a fair bit better here, due to
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=8a7d0701814a4e293efad22091d6f6fb441bbe1c

Regards,

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] Is the unfair lwlock behavior intended?

[Andres Freund]

On 2016-05-25 11:15:37 -0700, Andres Freund wrote:
> On 2016-05-25 14:09:43 -0400, Robert Haas wrote:
> I don't think anybody was doing that? The first questions on this thread
> were about upgrading and retesting...

Something I've repeatedly wondered about around this topic is whether we
could split ProcArrayLock into one that governs entering or leaving the
procarray from the one that's for consistent snapshots.  I think there's
no need for ProcArrayAdd/ProcArrayRemove/CountDBBackends()/CancelDBBackends()/
CountUserBackends()/CountOtherDBBackends() (and potentially some more)
to conflict with GetSnapshotData()/ProcArrayEndTransaction()/
TransactionIdIsInProgress()/TransactionIdIsActive()/GetOldestXmin()/...
as long as we're careful to ensure that by the time a entry is removed
ProcArrayEndTransaction() has been called.

Greetings,

Andres Freund


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

Re: [HACKERS] Changed SRF in targetlist handling

[Tom Lane]

Andres Freund  writes:
> On 2016-05-25 15:02:23 -0400, Tom Lane wrote:
>> [ shrug... ]  That seems like it's morally equivalent to (but uglier than)
>> what I wanted to do, which is to teach the planner to rewrite the query to
>> put the SRFs into a lateral FROM item.  Splitting the tlist into two
>> levels will work out to be exactly the same rewriting problem.

> I think that depends on how bug compatible we want to be. It seems
> harder to get the (rather odd!) lockstep iteration behaviour between two
> SRFS with the LATERAL approach?

We could certainly make a variant behavior in nodeFunctionscan.c that
emulates that, if we feel that being exactly bug-compatible on the point
is actually what we want.  I'm dubious about that though, not least
because I don't think *anyone* actually believes that that behavior isn't
broken.  Did you read my upthread message suggesting assorted compromise
choices?

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] pg_bsd_indent - improvements around offsetof and sizeof

[Tom Lane]

Robert Haas  writes:
> On Sun, May 22, 2016 at 4:16 PM, Piotr Stefaniak
>  wrote:
>> I think I've managed to improve pg_bsd_indent's handling of two types of
>> cases.

> Wow, that seems pretty great.  I haven't scrutinized your changes to
> pg_bsd_indent, but effect_on_pg.diff looks like a large improvement.

I'm excited about this too, not least because it suggests that maybe
bsdindent isn't quite as opaque as it appears.  I'd love to see a fix
for its brain damage around function pointer typedef formatting, too.

Assuming this patch withstands more careful review, we will need to think
about project policy for how/when to apply such fixes.  The last time
we made any real change to pgindent's behavior was when we changed its
wrapping of comment blocks back around 8.1 ... and I cursed that decision
at least weekly for the next five years, because it caused constant
back-patching pain.  If we make a change like this, I think we should
*strongly* consider reindenting all the live back branches along with
HEAD.

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: pg_restore parallel-execution-deadlock issue

[Michael Paquier]

On Fri, Apr 8, 2016 at 2:24 AM, Michael Paquier
 wrote:
> On Tue, Apr 5, 2016 at 9:28 AM, Armin Schöffmann
>  wrote:
>> If this is not the correct place to discuss patches, I'd be glad if somebody 
>> can notify the tool's maintainer, to take a look into it.
>
> Here or -bugs are correct places to discuss such issues. People doing
> from time to time work with Windows hang up on the two lists.

ea274b2 has changed the way disconnection is done is is now closing
both the read and write pipes. So you may want to retry if things get
better with the next round of minor releases.
-- 
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] Changed SRF in targetlist handling

[Andres Freund]

On 2016-05-25 15:02:23 -0400, Tom Lane wrote:
> Andres Freund  writes:
> > On 2016-05-23 13:10:29 -0400, Tom Lane wrote:
> >> Would that not lead to, in effect, duplicating all of execQual.c?  The new
> >> executor node would still have to be prepared to process all expression
> >> node types.
> 
> > I don't think it necessarily has to. ISTM that if we add a version of
> > ExecProject()/ExecTargetList() that continues returning multiple rows,
> > we can make the knowledge about the one type of expression we allow to
> > return multiple rows.  That'd require a bit of uglyness to implement
> > stuff like
> > SELECT generate_series(1, 2)::text, generate_series(1, 2) * 5;
> > etc. It seems we'd basically have to do one projection step for the
> > SRFs, and then another for the rest.  I'm inclined to think that's
> > acceptable to get rid of a lot of the related uglyness.
> 
> [ shrug... ]  That seems like it's morally equivalent to (but uglier than)
> what I wanted to do, which is to teach the planner to rewrite the query to
> put the SRFs into a lateral FROM item.  Splitting the tlist into two
> levels will work out to be exactly the same rewriting problem.

I think that depends on how bug compatible we want to be. It seems
harder to get the (rather odd!) lockstep iteration behaviour between two
SRFS with the LATERAL approach?

tpch[6098][1]=# SELECT generate_series(1, 3), generate_series(1,3);
┌─┬─┐
│ generate_series │ generate_series │
├─┼─┤
│   1 │   1 │
│   2 │   2 │
│   3 │   3 │
└─┴─┘
(3 rows)

tpch[6098][1]=# SELECT generate_series(1, 3), generate_series(1,4);
┌─┬─┐
│ generate_series │ generate_series │
├─┼─┤
│   1 │   1 │
│   2 │   2 │
│   3 │   3 │
│   1 │   4 │
│   2 │   1 │
│   3 │   2 │
│   1 │   3 │
│   2 │   4 │
│   3 │   1 │
│   1 │   2 │
│   2 │   3 │
│   3 │   4 │
└─┴─┘
(12 rows)


Regards,

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] Changed SRF in targetlist handling

[Tom Lane]

Andres Freund  writes:
> On 2016-05-23 13:10:29 -0400, Tom Lane wrote:
>> Would that not lead to, in effect, duplicating all of execQual.c?  The new
>> executor node would still have to be prepared to process all expression
>> node types.

> I don't think it necessarily has to. ISTM that if we add a version of
> ExecProject()/ExecTargetList() that continues returning multiple rows,
> we can make the knowledge about the one type of expression we allow to
> return multiple rows.  That'd require a bit of uglyness to implement
> stuff like
> SELECT generate_series(1, 2)::text, generate_series(1, 2) * 5;
> etc. It seems we'd basically have to do one projection step for the
> SRFs, and then another for the rest.  I'm inclined to think that's
> acceptable to get rid of a lot of the related uglyness.

[ shrug... ]  That seems like it's morally equivalent to (but uglier than)
what I wanted to do, which is to teach the planner to rewrite the query to
put the SRFs into a lateral FROM item.  Splitting the tlist into two
levels will work out to be exactly the same rewriting problem.

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] Changed SRF in targetlist handling

[Andres Freund]

On 2016-05-23 13:10:29 -0400, Tom Lane wrote:
> Andres Freund  writes:
> > One idea I circulated was to fix that by interjecting a special executor
> > node to process SRF containing targetlists (reusing Result possibly?).
> > That'd allow to remove the isDone argument from ExecEval*/ExecProject*
> > and get rid of ps_TupFromTlist which is fairly ugly.
> 
> Would that not lead to, in effect, duplicating all of execQual.c?  The new
> executor node would still have to be prepared to process all expression
> node types.

I don't think it necessarily has to. ISTM that if we add a version of
ExecProject()/ExecTargetList() that continues returning multiple rows,
we can make the knowledge about the one type of expression we allow to
return multiple rows.  That'd require a bit of uglyness to implement
stuff like
SELECT generate_series(1, 2)::text, generate_series(1, 2) * 5;
etc. It seems we'd basically have to do one projection step for the
SRFs, and then another for the rest.  I'm inclined to think that's
acceptable to get rid of a lot of the related uglyness.


> > One issue with removing targetlist SRFs is that they're currently
> > considerably faster than SRFs in FROM:
> 
> I suspect that depends greatly on your test case.  But in any case
> we could put more effort into optimizing nodeFunctionscan.

I doubt you'll find cases where it's significantly the other way round
for percall SRFs. The fundamental issue is that targetlist SRFs don't
have to spill to a tuplestore, whereas nodeFunctionscan ones have to
(even if they're percall).


-- 
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] statistics for shared catalogs not updated when autovacuum is off

[Tom Lane]

I wrote:
> In short, I think the attached is enough to fix it.  There's some cosmetic
> cleanup that could be done here: a lot of these functions could use better
> comments, and I'm not happy about the autovac launcher depending on
> MyDatabaseId being zero.  But those are not functional problems.

While re-reading this, I realized that there's a second serious bug in the
same area: commit 187492b6c basically broke the mechanism that prevents
duplicated writes when two backends send inquiries for the same DB at
about the same time.  The intention is that if an inquiry arrives with a
cutoff_time older than when we last wrote the DB's stats, we don't have to
do anything.  But as the code stands we will make a DBWriteRequest
unconditionally, because the previous request has been removed from the
list.  It would only succeed in merging requests if the second one arrives
before we have executed a write in response to the first one, which looks
to me to be impossible given the coding of the message-processing loop in
PgstatCollectorMain.

In the attached expanded patch, I fixed pgstat_recv_inquiry() to not make
a write request unless the cutoff-time restriction is met.  I also removed
DBWriteRequest.request_time, which was not being consulted at all.  At
this point the DBWriteRequest data structure is just overkill and could be
replaced by an OID list at very substantial notational savings, but I've
not done that yet.

Comments/objections?

regards, tom lane

diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 41f4374..66f0e9c 100644
*** a/src/backend/postmaster/pgstat.c
--- b/src/backend/postmaster/pgstat.c
*** static PgStat_GlobalStats globalStats;
*** 224,231 
  /* Write request info for each database */
  typedef struct DBWriteRequest
  {
! 	Oid			databaseid;		/* OID of the database to write */
! 	TimestampTz request_time;	/* timestamp of the last write request */
  	slist_node	next;
  } DBWriteRequest;
  
--- 224,230 
  /* Write request info for each database */
  typedef struct DBWriteRequest
  {
! 	Oid			databaseid;		/* OID of database to write (0 => shared) */
  	slist_node	next;
  } DBWriteRequest;
  
*** pgstat_recv_inquiry(PgStat_MsgInquiry *m
*** 4838,4850 
  	elog(DEBUG2, "received inquiry for database %u", msg->databaseid);
  
  	/*
! 	 * Find the last write request for this DB.  If it's older than the
! 	 * request's cutoff time, update it; otherwise there's nothing to do.
  	 *
  	 * Note that if a request is found, we return early and skip the below
  	 * check for clock skew.  This is okay, since the only way for a DB
  	 * request to be present in the list is that we have been here since the
! 	 * last write round.
  	 */
  	slist_foreach(iter, &last_statrequests)
  	{
--- 4836,4848 
  	elog(DEBUG2, "received inquiry for database %u", msg->databaseid);
  
  	/*
! 	 * If there's already a write request for this DB, there's nothing to do.
  	 *
  	 * Note that if a request is found, we return early and skip the below
  	 * check for clock skew.  This is okay, since the only way for a DB
  	 * request to be present in the list is that we have been here since the
! 	 * last write round.  It seems sufficient to check for clock skew once per
! 	 * write round.
  	 */
  	slist_foreach(iter, &last_statrequests)
  	{
*** pgstat_recv_inquiry(PgStat_MsgInquiry *m
*** 4853,4873 
  		if (req->databaseid != msg->databaseid)
  			continue;
  
- 		if (msg->cutoff_time > req->request_time)
- 			req->request_time = msg->cutoff_time;
  		return;
  	}
  
  	/*
! 	 * There's no request for this DB yet, so create one.
! 	 */
! 	newreq = palloc(sizeof(DBWriteRequest));
! 
! 	newreq->databaseid = msg->databaseid;
! 	newreq->request_time = msg->clock_time;
! 	slist_push_head(&last_statrequests, &newreq->next);
! 
! 	/*
  	 * If the requestor's local clock time is older than stats_timestamp, we
  	 * should suspect a clock glitch, ie system time going backwards; though
  	 * the more likely explanation is just delayed message receipt.  It is
--- 4851,4864 
  		if (req->databaseid != msg->databaseid)
  			continue;
  
  		return;
  	}
  
  	/*
! 	 * Check to see if we last wrote this database at a time >= the requested
! 	 * cutoff time.  If so, this is a stale request that was generated before
! 	 * we updated the DB file, and we don't need to do it again.
! 	 *
  	 * If the requestor's local clock time is older than stats_timestamp, we
  	 * should suspect a clock glitch, ie system time going backwards; though
  	 * the more likely explanation is just delayed message receipt.  It is
*** pgstat_recv_inquiry(PgStat_MsgInquiry *m
*** 4876,4882 
  	 * to update the stats file for a long time.
  	 */
  	dbentry = pgstat_get_db_entry(msg->databaseid, false);
! 	if ((dbentry != NULL) && (msg->clock_time < dbentry->stats_timestamp))
  	{
  		TimestampTz cur_ts = GetCurrentTimestamp();
  
--- 48

Re: [HACKERS] Is the unfair lwlock behavior intended?

[Andres Freund]

On 2016-05-25 14:09:43 -0400, Robert Haas wrote:
> I think this is looking at the problem from the wrong angle.  The OP's
> complaint is pretty fair: a 30-second wait for ProcArrayLock is
> horrendous, and if that's actually something that is happening with
> any significant regularity on well-configured systems, we need to fix
> it somehow.

No disagreement there.


> I don't think we can just say "oh, well,
> sometimes the system becomes totally unresponsive for more than 30
> seconds, but we don't care".  We have to care about that.

I don't think anybody was doing that? The first questions on this thread
were about upgrading and retesting...


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] pg_bsd_indent - improvements around offsetof and sizeof

[Robert Haas]

On Sun, May 22, 2016 at 4:16 PM, Piotr Stefaniak
 wrote:
> I think I've managed to improve pg_bsd_indent's handling of two types of
> cases.

Wow, that seems pretty great.  I haven't scrutinized your changes to
pg_bsd_indent, but effect_on_pg.diff looks like a large improvement.

-- 
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] Is the unfair lwlock behavior intended?

[Robert Haas]

On Tue, May 24, 2016 at 6:50 PM, Andres Freund  wrote:
> Now we potentially could mark individual lwlocks as being fair
> locks. But which ones would those be? Certainly not ProcArrayLock, it's
> way too heavily contended.

I think this is looking at the problem from the wrong angle.  The OP's
complaint is pretty fair: a 30-second wait for ProcArrayLock is
horrendous, and if that's actually something that is happening with
any significant regularity on well-configured systems, we need to fix
it somehow.  Whether FIFO queueing on LWLocks is the right way to fix
it is a separate question, and I agree that the answer is probably
"no" ... although it does strike me that Amit's work on group XID
clearing might make that a lot more palatable, since you won't
normally have more than one exclusive waiter in the queue at a time.
But regardless of that, I don't think we can just say "oh, well,
sometimes the system becomes totally unresponsive for more than 30
seconds, but we don't care".  We have to care about that.

What I think we need to understand better from is Tsunakawa-san is (1)
whether this behavior still occurs in 9.6 and (2) what sort of test
case is required to produce it.  Depending on how extreme that test
case is, we can decide how much of a problem we think this is.  And we
can test potential solutions to see whether and to what degree they
address it, as well as how they affect throughput.

-- 
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] [PROPOSAL] Move all am-related reloption code into src/backend/access/[am-name] and get rid of relopt_kind

[Alvaro Herrera]

Nikolay Shaplov wrote:
> В письме от 25 мая 2016 13:25:38 Вы написали:
> > Teodor Sigaev wrote:
> > > >This all should me moved behind "access method" abstraction...
> > > 
> > > +1 relopt_kind should be moved in am, at least. Or removed.
> > 
> > Hm, but we have tablespace options too, so I'm not sure that using AM as
> > abstraction level is correct.
> We will use am for all indexes, and keep all the rest in relopotion.c for 
> non-indexes. May be divided options catalog into sections one section for 
> each 
> kind.

That makes sense.  I can review the patch later.

> And as I also would like to use this code for dynamic attoptions later, I 
> would like to remove relopt_kind at all. Because it spoils live in that case.

As I remember, Fabrízio (in CC) had a patch for dynamic reloptions, but
there was some problem with it and we dumped it; see
https://www.postgresql.org/message-id/CAFcNs+rqCq1H5eXW-cvdti6T-xo2STEkhjREx=odmakk5ti...@mail.gmail.com
for previous discussion.

-- 
Á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] Inheritance

[Jim Nasby]

On 5/24/16 8:52 PM, Craig Ringer wrote:
> Absolutely, and I use OO heavily. But a half-assed "object relational"
> feature in the database that only kind-of works isn't OO, and it's
> generally less useful than using existing relational-to-oo modelling
> techniques like FK-related 1:1 child tables for specialisation.

How is it less useful than that? To me, the FK "solution" is the 
absolute worst of everything: you still have all the separate child 
tables that you must explicitly query *and* you have to get all the 
joins correct as well. And hope it doesn't have horrible performance.



Note that there was enough enthusiasm to adopt whole new database
engines, but not enough to use PostgreSQL's existing features for that.
Partly because they suck. In particular, people looking for this tend to
want to be able to create new subtypes without having to mess around
making schema changes and modelling everything.


Which is a decision people have come to regret, because then your 
codebase somehow has to deal with 38 different versions of what a 
"customer" is.



There's much more future in improving document-structured storage like
jsonb, and possibly extending in future toward hybrid storage with some
normal cols and some dynamic cols, than with Pg's
pseudo-object-relational inheritance feature.


I don't see why we can't do both. There's many cases where more 
flexibility in what output tuples look like would be very valuable. The 
JSON features are one aspect; crosstab is another.


Postgres is well past the point where our relational features are the 
big selling point. It's now about scale, an incredibly robust storage 
engine, and all the extensiblity opportunities. We've moved from being 
an RDBMS to being a "Data Platform". Improving our OO capabilities just 
continues that.

--
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
855-TREBLE2 (855-873-2532)   mobile: 512-569-9461


--
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] Deleting prepared statements from libpq.

[Dmitry Igrishin]

2016-05-25 16:50 GMT+03:00 Tom Lane :

> Dmitry Igrishin  writes:
> > "there is no libpq function for deleting a prepared statement".
> > Could you tell me please, what is the reason for this?
>
> You don't really need one, since you can just use DEALLOCATE.
>
Yes, but there are also Parse(F) message, while anybody can just use
PREPARE.

-- 
// Dmitry.

Re: [HACKERS] Does people favor to have matrix data type?

[Jim Nasby]

On 5/25/16 7:46 AM, Kouhei Kaigai wrote:

My only concern is that domain type is not allowed to define type cast.
If we could add type cast on domain, we can define type transformation from
other array type to matrix.


I've actually wished for that in the past, as well as casting to 
compound types. Having those would make it easier to mock up a real data 
type for experimentation.


I strongly encourage you to talk to the MADlib community about 
first-class matrix support. They currently emulate matrices via arrays. 
I don't know offhand if they support NULLs in their regular matrices. 
They also support a sparsematrix "type" that is actually implemented as 
a table that contains coordinates and a value for each value in the 
matrix. Having that as a type might also be interesting (if you're 
sparse enough, that will be cheaper than the current NULL bitmap 
implementation).


Related to this, Tom has mentioned in the past that perhaps we should 
support abstract use of the [] construct. Currently point finds a way to 
make use of [], but I think that's actually coded into the grammar.

--
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
855-TREBLE2 (855-873-2532)   mobile: 512-569-9461


--
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] [BUGS] BUG #13473: VACUUM FREEZE mistakenly cancel standby sessions

[Marco Nenciarini]

On 27/06/15 01:13, Jim Nasby wrote:
> On 6/26/15 8:50 AM, Marco Nenciarini wrote:
>>> >In the heap_xlog_freeze we need to subtract one to the value of
>>> cutoff_xid
>>> >before passing it to ResolveRecoveryConflictWithSnapshot.
>>> >
>>> >
>>> >
>> Attached a proposed patch that solves the issue.
> 

I have hit the bug again, as it has been fixed only from 9.5+

The procedure to reproduce it sent in the original post is not fully
accurate, below there is one that always works:

Run the following operation on an idle cluster.

1) connect to the master and run the following script

   create table t(id int primary key);
   insert into t select generate_series(1, 1);

2) connect to the standby and simulate a long running query:

   select pg_sleep(3600);

3) on the master and run the following commands:
   vacuum freeze verbose t;
   drop table t;

4) after 30 seconds the pg_sleep query on standby will be canceled.

Attached there is a patch that apply on every version that misses the
fix (9.0, 9.1, 9.2, 9.3, 9.4)

Regards,
Marco

-- 
Marco Nenciarini - 2ndQuadrant Italy
PostgreSQL Training, Services and Support
marco.nenciar...@2ndquadrant.it | www.2ndQuadrant.it
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index eb8eada..434880a 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -4764,7 +4764,13 @@ heap_xlog_freeze(XLogRecPtr lsn, XLogRecord *record)
 	 * consider the frozen xids as running.
 	 */
 	if (InHotStandby)
-		ResolveRecoveryConflictWithSnapshot(cutoff_xid, xlrec->node);
+	{
+		TransactionId latestRemovedXid = cutoff_xid;
+
+		TransactionIdRetreat(latestRemovedXid);
+
+		ResolveRecoveryConflictWithSnapshot(latestRemovedXid, xlrec->node);
+	}
 
 	/* If we have a full-page image, restore it and we're done */
 	if (record->xl_info & XLR_BKP_BLOCK(0))



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] [PROPOSAL] Move all am-related reloption code into src/backend/access/[am-name] and get rid of relopt_kind

[Nikolay Shaplov]

В письме от 25 мая 2016 13:25:38 Вы написали:
> Teodor Sigaev wrote:
> > >This all should me moved behind "access method" abstraction...
> > 
> > +1 relopt_kind should be moved in am, at least. Or removed.
> 
> Hm, but we have tablespace options too, so I'm not sure that using AM as
> abstraction level is correct.
We will use am for all indexes, and keep all the rest in relopotion.c for 
non-indexes. May be divided options catalog into sections one section for each 
kind.

And as I also would like to use this code for dynamic attoptions later, I 
would like to remove relopt_kind at all. Because it spoils live in that case.

-- 
Nikolay Shaplov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company


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

Re: [HACKERS] [PROPOSAL] Move all am-related reloption code into src/backend/access/[am-name] and get rid of relopt_kind

[Alvaro Herrera]

Teodor Sigaev wrote:
> >This all should me moved behind "access method" abstraction...
> 
> +1 relopt_kind should be moved in am, at least. Or removed.

Hm, but we have tablespace options too, so I'm not sure that using AM as
abstraction level is correct.

-- 
Á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] To-Do item: skip table scan for adding column with provable check constraints

[Jim Nasby]

On 5/24/16 9:56 PM, Craig Ringer wrote:

On 25 May 2016 at 06:56, Tom Lane mailto:t...@sss.pgh.pa.us>> wrote:

ilm...@ilmari.org  (Dagfinn Ilmari
=?utf-8?Q?Manns=C3=A5ker?=) writes:
> Tom Lane mailto:t...@sss.pgh.pa.us>> writes:
>> ... and if the CHECK expression is immutable ...

> Doesn't it have to be already?

AFAIK we don't insist on that currently.  You could imagine useful
checks
that are not, for example CHECK(write_timestamp <= now()).


That seems like abuse of CHECK to me, and a job for a trigger. If anyone
proposed allowing that and it wasn't already allowed (or at least not
prohibited explicitly) it'd get shot down in flames.


Yeah, non-IMMUTABLE checks are a really bad idea, especially because 
they will only trip you up well after the fact (like when restoring from 
a dump).



If we wanted checks that apply only on row insert/update a CHECK WRITE
or similar would seem suitable; something that implies that it's an
_action_ taken on write and doesn't stop the constraint later becoming
violated by unrelated changes. Like a trigger. Such a check could be
allowed to use subqueries, reference other tables, call functions and
all the other fun stuff you're not meant to do in a CHECK constraint.
Like a trigger.

Or we could use triggers.


Rather than creating new CHECK syntax, I'd rather have a notion of 
"check triggers" that simply evaluate a boolean expression (and don't 
require defining a function).

--
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
855-TREBLE2 (855-873-2532)   mobile: 512-569-9461


--
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: [HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Windows service is not starting so there’s message in log: FATAL: "could not create shared memory segment “Global/PostgreSQL.851401618

[Michael Paquier]

On Wed, May 25, 2016 at 12:11 AM, Amit Kapila  wrote:
> On Tue, May 17, 2016 at 2:31 AM, Michael Paquier 
> wrote:
>>
>> On Tue, May 17, 2016 at 4:16 AM, Amit Kapila 
>> wrote:
>> > On Mon, May 16, 2016 at 9:45 AM, Michael Paquier
>> > 
>> > wrote:
>> >>
>> >> On Sun, May 15, 2016 at 3:34 PM, Amit Kapila 
>> >> wrote:
>> >> > Sounds sensible, but if we want to that route, shall we have some
>> >> > mechanism
>> >> > such that if retrying it for 10 times (10 is somewhat arbitrary, but
>> >> > we
>> >> > retry 10 times in PGSharedMemoryCreate, so may be there is some
>> >> > consistency)
>> >> > doesn't give us unique name and we are getting EACCES error, then
>> >> > just
>> >> > throw
>> >> > the error instead of more retries.  This is to ensure that if the API
>> >> > is
>> >> > returning EACCES due to reason other than duplicate handle, then we
>> >> > won't
>> >> > retry indefinitely.
>> >>
>> >> The logic in win32_shmem.c relies on the fact that a segment will be
>> >> recycled, and the retry is here because it may take time at OS level.
>> >> On top of that it relies on the segment names being unique across
>> >> systems. So it seems to me that it is not worth the complication to
>> >> duplicate that logic in the dsm implementation.
>> >
>> > If we don't do retry for fixed number of times, then how will we handle
>> > the
>> > case if EACCES is due to the reason other than duplicate handle?
>>
>> EACCES is a bit too low-level... I had in mind to check GetLastError
>> with only ERROR_ACCESS_DENIED, and retry only in this case, which is
>> the case where one postmaster is trying to access the segment of
>> another.
>>
>
> Okay, attached patch just does that and I have verified that it allows to
> start multiple services in windows.  In off list discussion with Robert, he
> suggested not to complicate the patch by retrying for fixed number of times
> as there is no proof that ERROR_ACCESS_DENIED can occur due to any other
> reason in this code path.  This patch is based on Kyotaro san's patch posted
> upthread with just minor changes in comments and indentation.

Thanks for catching Robert and getting confirmation on the matter. In
the same line of thoughts, I think as well that it is definitely not
worth complicating the retry logic in dsm.c, but you knew that already
per last week's discussion.

Regarding the patch, this looks good to me.
-- 
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] Does people favor to have matrix data type?

[Tom Lane]

Kouhei Kaigai  writes:
> The 'matrix' data type as domain type of real[] is an option to implement.
> We can define operators on the domain types, thus, it allows us to process
> large amount of calculation by one operation, in native binary speed.

Don't go that way, it will cause you nothing but pain.  The ambiguous-
operator resolution rules are not friendly at all to operators with domain
arguments; basically only exact matches will be recognized.

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] pg_dump -j against standbys

[Tom Lane]

Magnus Hagander  writes:
>> Also, why didn't you keep using ExecuteSqlQueryForSingleRow()?

> The reason I did that is that ExecuteSqlQueryForSingleRow() is a static
> method in pg_dump.c. I was planning to go back and review that, and
> consider moving it, but I forgot it :S

> I think the clean thing is probably to use that one, and also move it over
> to not be a static method in pg_dump.c, but instead sit next to
> ExecuteSqlQuery in pg_backup_db.c. Do you agree that's reasonable, and
> something that's OK to backpatch?

No objection here, since it wouldn't be exposed outside pg_dump in any
case.

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] Parallel pg_dump's error reporting doesn't work worth squat

[Tom Lane]

Kyotaro HORIGUCHI  writes:
> I tried it on Windows 7/64 but first of all, I'm surprised to see
> that the following command line gets an error but it would be
> fine because it is consistent with what is written in the manual.

> | >pg_dump postgres://horiguti:hoge@localhost/postgres --jobs=9 -Fd -f 
> testdump
> | pg_dump: too many command-line arguments (first is "--jobs=9")
> | Try "pg_dump --help" for more information.

Where do you see an example like that?  We should fix it.  The only case
that is certain to work is switches before non-switch arguments, and so
we should not give any documentation examples in the other order.  On a
platform using GNU getopt(), getopt() will rearrange the argv[] array to
make such cases work, but non-GNU getopt() doesn't do that (and I don't
really trust the GNU version not to get confused, either).

> I might be wrong with something, but pg_dump seems to have some
> issues in thread handling.

Wouldn't surprise me ... there's a lot of code in there depending on
static variables, and we've only tried to thread-proof a few.  Still,
I think that's a separate issue from what this patch is addressing.

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] [BUGS] BUG #14155: bloom index error with unlogged table

[Tom Lane]

Jeff Janes  writes:
> Given what a Bloom filter is/does, I'm having a hard time seeing how it
> makes much sense to support the boolean type.

> My biggest gripe with it at the moment is that the signature size should be
> expressed in bits, and then internally rounded up to a multiple of 16,
> rather than having it be expressed in 'uint16'.

> If that were done it would be easier to fix the documentation to be more
> understandable.

+1 ... that sort of definition seems much more future-proof, too.
IMO it's not too late to change this.  (We probably don't want to change
the on-disk representation of the reloptions, but we could convert from
bits to words in bloptions().)

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] pg_dump -j against standbys

[Marko Tiikkaja]

On 25/05/16 15:59, Magnus Hagander wrote:

On Tue, May 24, 2016 at 5:39 PM, Tom Lane  wrote:

This patch will cause pg_dump to fail entirely against pre-9.0 servers.
You need to execute the test conditionally.



Ugh. can I blame coding while too jetlagged?


You could try blaming Magnus.  Oh, wait..


.m


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

Re: [HACKERS] pg_dump -j against standbys

[Magnus Hagander]

On Tue, May 24, 2016 at 5:39 PM, Tom Lane  wrote:

> Magnus Hagander  writes:
> > I think the cleanest way to do it is to just track if it's a standby in
> the
> > AH struct as written.
>
> > Comments?
>
> This patch will cause pg_dump to fail entirely against pre-9.0 servers.
> You need to execute the test conditionally.
>

Ugh. can I blame coding while too jetlagged?



> Also, in view of that, this test
>
> -   if (fout->remoteVersion >= 9)
> +   if (fout->remoteVersion >= 9 && fout->isStandby)
>
> could be reduced to just "if (fout->isStandby)".  And the comment in
> front of it could stand some attention (possibly just replace it with
> the one that's currently within the inner if() ... actually, that
> comment should move to where you moved the test to, no?)
>

True. Will fix.



> Also, why didn't you keep using ExecuteSqlQueryForSingleRow()?
> As coded, you're losing a sanity check that the query gives exactly
> one row back.
>
>
The reason I did that is that ExecuteSqlQueryForSingleRow() is a static
method in pg_dump.c. I was planning to go back and review that, and
consider moving it, but I forgot it :S

I think the clean thing is probably to use that one, and also move it over
to not be a static method in pg_dump.c, but instead sit next to
ExecuteSqlQuery in pg_backup_db.c. Do you agree that's reasonable, and
something that's OK to backpatch?

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

Re: [HACKERS] Deleting prepared statements from libpq.

[Tom Lane]

Dmitry Igrishin  writes:
> "there is no libpq function for deleting a prepared statement".
> Could you tell me please, what is the reason for this?

You don't really need one, since you can just use DEALLOCATE.

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] Move all am-related reloption code into src/backend/access/[am-name] and get rid of relopt_kind

[Teodor Sigaev]

This all should me moved behind "access method" abstraction...


+1 relopt_kind should be moved in am, at least. Or removed.

--
Teodor Sigaev   E-mail: teo...@sigaev.ru
   WWW: http://www.sigaev.ru/


--
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] Does people favor to have matrix data type?

[k...@rice.edu]

On Wed, May 25, 2016 at 09:10:02AM +, Kouhei Kaigai wrote:
> > -Original Message-
> > From: Simon Riggs [mailto:si...@2ndquadrant.com]
> > Sent: Wednesday, May 25, 2016 4:39 PM
> > To: Kaigai Kouhei(海外 浩平)
> > Cc: pgsql-hackers@postgresql.org
> > Subject: Re: [HACKERS] Does people favor to have matrix data type?
> > 
> > On 25 May 2016 at 03:52, Kouhei Kaigai  wrote:
> > 
> > 
> > In a few days, I'm working for a data type that represents matrix in
> > mathematical area. Does people favor to have this data type in the core,
> > not only my extension?
> > 
> > 
> > If we understood the use case, it might help understand whether to include 
> > it or not.
> > 
> > Multi-dimensionality of arrays isn't always useful, so this could be good.
> >
> As you may expect, the reason why I've worked for matrix data type is one of
> the groundwork for GPU acceleration, but not limited to.
> 
> What I tried to do is in-database calculation of some analytic algorithm; not
> exporting entire dataset to client side.
> My first target is k-means clustering; often used to data mining.
> When we categorize N-items which have M-attributes into k-clusters, the master
> data can be shown in NxM matrix; that is equivalent to N vectors in 
> M-dimension.
> The cluster centroid is also located inside of the M-dimension space, so it
> can be shown in kxM matrix; that is equivalent to k vectors in M-dimension.
> The k-means algorithm requires to calculate the distance to any cluster 
> centroid
> for each items, thus, it produces Nxk matrix; that is usually called as 
> distance
> matrix. Next, it updates the cluster centroid using the distance matrix, then
> repeat the entire process until convergence.
> 
> The heart of workload is calculation of distance matrix. When I tried to write
> k-means algorithm using SQL + R, its performance was not sufficient (poor).
>   https://github.com/kaigai/toybox/blob/master/Rstat/pgsql-kmeans.r
> 
> If we would have native functions we can use instead of the complicated SQL
> expression, it will make sense for people who tries in-database analytics.
> 
> Also, fortunately, PostgreSQL's 2-D array format is binary compatible to BLAS
> library's requirement. It will allow GPU to process large matrix in HPC grade
> performance.
> 
> Thanks,
> --
> NEC Business Creation Division / PG-Strom Project
> KaiGai Kohei 

Hi,

Have you looked at Perl Data Language under pl/perl? It has pretty nice support
for matrix calculations:

http://pdl.perl.org

Regards,
Ken


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

[Merlin Moncure]

On Tue, May 24, 2016 at 9:47 PM, Craig Ringer  wrote:
> On 24 May 2016 at 22:45, Konstantin Knizhnik 
> wrote:
>>
>> There is one aspect of inheritance support which was not mentioned:
>> polymorphic queries.
>> Actually polymorphism is the fundamental feature of OOP, without it there
>> is no behavioral inheritance and inheritance can be considered just as
>> "syntax sugar" for sharing some common subset of attributes between tables.
>>
>> The main problem with supporting polymorphic queries is that SQL query
>> returns set of tuples, not set of objects.
>> So there is no nice way to return both instances of based and derived
>> tables. There are several alternatives
>> (for example return joined set of attributes in all derived tables,
>> leaving missed as NULLs) but none of them is good.
>
>
> Exactly. We have a sort-of-object-ish storage option, but none of the
> surrounding stuff to make it useful for actual OO / object-relational work.
>
> The "joined set of attributes" approach is exactly what ORMs already do, and
> many direct implementations of the same idea will use too. So we'd offer no
> advantage over what they already do in a way that works with multiple
> DBMSes, except we might be able to do it faster. Maybe.
>
> The lack of polymorphism is critical. It's not really usefully OO but it
> costs you important relational features if you use it. We have some very
> limited polymorphism in the sense that you can query the parent table and
> see rows in child tables, but you only get the subset of cols that exists at
> that level of the heirarchy.
>
> One thing I'd like to explore one day is a nice, user-friendly way to
> express "SELECT this row and the corresponding sets of rows from [these
> tables and their children in turn] as a structured object". Right now users
> have to write series of LEFT JOINs and de-duplicate the left-hand sides. Or
> do multiple queries (n+1 selects), possibly expensively with repeated join
> work involved. Or they have to write pretty baroque queries to construct a
> jsonb object with jsonb_agg with multiple levels of group-by in
> subqueries-in-from. We should be able to do this for them, so they can say
>
> SELECTOBJECT customer
>   CHILD JOIN invoice ON (customer.customer_id = invoice.customer_id AND
> invoice_date > $1)
>   CHILD JOIN invoiceline USING (invoice_id)
>   CHILD JOIN address USING (customer_id)
> WHERE customer.in_debt_collections;
>
> instead of the current nested mess of aggregation and subqueries needed,
> like:
>
> SELECT
> to_jsonb(customer) || (
>   SELECT jsonb_build_object('invoices', jsonb_agg(invoice_obj))
>   FROM (
> SELECT to_jsonb(invoice) || jsonb_build_object('invoice_lines',
> jsonb_agg(invoice_line))
> FROM invoice
> LEFT OUTER JOIN invoice_line ON (invoice.invoice_id =
> invoice_line.invoice_id)
> WHERE invoice.customer_id = customer.customer_id AND invoice_date >=
> current_date
> GROUP BY invoice.invoice_id
>   ) invoice_obj
> ) || (
>   SELECT jsonb_build_object('addresses', jsonb_agg(address))
>   FROM address
>   WHERE address.customer_id = customer.customer_id
> )
> FROM customer
> WHERE customer.in_debt_collections

Well, I don't know. There's a lot of ways to write that type of thing.
Personally, I tend to prefer to delay the serialization to json as
long as possible (although it's sometimes unavoidable) because it
keeps the query cleaner.  I also sometimes use the array() subquery
syntax for sake of brevity, but this query could be restructured to
use proper aggregation on all levels if you're concerned about
performance (this query would tend to underperform yours for very
large compositions because of the second subquery scan vs the hash
join OTOH, it's a faster serialization model).  I didn't test the
syntax, but you get the idea.

SELECT to_json(q)
FROM
(
  SELECT
c.*,
array(
  SELECT
i.*,
array(
  SELECT il
  FROM invoice_line il
  WHERE il.invoice_id = i.invoice_id
) AS invoice_lines
  FROM invoice i
  WHERE i.customer_id = c.customer_id AND invoice_date >= current_date
) AS invoices
  FROM customer c
  WHERE c.in_debt_collections
) q

The point is this: the postgresql type system is flexible enough that
you can do arbitrary constructions pretty easy and the situation has
been one of continuous improvement over the last several releases.  It
isn't perfect, but json enhancements FWICT have made syntactical
approaches to the problem a dead end; json gets the job done is less
likely to cause problems with the SQL standard down the road.  For the
same set of reasons I no longer use crosstab.

In the 15+ years I've been watching postgres inheritance has gone
precisely nowhere, and there other ways to do the things it can do
that also supply a much broader range of use cases.  Plus, I'm biased:
I happen to think the 90's OO style of inheritance is pretty dumb :-).

merlin


--

Re: [HACKERS] Does people favor to have matrix data type?

[Kouhei Kaigai]

> On Wed, May 25, 2016 at 10:38 AM, Simon Riggs  wrote:
> > On 25 May 2016 at 03:52, Kouhei Kaigai  wrote:
> >>
> >> In a few days, I'm working for a data type that represents matrix in
> >> mathematical area. Does people favor to have this data type in the core,
> >> not only my extension?
> >
> >
> > If we understood the use case, it might help understand whether to include
> > it or not.
> >
> > Multi-dimensionality of arrays isn't always useful, so this could be good.
> 
> Many natural language and image processing methods extract feature
> vectors that then use some simple distance metric, like dot product to
> calculate vector similarity. For example we presented a latent
> semantic analysis prototype at pgconf.eu 2015 that used real[] to
> store the features and a dotproduct(real[], real[]) real function to
> do similarity matching. However using real[] instead of a hypothetical
> realvector or realmatrix did not prove to be a huge overhead, so
> overall I'm on the fence for the usefulness of a special type. Maybe a
> helper function or two to validate the additional restrictions in a
> check constraint would be enough.
>
The 'matrix' data type as domain type of real[] is an option to implement.
We can define operators on the domain types, thus, it allows us to process
large amount of calculation by one operation, in native binary speed.

My only concern is that domain type is not allowed to define type cast.
If we could add type cast on domain, we can define type transformation from
other array type to matrix.

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] [BUGS] BUG #14155: bloom index error with unlogged table

[Jeff Janes]

On May 24, 2016 5:27 PM, "David G. Johnston" 
wrote:
>
> Moving my griping to -hackers only
>
> On Tue, May 24, 2016 at 8:08 PM, Tom Lane  wrote:
>>
>> dig...@126.com writes:
>> > postgres=# create unlogged table u_tbl (id int);
>> > CREATE TABLE
>> > postgres=# create index idx_u_tbl on u_tbl using bloom (id);
>> > ERROR:  index "idx_u_tbl" already contains data
>>
>> Yeah, it looks like nobody ever tested bloom's unlogged-index support;
>> it doesn't work or even come very close to working.  Will fix, thanks
>> for the report!
>
>
> ​I'll tack on my own gripe here, just because.
>
> It doesn't give me a lot of confidence in what was committed when the
summary sentence for the module says:
>
> "
> bloom is a module which implements an index access method. It comes as an
example of custom access methods and generic WAL records usage. But it is
also useful in itself.
> ​"​
>
>
> ​Honestly, as a user I couldn't care less that bloom is "an example
custom access method"​.  I want to know what it does and that it does so
reliably, and has a easy-to-use interface.  I complained earlier about its
lack of direct support for the boolean type.  Teodor's response on the
thread wasn't particularly encouraging:
>

Given what a Bloom filter is/does, I'm having a hard time seeing how it
makes much sense to support the boolean type.

My biggest gripe with it at the moment is that the signature size should be
expressed in bits, and then internally rounded up to a multiple of 16,
rather than having it be expressed in 'uint16'.

If that were done it would be easier to fix the documentation to be more
understandable.

On the positive side, I've done extensive crash-recovery testing (not with
unlogged tables, obviously) and that part seems solid.

Cheers,

Jeff

Re: [HACKERS] [sqlsmith] PANIC: failed to add BRIN tuple

[Andreas Seltenreich]

I wrote:

> Re-fuzzing now with your patch applied.

This so far yielded three BRIN core dumps on different machines with the
same backtraces.  Crash recovery fails in these cases.

I've put the data directory here (before recovery):

http://ansel.ydns.eu/~andreas/brincrash2-spidew.tar.xz (9.1M)

Corresponding backtraces of the backend and startup core files below.

regards,
Andreas

Core was generated by `postgres: smith brintest [local] UPDATE  
   '.
Program terminated with signal SIGABRT, Aborted.
#0  0x7f5a49a9c067 in __GI_raise (sig=sig@entry=6) at 
../nptl/sysdeps/unix/sysv/linux/raise.c:56
56  ../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  0x7f5a49a9c067 in __GI_raise (sig=sig@entry=6) at 
../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1  0x7f5a49a9d448 in __GI_abort () at abort.c:89
#2  0x007ec979 in errfinish (dummy=dummy@entry=0) at elog.c:557
#3  0x007f012c in elog_finish (elevel=elevel@entry=20, 
fmt=fmt@entry=0x989cf0 "incorrect index offsets supplied") at elog.c:1378
#4  0x006eda2f in PageIndexDeleteNoCompact 
(page=page@entry=0x7f5a48c6f200 "Y", itemnos=itemnos@entry=0x7ffe6d0d0abc, 
nitems=nitems@entry=1) at bufpage.c:1011
#5  0x00470119 in brin_doupdate (idxrel=0x1c5a530, pagesPerRange=1, 
revmap=0x91a7d90, heapBlk=2166, oldbuf=3782, oldoff=2, origtup=0x719db80, 
origsz=3656, newtup=0x754e188, newsz=3656, samepage=1 '\001') at 
brin_pageops.c:181
#6  0x0046e5db in brininsert (idxRel=0x1c5a530, values=0x6591, 
nulls=0x6 , 
heaptid=0x, heapRel=0x7f5a4a72f700, 
checkUnique=UNIQUE_CHECK_NO) at brin.c:244
#7  0x005d888f in ExecInsertIndexTuples (slot=0x91a4870, 
tupleid=0x91a7df4, estate=0x91b9b38, noDupErr=0 '\000', specConflict=0x0, 
arbiterIndexes=0x0) at execIndexing.c:383
#8  0x005f74e5 in ExecUpdate (tupleid=0x7ffe6d0d0ed0, oldtuple=0x6591, 
slot=0x91a4870, planSlot=0x, epqstate=0x7f5a4a72f700, 
estate=0x91b9b38, canSetTag=1 '\001') at nodeModifyTable.c:1015
#9  0x005f7b7c in ExecModifyTable (node=0x71861c0) at 
nodeModifyTable.c:1501
#10 0x005dd5e8 in ExecProcNode (node=node@entry=0x71861c0) at 
execProcnode.c:396
#11 0x005d963f in ExecutePlan (dest=0x17bb870, direction=, numberTuples=0, sendTuples=, operation=CMD_UPDATE, 
use_parallel_mode=, planstate=0x71861c0, estate=0x91b9b38) at 
execMain.c:1567
#12 standard_ExecutorRun (queryDesc=0x17bb908, direction=, 
count=0) at execMain.c:338
#13 0x006f74d9 in ProcessQuery (plan=, 
sourceText=0x1670e18 "update public.brintest set \n  charcol = null, \n  
namecol = pg_catalog.name(cast(null as character varying)\n), \n  int8col = 
null, \n  int2col = public.brintest.int2col, \n  textcol = null, \n  float4col 
= null, \n  float8col = cast(coalesce(null,\n\npublic.brintest.float8col) 
as double precision), \n  inetcol = public.brintest.inetcol, \n  cidrcol = 
public.brintest.cidrcol, \n  bpcharcol = null, \n  datecol = (select datecol 
from public.brintest limit 1 offset 25)\n, \n  timecol = (select timecol from 
public.brintest limit 1 offset 42)\n, \n  timetzcol = (select timetzcol from 
public.brintest limit 1 offset 3)\n, \n  numericcol = (select numericcol from 
public.brintest limit 1 offset 32)\n, \n  uuidcol = 
public.brintest.uuidcol\nreturning \n  public.brintest.datecol as c0;", 
params=0x0, dest=0x17bb870, completionTag=0x7ffe6d0d10a0 "") at pquery.c:185
#14 0x006f776f in PortalRunMulti (portal=portal@entry=0x1611b68, 
isTopLevel=isTopLevel@entry=1 '\001', dest=dest@entry=0x17bb870, 
altdest=0xc96680 , 
completionTag=completionTag@entry=0x7ffe6d0d10a0 "") at pquery.c:1267
#15 0x006f7a1c in FillPortalStore (portal=portal@entry=0x1611b68, 
isTopLevel=isTopLevel@entry=1 '\001') at pquery.c:1044
#16 0x006f846d in PortalRun (portal=0x1611b68, 
count=9223372036854775807, isTopLevel=, dest=0x756c870, 
altdest=0x756c870, completionTag=0x7ffe6d0d1450 "") at pquery.c:782
#17 0x006f5c73 in exec_simple_query (query_string=) at 
postgres.c:1094
#18 PostgresMain (argc=23141224, argv=0x2cab518, dbname=0x15f3578 "brintest", 
username=0x2cab570 "\030\265\312\002") at postgres.c:4059
#19 0x0046c8d2 in BackendRun (port=0x1618880) at postmaster.c:4258
#20 BackendStartup (port=0x1618880) at postmaster.c:3932
#21 ServerLoop () at postmaster.c:1690
#22 0x006907fe in PostmasterMain (argc=argc@entry=4, 
argv=argv@entry=0x15f2560) at postmaster.c:1298
#23 0x0046d82d in main (argc=4, argv=0x15f2560) at main.c:228
(gdb) frame 4
#4  0x006eda2f in PageIndexDeleteNoCompact 
(page=page@entry=0x7f5a48c6f200 "Y", itemnos=itemnos@entry=0x7ffe6d0d0abc, 
nitems=nitems@entry=1) at bufpage.c:1011
1011elog(ERROR, "incorrect index offsets supplied");
(gdb) list
1006}
1007}
1008
1009/* this will catch invalid or out-of-order itemnos[] */
1010 

Re: [HACKERS] Does people favor to have matrix data type?

[Kouhei Kaigai]

> -Original Message-
> From: Simon Riggs [mailto:si...@2ndquadrant.com]
> Sent: Wednesday, May 25, 2016 4:39 PM
> To: Kaigai Kouhei(海外 浩平)
> Cc: pgsql-hackers@postgresql.org
> Subject: Re: [HACKERS] Does people favor to have matrix data type?
> 
> On 25 May 2016 at 03:52, Kouhei Kaigai  wrote:
> 
> 
>   In a few days, I'm working for a data type that represents matrix in
>   mathematical area. Does people favor to have this data type in the core,
>   not only my extension?
> 
> 
> If we understood the use case, it might help understand whether to include it 
> or not.
> 
> Multi-dimensionality of arrays isn't always useful, so this could be good.
>
As you may expect, the reason why I've worked for matrix data type is one of
the groundwork for GPU acceleration, but not limited to.

What I tried to do is in-database calculation of some analytic algorithm; not
exporting entire dataset to client side.
My first target is k-means clustering; often used to data mining.
When we categorize N-items which have M-attributes into k-clusters, the master
data can be shown in NxM matrix; that is equivalent to N vectors in M-dimension.
The cluster centroid is also located inside of the M-dimension space, so it
can be shown in kxM matrix; that is equivalent to k vectors in M-dimension.
The k-means algorithm requires to calculate the distance to any cluster centroid
for each items, thus, it produces Nxk matrix; that is usually called as distance
matrix. Next, it updates the cluster centroid using the distance matrix, then
repeat the entire process until convergence.

The heart of workload is calculation of distance matrix. When I tried to write
k-means algorithm using SQL + R, its performance was not sufficient (poor).
  https://github.com/kaigai/toybox/blob/master/Rstat/pgsql-kmeans.r

If we would have native functions we can use instead of the complicated SQL
expression, it will make sense for people who tries in-database analytics.

Also, fortunately, PostgreSQL's 2-D array format is binary compatible to BLAS
library's requirement. It will allow GPU to process large matrix in HPC grade
performance.

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] Does people favor to have matrix data type?

[Ants Aasma]

On Wed, May 25, 2016 at 10:38 AM, Simon Riggs  wrote:
> On 25 May 2016 at 03:52, Kouhei Kaigai  wrote:
>>
>> In a few days, I'm working for a data type that represents matrix in
>> mathematical area. Does people favor to have this data type in the core,
>> not only my extension?
>
>
> If we understood the use case, it might help understand whether to include
> it or not.
>
> Multi-dimensionality of arrays isn't always useful, so this could be good.

Many natural language and image processing methods extract feature
vectors that then use some simple distance metric, like dot product to
calculate vector similarity. For example we presented a latent
semantic analysis prototype at pgconf.eu 2015 that used real[] to
store the features and a dotproduct(real[], real[]) real function to
do similarity matching. However using real[] instead of a hypothetical
realvector or realmatrix did not prove to be a huge overhead, so
overall I'm on the fence for the usefulness of a special type. Maybe a
helper function or two to validate the additional restrictions in a
check constraint would be enough.

Regards,
Ants Aasma


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

[HACKERS] Deleting prepared statements from libpq.

[Dmitry Igrishin]

Hello,

According to
https://www.postgresql.org/docs/current/static/protocol-flow.html#PROTOCOL-FLOW-EXT-QUERY
there are Close message for closing prepared statements or portals, but
according to
https://www.postgresql.org/docs/current/static/libpq-exec.html#LIBPQ-PQPREPARE
"there is no libpq function for deleting a prepared statement".

Could you tell me please, what is the reason for this?

-- 
// Dmitry.

Re: [HACKERS] Parallel pg_dump's error reporting doesn't work worth squat

[Kyotaro HORIGUCHI]

At Wed, 25 May 2016 00:15:57 -0400, Tom Lane  wrote in 
<3149.1464149...@sss.pgh.pa.us>
> Kyotaro HORIGUCHI  writes:
> > Shouldn't archive_close_connection have an assertion (si->AHX !=
> > NULL) instead of checking "if (si->AHX)" in each path?
> 
> I thought about that but didn't see any really good argument for it.
> It'd be making that function dependent on the current behavior of
> unrelated code, when it doesn't need to be.

It's also fine with me.

I tried it on Windows 7/64 but first of all, I'm surprised to see
that the following command line gets an error but it would be
fine because it is consistent with what is written in the manual.

| >pg_dump postgres://horiguti:hoge@localhost/postgres --jobs=9 -Fd -f testdump
| pg_dump: too many command-line arguments (first is "--jobs=9")
| Try "pg_dump --help" for more information.


Next, I got the following behavior for the following command,
then freeze. Maybe stopping at the same point with the next
paragraph but I'm not sure. The same thing occurs this patch on
top of the current master but doesn't on Linux.

| >pg_dump -d postgres --jobs=9 -Fd -f testdump
| Password: 
| pg_dump: [archiver] WARNING: requested compression not available in this 
installation -- archive will be uncompressed
| pg_dump: [compress_io] not built with zlib support
| pg_dump: [parallel archiver] a worker process died unexpectedly
| pg_dump: [archiver (db)] query failed: ERROR:  invalid snapshot identifier: 
"2299-1"
| pg_dump: [archiver (db)] query was: SET TRANSACTION SNAPSHOT '2299-1'
| pg_dump: [archiver (db)] query failed: ERROR:  invalid snapshot identifier: 
"2299-1"
| pg_dump: [archiver (db)] query was: SET TRANSACTION SNAPSHOT '2299-1'
| pg_dump: [archiver (db)] query failed: ERROR:  invalid snapshot identifier: 
"2299-1"
| pg_dump: [archiver (db)] query was: SET TRANSACTION SNAPSHOT '2299-1'
| pg_dump: [archiver (db)] query failed: ERROR:  invalid snapshot identifier: 
"2299-1"
| pg_dump: [archiver (db)] query was: SET TRANSACTION SNAPSHOT '2299-1'
| pg_dump: [archiver (db)] query failed: ERROR:  invalid snapshot identifier: 
"2299-1"
| pg_dump: [archiver (db)] query was: SET TRANSACTION SNAPSHOT '2299-1'
| pg_dump: [archiver (db)] query failed: ERROR:  invalid snapshot identifier: 
"2299-1"
| pg_dump: [archiver (db)] query was: SET TRANSACTION SNAPSHOT '2299-1'


Third, I'm not sure on this detail, but pg_dump shows the
following message then freeze until Ctrl-C. I think that I forgot
to set password to the user for the time. It doesn't seem to
occur for this patch on top of the current master.

| >pg_dump --jobs=9 -Fd -f testdump 
"postgres://horiguti:hoge@localhost/postgres"
| pg_dump: [archiver] WARNING: requested compression not available in this 
installation -- archive will be uncompressed
| pg_dump: [compress_io] not built with zlib support
| pg_dump: [parallel archiver] a worker process died unexpectedly
| ^C
| >

The main thread was stopping at WaitForMultiplObjects() around
parallel.c:361(@master + this patch) but I haven't investigated
it any more.


Finally, I got the following expected result, which seems sane.

| >pg_dump --jobs=9 -Fd -f testdump 
"postgres://horiguti:hoge@localhost/postgres"
| pg_dump: [archiver] WARNING: requested compression not available in this 
installation -- archive will be uncompressed
| pg_dump: [archiver (db)] connection to database "postgres" failed: 
fe_sendauth:no password supplied
| pg_dump: [parallel archiver] a worker process died unexpectedly
| pg_dump: [archiver (db)] connection to database "postgres" failed: 
fe_sendauth:no password supplied
| pg_dump: [archiver (db)] connection to database "postgres" failed: 
fe_sendauth:no password supplied
| pg_dump: [archiver (db)] connection to database "postgres" failed: 
fe_sendauth:no password supplied
| pg_dump: [archiver (db)] connection to database "postgres" failed: 
fe_sendauth:no password supplied
| pg_dump: [archiver (db)] connection to database "postgres" failed: 
fe_sendauth:no password supplied
| pg_dump: [archiver (db)] connection to database "postgres" failed: 
fe_sendauth:no password supplied
| pg_dump: [archiver (db)] connection to database "postgres" failed: 
fe_sendauth:no password supplied
| pg_dump: [archiver (db)] connection to database "postgres" failed: 
fe_sendauth:no password supplied


I might be wrong with something, but pg_dump seems to have some
issues in thread handling.

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] Does people favor to have matrix data type?

[Simon Riggs]

On 25 May 2016 at 03:52, Kouhei Kaigai  wrote:

> In a few days, I'm working for a data type that represents matrix in
> mathematical area. Does people favor to have this data type in the core,
> not only my extension?
>

If we understood the use case, it might help understand whether to include
it or not.

Multi-dimensionality of arrays isn't always useful, so this could be good.

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

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

[HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Windows service is not starting so there’s message in log: FATAL: "could not create shared memory segment “Global/PostgreSQL.851401618

[Amit Kapila]

On Tue, May 17, 2016 at 2:31 AM, Michael Paquier 
wrote:
>
> On Tue, May 17, 2016 at 4:16 AM, Amit Kapila 
wrote:
> > On Mon, May 16, 2016 at 9:45 AM, Michael Paquier <
michael.paqu...@gmail.com>
> > wrote:
> >>
> >> On Sun, May 15, 2016 at 3:34 PM, Amit Kapila 
> >> wrote:
> >> > Sounds sensible, but if we want to that route, shall we have some
> >> > mechanism
> >> > such that if retrying it for 10 times (10 is somewhat arbitrary, but
we
> >> > retry 10 times in PGSharedMemoryCreate, so may be there is some
> >> > consistency)
> >> > doesn't give us unique name and we are getting EACCES error, then
just
> >> > throw
> >> > the error instead of more retries.  This is to ensure that if the
API is
> >> > returning EACCES due to reason other than duplicate handle, then we
> >> > won't
> >> > retry indefinitely.
> >>
> >> The logic in win32_shmem.c relies on the fact that a segment will be
> >> recycled, and the retry is here because it may take time at OS level.
> >> On top of that it relies on the segment names being unique across
> >> systems. So it seems to me that it is not worth the complication to
> >> duplicate that logic in the dsm implementation.
> >
> > If we don't do retry for fixed number of times, then how will we handle
the
> > case if EACCES is due to the reason other than duplicate handle?
>
> EACCES is a bit too low-level... I had in mind to check GetLastError
> with only ERROR_ACCESS_DENIED, and retry only in this case, which is
> the case where one postmaster is trying to access the segment of
> another.
>

Okay, attached patch just does that and I have verified that it allows to
start multiple services in windows.  In off list discussion with Robert, he
suggested not to complicate the patch by retrying for fixed number of times
as there is no proof that ERROR_ACCESS_DENIED can occur due to any other
reason in this code path.  This patch is based on Kyotaro san's patch
posted upthread with just minor changes in comments and indentation.


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


dsm_win_segment_access_v1.patch
Description: Binary data

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