Re: [HACKERS] extend pgbench expressions with functions

[Fabien COELHO]

Hello Robert,


And this one generates a core dump:
\set cid debug(-9223372036854775808 / -1)
Floating point exception: 8 (core dumped)


That does not seem acceptable to me. I don't want pgbench to die a 
horrible death if a floating point exception occurs any more than I want 
the server to do the same.  I want the error to be properly caught and 
reported.


Please note that this issue/bug/feature/whatever already exists just for 
these two values (1/2**128 probability), it is not related to this patch 
about functions.


  sh> pgbench --version
  pgbench (PostgreSQL) 9.5.0

  sh> cat div.sql
  \set i -9223372036854775807
  \set i :i - 1
  \set i :i / -1

  sh> pgbench -f div.sql
  starting vacuum...end.
  Floating point exception (core dumped)

I do not think that it is really worth fixing, but I will not prevent 
anyone to fix it.


--
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] Why format() adds double quote?

[Tatsuo Ishii]

> "Daniel Verite"  writes:
>> This boils down to the fact that the current quote_ident gives:
> 
>> =# select quote_ident('test․table');
>>  quote_ident  
>> --
>>  "test․table"
> 
>> whereas the quote_ident patched as proposed gives:
> 
>> =# select quote_ident('test․table');
>>  quote_ident 
>> -
>>  test․table
> 
>> So this is what I don't feel good about.
> 
> This patch was originally proposed as a simple, cost-free change,
> but it's becoming obvious that it is no such thing.  I think
> we should probably reject it and move on.

It seems I opend a can of worms. I'm going to reject my proposal
myself.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

-- 
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] remove wal_level archive

[Peter Eisentraut]

On 1/26/16 10:56 AM, Simon Riggs wrote:
> Removing one of "archive" or "hot standby" will just cause confusion and
> breakage, so neither is a good choice for removal.

I'm pretty sure nothing would break, but I do agree that it could be
confusing.

> What we should do is 
> 1. Map "archive" and "hot_standby" to one level with a new name that
> indicates that it can be used for both/either backup or replication.
>   (My suggested name for the new level is "replica"...)

I have been leaning toward making up a new name, too, but hadn't found a
good one.  I tend to like "replica", though.

> 2. Deprecate "archive" and "hot_standby" so that those will be removed
> in a later release.

If we do 1, then we might as well get rid of the old names right away.



-- 
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] Why format() adds double quote?

[Tatsuo Ishii]

> I've used white space in the example, but I'm concerned about
> punctuation too.
> 
> unicode.org has this helpful paper:
> http://www.unicode.org/L2/L2000/00260-sql.pdf
> which studies Unicode in SQL-99 identifiers.
> 
> The relevant BNF they extracted from the standard looks like this:
> 
> identifier body> ::=
>
>[ {  |  }... ]
> 
>  ::=
>
>| 
> 
>  ::=
> 
> | 
> | 
> | 
> | 
> | 
> | 
> | 
> | 
> 
>  ::=
>  
> 
>  ::= ...
> 
>  ::=
>
>| 
> 
> 
> 
> The current version of quote_ident() plays it safe by implementing
> the rule that, as soon it encounters a character outside
> of US-ASCII, it surrounds the identifier with double quotes, no matter
> to which category or block this character belongs.
> So its output is guaranteed to be compatible with the above grammar.
> 
> The change in the patch is that multibyte characters just don't imply
> quoting.
> 
> But according to the points 1 and 2 of the paper, the first character
> must have the Unicode alphabetic property, and it must not
> have the Unicode combining property.

Good point.

> I'm mostly ignorant in Unicode so I'm not sure of the precise
> implications of having such Unicode properties, but still my
> understanding is that the new quote_ident() ignores these rules,
> so in this sense it could produce outputs that wouldn't be
> compatible with SQL-99.
> 
> Also, here's what we say in the manual about non quoted identifiers:
> http://www.postgresql.org/docs/current/static/sql-syntax-lexical.html
> 
> "SQL identifiers and key words must begin with a letter (a-z, but also
> letters with diacritical marks and non-Latin letters) or an underscore
> (_). Subsequent characters in an identifier or key word can be
> letters, underscores, digits (0-9), or dollar signs ($)"
> 
> So it explicitly allows letters in general  (and also seems less
> strict than SQL-99 about underscore), but it makes no promise about
> Unicode punctuation or spaces, for instance, even though in practice
> the parser seems to accept them just fine.

You could arbitary extend your point, not only with Unicode
punctuation or spaces, There are number of characters look-alike "-"
in Unicode, for example.　Do we want to treat them like ASCII "-"?

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp


-- 
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 numeric_trim(numeric)

[Alvaro Herrera]

Marko Tiikkaja wrote:
> Hi,
> 
> Here's a patch for the second function suggested in 5643125e.1030...@joh.to.

I think this patch got useful feedback but we never saw a followup
version posted.  I closed it as returned-with-feedback.  Feel free to
submit a new version for the 2016-03 commitfest.

-- 
Á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] [PoC] Asynchronous execution again (which is not parallel)

[Robert Haas]

On Thu, Jan 21, 2016 at 4:26 AM, Kyotaro HORIGUCHI
 wrote:
> I put some consideration and trial on callbacks as a means to
> async(early)-execution.

Thanks for working on this.

>> > Suppose we equip each EState with the ability to fire "callbacks".
>> > Callbacks have the signature:
>> >
>> > typedef bool (*ExecCallback)(PlanState *planstate, TupleTableSlot
>> > *slot, void *context);
>> >
>> > Executor nodes can register immediate callbacks to be run at the
>> > earliest possible opportunity using a function like
>> > ExecRegisterCallback(estate, callback, planstate, slot, context).
>> > They can registered deferred callbacks that will be called when a file
>> > descriptor becomes ready for I/O, or when the process latch is set,
>> > using a call like ExecRegisterFileCallback(estate, fd, event,
>> > callback, planstate, slot, context) or
>> > ExecRegisterLatchCallback(estate, callback, planstate, slot, context).
>
> I considered on this. The immediate callbacks seems fine but
> using latch or fds to signal tuple availability doesn't seem to
> fit callbacks stored in estate. They are deferrable until
> parent's tuple request and such kind of events can be handled at
> the time as ExecGather does now. However some kind of
> synchronize/waiting mechanism like latch or select() is needed
> anyway.

I am not entirely sure I understand what you are trying to say here,
but if I do understand it then I disagree.  Consider an Append node
with 1000 children, each a ForeignScan.  What we want to do is fire
off all 1000 remote queries and then return a tuple from whichever
ForeignScan becomes ready first.  What we do NOT want to do is fire
off all 1000 remote queries and have to either (a) iterate through all
1000 subplans checking repeatedly whether each is ready or (b) pick
one of those 1000 subplans to wait for and ignore the other 999 until
the one we pick is ready.  I don't see any way to achieve what we need
here without some way to convince the executor to do a select() across
all 1000 fds and take some action based on which one becomes
read-ready.

> Callback is usable for not-so-common invoked-for-a-event-at-once
> operations such like error-handling. For this case, the
> operations can be asynch-execution of a node and the event can be
> just before ExecProcNode on the topmost node. The first patch
> attached allows async-capable nodes to register callbacks on Init
> phase and executes them just before Exec phase on the topmost
> node. It grately reduces the additional code as the result. My
> first impression from the word "callbacks" is this.

This strikes me as pretty much uninteresting.  I bet if you test this
you'll find that it doesn't make anything faster.  You're kicking
things off asynchronously only a tiny fraction of a second before you
would have started them anyway.  What we need is a way to proceed
asynchronously through the entire execution of the query, not just at
the beginning.  I understand that your merge-join-double-gather
example can benefit from this, but it does so by kicking off both
subplans before we know for sure that we'll want to read from both
subplans.  I admit that optimization rarely kicks in, but it's a
potentially huge savings of effort when it does.

> Instead, in the second patch, I modified ExecProcNode to return
> async status in EState. It will be EXEC_READY or EXEC_EOT(End of
> table/No more tuple?) for non-async-capable nodes and
> async-capable nodes can set it EXEC_NOT_READY, which indicates
> that there could be more tuple but not available yet.
>
> Async-aware nodes such as Append can go to the next child if the
> predecessor returned EXEC_NOT_READY. If all !EXEC_EOT nodes
> returned EXEC_NOT_READY, Append will wait using some signaling
> mechanism (it runs busily now instead.). As an example, the
> second patch modifies ExecAppend to handle it and modified
> ExecSeqScan to return EXEC_NOT_READY by certain probability as an
> emulation of asynchronous tuple fetching. The UNION ALL query
> above returns results stirred among the tree tables as the result.

I think the idea of distinguishing between "end of tuples" and "no
tuples currently ready" is a good one.  I am not particularly excited
about this particular signalling mechanism.  I am not sure why you
want to put this in the EState - isn't that shared between all nodes
in the plan tree, and doesn't that create therefore the risk of
confusion?  What I might imagine is giving ExecProcNode a second
argument that functions as an out parameter and is only meaningful
when TupIsNull(result).  It could for example be a boolean indicating
whether more tuples might become available later.  Maybe an enum is
better, but let's suppose a Boolean for now.  At the top of
ExecProcNode we would do *async_pending = false.  Then, we'd pass
async_pending to ExecWhatever function that is potentially
async-capable and it could set *async_pending = true if it wants.

>> Thanks for the attentive 

Re: [HACKERS] Why format() adds double quote?

[Dickson S. Guedes]

2016-01-26 23:40 GMT-02:00 Tatsuo Ishii :
>> Thanks for advocate, I see here that it even produces that output with
>> simple spaces.
>>
>> postgres=# create table x ("aí  " text);
>> CREATE TABLE
>> postgres=# \d x
>> Tabela "public.x"
>>   Coluna  | Tipo | Modificadores
>> --+--+---
>>  aí   | text |
>>
>>
>> This will break copy user actions and scripts that parses that output.
>>
>> Maybe the patch should consider left/right non-printable chars to
>> choose whether to show or not the " ?
>
> This is a totally different story from the topic discussed in this
> thread. psql never adds double quotations to column name even with
> upper case col names.

Indeed, you are right.

> If you want to change the existing psql's behavior, propose it
> yourself.

It could be interesting, maybe using a \pset quote_columns_char, I'll
think about, thank you.

Best regards.
-- 
Dickson S. Guedes
mail/xmpp: gue...@guedesoft.net - skype: guediz
http://github.com/guedes - http://guedesoft.net
http://www.postgresql.org.br


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

Re: [HACKERS] extend pgbench expressions with functions

[Michael Paquier]

On Thu, Jan 28, 2016 at 7:07 AM, Fabien COELHO  wrote:
> I do not think that it is really worth fixing, but I will not prevent anyone
> to fix it.

I still think it does. Well, if there is consensus to address this one
and optionally the other integer overflows even on back branches, I'll
write a patch and let's call that a deal. This is not a problem from
my side.
-- 
Michael


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

Re: [HACKERS] extend pgbench expressions with functions

[Fabien COELHO]

Hello Robert,


Attached is a rebase after recent changes in pgbench code & doc.


+/* use short names in the evaluator */
+#define INT(v) coerceToInt()
+#define DOUBLE(v) coerceToDouble()
+#define SET_INT(pv, ival) setIntValue(pv, ival)
+#define SET_DOUBLE(pv, dval) setDoubleValue(pv, dval)

I don't like this at all.  It seems to me that this really obscures
the code.  The few extra characters are a small price to pay for not
having to go look up the macro definition to understand what the code
is doing.


Hmmm. Postgres indentation rules for "switch" are peculiar to say the 
least and make it hard to write code that stay under 80 columns. The 
coerceToInt function name looks pretty long (I would rather have 
toInt/toDbl/setInt/setDbl) but I was "told" to use that, so I'm trying to 
find a tradeoff with a macro. Obviously I can substitude and have rather 
long lines that I personally find much uglier.



The third hunk in pgbench.c unnecessary deletes a blank line.


Yep, that is possible.


   /*
* inner expresion in (cut, 1] (if parameter > 0), rand in [0, 1)
+* Assert((1.0 - cut) != 0.0);
*/
-   Assert((1.0 - cut) != 0.0);
   rand = -log(cut + (1.0 - cut) * uniform) / parameter;
+

Moving the Assert() into the comment seems like a bad plan.  If the
Assert is true, it shouldn't be commented out.  If it's not, it
shouldn't be there at all.


I put this assertion when I initially wrote this code, but I think that it 
is proven so I moved it in comment just as a reminder for someone who 
might touch anything that this must hold.



Commit e41beea0ddb74ef975f08b917a354ec33cb60830, which you wrote, went
to some trouble to display good context for error messages.  What you
have here seems like a huge step backwards:

+   fprintf(stderr, "double to int overflow for
%f\n", dval);
+   exit(1);

So, we're just going to give up on all of that error context reporting
that we added back then?  That would be sad.


Well, I'm a lazy programmer, so I'm trying to measure the benefit. IMO 
there is no benefit to better manage this case, especially as the various 
solution I thought of where either ugly and/or had a significant impact on 
the code.


Note that in the best case the error would be detected and reported and 
the client is stopped, and other clients go on... But then, if you started 
a bench and some clients die while running probably your results are 
meaningless, so my opinion is that you are better off with an exit than
with some message that you may miss and performance results computed with 
much less clients than you asked for.


Pgbench is a bench tool, not a production tool.

--
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] [PATCH] we have added support for box type in SP-GiST index

[Alvaro Herrera]

Alexander Lebedev wrote:
> Hello, Hacker.
> 
> * [PATCH] add a box index to sp-gist

I closed this patch as "returned with feedback" because the author
didn't reply in three months.

-- 
Á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] Fwd: Core dump with nested CREATE TEMP TABLE

[Michael Paquier]

On Thu, Jan 28, 2016 at 12:40 PM, Noah Misch  wrote:
> On Sun, Jan 03, 2016 at 12:35:56AM -0500, Noah Misch wrote:
>> On Sat, Jan 02, 2016 at 07:22:13PM -0500, Tom Lane wrote:
>> > Noah Misch  writes:
>> > > I am inclined to add an Assert(portal->status != PORTAL_ACTIVE) to 
>> > > emphasize
>> > > that this is backup only.  MarkPortalActive() callers remain responsible 
>> > > for
>> > > updating the status to something else before relinquishing control.
>> >
>> > No, I do not think that would be an improvement.  There is no contract
>> > saying that this must be done earlier, IMO.
>>
>> Indeed, nobody wrote a contract.  The assertion would record what has been 
>> the
>> sole standing practice for eleven years (since commit a393fbf9).  It would
>> prompt discussion if a proposed patch would depart from that practice, and
>> that is a good thing.  Also, every addition of dead code should label that
>> code to aid future readers.
>
> Here's the patch I have in mind.  AtAbort_Portals() has an older
> MarkPortalFailed() call, and the story is somewhat different there per its new
> comment.  That call is plausible to reach with no bug involved, but
> MarkPortalFailed() would then be the wrong thing.

+ * fresh transaction.  No part of core PostgreSQL functions that way,
+ * though it's a fair thing to want.  Such code would wish the portal
>From the point of view of core code, this stands true, but, for my 2c,
honestly, I think that is just going to annoy more people working on
plugins and forks of Postgres. When working on Postgres-XC and
developing stuff for the core code, I recall having been annoyed a
couple of times by similar assert limitations, because sometimes they
did not actually make sense in the context of what I was doing, and
other times things got suddendly broken after bumping the forks code
base to a major upgrades because a routine used up to now broke its
contract. Perhaps I was doing it wrong at this point though, and
should have used something else.
-- 
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] Trivial doc fix in logicaldecoding.sgml

[Fujii Masao]

On Wed, Jan 27, 2016 at 7:34 PM, Shulgin, Oleksandr
 wrote:
> Hi,
>
> Please find attached a simple copy-paste fix for CREATE_REPLICATION_SLOT
> syntax.

We should change also START_REPLICATION SLOT syntax document as follows?

-  START_REPLICATION SLOT
slot_name LOGICAL
options
+  START_REPLICATION SLOT
slot_name LOGICAL
XXX/XXX
(options)

Regards,

-- 
Fujii Masao


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

Re: [HACKERS] extend pgbench expressions with functions

[Fabien COELHO]

Hello Robert,


Pgbench is a bench tool, not a production tool.


I don't really see how that's relevant.


My point is that I do not see any added value for pgbench to keep on 
executing a performance bench if some clients die due to script errors: it 
is more useful to stop the whole bench and report the issue, so the user 
can fix their script, than to keep going on with the remaining clients, 
from a benchmarking perspective.


So I'm arguing that exiting, with an error message, is better than 
handling user errors.


When I run a program and it dies after causing the operating system to 
send it a fatal signal, I say to myself "self, that program has a bug". 
Other people may have different reactions, but that's mine.


I was talking about an exit call generated on a float to int conversion 
error when there is an error in the user script. The bug is in the user 
script and this is clearly reported by pgbench.


However, your argument may be relevant for avoiding fatal signal such as 
generated by INT64_MAX / -1, because on this one the message error is 
terse so how to fix the issue is not clear to the user.


--
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] [POC] FETCH limited by bytes.

[Robert Haas]

On Mon, Jan 25, 2016 at 4:18 PM, Corey Huinker  wrote:
> Revised in patch v3:
> * get_option() and get_fetch_size() removed, fetch_size searches added to
> existing loops.
> * Move fetch_size <= 0 tests into postgres_fdw_validator() routine in
> option.c
> * DEBUG1 message removed, never intended that to live beyond the proof of
> concept.
> * Missing regression test mentioned in makefile de-mentioned, as there's
> nothing to see without the DEBUG1 message.
> * Multi-line comment shrunk

Looks pretty good.  You seem to have added a blank line at the end of
postgres_fdw.c, which should be stripped back out.

> I'm not too keen on having *no* new regression tests, but defer to your
> judgement.

So... I'm not *opposed* to regression tests.  I just don't see a
reasonable way to write one.  If you've got an idea, I'm all ears.

> Still not sure what you mean by remote execution options. But it might be
> simpler now that the patch is closer to your expectations.

I'm talking about the documentation portion of the patch, which
regrettably seems to have disappeared between v2 and v3.

-- 
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] [PATCH] better systemd integration

[Peter Eisentraut]

On 1/27/16 7:02 AM, Pavel Stehule wrote:
> The issues:
> 
> 1. configure missing systemd integration test, compilation fails:
> 
> postmaster.o postmaster.c
> postmaster.c:91:31: fatal error: systemd/sd-daemon.h: No such file or
> directory

Updated patch attached that fixes this by adding additional checking in
configure.

> 3. PostgreSQL is able to write to systemd log, but multiline entry was
> stored with different priorities

Yeah, as Tom had already pointed out, this doesn't work as I had
imagined it.  I'm withdrawing this part of the patch for now.  I'll come
back to it later.

> Second issue:
> 
> Mapping of levels between pg and journal levels is moved by1

This is the same as how the "syslog" destination works.

From 607323c95ca62d74668992219569c7cff470b63d Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Tue, 17 Nov 2015 06:47:18 -0500
Subject: [PATCH 1/3] Improve error reporting when location specified by
 postgres -D does not exist

Previously, the first error seen would be that postgresql.conf does not
exist.  But for the case where the whole directory does not exist, give
an error message about that, together with a hint for how to create one.
---
 src/backend/utils/misc/guc.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 38ba82f..b8d34b5 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -4463,6 +4463,17 @@ SelectConfigFiles(const char *userDoption, const char *progname)
 	else
 		configdir = make_absolute_path(getenv("PGDATA"));
 
+	if (configdir && stat(configdir, _buf) != 0)
+	{
+		write_stderr("%s: could not access \"%s\": %s\n",
+	 progname,
+	 configdir,
+	 strerror(errno));
+		if (errno == ENOENT)
+			write_stderr("Run initdb or pg_basebackup to initialize a PostgreSQL data directory.\n");
+		return false;
+	}
+
 	/*
 	 * Find the configuration file: if config_file was specified on the
 	 * command line, use it, else use configdir/postgresql.conf.  In any case
@@ -4498,7 +4509,7 @@ SelectConfigFiles(const char *userDoption, const char *progname)
 	 */
 	if (stat(ConfigFileName, _buf) != 0)
 	{
-		write_stderr("%s cannot access the server configuration file \"%s\": %s\n",
+		write_stderr("%s: could not access the server configuration file \"%s\": %s\n",
 	 progname, ConfigFileName, strerror(errno));
 		free(configdir);
 		return false;
-- 
2.7.0

From 339836d39d8566ed794f6e1d56384fd93073d5bf Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Tue, 17 Nov 2015 06:46:17 -0500
Subject: [PATCH 2/3] Add support for systemd service notifications

Insert sd_notify() calls at server start and stop for integration with
systemd.  This allows the use of systemd service units of type "notify",
which greatly simplifies the systemd configuration.
---
 configure   | 38 +
 configure.in|  9 +
 doc/src/sgml/installation.sgml  | 16 
 doc/src/sgml/runtime.sgml   | 24 +++
 src/Makefile.global.in  |  1 +
 src/backend/Makefile|  4 
 src/backend/postmaster/postmaster.c | 21 
 src/include/pg_config.h.in  |  3 +++
 8 files changed, 116 insertions(+)

diff --git a/configure b/configure
index 3dd1b15..5c2e767 100755
--- a/configure
+++ b/configure
@@ -709,6 +709,7 @@ with_libxml
 XML2_CONFIG
 UUID_EXTRA_OBJS
 with_uuid
+with_systemd
 with_selinux
 with_openssl
 krb_srvtab
@@ -830,6 +831,7 @@ with_ldap
 with_bonjour
 with_openssl
 with_selinux
+with_systemd
 with_readline
 with_libedit_preferred
 with_uuid
@@ -1518,6 +1520,7 @@ Optional Packages:
   --with-bonjour  build with Bonjour support
   --with-openssl  build with OpenSSL support
   --with-selinux  build with SELinux support
+  --with-systemd  build with systemd support
   --without-readline  do not use GNU Readline nor BSD Libedit for editing
   --with-libedit-preferred
   prefer BSD Libedit over GNU Readline
@@ -5695,6 +5698,41 @@ fi
 $as_echo "$with_selinux" >&6; }
 
 #
+# Systemd
+#
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether to build with systemd support" >&5
+$as_echo_n "checking whether to build with systemd support... " >&6; }
+
+
+
+# Check whether --with-systemd was given.
+if test "${with_systemd+set}" = set; then :
+  withval=$with_systemd;
+  case $withval in
+yes)
+
+$as_echo "#define USE_SYSTEMD 1" >>confdefs.h
+
+  ;;
+no)
+  :
+  ;;
+*)
+  as_fn_error $? "no argument expected for --with-systemd option" "$LINENO" 5
+  ;;
+  esac
+
+else
+  with_systemd=no
+
+fi
+
+
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $with_systemd" >&5
+$as_echo "$with_systemd" >&6; }
+
+#
 # Readline
 #
 
diff --git a/configure.in b/configure.in
index 9398482..20d9fe1 

Re: [HACKERS] Combining Aggregates

[Robert Haas]

On Sat, Jan 23, 2016 at 6:26 PM, Jeff Janes  wrote:
> On Fri, Jan 22, 2016 at 4:53 PM, David Rowley
>  wrote:
>>
>> It seems that I must have mistakenly believed that non-existing
>> columns for previous versions were handled using the power of magic.
>> Turns out that I was wrong, and they need to be included as dummy
>> columns in the queries for previous versions.
>>
>> The attached fixes the problem.
>
> Yep, that fixes it.

Committed.  Apologies for the delay.

-- 
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] Set search_path + server-prepared statements = cached plan must not change result type

[David G. Johnston]

On Monday, January 25, 2016, Vladimir Sitnikov 
wrote:

> I want to treat 'prepare' operation as an optimization step, so it is
> functionally equivalent to sending a query text.
>
> In other words, I would like backend to track search_path and other
> parameters if necessary transparently‎, creating (caching) different
> execution plans if different plans are required.
> ‎
> Does that make sense?‎
> ‎
>

Prepare creates a plan and a plan has a known output structure.  What you
want is an ability to give a name to a parsed but unplanned query.  This is
not something that prepare should do as it is not a natural extension of
its present responsibility.

Maybe call the new command "PARSE name AS query".

Subsequent prepare commands could refer to named parsed commands to
generate an execution plan in the current context.  If the current context
matches a previously existing plan the command would effectively become a
no-op.  Otherwise a new plan would be generated.  Or, more simply, using
execute and a named parsed query would implicitly perform prepare per the
description above.

I'm not sure how different this is from writing views...though it can be
used for stuff like updates and deletes as well.  You can, I think, already
get something similar by using set from current with a function...

David J.

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

[Noah Misch]

On Sun, Jan 03, 2016 at 12:35:56AM -0500, Noah Misch wrote:
> On Sat, Jan 02, 2016 at 07:22:13PM -0500, Tom Lane wrote:
> > Noah Misch  writes:
> > > I am inclined to add an Assert(portal->status != PORTAL_ACTIVE) to 
> > > emphasize
> > > that this is backup only.  MarkPortalActive() callers remain responsible 
> > > for
> > > updating the status to something else before relinquishing control.
> > 
> > No, I do not think that would be an improvement.  There is no contract
> > saying that this must be done earlier, IMO.
> 
> Indeed, nobody wrote a contract.  The assertion would record what has been the
> sole standing practice for eleven years (since commit a393fbf9).  It would
> prompt discussion if a proposed patch would depart from that practice, and
> that is a good thing.  Also, every addition of dead code should label that
> code to aid future readers.

Here's the patch I have in mind.  AtAbort_Portals() has an older
MarkPortalFailed() call, and the story is somewhat different there per its new
comment.  That call is plausible to reach with no bug involved, but
MarkPortalFailed() would then be the wrong thing.

nm
diff --git a/src/backend/utils/mmgr/portalmem.c 
b/src/backend/utils/mmgr/portalmem.c
index a53673c..632b202 100644
--- a/src/backend/utils/mmgr/portalmem.c
+++ b/src/backend/utils/mmgr/portalmem.c
@@ -762,13 +762,22 @@ AtAbort_Portals(void)
hash_seq_init(, PortalHashTable);
 
while ((hentry = (PortalHashEnt *) hash_seq_search()) != NULL)
{
Portal  portal = hentry->portal;
 
-   /* Any portal that was actually running has to be considered 
broken */
+   /*
+* See similar code in AtSubAbort_Portals().  This would fire 
if code
+* orchestrating multiple top-level transactions within a 
portal, such
+* as VACUUM, caught errors and continued under the same portal 
with a
+* fresh transaction.  No part of core PostgreSQL functions 
that way,
+* though it's a fair thing to want.  Such code would wish the 
portal
+* to remain ACTIVE, as in PreCommit_Portals(); we don't cater 
to
+* that.  Instead, like AtSubAbort_Portals(), interpret this as 
a bug.
+*/
+   Assert(portal->status != PORTAL_ACTIVE);
if (portal->status == PORTAL_ACTIVE)
MarkPortalFailed(portal);
 
/*
 * Do nothing else to cursors held over from a previous 
transaction.
 */
@@ -916,26 +925,29 @@ AtSubAbort_Portals(SubTransactionId mySubid,
if (portal->activeSubid == mySubid)
{
/* Maintain activeSubid until the portal is 
removed */
portal->activeSubid = parentSubid;
 
/*
-* Upper-level portals that failed while 
running in this
-* subtransaction must be forced into FAILED 
state, for the
-* same reasons discussed below.
+* If a bug in a MarkPortalActive() caller has 
it miss cleanup
+* after having failed while running an 
upper-level portal in
+* this subtransaction, we don't know what else 
in the portal
+* is wrong.  Force it into FAILED state, for 
the same reasons
+* discussed below.
 *
 * We assume we can get away without forcing 
upper-level READY
 * portals to fail, even if they were run and 
then suspended.
 * In theory a suspended upper-level portal 
could have
 * acquired some references to objects that are 
about to be
 * destroyed, but there should be sufficient 
defenses against
 * such cases: the portal's original query 
cannot contain such
 * references, and any references within, say, 
cached plans of
 * PL/pgSQL functions are not from active 
queries and should
 * be protected by revalidation logic.
 */
+   Assert(portal->status != PORTAL_ACTIVE);
if (portal->status == PORTAL_ACTIVE)
MarkPortalFailed(portal);
 
/*
 * Also, if we failed it during the current 
subtransaction
 * (either just above, or earlier), reattach 
its resource
@@ -957,14 +969,19 @@ 

Re: [HACKERS] GIN pending list clean up exposure to SQL

[Fujii Masao]

On Thu, Jan 28, 2016 at 5:54 AM, Julien Rouhaud
 wrote:
> On 27/01/2016 10:27, Fujii Masao wrote:
>> On Mon, Jan 25, 2016 at 3:54 PM, Jeff Janes 
>> wrote:
>>> On Wed, Jan 20, 2016 at 6:17 AM, Fujii Masao
>>>  wrote:
 On Sat, Jan 16, 2016 at 7:42 AM, Julien Rouhaud
  wrote:
> On 15/01/2016 22:59, Jeff Janes wrote:
>> On Sun, Jan 10, 2016 at 4:24 AM, Julien Rouhaud
>>  wrote:
>
> All looks fine to me, I flag it as ready for committer.

 When I compiled the PostgreSQL with the patch, I got the
 following error. ISTM that the inclusion of pg_am.h header file
 is missing in ginfast.c.
>>>
>>> Thanks.  Fixed.
>>>
 gin_clean_pending_list() must check whether the server is in
 recovery or not. If it's in recovery, the function must exit
 with an error. That is, IMO, something like the following check
 must be added.

 if (RecoveryInProgress()) ereport(ERROR,

 (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
 errmsg("recovery is in progress"), errhint("GIN pending list
 cannot be cleaned up during recovery.")));

 It's better to make gin_clean_pending_list() check whether the
 target index is temporary index of other sessions or not, like
 pgstatginindex() does.
>>>
>>> I've added both of these checks.  Sorry I overlooked your early
>>> email in this thread about those.
>>>

 +RelationindexRel = index_open(indexoid,
 AccessShareLock);

 ISTM that AccessShareLock is not safe when updating the pending
 list and GIN index main structure. Probaby we should use
 RowExclusiveLock?
>>>
>>> Other callers of the ginInsertCleanup function also do so while
>>> holding only the AccessShareLock on the index.  It turns out
>>> that there is a bug around this, as discussed in "Potential GIN
>>> vacuum bug"
>>> (http://www.postgresql.org/message-id/flat/CAMkU=1xalflhuuohfp5v33rzedlvb5aknnujceum9knbkrb...@mail.gmail.com)
>>>
>>>
>>>
> But, that bug has to be solved at a deeper level than this patch.
>>>
>>> I've also cleaned up some other conflicts, and chose a more
>>> suitable OID for the new catalog function.
>>>
>>> The number of new header includes needed to implement this makes
>>> me wonder if I put this code in the correct file, but I don't see
>>> a better location for it.
>>>
>>> New version attached.
>>
>> Thanks for updating the patch! It looks good to me.
>>
>> Based on your patch, I just improved the doc. For example, I added
>> the following note into the doc.
>>
>> +These functions cannot be executed during recovery. +Use
>> of these functions is restricted to superusers and the owner +
>> of the given index.
>>
>> If there is no problem, I'm thinking to commit this version.
>>
>
> Just a detail:
>
> +Note that the cleanup does not happen and the return value is 0
> +if the argument is the GIN index built with fastupdate
> +option disabled because it doesn't have a pending list.
>
> It should be "if the argument is *a* GIN index"
>
> I find this sentence a little confusing, maybe rephrase like this would
> be better:
>
> -Note that the cleanup does not happen and the return value is 0
> -if the argument is the GIN index built with fastupdate
> -option disabled because it doesn't have a pending list.
> +Note that if the argument is a GIN index built with
> fastupdate
> +option disabled, the cleanup does not happen and the return value
> is 0
> +because the index doesn't have a pending list.
>
> Otherwise, I don't see any problem on this version.

Thanks for the review! I adopted the description you suggested.
Just pushed the patch.

Regards,

-- 
Fujii Masao


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

[Etsuro Fujita]

On 2016/01/27 21:23, Rushabh Lathia wrote:

If I understood correctly, above documentation means, that if FDW have
DMLPushdown APIs that is enough. But in reality thats not the case, we
need  ExecForeignInsert, ExecForeignUpdate, or ExecForeignDelete in case
DML is not pushable.

And here fact is DMLPushdown APIs are optional for FDW, so that if FDW
don't have DMLPushdown APIs they can still very well perform the DML
operations using ExecForeignInsert, ExecForeignUpdate, or
ExecForeignDelete.


I agree with you.  I guess I was wrong. sorry.


So documentation should be like:

If the IsForeignRelUpdatable pointer is set to NULL, foreign tables are
assumed to be insertable, updatable, or deletable if the FDW provides
ExecForeignInsert, ExecForeignUpdate, or ExecForeignDelete respectively,

If FDW provides DMLPushdown APIs and the DML are pushable to the foreign
server, then FDW still needs ExecForeignInsert, ExecForeignUpdate, or
ExecForeignDelete for the non-pushable DML operation.

What's your opinion ?


I agree that we should add this to the documentation, too.

BTW, if I understand correctly, I think we should also modify 
relation_is_updatabale() accordingly.  Am I right?


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] Implementing a new Scripting Language

[Chapman Flack]

On 01/27/16 16:31, Igal @ Lucee.org wrote:

> This can be a major thing.  I will open a ticket in
> https://github.com/tada/pljava -- or is it already on the roadmap?

Now that you mention it, it isn't officially in a ticket. Though it's
not like I was going to forget. :)  I can guarantee it won't be in 1.5...

Speaking of tickets, I should probably make actual tickets, for after
1.5.0, out of all the items now showing in the "known issues" section
of the draft release notes. More chores

-Chap


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

Re: [HACKERS] RFC: replace pg_stat_activity.waiting with something more descriptive

[Amit Kapila]

On Thu, Jan 28, 2016 at 2:12 AM, Robert Haas  wrote:
>
> On Tue, Jan 26, 2016 at 3:10 AM, and...@anarazel.de 
wrote:
> > I do think there's a considerable benefit in improving the
> > instrumentation here, but his strikes me as making live more complex for
> > more users than it makes it easier. At the very least this should be
> > split into two fields (type & what we're actually waiting on). I also
> > strongly suspect we shouldn't use in band signaling ("process not
> > waiting"), but rather make the field NULL if we're not waiting on
> > anything.
>
> +1 for splitting it into two fields.
>

I will take care of this.

>
> Regarding making the field NULL, someone (I think you) proposed
> previously that we should have one field indicating whether we are
> waiting, and a separate field (or two) indicating the current or most
> recent wait event.
>

I think to do that way we need to change the meaning of pg_stat_activity.
waiting from waiting on locks to waiting on HWLocks and LWLocks and
other events which we add in future.  That can be again other source of
confusion where if existing users of pg_stat_activity.waiting are still
relying on it, they can get wrong information or if they are aware of the
recent change, then they need to add additional check like
(waiting = true && wait_event_type = 'HWLock')).  So I think it is better to
go with suggestion where we can display NULL in the new field when
there is no-wait.


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

Re: Odd behavior in foreign table modification (Was: Re: [HACKERS] Optimization for updating foreign tables in Postgres FDW)

[Robert Haas]

On Thu, Jan 21, 2016 at 4:05 AM, Etsuro Fujita
 wrote:
>> By the way, I'm not too sure I understand the need for the core
>> changes that are part of this patch, and I think that point merits
>> some discussion.  Whenever you change core like this, you're changing
>> the contract between the FDW and core; it's not just postgres_fdw that
>> needs updating, but every FDW.  So we'd better be pretty sure we need
>> these changes and they are adequately justified before we think about
>> putting them into the tree.  Are these core changes really needed
>> here, or can we fix this whole issue in postgres_fdw and leave the
>> core code alone?
>
> Well, if we think it is the FDW's responsibility to insert a valid value for
> tableoid in the returned slot during ExecForeignInsert, ExecForeignUpdate or
> ExecForeignDelete, we don't need those core changes.  However, I think it
> would be better that that's done by ModifyTable in the same way as
> ForeignScan does in ForeignNext, IMO. That eliminates the need for
> postgres_fdw or any other FDW to do that business in the callback routines.

I'm not necessarily opposed to the core changes, but I want to
understand better what complexity they are avoiding.  Can you send a
version of this patch that only touches postgres_fdw, so I can
compare?

-- 
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] Implementing a new Scripting Language

[Chapman Flack]

On 01/27/16 22:26, Igal @ Lucee.org wrote:

> If I can help with anything with the pl/Java project I'd love to help.

Man, you do NOT know what you just walked into.  :D

The most imminent thing I see happening is s/1.5.0-SNAPSHOT/1.5.0-BETA1
which is not far off, so testing is always good. Maybe a good opportunity
for you to try it out, get some Lucee code running ... but by separately
compiling and installing jars at first, no JSR two-twenty-three for the
near future. See if it even does what you're hoping it'll do. Depending
on what's reported during beta, there could be delightful diagnostic
diversions too.

I guess getting some time in playing with PostgreSQL and installing
PL/Java would be the right way to start.  Discussion that's specific
to PL/Java and might not interest all of -hackers can also happen on
pljava-...@lists.pgfoundry.org.

Cheers,
-Chap


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

Re: [HACKERS] extend pgbench expressions with functions

[Robert Haas]

On Wed, Jan 27, 2016 at 5:43 PM, Fabien COELHO  wrote:
> Pgbench is a bench tool, not a production tool.

I don't really see how that's relevant.  When I run a program and it
dies after causing the operating system to send it a fatal signal, I
say to myself "self, that program has a bug".  Other people may have
different reactions, but that's mine.

-- 
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] WAL Re-Writes

[Amit Kapila]

On Thu, Jan 28, 2016 at 1:34 AM, james  wrote:

> On 27/01/2016 13:30, Amit Kapila wrote:
>
>>
>> Thoughts?
>>
>> Are the decreases observed with SSD as well as spinning rust?
>
>
The test is done with WAL on SSD and data on spinning rust, but I
think the results should be similar if we would have done it
otherwise as well.  Having said that, I think still it is worth-while to
test it that way and I will do it.


> I might imagine that decreasing the wear would be advantageous,


Yes.


> especially if the performance decrease is less with low read latency.
>
>
Let me clarify again here that with 4096 bytes chunk size, there is
no performance decrease observed and rather there is a performance
increase though relatively-small (1~5%) and there is a reduction of ~35%
disk writes.  Only if we do exact writes or write with smaller chunk size
like (512 or 1024 bytes, basically lesser than OS block size), then we can
see performance decrease mainly for wal_level < ARCHIVE, but then
writes are much more smaller.  I would also like to mention that what we
call reduction in disk writes, this is the 7th column in stat file [1]
(write sectors - number of sectors written, for details you can refer
documentation of stat file [1]).


[1] - https://www.kernel.org/doc/Documentation/block/stat.txt

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

Re: [HACKERS] Set search_path + server-prepared statements = cached plan must not change result type

[Robert Haas]

On Mon, Jan 25, 2016 at 2:11 PM, Vladimir Sitnikov
 wrote:
> I want to treat 'prepare' operation as an optimization step, so it is 
> functionally equivalent to sending a query text.
>
> In other words, I would like backend to track search_path and other 
> parameters if necessary transparently‎, creating (caching) different 
> execution plans if different plans are required.
> ‎
> Does that make sense?‎

Hmm, so in your example, you actually want replanning to be able to
change the cached plan's result type?

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


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

Re: [HACKERS] Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

[Robert Haas]

On Thu, Jan 21, 2016 at 5:55 AM, Etsuro Fujita
 wrote:
> On 2016/01/21 7:04, Alvaro Herrera wrote:
>> Etsuro Fujita wrote:
>>>
>>> On second thought, I noticed that detecting whether we see a system
>>> column
>>> that way needs more cycles in cases where the reltargetlist and the
>>> restriction clauses don't contain any system columns.  ISTM that such
>>> cases
>>> are rather common, so I'm inclined to keep that code as-is.
>
>> Ah, I see now what you did there. I was thinking you'd have the
>> foreach(restrictinfo) loop, then once the loop is complete scan the
>> bitmapset; not scan the bitmap set scan inside the other loop.
>
> Ah, I got to the point.  I think that is a good idea.  The additional cycles
> for the worst case are bounded and negligible.  Please find attached an
> updated patch.

I don't think this is a good idea.  Most of the time, no system
columns will be present, and we'll just be scanning the Bitmapset
twice rather than once.  Sure, that doesn't take many extra cycles,
but if the point of all this is to micro-optimize this code, that
particular change is going in the wrong direction.

-- 
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] Fwd: Core dump with nested CREATE TEMP TABLE

[Robert Haas]

On Wed, Jan 27, 2016 at 10:40 PM, Noah Misch  wrote:
> On Sun, Jan 03, 2016 at 12:35:56AM -0500, Noah Misch wrote:
>> On Sat, Jan 02, 2016 at 07:22:13PM -0500, Tom Lane wrote:
>> > Noah Misch  writes:
>> > > I am inclined to add an Assert(portal->status != PORTAL_ACTIVE) to 
>> > > emphasize
>> > > that this is backup only.  MarkPortalActive() callers remain responsible 
>> > > for
>> > > updating the status to something else before relinquishing control.
>> >
>> > No, I do not think that would be an improvement.  There is no contract
>> > saying that this must be done earlier, IMO.
>>
>> Indeed, nobody wrote a contract.  The assertion would record what has been 
>> the
>> sole standing practice for eleven years (since commit a393fbf9).  It would
>> prompt discussion if a proposed patch would depart from that practice, and
>> that is a good thing.  Also, every addition of dead code should label that
>> code to aid future readers.
>
> Here's the patch I have in mind.  AtAbort_Portals() has an older
> MarkPortalFailed() call, and the story is somewhat different there per its new
> comment.  That call is plausible to reach with no bug involved, but
> MarkPortalFailed() would then be the wrong thing.

+Assert(portal->status != PORTAL_ACTIVE);
 if (portal->status == PORTAL_ACTIVE)
 MarkPortalFailed(portal);

Now that just looks kooky to me.  We assert that the portal isn't
active, but then cater to the possibility that it might be anyway?
That seems totally contrary to our usual programming practices, and a
bad idea for that reason.

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

[Rushabh Lathia]

On Thu, Jan 28, 2016 at 11:33 AM, Etsuro Fujita  wrote:

> On 2016/01/27 21:23, Rushabh Lathia wrote:
>
>> If I understood correctly, above documentation means, that if FDW have
>> DMLPushdown APIs that is enough. But in reality thats not the case, we
>> need  ExecForeignInsert, ExecForeignUpdate, or ExecForeignDelete in case
>> DML is not pushable.
>>
>> And here fact is DMLPushdown APIs are optional for FDW, so that if FDW
>> don't have DMLPushdown APIs they can still very well perform the DML
>> operations using ExecForeignInsert, ExecForeignUpdate, or
>> ExecForeignDelete.
>>
>
> I agree with you.  I guess I was wrong. sorry.
>
> So documentation should be like:
>>
>> If the IsForeignRelUpdatable pointer is set to NULL, foreign tables are
>> assumed to be insertable, updatable, or deletable if the FDW provides
>> ExecForeignInsert, ExecForeignUpdate, or ExecForeignDelete respectively,
>>
>> If FDW provides DMLPushdown APIs and the DML are pushable to the foreign
>> server, then FDW still needs ExecForeignInsert, ExecForeignUpdate, or
>> ExecForeignDelete for the non-pushable DML operation.
>>
>> What's your opinion ?
>>
>
> I agree that we should add this to the documentation, too.
>
> BTW, if I understand correctly, I think we should also modify
> relation_is_updatabale() accordingly.  Am I right?
>

Yep, we need to modify relation_is_updatable().


>
> Best regards,
> Etsuro Fujita
>
>
>


-- 
Rushabh Lathia

Re: [HACKERS] [POC] FETCH limited by bytes.

[Corey Huinker]

>
>
> Looks pretty good.  You seem to have added a blank line at the end of
> postgres_fdw.c, which should be stripped back out.
>

Done.


>
> > I'm not too keen on having *no* new regression tests, but defer to your
> > judgement.
>
> So... I'm not *opposed* to regression tests.  I just don't see a
> reasonable way to write one.  If you've got an idea, I'm all ears.
>

The possible tests might be:
- creating a server/table with fetch_size set
- altering an existing server/table to have fetch_size set
- verifying that the value exists in pg_foreign_server.srvoptions and
pg_foreign_table.ftoptions.
- attempting to set fetch_size to a non-integer value

None of which are very interesting, and none of which actually test what
fetch_size was actually used.

But I'm happy to add any of those that seem worthwhile.


> > Still not sure what you mean by remote execution options. But it might be
> > simpler now that the patch is closer to your expectations.
>
> I'm talking about the documentation portion of the patch, which
> regrettably seems to have disappeared between v2 and v3.
>

Ah, didn't realize you were speaking about the documentation, and since
that section was new, I wasn't familiar with the name. Moved.

...and not sure why the doc change didn't make it into the last patch, but
it's in this one.

I also moved the paragraph starting "When using the extensions option, *it
is the user's responsibility* that..." such that it is in the same
varlistentry as "extensions", though maybe that change should be delegated
to the patch that created the extensions option.

Enjoy.
diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c
index 86a5789..f89de2f 100644
--- a/contrib/postgres_fdw/option.c
+++ b/contrib/postgres_fdw/option.c
@@ -131,6 +131,17 @@ postgres_fdw_validator(PG_FUNCTION_ARGS)
 			/* check list syntax, warn about uninstalled extensions */
 			(void) ExtractExtensionList(defGetString(def), true);
 		}
+		else if (strcmp(def->defname, "fetch_size") == 0)
+		{
+			int		fetch_size;
+
+			fetch_size = strtol(defGetString(def), NULL,10);
+			if (fetch_size <= 0)
+ereport(ERROR,
+		(errcode(ERRCODE_SYNTAX_ERROR),
+		 errmsg("%s requires a non-negative integer value",
+def->defname)));
+		}
 	}
 
 	PG_RETURN_VOID();
@@ -162,6 +173,9 @@ InitPgFdwOptions(void)
 		/* updatable is available on both server and table */
 		{"updatable", ForeignServerRelationId, false},
 		{"updatable", ForeignTableRelationId, false},
+		/* fetch_size is available on both server and table */
+		{"fetch_size", ForeignServerRelationId, false},
+		{"fetch_size", ForeignTableRelationId, false},
 		{NULL, InvalidOid, false}
 	};
 
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 374faf5..f71bf61 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -68,7 +68,9 @@ enum FdwScanPrivateIndex
 	/* SQL statement to execute remotely (as a String node) */
 	FdwScanPrivateSelectSql,
 	/* Integer list of attribute numbers retrieved by the SELECT */
-	FdwScanPrivateRetrievedAttrs
+	FdwScanPrivateRetrievedAttrs,
+	/* Integer representing the desired fetch_size */
+	FdwScanPrivateFetchSize
 };
 
 /*
@@ -126,6 +128,8 @@ typedef struct PgFdwScanState
 	/* working memory contexts */
 	MemoryContext batch_cxt;	/* context holding current batch of tuples */
 	MemoryContext temp_cxt;		/* context for per-tuple temporary data */
+
+	int			fetch_size;		/* number of tuples per fetch */
 } PgFdwScanState;
 
 /*
@@ -380,6 +384,7 @@ postgresGetForeignRelSize(PlannerInfo *root,
 	fpinfo->fdw_startup_cost = DEFAULT_FDW_STARTUP_COST;
 	fpinfo->fdw_tuple_cost = DEFAULT_FDW_TUPLE_COST;
 	fpinfo->shippable_extensions = NIL;
+	fpinfo->fetch_size = 100;
 
 	foreach(lc, fpinfo->server->options)
 	{
@@ -394,16 +399,17 @@ postgresGetForeignRelSize(PlannerInfo *root,
 		else if (strcmp(def->defname, "extensions") == 0)
 			fpinfo->shippable_extensions =
 ExtractExtensionList(defGetString(def), false);
+		else if (strcmp(def->defname, "fetch_size") == 0)
+			fpinfo->fetch_size = strtol(defGetString(def), NULL,10);
 	}
 	foreach(lc, fpinfo->table->options)
 	{
 		DefElem*def = (DefElem *) lfirst(lc);
 
 		if (strcmp(def->defname, "use_remote_estimate") == 0)
-		{
 			fpinfo->use_remote_estimate = defGetBoolean(def);
-			break;/* only need the one value */
-		}
+		else if (strcmp(def->defname, "fetch_size") == 0)
+			fpinfo->fetch_size = strtol(defGetString(def), NULL,10);
 	}
 
 	/*
@@ -1069,6 +1075,9 @@ postgresGetForeignPlan(PlannerInfo *root,
 	 */
 	fdw_private = list_make2(makeString(sql.data),
 			 retrieved_attrs);
+	fdw_private = list_make3(makeString(sql.data),
+			 retrieved_attrs,
+			 makeInteger(fpinfo->fetch_size));
 
 	/*
 	 * Create the ForeignScan node from target list, filtering expressions,
@@ -1147,6 +1156,8 @@ postgresBeginForeignScan(ForeignScanState *node, int eflags)
 	 FdwScanPrivateSelectSql));
 

Re: [HACKERS] remove wal_level archive

[Michael Paquier]

On Thu, Jan 28, 2016 at 10:53 AM, Peter Eisentraut  wrote:
> On 1/26/16 10:56 AM, Simon Riggs wrote:
>> What we should do is
>> 1. Map "archive" and "hot_standby" to one level with a new name that
>> indicates that it can be used for both/either backup or replication.
>>   (My suggested name for the new level is "replica"...)
>
> I have been leaning toward making up a new name, too, but hadn't found a
> good one.  I tend to like "replica", though.

"replica" sounds nice.

>> 2. Deprecate "archive" and "hot_standby" so that those will be removed
>> in a later release.
>
> If we do 1, then we might as well get rid of the old names right away.

+1.
-- 
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: CustomScan in a larger structure (RE: [HACKERS] CustomScan support on readfuncs.c)

[Robert Haas]

On Mon, Jan 25, 2016 at 8:06 PM, Kouhei Kaigai  wrote:
> Sorry for my late response. I've been unavailable to have enough
> time to touch code for the last 1.5 month.
>
> The attached patch is a revised one to handle private data of
> foregn/custom scan node more gracefully.
>
> The overall consensus upthread were:
> - A new ExtensibleNodeMethods structure defines a unique name
>   and a set of callbacks to handle node copy, serialization,
>   deserialization and equality checks.
> - (Foreign|Custom)(Path|Scan|ScanState) are first host of the
>   ExtensibleNodeMethods, to allow extension to define larger
>   structure to store its private fields.
> - ExtensibleNodeMethods does not support variable length
>   structure (like a structure with an array on the tail, use
>   separately allocated array).
> - ExtensibleNodeMethods shall be registered on _PG_init() of
>   extensions.
>
> The 'pgsql-v9.6-custom-private.v3.patch' is the main part of
> this feature. As I pointed out before, it uses dynhash instead
> of the self invented hash table.

On a first read-through, I see nothing in this patch to which I would
want to object except for the fact that the comments and documentation
need some work from a native speaker of English.  It looks like what
we discussed, and I think it's an improvement over what we have now.

-- 
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] Fwd: Core dump with nested CREATE TEMP TABLE

[Noah Misch]

On Wed, Jan 27, 2016 at 11:04:33PM -0500, Robert Haas wrote:
> +Assert(portal->status != PORTAL_ACTIVE);
>  if (portal->status == PORTAL_ACTIVE)
>  MarkPortalFailed(portal);
> 
> Now that just looks kooky to me.  We assert that the portal isn't
> active, but then cater to the possibility that it might be anyway?

Right.

> That seems totally contrary to our usual programming practices, and a
> bad idea for that reason.

It is contrary to our usual programming practices, I agree.  I borrowed the
idea from untenured code (da3751c8, 2015-11-11) in load_relcache_init_file():

if (nailed_rels != NUM_CRITICAL_SHARED_RELS ||
nailed_indexes != NUM_CRITICAL_SHARED_INDEXES)
{
elog(WARNING, "found %d nailed shared rels and %d 
nailed shared indexes in init file, but expected %d and %d respectively",
 nailed_rels, nailed_indexes,
 NUM_CRITICAL_SHARED_RELS, 
NUM_CRITICAL_SHARED_INDEXES);
/* Make sure we get developers' attention about this */
Assert(false);

I liked this pattern.  It's a good fit for cases that we design to be
impossible yet for which we have a workaround if they do happen.  That being
said, if you feel it's bad, I would be fine using elog(FATAL).  I envision
this as a master-only change in any case.  No PGXN module references
PORTAL_ACTIVE or MarkPortalActive(), so it's unlikely that extension code will
notice the change whether in Assert() form or in elog() form.  What is best?


-- 
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] Implementing a new Scripting Language

[Igal @ Lucee.org]

On 1/27/2016 8:07 PM, Chapman Flack wrote:

I guess getting some time in playing with PostgreSQL and installing
PL/Java would be the right way to start.  Discussion that's specific
to PL/Java and might not interest all of -hackers can also happen on
pljava-...@lists.pgfoundry.org.


Sounds like a good place to start.

I will continue the conversation at pljava-...@lists.pgfoundry.org


Igal


--
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] CustomScan under the Gather node?

[Kouhei Kaigai]

> On Tue, Jan 26, 2016 at 1:30 AM, Kouhei Kaigai  wrote:
> > What enhancement will be necessary to implement similar feature of
> > partial seq-scan using custom-scan interface?
> >
> > It seems to me callbacks on the three points below are needed.
> > * ExecParallelEstimate
> > * ExecParallelInitializeDSM
> > * ExecParallelInitializeWorker
> >
> > Anything else?
> > Does ForeignScan also need equivalent enhancement?
> 
> For postgres_fdw, running the query from a parallel worker would
> change the transaction semantics.  Suppose you begin a transaction,
> UPDATE data on the foreign server, and then run a parallel query.  If
> the leader performs the ForeignScan it will see the uncommitted
> UPDATE, but a worker would have to make its own connection which not
> be part of the same transaction and which would therefore not see the
> update.  That's a problem.
>
Ah, yes, as long as FDW driver ensure the remote session has no
uncommitted data, pg_export_snapshot() might provide us an opportunity,
however, once a session writes something, FDW driver has to prohibit it.

> Also, for postgres_fdw, and many other FDWs I suspect, the assumption
> is that most of the work is being done on the remote side, so doing
> the work in a parallel worker doesn't seem super interesting.  Instead
> of incurring transfer costs to move the data from remote to local, we
> incur two sets of transfer costs: first remote to local, then worker
> to leader.  Ouch.  I think a more promising line of inquiry is to try
> to provide asynchronous execution when we have something like:
> 
> Append
> -> Foreign Scan
> -> Foreign Scan
> 
> ...so that we can return a row from whichever Foreign Scan receives
> data back from the remote server first.
> 
> So it's not impossible that an FDW author could want this, but mostly
> probably not.  I think.
>
Yes, I also have same opinion. Likely, local parallelism is not
valuable for the class of FDWs that obtains data from the remote
server (e.g, postgres_fdw, ...), expect for the case when packing
and unpacking cost over the network is major bottleneck.

On the other hands, it will be valuable for the class of FDW that
performs as a wrapper to local data structure, as like current
partial seq-scan doing. (e.g, file_fdw, ...)
Its data source is not under the transaction control, and 'remote
execution' of these FDWs are eventually executed on the local
computing resources.

If I would make a proof-of-concept patch with interface itself, it
seems to me file_fdw may be a good candidate for this enhancement.
It is not a field for postgres_fdw.

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


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

Re: [HACKERS] insert/update performance

[Jinhua Luo]

>
> But what kind of rows would satisfy heap_page_prune() and what would not?
>
> In my case all updates are doing the same thing (there is no HOT
> updates, obviously), but why some updated rows are reported by
> heap_page_prune() but the others are not? And it's also a random
> issue. That means sometimes heap_page_prune() would report all
> removable rows, and sometimes it reports no rows.
>

I check the codes again.

The heap_page_prune() would skip items if ItemIdIsDead() returns true.

That means some obsoleted items are flagged dead before vacuum, and I
found 3 places:

1) heap_page_prune_opt() --> heap_page_prune() --> ItemIdSetDead()
2) _bt_check_unique() --> ItemIdMarkDead()
3) _bt_killitems() --> ItemIdMarkDead()

In my case, the first one happens most frequently.
And it's interesting that it's invoked from select statement!

 0x80ca000 : heap_page_prune_opt+0x0/0x1a0
 0x80d030d : index_fetch_heap+0x11d/0x140
 0x80d035e : index_getnext+0x2e/0x40
 0x81eec9b : IndexNext+0x3b/0x100
 0x81e4ddf : ExecScan+0x15f/0x290
 0x81eed8d : ExecIndexScan+0x2d/0x50
 0x81ddb20 : ExecProcNode+0x1f0/0x2a0
 0x81dac6c : standard_ExecutorRun+0xfc/0x160
 0x82d0503 : PortalRunSelect+0x183/0x200
 0x82d17da : PortalRun+0x26a/0x3c0
 0x82cf452 : PostgresMain+0x2282/0x2fc0
 0x8097f52 : ServerLoop+0xb1b/0xec2
 0x82793d7 : PostmasterMain+0x1237/0x13c0
 0x8098b6c : main+0x48c/0x4d4
 0xb754fa83 : __libc_start_main+0xf3/0x210
 0x8098bd5 : _start+0x21/0x2c


Regards,
Jinhua Luo


-- 
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] Why format() adds double quote?

[Daniel Verite]

Tatsuo Ishii wrote:

> 2) What does the SQL standard say? Do they say that non ASCII white
>   spaces should be treated as ASCII white spaces?

I've used white space in the example, but I'm concerned about
punctuation too.

unicode.org has this helpful paper:
http://www.unicode.org/L2/L2000/00260-sql.pdf
which studies Unicode in SQL-99 identifiers.

The relevant BNF they extracted from the standard looks like this:

identifier body> ::=
   
   [ {  |  }... ]

 ::=
   
   | 

 ::=

| 
| 
| 
| 
| 
| 
| 
| 

 ::=
 

 ::= ...

 ::=
   
   | 



The current version of quote_ident() plays it safe by implementing
the rule that, as soon it encounters a character outside
of US-ASCII, it surrounds the identifier with double quotes, no matter
to which category or block this character belongs.
So its output is guaranteed to be compatible with the above grammar.

The change in the patch is that multibyte characters just don't imply
quoting.

But according to the points 1 and 2 of the paper, the first character
must have the Unicode alphabetic property, and it must not
have the Unicode combining property.

I'm mostly ignorant in Unicode so I'm not sure of the precise
implications of having such Unicode properties, but still my
understanding is that the new quote_ident() ignores these rules,
so in this sense it could produce outputs that wouldn't be
compatible with SQL-99.

Also, here's what we say in the manual about non quoted identifiers:
http://www.postgresql.org/docs/current/static/sql-syntax-lexical.html

"SQL identifiers and key words must begin with a letter (a-z, but also
letters with diacritical marks and non-Latin letters) or an underscore
(_). Subsequent characters in an identifier or key word can be
letters, underscores, digits (0-9), or dollar signs ($)"

So it explicitly allows letters in general  (and also seems less
strict than SQL-99 about underscore), but it makes no promise about
Unicode punctuation or spaces, for instance, even though in practice
the parser seems to accept them just fine.


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] Existence check for suitable index in advance when concurrently refreshing.

[Masahiko Sawada]

On Wed, Jan 27, 2016 at 4:42 PM, Fujii Masao  wrote:
> On Tue, Jan 26, 2016 at 9:33 PM, Masahiko Sawada  
> wrote:
>> Hi all,
>>
>> In concurrently refreshing materialized view, we check whether that
>> materialized view has suitable index(unique and not having WHERE
>> condition), after filling data to new snapshot
>> (refresh_matview_datafill()).
>> This logic leads to taking a lot of time until postgres returns ERROR
>> log if that table doesn't has suitable index and table is large. it
>> wastes time.
>> I think we should check whether that materialized view can use
>> concurrently refreshing or not in advance.
>
> +1
>
>> The patch is attached.
>>
>> Please give me feedbacks.

Thank you for having look at this patch.

> +indexRel = index_open(indexoid, RowExclusiveLock);
>
> Can we use AccessShareLock here, instead?

Yeah, I think we can use it. Fixed.

> +if (indexStruct->indisunique &&
> +IndexIsValid(indexStruct) &&
> +RelationGetIndexExpressions(indexRel) == NIL &&
> +RelationGetIndexPredicate(indexRel) == NIL)
> +hasUniqueIndex = true;
> +
> +index_close(indexRel, RowExclusiveLock);
>
> In the case where hasUniqueIndex = true, ISTM that we can get out of
> the loop immediately just after calling index_close(). No?

Fixed.

> +/* Must have at least one unique index */
> +Assert(foundUniqueIndex);
>
> Can we guarantee that there is at least one valid unique index here?
> If yes, it's better to write the comment about that.
>

Added.

Attached latest patch. Please review it.

Regards,

--
Masahiko Sawada


matview_concurrently_refresh_check_index_v2.patch
Description: binary/octet-stream

-- 
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] Why format() adds double quote?

[Daniel Verite]

Tatsuo Ishii wrote:

> What is the "visual hint"? If you are talking about psql's output, it 
> never adds "visual hint" (double quotations). 
> 
> If you are talking about the string handling in a program, what kind 
> of program cares about "visiual"? 

Sure. The scenario I'm thinking about would be someone inspecting
generated queries, say for controlling or troubleshooting,
the queries containing identifiers injected with the help of
quote_ident/format. That could be from the logs, or
monitoring or audit tools.

If identifiers contain weird Unicode characters, currently 
that's relatively easy to spot because they get surrounded by
double quotes.

If I see something like this: UPDATE "test․table" SET...
I immediately think that there's something fishy. It looks like test
should be a schema, but the surrounding quotes say otherwise.
In any case, it's clear that it updates a table in the current schema.

But if I see that: UPDATE test․table SET...
is seems legit and seems to update "table" inside the "test" schema
even though that's not what it does, it actually updates the
"test․table" table in the current schema, because the dot between
test and table is not the US-ASCII U+002E,  it's U+2024,
'ONE DOT LEADER'
On my display, they are almost indiscernible.

This boils down to the fact that the current quote_ident gives:

=# select quote_ident('test․table');
 quote_ident  
--
 "test․table"

whereas the quote_ident patched as proposed gives:

=# select quote_ident('test․table');
 quote_ident 
-
 test․table


So this is what I don't feel good about.

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

[HACKERS] Implementing a new Scripting Language

[Igal @ Lucee.org]

Hi all,

We have an open source scripting engine named Lucee that is used 
primarily for web application -- https://github.com/lucee/Lucee -- it is 
written in Java and is usually run as a servlet, but can be accessed in 
other ways (like JSR-223).


You can think of the language as a combination of PHP and Javascript, 
albeit much simpler and more intuitive IMHO.


I was wondering how difficult it would be to implement a Postgres 
extension that will act as a wrapper around it and will allow to write 
functions in that language?


I am a Java developer myself and have a very limited knowledge of C/C++, 
but if someone can point me in the right direction of whatever 
documentation there is on the subject -- that would be great.


Thanks,

--

Igal Sapir
Lucee Core Developer
Lucee.org 

Re: [HACKERS] Why format() adds double quote?

[Tom Lane]

"Daniel Verite"  writes:
> This boils down to the fact that the current quote_ident gives:

> =# select quote_ident('test․table');
>  quote_ident  
> --
>  "test․table"

> whereas the quote_ident patched as proposed gives:

> =# select quote_ident('test․table');
>  quote_ident 
> -
>  test․table

> So this is what I don't feel good about.

This patch was originally proposed as a simple, cost-free change,
but it's becoming obvious that it is no such thing.  I think
we should probably reject it and move on.

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] Implementing a new Scripting Language

[Vladimir Sitnikov]

Why do you want that at the database level?
Do you have end-to-end scenario that benefits from using Lucee?

>I was wondering how difficult it would be to implement a Postgres extension 
>that will act as a wrapper around it and will allow to write functions in that 
>language?

Have you checked PL/Java?

Vladimir


-- 
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] Tsvector editing functions

[Stas Kelvich]

Hi

> On 22 Jan 2016, at 19:03, Tomas Vondra  wrote:
> OK, although I do recommend using more sensible variable names, i.e. why how 
> to use 'lexemes' instead of 'lexarr' for example? Similarly for the other 
> functions.


Changed. With old names I tried to follow conventions in surrounding code, but 
probably that is a good idea to switch to more meaningful names in new code.

>> 
>> 
>> delete(tsin tsvector, tsv_filter tsvector) — Delete lexemes and/or positions 
>> of tsv_filter from tsin. When lexeme in tsv_filter has no positions function 
>> will delete any occurrence of same lexeme in tsin. When tsv_filter lexeme 
>> have positions function will delete them from positions of matching lexeme 
>> in tsin. If after such removal resulting positions set is empty then 
>> function will delete that lexeme from resulting tsvector.
>> 
> 
> I can't really imagine situation in which I'd need this, but if you do have a 
> use case for it ... although in the initial paragraph you say "... but if 
> somebody wants to delete for example ..." which suggests you may not have 
> such use case.
> 
> Based on bad experience with extending API based on vague ideas, I recommend 
> only really adding functions with existing need. It's easy to add a function 
> later, much more difficult to remove it or change the signature.

I tried to create more or less self-contained api, e.g. have ability to negate 
effect of concatenation. But i’ve also asked people around what they think 
about extending API and everybody convinced that it is better to stick to 
smaller API. So let’s drop it. At least that functions exists in mail list in 
case if somebody will google for such kind of behaviour.

>> 
>> Also if we want some level of completeness of API and taking into account 
>> that concat() function shift positions on second argument I thought that it 
>> can be useful to also add function that can shift all positions of specific 
>> value. This helps to undo concatenation: delete one of concatenating 
>> tsvectors and then shift positions in resulting tsvector. So I also wrote 
>> one another small function:
>> 
>> shift(tsin tsvector,offset int16) — Shift all positions in tsin by given 
>> offset
> 
> That seems rather too low-level. Shouldn't it be really built into delete() 
> directly somehow?


I think it is ambiguous task on delete. But if we are dropping support of 
delete(tsvector, tsvector) I don’t see points in keeping that functions.

>>> 
>>> 7) Some of the functions use intexterm that does not match the function
>>>   name. I see two such cases - to_tsvector and setweight. Is there a
>>>   reason for that?
>>> 
>> 
>> Because sgml compiler wants unique indexterm. Both functions that
>> youmentioned use overloading of arguments and have non-unique name.
> 
> As Michael pointed out, that should probably be handled by using  
> and  tags.


Done.


> On 19 Jan 2016, at 00:21, Alvaro Herrera  wrote:
> 
> 
> It's a bit funny that you reintroduce the "unrecognized weight: %d"
> (instead of %c) in tsvector_setweight_by_filter.
> 


Ah, I was thinking about moving it to separate diff and messed. Fixed and 
attaching diff with same fix for old tsvector_setweight.




tsvector_ops-v2.1.diff
Description: Binary data


tsvector_ops-v2.2.diff
Description: Binary data



---
Stas Kelvich
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company


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

Re: [HACKERS] pgbench stats per script & other stuff

[Fabien COELHO]

Hello again,


Obviously this would work. I did not think the special case was worth the
extra argument. This one has some oddity too, because the second argument is
ignored depending on the third. Do as you feel.


Actually my question was whether keeping the original start_time was the
intended design.


Sorry I misunderstood the question.

The answer is essentially yes, the field is needed for the "aggregated" 
mode where this specific behavior is used.


However, after some look at the code I think that it is possible to do 
without.


I also spotted an small issue under low tps where the last aggregation was 
not shown.


With the attached version these problems have been removed, no conditional 
initialization. There is also a small diff with the version you sent.


--
Fabien.diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index d5f242c..b3fe994 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -166,10 +166,8 @@ int			agg_interval;		/* log aggregates instead of individual
  * transactions */
 int			progress = 0;		/* thread progress report every this seconds */
 bool		progress_timestamp = false; /* progress report with Unix time */
-int			progress_nclients = 0;		/* number of clients for progress
-		 * report */
-int			progress_nthreads = 0;		/* number of threads for progress
-		 * report */
+int			nclients = 1;		/* number of clients */
+int			nthreads = 1;		/* number of threads */
 bool		is_connect;			/* establish connection for each transaction */
 bool		is_latencies;		/* report per-command latencies */
 int			main_pid;			/* main process id used in log filename */
@@ -193,6 +191,35 @@ typedef struct
 #define SHELL_COMMAND_SIZE	256 /* maximum size allowed for shell command */
 
 /*
+ * Simple data structure to keep stats about something.
+ *
+ * XXX probably the first value should be kept and used as an offset for
+ * better numerical stability...
+ */
+typedef struct
+{
+	int64		count;			/* how many values were encountered */
+	double		min;			/* the minimum seen */
+	double		max;			/* the maximum seen */
+	double		sum;			/* sum of values */
+	double		sum2;			/* sum of squared values */
+} SimpleStats;
+
+/*
+ * Data structure to hold various statistics, used for interval statistics as
+ * well as file statistics.
+ */
+typedef struct
+{
+	long		start_time;		/* interval start time, for aggregates */
+	int64		cnt;			/* number of transactions */
+	int64		skipped;		/* number of transactions skipped under --rate
+ * and --latency-limit */
+	SimpleStats latency;
+	SimpleStats lag;
+} StatsData;
+
+/*
  * Connection state
  */
 typedef struct
@@ -213,10 +240,8 @@ typedef struct
 	bool		prepared[MAX_SCRIPTS];	/* whether client prepared the script */
 
 	/* per client collected stats */
-	int			cnt;			/* xacts count */
+	int64		cnt;			/* transaction count */
 	int			ecnt;			/* error count */
-	int64		txn_latencies;	/* cumulated latencies */
-	int64		txn_sqlats;		/* cumulated square latencies */
 } CState;
 
 /*
@@ -228,19 +253,14 @@ typedef struct
 	pthread_t	thread;			/* thread handle */
 	CState	   *state;			/* array of CState */
 	int			nstate;			/* length of state[] */
-	instr_time	start_time;		/* thread start time */
-	instr_time *exec_elapsed;	/* time spent executing cmds (per Command) */
-	int		   *exec_count;		/* number of cmd executions (per Command) */
 	unsigned short random_state[3];		/* separate randomness for each thread */
 	int64		throttle_trigger;		/* previous/next throttling (us) */
 
 	/* per thread collected stats */
+	instr_time	start_time;		/* thread start time */
 	instr_time	conn_time;
-	int64		throttle_lag;	/* total transaction lag behind throttling */
-	int64		throttle_lag_max;		/* max transaction lag */
-	int64		throttle_latency_skipped;		/* lagging transactions
- * skipped */
-	int64		latency_late;	/* late transactions */
+	StatsData	stats;
+	int64		latency_late;	/* executed but late transactions */
 } TState;
 
 #define INVALID_THREAD		((pthread_t) 0)
@@ -272,33 +292,14 @@ typedef struct
 	char	   *argv[MAX_ARGS]; /* command word list */
 	int			cols[MAX_ARGS]; /* corresponding column starting from 1 */
 	PgBenchExpr *expr;			/* parsed expression */
+	SimpleStats stats;			/* time spent in this command */
 } Command;
 
-typedef struct
-{
-
-	long		start_time;		/* when does the interval start */
-	int			cnt;			/* number of transactions */
-	int			skipped;		/* number of transactions skipped under --rate
- * and --latency-limit */
-
-	double		min_latency;	/* min/max latencies */
-	double		max_latency;
-	double		sum_latency;	/* sum(latency), sum(latency^2) - for
- * estimates */
-	double		sum2_latency;
-
-	double		min_lag;
-	double		max_lag;
-	double		sum_lag;		/* sum(lag) */
-	double		sum2_lag;		/* sum(lag*lag) */
-} AggVals;
-
 static struct
 {
 	const char *name;
-	Command	 **commands;
-} sql_script[MAX_SCRIPTS];		/* SQL script files */
+	Command   **commands;
+}	

Re: [HACKERS] Patch: fix lock contention for HASHHDR.mutex

[Dilip Kumar]

On Wed, Jan 27, 2016 at 5:15 PM, Aleksander Alekseev <
a.aleks...@postgrespro.ru> wrote:

> Most likely HASHHDR.mutex is not only bottleneck in your case so my
> patch doesn't help much. Unfortunately I don't have access to any
> POWER8 server so I can't investigate this issue. I suggest to use a
> gettimeofday trick I described in a first message of this thread. Its
> time consuming but it gives a clear understanding which code is keeping
> a lock.
>

I have also tested the pgbench Readonly test when data don't fit into
shared buffer,
Because in this case HASHHDR.mutex access will be quite frequent.
And in this case i do see very good improvement in POWER8 server.

Test Result:

Scale Factor:300
Shared Buffer:512MB
pgbench -c$ -j$ -S -M Prepared postgres

Clientbasepatch
64   222173318318
128195805262290

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

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

[Pavel Stehule]

Hi

>
> > I though about it lot of, and I see a  the core of problems in orthogonal
> > constructed exceptions in Python and Postgres. We working with statements
> > elog, ereport, RAISE EXCEPTION - and these statements are much more like
> > templates - can generate any possible exception. Python is based on
> working
> > with instances of predefined exceptions. And it is core of problem.
> > Template like class cannot be ancestor of instance oriented classes. This
> > is task for somebody who knows well OOP and meta OOP in Python - total
>
> I've been following this discussion with great interest, because PL/Java
> also is rather overdue for tackling the same issues: it has some partial
> ability to catch things from PostgreSQL and even examine them in proper
> detail, but very limited ability to throw things as information-rich as
> is possible from C with ereport. And that work is not as far along as
> you are with PL/Python, there is just a preliminary design/discussion
> wiki document at
>   https://github.com/tada/pljava/wiki/Thoughts-on-logging
>
> I was unaware of this project in PL/Pythonu when I began it, then added
> the reference when I saw this discussion.
>
>
I read your article and it is exactly same situation.

It is conflict between procedural (PostgreSQL) and OOP (Python, Java) API.
I see possible solution in design independent class hierarchies - static
(buildin exceptions) and dynamic (custom exceptions). It cannot be mixed,
but there can be some abstract ancestor. Second solution is defensive -
using procedural API for custom exceptions - what i doing in PLPythonu.

Regards

Pavel

Re: [HACKERS] Implementing a new Scripting Language

[Igal @ Lucee.org]

On 1/27/2016 8:40 AM, Vladimir Sitnikov wrote:

Why do you want that at the database level?
Do you have end-to-end scenario that benefits from using Lucee?
Lucee is very intuitive and powerful, so it's more for ease of use than 
anything, and to attract more Lucee users to use PostgreSQL (most of 
them use SQL Server or MySQL).


If the pl/v8 was easily ported to Windows then I probably wouldn't even 
try to add Lucee, but it seems to be quite difficult to compile the 
latest versions for Windows and it looks like the project is not 
maintained (for example, it uses an older version of V8 with no known 
intention to upgrade).


If I had more experience with C++ then I'd probably try to update V8, 
but so far my attempts have not been very fruitful.



I was wondering how difficult it would be to implement a Postgres extension 
that will act as a wrapper around it and will allow to write functions in that 
language?

Have you checked PL/Java?
That seems like a good place to start, thanks.  Are there also any docs 
about the subject?



Igal


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

[Etsuro Fujita]

On 2016/01/28 15:20, Rushabh Lathia wrote:

On Thu, Jan 28, 2016 at 11:33 AM, Etsuro Fujita
> wrote:

On 2016/01/27 21:23, Rushabh Lathia wrote:

If I understood correctly, above documentation means, that if
FDW have
DMLPushdown APIs that is enough. But in reality thats not the
case, we
need  ExecForeignInsert, ExecForeignUpdate, or ExecForeignDelete
in case
DML is not pushable.

And here fact is DMLPushdown APIs are optional for FDW, so that
if FDW
don't have DMLPushdown APIs they can still very well perform the DML
operations using ExecForeignInsert, ExecForeignUpdate, or
ExecForeignDelete.



I agree with you.  I guess I was wrong. sorry.

So documentation should be like:

If the IsForeignRelUpdatable pointer is set to NULL, foreign
tables are
assumed to be insertable, updatable, or deletable if the FDW
provides
ExecForeignInsert, ExecForeignUpdate, or ExecForeignDelete
respectively,

If FDW provides DMLPushdown APIs and the DML are pushable to the
foreign
server, then FDW still needs ExecForeignInsert,
ExecForeignUpdate, or
ExecForeignDelete for the non-pushable DML operation.

What's your opinion ?



I agree that we should add this to the documentation, too.

BTW, if I understand correctly, I think we should also modify
relation_is_updatabale() accordingly.  Am I right?



Yep, we need to modify relation_is_updatable().


OK, will do.

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] Minor code improvements to create_foreignscan_plan/ExecInitForeignScan

[Etsuro Fujita]

On 2016/01/28 12:13, Robert Haas wrote:

On Thu, Jan 21, 2016 at 5:55 AM, Etsuro Fujita
 wrote:

On 2016/01/21 7:04, Alvaro Herrera wrote:

Etsuro Fujita wrote:


On second thought, I noticed that detecting whether we see a system
column
that way needs more cycles in cases where the reltargetlist and the
restriction clauses don't contain any system columns.  ISTM that such
cases
are rather common, so I'm inclined to keep that code as-is.



Ah, I see now what you did there. I was thinking you'd have the
foreach(restrictinfo) loop, then once the loop is complete scan the
bitmapset; not scan the bitmap set scan inside the other loop.



Ah, I got to the point.  I think that is a good idea.  The additional cycles
for the worst case are bounded and negligible.  Please find attached an
updated patch.



I don't think this is a good idea.  Most of the time, no system
columns will be present, and we'll just be scanning the Bitmapset
twice rather than once.  Sure, that doesn't take many extra cycles,
but if the point of all this is to micro-optimize this code, that
particular change is going in the wrong direction.


I thought that is a good idea, considering the additional overhead is 
little, because that would be useful for some use-cases.  But I think we 
need more discussions about that, so if there are no objections from 
Alvaro (or anyone), I'll leave that part as-is.


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] Re: BUG #13685: Archiving while idle every archive_timeout with wal_level hot_standby

[Michael Paquier]

On Tue, Jan 19, 2016 at 5:10 PM, Amit Kapila  wrote:
> On Tue, Jan 19, 2016 at 12:41 PM, Michael Paquier
>  wrote:
>>
>> On Mon, Jan 18, 2016 at 10:19 PM, Amit Kapila 
>> wrote:
>> > On Mon, Jan 18, 2016 at 10:54 AM, Michael Paquier
>> >  wrote:
>> >> Yes, the thing is that, as mentioned at the beginning of the thread, a
>> >> low value of archive_timeout causes a segment to be forcibly switched
>> >> at the end of the timeout defined by this parameter. In combination
>> >> with the standby snapshot taken in bgwriter since 9.4, this is causing
>> >> segments to be always switched even if a system is completely idle.
>> >> Before that, in 9.3 and older versions, we would have a segment
>> >> forcibly switched on an idle system only once per checkpoint.
>> >>
>> >
>> > Okay, so this will fix the case where the system is idle, but what I
>> > am slightly worried is that it should not miss to log the standby
>> > snapshot
>> > due to this check when there is actually some activity in the system.
>> > Why is not possible to have a case such that the segment is forcibly
>> > switched due to reason other than checkpoint (user has requested the
>> > same) and the current insert LSN is at beginning of a new segment
>> > due to write activity? If that is possible, which to me theoretically
>> > seems
>> > possible, then I think bgwriter will miss to log the standby snapshot.
>>
>> Yes, with segments forcibly switched by users this check would make
>> the bgwriter not log in a snapshot if the last action done by server
>> was XLOG_SWITCH. Based on the patch I sent, the only alternative would
>> be to not forcedSegSwitchLSN in those code paths. Perhaps that's fine
>> enough for back-branches..
>>
>
> Yeah, that can work, but I think the code won't look too clean.  I think
> lets try out something on lines of what is suggested by Andres and
> see how it turns out.

OK, so as a first step and after thinking about the whole for a while,
I have finished with the patch attached. This patch is aimed at
avoiding unnecessary checkpoints on idle systems when wal_level >=
hot_standby by centralizing the check to look at if there has some WAL
activity since the last checkpoint. This way, the system does not
generate anymore standby-related records if no activity has occurred.
I think that this is a good step forward, still it does not address
yet the problem of segments forcibly switched, particularly when
archive_timeout has a low value.

In order to address the other problem with switched segments, I think
that we could extend the routine XLOGHasActivity that the patch
attached introduces with some more fine-grained checks. After looking
at the code I think as well that we had better save into shared memory
the last LSN position that was forcibly switched and make use of that
in the bgwriter.

So, thoughts about the attached?
-- 
Michael
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index a2846c4..51fd9e9 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -7869,6 +7869,27 @@ GetFlushRecPtr(void)
 }
 
 /*
+ * XLOGHasActivity -- Check if XLOG had some significant activity or
+ * if it is idle lately. This is primarily used to check if there has
+ * been some WAL activity since the last checkpoint that occurred on
+ * system to control the generaton of XLOG record related to standbys.
+ */
+bool
+XLOGHasActivity(void)
+{
+	XLogCtlInsert *Insert = >Insert;
+	XLogRecPtr	redo_lsn = ControlFile->checkPointCopy.redo;
+	uint64		prev_bytepos;
+
+	/* Check if any activity has happened since last checkpoint */
+	SpinLockAcquire(>insertpos_lck);
+	prev_bytepos = Insert->PrevBytePos;
+	SpinLockRelease(>insertpos_lck);
+
+	return XLogBytePosToRecPtr(prev_bytepos) == redo_lsn;
+}
+
+/*
  * Get the time of the last xlog segment switch
  */
 pg_time_t
@@ -8239,7 +8260,7 @@ CreateCheckPoint(int flags)
 	if ((flags & (CHECKPOINT_IS_SHUTDOWN | CHECKPOINT_END_OF_RECOVERY |
   CHECKPOINT_FORCE)) == 0)
 	{
-		if (prevPtr == ControlFile->checkPointCopy.redo &&
+		if (XLOGHasActivity() &&
 			prevPtr / XLOG_SEG_SIZE == curInsert / XLOG_SEG_SIZE)
 		{
 			WALInsertLockRelease();
@@ -8404,10 +8425,10 @@ CreateCheckPoint(int flags)
 	 * allows us to reconstruct the state of running transactions during
 	 * archive recovery, if required. Skip, if this info disabled.
 	 *
-	 * If we are shutting down, or Startup process is completing crash
-	 * recovery we don't need to write running xact data.
+	 * If we are shutting down, are on a non-idle system or Startup process
+	 * is completing crash recovery we don't need to write running xact data.
 	 */
-	if (!shutdown && XLogStandbyInfoActive())
+	if (!shutdown && XLOGHasActivity() && XLogStandbyInfoActive())
 		LogStandbySnapshot();
 
 	START_CRIT_SECTION();
diff --git a/src/backend/postmaster/bgwriter.c 

Re: [HACKERS] Log operating system user connecting via unix socket

[José Arthur Benetasso Villanova]

Hi again.

About the privileges, our support can create roles / databases, drop
existing databases, dump /restore, change other users passwords. It's not
feasible right now create a 1:1 map of system users and postgres users.
Maybe in the future.

I wrote 2 possible patches, both issuing a detail message only if
log_connections is enabled.

The first one using the Stephen Frost suggestion, inside the Port struct (I
guess that this is the one, I coudn't find the Peer struct)

The second one following the same approach of cf commit 5e0b5dcab, as
pointed by Tom Lane.

Again, feel free to comment and criticize.

On Sun, Jan 17, 2016 at 3:07 PM, Stephen Frost  wrote:

> Tom,
>
> * Tom Lane (t...@sss.pgh.pa.us) wrote:
> > Stephen Frost  writes:
> > > What I think we really want here is logging of the general 'system
> > > user' for all auth methods instead of only for the 'peer' method.
> >
> > Well, we don't really know that except in a small subset of auth
> > methods.  I agree that when we do know it, it's useful info to log.
>
> Right.
>
> > My big beef with the proposed patch is that the log message is emitted
> > unconditionally.  There are lots and lots of users who feel that during
> > normal operation, *zero* log messages should get emitted.  Those
> villagers
> > would be on our doorsteps with pitchforks if we shipped this patch as-is.
>
> Agreed.
>
> > I would propose that this information should be emitted only when
> > log_connections is enabled, and indeed that it should be part of the
> > log_connections message not a separate message.  So this leads to
> > thinking that somehow, the code for individual auth methods should
> > be able to return an "additional info" field for inclusion in
> > log_connections.  We already have such a concept for auth failures,
> > cf commit 5e0b5dcab.
>
> Apologies if it wasn't clear, but that's exactly what I was suggesting
> by saying to add it to PerformAuthentication, which is where we emit
> the connection info when log_connections is enabled.
>
> > > ... and also make it available in pg_stat_activity.
> >
> > That's moving the goalposts quite a bit, and I'm not sure it's necessary
> > or even desirable.  Let's just get this added to log_connections output,
> > and then see if there's field demand for more.
>
> This was in context of peer_cn, which is just a specific "system user"
> value and which we're already showing in pg_stat_* info tables.  I'd
> love to have the Kerberos principal available, but I don't think it'd
> make sense to have a 'pg_stat_kerberos' just for that.
>
> I agree that it's moving the goalposts for this patch and could be an
> independent patch, but I don't see it as any different, from a
> desirability and requirements perspective, than what we're doing for SSL
> connections.
>
> Thanks!
>
> Stephen
>



-- 
José Arthur Benetasso Villanova
commit 76594784c50bca1b09f687e58f17ff27230076be
Author: Jose Arthur Benetasso Villanova 
Date:   Tue Jan 19 11:50:22 2016 -0200

Log message

diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 57c2f48..ac1c785 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -991,6 +991,7 @@ pg_GSS_recvauth(Port *port)
 		return STATUS_ERROR;
 	}
 
+	port->system_user = pstrdup(gbuf.value);
 	ret = check_usermap(port->hba->usermap, port->user_name, gbuf.value,
 		pg_krb_caseins_users);
 
@@ -1291,6 +1292,7 @@ pg_SSPI_recvauth(Port *port)
 		int			retval;
 
 		namebuf = psprintf("%s@%s", accountname, domainname);
+		port->system_user = pstrdup(namebuf);
 		retval = check_usermap(port->hba->usermap, port->user_name, namebuf, true);
 		pfree(namebuf);
 		return retval;
@@ -1561,8 +1563,11 @@ ident_inet_done:
 		pg_freeaddrinfo_all(local_addr.addr.ss_family, la);
 
 	if (ident_return)
+	{
 		/* Success! Check the usermap */
+		port->system_user = pstrdup(ident_user);
 		return check_usermap(port->hba->usermap, port->user_name, ident_user, false);
+	}
 	return STATUS_ERROR;
 }
 
@@ -1609,6 +1614,8 @@ auth_peer(hbaPort *port)
 	}
 
 	strlcpy(ident_user, pw->pw_name, IDENT_USERNAME_MAX + 1);
+	port->system_user = pstrdup(ident_user);
+
 
 	return check_usermap(port->hba->usermap, port->user_name, ident_user, false);
 }
@@ -2124,6 +2131,7 @@ CheckLDAPAuth(Port *port)
 		return STATUS_ERROR;
 	}
 
+	port->system_user = pstrdup(fulluser);
 	pfree(fulluser);
 
 	return STATUS_OK;
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index e22d4db..f425808 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -255,7 +255,8 @@ PerformAuthentication(Port *port)
 #endif
 ereport(LOG,
 		(errmsg("replication connection authorized: user=%s",
-port->user_name)));
+port->user_name),
+		port->system_user ? errdetail_log("system_user=%s", port->system_user) : 0));
 		}
 		else
 		{
@@ -269,7 +270,8 @@ PerformAuthentication(Port 

Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

[Pavel Stehule]

Hi

2016-01-18 21:37 GMT+01:00 Alvaro Herrera :

> > diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
> > new file mode 100644
> > index 1ae4bb7..c819517
> > *** a/src/pl/plpgsql/src/pl_comp.c
> > --- b/src/pl/plpgsql/src/pl_comp.c
> > *** plpgsql_parse_tripword(char *word1, char
> > *** 1617,1622 
> > --- 1617,1677 
> >   return false;
> >   }
> >
> > + /*
> > +  * Derive type from ny base type controlled by reftype_mode
> > +  */
> > + static PLpgSQL_type *
> > + derive_type(PLpgSQL_type *base_type, int reftype_mode)
> > + {
> > + Oid typoid;
>
> I think you should add a typedef to the REFTYPE enum, and have this
> function take that type rather than int.
>

done


>
> > + case PLPGSQL_REFTYPE_ARRAY:
> > + {
> > + /*
> > +  * Question: can we allow anyelement (array or
> nonarray) -> array direction.
> > +  * if yes, then probably we have to modify
> enforce_generic_type_consistency,
> > +  * parse_coerce.c where still is check on scalar
> type -> raise error
> > +  * ERROR:  42704: could not find array type for
> data type integer[]
> > +  *
> > + if
> (OidIsValid(get_element_type(base_type->typoid)))
> > + return base_type;
> > + */
>
> I think it would be better to resolve this question outside a code
> comment.
>

done


>
> > + typoid = get_array_type(base_type->typoid);
> > + if (!OidIsValid(typoid))
> > + ereport(ERROR,
> > +
>  (errcode(ERRCODE_DATATYPE_MISMATCH),
> > +  errmsg("there are not
> array type for type %s",
> > +
>  format_type_be(base_type->typoid;
>
> nodeFuncs.c uses this wording:
> errmsg("could not find array type for data type %s",
> which I think you should adopt.
>

sure, fixed


>
> > --- 1681,1687 
> >* --
> >*/
> >   PLpgSQL_type *
> > ! plpgsql_parse_wordtype(char *ident, int reftype_mode)
> >   {
> >   PLpgSQL_type *dtype;
> >   PLpgSQL_nsitem *nse;
>
> Use the typedef'ed enum, as above.
>
> > --- 1699,1721 
> >   switch (nse->itemtype)
> >   {
> >   case PLPGSQL_NSTYPE_VAR:
> > ! {
> > ! dtype = ((PLpgSQL_var *)
> (plpgsql_Datums[nse->itemno]))->datatype;
> > ! return derive_type(dtype, reftype_mode);
> > ! }
> >
> > ! case PLPGSQL_NSTYPE_ROW:
> > ! {
> > ! dtype = ((PLpgSQL_row *)
> (plpgsql_Datums[nse->itemno]))->datatype;
> > ! return derive_type(dtype, reftype_mode);
> > ! }
> >
> > + /*
> > +  * XXX perhaps allow REC here? Probably it has not
> any sense, because
> > +  * in this moment, because PLpgSQL doesn't support
> rec parameters, so
> > +  * there should not be any rec polymorphic
> parameter, and any work can
> > +  * be done inside function.
> > +  */
>
> I think you should remove from the "?" onwards in that comment, i.e.
> just keep what was already in the original comment (minus the ROW)
>

I tried to fix it, not sure if understood well.


>
> > --- 1757,1763 
> >* --
> >*/
> >   PLpgSQL_type *
> > ! plpgsql_parse_cwordtype(List *idents, int reftype_mode)
> >   {
> >   PLpgSQL_type *dtype = NULL;
> >   PLpgSQL_nsitem *nse;
>
> Typedef.
>
> > --- 2720,2737 
> >   tok = yylex();
> >   if (tok_is_keyword(tok, ,
> >  K_TYPE, "type"))
> > ! result = plpgsql_parse_wordtype(dtname,
> PLPGSQL_REFTYPE_TYPE);
> > ! else if (tok_is_keyword(tok, ,
> > !
>  K_ELEMENTTYPE, "elementtype"))
> > ! result = plpgsql_parse_wordtype(dtname,
> PLPGSQL_REFTYPE_ELEMENT);
> > ! else if (tok_is_keyword(tok, ,
> > !
>  K_ARRAYTYPE, "arraytype"))
> > ! result = plpgsql_parse_wordtype(dtname,
> PLPGSQL_REFTYPE_ARRAY);
> >   else if (tok_is_keyword(tok, ,
> >
>  K_ROWTYPE, "rowtype"))
> >   result = plpgsql_parse_wordrowtype(dtname);
> > ! if (result)
> > ! return result;
> >   }
>
> This plpgsql parser stuff is pretty tiresome.  (Not this patch's fault
> -- just saying.)
>
>
> > *** extern bool plpgsql_parse_dblword(char *
> > *** 961,968 
> > PLwdatum *wdatum, 

Re: [HACKERS] Implementing a new Scripting Language

[Igal @ Lucee.org]

On 1/27/2016 9:57 AM, Vladimir Sitnikov wrote:

That is a good question. ChakraCore has been open sourced recently. It
might be easier to build under Windows.
interesting.  but now we will need to write an extension for that, e.g. 
PL/Chakra, which brings back my original question:

are there any docs as to how to implement a new scripting language? ;)

I am not sure you would be able to bind high performance java runtime
with the backend. There are no that many JREs, and not much of them
are good at "in-backend" operation.

Thus your question boils down to 2 possibilities:
1) You execute Lucee in some JRE that runs in the backend (frankly
speaking, I doubt it is a good way to go)
yes, that's what I had in mind.  I wasn't thinking of an embedded JRE.  
TBH I didn't think there were any of those until your email.


thanks,


Igal


--
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] Implementing a new Scripting Language

[Chapman Flack]

On 01/27/2016 11:46 AM, Igal @ Lucee.org wrote:

>> Have you checked PL/Java?
> That seems like a good place to start, thanks.  Are there also any docs
> about the subject?

I just did a quick search on Lucee and what I found suggests that
it compiles to JVM bytecode and runs on a JVM. If that is the
case, and it can compile methods that will have the sort of
method signatures PL/Java expects, and you can put the .class
files in a jar and load it, your job should be just about done. :)

Or, you might end up writing thin wrappers in Java, probably
nothing more.

Another possibility: Java has pluggable script engine support
(java specification request 233, see the javax.script package).
Does Lucee have an existing JSR 233 engine implementation?

PL/Java does not _currently_ have JSR233 support, but it is
definitely being thought about ... the idea being, put a Lucee
JSR233 engine jar on the classpath, define it as a new PostgreSQL
language (PL/Java will handle the interfacing), and then actually
write stuff like

DO $$echo("Hello World");$$ LANGUAGE lucee;

As I said, in current PL/Java version, JSR 233 is still
science fiction ... but it's "hard science fiction", not the
fantasy stuff.

http://tada.github.io/pljava/

-Chap


-- 
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] Implementing a new Scripting Language

[Vladimir Sitnikov]

> If the pl/v8 was easily ported to Windows then I probably wouldn't even try 
> to add Lucee,

That is a good question. ChakraCore has been open sourced recently. It
might be easier to build under Windows.

>That seems like a good place to start, thanks
I am not sure you would be able to bind high performance java runtime
with the backend. There are no that many JREs, and not much of them
are good at "in-backend" operation.

Thus your question boils down to 2 possibilities:
1) You execute Lucee in some JRE that runs in the backend (frankly
speaking, I doubt it is a good way to go)
2) You implement Lucee parser/executor/compiler in C and use it as
typical PostgreSQL extension

Vladimir


-- 
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] Implementing a new Scripting Language

[Igal @ Lucee.org]

On 1/27/2016 9:58 AM, jflack wrote:

I just did a quick search on Lucee and what I found suggests that
it compiles to JVM bytecode and runs on a JVM. If that is the
case, and it can compile methods that will have the sort of
method signatures PL/Java expects, and you can put the .class
files in a jar and load it, your job should be just about done. :)

yes, Lucee uses ASM4 to construct class files which are mostly POJOs

Or, you might end up writing thin wrappers in Java, probably
nothing more.

Another possibility: Java has pluggable script engine support
(java specification request 233, see the javax.script package).
Does Lucee have an existing JSR 233 engine implementation?
the next version of Lucee (currently in Beta) does support JSR-223, 
which I actually mentioned as a viable solution in my first email in 
this thread.  That would be awesome if PL/Java would support JSR-223.


thanks,


Igal


--
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] Log operating system user connecting via unix socket

[Stephen Frost]

José,

* José Arthur Benetasso Villanova (jose.art...@gmail.com) wrote:
> I wrote 2 possible patches, both issuing a detail message only if
> log_connections is enabled.
> 
> The first one using the Stephen Frost suggestion, inside the Port struct (I
> guess that this is the one, I coudn't find the Peer struct)

Ah, yes, apologies for the typo.

> The second one following the same approach of cf commit 5e0b5dcab, as
> pointed by Tom Lane.

This really isn't quite following along in the approach used by
5e0b5dcab, from my viewing of it.  I believe Tom was suggesting that an
essentially opaque value be returned to be included rather than what
you've done which codifies it as 'system_user'.

I'm not a fan of that approach though, as the mapping system we have in
pg_ident is generalized and this should be implemented generally by all
authentication methods which support mappings.

That's also why I was suggesting to get rid of peer_cn in the Port
structure in favor of having the 'system_user' or similar variable and
using it in all of these cases where we provide mapping support- then
all of the auth methods which support mappings would set that value,
including the existing SSL code.  You might need to check and see if
there's anything which depends on peer_cn being NULL for non-SSL
connections and adjust that logic, but hopefully that's not what we're
relying on.  I don't see anything like that on a quick glance through
the peer_cn usage.

Also, it looks like you missed one of the exit cases from
pg_SSPI_recvauth(), no?  You may need to refactor that code a bit to
provide easy access to what the system username used is, or simply make
sure to set the port->system_user value in both paths.

Lastly, are you sure that you have the right memory context for the
pstrdup() calls that you're making?  be-secure-openssl.c goes to some
effort to ensure that the memory allocated for peer_cn is in the
TopMemoryContext, but I don't see anything like that in the code
proposed, which makes me worried you didn't consider which memory
context you were allocating in.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Implementing a new Scripting Language

[Pavel Stehule]

2016-01-27 19:21 GMT+01:00 Igal @ Lucee.org :

> On 1/27/2016 9:57 AM, Vladimir Sitnikov wrote:
>
>> That is a good question. ChakraCore has been open sourced recently. It
>> might be easier to build under Windows.
>>
> interesting.  but now we will need to write an extension for that, e.g.
> PL/Chakra, which brings back my original question:
> are there any docs as to how to implement a new scripting language? ;)
>

David Fetter wrote some presentation - some years ago was popular to write
own PL

me too

 https://wiki.postgresql.org/images/a/a2/Plpgsql_internals.pdf

source code of plpgsql is good example - it pretty simple

you have to write handler

https://github.com/petere/plsh



I am not sure you would be able to bind high performance java runtime
>> with the backend. There are no that many JREs, and not much of them
>> are good at "in-backend" operation.
>>
>> Thus your question boils down to 2 possibilities:
>> 1) You execute Lucee in some JRE that runs in the backend (frankly
>> speaking, I doubt it is a good way to go)
>>
> yes, that's what I had in mind.  I wasn't thinking of an embedded JRE.
> TBH I didn't think there were any of those until your email.
>
> thanks,
>
>
> Igal
>
>
>
> --
> 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] Implementing a new Scripting Language

[Chapman Flack]

On 01/27/2016 01:17 PM, Igal @ Lucee.org wrote:

> the next version of Lucee (currently in Beta) does support JSR-223,
> which I actually mentioned as a viable solution in my first email in

Sorry, I jumped in late.

> this thread.  That would be awesome if PL/Java would support JSR-223.

Ok, if your 233 support is already in beta, you'll get there
before we do, but the paths should intersect eventually. :)

-Chap


-- 
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] Implementing a new Scripting Language

[Igal @ Lucee.org]

On 1/27/2016 10:48 AM, Chapman Flack wrote:


Ok, if your 233 support is already in beta, you'll get there
before we do, but the paths should intersect eventually. :)


Actually, once your support for JSR-223 is implemented (it's 
two-twenty-three, not thirty ;)), we will be able to use Javascript 
through that via the Nashorn project, which seems like a better (at 
least as a cross-platform) solution than V8 or Chakra.

http://www.oracle.com/technetwork/articles/java/jf14-nashorn-2126515.html

Now that would be very exciting IMO!


Igal




--
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] Implementing a new Scripting Language

[Pavel Stehule]

2016-01-27 19:37 GMT+01:00 Pavel Stehule :

>
>
> 2016-01-27 19:21 GMT+01:00 Igal @ Lucee.org :
>
>> On 1/27/2016 9:57 AM, Vladimir Sitnikov wrote:
>>
>>> That is a good question. ChakraCore has been open sourced recently. It
>>> might be easier to build under Windows.
>>>
>> interesting.  but now we will need to write an extension for that, e.g.
>> PL/Chakra, which brings back my original question:
>> are there any docs as to how to implement a new scripting language? ;)
>>
>
> David Fetter wrote some presentation - some years ago was popular to write
> own PL
>
> me too
>
>  https://wiki.postgresql.org/images/a/a2/Plpgsql_internals.pdf
>
> source code of plpgsql is good example - it pretty simple
>
> you have to write handler
>
> https://github.com/petere/plsh
>

same author

 https://github.com/petere/plhaskell
https://github.com/petere/plxslt

>
>
>
>
> I am not sure you would be able to bind high performance java runtime
>>> with the backend. There are no that many JREs, and not much of them
>>> are good at "in-backend" operation.
>>>
>>> Thus your question boils down to 2 possibilities:
>>> 1) You execute Lucee in some JRE that runs in the backend (frankly
>>> speaking, I doubt it is a good way to go)
>>>
>> yes, that's what I had in mind.  I wasn't thinking of an embedded JRE.
>> TBH I didn't think there were any of those until your email.
>>
>> thanks,
>>
>>
>> Igal
>>
>>
>>
>> --
>> 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] Implementing a new Scripting Language

[Igal @ Lucee.org]

On 1/27/2016 10:41 AM, Pavel Stehule wrote:


2016-01-27 19:37 GMT+01:00 Pavel Stehule >:



David Fetter wrote some presentation - some years ago was popular
to write own PL

me too

https://wiki.postgresql.org/images/a/a2/Plpgsql_internals.pdf

source code of plpgsql is good example - it pretty simple

you have to write handler

https://github.com/petere/plsh


same author

https://github.com/petere/plhaskell
https://github.com/petere/plxslt


Thanks!  I'll take a look.


Igal

[HACKERS] pgbench small bug fix

[Fabien COELHO]

While testing for something else I encountered two small bugs under very 
low rate (--rate=0.1). The attached patches fixes these.


 - when a duration (-T) is specified, ensure that pgbench ends at that
   time (i.e. do not wait for a transaction beyond the end of the run).

 - when there is a progress (-P) report, ensure that all progress
   reports are shown even if no more transactions are schedule.

--
Fabien.diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index d5f242c..6cd6500 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -1195,6 +1195,12 @@ top:
 		thread->throttle_trigger += wait;
 		st->txn_scheduled = thread->throttle_trigger;
 
+		/* stop client if next transaction is beyond pgbench end of execution */
+		if (duration &&
+			(st->txn_scheduled / 100.0) >
+			INSTR_TIME_GET_DOUBLE(thread->start_time) + duration)
+			return false;
+
 		/*
 		 * If this --latency-limit is used, and this slot is already late so
 		 * that the transaction will miss the latency limit even if it
@@ -3674,7 +3680,10 @@ threadRun(void *arg)
 		}
 	}
 
-	while (remains > 0)
+	while (remains > 0 ||
+		   /* thread zero keeps on printing progress report if any */
+		   (progress && thread->tid == 0 && duration &&
+			next_report <= thread_start + (int64) duration * 100))
 	{
 		fd_set		input_mask;
 		int			maxsock;	/* max socket number to be waited */

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

Re: [HACKERS] [Proposal] Table partition + join pushdown

[Robert Haas]

On Mon, Jan 25, 2016 at 9:09 PM, Kouhei Kaigai  wrote:
> Of course, its implementation is not graceful enough, especially, above
> point because this extra filter will change expected number of rows to
> be produced by inner relation, and relevant cost.
> Right now, his patch calls cost_seqscan() and others according to the
> type of inner relation by itself. Of course, it is not a portable way,
> if inner relation would not be a simple relations scan.
>
> Due to path construction staging, AppendPath with underlying join paths
> has to be constructed on join path investigation steps. So, what is the
> reasonable way to make inner relation's path node with filters pushed-
> down?
> It is the most ugly part of the current patch.

I think that it needs to be done only in contexts where we can
guarantee that the optimization is correct, like declarative hash
partitioning:

http://www.postgresql.org/message-id/ca+tgmob2wfjivfocdluunovjsftp6qsqxippxvnnogly+3r...@mail.gmail.com

As I said upthread, in general your proposal will not work and will
lead to wrong answers to queries.

-- 
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] Implementing a new Scripting Language

[Chapman Flack]

On 01/27/2016 02:06 PM, Igal @ Lucee.org wrote:

> two-twenty-three, not thirty ;)),

Thanks. :)  On occasions in the past I have written it
correctly ... there is evidence in the archives 

> we will be able to use Javascript
> through that via the Nashorn project,

Yes, that's an attraction for me, as the JSR-two-twenty-three
engine for JS is now typically already included with the JRE.
(Technically, I keep reading that there is no two-twenty-three
engine that *needs* to be included and it's totally up to the
JRE packager ... but there have actually been some JavaScript
snippets in PL/Java's build process since July 2014 and no one
has yet reported they broke the build.)

-Chap


-- 
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] WAL Re-Writes

[james]

On 27/01/2016 13:30, Amit Kapila wrote:


Thoughts?


Are the decreases observed with SSD as well as spinning rust?

I might imagine that decreasing the wear would be advantageous, 
especially if the performance decrease is less with low read latency.





--
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] Batch update of indexes

[Robert Haas]

On Wed, Jan 20, 2016 at 4:28 AM, Konstantin Knizhnik
 wrote:
> Please notice that such alter table statement, changing condition for
> partial index, is not supported now.
> But I do not see any principle problems with supporting such construction.
> We should just include in the index all records which match new condition
> and do not match old condition:
>
>ts < '21/01/2016' and not (ts < '20/01/2016')

You'd also need to remove any rows from the index that match the old
condition but not the new one.  In your example, that's impossible,
but in general, it's definitely possible.

-- 
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] CustomScan under the Gather node?

[Robert Haas]

On Tue, Jan 26, 2016 at 1:30 AM, Kouhei Kaigai  wrote:
> What enhancement will be necessary to implement similar feature of
> partial seq-scan using custom-scan interface?
>
> It seems to me callbacks on the three points below are needed.
> * ExecParallelEstimate
> * ExecParallelInitializeDSM
> * ExecParallelInitializeWorker
>
> Anything else?
> Does ForeignScan also need equivalent enhancement?

For postgres_fdw, running the query from a parallel worker would
change the transaction semantics.  Suppose you begin a transaction,
UPDATE data on the foreign server, and then run a parallel query.  If
the leader performs the ForeignScan it will see the uncommitted
UPDATE, but a worker would have to make its own connection which not
be part of the same transaction and which would therefore not see the
update.  That's a problem.

Also, for postgres_fdw, and many other FDWs I suspect, the assumption
is that most of the work is being done on the remote side, so doing
the work in a parallel worker doesn't seem super interesting.  Instead
of incurring transfer costs to move the data from remote to local, we
incur two sets of transfer costs: first remote to local, then worker
to leader.  Ouch.  I think a more promising line of inquiry is to try
to provide asynchronous execution when we have something like:

Append
-> Foreign Scan
-> Foreign Scan

...so that we can return a row from whichever Foreign Scan receives
data back from the remote server first.

So it's not impossible that an FDW author could want this, but mostly
probably not.  I think.

-- 
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] extend pgbench expressions with functions

[Robert Haas]

On Sat, Jan 16, 2016 at 1:10 PM, Fabien COELHO  wrote:
> All these results are fine from my point of view.
>
>> And this one generates a core dump:
>> \set cid debug(-9223372036854775808 / -1)
>> Floating point exception: 8 (core dumped)

That does not seem acceptable to me.  I don't want pgbench to die a
horrible death if a floating point exception occurs any more than I
want the server to do the same.  I want the error to be properly
caught and reported.

-- 
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] RFC: replace pg_stat_activity.waiting with something more descriptive

[Robert Haas]

On Tue, Jan 26, 2016 at 3:10 AM, and...@anarazel.de  wrote:
> I do think there's a considerable benefit in improving the
> instrumentation here, but his strikes me as making live more complex for
> more users than it makes it easier. At the very least this should be
> split into two fields (type & what we're actually waiting on). I also
> strongly suspect we shouldn't use in band signaling ("process not
> waiting"), but rather make the field NULL if we're not waiting on
> anything.

+1 for splitting it into two fields.

Regarding making the field NULL, someone (I think you) proposed
previously that we should have one field indicating whether we are
waiting, and a separate field (or two) indicating the current or most
recent wait event.  That would be similar to how
pg_stat_activity.{query,state} 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] GIN pending list clean up exposure to SQL

[Julien Rouhaud]

On 27/01/2016 10:27, Fujii Masao wrote:
> On Mon, Jan 25, 2016 at 3:54 PM, Jeff Janes 
> wrote:
>> On Wed, Jan 20, 2016 at 6:17 AM, Fujii Masao
>>  wrote:
>>> On Sat, Jan 16, 2016 at 7:42 AM, Julien Rouhaud 
>>>  wrote:
 On 15/01/2016 22:59, Jeff Janes wrote:
> On Sun, Jan 10, 2016 at 4:24 AM, Julien Rouhaud 
>  wrote:
 
 All looks fine to me, I flag it as ready for committer.
>>> 
>>> When I compiled the PostgreSQL with the patch, I got the
>>> following error. ISTM that the inclusion of pg_am.h header file
>>> is missing in ginfast.c.
>> 
>> Thanks.  Fixed.
>> 
>>> gin_clean_pending_list() must check whether the server is in
>>> recovery or not. If it's in recovery, the function must exit
>>> with an error. That is, IMO, something like the following check
>>> must be added.
>>> 
>>> if (RecoveryInProgress()) ereport(ERROR,
>>> 
>>> (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), 
>>> errmsg("recovery is in progress"), errhint("GIN pending list
>>> cannot be cleaned up during recovery.")));
>>> 
>>> It's better to make gin_clean_pending_list() check whether the
>>> target index is temporary index of other sessions or not, like
>>> pgstatginindex() does.
>> 
>> I've added both of these checks.  Sorry I overlooked your early
>> email in this thread about those.
>> 
>>> 
>>> +RelationindexRel = index_open(indexoid,
>>> AccessShareLock);
>>> 
>>> ISTM that AccessShareLock is not safe when updating the pending
>>> list and GIN index main structure. Probaby we should use
>>> RowExclusiveLock?
>> 
>> Other callers of the ginInsertCleanup function also do so while 
>> holding only the AccessShareLock on the index.  It turns out
>> that there is a bug around this, as discussed in "Potential GIN
>> vacuum bug" 
>> (http://www.postgresql.org/message-id/flat/CAMkU=1xalflhuuohfp5v33rzedlvb5aknnujceum9knbkrb...@mail.gmail.com)
>>
>>
>> 
But, that bug has to be solved at a deeper level than this patch.
>> 
>> I've also cleaned up some other conflicts, and chose a more
>> suitable OID for the new catalog function.
>> 
>> The number of new header includes needed to implement this makes
>> me wonder if I put this code in the correct file, but I don't see
>> a better location for it.
>> 
>> New version attached.
> 
> Thanks for updating the patch! It looks good to me.
> 
> Based on your patch, I just improved the doc. For example, I added
> the following note into the doc.
> 
> +These functions cannot be executed during recovery. +Use
> of these functions is restricted to superusers and the owner +
> of the given index.
> 
> If there is no problem, I'm thinking to commit this version.
> 

Just a detail:

+Note that the cleanup does not happen and the return value is 0
+if the argument is the GIN index built with fastupdate
+option disabled because it doesn't have a pending list.

It should be "if the argument is *a* GIN index"

I find this sentence a little confusing, maybe rephrase like this would
be better:

-Note that the cleanup does not happen and the return value is 0
-if the argument is the GIN index built with fastupdate
-option disabled because it doesn't have a pending list.
+Note that if the argument is a GIN index built with
fastupdate
+option disabled, the cleanup does not happen and the return value
is 0
+because the index doesn't have a pending list.

Otherwise, I don't see any problem on this version.

Regards.

> Regards,
> 
> 
> 
> 


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


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

Re: [HACKERS] [PROPOSAL] VACUUM Progress Checker.

[Robert Haas]

On Tue, Jan 26, 2016 at 11:37 PM, Vinayak Pokale  wrote:
> Hi,
>
> Please find attached updated patch with an updated interface.

Well, this isn't right.  You've got this sort of thing:

+scanned_index_pages += RelationGetNumberOfBlocks(Irel[i]);
+/* Report progress to the statistics collector */
+pgstat_report_progress_update_message(0, progress_message);
+pgstat_report_progress_update_counter(1, scanned_heap_pages);
+pgstat_report_progress_update_counter(3, scanned_index_pages);
+pgstat_report_progress_update_counter(4,
vacrelstats->num_index_scans + 1);

The point of having pgstat_report_progress_update_counter() is so that
you can efficiently update a single counter without having to update
everything, when only one counter has changed.  But here you are
calling this function a whole bunch of times in a row, which
completely misses the point - if you are updating all the counters,
it's more efficient to use an interface that does them all at once
instead of one at a time.

But there's a second problem here, too, which is that I think you've
got this code in the wrong place.  The second point of the
pgstat_report_progress_update_counter interface is that this should be
cheap enough that we can do it every time the counter changes.  That's
not what you are doing here.  You're updating the counters at various
points in the code and just pushing new values for all of them
regardless of which ones have changed.  I think you should find a way
that you can update the value immediately at the exact moment it
changes.  If that seems like too much of a performance hit we can talk
about it, but I think the value of this feature will be greatly
weakened if users can't count on it to be fully and continuously up to
the moment.  When something gets stuck, you want to know where it's
stuck, not approximately kinda where it's stuck.

+if(!scan_all)
+scanned_heap_pages = scanned_heap_pages +
next_not_all_visible_block;

I don't want to be too much of a stickler for details here, but it
seems to me that this is an outright lie.  The number of scanned pages
does not go up when we decide to skip some pages, because scanning and
skipping aren't the same thing.  If we're only going to report one
number here, it needs to be called something like "current heap page",
and then you can just report blkno at the top of each iteration of
lazy_scan_heap's main loop.  If you want to report the numbers of
scanned and skipped pages separately that'd be OK too, but you can't
call it the number of scanned pages and then actually report a value
that is not that.

+/*
+ * Reporting vacuum progress to statistics collector
+ */

This patch doesn't report anything to the statistics collector, nor should it.

Instead of making the SQL-visible function
pg_stat_get_vacuum_progress(), I think it should be something more
generic like pg_stat_get_command_progress().  Maybe VACUUM will be the
only command that reports into that feature for right now, but I'd
hope for us to change that pretty soon after we get the first patch
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] Interesting read on SCM upending software and hardware architecture

[Tomasz Rybak]

W dniu 18.01.2016, pon o godzinie 18∶55 -0600, użytkownik Jim Nasby
napisał:
[ cut ]
> 
> My original article doesn't talk about SSDs; it's talking about 
> non-volatile memory architectures (quoted extract below). Fusion IO
> is 
> an example of this, and if NVDIMMs become available we'll see even 
> faster non-volatile performance.
> 
> To me, the most interesting point the article makes is that systems
> now 
> need much better support for multiple classes of NV storage. I agree 
> with your point that spinning rust is here to stay for a long time, 
> simply because it's cheap as heck. So systems need to become much
> better 
> at moving data between different layers of NV storage so that you're 
> getting the biggest bang for the buck. That will remain critical as
> long 
> as SCM's remain 25x more expensive than rust.
> 

I guess PostgreSQL is getting ready for such a world.
Parallel sequential scan, while not useful for spinning drives,
should shine with hardware describe in that article.

Add some tuning of effective_io_concurrency and we might
have some gains even without new storage layer.
Of course ability to change storage subsystem might
help with experimentation, but even now (OK, when 9.6 is out)
we might use increased IO concurrency.

-- 
Tomasz Rybak  GPG/PGP key ID: 2AD5 9860
Fingerprint A481 824E 7DD3 9C0E C40A  488E C654 FB33 2AD5 9860
http://member.acm.org/~tomaszrybak




signature.asc
Description: This is a digitally signed message part

Re: [HACKERS] Implementing a new Scripting Language

[Igal @ Lucee.org]

On 1/27/2016 11:47 AM, Chapman Flack wrote:


Thanks. :)  On occasions in the past I have written it
correctly ... there is evidence in the archives 
I believe that!  I actually never remember if it's 223 or 233 and I 
always google it before I post, so in a way I cheated ;)



we will be able to use Javascript
through that via the Nashorn project,

Yes, that's an attraction for me, as the JSR-two-twenty-three
engine for JS is now typically already included with the JRE.
(Technically, I keep reading that there is no two-twenty-three
engine that *needs* to be included and it's totally up to the
JRE packager ... but there have actually been some JavaScript
snippets in PL/Java's build process since July 2014 and no one
has yet reported they broke the build.)
This can be a major thing.  I will open a ticket in 
https://github.com/tada/pljava -- or is it already on the roadmap?



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

Re: [HACKERS] extend pgbench expressions with functions

[Robert Haas]

On Wed, Jan 27, 2016 at 3:39 AM, Fabien COELHO  wrote:
> Attached is a rebase after recent changes in pgbench code & doc.

+/* use short names in the evaluator */
+#define INT(v) coerceToInt()
+#define DOUBLE(v) coerceToDouble()
+#define SET_INT(pv, ival) setIntValue(pv, ival)
+#define SET_DOUBLE(pv, dval) setDoubleValue(pv, dval)

I don't like this at all.  It seems to me that this really obscures
the code.  The few extra characters are a small price to pay for not
having to go look up the macro definition to understand what the code
is doing.

The third hunk in pgbench.c unnecessary deletes a blank line.

/*
 * inner expresion in (cut, 1] (if parameter > 0), rand in [0, 1)
+* Assert((1.0 - cut) != 0.0);
 */
-   Assert((1.0 - cut) != 0.0);
rand = -log(cut + (1.0 - cut) * uniform) / parameter;
+

Moving the Assert() into the comment seems like a bad plan.  If the
Assert is true, it shouldn't be commented out.  If it's not, it
shouldn't be there at all.

Commit e41beea0ddb74ef975f08b917a354ec33cb60830, which you wrote, went
to some trouble to display good context for error messages.  What you
have here seems like a huge step backwards:

+   fprintf(stderr, "double to int overflow for
%f\n", dval);
+   exit(1);

So, we're just going to give up on all of that error context reporting
that we added back then?  That would be sad.

-- 
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] Patch: ResourceOwner optimization for tables with many partitions

[Robert Haas]

On Wed, Jan 27, 2016 at 3:57 AM, Aleksander Alekseev
 wrote:
> I'm a bit concerned regarding assumption that sizeof int never exceeds 4
> bytes. While this could be true today for most C compilers, standard
> [1][2] doesn't guarantee that. Perhaps we should add something like:
>
> StaticAssertStmt(sizeof(int) <= sizeof(int32),
> "int size exceeds int32 size");

I suspect that if this ever failed to be true, resowner.c would not be
the only thing having problems.

-- 
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] Implementing a new Scripting Language

[Igal @ Lucee.org]

On 1/27/2016 7:12 PM, Chapman Flack wrote:


Now that you mention it, it isn't officially in a ticket. Though it's
not like I was going to forget. :)  I can guarantee it won't be in 1.5...

Speaking of tickets, I should probably make actual tickets, for after
1.5.0, out of all the items now showing in the "known issues" section
of the draft release notes. More chores


Well, I'm not very familiar with PostgreSQL yet, but am trying to learn.

If I can help with anything with the pl/Java project I'd love to help.


Igal


--
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] checkpointer continuous flushing

[Robert Haas]

On Wed, Jan 20, 2016 at 9:02 AM, Andres Freund  wrote:
> Chatting on IM with Heikki, I noticed that we're pretty pessimistic in
> SetHintBits(). Namely we don't set the bit if XLogNeedsFlush(commitLSN),
> because we can't easily set the LSN. But, it's actually fairly common
> that the pages LSN is already newer than the commitLSN - in which case
> we, afaics, just can go ahead and set the hint bit, no?
>
> So, instead of
> if (XLogNeedsFlush(commitLSN) && BufferIsPermanent(buffer)
> return; /* not flushed yet, 
> so don't set hint */
> we do
> if (BufferIsPermanent(buffer) && XLogNeedsFlush(commitLSN)
> && BufferGetLSNAtomic(buffer) < commitLSN)
> return; /* not flushed yet, 
> so don't set hint */
>
> In my tests with pgbench -s 100, 2GB of shared buffers, that's recovers
> a large portion of the hint writes that we currently skip.

Dang.  That's a really good idea.  Although I think you'd probably
better revise the comment, since it will otherwise be false.

-- 
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] Fwd: Core dump with nested CREATE TEMP TABLE

[Noah Misch]

On Thu, Jan 28, 2016 at 12:57:36PM +0900, Michael Paquier wrote:
> On Thu, Jan 28, 2016 at 12:40 PM, Noah Misch  wrote:
> + * fresh transaction.  No part of core PostgreSQL functions that way,
> + * though it's a fair thing to want.  Such code would wish the portal

> From the point of view of core code, this stands true, but, for my 2c,
> honestly, I think that is just going to annoy more people working on
> plugins and forks of Postgres. When working on Postgres-XC and
> developing stuff for the core code, I recall having been annoyed a
> couple of times by similar assert limitations

At first, I left out that assertion in case some extension code did the thing
I described, perhaps in a background worker.  I then realized that
MarkPortalFailed() is the wrong thing for such code, which would want
treatment similar to this bit of PreCommit_Portals():

/*
 * Do not touch active portals --- this can only happen in the 
case of
 * a multi-transaction utility command, such as VACUUM.
 *
 * Note however that any resource owner attached to such a 
portal is
 * still going to go away, so don't leave a dangling pointer.
 */
if (portal->status == PORTAL_ACTIVE)
{
portal->resowner = NULL;
continue;
}

If you can think of a case where the code would work okay despite its active
portal being marked as failed, that would be a good reason to omit the one
assertion.  Otherwise, an assertion seems better than silently doing the
known-wrong thing.


-- 
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] Mac OS: invalid byte sequence for encoding "UTF8"

[Artur Zakirov]

On 27.01.2016 14:14, Stas Kelvich wrote:

Hi.

I tried that and confirm strange behaviour. It seems that problem with small 
cyrillic letter ‘х’. (simplest obscene language filter? =)

That can be reproduced with simpler test

Stas




The test program was corrected. Now it uses wchar_t type. And it works 
correctly and gives right output.


I think the NIImportOOAffixes() in spell.c should be corrected to avoid 
this bug.


--
Artur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
#include 
#include 
#include 

char *src = "SFX Y   хаться шутсяхаться";

int
main(int argc, char *argv[])
{
	wchar_t c1[1024], c2[1024], c3[1024], c4[1024], c5[1024];

	setlocale(LC_CTYPE, "ru_RU.UTF-8");

	sscanf(src, "%6ls %204ls %204ls %204ls %204ls", c1, c2, c3, c4, c5);

	printf("%ls/%ls/%ls/%ls/%ls\n", c1, c2, c3, c4, c5);

	return 0;
}

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

[HACKERS] Fwd: [DOCS] pgbench doc typos

[Erik Rijkers]

Two trivial changes to  doc/src/sgml/ref/pgbench.sgml

Erik Rijkers


--
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: fix lock contention for HASHHDR.mutex

[Aleksander Alekseev]

> > This patch affects header files. By any chance didn't you forget to
> > run `make clean` after applying it? As we discussed above, when you
> > change .h files autotools doesn't rebuild dependent .c files:
> >
> 
> Yes, actually i always compile using "make clean;make -j20; make
> install" If you want i will run it again may be today or tomorrow and
> post the result.
> 
> 

Most likely HASHHDR.mutex is not only bottleneck in your case so my
patch doesn't help much. Unfortunately I don't have access to any
POWER8 server so I can't investigate this issue. I suggest to use a
gettimeofday trick I described in a first message of this thread. Its
time consuming but it gives a clear understanding which code is keeping
a lock.


-- 
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] better systemd integration

[Pavel Stehule]

Hi

2015-11-17 15:08 GMT+01:00 Peter Eisentraut :

> I have written a couple of patches to improve the integration of the
> postgres daemon with systemd.
>
> The setup that is shipped with Red Hat- and Debian-family packages at
> the moment is just an imitation of the old shell scripts, relying on
> polling by pg_ctl for readiness, with various custom pieces of
> complexity for handling custom port numbers and such.
>
> In the first patch, my proposal is to use sd_notify() calls from
> libsystemd to notify the systemd daemon directly when startup is
> completed.  This is the recommended low-overhead solution that is now
> being adopted by many other server packages.  It allows us to cut out
> pg_ctl completely from the startup configuration and makes the startup
> configuration manageable by non-wizards.  An example is included in the
> patch.
>
> The second patch improves integration with the system journal managed by
> systemd.  This is a facility that captures a daemon's standard output
> and error and records it in configurable places, including syslog.  The
> patch adds a new log_destination that is like stderr but marks up the
> output so that systemd knows the severity.  With that in place, users
> can choose to do away with the postgres log file management and let
> systemd do it.
>
> The third patch is technically unrelated but arose while I was working
> on this.  It improves error reporting when the data directory is missing.
>


2. all tests passed

The issues:

1. configure missing systemd integration test, compilation fails:

postmaster.o postmaster.c
postmaster.c:91:31: fatal error: systemd/sd-daemon.h: No such file or
directory

3. PostgreSQL is able to write to systemd log, but multiline entry was
stored with different priorities

 do $$ begin raise warning 'NAZDAREK'; end $$;

first line

{
"__CURSOR" :
"s=cac797bc03f242febea9f32357bba773;i=b4a5;b=e8d5b3df2ebf46dd86c39046b326bd32;m=1cb792a63b;t=52a4f3ad40860;x=57014959bf6e3481",
"__REALTIME_TIMESTAMP" : "1453894661310560",
"__MONOTONIC_TIMESTAMP" : "123338925627",
"_BOOT_ID" : "e8d5b3df2ebf46dd86c39046b326bd32",
"SYSLOG_FACILITY" : "3",
"_UID" : "1001",
"_GID" : "1001",
"_CAP_EFFECTIVE" : "0",
"_SELINUX_CONTEXT" : "system_u:system_r:init_t:s0",
"_MACHINE_ID" : "b8299a722638414a8776d3e130e228e4",
"_HOSTNAME" : "localhost.localdomain",
"_SYSTEMD_SLICE" : "system.slice",
"_TRANSPORT" : "stdout",
"SYSLOG_IDENTIFIER" : "postgres",
"_PID" : "3150",
"_COMM" : "postgres",
"_EXE" : "/usr/local/pgsql/bin/postgres",
"_CMDLINE" : "/usr/local/pgsql/bin/postgres -D /usr/local/pgsql/data -c
log_destination=systemd",
"_SYSTEMD_CGROUP" : "/system.slice/postgresql.service",
"_SYSTEMD_UNIT" : "postgresql.service",
"PRIORITY" : "5",
"MESSAGE" : "WARNING:  NAZDAREK"
}

second line

{
"__CURSOR" :
"s=cac797bc03f242febea9f32357bba773;i=b4a6;b=e8d5b3df2ebf46dd86c39046b326bd32;m=1cb792a882;t=52a4f3ad40aa6;x=ae9801b2ecbd4da3",
"__REALTIME_TIMESTAMP" : "1453894661311142",
"__MONOTONIC_TIMESTAMP" : "123338926210",
"_BOOT_ID" : "e8d5b3df2ebf46dd86c39046b326bd32",
"PRIORITY" : "6",
"SYSLOG_FACILITY" : "3",
"_UID" : "1001",
"_GID" : "1001",
"_CAP_EFFECTIVE" : "0",
"_SELINUX_CONTEXT" : "system_u:system_r:init_t:s0",
"_MACHINE_ID" : "b8299a722638414a8776d3e130e228e4",
"_HOSTNAME" : "localhost.localdomain",
"_SYSTEMD_SLICE" : "system.slice",
"_TRANSPORT" : "stdout",
"SYSLOG_IDENTIFIER" : "postgres",
"_PID" : "3150",
"_COMM" : "postgres",
"_EXE" : "/usr/local/pgsql/bin/postgres",
"_CMDLINE" : "/usr/local/pgsql/bin/postgres -D /usr/local/pgsql/data -c
log_destination=systemd",
"_SYSTEMD_CGROUP" : "/system.slice/postgresql.service",
"_SYSTEMD_UNIT" : "postgresql.service",
"MESSAGE" : "CONTEXT:  PL/pgSQL function inline_code_block line 1 at
RAISE"
}

Is it expected?

Second issue:

Mapping of levels between pg and journal levels is moved by1

+case DEBUG1:
+systemd_log_prefix = "<7>" /* SD_DEBUG */;
+break;
+case LOG:
+case COMMERROR:
+case INFO:
+systemd_log_prefix = "<6>" /* SD_INFO */;
+break;
+case NOTICE:
+case WARNING:
+systemd_log_prefix = "<5>" /* SD_NOTICE */;
+break;
+case ERROR:
+systemd_log_prefix = "<4>" /* SD_WARNING */;
+break;
+case FATAL:
+systemd_log_prefix = "<3>" /* SD_ERR */;
+break;
+case PANIC:

is it expected?

This is little bit unexpected - (can be correct).

When I use filtering "warnings", then I got errors, etc. I can understand
so these systems are not compatible, but these differences should be well
documented.

I didn't find any other issues. It is working without any problems.

Re: [HACKERS] pgbench stats per script & other stuff

[Alvaro Herrera]

Fabien COELHO wrote:

> >It seems a bit funny to have the start_time not be reset when 0.0 is
> >passed, which is almost all the callers.  Using a float as a boolean
> >looks pretty odd; is that kosher?  Maybe it'd be a good idea to have a
> >separate boolean flag instead?

> Obviously this would work. I did not think the special case was worth the
> extra argument. This one has some oddity too, because the second argument is
> ignored depending on the third. Do as you feel.

Actually my question was whether keeping the original start_time was the
intended design.  I think some places are okay with keeping the original
value, but the ones in addScript, the per-thread loop in main(), and the
global one also in main() should all be getting a 0.0 instead of leaving
the value uninitialized.

(I did turn the arguments around so that the bool is second and the
float is third.  Thanks for the suggestion.)

-- 
Á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] Mac OS: invalid byte sequence for encoding "UTF8"

[Stas Kelvich]

Hi.

I tried that and confirm strange behaviour. It seems that problem with small 
cyrillic letter ‘х’. (simplest obscene language filter? =)

That can be reproduced with simpler test

Stas



test.c
Description: Binary data

 
> On 27 Jan 2016, at 13:59, Artur Zakirov  wrote:
> 
> On 27.01.2016 13:46, Shulgin, Oleksandr wrote:
>> 
>> Not sure why the file uses "SET KOI8-R" directive then?
>> 
> 
> This directive is used only by Hunspell program. PostgreSQL ignores this 
> directive and assumes that input affix and dictionary files in the UTF-8 
> encoding.
> 
>> 
>> 
>> What error message do you get with this test program?  (I don't get any,
>> but I'm not on Mac OS.)
>> --
>> Alex
>> 
>> 
> 
> With this program you will get wrong output. A error message is not called. 
> You can execute the following commands:
> 
> > cc test.c -o test
> > ./test
> 
> You will get the output:
> 
> SFX/Y/?/аться/шутся
> 
> Although the output should be:
> 
> SFX/Y/хаться/шутся/хаться
> 
> -- 
> Artur Zakirov
> Postgres Professional: http://www.postgrespro.com
> Russian Postgres Company
> 
> 
> -- 
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers


-- 
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] Fwd: [DOCS] pgbench doc typos

[Erik Rijkers]

On 2016-01-27 11:06, Erik Rijkers wrote:

Two trivial changes to  doc/src/sgml/ref/pgbench.sgml


Sorry - now attached.

Erik Rijkers

--- ./doc/src/sgml/ref/pgbench.sgml.orig	2016-01-27 12:29:16.857488633 +0100
+++ ./doc/src/sgml/ref/pgbench.sgml	2016-01-27 12:30:09.643862616 +0100
@@ -1056,7 +1056,7 @@
  0 84 4142 0 1412881037 918023 2333
  0 85 2465 0 1412881037 919759 740
 
-   In this example, transaction 82 was late, because it's latency (6.173 ms) was
+   In this example, transaction 82 was late, because its latency (6.173 ms) was
over the 5 ms limit. The next two transactions were skipped, because they
were already late before they were even started.
   
@@ -1097,7 +1097,7 @@
   
 
   
-   Here is example outputs:
+   Here is example output:
 
 1345828501 5601 1542744 483552416 61 2573
 1345828503 7884 1979812 565806736 60 1479

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

[HACKERS] Using user mapping OID as hash key for connection hash

[Ashutosh Bapat]

Hi All,
As discussed in postgres_fdw join pushdown thread [1], for two different
effective local users which use public user mapping we will be creating two
different connections to the foreign server with the same credentials.

Robert suggested [2] that we should use user mapping OID as the connection
cache key instead of current userid and serverid.

There are two patches attached here:
1. pg_fdw_concache.patch.short - shorter version of the fix. Right now
ForeignTable, ForeignServer have corresponding OIDs saved in these
structures. But UserMapping doesn't. Patch adds user mapping OID as a
member to this structure. This member is then used as key in
GetConnection().
2. pg_fdw_concache.patch.large - most of the callers of GetConnection() get
ForeignServer object just to pass it to GetConnection(). GetConnection can
obtain ForeignServer by using serverid from UserMapping and doesn't need
ForeignServer to be an argument. Larger version of patch has this change.

GetConnection has named the UserMapping argument as just "user", ideally it
should have been named user_mapping. But that seems to be too obvious to be
unintentional. So, I have left that change.

The patch has added userid and user mapping oid to a debug3 message in
GetConnection(). the message is displayed when a new connection to foreign
server is created. With only that change, if we run script multi_conn.sql
(attached) we see that it's creating two connections when same user mapping
is used. Also attached is the output of the same script run on my setup.
Since min_messages is set to DEBUG3 there are too many unrelated messages.
So, search for "new postgres_fdw connection .." for new connection messages.

I have included the changes to the DEBUG3 message in GetConnection(), since
it may be worth committing those changes. In case, reviewers/committers
disagree, those chagnes can be removed.

[1]
http://www.postgresql.org/message-id/CAFjFpRf-LiD5bai4D6cSUseJh=xxjqipo_vn8mtnzg16tmw...@mail.gmail.com
[2]
http://www.postgresql.org/message-id/ca+tgmoymmv_du-vppq1d7ufsjaopbq+lgpxtchnuqfobjg2...@mail.gmail.com
-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company


pg_fdw_concache.patch.large
Description: Binary data


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

[Rushabh Lathia]

On Wed, Jan 27, 2016 at 2:50 PM, Etsuro Fujita 
wrote:

> On 2016/01/27 12:20, Etsuro Fujita wrote:
>
>> On 2016/01/26 22:57, Rushabh Lathia wrote:
>>
>>> On Tue, Jan 26, 2016 at 4:15 PM, Etsuro Fujita
>>> >
>>> wrote:
>>>
>>> On 2016/01/25 17:03, Rushabh Lathia wrote:
>>>
>>
> int
>>> IsForeignRelUpdatable (Relation rel);
>>>
>>
> Documentation for IsForeignUpdatable() need to change as it says:
>>>
>>> If the IsForeignRelUpdatable pointer is set to NULL, foreign
>>> tables are
>>> assumed
>>> to be insertable, updatable, or deletable if the FDW provides
>>> ExecForeignInsert,
>>> ExecForeignUpdate or ExecForeignDelete respectively.
>>>
>>> With introduce of DMLPushdown API now this is no more correct,
>>> as even if
>>> FDW don't provide ExecForeignInsert, ExecForeignUpdate or
>>> ExecForeignDelete API
>>> still foreign tables are assumed to be updatable or deletable
>>> with
>>> DMLPushdown
>>> API's, right ?
>>>
>>
> That's what I'd like to discuss.
>>>
>>> I intentionally leave that as-is, because I think we should
>>> determine the updatability of a foreign table in the current
>>> manner.  As you pointed out, even if the FDW doesn't provide eg,
>>> ExecForeignUpdate, an UPDATE on a foreign table could be done using
>>> the DML pushdown APIs if the UPDATE is *pushdown-safe*.  However,
>>> since all UPDATEs on the foreign table are not necessarily
>>> pushdown-safe, I'm not sure it's a good idea to assume the
>>> table-level updatability if the FDW provides the DML pushdown
>>> callback routines.  To keep the existing updatability decision, I
>>> think the FDW should provide the DML pushdown callback routines
>>> together with ExecForeignInsert, ExecForeignUpdate, or
>>> ExecForeignDelete.  What do you think about that?
>>>
>>
> Sorry but I am not in favour of adding compulsion that FDW should provide
>>> the DML pushdown callback routines together with existing
>>> ExecForeignInsert,
>>> ExecForeignUpdate or ExecForeignDelete APIs.
>>>
>>> May be we should change the documentation in such way, that explains
>>>
>>> a) If FDW PlanDMLPushdown is NULL, then check for ExecForeignInsert,
>>> ExecForeignUpdate or ExecForeignDelete APIs
>>> b) If FDW PlanDMLPushdown is non-NULL and plan is not pushable
>>> check for ExecForeignInsert, ExecForeignUpdate or ExecForeignDelete APIs
>>> c) If FDW PlanDMLPushdown is non-NULL and plan is pushable
>>> check for DMLPushdown APIs.
>>>
>>> Does this sounds wired ?
>>>
>>
> Yeah, but I think that that would be what is done during executor
>> startup (see CheckValidResultRel()), while what the documentation is
>> saying is about relation_is_updatable(); that is, how to decide the
>> updatability of a given foreign table, not how the executor processes an
>> individual INSERT/UPDATE/DELETE on a updatable foreign table.  So, I'm
>> not sure it's a good idea to modify the documentation in such a way.
>>
>
> However, I agree that we should add a documentation note about the
>> compulsion somewhere.  Maybe something like this:
>>
>> The FDW should provide DML pushdown callback routines together with
>> table-updating callback routines described above.  Even if the callback
>> routines are provided, the updatability of a foreign table is determined
>> based on the presence of ExecForeignInsert, ExecForeignUpdate or
>> ExecForeignDelete if the IsForeignRelUpdatable pointer is set to NULL.
>>
>
> On second thought, I think it might be okay to assume the presence of
> PlanDMLPushdown, BeginDMLPushdown, IterateDMLPushdown, and EndDMLPushdown
> is also sufficient for the insertablity, updatability, and deletability of
> a foreign table, if the IsForeignRelUpdatable pointer is set to NULL.  How
> about modifying the documentation like this:
>
> If the IsForeignRelUpdatable pointer is set to NULL, foreign tables are
> assumed to be insertable, updatable, or deletable if the FDW provides
> ExecForeignInsert, ExecForeignUpdate, or ExecForeignDelete respectively, or
> if the FDW provides PlanDMLPushdown, BeginDMLPushdown, IterateDMLPushdown,
> and EndDMLPushdown described below.
>
> Of course, we also need to modify relation_is_updatable() accordingly.
>
>
> What's your opinion?
>
>
If I understood correctly, above documentation means, that if FDW have
DMLPushdown APIs that is enough. But in reality thats not the case, we need
 ExecForeignInsert, ExecForeignUpdate, or ExecForeignDelete in case DML is
not pushable.

And here fact is DMLPushdown APIs are optional for FDW, so that if FDW
don't have DMLPushdown APIs they can still very well perform the DML
operations using ExecForeignInsert, ExecForeignUpdate, or
ExecForeignDelete. So documentation should be like:

If the IsForeignRelUpdatable pointer is set to 

Re: [HACKERS] GIN pending list clean up exposure to SQL

[Fujii Masao]

On Mon, Jan 25, 2016 at 3:54 PM, Jeff Janes  wrote:
> On Wed, Jan 20, 2016 at 6:17 AM, Fujii Masao  wrote:
>> On Sat, Jan 16, 2016 at 7:42 AM, Julien Rouhaud
>>  wrote:
>>> On 15/01/2016 22:59, Jeff Janes wrote:
 On Sun, Jan 10, 2016 at 4:24 AM, Julien Rouhaud
  wrote:
>>>
>>> All looks fine to me, I flag it as ready for committer.
>>
>> When I compiled the PostgreSQL with the patch, I got the following error.
>> ISTM that the inclusion of pg_am.h header file is missing in ginfast.c.
>
> Thanks.  Fixed.
>
>> gin_clean_pending_list() must check whether the server is in recovery or not.
>> If it's in recovery, the function must exit with an error. That is, IMO,
>> something like the following check must be added.
>>
>>  if (RecoveryInProgress())
>>  ereport(ERROR,
>>
>> (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
>>   errmsg("recovery is in progress"),
>>   errhint("GIN pending list cannot be
>> cleaned up during recovery.")));
>>
>> It's better to make gin_clean_pending_list() check whether the target index
>> is temporary index of other sessions or not, like pgstatginindex() does.
>
> I've added both of these checks.  Sorry I overlooked your early email
> in this thread about those.
>
>>
>> +RelationindexRel = index_open(indexoid, AccessShareLock);
>>
>> ISTM that AccessShareLock is not safe when updating the pending list and
>> GIN index main structure. Probaby we should use RowExclusiveLock?
>
> Other callers of the ginInsertCleanup function also do so while
> holding only the AccessShareLock on the index.  It turns out that
> there is a bug around this, as discussed in "Potential GIN vacuum bug"
> (http://www.postgresql.org/message-id/flat/CAMkU=1xalflhuuohfp5v33rzedlvb5aknnujceum9knbkrb...@mail.gmail.com)
>
> But, that bug has to be solved at a deeper level than this patch.
>
> I've also cleaned up some other conflicts, and chose a more suitable
> OID for the new catalog function.
>
> The number of new header includes needed to implement this makes me
> wonder if I put this code in the correct file, but I don't see a
> better location for it.
>
> New version attached.

Thanks for updating the patch! It looks good to me.

Based on your patch, I just improved the doc. For example,
I added the following note into the doc.

+These functions cannot be executed during recovery.
+Use of these functions is restricted to superusers and the owner
+of the given index.

If there is no problem, I'm thinking to commit this version.

Regards,

-- 
Fujii Masao
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***
*** 18036,18044  postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup());
--- 18036,18051 
  brin_summarize_new_values
 
  
+
+ gin_clean_pending_list
+
+ 
 
   shows the functions
  available for index maintenance tasks.
+ These functions cannot be executed during recovery.
+ Use of these functions is restricted to superusers and the owner
+ of the given index.
 
  
 
***
*** 18056,18061  postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup());
--- 18063,18075 
 integer
 summarize page ranges not already summarized

+   
+
+ gin_clean_pending_list(index regclass)
+
+bigint
+move GIN pending list entries into main index structure
+   
   
  
 
***
*** 18069,18074  postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup());
--- 18083,18100 
  into the index.
 
  
+
+ gin_clean_pending_list accepts the OID or name of
+ a GIN index and cleans up the pending list of the specified GIN index
+ by moving entries in it to the main GIN data structure in bulk.
+ It returns the number of pages cleaned up from the pending list.
+ Note that the cleanup does not happen and the return value is 0
+ if the argument is the GIN index built with fastupdate
+ option disabled because it doesn't have a pending list.
+ Please see  and 
+ for details of the pending list and fastupdate option.
+
+ 

  

*** a/doc/src/sgml/gin.sgml
--- b/doc/src/sgml/gin.sgml
***
*** 734,740 
 from the indexed item). As of PostgreSQL 8.4,
 GIN is capable of postponing much of this work by inserting
 new tuples into a temporary, unsorted list of pending entries.
!When the table is vacuumed, or if the pending list becomes larger than
 , the entries are moved to the
 main GIN data structure using the same bulk insert
 techniques used during initial index creation.  This greatly improves
--- 734,742 
 from the indexed item). As of PostgreSQL 8.4,
 GIN is capable of postponing much of this work 

Re: [HACKERS] WIP: Failover Slots

[Craig Ringer]

Hi all

Here's v3 of failover slots.

It doesn't add the UI yet, but it's now functionally complete except for
timeline following for logical slots, and I have a plan for that.
From 533a9327b54ba744b0a1fb0048e8cfe7d3d45ea1 Mon Sep 17 00:00:00 2001
From: Craig Ringer 
Date: Wed, 20 Jan 2016 17:16:29 +0800
Subject: [PATCH 1/2] Implement failover slots

Originally replication slots were unique to a single node and weren't
recorded in WAL or replicated. A logical decoding client couldn't follow
a physical standby failover and promotion because the promoted replica
didn't have the original master's slots. The replica may not have
retained all required WAL and there was no way to create a new logical
slot and rewind it back to the point the logical client had replayed to
anyway.

Failover slots lift this limitation by replicating slots consistently to
physical standbys, keeping them up to date and using them in WAL
retention calculations. This allows a logical decoding client to follow
a physical failover and promotion without losing its place in the change
stream.

Simon Riggs and Craig Ringer

WIP. Open items:

* Testing
* Implement !failover slots and UI for marking slots as failover slots
* Fix WAL retention for slots created before a basebackup
---
 src/backend/access/rmgrdesc/Makefile   |   2 +-
 src/backend/access/rmgrdesc/replslotdesc.c |  63 
 src/backend/access/transam/rmgr.c  |   1 +
 src/backend/access/transam/xlogutils.c |   6 +-
 src/backend/commands/dbcommands.c  |   3 +
 src/backend/replication/basebackup.c   |  12 -
 src/backend/replication/logical/decode.c   |   1 +
 src/backend/replication/logical/logical.c  |  19 +-
 src/backend/replication/logical/logicalfuncs.c |   3 +
 src/backend/replication/slot.c | 439 -
 src/backend/replication/slotfuncs.c|   1 +
 src/bin/pg_xlogdump/replslotdesc.c |   1 +
 src/bin/pg_xlogdump/rmgrdesc.c |   1 +
 src/include/access/rmgrlist.h  |   1 +
 src/include/replication/slot.h |  61 +---
 src/include/replication/slot_xlog.h| 103 ++
 16 files changed, 624 insertions(+), 93 deletions(-)
 create mode 100644 src/backend/access/rmgrdesc/replslotdesc.c
 create mode 12 src/bin/pg_xlogdump/replslotdesc.c
 create mode 100644 src/include/replication/slot_xlog.h

diff --git a/src/backend/access/rmgrdesc/Makefile b/src/backend/access/rmgrdesc/Makefile
index c72a1f2..600b544 100644
--- a/src/backend/access/rmgrdesc/Makefile
+++ b/src/backend/access/rmgrdesc/Makefile
@@ -10,7 +10,7 @@ include $(top_builddir)/src/Makefile.global
 
 OBJS = brindesc.o clogdesc.o committsdesc.o dbasedesc.o gindesc.o gistdesc.o \
 	   hashdesc.o heapdesc.o mxactdesc.o nbtdesc.o relmapdesc.o \
-	   replorigindesc.o seqdesc.o smgrdesc.o spgdesc.o \
+	   replorigindesc.o replslotdesc.o seqdesc.o smgrdesc.o spgdesc.o \
 	   standbydesc.o tblspcdesc.o xactdesc.o xlogdesc.o
 
 include $(top_srcdir)/src/backend/common.mk
diff --git a/src/backend/access/rmgrdesc/replslotdesc.c b/src/backend/access/rmgrdesc/replslotdesc.c
new file mode 100644
index 000..b882846
--- /dev/null
+++ b/src/backend/access/rmgrdesc/replslotdesc.c
@@ -0,0 +1,63 @@
+/*-
+ *
+ * replslotdesc.c
+ *	  rmgr descriptor routines for replication/slot.c
+ *
+ * Portions Copyright (c) 2015, PostgreSQL Global Development Group
+ *
+ *
+ * IDENTIFICATION
+ *	  src/backend/access/rmgrdesc/replslotdesc.c
+ *
+ *-
+ */
+#include "postgres.h"
+
+#include "replication/slot_xlog.h"
+
+void
+replslot_desc(StringInfo buf, XLogReaderState *record)
+{
+	char	   *rec = XLogRecGetData(record);
+	uint8		info = XLogRecGetInfo(record) & ~XLR_INFO_MASK;
+
+	switch (info)
+	{
+		case XLOG_REPLSLOT_UPDATE:
+			{
+ReplicationSlotInWAL xlrec;
+
+xlrec = (ReplicationSlotInWAL) rec;
+
+appendStringInfo(buf, "slot %s to xmin=%u, catmin=%u, restart_lsn="UINT64_FORMAT"@%u",
+		NameStr(xlrec->name), xlrec->xmin, xlrec->catalog_xmin,
+		xlrec->restart_lsn, xlrec->restart_tli);
+
+break;
+			}
+		case XLOG_REPLSLOT_DROP:
+			{
+xl_replslot_drop *xlrec;
+
+xlrec = (xl_replslot_drop *) rec;
+
+appendStringInfo(buf, "slot %s", NameStr(xlrec->name));
+
+break;
+			}
+	}
+}
+
+const char *
+replslot_identify(uint8 info)
+{
+	switch (info)
+	{
+		case XLOG_REPLSLOT_UPDATE:
+			return "CREATE_OR_UPDATE";
+		case XLOG_REPLSLOT_DROP:
+			return "DROP";
+		default:
+			return NULL;
+	}
+}
diff --git a/src/backend/access/transam/rmgr.c b/src/backend/access/transam/rmgr.c
index 7c4d773..0bd5796 100644
--- a/src/backend/access/transam/rmgr.c
+++ b/src/backend/access/transam/rmgr.c
@@ -24,6 +24,7 @@
 #include "commands/sequence.h"
 #include "commands/tablespace.h"
 #include 

[HACKERS] Trivial doc fix in logicaldecoding.sgml

[Shulgin, Oleksandr]

Hi,

Please find attached a simple copy-paste fix for CREATE_REPLICATION_SLOT
syntax.

--
Alex
From 05119485a473febe8ffd95103fd7774bc31ee079 Mon Sep 17 00:00:00 2001
From: Oleksandr Shulgin 
Date: Wed, 27 Jan 2016 11:27:35 +0100
Subject: [PATCH] Fix CREATE_REPLICATION_SLOT syntax in logicaldecoding.sgml

---
 doc/src/sgml/logicaldecoding.sgml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/src/sgml/logicaldecoding.sgml b/doc/src/sgml/logicaldecoding.sgml
index 1ae5eb6..926637b 100644
--- a/doc/src/sgml/logicaldecoding.sgml
+++ b/doc/src/sgml/logicaldecoding.sgml
@@ -280,7 +280,7 @@ $ pg_recvlogical -d postgres --slot test --drop-slot
 The commands
 
  
-  CREATE_REPLICATION_SLOT slot_name LOGICAL options
+  CREATE_REPLICATION_SLOT slot_name LOGICAL output_plugin
  
 
  
-- 
2.5.0


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

Re: [HACKERS] extend pgbench expressions with functions

[Fabien COELHO]

OK, so I had an extra look at this patch and I am marking it as ready
for committer.


Ok.


Attached is a rebase after recent changes in pgbench code & doc.

--
Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 42d0667..d42208a 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -796,17 +796,21 @@ pgbench  options  dbname
   Sets variable varname to an integer value calculated
   from expression.
   The expression may contain integer constants such as 5432,
-  references to variables :variablename,
+  double constants such as 3.14159,
+  references to integer variables :variablename,
   and expressions composed of unary (-) or binary operators
-  (+, -, *, /, %)
-  with their usual associativity, and parentheses.
+  (+, -, *, /,
+  %) with their usual associativity, function calls and
+  parentheses.
+   shows the available
+  functions.
  
 
  
   Examples:
 
 \set ntellers 10 * :scale
-\set aid (1021 * :aid) % (10 * :scale) + 1
+\set aid (1021 * random(1, 10 * :scale)) % (10 * :scale) + 1
 
 

@@ -826,66 +830,35 @@ pgbench  options  dbname
  
 
  
-  By default, or when uniform is specified, all values in the
-  range are drawn with equal probability.  Specifying gaussian
-  or  exponential options modifies this behavior; each
-  requires a mandatory parameter which determines the precise shape of the
-  distribution.
- 
+  
+   
+
+ \setrandom n 1 10 or \setrandom n 1 10 uniform
+ is equivalent to \set n random(1, 10) and uses a uniform
+ distribution.
+
+   
 
- 
-  For a Gaussian distribution, the interval is mapped onto a standard
-  normal distribution (the classical bell-shaped Gaussian curve) truncated
-  at -parameter on the left and +parameter
-  on the right.
-  Values in the middle of the interval are more likely to be drawn.
-  To be precise, if PHI(x) is the cumulative distribution
-  function of the standard normal distribution, with mean mu
-  defined as (max + min) / 2.0, with
-
- f(x) = PHI(2.0 * parameter * (x - mu) / (max - min + 1)) /
-(2.0 * PHI(parameter) - 1.0)
-
-  then value i between min and
-  max inclusive is drawn with probability:
-  f(i + 0.5) - f(i - 0.5).
-  Intuitively, the larger parameter, the more
-  frequently values close to the middle of the interval are drawn, and the
-  less frequently values close to the min and
-  max bounds. About 67% of values are drawn from the
-  middle 1.0 / parameter, that is a relative
-  0.5 / parameter around the mean, and 95% in the middle
-  2.0 / parameter, that is a relative
-  1.0 / parameter around the mean; for instance, if
-  parameter is 4.0, 67% of values are drawn from the
-  middle quarter (1.0 / 4.0) of the interval (i.e. from
-  3.0 / 8.0 to 5.0 / 8.0) and 95% from
-  the middle half (2.0 / 4.0) of the interval (second and
-  third quartiles). The minimum parameter is 2.0 for
-  performance of the Box-Muller transform.
- 
+  
+   
+\setrandom n 1 10 exponential 3.0 is equivalent to
+\set n random_exponential(1, 10, 3.0) and uses an
+exponential distribution.
+   
+  
 
- 
-  For an exponential distribution, parameter
-  controls the distribution by truncating a quickly-decreasing
-  exponential distribution at parameter, and then
-  projecting onto integers between the bounds.
-  To be precise, with
-
-f(x) = exp(-parameter * (x - min) / (max - min + 1)) / (1.0 - exp(-parameter))
-
-  Then value i between min and
-  max inclusive is drawn with probability:
-  f(x) - f(x + 1).
-  Intuitively, the larger parameter, the more
-  frequently values close to min are accessed, and the
-  less frequently values close to max are accessed.
-  The closer to 0 parameter, the flatter (more uniform)
-  the access distribution.
-  A crude approximation of the distribution is that the most frequent 1%
-  values in the range, close to min, are drawn
-  parameter% of the time.
-  parameter value must be strictly positive.
+  
+   
+\setrandom n 1 10 gaussian 2.0 is equivalent to
+\set n random_gaussian(1, 10, 2.0), and uses a gaussian
+distribution.
+   
+  
+ 
+
+   See the documentation of these functions below for further information
+   about the precise shape of these distributions, depending on the value
+   of the parameter.
  
 
  
@@ -965,18 +938,184 @@ f(x) = exp(-parameter * (x - min) / (max - min + 1)) / (1.0 - exp(-parameter))

   
 
+   
+   
+PgBench Functions
+
+ 
+  
+   Function
+   Return Type
+   Description
+   Example
+   Result
+  
+ 
+ 
+  

Re: [HACKERS] pgbench stats per script & other stuff

[Fabien COELHO]

Hello again,


Here's part b rebased, pgindented and with some minor additional tweaks
(mostly function commands and the function renames I mentioned).


Patch looks ok to me, various tests where ok as well.


Still concerned about the unlocked stat accums.


See my arguments in other mail. I can add a lock if this is a blocker, but 
I think that it is actually better without, because of quantum: the 
measuring process should avoid affecting the measured data, and locking is 
not cheap.



I haven't tried to rebase the other ones yet, they need manual conflict
fixes.


Find attached 14-c/d/e rebased patches.

About e, for some obscure reason I failed in my initial attempt at 
inserting the misplaced options in their rightfull position in the option 
list. Sorry for the noise.


--
Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 42d0667..ade1b53 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -1138,6 +1138,9 @@ number of transactions actually processed: 1/1
 tps = 618.764555 (including connections establishing)
 tps = 622.977698 (excluding connections establishing)
 SQL script 1: builtin: TPC-B (sort of)
+ - 1 transactions (100.0% of total, tps = 618.764555)
+ - latency average = 15.844 ms
+ - latency stddev = 2.715 ms
  - statement latencies in milliseconds:
 0.004386\set nbranches 1 * :scale
 0.001343\set ntellers 10 * :scale
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 305c319..5594d1c 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -164,6 +164,7 @@ bool		use_log;			/* log transaction latencies to a file */
 bool		use_quiet;			/* quiet logging onto stderr */
 int			agg_interval;		/* log aggregates instead of individual
  * transactions */
+boolper_script_stats = false; /* whether to collect stats per script */
 int			progress = 0;		/* thread progress report every this seconds */
 bool		progress_timestamp = false; /* progress report with Unix time */
 int			nclients = 1;		/* number of clients */
@@ -299,6 +300,7 @@ static struct
 {
 	const char *name;
 	Command   **commands;
+	StatsData stats;
 }	sql_script[MAX_SCRIPTS];	/* SQL script files */
 static int	num_scripts;		/* number of scripts in sql_script[] */
 static int	num_commands = 0;	/* total number of Command structs */
@@ -1308,7 +1310,7 @@ top:
 		/* transaction finished: calculate latency and log the transaction */
 		if (commands[st->state + 1] == NULL)
 		{
-			if (progress || throttle_delay || latency_limit || logfile)
+			if (progress || throttle_delay || latency_limit || per_script_stats || logfile)
 doTxStats(thread, st, , false, logfile, agg);
 			else
 thread->stats.cnt++;
@@ -1401,7 +1403,7 @@ top:
 	}
 
 	/* Record transaction start time under logging, progress or throttling */
-	if ((logfile || progress || throttle_delay || latency_limit) && st->state == 0)
+	if ((logfile || progress || throttle_delay || latency_limit || per_script_stats) && st->state == 0)
 	{
 		INSTR_TIME_SET_CURRENT(st->txn_begin);
 
@@ -1889,6 +1891,9 @@ doTxStats(TState *thread, CState *st, instr_time *now,
 
 	if (use_log)
 		doLog(thread, st, logfile, now, agg, skipped, latency, lag);
+
+	if (per_script_stats) /* mutex? hmmm... these are only statistics */
+		doStats(& sql_script[st->use_file].stats, skipped, latency, lag);
 }
 
 
@@ -2678,6 +2683,7 @@ addScript(const char *name, Command **commands)
 
 	sql_script[num_scripts].name = name;
 	sql_script[num_scripts].commands = commands;
+	initStats(& sql_script[num_scripts].stats, 0.0);
 	num_scripts++;
 }
 
@@ -2761,22 +2767,40 @@ printResults(TState *threads, StatsData *total, instr_time total_time,
 	printf("tps = %f (including connections establishing)\n", tps_include);
 	printf("tps = %f (excluding connections establishing)\n", tps_exclude);
 
-	/* Report per-command latencies */
-	if (is_latencies)
+	/* Report per-script stats */
+	if (per_script_stats)
 	{
 		int			i;
 
 		for (i = 0; i < num_scripts; i++)
 		{
-			Command   **commands;
+			printf("SQL script %d: %s\n"
+   " - "INT64_FORMAT" transactions (%.1f%% of total, tps = %f)\n",
+   i+1, sql_script[i].name,
+   sql_script[i].stats.cnt,
+   100.0 * sql_script[i].stats.cnt / total->cnt,
+   sql_script[i].stats.cnt / time_include);
 
-			printf("SQL script %d: %s\n", i + 1, sql_script[i].name);
-			printf(" - statement latencies in milliseconds:\n");
+			if (latency_limit)
+printf(" - number of transactions skipped: "INT64_FORMAT" (%.3f%%)\n",
+	   sql_script[i].stats.skipped,
+	   100.0 * sql_script[i].stats.skipped /
+	   (sql_script[i].stats.skipped + sql_script[i].stats.cnt));
 
-			for (commands = sql_script[i].commands; *commands != NULL; commands++)
-printf("   %11.3f  %s\n",
-   1000.0 * (*commands)->stats.sum / (*commands)->stats.count,
-	   (*commands)->line);
+			printSimpleStats(" - latency", & 

Re: [HACKERS] Mac OS: invalid byte sequence for encoding "UTF8"

[Shulgin, Oleksandr]

On Wed, Jan 27, 2016 at 10:59 AM, Artur Zakirov 
wrote:

> Hello.
>
> When a user try to create a text search dictionary for the russian
> language on Mac OS then called the following error message:
>
>   CREATE EXTENSION hunspell_ru_ru;
> + ERROR:  invalid byte sequence for encoding "UTF8": 0xd1
> + CONTEXT:  line 341 of configuration file
> "/Users/stas/code/postgrespro2/tmp_install/Users/stas/code/postgrespro2/install/share/tsearch_data/ru_ru.affix":
> "SFX Y   хаться шутсяхаться
>
> Russian dictionary was downloaded from
> http://extensions.openoffice.org/en/project/slovari-dlya-russkogo-yazyka-dictionaries-russian
> Affix and dictionary files was extracted from the archive and converted to
> UTF-8. Also a converted dictionary can be downloaded from
> https://github.com/select-artur/hunspell_dicts/tree/master/ru_ru


Not sure why the file uses "SET KOI8-R" directive then?

This behavior occurs on:
> - Mac OS X 10.10 Yosemite and Mac OS X 10.11 El Capitan.
> - latest PostgreSQL version from git and PostgreSQL 9.5 (probably also on
> 9.4.5).
>
> There is also the test to reproduce this bug in the attachment.
>

What error message do you get with this test program?  (I don't get any,
but I'm not on Mac OS.)

--
Alex

[HACKERS] Minor improvement to fdwhandler.sgml

[Etsuro Fujita]

Here is a small patch to do s/for/For/ to two section titles in
fdwhandlers.sgml, for consistency.

Best regards,
Etsuro Fujita
diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml
index dc2d890..9c8406c 100644
--- a/doc/src/sgml/fdwhandler.sgml
+++ b/doc/src/sgml/fdwhandler.sgml
@@ -798,7 +798,7 @@ RecheckForeignScan (ForeignScanState *node, TupleTableSlot *slot);

 

-FDW Routines for EXPLAIN
+FDW Routines For EXPLAIN
 
 
 
@@ -851,7 +851,7 @@ ExplainForeignModify (ModifyTableState *mtstate,

 

-FDW Routines for ANALYZE
+FDW Routines For ANALYZE
 
 
 

-- 
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: ResourceOwner optimization for tables with many partitions

[Aleksander Alekseev]

Hello, Tom.

I'm a bit concerned regarding assumption that sizeof int never exceeds 4
bytes. While this could be true today for most C compilers, standard
[1][2] doesn't guarantee that. Perhaps we should add something like:

StaticAssertStmt(sizeof(int) <= sizeof(int32),
"int size exceeds int32 size");

It costs nothing but could save a lot of time (not mentioning data
loss) some unlucky user.

[1] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1124.pdf
[2] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf


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

[Etsuro Fujita]

On 2016/01/27 12:20, Etsuro Fujita wrote:

On 2016/01/26 22:57, Rushabh Lathia wrote:

On Tue, Jan 26, 2016 at 4:15 PM, Etsuro Fujita
> wrote:

On 2016/01/25 17:03, Rushabh Lathia wrote:



int
IsForeignRelUpdatable (Relation rel);



Documentation for IsForeignUpdatable() need to change as it says:

If the IsForeignRelUpdatable pointer is set to NULL, foreign
tables are
assumed
to be insertable, updatable, or deletable if the FDW provides
ExecForeignInsert,
ExecForeignUpdate or ExecForeignDelete respectively.

With introduce of DMLPushdown API now this is no more correct,
as even if
FDW don't provide ExecForeignInsert, ExecForeignUpdate or
ExecForeignDelete API
still foreign tables are assumed to be updatable or deletable
with
DMLPushdown
API's, right ?



That's what I'd like to discuss.

I intentionally leave that as-is, because I think we should
determine the updatability of a foreign table in the current
manner.  As you pointed out, even if the FDW doesn't provide eg,
ExecForeignUpdate, an UPDATE on a foreign table could be done using
the DML pushdown APIs if the UPDATE is *pushdown-safe*.  However,
since all UPDATEs on the foreign table are not necessarily
pushdown-safe, I'm not sure it's a good idea to assume the
table-level updatability if the FDW provides the DML pushdown
callback routines.  To keep the existing updatability decision, I
think the FDW should provide the DML pushdown callback routines
together with ExecForeignInsert, ExecForeignUpdate, or
ExecForeignDelete.  What do you think about that?



Sorry but I am not in favour of adding compulsion that FDW should provide
the DML pushdown callback routines together with existing
ExecForeignInsert,
ExecForeignUpdate or ExecForeignDelete APIs.

May be we should change the documentation in such way, that explains

a) If FDW PlanDMLPushdown is NULL, then check for ExecForeignInsert,
ExecForeignUpdate or ExecForeignDelete APIs
b) If FDW PlanDMLPushdown is non-NULL and plan is not pushable
check for ExecForeignInsert, ExecForeignUpdate or ExecForeignDelete APIs
c) If FDW PlanDMLPushdown is non-NULL and plan is pushable
check for DMLPushdown APIs.

Does this sounds wired ?



Yeah, but I think that that would be what is done during executor
startup (see CheckValidResultRel()), while what the documentation is
saying is about relation_is_updatable(); that is, how to decide the
updatability of a given foreign table, not how the executor processes an
individual INSERT/UPDATE/DELETE on a updatable foreign table.  So, I'm
not sure it's a good idea to modify the documentation in such a way.



However, I agree that we should add a documentation note about the
compulsion somewhere.  Maybe something like this:

The FDW should provide DML pushdown callback routines together with
table-updating callback routines described above.  Even if the callback
routines are provided, the updatability of a foreign table is determined
based on the presence of ExecForeignInsert, ExecForeignUpdate or
ExecForeignDelete if the IsForeignRelUpdatable pointer is set to NULL.


On second thought, I think it might be okay to assume the presence of 
PlanDMLPushdown, BeginDMLPushdown, IterateDMLPushdown, and 
EndDMLPushdown is also sufficient for the insertablity, updatability, 
and deletability of a foreign table, if the IsForeignRelUpdatable 
pointer is set to NULL.  How about modifying the documentation like this:


If the IsForeignRelUpdatable pointer is set to NULL, foreign tables are 
assumed to be insertable, updatable, or deletable if the FDW provides 
ExecForeignInsert, ExecForeignUpdate, or ExecForeignDelete respectively, 
or if the FDW provides PlanDMLPushdown, BeginDMLPushdown, 
IterateDMLPushdown, and EndDMLPushdown described below.


Of course, we also need to modify relation_is_updatable() accordingly.

What's your opinion?

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] pgbench stats per script & other stuff

[Fabien COELHO]

Hello again,


If you want to implement real non-ambiguous-prefix code (i.e. have "se"
for "select-only", but reject "s" as ambiguous) be my guest.


I'm fine with filtering out ambiguous cases (i.e. just the "s" case). 
Attached a small patch for that.


--
Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 42d0667..124e70d 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -269,6 +269,9 @@ pgbench  options  dbname
 Add the specified builtin script to the list of executed scripts.
 Available builtin scripts are: tpcb-like,
 simple-update and select-only.
+The provided scriptname needs only be an unambiguous
+prefix of the builtin name, hence si would be enough to
+select simple-update.
 With special name list, show the list of builtin scripts
 and exit immediately.

diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index d5f242c..6350948 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -2649,22 +2649,32 @@ listAvailableScripts(void)
 	fprintf(stderr, "\n");
 }
 
+/* return commands for selected builtin script, if unambiguous */
 static char *
 findBuiltin(const char *name, char **desc)
 {
-	int			i;
+	int			i, found = 0, len = strlen(name);
+	char	   *commands = NULL;
 
 	for (i = 0; i < N_BUILTIN; i++)
 	{
-		if (strncmp(builtin_script[i].name, name,
-	strlen(builtin_script[i].name)) == 0)
+		if (strncmp(builtin_script[i].name, name, len) == 0)
 		{
 			*desc = builtin_script[i].desc;
-			return builtin_script[i].commands;
+			commands = builtin_script[i].commands;
+			found++;
 		}
 	}
 
-	fprintf(stderr, "no builtin script found for name \"%s\"\n", name);
+	if (found == 1)
+		return commands;
+
+	/* error cases */
+	if (found == 0)
+		fprintf(stderr, "no builtin script found for name \"%s\"\n", name);
+	else /* found > 1 */
+		fprintf(stderr,
+"%d builtin scripts found for prefix \"%s\"\n", found, name);
 	listAvailableScripts();
 	exit(1);
 }

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

[HACKERS] Mac OS: invalid byte sequence for encoding "UTF8"

[Artur Zakirov]

Hello.

When a user try to create a text search dictionary for the russian 
language on Mac OS then called the following error message:


  CREATE EXTENSION hunspell_ru_ru;
+ ERROR:  invalid byte sequence for encoding "UTF8": 0xd1
+ CONTEXT:  line 341 of configuration file 
"/Users/stas/code/postgrespro2/tmp_install/Users/stas/code/postgrespro2/install/share/tsearch_data/ru_ru.affix": 
"SFX Y   хаться шутсяхаться


Russian dictionary was downloaded from 
http://extensions.openoffice.org/en/project/slovari-dlya-russkogo-yazyka-dictionaries-russian
Affix and dictionary files was extracted from the archive and converted 
to UTF-8. Also a converted dictionary can be downloaded from 
https://github.com/select-artur/hunspell_dicts/tree/master/ru_ru


This behavior occurs on:
- Mac OS X 10.10 Yosemite and Mac OS X 10.11 El Capitan.
- latest PostgreSQL version from git and PostgreSQL 9.5 (probably also 
on 9.4.5).


There is also the test to reproduce this bug in the attachment.

Did you meet this bug? Do you have a solution or a workaround?

Thanks in advance.

--
Artur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
#include 
#include 

char *src = "SFX Y   хаться шутсяхаться";

int
main(int argc, char *argv[])
{
	char c1[1024], c2[1024], c3[1024], c4[1024], c5[1024];

	setlocale(LC_CTYPE, "ru_RU.UTF-8");

	sscanf(src, "%6s %204s %204s %204s %204s", c1, c2, c3, c4, c5);

	printf("%s/%s/%s/%s/%s\n", c1, c2, c3, c4, c5);

	return 0;
}

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