Re: [HACKERS] Parallel indicators not written by pg_get_functiondef

[Feike Steenbergen]

This patch is redundant as of
commit 2ac3be2e763d9b971352819f285dd51519e0aeb9

(Ashutosh Sharma diagnosed and patched the same problem)

On 15 April 2016 at 14:13, Michael Paquier 
wrote:

> On Fri, Apr 15, 2016 at 8:48 PM, Feike Steenbergen
>  wrote:
> > pg_get_functiondef(oid) does not return the PARALLEL indicators.
> >
> > Attached a small patch to have pg_get_functiondef actually add these
> > indicators, using commit 7aea8e4f2 (pg_dump.c) as a guide.
> >
> > The logic is the same as with the volatility: we only append non-default
> > indicators.
>
> +1 for a good catch. Your patch is correct.
> --
> Michael
>

Re: [HACKERS] Parallel build with MSVC

[Christian Ullrich]

* From: Michael Paquier [mailto:michael.paqu...@gmail.com]

> On Tue, Apr 26, 2016 at 10:09 PM, Christian Ullrich
>  wrote:

>  
> -build DEBUG
> +build -c DEBUG
>  
> To build just a single project, for example psql, run the commands:
>  
>  build psql
> -build DEBUG psql
> +build -c DEBUG psql
>  
>
> Why is that? Your patch just has a look at argv[0] to see if that's a
> debug or release build.

Sorry, forgot to fix that. I originally used Getopt in build.pl, then realized 
maintaining compatibility was more important.

Thanks for noticing; new patches attached; the second one is unmodified.

> +use File::Spec::Functions qw(splitpath catpath);
> This is present since at least perl 5.8, so that's not a blocker.
> 
> vcbuild also supports /m. Wouldn't it make sense to have a environment
> variable flag for it as well? vcbuild has been replaced by msbuild in
> VS2010 but I would think that in back-branches this would have value
> for users still compiling with VS2008 or older, and those are still
> supported things.

Good point, but I have no installation of 2008 around, so I cannot test it. 
Perhaps there is someone around who could do that (just add the $msbflags as 
the first argument to vcbuild)?

-- 
Christian



0001-Support-passing-arbitrary-arguments-to-MSBuild.patch
Description: 0001-Support-passing-arbitrary-arguments-to-MSBuild.patch


0002-Support-parallel-build-with-MSBuild.patch
Description: 0002-Support-parallel-build-with-MSBuild.patch

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

Re: [HACKERS] Parallel build with MSVC

[Michael Paquier]

On Wed, Apr 27, 2016 at 4:06 PM, Christian Ullrich  wrote:
> * From: Michael Paquier [mailto:michael.paqu...@gmail.com]
>> Why is that? Your patch just has a look at argv[0] to see if that's a
>> debug or release build.
>
> Sorry, forgot to fix that. I originally used Getopt in build.pl, then 
> realized maintaining compatibility was more important.
>
> Thanks for noticing; new patches attached; the second one is unmodified.

Thanks for the updated patches, those look good to me. The environment
flag is missing with vcbuild. you'd want to add that at the end.

>> +use File::Spec::Functions qw(splitpath catpath);
>> This is present since at least perl 5.8, so that's not a blocker.
>>
>> vcbuild also supports /m. Wouldn't it make sense to have a environment
>> variable flag for it as well? vcbuild has been replaced by msbuild in
>> VS2010 but I would think that in back-branches this would have value
>> for users still compiling with VS2008 or older, and those are still
>> supported things.
>
> Good point, but I have no installation of 2008 around, so I cannot test it. 
> Perhaps there is someone around who could do that (just add the $msbflags as 
> the first argument to vcbuild)?

I forgot that I have actually one! Bumping my Win7 VM CPU from 1 to 2,
and using VS2008 command prompt with vcbuild /m I am not seeing
issues. Moving symbols.out would be the main issue, but I am not
noticing problems related to that.
-- 
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] psql :: support for \ev viewname and \sv viewname

[Dean Rasheed]

[Looking back over old threads]

On 22 July 2015 at 22:00, Dean Rasheed  wrote:
> This appears to be missing support for view options (WITH CHECK OPTION
> and security_barrier), so editing a view with either of those options
> will cause them to be stripped off.

It seems like this issue was never addressed, and it needs to be fixed for 9.6.

Here is a rough patch based on the way pg_dump handles this. It still
needs a bit of polishing -- in particular I think fmtReloptionsArray()
(copied from pg_dump) should probably be moved to string_utils.c so
that it can be shared between pg_dump and psql. Also, I'm not sure
that's the best name for it -- I think appendReloptionsArray() is a
more accurate description of what is does.

Regards,
Dean
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
new file mode 100644
index 4fa7760..96bc64d
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -72,6 +72,7 @@ static bool lookup_object_oid(EditableOb
   Oid *obj_oid);
 static bool get_create_object_cmd(EditableObjectType obj_type, Oid oid,
 	  PQExpBuffer buf);
+static bool fmtReloptionsArray(PQExpBuffer buffer, const char *reloptions);
 static int	strip_lineno_from_objdesc(char *obj);
 static int	count_lines_in_buf(PQExpBuffer buf);
 static void print_with_linenumbers(FILE *output, char *lines,
@@ -3274,12 +3275,51 @@ get_create_object_cmd(EditableObjectType
 			 * CREATE for ourselves.  We must fully qualify the view name to
 			 * ensure the right view gets replaced.  Also, check relation kind
 			 * to be sure it's a view.
+			 *
+			 * Starting with 9.2, views may have reloptions (security_barrier)
+			 * and from 9.4 onwards they may also have WITH [LOCAL|CASCADED]
+			 * CHECK OPTION.  These are not part of the view definition
+			 * returned by pg_get_viewdef() and so need to be retrieved
+			 * separately.  Materialized views (introduced in 9.3) may have
+			 * arbitrary storage parameter reloptions.
 			 */
-			printfPQExpBuffer(query,
-			  "SELECT nspname, relname, relkind, pg_catalog.pg_get_viewdef(c.oid, true) FROM "
- "pg_catalog.pg_class c LEFT JOIN pg_catalog.pg_namespace n "
-			  "ON c.relnamespace = n.oid WHERE c.oid = %u",
-			  oid);
+			if (pset.sversion >= 90400)
+			{
+printfPQExpBuffer(query,
+  "SELECT nspname, relname, relkind, "
+  "pg_catalog.pg_get_viewdef(c.oid, true), "
+  "array_remove(array_remove(c.reloptions,'check_option=local'),'check_option=cascaded') AS reloptions, "
+  "CASE WHEN 'check_option=local' = ANY (c.reloptions) THEN 'LOCAL'::text "
+  "WHEN 'check_option=cascaded' = ANY (c.reloptions) THEN 'CASCADED'::text ELSE NULL END AS checkoption "
+  "FROM pg_catalog.pg_class c "
+  "LEFT JOIN pg_catalog.pg_namespace n "
+"ON c.relnamespace = n.oid WHERE c.oid = %u",
+  oid);
+			}
+			else if (pset.sversion >= 90200)
+			{
+printfPQExpBuffer(query,
+  "SELECT nspname, relname, relkind, "
+  "pg_catalog.pg_get_viewdef(c.oid, true), "
+  "c.reloptions AS reloptions, "
+  "NULL AS checkoption "
+  "FROM pg_catalog.pg_class c "
+  "LEFT JOIN pg_catalog.pg_namespace n "
+"ON c.relnamespace = n.oid WHERE c.oid = %u",
+  oid);
+			}
+			else
+			{
+printfPQExpBuffer(query,
+  "SELECT nspname, relname, relkind, "
+  "pg_catalog.pg_get_viewdef(c.oid, true), "
+  "NULL AS reloptions, "
+  "NULL AS checkoption "
+  "FROM pg_catalog.pg_class c "
+  "LEFT JOIN pg_catalog.pg_namespace n "
+"ON c.relnamespace = n.oid WHERE c.oid = %u",
+  oid);
+			}
 			break;
 	}
 
@@ -3304,6 +3344,8 @@ get_create_object_cmd(EditableObjectType
 	char	   *relname = PQgetvalue(res, 0, 1);
 	char	   *relkind = PQgetvalue(res, 0, 2);
 	char	   *viewdef = PQgetvalue(res, 0, 3);
+	char	   *reloptions = PQgetvalue(res, 0, 4);
+	char	   *checkoption = PQgetvalue(res, 0, 5);
 
 	/*
 	 * If the backend ever supports CREATE OR REPLACE
@@ -3328,11 +3370,30 @@ get_create_object_cmd(EditableObjectType
 			break;
 	}
 	appendPQExpBuffer(buf, "%s.", fmtId(nspname));
-	appendPQExpBuffer(buf, "%s AS\n", fmtId(relname));
-	appendPQExpBufferStr(buf, viewdef);
+	appendPQExpBufferStr(buf, fmtId(relname));
+
+	/* reloptions, if not an empty array "{}" */
+	if (reloptions != NULL && strlen(reloptions) > 2)
+	{
+		appendPQExpBufferStr(buf, "\n WITH (");
+		if (!fmtReloptionsArray(buf, reloptions))
+		{
+			psql_error("Could not parse reloptions array\n");
+			result = false;
+		}
+		appendPQExpBufferStr(buf, ")");
+	}
+
+	/* View definition from pg_get_viewdef (a SELECT query) */
+	appendPQExpBuffer(buf, " AS\n%s", viewdef);
+
 	/* Get rid of the semicolon that pg_get_viewdef appends */
 	if (buf->len > 0 && buf->data[buf->len - 1] == ';')
 		buf->data[--(buf->len)] = '

Re: [HACKERS] Parallel build with MSVC

[Christian Ullrich]

* From: Michael Paquier [mailto:michael.paqu...@gmail.com]

> On Wed, Apr 27, 2016 at 4:06 PM, Christian Ullrich 
> wrote:

> > * From: Michael Paquier [mailto:michael.paqu...@gmail.com]

> >> vcbuild also supports /m. Wouldn't it make sense to have a environment
> >> variable flag for it as well? vcbuild has been replaced by msbuild in
> >> VS2010 but I would think that in back-branches this would have value
> >> for users still compiling with VS2008 or older, and those are still
> >> supported things.
> >
> > Good point, but I have no installation of 2008 around, so I cannot test
> > it. Perhaps there is someone around who could do that (just add the
> > $msbflags as the first argument to vcbuild)?
> 
> I forgot that I have actually one! Bumping my Win7 VM CPU from 1 to 2,
> and using VS2008 command prompt with vcbuild /m I am not seeing
> issues. Moving symbols.out would be the main issue, but I am not
> noticing problems related to that.

OK then, hopefully last round attached.

-- 
Christian



0001-Support-passing-arbitrary-arguments-to-MSBuild-VCBui.patch
Description: 0001-Support-passing-arbitrary-arguments-to-MSBuild-VCBui.patch


0002-Make-gendef.pl-work-in-MSVC-parallel-build.patch
Description: 0002-Make-gendef.pl-work-in-MSVC-parallel-build.patch

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

[HACKERS] 9.6 and fsync=off

[Craig Ringer]

Hi all

After helping clean up the mess from another user who turned fsync=off
because they read (bad) tuning advice that it was faster, I'd really like
to change the config file comment.

Really.

#fsync = on # turns forced synchronization on
or off

Now, we can't rename fsync to disable_crash_safety=on or
corrupt_my_database=on. But the comment needs changing.

How about:

#fsync = on # force disk flushes required for
crash safety

or, preferably something like:

"Enable forced disk flushes when they are required for crash safety.
Disabling fsync can lead to unrecoverable database corruption in a crash of
the host system."

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

Re: [HACKERS] 9.6 and fsync=off

[Abhijit Menon-Sen]

At 2016-04-27 17:58:08 +0800, cr...@2ndquadrant.com wrote:
>
> #fsync = on # turns forced synchronization on or 
> off

I suggest:# provide crash safety by flushing 
disk writes
  # (Disabling this can lead to 
unrecoverable data
  # loss if the system crashes.)

-- Abhijit


-- 
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] 9.6 and fsync=off

[Magnus Hagander]

On Wed, Apr 27, 2016 at 12:43 PM, Abhijit Menon-Sen 
wrote:

> At 2016-04-27 17:58:08 +0800, cr...@2ndquadrant.com wrote:
> >
> > #fsync = on # turns forced synchronization
> on or off
>
> I suggest:# provide crash safety by
> flushing disk writes
>   # (Disabling this can lead to
> unrecoverable data
>   # loss if the system crashes.)
>

+1 for the change. I suggest shortening it to just "disabling this can lead
to unrecoverable data corruption" (I think corruption is better than loss,
mainly because too many people equate loss with "i may loose my last 10
updates, and I'm fine with that).

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

Re: [HACKERS] 9.6 and fsync=off

[Petr Jelinek]

On 27/04/16 12:53, Magnus Hagander wrote:



On Wed, Apr 27, 2016 at 12:43 PM, Abhijit Menon-Sen mailto:a...@2ndquadrant.com>> wrote:

At 2016-04-27 17:58:08 +0800, cr...@2ndquadrant.com
 wrote:
>
> #fsync = on # turns forced synchronization on 
or off

I suggest:# provide crash safety by
flushing disk writes
   # (Disabling this can
lead to unrecoverable data
   # loss if the system
crashes.)


+1 for the change. I suggest shortening it to just "disabling this can
lead to unrecoverable data corruption" (I think corruption is better
than loss, mainly because too many people equate loss with "i may loose
my last 10 updates, and I'm fine with that).



+1 (Abhijit's wording with data loss changed to data corruption)

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


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

Re: [HACKERS] 9.6 and fsync=off

[Abhijit Menon-Sen]

Here's a patch just to help things along.

-- Abhijit
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index f3e3de0..353de2e 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -182,7 +182,9 @@
 
 #wal_level = minimal			# minimal, replica, or logical
 	# (change requires restart)
-#fsync = on# turns forced synchronization on or off
+#fsync = on# provide crash safety by flushing disk writes
+	# (disabling this can lead to unrecoverable
+	# data corruption)
 #synchronous_commit = on		# synchronization level;
 	# off, local, remote_write, remote_apply, or on
 #wal_sync_method = fsync		# the default is the first option

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

[HACKERS] Processes and caches in postgresql

[Anastasia Lubennikova]

Hi, hackers.
There's a couple of questions about processes.

I found EXEC_BACKEND flag, while reading the code.
As I understood, it exists because we have to emulate fork() on WIN32.
And also it allows to debug the same behavior on Linux.
Is it right? Are there any other use cases?

Another question is about "--fork" argument (see code below).
I didn't find it in documentation, so I'm a bit confused.
I wonder if it exists only for internal purposes?

#ifdef EXEC_BACKEND
if (argc > 1 && strncmp(argv[1], "--fork", 6) == 0)
SubPostmasterMain(argc, argv);/* does not return */
#endif

And the last, but not least. Do we have any 
presentations/articles/READMEs/whatever

about caches (src/backend/utils/cache/*) in postgresql?
I found nothing, besides comments in the code.

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



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

[HACKERS] Re: [COMMITTERS] Re: pgsql: Convert contrib/seg's bool-returning SQL functions to V1 call co

[Robert Haas]

On Tue, Apr 26, 2016 at 10:59 PM, Tom Lane  wrote:
> Andres Freund  writes:
>> I don't understand why we don't just drop V0. It makes debugging harder,
>> exploitation easier (call arbitrary functions), and really has no
>> features making it desirable.
>
> What's the argument that it makes debugging harder?  Especially if
> you aren't using it?
>
> I don't particularly buy the "easier exploitation" argument, either.
> You can't create a C function without superuser, and if you've got
> superuser there are plenty of ways to run arbitrary code.
>
> I'd agree that there are no desirable features that would motivate
> writing new code in V0.  But that's not the reason for keeping it;
> the reason for keeping it is to avoid unnecessarily breaking
> existing extension code.

I don't think that argument holds much water any more.  The V1
interface was added in 0a7fb4e9184539afcb6fed0f1d2bc0abddc2b0a6, more
than 15 years ago.  Anybody who has extension code that old that still
does anything useful and hasn't needed much bigger changes that
conversion to V1 calling convention deserves a medal.  But more than
that, it's unreasonable to expect 15-year-plus deprecation windows for
features that are only exposed via C.  If we stuck to that rigidly it
would be a major impediment to forward progress.

Let's add a GUC allow_oldstyle_functions with a default of off, and
make fetch_finfo_record() throw an ERROR in the infofunc == NULL case
unless allow_oldstyle_functions = on.  That way, anybody who still
really wants to use V0 can do so.  For everybody else, the very first
attempt to execute a function where you've forgotten or fat-fingered
the PG_FUNCTION_INFO_V1 decoration will produce a clear error message
telling you exactly what you did wrong.  I confidently predict a lot
more people will be happy about that development than will be sad
about having to set a GUC to make their V0 functions work.

-- 
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] pg_dump dump catalog ACLs

[Stephen Frost]

Noah,

On Wednesday, April 27, 2016, Noah Misch  wrote:

> A later thought:
>
> On Mon, Apr 25, 2016 at 12:39:09AM -0400, Stephen Frost wrote:
> > src/bin/pg_dump: make check
> >
> > implemented using the TAP testing system.  There are a total of 360
> > tests, generally covering:
> >
> > Various invalid sets of command-line options.
> >
> > Valid command-line options (generally independently used):
> >
> >   (no options- defaults)
> >   --clean
> >   --clean --if-exists
> >   --data-only
> >   --format=c (tested with pg_restore)
> >   --format=d (tested with pg_restore)
> >   --format=t (tested with pg_restore)
> >   --format=d --jobs=2 (tested with pg_restore)
> >   --exclude-schema
> >   --exclude-table
> >   --no-privileges
> >   --no-owner
> >   --schema
> >   --schema-only
> >
> > Note that this is only including tests for basic schemas, tables, data,
> > and privileges, so far.  I believe it's pretty obvious that we want to
> > include all object types and include testing of extensions, I just
> > haven't gotten there yet and figured now would be a good time to solicit
> > feedback on the approach I've used.
> >
> > I'm not sure how valuable it is to test all the different combinations
> > of command-line options, though clearly some should be tested (eg:
> > include-schema, exclude table in that schema, and similar cases).
>
> You mention that you test valid options independently.  Keep an eye out for
> good opportunities to save runtime by testing multiple options per
> invocation.
> To give one example, --exclude-table seems fairly independent of --format;
> maybe those could test as a group.  That complicates the suite, but saving
> ten
> or more seconds might justify the complexity.
>

That thought had occurred to me also and I began combining some options,
though I could probably do more to reduce the runtime:

The current challenge I've been trying to find a solution to is testing the
extension handling. Only core extensions (plpgsql, plperl, etc) are
available when the TAP tests are running for pg_dump, it seems. I certainly
don't want to add a testing extension in such a way that it would get
installed for users, but it doesn't seem obvious how to create an extension
from another directory or to even find the directory where I could write
the control and SQL files into to use.

Any thoughts on that would certainly be welcome.

Thanks!

Stephen

Re: [HACKERS] Add jsonb_compact(...) for whitespace-free jsonb to text

[Dave Cramer]

On 26 April 2016 at 12:49, Stephen Frost  wrote:

> * Ryan Pedela (rped...@datalanche.com) wrote:
> > On Sun, Apr 24, 2016 at 4:02 PM, Sehrope Sarkuni 
> wrote:
> > > The default text representation of jsonb adds whitespace in between
> > > key/value pairs (after the colon ":") and after successive properties
> > > (after the comma ","):
>
> [...]
>
> > > It'd be nice to have a stable text representation of a jsonb value with
> > > minimal whitespace. The latter would also save a few bytes per record
> in
> > > text output formats, on the wire, and in backups (ex: COPY ... TO
> STDOUT).
> >
> > +1
> >
> > I cannot comment on the patch itself, but I welcome jsonb_compact() or
> some
> > way to get JSON with no inserted whitespace.
>
> As I mentioned to Sehrope on IRC, at least for my 2c, if you want a
> compact JSON format to reduce the amount of traffic over the wire or to
> do things with on the client side, we should probably come up with a
> binary format, rather than just hack out the whitespace.  It's not like
> representing numbers using ASCII characters is terribly efficient
> either.
>
>
+1



Dave Cramer

da...@postgresintl.com
www.postgresintl.com

Re: [HACKERS] Add jsonb_compact(...) for whitespace-free jsonb to text

[Merlin Moncure]

On Tue, Apr 26, 2016 at 11:49 AM, Stephen Frost  wrote:
> * Ryan Pedela (rped...@datalanche.com) wrote:
>> On Sun, Apr 24, 2016 at 4:02 PM, Sehrope Sarkuni  wrote:
>> > The default text representation of jsonb adds whitespace in between
>> > key/value pairs (after the colon ":") and after successive properties
>> > (after the comma ","):
>
> [...]
>
>> > It'd be nice to have a stable text representation of a jsonb value with
>> > minimal whitespace. The latter would also save a few bytes per record in
>> > text output formats, on the wire, and in backups (ex: COPY ... TO STDOUT).
>>
>> +1
>>
>> I cannot comment on the patch itself, but I welcome jsonb_compact() or some
>> way to get JSON with no inserted whitespace.
>
> As I mentioned to Sehrope on IRC, at least for my 2c, if you want a
> compact JSON format to reduce the amount of traffic over the wire or to
> do things with on the client side, we should probably come up with a
> binary format, rather than just hack out the whitespace.  It's not like
> representing numbers using ASCII characters is terribly efficient
> either.

-1

This will benefit pretty much nobody unless you are writing a hand
crafted C application that consumes and processes the data directly.
I'd venture to guess this is a tiny fraction of pg users these days.
I do not understand at all the objection to removing whitespace.
Extra whitespace does nothing but pad the document as humans will
always run the document through a prettifier tuned to their specific
requirements (generally starting with, intelligent placement of
newlines) if reading directly.

Also, binary formats are not always smaller than text formats.

> Compression might be another option, though that's certainly less
> flexible and only (easily) used in combination with SSL, today.

right, exactly.

merlin


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

Re: [HACKERS] 9.6 and fsync=off

[Tom Lane]

Petr Jelinek  writes:
> +1 (Abhijit's wording with data loss changed to data corruption)

I'd suggest something like

#fsync = on # flush data to disk for crash safety
# (turning this off can cause
# unrecoverable data corruption!)

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] Re: [COMMITTERS] Re: pgsql: Convert contrib/seg's bool-returning SQL functions to V1 call co

[Tom Lane]

Robert Haas  writes:
> Let's add a GUC allow_oldstyle_functions with a default of off, and
> make fetch_finfo_record() throw an ERROR in the infofunc == NULL case
> unless allow_oldstyle_functions = on.

This is not about "V0 calling convention is awful".  It isn't, really,
unless you need to deal with nulls.  This is about "allowing the finfo
record to be missing is error-prone".  What about inventing a
PG_FUNCTION_INFO_V0() macro, and then defining the new GUC as "must
find a matching finfo record"?  That would present less conversion
work for people who still have V0-style functions (and I'm sure there
are more than a few of them out there).

Now, if VS2015 actually has broken bool-returning V0, the argument for
keeping it going becomes a lot weaker.  But at this point I suspect
strongly that that's not what happened but rather we've got a faulty
bool cast there, which if true is something we need to fix regardless
of any considerations about V0.  Would someone please try the experiment
requested upthread?

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] Re: [COMMITTERS] Re: pgsql: Convert contrib/seg's bool-returning SQL functions to V1 call co

[Robert Haas]

On Wed, Apr 27, 2016 at 9:53 AM, Tom Lane  wrote:
> Robert Haas  writes:
>> Let's add a GUC allow_oldstyle_functions with a default of off, and
>> make fetch_finfo_record() throw an ERROR in the infofunc == NULL case
>> unless allow_oldstyle_functions = on.
>
> This is not about "V0 calling convention is awful".  It isn't, really,
> unless you need to deal with nulls.  This is about "allowing the finfo
> record to be missing is error-prone".  What about inventing a
> PG_FUNCTION_INFO_V0() macro, and then defining the new GUC as "must
> find a matching finfo record"?  That would present less conversion
> work for people who still have V0-style functions (and I'm sure there
> are more than a few of them out there).

I'm not, but I don't have a problem with PG_FUNCTION_INFO_V0.

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

[Bruce Momjian]

On Wed, Apr 27, 2016 at 10:51:55AM -0400, Robert Haas wrote:
> I think it's about time for us to run pgindent.  I did a trial run
> today of pgindent today and came up with the attached patch for
> typedefs.list, which I'd like to commit more or less immediately,
> barring objections.  It mostly just adds new typedefs that have
> appeared over the last year, but it also realphabetizes the file -
> some things that were added incrementally seem to have ended up in
> what is, at least according to what sort likes to do on my machine,
> the wrong place in the file.
> 
> With this applied, I get a fairly clean pgindent run.  There are some
> problems with comments getting mangled, and in a couple of cases
> function definitions getting mangled, that need more investigation.
> I'll try to find time to look into that soon and follow up, unless
> somebody else beats me to it.  As far as possible, I think it's
> desirable to clean up those things before rather than after running
> pgindent, because unmangling ASCII art that pgindent has stepped on is
> a thankless chore.

Agreed.  Sounds like a good plan --- a better plan than I have used in
the past.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +


-- 
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] Removing faulty hyperLogLog merge function

[Robert Haas]

On Wed, Apr 27, 2016 at 1:37 AM, Michael Paquier
 wrote:
>> I haven't followed this issue all that closely, but to me it seems
>> pretty clear. If the function is brand new to 9.6, buggy, and not even
>> used anywhere, I cannot imagine why we would leave it in the tree.
>
> +1. We should definitely not encourage its use for 3rd-part plugins.

So, now there's clearly a consensus.  Committed.  :-)

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

[HACKERS] pgindent

[Robert Haas]

I think it's about time for us to run pgindent.  I did a trial run
today of pgindent today and came up with the attached patch for
typedefs.list, which I'd like to commit more or less immediately,
barring objections.  It mostly just adds new typedefs that have
appeared over the last year, but it also realphabetizes the file -
some things that were added incrementally seem to have ended up in
what is, at least according to what sort likes to do on my machine,
the wrong place in the file.

With this applied, I get a fairly clean pgindent run.  There are some
problems with comments getting mangled, and in a couple of cases
function definitions getting mangled, that need more investigation.
I'll try to find time to look into that soon and follow up, unless
somebody else beats me to it.  As far as possible, I think it's
desirable to clean up those things before rather than after running
pgindent, because unmangling ASCII art that pgindent has stepped on is
a thankless chore.

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


typedefs-cleanup.patch
Description: application/download

-- 
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] 9.6 and fsync=off

[Craig Ringer]

On 27 April 2016 at 21:44, Tom Lane  wrote:

> Petr Jelinek  writes:
> > +1 (Abhijit's wording with data loss changed to data corruption)
>
> I'd suggest something like
>
> #fsync = on # flush data to disk for crash
> safety
> # (turning this off can cause
> # unrecoverable data corruption!)
>
>
Looks good.

The docs on fsync are already good, it's just a matter of making people
think twice and actually look at them.

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

Re: [HACKERS] Support for N synchronous standby servers - take 2

[Tom Lane]

Kyotaro HORIGUCHI  writes:
> Sorry, I have attached an empty patch. This is another one that should
> be with content.

I started to review this, and in passing came across this gem in
syncrep_scanner.l:


/*
  * flex emits a yy_fatal_error() function that it calls in response to
  * critical errors like malloc failure, file I/O errors, and detection of
  * internal inconsistency.  That function prints a message and calls exit().
  * Mutate it to instead call ereport(FATAL), which terminates this process.
  *
  * The process that causes this fatal error should be terminated.
  * Otherwise it has to abandon the new setting value of
  * synchronous_standby_names and keep running with the previous one
  * while the other processes switch to the new one.
  * This inconsistency of the setting that each process is based on
  * can cause a serious problem. Though it's basically not good idea to
  * use FATAL here because it can take down the postmaster,
  * we should do that in order to avoid such an inconsistency.
  */
#undef fprintf
#define fprintf(file, fmt, msg) syncrep_flex_fatal(fmt, msg)

static void
syncrep_flex_fatal(const char *fmt, const char *msg)
{
ereport(FATAL, (errmsg_internal("%s", msg)));
}


This is the faultiest reasoning possible.  There are a hundred reasons why
a process might fail to absorb a GUC setting, and causing just one such
code path to FATAL out is not going to improve system stability one bit.

If you think it is absolutely imperative that all processes in the system
have identical synchronous_standby_names settings, then we need to make
it be PGC_POSTMASTER, not indulge in half-baked non-solutions like this.
But I'd like to know why that is so essential.  It looks to me like what
matters is only whether each individual walsender thinks its client is
a sync standby, and so inconsistent settings between different walsenders
don't really matter.  Which is a good thing, because if it's to remain
SIGHUP, you can't promise that they'll all absorb a new value at the same
instant anyway.

In short, I don't see any good reason not to make this be a plain ERROR
like it is in every other scanner in the backend.

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

[Andres Freund]

On 2016-04-27 10:51:55 -0400, Robert Haas wrote:
> I think it's about time for us to run pgindent.  Sounds reasonable.

> I did a trial run
> today of pgindent today and came up with the attached patch for
> typedefs.list, which I'd like to commit more or less immediately,
> barring objections.

Yes, that makes sense. That way other can easily look at "their" code,
to see whether it can be made more pgindent resistant ;)

> It mostly just adds new typedefs that have
> appeared over the last year, but it also realphabetizes the file -
> some things that were added incrementally seem to have ended up in
> what is, at least according to what sort likes to do on my machine,
> the wrong place in the file.

Is it just me, or is the sort order in that file a bit confusing? The
whole thing about upper and lower case being separated seems to make it
much harder than necessary to manually insert something in the right
place..

- 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] Suspicious behaviour on applying XLOG_HEAP2_VISIBLE.

[Andres Freund]

On 2016-04-27 14:27:33 +0900, Michael Paquier wrote:
> On Wed, Apr 27, 2016 at 12:25 AM, Robert Haas  wrote:
> > In other words, I think Masahiko Sawada's patch in the original post
> > is pretty much right on target, except that we don't need to do that
> > always, but rather only in the FPI case when the call to
> > visibilitymap_pin() is being optimized away.  If we solve the problem
> > that way, I don't think we even need a new WAL record for this case,
> > which is a non-trivial fringe benefit.
> 
> The visibility map is not the only thing that need to be addressed,
> no?

If I understand Robert correctly his point is about fixing the smgr
inval alone - without the invalidation fix that'd not be enough because
the relcache info with outdated information (particularly relallvisible
et al), would continue to be valid. Relcache invalidations imply an smgr
one, but not the other way round.

The reason the fix for nmsg > 0 && !markXidCommitted isn't entirely
sufficient is because the smgr invalidation isn't transaction bound,
i.e. sent out immediately. So, to have the same behaviour on master/HS,
we need a way to send out the invalidiation properly in lockstep with
replay.

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

[Robert Haas]

On Wed, Apr 27, 2016 at 11:45 AM, Andres Freund  wrote:
> Yes, that makes sense. That way other can easily look at "their" code,
> to see whether it can be made more pgindent resistant ;)

Right.

>> It mostly just adds new typedefs that have
>> appeared over the last year, but it also realphabetizes the file -
>> some things that were added incrementally seem to have ended up in
>> what is, at least according to what sort likes to do on my machine,
>> the wrong place in the file.
>
> Is it just me, or is the sort order in that file a bit confusing? The
> whole thing about upper and lower case being separated seems to make it
> much harder than necessary to manually insert something in the right
> place..

Except for recently-manually-added entries, it seems to match what
sort wants to do on my system exactly.  Which seems good.

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

[Robert Haas]

On Wed, Apr 27, 2016 at 10:57 AM, Bruce Momjian  wrote:
> Agreed.  Sounds like a good plan --- a better plan than I have used in
> the past.

Thinking about this a bit more, what I am inclined to do is:

1. Run pgindent.

2. Read the diff and revert any changes that look icky.

3. Commit the rest.

4. Run pgindent again and post the diff.  Discuss how to fix the
ickiness contained therein, then proceed accordingly.

How does that sound?

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

[Bruce Momjian]

On Wed, Apr 27, 2016 at 11:51:42AM -0400, Robert Haas wrote:
> >> It mostly just adds new typedefs that have
> >> appeared over the last year, but it also realphabetizes the file -
> >> some things that were added incrementally seem to have ended up in
> >> what is, at least according to what sort likes to do on my machine,
> >> the wrong place in the file.
> >
> > Is it just me, or is the sort order in that file a bit confusing? The
> > whole thing about upper and lower case being separated seems to make it
> > much harder than necessary to manually insert something in the right
> > place..
> 
> Except for recently-manually-added entries, it seems to match what
> sort wants to do on my system exactly.  Which seems good.

Well, sort ordering is all related to your collation setting:

$ echo "a
> A" | LC_COLLATE="" sort
a
A

$ echo "a
A" | LC_COLLATE="en_US" sort
A
a

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +


-- 
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] Suspicious behaviour on applying XLOG_HEAP2_VISIBLE.

[Robert Haas]

On Wed, Apr 27, 2016 at 11:51 AM, Andres Freund  wrote:
> On 2016-04-27 14:27:33 +0900, Michael Paquier wrote:
>> On Wed, Apr 27, 2016 at 12:25 AM, Robert Haas  wrote:
>> > In other words, I think Masahiko Sawada's patch in the original post
>> > is pretty much right on target, except that we don't need to do that
>> > always, but rather only in the FPI case when the call to
>> > visibilitymap_pin() is being optimized away.  If we solve the problem
>> > that way, I don't think we even need a new WAL record for this case,
>> > which is a non-trivial fringe benefit.
>>
>> The visibility map is not the only thing that need to be addressed,
>> no?
>
> If I understand Robert correctly his point is about fixing the smgr
> inval alone - without the invalidation fix that'd not be enough because
> the relcache info with outdated information (particularly relallvisible
> et al), would continue to be valid. Relcache invalidations imply an smgr
> one, but not the other way round.
>
> The reason the fix for nmsg > 0 && !markXidCommitted isn't entirely
> sufficient is because the smgr invalidation isn't transaction bound,
> i.e. sent out immediately. So, to have the same behaviour on master/HS,
> we need a way to send out the invalidiation properly in lockstep with
> replay.

What I'm confused about here is:

Masahiko Sawada posted a patch that fixes the problem for him, which
does not involve any new WAL record type.  It also seems to be fixing
the problem in a way that is clean and consistent with what we've done
elsewhere.

The patch actually under discussion here manages to introduce a new
WAL record type without fixing that problem.

Therefore I include that the committed patch fixes some *other*
problem, not the one that this thread is ostensibly about.

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


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

Re: [HACKERS] pgindent

[Robert Haas]

On Wed, Apr 27, 2016 at 11:57 AM, Bruce Momjian  wrote:
> On Wed, Apr 27, 2016 at 11:51:42AM -0400, Robert Haas wrote:
>> >> It mostly just adds new typedefs that have
>> >> appeared over the last year, but it also realphabetizes the file -
>> >> some things that were added incrementally seem to have ended up in
>> >> what is, at least according to what sort likes to do on my machine,
>> >> the wrong place in the file.
>> >
>> > Is it just me, or is the sort order in that file a bit confusing? The
>> > whole thing about upper and lower case being separated seems to make it
>> > much harder than necessary to manually insert something in the right
>> > place..
>>
>> Except for recently-manually-added entries, it seems to match what
>> sort wants to do on my system exactly.  Which seems good.
>
> Well, sort ordering is all related to your collation setting:
>
> $ echo "a
> > A" | LC_COLLATE="" sort
> a
> A
>
> $ echo "a
> A" | LC_COLLATE="en_US" sort
> A
> a

Sure.  I guess we could resort that file with LC_COLLATE=C.  But my
point was mostly just that the ordering is hardly haphazard.

-- 
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] Suspicious behaviour on applying XLOG_HEAP2_VISIBLE.

[Andres Freund]

On 2016-04-27 11:59:39 -0400, Robert Haas wrote:
> Masahiko Sawada posted a patch that fixes the problem for him, which
> does not involve any new WAL record type.  It also seems to be fixing
> the problem in a way that is clean and consistent with what we've done
> elsewhere.

It only fixes one symptom, the relcache entry is still wrong
afterwards. Which is pretty relevant for planning.


> The patch actually under discussion here manages to introduce a new
> WAL record type without fixing that problem.

It does fix the problem, just not in a super robust way. Which is why I
think we should add something like Masahiko's fix additionally.

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] Suspicious behaviour on applying XLOG_HEAP2_VISIBLE.

[Robert Haas]

On Wed, Apr 27, 2016 at 12:03 PM, Andres Freund  wrote:
> On 2016-04-27 11:59:39 -0400, Robert Haas wrote:
>> Masahiko Sawada posted a patch that fixes the problem for him, which
>> does not involve any new WAL record type.  It also seems to be fixing
>> the problem in a way that is clean and consistent with what we've done
>> elsewhere.
>
> It only fixes one symptom, the relcache entry is still wrong
> afterwards. Which is pretty relevant for planning.
>
>
>> The patch actually under discussion here manages to introduce a new
>> WAL record type without fixing that problem.
>
> It does fix the problem, just not in a super robust way. Which is why I
> think we should add something like Masahiko's fix additionally.

OK, that works for me.

-- 
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] Show dropped users' backends in pg_stat_activity

[Bruce Momjian]

On Thu, Apr  7, 2016 at 11:22:08PM +0300, Oskari Saarenmaa wrote:
> 24.03.2016, 18:03, Tom Lane kirjoitti:
> >Robert Haas  writes:
> >>I am not really in favor of half-fixing this.  If we can't
> >>conveniently wait until a dropped role is completely out of the
> >>system, then I don't see a lot of point in trying to do it in the
> >>limited cases where we can.  If LEFT JOIN is the way to go, then,
> >>blech, but, so be it.
> >
> >I concur.  Let's put the left join(s) into those views and call it
> >good.
> >
> >BTW, I think we would need the left joins even if we had interlocking
> >in DROP, just to protect ourselves against race conditions.  Remember
> >that what pg_stat_activity shows is a snapshot, which might be more or
> >less out of date compared to the catalog contents.
> 
> Added my patch to the 2016-09 commitfest
> (https://commitfest.postgresql.org/10/601/) as a bug fix as I thought not
> showing all backends in pg_stat_activity is a bug.  Any chance to get it in
> 9.6?

Do we need a comment in the query explaining why a left join is needed,
e.g. "Use LEFT JOIN in case the role has been dropped"?  That wouldn't
be obvious to me.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +


-- 
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] EXPLAIN VERBOSE with parallel Aggregate

[Robert Haas]

On Tue, Apr 26, 2016 at 11:38 PM, David Rowley
 wrote:
> On 27 April 2016 at 15:12, Robert Haas  wrote:
>> On Tue, Apr 26, 2016 at 10:57 PM, David Rowley
>>  wrote:
>>> On 27 April 2016 at 14:30, Robert Haas  wrote:
 On Tue, Apr 26, 2016 at 9:56 PM, Robert Haas  wrote:
> On Tue, Apr 26, 2016 at 9:14 PM, David Rowley
>  wrote:
>> I'd also have expected the output of both partial nodes to be the
>> same, i.e. both prefixed with PARTIAL. Is it intended that they don't?
>> or have I made some other mistake?
>
> No, that's a defect in the patch.  I didn't consider that we need to
> support nodes with finalizeAggs = false and combineStates = true,
> which is why that ERROR was there.  Working on a fix now.

 I think this version should work, provided you use
 partial_grouping_target where needed.
>>>
>>> +static void get_special_variable(Node *node, deparse_context *context,
>>> + void *private);
>>>
>>> "private" is reserved in C++? I understood we want our C code to
>>> compile as C++ too, right? or did I get my wires crossed somewhere?
>>
>> I can call it something other than "private", if you have a
>> suggestion; normally I would have used "context", but that's already
>> taken in this case.  private_context would work, I guess.
>
> It's fine. After Andres' email I looked and saw many other usages of
> C++ keywords in our C code. Perhaps it would be a good idea to name it
> something else we wanted to work towards it, but it sounds like it's
> not, so probably keep what you've got.
>
> The patch looks good. The only other thing I thought about was perhaps
> it would be a good idea to re-add the sanity checks in execQual.c.
> Patch for that is attached.
>
> I removed the aggoutputtype check, as I only bothered adding that in
> the first place because I lost the aggpartial field in some previous
> revision of the parallel aggregate developments. I'd say the
> aggpartial check makes it surplus to requirements.

OK, committed.

-- 
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] Rename max_parallel_degree?

[Daniel Verite]

Robert Haas wrote:

> Of course, we could make this value 1-based rather than 0-based, as
> Peter Geoghegan suggested a while back.  But as I think I said at the
> time, I think that's more misleading than helpful.  The leader
> participates in the parallel plan, but typically does far less of the
> work beneath the Gather node than the other nodes involved in the
> query, often almost none.  In short, the leader is special.
> Pretending that it's just another process involved in the parallel
> group isn't doing anyone a favor.

FWIW, that's not how it looks from the outside (top or vmstat).
I'm ignorant about how parallel tasks are assigned in the planner,
but when trying various values for max_parallel_degree and running
simple aggregates on large tables on a single 4 core CPU doing
nothing else, I'm only ever seeing max_parallel_degree+1 processes
indiscriminately at work, often in the same state (R running or
D waiting for disk).

Also when looking at exec times, for a CPU-bound sample query, I get
for instance the results below, when increasing parallelism one step
at a time, on a 4-core CPU.
I've checked with EXPLAIN that the planner allocates each time
a number of workers that's exactly equal to max_parallel_degree.
("Workers Planned" under the Gather node).

 mp_degree | exec_time | speedup (against degree=0)
---+---+-
 0 | 10850.835 |1.00
 1 |  5833.208 |1.86
 2 |  3990.144 |2.72
 3 |  3165.606 |3.43
 4 |  3315.153 |3.27
 5 |  .931 |3.25
 6 |  3354.995 |3.23

If the leader didn't do much work here, how would degree=1 produce
such a speedup (1.86) ?

There's also the fact that allowing 4 workers does not help compared
to 3, even though there are 4 cores here. Again, doesn't it make sense
only if the leader is as important as the workers in terms of CPU
usage? 


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


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

Re: [HACKERS] 9.6 and fsync=off

[Robert Haas]

On Wed, Apr 27, 2016 at 11:04 AM, Craig Ringer 
wrote:>> I'd suggest something like
>>
>> #fsync = on # flush data to disk for crash
>> safety
>> # (turning this off can cause
>> # unrecoverable data corruption!)
>>
>
> Looks good.
>
> The docs on fsync are already good, it's just a matter of making people
> think twice and actually look at them.

Committed that way.  Thanks for suggesting this, Craig.

-- 
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] Timeline following for logical slots

[Robert Haas]

On Tue, Apr 26, 2016 at 4:35 PM, Alvaro Herrera
 wrote:
> Craig Ringer wrote:
>> The reason the new src/test/recovery/ tests don't notice this is that using
>> pg_recvlogical from the TAP tests is currently pretty awkward.
>> pg_recvlogical has no way to specify a stop-point or return when there's no
>> immediately pending data like the SQL interface does. So you have to read
>> from it until you figure it's not going to return anything more then kill
>> it and look at what it did return and hope you don't lose anything in
>> buffering.I don't much like relying on timeouts as part of normal
>> successful results since they can upset some of the older and slower
>> buildfarm members. I'd rather be able to pass a --stoppos= or a --n-xacts=
>> option, but it was a bit too late to add those.
>
> Considering that pg_recvlogical was introduced mostly as a way to test
> logical decoding features, I think this is a serious oversight and we
> should patch it.  I suppose we could leave it for 9.7, thought I admit I
> would prefer it to introduce it in 9.6.  Now everyone throwing stones at
> me in 3 ... 2 ...

If that's a small and relatively contained change, I don't object to
the idea of you making it, provided it's done pretty soon, and
definitely before beta1.  If it's going to require substantial
refactoring or can't be done quickly, then, yes, I object.

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

[Tom Lane]

Robert Haas  writes:
> I think it's about time for us to run pgindent.  I did a trial run
> today of pgindent today and came up with the attached patch for
> typedefs.list, which I'd like to commit more or less immediately,
> barring objections.

Um, we normally take the buildfarm's list of typedefs, not anything
manually created.

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] Timeline following for logical slots

[Andres Freund]

On 2016-04-27 14:21:53 -0400, Robert Haas wrote:
> > Considering that pg_recvlogical was introduced mostly as a way to test
> > logical decoding features, I think this is a serious oversight and we
> > should patch it.  I suppose we could leave it for 9.7, thought I admit I
> > would prefer it to introduce it in 9.6.  Now everyone throwing stones at
> > me in 3 ... 2 ...
> 
> If that's a small and relatively contained change, I don't object to
> the idea of you making it, provided it's done pretty soon, and
> definitely before beta1.  If it's going to require substantial
> refactoring or can't be done quickly, then, yes, I object.

I don't object either, I was looking for the feature myself a number of
times (for tap tests in my case).

It requires a some amount of thinking about what the limit applies to
though. "messages sent by server", Bytes? TCP messages? xids? Time?

- Andres


-- 
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: [COMMITTERS] pgsql: Modify the isolation tester so that multiple sessions can wait.

[Alvaro Herrera]

Robert Haas wrote:
> Modify the isolation tester so that multiple sessions can wait.

Mikael was kind enough to set up an OpenBSD 5.9 buildfarm member, and
it's been failing in isolationtester.  (I kindly accepted my suggestion
to put it to run even though it is failing so that we could look at the
logs).  I think this commit is the problem; see the backtrace he sent me
on private email.

#0  0x1e18c62af87a in thrkill () at :2
2   : No such file or directory.
in 
(gdb) bt
#0  0x1e18c62af87a in thrkill () at :2
#1  0x1e18c62aaf39 in *_libc_abort () at
/usr/src/lib/libc/stdlib/abort.c:52
#2  0x1e18c62a7838 in *_libc_memcpy (dst0=0x0, src0=0x6, length=0) at
/usr/src/lib/libc/string/memcpy.c:65
#3  0x1e16a7904a7e in run_permutation (testspec=0x1e16a7d08b20,
nsteps=11, steps=Variable "steps" is not available.
) at isolationtester.c:617
#4  0x1e16a7904d7c in run_testspec (testspec=0x1e16a7d08b20) at
isolationtester.c:369
#5  0x1e16a79053ce in main (argc=Variable "argc" is not available.
) at isolationtester.c:251

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

[HACKERS] Re: [COMMITTERS] pgsql: Modify the isolation tester so that multiple sessions can wait.

[Robert Haas]

On Wed, Apr 27, 2016 at 1:51 PM, Alvaro Herrera
 wrote:
> Robert Haas wrote:
>> Modify the isolation tester so that multiple sessions can wait.
>
> Mikael was kind enough to set up an OpenBSD 5.9 buildfarm member, and
> it's been failing in isolationtester.  (I kindly accepted my suggestion
> to put it to run even though it is failing so that we could look at the
> logs).  I think this commit is the problem; see the backtrace he sent me
> on private email.
>
> #0  0x1e18c62af87a in thrkill () at :2
> 2   : No such file or directory.
> in 
> (gdb) bt
> #0  0x1e18c62af87a in thrkill () at :2
> #1  0x1e18c62aaf39 in *_libc_abort () at
> /usr/src/lib/libc/stdlib/abort.c:52
> #2  0x1e18c62a7838 in *_libc_memcpy (dst0=0x0, src0=0x6, length=0) at
> /usr/src/lib/libc/string/memcpy.c:65
> #3  0x1e16a7904a7e in run_permutation (testspec=0x1e16a7d08b20,
> nsteps=11, steps=Variable "steps" is not available.
> ) at isolationtester.c:617
> #4  0x1e16a7904d7c in run_testspec (testspec=0x1e16a7d08b20) at
> isolationtester.c:369
> #5  0x1e16a79053ce in main (argc=Variable "argc" is not available.
> ) at isolationtester.c:251

That looks like malloc() returned NULL.  I noticed when writing that
patch that isolationtester has never had any checks for malloc
returning NULL, which is bad, and probably worth fixing, but I didn't
choose to stop and fix it at that time.

I don't know off-hand why you see that problem starting at this commit
and not before, or why it doesn't show up on other machines.

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

[Robert Haas]

On Wed, Apr 27, 2016 at 2:23 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> I think it's about time for us to run pgindent.  I did a trial run
>> today of pgindent today and came up with the attached patch for
>> typedefs.list, which I'd like to commit more or less immediately,
>> barring objections.
>
> Um, we normally take the buildfarm's list of typedefs, not anything
> manually created.

Well, we can still do that, but I don't see much advantage in it.  It
just churns the file to the extent that manual review of the changes
is impossible, and then when pgindent does the wrong thing it only
gets reported after the fact.  How is that better than making sure
that the contents of the file are such as to actually produce good
output from pgindent?

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

[HACKERS] Re: [COMMITTERS] pgsql: Modify the isolation tester so that multiple sessions can wait.

[Alvaro Herrera]

Robert Haas wrote:

> That looks like malloc() returned NULL.  I noticed when writing that
> patch that isolationtester has never had any checks for malloc
> returning NULL, which is bad, and probably worth fixing, but I didn't
> choose to stop and fix it at that time.

I didn't actually check closely but I wondered whether the pointer
arithmetic is actually correct.  Note that the memcpy length is zero.
I doubt malloc returning null is the problem; how could it happen
exactly at the same spot every time the suite has run?

> I don't know off-hand why you see that problem starting at this commit
> and not before, or why it doesn't show up on other machines.

Perhaps it's only a problem for OpenBSD's libc and not for glibc which
is the most common.  The only other openbsd machine in buildfarm doesn't
run the isolation tests.

-- 
Á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_dump dump catalog ACLs

[Alvaro Herrera]

Stephen Frost wrote:

> The current challenge I've been trying to find a solution to is testing the
> extension handling. Only core extensions (plpgsql, plperl, etc) are
> available when the TAP tests are running for pg_dump, it seems. I certainly
> don't want to add a testing extension in such a way that it would get
> installed for users, but it doesn't seem obvious how to create an extension
> from another directory or to even find the directory where I could write
> the control and SQL files into to use.
> 
> Any thoughts on that would certainly be welcome.

Did you notice the src/test/modules/test_extensions thingy?  Supposedly
we do try pg_dump with custom extensions there.

-- 
Á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] Bogus cleanup code in PostgresNode.pm

[Alvaro Herrera]

Michael Paquier wrote:
> On Tue, Apr 26, 2016 at 2:24 PM, Tom Lane  wrote:
> > Now, whether using END is really an improvement is a separate question.
> > I have the impression that END calls happen in a better-defined order,
> > but I'm not a perl monk.
> 
> For the archive's sake, 08af9219 is the result commit.

Thanks!

-- 
Á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_dump dump catalog ACLs

[Stephen Frost]

Alvaro,

On Wednesday, April 27, 2016, Alvaro Herrera 
wrote:

> Stephen Frost wrote:
>
> > The current challenge I've been trying to find a solution to is testing
> the
> > extension handling. Only core extensions (plpgsql, plperl, etc) are
> > available when the TAP tests are running for pg_dump, it seems. I
> certainly
> > don't want to add a testing extension in such a way that it would get
> > installed for users, but it doesn't seem obvious how to create an
> extension
> > from another directory or to even find the directory where I could write
> > the control and SQL files into to use.
> >
> > Any thoughts on that would certainly be welcome.
>
> Did you notice the src/test/modules/test_extensions thingy?  Supposedly
> we do try pg_dump with custom extensions there.
>

Hmm. I think the issue I was having is that I wanted to test the extension
handling in pg_dump's 'make check' run, but that's before any of those
extension pieces are installed or tested.

However, perhaps I can test the pg_dump parts as part of the extension
'make check' runs instead. Will check it out.

Thanks!

Stephen

Re: [HACKERS] Re: [COMMITTERS] pgsql: Modify the isolation tester so that multiple sessions can wait.

[Thomas Munro]

On Thu, Apr 28, 2016 at 7:02 AM, Alvaro Herrera
 wrote:
> Robert Haas wrote:
>
>> That looks like malloc() returned NULL.  I noticed when writing that
>> patch that isolationtester has never had any checks for malloc
>> returning NULL, which is bad, and probably worth fixing, but I didn't
>> choose to stop and fix it at that time.
>
> I didn't actually check closely but I wondered whether the pointer
> arithmetic is actually correct.  Note that the memcpy length is zero.
> I doubt malloc returning null is the problem; how could it happen
> exactly at the same spot every time the suite has run?
>
>> I don't know off-hand why you see that problem starting at this commit
>> and not before, or why it doesn't show up on other machines.
>
> Perhaps it's only a problem for OpenBSD's libc and not for glibc which
> is the most common.  The only other openbsd machine in buildfarm doesn't
> run the isolation tests.

Also happens on OpenBSD 5.8.  Isn't this a classic case where memmove
is called for?  Replacing the memcpy at line 617 with memmove makes
the tests run successfully, but at first glance the other two
instances of memcpy in run_permutation should also be changed to
memmove, no?

-- 
Thomas Munro
http://www.enterprisedb.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] pgindent

[Bruce Momjian]

On Wed, Apr 27, 2016 at 02:54:35PM -0400, Robert Haas wrote:
> On Wed, Apr 27, 2016 at 2:23 PM, Tom Lane  wrote:
> > Robert Haas  writes:
> >> I think it's about time for us to run pgindent.  I did a trial run
> >> today of pgindent today and came up with the attached patch for
> >> typedefs.list, which I'd like to commit more or less immediately,
> >> barring objections.
> >
> > Um, we normally take the buildfarm's list of typedefs, not anything
> > manually created.
> 
> Well, we can still do that, but I don't see much advantage in it.  It
> just churns the file to the extent that manual review of the changes
> is impossible, and then when pgindent does the wrong thing it only
> gets reported after the fact.  How is that better than making sure
> that the contents of the file are such as to actually produce good
> output from pgindent?

Using the buildfarm typedefs assures they are always generated in a
consistent way.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +


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

Re: [HACKERS] Add jsonb_compact(...) for whitespace-free jsonb to text

[Stephen Frost]

* Merlin Moncure (mmonc...@gmail.com) wrote:
> On Tue, Apr 26, 2016 at 11:49 AM, Stephen Frost  wrote:
> > As I mentioned to Sehrope on IRC, at least for my 2c, if you want a
> > compact JSON format to reduce the amount of traffic over the wire or to
> > do things with on the client side, we should probably come up with a
> > binary format, rather than just hack out the whitespace.  It's not like
> > representing numbers using ASCII characters is terribly efficient
> > either.
> 
> -1
> 
> This will benefit pretty much nobody unless you are writing a hand
> crafted C application that consumes and processes the data directly.

That's not accurate.  All that's needed is for the libraries which
either wrap libpq or re-implement it to be updated to understand the
format and then convert the data into whatever structure makes sense for
the language (or library that the language includes for working with
JSON data).

One of the unfortunate realities with JSON is that there isn't a
terribly good binary representation, afaik.  As I understand it, BSON
doesn't support all the JSON structures that we do; if it did, I'd
suggest we provide a way to convert our structure into BSON.

> I'd venture to guess this is a tiny fraction of pg users these days.
> I do not understand at all the objection to removing whitespace.
> Extra whitespace does nothing but pad the document as humans will
> always run the document through a prettifier tuned to their specific
> requirements (generally starting with, intelligent placement of
> newlines) if reading directly.

The objection is that it's a terribly poor solution as it simply makes
things ugly for a pretty small amount of improvement.  Looking at it
from the perspective of just "whitespace is bad!" it might look like a
good answer to just remove whitespace, but we should be looking at it
from the perspective of "how do we make this more efficient?".  Under
that lense, removing whitespace appears to be minimally effective
whereas passing the data back in a binary structure looks likely to
greatly improve the efficiency on a number of levels.

> Also, binary formats are not always smaller than text formats.

While true, I don't think that would be true in this case.

Of course, there's nothing like actually trying it and seeing.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Parallel indicators not written by pg_get_functiondef

[Robert Haas]

On Wed, Apr 27, 2016 at 3:02 AM, Feike Steenbergen
 wrote:
> This patch is redundant as of commit
> 2ac3be2e763d9b971352819f285dd51519e0aeb9
>
> (Ashutosh Sharma diagnosed and patched the same problem)

Oops.  I completely failed to notice this prior report.

Sorry about that.

-- 
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] WIP: Covering + unique indexes.

[Peter Geoghegan]

On Tue, Apr 12, 2016 at 9:14 AM, Anastasia Lubennikova
 wrote:
> Sooner or later, I'd like to see this patch finished.

Me, too.

> For now, it has two complaints:
> - support of expressions as included columns.
> Frankly, I don't understand, why it's a problem of the patch.
> The patch is  already big enough and it will be much easier to add
> expressions support in the following patch, after the first one will be
> stable.
> I wonder, if someone has objections to that?

Probably. If we limit the scope of something, it's always in a way
that limits the functionality available to users, rather than limits
how generalized the new functionality is, and so cutting scope
sometimes isn't possible. There is a very high value placed on
features working well together. A user ought to be able to rely on the
intuition that features work well together. Preserving that general
ability for users to guess correctly what will work based on what they
already know is seen as important.

For example, notice that the INSERT documentation allows UPSERT unique
index inference to optionally accept an opclass or collation. So far,
the need for this functionality is totally theoretical (in practice
all B-Tree opclasses have the same idea about equality across a given
type, and we have no case insensitive collations), but it's still
there. Making that work was not a small effort (there was a follow-up
bugfix commit just for that, too). This approach is mostly about
making the implementation theoretically sound (or demonstrating that
it is) by considering edge-cases up-front. Often, there will be
benefits to a maximally generalized approach that were not initially
anticipated by the patch author, or anyone else.

I agree that it is difficult to uphold this standard at all times, but
there is something to be said for it. Postgres development must have a
very long term outlook, and this approach tends to make things easier
for future patch authors by making the code more maintainable. Even if
this is the wrong thing in specific cases, it's sometimes easier to
just do it than to convince others that their concern is misplaced in
this one instance.

> Yes, it's a kind of delayed feature. But should we wait for every patch when
> it will be entirely completed?

I think that knowing where and how to cut scope is an important skill.
If this question is asked as a general question, then the answer must
be "yes". I suggest asking a more specific question. :-)

> - lack of review and testing
> Obviously I did as much testing as I could.
> So, if reviewers have any concerns about the patch, I'm waiting forward to
> see them.

For what it's worth, I agree that you put a great deal of effort into
this patch, and it did not get in to 9.6 because of a collective
failure to focus minds on the patch. Your patch was a credible
attempt, which is impressive when you consider that the B-Tree code is
so complicated. There is also the fact that there is now a very small
list of credible reviewers for B-Tree patches; you must have noticed
that not even amcheck was committed, even though I was asked to
produce a polished version in February during the FOSDEM dev meeting,
and even though it's just a contrib module that is totally orientated
around finding bugs and so on. I'm not happy about that either, but
that's just something I have to swallow.

I fancy myself as am expert on the B-Tree code, but I've never managed
to make an impact in improving its performance at all (I've never made
a serious effort, but have had many ideas). So, in case it needs to be
said, I'll say it: You've chosen a very ambitious set of projects to
work on, by any standard. I think it's a good thing that you've been
ambitious, and I don't suggest changing that, since I think that you
have commensurate skill. But, in order to be successful in these
projects, patience and resolve are very important.

-- 
Peter Geoghegan


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

Re: [HACKERS] Patch: typo, s/espaced/escaped/

[Bruce Momjian]

On Wed, Mar 16, 2016 at 05:10:57PM +0300, Aleksander Alekseev wrote:
> There is a typo in one word in this commit:
> 
> http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=9a206d063c410df7cd5da01b169b23bff413fef5
> 
> Patch attached.

This patch has been applied by Teodor.  Thanks.

---


> 
> -- 
> Best regards,
> Aleksander Alekseev
> http://eax.me/

> diff --git a/contrib/unaccent/generate_unaccent_rules.py 
> b/contrib/unaccent/generate_unaccent_rules.py
> index 2f5520c..a5eb42f 100644
> --- a/contrib/unaccent/generate_unaccent_rules.py
> +++ b/contrib/unaccent/generate_unaccent_rules.py
> @@ -95,7 +95,7 @@ def 
> parse_cldr_latin_ascii_transliterator(latinAsciiFilePath):
>  # to the characters.
>  #
>  # Group 1: plain "src" char. Empty if group 2 is not.
> -# Group 2: unicode-espaced "src" char (e.g. "\u0110"). Empty if 
> group 1 is not.
> +# Group 2: unicode-escaped "src" char (e.g. "\u0110"). Empty if 
> group 1 is not.
>  #
>  # Group 3: plain "trg" char. Empty if group 4 is not.
>  # Group 4: plain "trg" char between quotes. Empty if group 3 is not.

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


-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +


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

[Bruce Momjian]

On Wed, Apr 27, 2016 at 05:01:14PM -0400, Bruce Momjian wrote:
> On Wed, Apr 27, 2016 at 02:54:35PM -0400, Robert Haas wrote:
> > On Wed, Apr 27, 2016 at 2:23 PM, Tom Lane  wrote:
> > > Robert Haas  writes:
> > >> I think it's about time for us to run pgindent.  I did a trial run
> > >> today of pgindent today and came up with the attached patch for
> > >> typedefs.list, which I'd like to commit more or less immediately,
> > >> barring objections.
> > >
> > > Um, we normally take the buildfarm's list of typedefs, not anything
> > > manually created.
> > 
> > Well, we can still do that, but I don't see much advantage in it.  It
> > just churns the file to the extent that manual review of the changes
> > is impossible, and then when pgindent does the wrong thing it only
> > gets reported after the fact.  How is that better than making sure
> > that the contents of the file are such as to actually produce good
> > output from pgindent?
> 
> Using the buildfarm typedefs assures they are always generated in a
> consistent way.

Oh, and as I remember the buildfarm merges several platforms, including
Windows, to make that list, so I suggest you use that one.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +


-- 
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] Returning 'Infinity'::TIMESTAMPTZ from "to_timestamp" function

[Bruce Momjian]

FYI, I show this as fixed in 9.6:

test=> SELECT to_timestamp('Infinity'::float);
 to_timestamp
--
 infinity
(1 row)

---

On Sun, Nov  8, 2015 at 09:15:16PM -0800, Vitaly Burovoy wrote:
> Hello everyone!
> 
> Continuing the topic of extracting EPOCH from 'Infinity'::TIMESTAMPTZ
> and according to an item "converting between infinity timestamp and
> float8" in the TODO list...
> 
> Even when "SELECT extract(EPOCH FROM TIMESTAMPTZ 'Infinity')" results
> 'Infinity'::float, there is still trouble to convert it back:
> # SELECT to_timestamp('Infinity'::float);
> ERROR:  timestamp out of range
> CONTEXT:  SQL function "to_timestamp" statement 1
> 
> The function "to_timestamp(double precision)" is defined as an SQL-script:
> select ('epoch'::pg_catalog.timestamptz + $1 * '1 
> second'::pg_catalog.interval)
> 
> Whereas error message points to a function "timestamptz_pl_interval",
> there is still a nuance in a function "interval_mul", because it
> returns "Interval->time" as "-Infinity" for both +/-infinity as an
> input value (apart from the fact that INTERVAL does not support
> infinite values officially).
> 
> To add an ability to construct 'Infinity' TIMESTAMPTZ via
> "to_timestamp" call, there are two ways:
> 
> 1. Rewrite the function "pg_catalog.to_timestamp(double precision)" as
> an internal one. It's the easiest way, because it allows to avoid
> usage of INTERVAL as a helper (of course, there is still possible to
> use intervals as shown above in user's scripts, but without "Infinity"
> support).
> 
> 2. Add support of infinite intervals. It is harder, because it touches
> a lot of functions. I can add that support if it is in demand.
> 
> Which way is preferred?
> -- 
> Best regards,
> Vitaly Burovoy
> 
> 
> -- 
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +


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

[Bruce Momjian]

On Sat, Jan 30, 2016 at 11:06:10PM -0800, Vitaly Burovoy wrote:
> Hackers,
> 
> I've just found a little bug: extracting "epoch" from the last 30
> years before Postgres' "+Infinity" leads an integer overflow:
> 
> postgres=# SELECT x::timestamptz, extract(epoch FROM x::timestamptz)
> postgres-# FROM
> postgres-# (VALUES
> postgres(#   ('294247-01-10 04:00:54.775805'),
> postgres(#   ('294247-01-10 04:00:55.775806'),
> postgres(#   ('294277-01-09 04:00:54.775806'), -- the last value before 'Inf'
> postgres(#   ('294277-01-09 04:00:54.775807')  -- we've discussed, it
> should be fixed
> postgres(# ) as t(x);
> x| date_part
> -+---
>  294247-01-10 04:00:54.775805+00 |  9223372036854.78
>  294247-01-10 04:00:55.775806+00 | -9223372036853.78
>  294277-01-09 04:00:54.775806+00 | -9222425352054.78
>  infinity|  Infinity
> (4 rows)
> 
> With the attached patch it becomes positive:
> x|date_part
> -+--
>  294247-01-10 04:00:54.775805+00 | 9223372036854.78
>  294247-01-10 04:00:55.775806+00 | 9223372036855.78
>  294277-01-09 04:00:54.775806+00 | 9224318721654.78
>  infinity| Infinity
> (4 rows)

FYI, in 9.6 this will return an error:

test=> SELECT x::timestamptz, extract(epoch FROM x::timestamptz)
FROM
(VALUES
('294247-01-10 04:00:54.775805'),
('294247-01-10 04:00:55.775806'),
('294277-01-09 04:00:54.775806'), -- the last value before 'Inf'
('294277-01-09 04:00:54.775807')  -- we've discussed, it
) as t(x);
ERROR:  timestamp out of range: "294277-01-09 04:00:54.775806"

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +


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

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

[David Steele]

On 4/27/16 5:08 PM, Peter Geoghegan wrote:

> So, in case it needs to be
> said, I'll say it: You've chosen a very ambitious set of projects to
> work on, by any standard. I think it's a good thing that you've been
> ambitious, and I don't suggest changing that, since I think that you
> have commensurate skill. But, in order to be successful in these
> projects, patience and resolve are very important.

+1.

This is very exciting work and I look forward to seeing it continue.
The patch was perhaps not a good fit for the last CF of 9.6 but that
doesn't mean it can't have a bright future.

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


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

Re: [HACKERS] Re: [COMMITTERS] pgsql: Modify the isolation tester so that multiple sessions can wait.

[Thomas Munro]

On Thu, Apr 28, 2016 at 8:46 AM, Thomas Munro
 wrote:
> On Thu, Apr 28, 2016 at 7:02 AM, Alvaro Herrera
>  wrote:
>> Robert Haas wrote:
>>
>>> That looks like malloc() returned NULL.  I noticed when writing that
>>> patch that isolationtester has never had any checks for malloc
>>> returning NULL, which is bad, and probably worth fixing, but I didn't
>>> choose to stop and fix it at that time.
>>
>> I didn't actually check closely but I wondered whether the pointer
>> arithmetic is actually correct.  Note that the memcpy length is zero.
>> I doubt malloc returning null is the problem; how could it happen
>> exactly at the same spot every time the suite has run?
>>
>>> I don't know off-hand why you see that problem starting at this commit
>>> and not before, or why it doesn't show up on other machines.
>>
>> Perhaps it's only a problem for OpenBSD's libc and not for glibc which
>> is the most common.  The only other openbsd machine in buildfarm doesn't
>> run the isolation tests.
>
> Also happens on OpenBSD 5.8.  Isn't this a classic case where memmove
> is called for?  Replacing the memcpy at line 617 with memmove makes
> the tests run successfully, but at first glance the other two
> instances of memcpy in run_permutation should also be changed to
> memmove, no?

That abort at memcpy.c:65 is indeed a check for overlapping regions:

http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/lib/libc/string/memcpy.c?rev=1.2&content-type=text/x-cvsweb-markup

That leaves the question of why the backtrace shows arguments that
wouldn't trigger it (for example memcpy exits early for length == 0).
But it appears that those stack arguments are mangled by the time gdb
dumps them, like this:

#include 
int main(int argc, char *argv[])
{
  char buffer[] = "hello world";
  memcpy(&buffer[0], &buffer[1], 2);
  return 0;
}

#0  0x0bce27eab90a in kill () at :2
#1  0x0bce27ee5b19 in abort () at /usr/src/lib/libc/stdlib/abort.c:53
#2  0x0bce27ebcde8 in memcpy (dst0=0xf7ec5, src0=0x6, length=0) at
/usr/src/lib/libc/string/memcpy.c:65
#3  0x0bcb5fc00c7a in main (argc=1, argv=0x7f7f6818) at foo.c:6

Suggested patch attached.

-- 
Thomas Munro
http://www.enterprisedb.com


memmove.patch
Description: Binary data

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

Re: [HACKERS] Support for N synchronous standby servers - take 2

[Tom Lane]

Kyotaro HORIGUCHI  writes:
> Sorry, I have attached an empty patch. This is another one that should
> be with content.

I pushed this after whacking it around some, and cleaning up some
sort-of-related problems in the syncrep parser/lexer.

There remains a point that I'm not very happy about, which is the code
in check_synchronous_standby_names to emit a WARNING if the num_sync
setting is too large.  That's a pretty bad compromise: we should either
decide that the case is legal or that it is not.  If it's legal, people
who are correctly using the case will not thank us for logging a WARNING
every single time the postmaster gets a SIGHUP (and those who aren't using
it correctly will have their systems freezing up, warning or no warning).
If it's not legal, we should make it an error not a warning.

My inclination is to just rip out the warning.  But I wonder whether the
desire to have one doesn't imply that the semantics are poorly chosen
and should be revisited.

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

[Tom Lane]

Robert Haas  writes:
> On Wed, Apr 27, 2016 at 2:23 PM, Tom Lane  wrote:
>> Um, we normally take the buildfarm's list of typedefs, not anything
>> manually created.

> Well, we can still do that, but I don't see much advantage in it.  It
> just churns the file to the extent that manual review of the changes
> is impossible, and then when pgindent does the wrong thing it only
> gets reported after the fact.  How is that better than making sure
> that the contents of the file are such as to actually produce good
> output from pgindent?

On what grounds do you claim the buildfarm result is unstable?
I've been using that for a long time and it works fine.  Moreover,
ignoring that data is a bad idea because it reflects platform-specific
variations in the set of typedefs that are known.  If you build a
typedefs list based only on what works on your machine, it likely
won't work for other people.

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] Re: [COMMITTERS] pgsql: Modify the isolation tester so that multiple sessions can wait.

[Tom Lane]

Thomas Munro  writes:
>> Also happens on OpenBSD 5.8.  Isn't this a classic case where memmove
>> is called for?  Replacing the memcpy at line 617 with memmove makes
>> the tests run successfully, but at first glance the other two
>> instances of memcpy in run_permutation should also be changed to
>> memmove, no?

Yeah, that's clearly busted.  Surprising that it has not failed on
any other platforms.

> Suggested patch attached.

Will push in a moment.

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] Re: [COMMITTERS] pgsql: Modify the isolation tester so that multiple sessions can wait.

[Mikael Kjellström]

On 2016-04-28 00:15, Tom Lane wrote:

Thomas Munro  writes:

Also happens on OpenBSD 5.8.  Isn't this a classic case where memmove
is called for?  Replacing the memcpy at line 617 with memmove makes
the tests run successfully, but at first glance the other two
instances of memcpy in run_permutation should also be changed to
memmove, no?


Yeah, that's clearly busted.  Surprising that it has not failed on
any other platforms.


Suggested patch attached.


Will push in a moment.


And that seems to have done the trick.  I started a manual run of HEAD 
on curculio and now it's green on the build farm.


Thanks!

/Mikael


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

Re: [HACKERS] Add jsonb_compact(...) for whitespace-free jsonb to text

[David G. Johnston]

On Sun, Apr 24, 2016 at 3:02 PM, Sehrope Sarkuni  wrote:

> It'd be nice to have a stable text representation of a jsonb value with
> minimal whitespace. The latter would also save a few bytes per record in
> text output formats, on the wire, and in backups (ex: COPY ... TO STDOUT).
>
​
>
Attached is a *very* work in progress patch that adds a
> jsonb_compact(jsonb)::text function. It generates a text representation
> without extra whitespace but does not yet try to enforce a stable order of
> the properties within a jsonb value.
>
> ​
This doesn't impact backups unless you do away with the function and change
the output i/o function for the type.​  In that scenario the function is no
longer necessary.

David J.
​

Re: [HACKERS] Re: [COMMITTERS] pgsql: Modify the isolation tester so that multiple sessions can wait.

[Tom Lane]

=?UTF-8?Q?Mikael_Kjellstr=c3=b6m?=  writes:
> And that seems to have done the trick.  I started a manual run of HEAD 
> on curculio and now it's green on the build farm.

> Thanks!

Thank *you*, for putting up a buildfarm member that caught this bug.

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] Add jsonb_compact(...) for whitespace-free jsonb to text

[David G. Johnston]

On Sun, Apr 24, 2016 at 3:02 PM, Sehrope Sarkuni  wrote:

> Attached is a *very* work in progress patch that adds a
> jsonb_compact(jsonb)::text function. It generates a text representation
> without extra whitespace but does not yet try to enforce a stable order of
> the properties within a jsonb value.
>

​I think that having a jsonb_compact function that complements the existing
jsonb_pretty function is a well scoped and acceptable​ feature.  I do not
believe that it should also take on the role of canonicalization.

I'd suggest that any discussions regarding stability of jsonb output be
given its own thread.

That topic also seems separate from a discussion on how to implement a
binary transport protocol for jsonb.

​Lastly would be whether we change our default text representation so that
users utilizing COPY get the added benefit of a maximally minimized text
representation.

As an aside on the last topic, has there ever been considered to have a way
to specify a serialization function to use for a given type (or maybe
column) specified in a copy command?

Something like: COPY [...] WITH (jsonb USING jsonb_compact)

I'm thinking this would hard and undesirable given the role copy plays and
limited intelligence that it has in order to maximize its efficiency in
fulfilling its role.

Backups get compressed already so bandwidth seems the bigger goal there.
Otherwise I'd say that we lack any kind of overwhelming evidence that
making such a change would be warranted.

While these are definitely related topics it doesn't seem like any are
pre-requisites for the others.  I think this thread is going to become hard
to follow and trail off it continues to try and address all of these topics
randomly as people see fit to reply.  And it will quickly become hard for
anyone to jump in and understand the topics at hand.

David J.

[HACKERS] Shared memory and processes

[david]

Does Postgres provide a convenient way for one process to pass data to another 
using shared memory?

 

Regards

David M Bennett FACS

  _  

Andl - A New Database Language - andl.org

 

Re: [HACKERS] Shared memory and processes

[Thomas Munro]

On Thu, Apr 28, 2016 at 12:56 PM,   wrote:
> Does Postgres provide a convenient way for one process to pass data to
> another using shared memory?

1.  Look at ShmemInitStruct, ShmemInitHash (as in hash table),
ShmemInitQueue in storage/shmem.h.  These use memory that is mapped at
the same address in every backend (process) which is convenient for
sharing data structures with internal pointers.  The amount of memory
available is fixed at cluster startup however.

2.  Look at dsm_XXX in storage/dsm.h.  This subsystem provides
segments of memory that is "dynamic" in the sense that it is limited
only by your system's available virtual memory, but you have to
explicitly attach these segment in any backend that wants to access
them by passing a handle around and the memory may be mapped at any
address in each backend, so you need to work harder to build data
structures that reference each other (using offsets rather than
pointers, that kind of thing).  DSM segments won't work well if you
try to create large numbers of them, so you'll need to provide a way
to manage the space inside a smallish number of large chunks of DSM
memory yourself (this is a problem I'm working to fix by providing a
general purpose allocator backed by a bunch of DSM segments -- watch
this space).  LWLocks (our usual lock primitive for cases where
spinlocks are inappropriate) currently don't work correctly inside DSM
segments (this too will be fixed).

-- 
Thomas Munro
http://www.enterprisedb.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] EXPLAIN VERBOSE with parallel Aggregate

[David Rowley]

On 28 April 2016 at 04:08, Robert Haas  wrote:
> On Tue, Apr 26, 2016 at 11:38 PM, David Rowley
>> The patch looks good. The only other thing I thought about was perhaps
>> it would be a good idea to re-add the sanity checks in execQual.c.
>> Patch for that is attached.
>>
>> I removed the aggoutputtype check, as I only bothered adding that in
>> the first place because I lost the aggpartial field in some previous
>> revision of the parallel aggregate developments. I'd say the
>> aggpartial check makes it surplus to requirements.
>
> OK, committed.

Great. Thank you.

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


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

Re: [HACKERS] Shared memory and processes

[Michael Paquier]

On Thu, Apr 28, 2016 at 9:56 AM,   wrote:
> Does Postgres provide a convenient way for one process to pass data to
> another using shared memory?

If you are talking about enabling the use of shared memory by 3rd-part
plugins or modules, there is shmem_startup_hook for this purpose.
Another thing that you may want to look at is DSM (dynamic shared
memory), with its interface in dsm.h. An example of use is in
src/test/modules/test_shm_mq.
-- 
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] Fix for OpenSSL error queue bug

[Alvaro Herrera]

Peter Geoghegan wrote:

> I'm not sure if project policy around backpatching (that commit
> messages and so on should match exactly) has anything to say about the
> case where backpatching follows several weeks after commit to the
> master branch.

There is no value in providing exact message matches when the backpatch
occurs even after one other commit in the master branch.  The principal
reason for the requirement is so that src/tools/git_changelog is able to
merge the commits as a single entry, and that's not going to happen when
you have one or more other commits in the master branch anyway.  I find
it more productive to mention, in the backpatched commit message, what
is the commit ID in the master branch.  That way it can be more easily
be searched for, later on.

-- 
Á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] Make PG's "NOT NULL"s and attnotnull ("is_nullable") conform to SQL-2011

[Alvaro Herrera]

Vitaly Burovoy wrote:

Hi,

> But before starting working on it I had a look at the SQL-2011
> standard (ISO/IEC 9075-2)[3] and found that:
> 
> 1. A name for a "NOT NULL" constraint  can be given by a table
> definition (subcl. 11.4, "Format"->"column constraint definition").
> 2. The standard splits NNC and CHECK constraints (subcl. 11.4,
> "Format"-> "column constraint")

Point 2 is where things differ from what I remember; my (possibly
flawed) understanding was that there's no difference between those
things.  Many (maybe all) of the things from this point on are probably
fallout from that one change.

> III. "pg_attribute" table should have an "attnotnullid oid" as an
> indicator of "NOT NULL" (p.4) and points to a CHECK constraint; It is
> in addition to a "Nullability characteristic" "attnotnull" (p.3).
> IV. "pg_constraint" should have a column "connotnullkey int2[]" as a
> "list of the nullable columns" which references to
> "pg_attribute.attnum" for fast checking whether a column is still
> nullable after deleting/updating constraints or not. Array is
> necessary for cases like "CHECK ((col1 IS NOT NULL) AND (col2 IS NOT
> NULL))" and for nondeferrable PKs.

I think these points warrant some more consideration.  I don't like the
idea that pg_attribute and pg_constraint are both getting considerably
bloated to support this.

> P.S.:
> Since the SQL standard defines that "col NOT NULL" as an equivalent to
> "CHECK (col IS NOT NULL)" (p.8) what to do with that behavior:
> 
> postgres=# create type t as (x int);
> CREATE TYPE
> postgres=# SELECT v, v IS NOT NULL AS should_be_in_table FROM
> (VALUES('(1)'::t),('()'),(NULL)) AS x(v);
>   v  | should_be_in_table
> -+
>  (1) | t
>  ()  | f
>  | f
> (3 rows)
> 
> "attnotnull" in such case is stricter, like "CHECK (col IS DISTINCT FROM 
> NULL)".
> 
> Should such values (with NULL in each attribute of a composite type)
> violate NOT NULL constraints?

I wonder if the standard has a concept of null composite values.  If
not, then there is no difference between IS NOT NULL and IS DISTINCT
FROM NULL, which explains why they define NNC in terms of the former.


I think your email was too hard to read because of excessive density,
which would explain the complete lack of response.  I haven't had the
chance to work on this topic again, but I encourage you to, if you have
the resources.  (TBH I haven't had the chance to study your proposed
design in detail, either).

-- 
Á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] Parallel build with MSVC

[Michael Paquier]

On Wed, Apr 27, 2016 at 5:15 PM, Christian Ullrich  wrote:
> OK then, hopefully last round attached.

Thanks. Those are fine in my view. It is hard to tell if a committer
is going to have a look at that soon, so I think that it would be
wiser to add that to the next CF so as those patches don't fall into
oblivion.
-- 
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] Fix for OpenSSL error queue bug

[Tom Lane]

Alvaro Herrera  writes:
> Peter Geoghegan wrote:
>> I'm not sure if project policy around backpatching (that commit
>> messages and so on should match exactly) has anything to say about the
>> case where backpatching follows several weeks after commit to the
>> master branch.

> There is no value in providing exact message matches when the backpatch
> occurs even after one other commit in the master branch.

Actually, git_changelog can merge identically-messaged commits despite
intervening commits.  It's set up to not merge commits more than 24 hours
apart, though.  We could loosen that requirement but I'm afraid it would
make things confusing to merge far-apart commits.

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] Shared memory and processes

[david]

> From: Thomas Munro [mailto:thomas.mu...@enterprisedb.com]
> > Does Postgres provide a convenient way for one process to pass data to
> > another using shared memory?
> 
> 1.  Look at ShmemInitStruct, ShmemInitHash (as in hash table), ShmemInitQueue
> in storage/shmem.h.  These use memory that is mapped at the same address in
> every backend (process) which is convenient for sharing data structures with
> internal pointers.  The amount of memory available is fixed at cluster
> startup however.

Thanks. That limit could be an issue. The notes in shmem.c are helpful.

> 2.  Look at dsm_XXX in storage/dsm.h.  This subsystem provides segments of
> memory that is "dynamic" in the sense that it is limited only by your
> system's available virtual memory, but you have to explicitly attach these
> segment in any backend that wants to access them by passing a handle around
> and the memory may be mapped at any address in each backend, so you need to
> work harder to build data structures that reference each other (using offsets
> rather than pointers, that kind of thing).  DSM segments won't work well if
> you try to create large numbers of them, so you'll need to provide a way to
> manage the space inside a smallish number of large chunks of DSM memory
> yourself (this is a problem I'm working to fix by providing a general purpose
> allocator backed by a bunch of DSM segments -- watch this space).  LWLocks
> (our usual lock primitive for cases where spinlocks are inappropriate)
> currently don't work correctly inside DSM segments (this too will be fixed).

I've now found this through the test_shm_mq sample. Looks like an answer, if 
quite a bit of machinery needed.

Thanks for the pointers.


Regards
David M Bennett FACS

Andl - A New Database Language - andl.org





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

Re: [HACKERS] Shared memory and processes

[david]

> From: Michael Paquier [mailto:michael.paqu...@gmail.com]
> > Does Postgres provide a convenient way for one process to pass data to
> > another using shared memory?
> 
> If you are talking about enabling the use of shared memory by 3rd-part
> plugins or modules, there is shmem_startup_hook for this purpose.

Thanks -- looks interesting.

> Another thing that you may want to look at is DSM (dynamic shared memory),
> with its interface in dsm.h. An example of use is in 
> src/test/modules/test_shm_mq.

Yes, that is useful. I'm curious why the documentation (F.41) was removed 
between 9.4 and 9.5 -- is there a problem?

Regards
David M Bennett FACS

Andl - A New Database Language - andl.org







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

Re: [HACKERS] pgindent

[Robert Haas]

On Wed, Apr 27, 2016 at 6:10 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Wed, Apr 27, 2016 at 2:23 PM, Tom Lane  wrote:
>>> Um, we normally take the buildfarm's list of typedefs, not anything
>>> manually created.
>
>> Well, we can still do that, but I don't see much advantage in it.  It
>> just churns the file to the extent that manual review of the changes
>> is impossible, and then when pgindent does the wrong thing it only
>> gets reported after the fact.  How is that better than making sure
>> that the contents of the file are such as to actually produce good
>> output from pgindent?
>
> On what grounds do you claim the buildfarm result is unstable?
> I've been using that for a long time and it works fine.  Moreover,
> ignoring that data is a bad idea because it reflects platform-specific
> variations in the set of typedefs that are known.  If you build a
> typedefs list based only on what works on your machine, it likely
> won't work for other people.

/me shrugs

Well, let's get the list, then, and compare it to what's in the file
now.  How do we do that exactly?

-- 
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] Optimization for updating foreign tables in Postgres FDW

[Michael Paquier]

On Wed, Apr 27, 2016 at 12:16 PM, Etsuro Fujita
 wrote:
> On 2016/04/26 21:45, Etsuro Fujita wrote:
>> While re-reviewing the fix, I noticed that since PQcancel we added to
>> pgfdw_xact_callback to cancel a DML pushdown query isn't followed by a
>> ROLLBACK, the connection to the remote server will be discarded at the
>> end of the while loop in that function, which will cause a FATAL error
>> of "connection to client lost".  Probably, that was proposed by me in
>> the first version of the patch, but I don't think that's a good idea.
>> Shouldn't we execute ROLLBACK after that PQcancel?
>>
>> Another thing I noticed is, ISTM that we miss the case where DML
>> pushdown queries are performed in subtransactions.  I think cancellation
>> logic would also need to be added to pgfdw_subxact_callback.
>
>
> Attached is a patch for that.

I have spent some time looking at that...

And yeah, losing the connection because of that is a little bit
annoying if there are ways to make things clean, and as a START
TRANSACTION is always sent for such queries it seems really better to
issue a ROLLBACK in any case. Actually, by using PQcancel there is no
way to be sure if the cancel will be effective or not. So it could be
possible that the command is still able to complete correctly, or it
could be able to cancel correctly and it would return an ERROR
earlier. In any case, doing the ROLLBACK unconditionally seems adapted
to me because we had better clean up the remote state in both cases.
-- 
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] Shared memory and processes

[Michael Paquier]

On Thu, Apr 28, 2016 at 12:24 PM,   wrote:
>> From: Michael Paquier [mailto:michael.paqu...@gmail.com]
>> Another thing that you may want to look at is DSM (dynamic shared memory),
>> with its interface in dsm.h. An example of use is in 
>> src/test/modules/test_shm_mq.
>
> Yes, that is useful. I'm curious why the documentation (F.41) was removed 
> between 9.4 and 9.5 -- is there a problem?

No, some of the contrib/ modules have been moved in the code tree and
are now considered as only test modules, which do not get installed by
default. As a result, their documentation has been removed.
-- 
Michael


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

Re: [HACKERS] Re: [COMMITTERS] Re: pgsql: Convert contrib/seg's bool-returning SQL functions to V1 call co

[Michael Paquier]

On Wed, Apr 27, 2016 at 10:53 PM, Tom Lane  wrote:
> Now, if VS2015 actually has broken bool-returning V0, the argument for
> keeping it going becomes a lot weaker.  But at this point I suspect
> strongly that that's not what happened but rather we've got a faulty
> bool cast there, which if true is something we need to fix regardless
> of any considerations about V0.  Would someone please try the experiment
> requested upthread?

I just gave it a try. And by using PG_1_BYTE() the tests of
contrib/seg/ are passing.
-- 
Michael


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

Re: [HACKERS] Re: [COMMITTERS] Re: pgsql: Convert contrib/seg's bool-returning SQL functions to V1 call co

[Tom Lane]

Michael Paquier  writes:
> On Wed, Apr 27, 2016 at 10:53 PM, Tom Lane  wrote:
>> Now, if VS2015 actually has broken bool-returning V0, the argument for
>> keeping it going becomes a lot weaker.  But at this point I suspect
>> strongly that that's not what happened but rather we've got a faulty
>> bool cast there, which if true is something we need to fix regardless
>> of any considerations about V0.  Would someone please try the experiment
>> requested upthread?

> I just gave it a try. And by using PG_1_BYTE() the tests of
> contrib/seg/ are passing.

Thanks!  Barring objections, I will revert c8e81afc60093b19 tomorrow
and instead fix DatumGetBool.

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] GIN data corruption bug(s) in 9.6devel

[Noah Misch]

On Tue, Apr 26, 2016 at 08:22:03PM +0300, Teodor Sigaev wrote:
> >>Check my reasoning: In version 4 I added a remebering of tail of pending
> >>list into blknoFinish variable. And when we read page which was a tail on
> >>cleanup start then we sets cleanupFinish variable and after cleaning that
> >>page we will stop further cleanup. Any insert caused during cleanup will be
> >>placed after blknoFinish (corner case: in that page), so, vacuum should not
> >>miss tuples marked as deleted.
> >
> >Yes, I agree with the correctness of v4.  But I do wonder if we should
> >use that early stopping for vacuum and gin_clean_pending_list, rather
> Interesting, I've missed this possible option
> 
> >than just using it for user backends.  While I think correctness
> >allows it to stop early, since these routines are explicitly about
> >cleaning things up it seems like they should volunteer to clean the
> >whole thing.
> 
> I believe that autovacuum should not require guaranteed full clean up, only
> vacuum and gin_clean_pending_list() should do that. In all other cases it
> should stop early to prevent possible infinite cleanup. Patch attached.

Teodor, this item remains open after twenty-three days of you owning it.  At
this pace, 9.6beta1 will have an unreliable GIN implementation.  What things
must happen so you can close this item, and when will you do those things?


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