Re: [HACKERS] INSERT .. SET syntax

[Amit Kapila]

On Mon, Jul 4, 2016 at 1:06 AM, Marko Tiikkaja  wrote:
> Hi,
>
> Here's a patch for $SUBJECT.  I'll probably work on the docs a bit more
> before the next CF, but I thought I'd post it anyway.
>

I could see that it can be useful in certain cases as described in the
documentation part of the patch.  I noticed that you have used
transformUpdateTargetList() to generate expression list for this case,
but that function raises some internal errors which indicates that
error is from Update.  I think that might be misleading to users, if
they ever got raised.

-- 
With Regards,
Amit Kapila.
EnterpriseDB: 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] [CF2016-9] Allow spaces in working path on tap-tests

[Michael Paquier]

On Sun, Jul 10, 2016 at 5:50 AM, Tom Lane  wrote:
> This seemed like a bug fix to me ...

Yes, it is.

> ... so I went ahead and pushed it. I don't
> have any ability to test the Windows parts, so it's possible I missed
> something in the back-patching; please review.

Thanks! What you have pushed looks fine to me. Also, the portion for
src/tools/msvc needs to go further down, I should have precised that
earlier. Do you want a patch for 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] Showing parallel status in \df+

[Michael Paquier]

On Sat, Jul 9, 2016 at 8:12 AM, Michael Paquier
 wrote:
> So to sum up:
> - Add "Parallel" column
> - Add ACLs
> - Reordering the columns, I'd suggest as follows):
> -- Schema
> -- Name
> -- Result data type
> -- Argument data types
> -- Type
> -- Language
> -- Volatility
> -- Parallel
> -- Owner
> -- Security
> -- ACL
> -- Source code
> -- Description
> Or by thema, 1) General info, 2) specificity (volatility, parallel,
> type), 3) Ownership.
> And regarding "source code", I think that's useful for debugging.

Giving the attached, including doc clarifications and column
reshuffling with translatable state set up as well.
-- 
Michael
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index aeffd63..e7bd2d7 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1380,8 +1380,9 @@ testdb=
 objects are shown; supply a pattern or the S
 modifier to include system objects.
 If the form \df+ is used, additional information
-about each function is shown, including security classification,
-volatility, owner, language, source code and description.
+about each function is shown, including language, volatility,
+parallel mode, owner, security classification, access privileges,
+source code and description.
 
 
 
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 2cdc5ac..e297891 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -298,7 +298,7 @@ describeFunctions(const char *functypes, const char *pattern, bool verbose, bool
 	PQExpBufferData buf;
 	PGresult   *res;
 	printQueryOpt myopt = pset.popt;
-	static const bool translate_columns[] = {false, false, false, false, true, true, true, false, false, false, false};
+	static const bool translate_columns[] = {false, false, false, false, true, false, true, false, false, true, false, true, false};
 
 	if (strlen(functypes) != strspn(functypes, "antwS+"))
 	{
@@ -410,28 +410,42 @@ describeFunctions(const char *functypes, const char *pattern, bool verbose, bool
 		  gettext_noop("Type"));
 
 	if (verbose)
+	{
 		appendPQExpBuffer(,
-  ",\n CASE WHEN prosecdef THEN '%s' ELSE '%s' END AS \"%s\""
+		  ",\n l.lanname as \"%s\""
 		  ",\n CASE\n"
 		  "  WHEN p.provolatile = 'i' THEN '%s'\n"
 		  "  WHEN p.provolatile = 's' THEN '%s'\n"
 		  "  WHEN p.provolatile = 'v' THEN '%s'\n"
 		  " END as \"%s\""
-   ",\n  pg_catalog.pg_get_userbyid(p.proowner) as \"%s\",\n"
-		  "  l.lanname as \"%s\",\n"
-		  "  p.prosrc as \"%s\",\n"
-  "  pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"",
-		  gettext_noop("definer"),
-		  gettext_noop("invoker"),
-		  gettext_noop("Security"),
+		  ",\n CASE\n"
+		  "  WHEN p.proparallel = 'r' THEN '%s'\n"
+		  "  WHEN p.proparallel = 's' THEN '%s'\n"
+		  "  WHEN p.proparallel = 'u' THEN '%s'\n"
+		  " END as \"%s\""
+   ",\n pg_catalog.pg_get_userbyid(p.proowner) as \"%s\""
+		  ",\n CASE WHEN prosecdef THEN '%s' ELSE '%s' END AS \"%s\"",
+		  gettext_noop("Language"),
 		  gettext_noop("immutable"),
 		  gettext_noop("stable"),
 		  gettext_noop("volatile"),
 		  gettext_noop("Volatility"),
+		  gettext_noop("restricted"),
+		  gettext_noop("safe"),
+		  gettext_noop("unsafe"),
+		  gettext_noop("Parallel"),
 		  gettext_noop("Owner"),
-		  gettext_noop("Language"),
+		  gettext_noop("definer"),
+		  gettext_noop("invoker"),
+		  gettext_noop("Security"));
+		appendPQExpBufferStr(, ",\n  ");
+		printACLColumn(, "p.proacl");
+		appendPQExpBuffer(,
+		  ",\n p.prosrc as \"%s\""
+  ",\n pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"",
 		  gettext_noop("Source code"),
 		  gettext_noop("Description"));
+	}
 
 	appendPQExpBufferStr(,
 		 "\nFROM pg_catalog.pg_proc p"

-- 
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] [CF2016-9] Allow spaces in working path on tap-tests

[Tom Lane]

Michael Paquier  writes:
> Thanks! What you have pushed looks fine to me. Also, the portion for
> src/tools/msvc needs to go further down, I should have precised that
> earlier. Do you want a patch for that?

Yes, please --- I thought it'd all gotten done.

regards, tom lane


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

Re: [HACKERS] pg_hba_lookup function to get all matching pg_hba.conf entries

[Haribabu Kommi]

On Sat, Apr 9, 2016 at 6:36 AM, Tom Lane  wrote:
>
> More generally, I'm not convinced about the use-case for this patch.
> What problem is it supposed to help in dealing with, exactly?  Not syntax
> errors in the hba file, evidently, since it doesn't make any attempt to
> instrument the file parser.  And it's not very clear that it'll help
> with "I can't connect", either, because if you can't connect you're
> not going to be running this function.

Apologies for replying an old thread.

The main reason behind of this patch is for the administrators to control
and verify the authentication mechanism that was deployed for several
users in the database is correctly picking the assigned hba config or not?

I feel this SQL function is useful for administrators and not for normal
users.

If anyone is not against to the above use case, i will update the patch based
on the review comments and post it later.

Regards,
Hari Babu
Fujitsu Australia


-- 
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] doc: Incorrect return type of IsForeignScanParallelSafe in fdwhandler.sgml

[Etsuro Fujita]

On 2016/07/09 1:41, Tom Lane wrote:

Etsuro Fujita  writes:

I noticed that the return type of IsForeignScanParallelSafe described in
fdwhandler.sgml isn't correct; that should be bool, not Size.  Please
find attached a small patch for that.



Pushed, thanks!


Thank you.

Best regards,
Etsuro Fujita




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

Re: [HACKERS] [CF2016-9] Allow spaces in working path on tap-tests

[Michael Paquier]

On Sun, Jul 10, 2016 at 11:52 PM, Tom Lane  wrote:
> Michael Paquier  writes:
>> Thanks! What you have pushed looks fine to me. Also, the portion for
>> src/tools/msvc needs to go further down, I should have precised that
>> earlier. Do you want a patch for that?
>
> Yes, please --- I thought it'd all gotten done.

OK, here are patches for 9.1, 9.2 and 9.3.
-- 
Michael
diff --git a/src/tools/msvc/Install.pm b/src/tools/msvc/Install.pm
index b779b31..bbb5c39 100644
--- a/src/tools/msvc/Install.pm
+++ b/src/tools/msvc/Install.pm
@@ -268,12 +268,19 @@ sub GenerateTimezoneFiles
 my $conf = shift;
 my $mf = read_file("src/timezone/Makefile");
 $mf =~ s{\\\s*[\r\n]+}{}mg;
-$mf =~ /^TZDATA\s*:?=\s*(.*)$/m || die "Could not find TZDATA row in timezone makefile\n";
+$mf =~ /^TZDATA\s*:?=\s*(.*)$/m || die "Could not find TZDATA line in timezone makefile\n";
 my @tzfiles = split /\s+/,$1;
-unshift @tzfiles,'';
 print "Generating timezone files...";
-system(
-"$conf\\zic\\zic -d \"$target/share/timezone\" " . join(" src/timezone/data/", @tzfiles));
+my @args = ("$conf/zic/zic",
+'-d',
+"$target/share/timezone");
+foreach (@tzfiles)
+{
+my $tzfile = $_;
+push(@args, "src/timezone/data/$tzfile")
+}
+
+system(@args);
 print "\n";
 }
 
@@ -490,8 +497,10 @@ sub CopyIncludeFiles
 next unless (-d "src/include/$d");
 
 EnsureDirectories("$target/include/server/$d");
-system(qq{xcopy /s /i /q /r /y src\\include\\$d\\*.h "$ctarget\\include\\server\\$d\\"})
-  && croak("Failed to copy include directory $d\n");
+my @args = ('xcopy', '/s', '/i', '/q', '/r', '/y',
+"src\\include\\$d\\*.h",
+"$ctarget\\include\\server\\$d\\");
+system(@args) && croak("Failed to copy include directory $d\n");
 }
 closedir($D);
 
@@ -545,9 +554,11 @@ sub GenerateNLSFiles
 $lang = $1;
 
 EnsureDirectories($target, "share/locale/$lang", "share/locale/$lang/LC_MESSAGES");
-system(
-"\"$nlspath\\bin\\msgfmt\" -o \"$target\\share\\locale\\$lang\\LC_MESSAGES\\$prgm-$majorver.mo\" $_"
-)&& croak("Could not run msgfmt on $dir\\$_");
+my @args = ("$nlspath\\bin\\msgfmt",
+'-o',
+"$target\\share\\locale\\$lang\\LC_MESSAGES\\$prgm-$majorver.mo",
+$_);
+system(@args) && croak("Could not run msgfmt on $dir\\$_");
 print ".";
 }
 }
diff --git a/src/tools/msvc/Install.pm b/src/tools/msvc/Install.pm
index 7205e23..671d860 100644
--- a/src/tools/msvc/Install.pm
+++ b/src/tools/msvc/Install.pm
@@ -309,12 +309,21 @@ sub GenerateTimezoneFiles
 	my $mf = read_file("src/timezone/Makefile");
 	$mf =~ s{\\\s*[\r\n]+}{}mg;
 	$mf =~ /^TZDATA\s*:?=\s*(.*)$/m
-	  || die "Could not find TZDATA row in timezone makefile\n";
+	  || die "Could not find TZDATA line in timezone makefile\n";
 	my @tzfiles = split /\s+/, $1;
-	unshift @tzfiles, '';
+
 	print "Generating timezone files...";
-	system("$conf\\zic\\zic -d \"$target/share/timezone\" "
-		  . join(" src/timezone/data/", @tzfiles));
+
+	my @args = ("$conf/zic/zic",
+'-d',
+"$target/share/timezone");
+	foreach (@tzfiles)
+	{
+		my $tzfile = $_;
+		push(@args, "src/timezone/data/$tzfile")
+	}
+
+	system(@args);
 	print "\n";
 }
 
@@ -542,9 +551,10 @@ sub CopyIncludeFiles
 		next unless (-d "src/include/$d");
 
 		EnsureDirectories("$target/include/server/$d");
-		system(
-qq{xcopy /s /i /q /r /y src\\include\\$d\\*.h "$ctarget\\include\\server\\$d\\"}
-		) && croak("Failed to copy include directory $d\n");
+		my @args = ('xcopy', '/s', '/i', '/q', '/r', '/y',
+ "src\\include\\$d\\*.h",
+ "$ctarget\\include\\server\\$d\\");
+		system(@args) && croak("Failed to copy include directory $d\n");
 	}
 	closedir($D);
 
@@ -597,9 +607,11 @@ sub GenerateNLSFiles
 
 			EnsureDirectories($target, "share/locale/$lang",
 "share/locale/$lang/LC_MESSAGES");
-			system(
-"\"$nlspath\\bin\\msgfmt\" -o \"$target\\share\\locale\\$lang\\LC_MESSAGES\\$prgm-$majorver.mo\" $_"
-			) && croak("Could not run msgfmt on $dir\\$_");
+			my @args = ("$nlspath\\bin\\msgfmt",
+			   '-o',
+			   "$target\\share\\locale\\$lang\\LC_MESSAGES\\$prgm-$majorver.mo",
+			   $_);
+			system(@args) && croak("Could not run msgfmt on $dir\\$_");
 			print ".";
 		}
 	}
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index adbfa9f..390de4e 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -270,7 +270,7 @@ sub upgradecheck
 	Install($tmp_install, $config);
 	# Install does a chdir, so change back after that
 	chdir $cwd;
-	my ($bindir,$libdir,$oldsrc,$newsrc) = 
+	my ($bindir,$libdir,$oldsrc,$newsrc) =
 	  ("$tmp_install/bin", "$tmp_install/lib", $topdir, $topdir);
 	$ENV{PATH} = 

Re: [HACKERS] can we optimize STACK_DEPTH_SLOP

[Tom Lane]

I wrote:
> So, agreed, let's commit some temporary debug code and see what the
> buildfarm can teach us.  Will go work on that in a bit.

After reviewing the buildfarm results, I'm feeling nervous about this
whole idea again.  For the most part, the unaccounted-for daylight between
the maximum stack depth measured by check_stack_depth and the actually
dirtied stack space reported by pmap is under 100K.  But there are a
pretty fair number of exceptions.  The worst cases I found were on
"dunlin", which approached 200K extra space in a couple of places:

 dunlin| 2016-07-09 22:05:09 | check.log
  | 72667000 268 208 208 rw---   [ stack ]
 dunlin| 2016-07-09 22:05:09 | check.log
  | max measured stack depth 14kB
 dunlin| 2016-07-09 22:05:09 | install-check-C.log  
  | 7fffee65 268 200 200 rw---   [ stack ]
 dunlin| 2016-07-09 22:05:09 | install-check-C.log  
  | max measured stack depth 14kB

This appears to be happening in the tsdicts test script.  Other machines
also show a significant discrepancy between pmap and check_stack_depth
results for that test, which suggests that maybe the tsearch code is being
overly reliant on large local variables.  But I haven't dug through it.

Another area of concern is PLs.  For instance, on capybara, a machine
otherwise pretty unexceptional in stack-space appetite, quite a few of the
PL tests ate ~100K of unaccounted-for space:

 capybara  | 2016-07-09 21:15:56 | pl-install-check-C.log   
  | 7ffc61bbe000 132 104 104 rw---[ stack ]
 capybara  | 2016-07-09 21:15:56 | pl-install-check-C.log   
  | 7ffc61bbe000 132   0   0 rw---[ stack ]
 capybara  | 2016-07-09 21:15:56 | pl-install-check-C.log   
  | max measured stack depth 8kB
 capybara  | 2016-07-09 21:15:56 | pl-install-check-C.log   
  | 7ffc61bbd000 136 136 136 rw---[ stack ]
 capybara  | 2016-07-09 21:15:56 | pl-install-check-C.log   
  | 7ffc61bbd000 136   0   0 rw---[ stack ]
 capybara  | 2016-07-09 21:15:56 | pl-install-check-C.log   
  | max measured stack depth 0kB
 capybara  | 2016-07-09 21:15:56 | pl-install-check-C.log   
  | 7ffc61bbe000 132 104 104 rw---[ stack ]
 capybara  | 2016-07-09 21:15:56 | pl-install-check-C.log   
  | 7ffc61bbe000 132   0   0 rw---[ stack ]
 capybara  | 2016-07-09 21:15:56 | pl-install-check-C.log   
  | max measured stack depth 5kB
 capybara  | 2016-07-09 21:15:56 | pl-install-check-C.log   
  | 7ffc61bbe000 132 116 116 rw---[ stack ]
 capybara  | 2016-07-09 21:15:56 | pl-install-check-C.log   
  | 7ffc61bbe000 132   0   0 rw---[ stack ]
 capybara  | 2016-07-09 21:15:56 | pl-install-check-C.log   
  | max measured stack depth 7kB

Presumably that reflects some oddity of the local version of perl or
python, but I have no idea what.

So while we could possibly get away with reducing STACK_DEPTH_SLOP
to 256K, there is good reason to think that that would be leaving
little or no safety margin.

At this point I'm inclined to think we should leave well enough alone.
At the very least, if we were to try to reduce that number, I'd want
to have some plan for tracking our stack space consumption better than
we have done in the past.

regards, tom lane


PS: for amusement's sake, here are some numbers I extracted concerning
the relative stack-hungriness of different buildfarm members.  First,
the number of recursion levels each machine could accomplish before
hitting "stack too deep" in the errors.sql regression test (measured by
counting the number of CONTEXT lines in the relevant error message):

sysname|  snapshot   | count 
---+-+---
 protosciurus  | 2016-07-10 12:03:06 |   731
 chub  | 2016-07-10 15:10:01 |  1033
 quokka| 2016-07-10 02:17:31 |  1033
 hornet| 2016-07-09 23:42:32 |  1156
 clam  | 2016-07-09 22:00:01 |  1265
 anole | 2016-07-09 22:41:40 |  1413
 spoonbill | 2016-07-09 23:00:05 |  1535
 sungazer  | 2016-07-09 23:51:33 |  1618
 gaur  | 2016-07-09 04:53:13 |  1634
 kouprey   | 2016-07-10 04:58:00 |  1653
 nudibranch| 2016-07-10 09:18:10 |  1664
 grouse| 2016-07-10 08:43:02 |  1708
 sprat | 2016-07-10 08:43:55 |  1717
 pademelon | 2016-07-09 06:12:10 |  1814
 mandrill  | 2016-07-10 00:10:02 |  2093
 gharial   | 2016-07-10 01:15:50 |  2248
 francolin | 2016-07-10 13:00:01 |  2379
 

Re: [HACKERS] Logical decoding

[Craig Ringer]

On 9 July 2016 at 01:57, Joshua Bay  wrote:


> where are the entry points to logical decoding?
>

Well, it depends on what level you're looking at.

Data is read using the xlogreader and fed into the decoding system by the
walsender (when using a logical slot) and the SQL level
pg_logical_slot_[get|peek]_changes functions. See XLogSendLogical() as
called by WalSndLoop() and see pg_logical_slot_get_changes_guts() .

WAL records are passed through to logical decoding and depending on the
xlog entry type the reorder buffer and/or snapshot builder. Then when a
transaction commit is detected the registered output plugin is invoked and
its callbacks are used to process the buffered transaction.


> Specifically, we want to know whether logical decoding happens immediately
> after commit, or whether there is a polling thread that scans the Write
> Ahead Log and then dumps to the special table?
>

There's no "polling thread", but that's probably the closer of the two. A
walsender using a logical slot reads WAL as it becomes available. It sleeps
on a latch if it runs out of WAL to read and is woken when there's more. It
isn't dumped to some special table, it's managed by the reorder buffer
infrastructure which uses its own special data structures. Reading may lag
behind WAL generation since it's "pulled" by the client and happens lazily
after commit.


> and where are this code is in the codebase?
>


src/backend/replication/logical/*
src/backend/replication/walsender.c
src/backend/access/transam/xlogreader.c
src/include/access/xlogreader.h
src/include/replication/output_plugin.h
contrib/test_decoding/
doc/src/sgml/logicaldecoding.sgml




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

Re: [HACKERS] Fix Error Message for allocate_recordbuf() Failure

[Michael Paquier]

On Sat, Jul 9, 2016 at 2:58 AM, Shoaib Lari  wrote:
> Besides making the error message more informative, we had to modify
> allocate_recordbuf() to return the actual number of bytes that were being
> allocated.

-   report_invalid_record(state, "record length %u at %X/%X too long",
- total_len,
- (uint32) (RecPtr >> 32), (uint32) RecPtr);
+   report_invalid_record(state,
+ "cannot allocate %u bytes for record
length %u at %X/%X",
+ newSizeOut, total_len, (uint32) (RecPtr >> 32),
+ (uint32) RecPtr);

It does not look like a good idea to me to complicate the interface of
allocate_recordbuf just to make more verbose one error message,
meaning that it basically a complain related to the fact that
palloc_extended(MCXT_ALLOC_NO_OOM) does not mention to the user the
size that it has tried to allocate before returning NULL. Do you have
a use case that would prove to be useful if this extra piece of
information is provided? Because it seems to me that we don't really
care if we know how much memory it has failed to allocate, we only
want to know that it *has* failed and take actions only based on that.

And even if we make this thing more verbose, a better approach would
be surely to generate a WARNING message for backends in mcxt.c and
have something printed to stderr for frontends in fe_memutils.c
without calling exit().
-- 
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] pgbench - minor fix for meta command only scripts

[Michael Paquier]

On Sat, Jul 9, 2016 at 4:09 PM, Fabien COELHO  wrote:
>
> While testing meta-command pgbench only scripts, I noticed that there is an
> infinite loop in threadRun, which means that other tasks such as reporting
> progress do not get a chance.
>
> The attached patch breaks this loop by always returning at the end of a
> script.
>
> On "pgbench -T 3 -P 1 -f noop.sql", before this patch, the progress is not
> shown, after it is.

You may want to name your patches with .patch or .diff. Using .sql is
disturbing style :)

Indeed, not reporting the progress back to the client in the case of a
script with only meta commands is non-intuitive.

-   /* after a meta command, immediately proceed with next command */
-   goto top;
+   /*
+* After a meta command, immediately proceed with next command...
+* although not if last. This exception ensures that a meta command
+* only script does not always loop in doCustom, so that other tasks
+* in threadRun, eg progress reporting or switching client,
get a chance.
+*/
+   if (commands[st->state + 1] != NULL)
+   goto top;

This looks good to me. I'd just rewrite the comment block with
something like that, more simplified:
+   /*
+* After a meta command, immediately proceed with next command.
+* But if this is the last command, just leave.
+*/
-- 
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: [COMMITTERS] [HACKERS] Logical decoding

[Michael Paquier]

On Mon, Jul 11, 2016 at 2:03 PM, Craig Ringer  wrote:
> On 9 July 2016 at 01:57, Joshua Bay  wrote:
>> and where are this code is in the codebase?
>
> src/backend/replication/logical/*
> src/backend/replication/walsender.c
> src/backend/access/transam/xlogreader.c
> src/include/access/xlogreader.h
> src/include/replication/output_plugin.h
> contrib/test_decoding/
> doc/src/sgml/logicaldecoding.sgml

Some other references:
https://wiki.postgresql.org/images/7/77/Fosdem-2015-logical-decoding.pdf

And some other examples of plugins:
https://github.com/leptonix/decoding-json
https://github.com/xstevens/decoderbufs
https://github.com/confluentinc/bottledwater-pg (for kafka)
https://github.com/michaelpq/pg_plugins/tree/master/decoder_raw (wrote this one)
-- 
Michael


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

Re: [HACKERS] Fix Error Message for allocate_recordbuf() Failure

[Craig Ringer]

On 11 July 2016 at 12:04, Michael Paquier  wrote:

> On Sat, Jul 9, 2016 at 2:58 AM, Shoaib Lari  wrote:
> > Besides making the error message more informative, we had to modify
> > allocate_recordbuf() to return the actual number of bytes that were being
> > allocated.
>
> -   report_invalid_record(state, "record length %u at %X/%X too long",
> - total_len,
> - (uint32) (RecPtr >> 32), (uint32) RecPtr);
> +   report_invalid_record(state,
> + "cannot allocate %u bytes for record
> length %u at %X/%X",
> + newSizeOut, total_len, (uint32) (RecPtr >>
> 32),
> + (uint32) RecPtr);
>
> It does not look like a good idea to me to complicate the interface of
> allocate_recordbuf just to make more verbose one error message,
> meaning that it basically a complain related to the fact that
> palloc_extended(MCXT_ALLOC_NO_OOM) does not mention to the user the
> size that it has tried to allocate before returning NULL. Do you have
> a use case that would prove to be useful if this extra piece of
> information is provided? Because it seems to me that we don't really
> care if we know how much memory it has failed to allocate, we only
> want to know that it *has* failed and take actions only based on that.
>

I actually find details of how much memory we tried to allocate to be
really useful in other places that do emit it. Sometimes it's been an
important clue when trying to figure out what's going on on a remote system
with no or limited access. Even if it just tells me "we were probably
incrementally allocating lots of small values since this failure is small"
vs "this allocation is huge, look for something unusually large" or "this
allocation is impossibly huge / some suspicious value look for memory
corruption".

I tend to agree with your suggestion about a better approach but do think
emitting details on allocation failures is useful. At least when people
turn VM overcommit off ...

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

Re: [HACKERS] Disable WAL completely - Performance and Persistency research

[Craig Ringer]

On 10 July 2016 at 18:27, Netanel Katzburg  wrote:


> BUT, both options are not good, as they are stopping me from even running i
> *nitdb.*
>
>
>
The easiest path for testing will be to use an unpatched PostgreSQL to
`initdb` and create a new database. Then start up a patched one that simply
skips WAL writing against an already-`initdb`'d data directory.

You probably won't be able to safely restart PostgreSQL, but all you're
doing is performance analsys so one-shot operation on a throw-away data
directory is probably fine.


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

Re: [HACKERS] Issue with bgworker, SPI and pgstat_report_stat

[Michael Paquier]

On Mon, Jul 11, 2016 at 3:36 AM, Julien Rouhaud
 wrote:
> I'm not opposed, but in this case we should also provide a proper
> documentation of all the required actions to mimick normal backends.

I'd rather just add a note like "Have a look at PostgresMain if you
want to imitate a backend able to run queries!" instead of listing all
the actions doable. If low-level things are added or removed in the
future in PostgresMain, it is very likely that the documentation will
not be updated at the same time, killing its purpose at the end.
-- 
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] pgbench - minor doc improvements

[Michael Paquier]

On Sat, Jul 9, 2016 at 4:48 PM, Fabien COELHO  wrote:
>
> Minor pgbench documentation improvements so that the description is more
> precise:
>
>  - a pgbench script may not contain SQL commands, it only needs not to be
>empty.

Halfly true as far as I recall. This works and generates two queries:
SELECT 1; \set two 3
SELECT :two;
But any query added after the meta-command cannot be parsed.

>  - point out explicitely variable setting meta commands.
>  - the formula is short enough to fit on a line.

 
-f(x) = PHI(2.0 * parameter * (x - mu) / (max - min + 1)) /
-   (2.0 * PHI(parameter) - 1)
+f(x) = PHI(2.0 * parameter * (x - mu) / (max - min + 1)) / (2.0 *
PHI(parameter) - 1)
At full-length this is 85 characters. But I agree that it is more
readable to put that into a single line. Now we could as well trick
the limit by using "param" instead of "parameter".
-- 
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] Showing parallel status in \df+

[Michael Paquier]

On Mon, Jul 11, 2016 at 12:42 AM, Tom Lane  wrote:
> If we're keeping the "Source code" column, I'd be inclined to keep
> "Language" adjacent to that.  When thinking of a function as a black
> box, both language and source code are implementation details; but
> all the other properties listed here are of interest anyway.

OK, no objections to that. And this gives the attached.

> (Of course, if we were to get rid of "Source code", the point
> would be moot ...)

I still think that having source code is useful for debugging, so I
left it out. Note for the committer who will perhaps pick up this
patch: I left out "Source Code", but feel free to remove it if you
think the contrary. It is easier to remove code than adding it back.
-- 
Michael
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index aeffd63..e7bd2d7 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1380,8 +1380,9 @@ testdb=
 objects are shown; supply a pattern or the S
 modifier to include system objects.
 If the form \df+ is used, additional information
-about each function is shown, including security classification,
-volatility, owner, language, source code and description.
+about each function is shown, including language, volatility,
+parallel mode, owner, security classification, access privileges,
+source code and description.
 
 
 
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 2cdc5ac..8559b68 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -298,7 +298,7 @@ describeFunctions(const char *functypes, const char *pattern, bool verbose, bool
 	PQExpBufferData buf;
 	PGresult   *res;
 	printQueryOpt myopt = pset.popt;
-	static const bool translate_columns[] = {false, false, false, false, true, true, true, false, false, false, false};
+	static const bool translate_columns[] = {false, false, false, false, true, true, false, false, true, false, false, true, false};
 
 	if (strlen(functypes) != strspn(functypes, "antwS+"))
 	{
@@ -410,28 +410,42 @@ describeFunctions(const char *functypes, const char *pattern, bool verbose, bool
 		  gettext_noop("Type"));
 
 	if (verbose)
+	{
 		appendPQExpBuffer(,
-  ",\n CASE WHEN prosecdef THEN '%s' ELSE '%s' END AS \"%s\""
 		  ",\n CASE\n"
 		  "  WHEN p.provolatile = 'i' THEN '%s'\n"
 		  "  WHEN p.provolatile = 's' THEN '%s'\n"
 		  "  WHEN p.provolatile = 'v' THEN '%s'\n"
 		  " END as \"%s\""
-   ",\n  pg_catalog.pg_get_userbyid(p.proowner) as \"%s\",\n"
-		  "  l.lanname as \"%s\",\n"
-		  "  p.prosrc as \"%s\",\n"
-  "  pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"",
-		  gettext_noop("definer"),
-		  gettext_noop("invoker"),
-		  gettext_noop("Security"),
+		  ",\n CASE\n"
+		  "  WHEN p.proparallel = 'r' THEN '%s'\n"
+		  "  WHEN p.proparallel = 's' THEN '%s'\n"
+		  "  WHEN p.proparallel = 'u' THEN '%s'\n"
+		  " END as \"%s\""
+   ",\n pg_catalog.pg_get_userbyid(p.proowner) as \"%s\""
+		  ",\n CASE WHEN prosecdef THEN '%s' ELSE '%s' END AS \"%s\"",
 		  gettext_noop("immutable"),
 		  gettext_noop("stable"),
 		  gettext_noop("volatile"),
 		  gettext_noop("Volatility"),
+		  gettext_noop("restricted"),
+		  gettext_noop("safe"),
+		  gettext_noop("unsafe"),
+		  gettext_noop("Parallel"),
 		  gettext_noop("Owner"),
+		  gettext_noop("definer"),
+		  gettext_noop("invoker"),
+		  gettext_noop("Security"));
+		appendPQExpBufferStr(, ",\n  ");
+		printACLColumn(, "p.proacl");
+		appendPQExpBuffer(,
+		  ",\n l.lanname as \"%s\""
+		  ",\n p.prosrc as \"%s\""
+  ",\n pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"",
 		  gettext_noop("Language"),
 		  gettext_noop("Source code"),
 		  gettext_noop("Description"));
+	}
 
 	appendPQExpBufferStr(,
 		 "\nFROM pg_catalog.pg_proc p"

-- 
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] Issue with bgworker, SPI and pgstat_report_stat

[Craig Ringer]

On 11 July 2016 at 11:49, Michael Paquier  wrote:

> On Mon, Jul 11, 2016 at 3:36 AM, Julien Rouhaud
>  wrote:
> > I'm not opposed, but in this case we should also provide a proper
> > documentation of all the required actions to mimick normal backends.
>
> I'd rather just add a note like "Have a look at PostgresMain if you
> want to imitate a backend able to run queries!" instead of listing all
> the actions doable. If low-level things are added or removed in the
> future in PostgresMain, it is very likely that the documentation will
> not be updated at the same time, killing its purpose at the end.


That sounds like a bug breeding ground. Especially with extensions whose
bgworkers operate across Pg versions, extensions that get updated without
re-checking PostgresMain and don't notice some new housekeeping task, etc.

Rather than encouraging every extension author to copy and paste random
chunks of PostgresMain, probably incorrectly, I really think factoring the
required logic out into something reusable by bgworkers is going to be the
way to go.

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

Re: [HACKERS] pgbench - minor fix for meta command only scripts

[Fabien COELHO]

Hello Michaël,

You may want to name your patches with .patch or .diff. Using .sql is 
disturbing style :)


Indeed! :-)


Indeed, not reporting the progress back to the client in the case of a
script with only meta commands is non-intuitive.

This looks good to me. I'd just rewrite the comment block with
something like that, more simplified:


Ok. Here is an updated version, with a better suffix and a simplified 
comment.


Thanks,

--
Fabien.diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 87fb006..4e7449e 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -2172,8 +2172,12 @@ top:
 st->listen = true;
 		}
 
-		/* after a meta command, immediately proceed with next command */
-		goto top;
+		/*
+		 * After a meta command immediately proceed with next command,
+		 * but if it is the last command, just leave.
+		 */
+		if (commands[st->state + 1] != NULL)
+			goto top;
 	}
 
 	return true;

-- 
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] PSA: Systemd will kill PostgreSQL

[Craig Ringer]

On 11 July 2016 at 01:56, Joshua D. Drake  wrote:

> Hackers,
>
> This just came across my twitter feed:
>
> https://lists.freedesktop.org/archives/systemd-devel/2014-April/018373.html
>
> tl;dr; Systemd 212 defaults to remove all IPC (including SYSV memory) when
> a user "fully" logs out.
>
>
The underlying change sounds like a fix, not a problem. It ensures that
when a user logs out, various dangling processes are cleaned up. Given the
amount of work PostgreSQL has to do to try to make sure it's really gone,
having systemd be able to just clobber everything is pretty nice. So long
as there's control over it.

However, it will break existing deployments that use "non-system" users to
run PostgreSQL. I had a look and didn't find any useful definition of what
systemd considers a "system user". Perhaps by uid threshold in login.defs?
But then what happens for people who're managing users via a directory, who
need to avoid conflicting with host-local UIDs, but also need some of those
users to have systemd "system user" like behaviour?

It's also not clear if there's any API apps can use to exempt themselves
from this, or any wrapper command to spawn processes that aren't clobbered.
With appropriate user privileges to permit it, at least.

I've asked for clarification on the bug, so I'd better don my fire-proof
suit.

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

Re: [HACKERS] pgbench - minor doc improvements

[Fabien COELHO]

Hello Michaël,


Minor pgbench documentation improvements so that the description is more
precise:

 - a pgbench script may not contain SQL commands, it only needs not to be
   empty.


Halfly true as far as I recall. This works and generates two queries:
SELECT 1; \set two 3


Maybe it used to work, but not anymore:

  client 0 aborted in state 0: ERROR:  syntax error at or near "\"
  LINE 1: SELECT 1 ; \set two 3


 - point out explicitely variable setting meta commands.
 - the formula is short enough to fit on a line.



-f(x) = PHI(2.0 * parameter * (x - mu) / (max - min + 1)) /
-   (2.0 * PHI(parameter) - 1)
+f(x) = PHI(2.0 * parameter * (x - mu) / (max - min + 1)) / (2.0 *
PHI(parameter) - 1)
At full-length this is 85 characters.


I agree on principle, however the issue is the "literal layout", the line 
is also broken in the resulting HTML and it looks pretty strange there:


  https://www.postgresql.org/docs/devel/static/pgbench.html

But I agree that it is more readable to put that into a single line. Now 
we could as well trick the limit by using "param" instead of 
"parameter".


ISTM that it would mean changing the text in a dozen places to be 
consistent, and it would not necessary be an improvement in some of those 
places... So I'm hesitant to follow on that suggestion. For me the "under 
80 column" rule cannot really apply on a "literal layout" block.


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

Re: [HACKERS] Showing parallel status in \df+

[Tom Lane]

Michael Paquier  writes:
>> - Reordering the columns, I'd suggest as follows):
>> -- Schema
>> -- Name
>> -- Result data type
>> -- Argument data types
>> -- Type
>> -- Language
>> -- Volatility
>> -- Parallel
>> -- Owner
>> -- Security
>> -- ACL
>> -- Source code
>> -- Description

If we're keeping the "Source code" column, I'd be inclined to keep
"Language" adjacent to that.  When thinking of a function as a black
box, both language and source code are implementation details; but
all the other properties listed here are of interest anyway.

(Of course, if we were to get rid of "Source code", the point
would be moot ...)

regards, tom lane


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

[HACKERS] PSA: Systemd will kill PostgreSQL

[Joshua D. Drake]

Hackers,

This just came across my twitter feed:

https://lists.freedesktop.org/archives/systemd-devel/2014-April/018373.html

tl;dr; Systemd 212 defaults to remove all IPC (including SYSV memory) 
when a user "fully" logs out.


JD


--
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] Issue with bgworker, SPI and pgstat_report_stat

[Julien Rouhaud]

On 08/07/2016 01:53, Michael Paquier wrote:
> On Fri, Jul 8, 2016 at 3:06 AM, Andres Freund  wrote:
>> On 2016-07-07 14:04:36 -0400, Robert Haas wrote:
>>> On Thu, Jul 7, 2016 at 1:52 PM, Julien Rouhaud
>>>  wrote:
 Should a bgworker modifing data have to call pgstat_report_stat() to
 avoid this problem? I don't find any documentation suggesting it, and it
 seems that worker_spi (used as a template for this bgworker and I
 suppose a lot of other one) is also affected.
>>>
>>> That certainly seems like the simplest fix.  Not sure if there's a better 
>>> one.
>>
>> I think a better fix would be to unify the startup & error handling
>> code. We have way to many slightly diverging copies. But that's a bigger
>> task, so I'm not protesting against making a more localized fix.
> 
> It seems to me that the only fix is to have the bgworker call
> pgstat_report_stat() and not mess up with the in-core backend code.
> Personally, I always had the image of a bgworker to be an independent
> process, so when it wants to do something, be it mimicking normal
> backends, it should do it by itself. Take the example of SIGHUP
> processing. If the bgworker does not ProcessConfigFile() no parameters
> updates will happen in the context of the bgworker.
> 

I'm not opposed, but in this case we should also provide a proper
documentation of all the required actions to mimick normal backends.

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


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

Re: [HACKERS] Disable WAL completely - Performance and Persistency research

[Netanel Katzburg]

Hi Michael,

Sorry for the delay,
The answer is yes,

I tried 2 things so far:
1. As I understand:

*XLogRecPtr*
*XLogInsert(RmgrId rmid, uint8 info)*

is the primary insert function in xloginsert.c.
I tried commenting the following line at this function, so I can return a
phony pointer every time the function is called,  just as in bootstrap mode.

*if (IsBootstrapProcessingMode() && rmid != RM_XLOG_ID)*


2. At xlog.c, CopyXLogRecordToWAL(int write_len, bool isLogSwitch,
XLogRecData *rdata,
XLogRecPtr StartPos, XLogRecPtr EndPos), Commenting the memcpy syscall:

...

memcpy(currpos, rdata_data, rdata_len);

...


BUT, both options are not good, as they are stopping me from even running i
*nitdb.*


Maybe someone have a lead regarding changes to be done at xlog.c:

*XLogInsertRecord(XLogRecData *rdata, XLogRecPtr fpw_lsn)*

Any other lead  regarding xloginsert.c is welcomed as well.


Regards,

Netanel

On Thu, Jul 7, 2016 at 4:17 PM, Michael Paquier 
wrote:

> On Thu, Jul 7, 2016 at 5:01 PM, Netanel Katzburg 
> wrote:
> > 1. Disable the WAL by not writing anything to the xlog directory. I don't
> > care about recovery/fault tolerance or PITR/ replication etc at the
> moment.
> > I'm aware that the WAL and checkpoint are bind in many ways and are
> crucial
> > for PG core features.
> > Any guidance on how to do so would be appreciated :)
>
> WAL insertion routines are in xloginsert.c. Did you try to play with those?
> --
> Michael
>

Re: [HACKERS] PSA: Systemd will kill PostgreSQL

[Julien Rouhaud]

On 10/07/2016 19:56, Joshua D. Drake wrote:
> Hackers,
> 
> This just came across my twitter feed:
> 
> https://lists.freedesktop.org/archives/systemd-devel/2014-April/018373.html
> 
> tl;dr; Systemd 212 defaults to remove all IPC (including SYSV memory)
> when a user "fully" logs out.
> 

AFAIK it's only the case if the user is not a system user, and postgres
user should be (at least with community packages).

See https://github.com/systemd/systemd/issues/2039

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


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