Re: [HACKERS] OS X El Capitan and DYLD_LIBRARY_PATH

[Andres Freund]

On 2015-11-05 12:51:48 +0900, Michael Paquier wrote:
> On Thu, Nov 5, 2015 at 12:17 PM, Peter Eisentraut  wrote:
> > On 11/3/15 6:36 AM, Andres Freund wrote:
> >> I wonder if we could fix this by using install_name_tool during the
> >> tempinstall to add an appropriate rpath.
> >>
> >> Alternatively we could, apparently, specify a relative path to libraries
> >> as explained here:
> >> http://qin.laya.com/tech_coding_help/dylib_linking.html
> >>  % install_name_tool -change libbz2.1.0.2.dylib  
> >> @executable_path/../Frameworks/libbz2.1.0.2.dylib MyFunBinary
> >>
> >> which ought to work independently from the tempinstall and normal
> >> installation path.
> >
> > That might be worth a try.

It'll probably not me though, given that I scream bloody murder
everytime I've to use OSX...

> > I ended up disabling system integrity
> > protection, which also fixed a few other strange behaviors (mysterious
> > regression test failures in ecpg, for instance, if anyone stumbles
> > across that).
> 
> Yeah, that's wiser IMO.

That's probably true in the short term. I doubt it'll be viable in the
long run. Both because apple obviously doesn't care one iota about the
developer experience around this, and thus probably has a limited
interest keeping related procedures working the same, and because once
more people use El Captan it'll have to be done by more and more
people.

Andres


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

Re: [HACKERS] psql completion for ids in multibyte string

[Amit Langote]

Horiguchi-san,

On 2015/11/05 18:10, Kyotaro HORIGUCHI wrote:
> Hello. I don't know whether this is a bug fix or improvement,

Would it be 50-50? :-)

...

> 
> During the investigation into this issue, I found a mistake in
> the comment for PQmblen. It give the byte length of the character
> at the point, not word. The attached patche also fixes it.
> 
>> /*
>>  * returns the byte length of the word beginning s, using the
>>  * specified encoding.
>>  */
>> int
>> PQmblen(const char *s, int encoding)
> 

In the following change,

- * returns the byte length of the word beginning s, using the
- * specified encoding.
+ * returns the byte length of the character beginning s, using the specified
+ * encoding.


Just a minor nitpick -

... character beginning *at* s ...?

If so, there would be one more instance to fix.

Thanks,
Amit



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

Re: [HACKERS] September 2015 Commitfest

[Torsten Zühlsdorff]

Hello,


+1.  FWIW, I'm willing to review some patches for this CommitFest, but
if the committers have to do first-round review as well as
committer-review of every patch in the CommitFest, this is going to be
long, ugly, and painful.  We need to have a substantial pool of
non-committers involved in the review process so that at least some of
the work gets spread out.


As a non-committer, let me offer my thoughts.

First, I would ask that every patch include a commit id that the patch
was generated against.  For example, I was looking at the "group command
option for psql" patch.  I created a branch off of master, but the patch
doesn't apply cleanly.  On further investigation, it looks like Adam
Brightwell noted this on September 21, but the patch hasn't been
updated.  If I knew which commit id the patch was created against, I
could create a branch from there and test the patch, but without, I need
to figure that out which is more work, or I need to re-create the patch,
which is also more work.


+ 1


Second, it would be convenient if there were a make target that would
set up a test environment.  Effectively do what the 'make check' does,
but don't run the tests and leave the database up.  It should probably
drop you into a shell that has the paths set up as well.  Another
target should be available to shut it down.  So, what would be cool,
and make testing easier is if I could do a 'git checkout -b feature
abcdef' (or whatever the commit id is and branch name you want to use)
Then from there a

make
make check
make testrig

make testshutdown

These two would go a long way to making the process of actually
doing a review easier.


From my point of view it is very hard to review a patch without having 
much time. My C knowledge is very very basic. I read many patches just 
to get better with this, but as you (not) noticed, there is rarely 
feedback from me.


Often it is unclear what to test. The threads about the features are 
very long and mostly very technical. While interesting for a good 
C-programmer this is not helpful for an end user. When there is a patch 
i am missing:
- a short description how to set up an test installation (just a link to 
the wiki would be very helpful)

- what is the patch proposed to do
- what should it not do
- how can it be tested
- how can the updated documentation - if existent - generated and used

Often terms, configuration options or syntax changes between the 
patches. This is needed and very good - but currently requires me to 
read the complete thread, which has many many information i do not 
understand because of the missing inside.


I believe that we need to lower the barrier for testing. This would 
require another step of work. But this step is not necessarily done by 
the author itself. I am under the impression that there are many readers 
at the mailing list willing but unable to participate. This could be a 
"grunt-work" task like in this discussion about the bugtracker.


I could even imagine to set up an open for everyone test-instance of 
HEAD where users are allowed to test like the wanted. Than the barrier 
is reduced to "connect to PostgreSQL and execute SQL".


Greetings,
Torsten


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

[HACKERS] psql completion for ids in multibyte string

[Kyotaro HORIGUCHI]

Hello. I don't know whether this is a bug fix or improvement,
anyway show you a patch for psql completion.


psql completes identifiers in many places but donesn't for
multibyte identifiers.

=> ALTER TABLE "[tab]
"いろは" "with space"

=> ALTER TABLE "い[tab]


Currently psql counts the length of the string to complete in
bytes but it is desirable to count in client encoding. The
attached patch does so and the uncompleted completion would
be completed.

=> ALTER TABLE "い[tab]
=> ALTER TABLE "いろは" _

During the investigation into this issue, I found a mistake in
the comment for PQmblen. It give the byte length of the character
at the point, not word. The attached patche also fixes it.

> /*
>  * returns the byte length of the word beginning s, using the
>  * specified encoding.
>  */
> int
> PQmblen(const char *s, int encoding)


What do you think about this?

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
>From 7311e03b4656b724754be4697e59291844901fb8 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi 
Date: Mon, 2 Nov 2015 12:46:45 +0900
Subject: [PATCH] Fix identifier completion of multibyte characters.

_copletion_from_query took the byte length of the given text as its
character length. This should be counted in the environmental
encoding.

This patch also fixes a comment for PQmblen.
---
 src/bin/psql/tab-complete.c| 13 ++---
 src/interfaces/libpq/fe-misc.c |  4 ++--
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 0e8d395..e6e22c4 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -4283,8 +4283,8 @@ complete_from_schema_query(const char *text, int state)
 static char *
 _complete_from_query(int is_schema_query, const char *text, int state)
 {
-	static int	list_index,
-string_length;
+	static int	list_index;
+	int 		string_length = 0;
 	static PGresult *result = NULL;
 
 	/*
@@ -4297,9 +4297,16 @@ _complete_from_query(int is_schema_query, const char *text, int state)
 		char	   *e_text;
 		char	   *e_info_charp;
 		char	   *e_info_charp2;
+		const char *pstr = text;
 
 		list_index = 0;
-		string_length = strlen(text);
+
+		/* count length as a multibyte text */
+		while (*pstr)
+		{
+			string_length++;
+			pstr += PQmblen(pstr, pset.encoding);
+		}
 
 		/* Free any prior result */
 		PQclear(result);
diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 0dbcf73..9eb09e3 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -1179,8 +1179,8 @@ pqSocketPoll(int sock, int forRead, int forWrite, time_t end_time)
  */
 
 /*
- * returns the byte length of the word beginning s, using the
- * specified encoding.
+ * returns the byte length of the character beginning s, using the specified
+ * encoding.
  */
 int
 PQmblen(const char *s, int encoding)
-- 
1.8.3.1


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

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

[Kyotaro HORIGUCHI]

Hello, I had a look on v21 patch.

Though I haven't looked the whole of the patch, I'd like to show
you some comments only for visibilitymap.c and a part of the
documentation.


1. Patch application

   patch command complains about offsets for heapam.c at current
   master.

2. visitibilymap_test()

-  if (visibilitymap_test(rel, blkno, ))
+  if (visibilitymap_test(rel, blkno, , VISIBILITYMAP_ALL_VISIBLE)

 The old VM was a simple bitmap so the name _test and the
 function are proper but now the bitmap is quad state so it'd be
 better chainging the function. Alghough it is not so expensive
 to call it twice successively, it is a bit uneasy for me doing
 so. One possible shape would be like the following.

 lazy_vacuum_page()
 > int vmstate = visibilitymap_get_status(rel, blkno, );
 > if (!(vmstate  & VISIBILITYMAP_ALL_VISIBLE))
 >   ...
 > if (all_frozen && !(vmstate  & VISIBILITYMAP_ALL_FROZEN))
 >   ...
 > if (flags != vmstate)
 >   visibilitymap_set(, flags);

 and defining two macros for indivisual tests,

 > #define VM_ALL_VISIBLE(r, b, v) ((vm_get_status((r), (b), (v)) & .._VISIBLE) 
 > != 0)
 > if (VM_ALL_VISIBLE(rel, blkno, ))
 and
 > if (VM_ALL_FROZEN(rel, blkno, ))

 How about this?


3. visibilitymap.c

 - HEAPBLK_TO_MAPBIT

  In visibilitymap_clear and other functions, mapBit means
  mapDualBit in the patch, and mapBit always appears in the form
  "mapBit * BITS_PER_HEAPBLOCK". So, it'd be better to change the
  definition of HEAPBLK_TO_MAPBIT so that it calculates really
  the bit position in a byte.

  - #define HEAPBLK_TO_MAPBIT(x) ((x) % HEAPBLOCKS_PER_BYTE)
  + #define HEAPBLK_TO_MAPBIT(x) (((x) % HEAPBLOCKS_PER_BYTE) * 
BITS_PER_HEAPBLOCK)


 - visibilitymap_count()

  The third argument all_frozen is not necessary in some
  usage. So this interface would be preferable to be as
  following,

  BlockNumber
  visibilitymap_count(Relation rel, BlockNumber *all_frozen)
  {
 BlockNumber all_visible = 0;
  ...
 if (all_frozen)
   *all_frozen = 0;
  ... something like ...

  - visibilitymap_set()

   The check for ALL_VISIBLE is duplicate in the following
   assertion.

   > Assert((flags & VISIBILITYMAP_ALL_VISIBLE) ||
   >   (flags & (VISIBILITYMAP_ALL_VISIBLE | VISIBILITYMAP_ALL_FROZEN)));



4. documentation

  - 18.11.1 Statement Hehavior

A typo.

> VACUUM performs *a* aggressive freezing

However I am not a fluent English speaker, and such
wordsmithing would be done by someone else, I feel that
"eager/greedy" is more suitable for this meaning..,
nevertheless, the term "whole-table freezing" that you wrote
elsewhere in this patch would be usable.

"VACUUM performs a whole-table freezing"

All "a table scan/sweep"s and something has the similar
meaning would be better be changed to "a whole-table
freezing"

In similar manner, "tuples/rows that are marked as frozen"
could be replaced with "unfrozen tuples/rows".

  - 23.1.5 Preventing Transaction ID Wraparound Failures

"The whole table is scanned only when all pages happen to
 require vacuuming to remove dead row versions."

This description looks a bit out-of-point. "the whole table
scan" in the original description is what is triggered by
relfrozenxid so the correspondent in the revised description
is "the whole-table freezing", maybe.

"The whole-table feezing takes place when
 relfrozenxid is more than
 vacuum_freeze_table_age transactions old or when
 VACUUM's FREEZE option is used. The
 whole-table freezing scans all unfreezed pages."

The last sentence might be unnecessary.


 - 63.4 Visibility Map

   "pages contain only tuples that are marked as frozen" would be
enough to be "pages contain only frozen tuples"

and according to the discussion upthread, we might be good to
have some desciption that the name is historically omitting
the aspect of freezemap.


At Sat, 31 Oct 2015 18:07:32 +0530, Amit Kapila  wrote 
in 
amit.kapila16> On Fri, Oct 30, 2015 at 6:03 AM, Masahiko Sawada 

> Couple of more review comments:
> --
> 
> 1.
> @@ -615,6 +617,8 @@ typedef struct PgStat_StatTabEntry
>   PgStat_Counter n_dead_tuples;
>   PgStat_Counter
> changes_since_analyze;
> 
> + int32 n_frozen_pages;
> +
>   PgStat_Counter blocks_fetched;
>   PgStat_Counter
> blocks_hit;
> 
> As you are changing above structure, you need to update
> PGSTAT_FILE_FORMAT_ID, refer below code:
> #define PGSTAT_FILE_FORMAT_ID 0x01A5BC9D
> 
> 2. It seems that n_frozen_page is not initialized/updated properly
> for toast tables:
> 
> Try with below steps:
> 
> postgres=# create table t4(c1 int, c2 text);
> CREATE TABLE
> postgres=# select oid, relname from pg_class where relname like '%t4%';
>   oid  | relname
> ---+-
>  16390 | t4
> (1 row)
> 
> 
> 

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

[David Steele]

On 11/4/15 4:55 PM, Stephen Frost wrote:
> * Joe Conway (m...@joeconway.com) wrote:
>> On 11/04/2015 01:24 PM, Alvaro Herrera wrote:
>>> I agree with Pavel.  Having a transaction timeout just does not make any
>>> sense.  I can see absolutely no use for it.  An idle-in-transaction
>>> timeout, on the other hand, is very useful.
>>
>> +1 -- agreed
>
> I'm not sure of that.  I can certainly see a use for transaction
> timeouts- after all, they hold locks and can be very disruptive in the
> long run.  Further, there are cases where a transaction is normally very
> fast and in a corner case it becomes extremely slow and disruptive to
> the rest of the system.  In those cases, having a timeout for it is
> valuable.
>
> David (adding him to the CC) actually developed a utility specifically
> to identify what transactions are blocking what others and to kill off
> other processes if they were running for too long and blocking higher
> priority processes.  It didn't matter, in that environment, if they were
> idle-in-transaction or actively running.

You are remembering correctly, Stephen, though there were different
timeouts for blocking transactions that were running and those that were
idle-in-transaction.  We usually set the idle-in-transaction timeout
much lower as it measured not total transaction time but idle time since
the last state change.  In that environment, at least, an
idle-in-transaction session was always due to a stuck process, bug, or
user session left open overnight.  Because partitions and FKs were
continuously being created even ACCESS SHARE locks could be a problem.

The important thing about this implementation was that nothing was
terminated unless it had exceed a timeout AND was blocking another
process.  A feature of this particular system was that it had very long
running transactions that needed to execute unless there was a conflict.

Even then, we'd get an alert some time in advance of the transaction
being terminated so we could make the judgement call to terminate the
other process(es) instead.

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



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] September 2015 Commitfest

[Craig Ringer]

On 5 November 2015 at 15:59, Torsten Zühlsdorff
 wrote:
> Hello,
>
>>> +1.  FWIW, I'm willing to review some patches for this CommitFest, but
>>> if the committers have to do first-round review as well as
>>> committer-review of every patch in the CommitFest, this is going to be
>>> long, ugly, and painful.  We need to have a substantial pool of
>>> non-committers involved in the review process so that at least some of
>>> the work gets spread out.
>>
>>
>> As a non-committer, let me offer my thoughts.
>>
>> First, I would ask that every patch include a commit id that the patch
>> was generated against.
>
> + 1

Including a git ref in the commitfest entry helps a lot with that.

So does using "git format-patch" when generating patches, now that
everyone seems to have pretty much stopped caring about context diffs.

>> Second, it would be convenient if there were a make target that would
>> set up a test environment.

Yes, a "make check NOSTOP=1" would be handy.

That said, it's not hard to "make temp-install" then "PGPORT=whatever
make installcheck"

> From my point of view it is very hard to review a patch without having much
> time.

That's the eternal problem with patch review. I know the feeling.

> My C knowledge is very very basic. I read many patches just to get
> better with this, but as you (not) noticed, there is rarely feedback from
> me.

Reading C is useful for learning, but learned massively faster when I
started writing extensions, etc. Then my reading tended to be more
directed too, and with better retention.

> Often it is unclear what to test. The threads about the features are very
> long and mostly very technical. While interesting for a good C-programmer
> this is not helpful for an end user. When there is a patch i am missing:
> - a short description how to set up an test installation (just a link to the
> wiki would be very helpful)

Wiki links for patches would be a huge plus. The CF app "Links"
section can be used for that, and should be more than it is.

Peter Geoghegan did this with the insert ... on commit update patch,
to useful effect.

I try to include enough documentation directly in the patch - either
in the commit message(s) from git format-patch, or in in-patch README
text etc - for this not to be necessary. But for bigger patches it's
hard to avoid, and I'd love it if more people did it.

> Often terms, configuration options or syntax changes between the patches.
> This is needed and very good - but currently requires me to read the
> complete thread, which has many many information i do not understand because
> of the missing inside.

Especially when threads reference things that were discussed months or
years prior!

> I believe that we need to lower the barrier for testing.

While I agree, I'd also like to note that formulaic testing is often
of limited utility. Good testing still requires a significant
investment of time and effort to understand the changes made by a
patch, which areas need focused attention, think about corner cases,
etc.

"make check passes" doesn't really tell anyone that much.

> I could even imagine to set up an open for everyone test-instance of HEAD
> where users are allowed to test like the wanted. Than the barrier is reduced
> to "connect to PostgreSQL and execute SQL".

Gee, that'd be fun to host ;)

More seriously, it's not HEAD that's of that much interest, it's HEAD
+ [some patch or set of patches].

There are systems that can pull in patchsets, build a project, and run
it. But for something like PostgreSQL it'd be pretty hard to offer
wide public access, given the trivial potential for abuse.

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


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

Re: [HACKERS] Foreign join pushdown vs EvalPlanQual

[Etsuro Fujita]

On 2015/11/04 18:50, Etsuro Fujita wrote:

On 2015/11/04 17:10, Kouhei Kaigai wrote:

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

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

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

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



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



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



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



It looks to me, we should not use ctid system column to identify remote
row when postgres_fdw tries to support late row locking.



The "rowid" should not be changed once it is fetched from the remote side
until it is actually updated, deleted or locked, for correct
identification.
If ctid is used for this purpose, it is safe only when remote row is
locked
when it is fetched - it is exactly early row locking behavior, isn't it?



In case of SELECT FOR UPDATE, I think we are allowed to use ctid to
identify target rows for late row locking, but I think the above SQL
should be changed to something like this:

SELECT * FROM (SELECT * FROM ft1 WHERE ft1.tid = $0 FOR UPDATE) ss1 LEFT
JOIN (SELECT * FROM ft2 WHERE ft2.tid = $1) ss2 ON ss1.x = ss2.x


I noticed that the modofied SQL was still wrong; ss1 would produce no 
tuple, if using eg, a sequential scan for ss1, as discussed above. 
Sheesh, where is my brain?


I still think we are allowed to do that, but what is the right SQL for 
that?  In the current implementation of postgres_fdw, we need not take 
into consideration that what was fetched was an updated version of the 
tuple rather than the same version previously obtained, since that 
always uses at least REPEATABLE READ in the remote session.  But 
otherwise it would be possible that what was fetched was an updated 
version of the tuple, having a different ctid value, which wouldn't 
satisfy the condition like "ft1.tid = $0" in ss1 any more.


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] Note about comparation PL/SQL packages and our schema/extensions

[Craig Ringer]

On 5 November 2015 at 14:36, Pavel Stehule  wrote:

> 1. The encapsulation and local scope - all objects in schema are accessible
> from other objects in schema  by default (can be rewritten by explicit
> granting). Local objects are visible only from objects in schema. This needs
> enhancing of our search_path mechanism.

Yep. It's as if, within function packagename.funcname, packagename is
implicitly prefixed to search_path .

I can see that being handy, but not especially important.

> 2. The schema variables - a server side session (can be emulated now) and
> server side local schema session variables (doesn't exist) is pretty useful
> for storing some temp data or high frequent change data - and can
> significantly increase speed of some use cases. Now we emulate it via PLPerl
> shared array, but the encapsulation is missing.

This is the feature I feel we could really use.

I see *lots* of people emulating session variables by (ab)using custom
GUCs. The missing-ok variant of current_setting helps with this to the
point where it's fairly OK now.

The main advantage package variables have - IMO - are package
permissions. You can define a variable that is writeable only by
functions within a package. That's really handy for things like row
security since it lets you have variables you can only set via a
function that can do things like refuse to run again with different
args, validate input, etc. So you can do expensive work once, then
cheap row security checks against the preset variable. Or use it for
things like "current customer" settings when using pooled connections.

It might make sense to extend custom GUCs for this rather than invent
a new mechanism, since GUCs have lots of useful properties like
global, db, user, session and transaction scoping, etc. I'm not really
sure... I just agree that it's a good idea to be able to have
something with similar capabilities to package variables. Especially
security properties.

> 3. The initialization routines - the routines called when any object from
> schema is used first time.

... which is somewhat similar to having an "on session start" trigger.
Also an oft-wanted feature.

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


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

Re: [HACKERS] September 2015 Commitfest

[Torsten Zuehlsdorff]

On 05.11.2015 13:49, Craig Ringer wrote:


I believe that we need to lower the barrier for testing.


While I agree, I'd also like to note that formulaic testing is often
of limited utility. Good testing still requires a significant
investment of time and effort to understand the changes made by a
patch, which areas need focused attention, think about corner cases,
etc.


Yes, you are right. But a limited test is better than no test at all. 
But of course not enough.


For me it is easy to check comments or sql commands, but not the c code. 
But with lower barriers it would be easier to test 2 of the 3 mentioned 
items. At the moment its often none, because its hard.



"make check passes" doesn't really tell anyone that much.


I could even imagine to set up an open for everyone test-instance of HEAD
where users are allowed to test like the wanted. Than the barrier is reduced
to "connect to PostgreSQL and execute SQL".


Gee, that'd be fun to host ;)

>

More seriously, it's not HEAD that's of that much interest, it's HEAD
+ [some patch or set of patches].

There are systems that can pull in patchsets, build a project, and run
it. But for something like PostgreSQL it'd be pretty hard to offer
wide public access, given the trivial potential for abuse.


Yes, but i would do this. Creating a FreeBSD Jail which is reset 
regularly is no great deal and very secure. My bigger problem is the 
lack of IPv4 addresses. At the moment i am limited to IPv6 only hosts.


Greetings,
Torsten


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

Here is a v13 and a small test script.

 - change names to random(), random_exponential() and random_gaussian()
   I find them too long, but if the committer want that I cannot help
   it:-)

 - more comments, especially about the expression evaluation &
   type system.

 - improved documentation, in particular to include suggestions by Tomas
   Vondra about clarifying explanations about the gaussian &
   exponential random generators, and clear references from \setrandom
   to the \set expressions.

 - still just one patch, because removing double would mean removing the 2
   exponential & gaussian random functions which require a double
   argument.

   Note that I started with one small patch for adding the infrastructure,
   but then Heikki requested more functions including double type stuff to
   illustrate the point, then Robert asks to break it back, going forward
   and backward is tiring...

 - still "lousy" *debug functions, because I found them useful for
   debugging and testing, really. It is easy to remove them, but I would
   advise against doing that as it would make debugging an expression
   much less straightforward.

--
Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 0ac40f1..38d0994 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -771,24 +771,35 @@ 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.14156,
+  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.
+ 
+
+ 
+  Typing between integer and double is implicit and descendant: the type of
+  an operator or function depends on the expected type of the result.
+  For instance, if an integer is expected, exp1 + exp2 will cast
+  both operands to int and use the integer addition.
  
 
  
   Examples:
 
 \set ntellers 10 * :scale
-\set aid (1021 * :aid) % (10 * :scale) + 1
+\set aid (1021 * rand(1, 10 * :scale)) % (10 * :scale) + 1
 
 

 

 
- \setrandom varname min max [ uniform | { gaussian | exponential } threshold ]
+ \setrandom varname min max [ uniform | { gaussian | exponential } param ]
  
 
 
@@ -801,57 +812,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 threshold 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 -threshold on the left and +threshold
-  on the right.
-  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, then value i
-  between min and max inclusive is drawn
-  with probability:
-  
-(PHI(2.0 * threshold * (i - min - mu + 0.5) / (max - min + 1)) -
- PHI(2.0 * threshold * (i - min - mu - 0.5) / (max - min + 1))) /
- (2.0 * PHI(threshold) - 1.0).
-  Intuitively, the larger the threshold, 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 / threshold
-  and 95% in the middle 2.0 / threshold; for instance, if
-  threshold is 4.0, 67% of values are drawn from the middle
-  quarter and 95% from the middle half of the interval.
-  The minimum threshold 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, the threshold
-  parameter controls the distribution by truncating a quickly-decreasing
-  exponential distribution at threshold, and then
-  projecting onto integers between the bounds.
-  To be precise, value i between min and
-  max inclusive is drawn with 

Re: [HACKERS] [PATCH] Skip ALTER x SET SCHEMA if the schema didn't change

[Haribabu Kommi]

On Tue, Sep 29, 2015 at 12:17 AM, Marti Raudsepp  wrote:
> Hi list
>
> The attached patch changes the behavior of multiple ALTER x SET SCHEMA
> commands, to skip, rather than fail, when the old and new schema is
> the same.
>
> The advantage is that it's now easier to write DDL scripts that are 
> indempotent.
>
> This already matches the behavior of ALTER EXTENSION SET SCHEMA in
> earlier versions, as well as many other SET-ish commands, e.g. ALTER
> TABLE SET TABLESPACE, OWNER TO, CLUSTER ON, SET (storage_parameter...)
> etc. I don't see why SET SCHEMA should be treated any differently.
>
> The code is written such that object_access_hook is still called for
> each object.
>
> Regression tests included. I couldn't find any documentation that
> needs changing.

I went through the patch, following are my observations,

Patch applied with hunks and compiled with out warnings.
Basic tests are passed.

In AlterTableNamespaceInternal function, if a table or matview called
for set schema,
If the object contains any constraints, the constraint gets updated
with new schema.

In AlterTypeNamespaceInternal function, the InvokeObjectPostAlterHook function
doesn't get called if the type is of composite type, domain and array
types as because
it just returns from top of the function.


Regards,
Hari Babu
Fujitsu Australia


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

Re: [HACKERS] extend pgbench expressions with functions

[Fabien COELHO]

Hello again,

The v14 also remove references to the "threshold" word about gaussian and 
exponential random generation in the source code (comments and variable 
names), as it has no clear meaning, to replace it with param/parameter 
depending on the context, as discussed in another thread started by Tomas 
Vondra.


Sorry for the noise.


Here is a v13 and a small test script.

- change names to random(), random_exponential() and random_gaussian()
  I find them too long, but if the committer want that I cannot help
  it:-)

- more comments, especially about the expression evaluation &
  type system.

- improved documentation, in particular to include suggestions by Tomas
  Vondra about clarifying explanations about the gaussian &
  exponential random generators, and clear references from \setrandom
  to the \set expressions.

- still just one patch, because removing double would mean removing the 2
  exponential & gaussian random functions which require a double
  argument.

  Note that I started with one small patch for adding the infrastructure,
  but then Heikki requested more functions including double type stuff to
  illustrate the point, then Robert asks to break it back, going forward
  and backward is tiring...

- still "lousy" *debug functions, because I found them useful for
  debugging and testing, really. It is easy to remove them, but I would
  advise against doing that as it would make debugging an expression
  much less straightforward.


--
Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 0ac40f1..38d0994 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -771,24 +771,35 @@ 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.14156,
+  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.
+ 
+
+ 
+  Typing between integer and double is implicit and descendant: the type of
+  an operator or function depends on the expected type of the result.
+  For instance, if an integer is expected, exp1 + exp2 will cast
+  both operands to int and use the integer addition.
  
 
  
   Examples:
 
 \set ntellers 10 * :scale
-\set aid (1021 * :aid) % (10 * :scale) + 1
+\set aid (1021 * rand(1, 10 * :scale)) % (10 * :scale) + 1
 
 

 

 
- \setrandom varname min max [ uniform | { gaussian | exponential } threshold ]
+ \setrandom varname min max [ uniform | { gaussian | exponential } param ]
  
 
 
@@ -801,57 +812,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 threshold 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 -threshold on the left and +threshold
-  on the right.
-  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, then value i
-  between min and max inclusive is drawn
-  with probability:
-  
-(PHI(2.0 * threshold * (i - min - mu + 0.5) / (max - min + 1)) -
- PHI(2.0 * threshold * (i - min - mu - 0.5) / (max - min + 1))) /
- (2.0 * PHI(threshold) - 1.0).
-  Intuitively, the larger the threshold, 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 / threshold
-  and 95% in the middle 2.0 / threshold; for instance, if
-  threshold is 4.0, 67% of values are drawn from the middle
-  quarter and 95% from the middle half of the interval.
-  The minimum threshold 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 

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

[Robert Haas]

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

OK, committed.

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


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

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

[Robert Haas]

On Wed, Nov 4, 2015 at 9:15 PM, Tomas Vondra
 wrote:
> I certainly understand there are cases that require care - like the
> leakproof thing pointed out by Robert for example. I don't immediately see
> why evaluation against dead rows would be a problem.

Well, one thing is that you might leak information about
already-deleted rows, which could be a security vulnerability, or more
mundanely cause a function to error out when there are no actually
visible rows that could trigger such an error.  It would be surprising
if you could add a CHECK(b != 0) constraint to a table, query it for
rows where a/b>1, and get a division by zero error.

But I think it's even worse: I believe there can be dead rows in the
heap whose tuple descriptor doesn't even match the current
pg_attribute contents.  Consider BEGIN; ALTER TABLE ... ADD COLUMN ...
int8; INSERT ...; ROLLBACK; ALTER TABLE .. ADD COLUMN .. text; SELECT
...  If the SELECT tries to decode one of the tuples added by the
failed transaction considering the new column as text when the dead
tuples in question had it as an int8, I suspect that you can crash the
server.  Nothing good will happen, anyway.

-- 
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] FORCE ROW LEVEL SECURITY

[Robert Haas]

On Wed, Nov 4, 2015 at 2:39 PM, Stephen Frost  wrote:
> In this case, you ran it as superuser which automatically has the
> 'BYPASSRLS' privilege, which means that RLS is bypassed always.
>
> The change to how BYPASSRLS works was discussed with and ultimately
> implemented by Noah, as I recall.

Hmm, OK.  I guess I missed the reasoning behind that decision, but
maybe it's not important.

-- 
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] Note about comparation PL/SQL packages and our schema/extensions

[José Luis Tallón]

On 11/05/2015 01:31 PM, Craig Ringer wrote:

On 5 November 2015 at 14:36, Pavel Stehule  wrote:
[snip]


2. The schema variables - a server side session (can be emulated now) and
server side local schema session variables (doesn't exist) is pretty useful
for storing some temp data or high frequent change data - and can
significantly increase speed of some use cases. Now we emulate it via PLPerl
shared array, but the encapsulation is missing.

This is the feature I feel we could really use.

I see *lots* of people emulating session variables by (ab)using custom
GUCs. The missing-ok variant of current_setting helps with this to the
point where it's fairly OK now.


AFAICS, (ab)using  custom GUCs is the "blessed" (by Tom, no less) way to 
do it...

See http://www.postgresql.org/message-id/16931.1172871...@sss.pgh.pa.us
 and really made possible in 9.4  :)

Though the "usual" @@ syntax would certainly help some users migrate 
over ...



The main advantage package variables have - IMO - are package
permissions. You can define a variable that is writeable only by
functions within a package. That's really handy for things like row
security since it lets you have variables you can only set via a
function that can do things like refuse to run again with different
args, validate input, etc. So you can do expensive work once, then
cheap row security checks against the preset variable. Or use it for
things like "current customer" settings when using pooled connections.


Some sort of "packages" ---in this sense--- could be implemented as 
extensions, but I guess a more integrated approach would be welcome.



It might make sense to extend custom GUCs for this rather than invent
a new mechanism, since GUCs have lots of useful properties like
global, db, user, session and transaction scoping, etc. I'm not really
sure... I just agree that it's a good idea to be able to have
something with similar capabilities to package variables. Especially
security properties.


3. The initialization routines - the routines called when any object from
schema is used first time.

... which is somewhat similar to having an "on session start" trigger.
Also an oft-wanted feature.


Frequently requested, only because one other database requires it for 
what we do with role-level configuration via GUCs.
The other use case I see would definitively be accomodated by having 
packages with the properties you describe above.


These properties might be even emulated via some clever extension 


Just my two cents.


Thanks,

/ J.L.



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

Re: [HACKERS] NOTIFY in Background Worker

[Robert Haas]

On Thu, Nov 5, 2015 at 12:34 AM, Haribabu Kommi
 wrote:
> I marked this patch as ready for committer.

The patch says:

If a background worker registers to receive asynchronous notifications
with the LISTEN through SPI,
there is currently no way for incoming notifications to be received.

But wouldn't it be more correct to say:

If a background worker registers to receive asynchronous notifications
with the LISTEN through SPI, the
worker will log those notifications, but there is no programmatic way
for the worker to intercept and respond to those notifications.

Or something like that?

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


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

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

[Masahiko Sawada]

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

Yeah, I'm going to do that changes if there is no strong objection from hackers.

Regards,

--
Masahiko Sawada


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

Re: [HACKERS] [patch] Proposal for \rotate in psql

[Joe Conway]

On 11/04/2015 10:46 PM, Pavel Stehule wrote:
> 2015-11-05 7:39 GMT+01:00 Craig Ringer wrote:
> I see constant confusion between \copy and COPY. It's a really good
> reason NOT to overload other psql commands IMO.
> 
> but crosstab is one old function from old extension with unfriendly
> design.

Hey, I resemble that remark ;-)

> When we support PIVOT/UNPIVOT - the crosstab function will be
> obsolete. It is not often used command.

But agreed, once we have proper support for PIVOT built into the
grammar, the entire tablefunc extension becomes obsolete, so perhaps
overloading \crosstab is not so bad.

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] Declarative partitioning

[Robert Haas]

On Fri, Oct 30, 2015 at 6:08 AM, Amit Langote
 wrote:
> The DDL and catalogs part are not much different from what I had last
> described though I took a few steps to simplify things. I dropped the
> multi-level partitioning bit

Hmm, that doesn't sound good to me.  I think multi-level partitioning
is a reasonably important use case.

-- 
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] Proposal for \rotate in psql

[Alvaro Herrera]

Joe Conway wrote:
> On 11/04/2015 10:46 PM, Pavel Stehule wrote:

> > but crosstab is one old function from old extension with unfriendly
> > design.
> 
> Hey, I resemble that remark ;-)

You may be old all you want, but certainly not unfriendly!

-- 
Á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] CustomScan support on readfuncs.c

[Robert Haas]

On Wed, Nov 4, 2015 at 10:13 AM, Kouhei Kaigai  wrote:
> Sorry for my late.
>
> I confirmed that CustomScan support of readfuncs.c works fine using the
> attached patch for the PG-Strom tree. It worked as expected - duplication,
> serialization and deserialization by:
>   plan_dup = stringToNode(nodeToString(copyObject(plan_orig)))
>
> So, the custom-scan-on-readfuncs.v1.path itself is good for me.

I don't think this is is what we want.  In particular, I like this:

Plan trees must be able to be duplicated using copyObject,
-   so all the data stored within the custom fields must consist of
-   nodes that that function can handle.  Furthermore, custom scan providers
-   cannot substitute a larger structure that embeds
-   a CustomScan for the structure itself, as would be possible
-   for a CustomPath or CustomScanState.
+   serialized using nodeToString and deserialized using
+   stringToNode, so so all the data stored within
+   the custom fields must consist of nodes that that function
+   can handle.
+   Furthermore, custom scan providers have to implement optional
+   callbacks if it defines substitute a larger structure that embeds
+   a CustomScan for the structure itself.
+  

The purposes of this patch is supposedly to add readfuncs.c support to
custom scans - I agree with that goal.  This documentation change is
talking about something else, namely using a CustomScan as the fist
field in a larger structure.  I'm not sure I agree with that goal, and
in any case it seems like it ought to be a separate patch.  This patch
would be a lot smaller if it weren't trying to make that change too.

I also don't think that this patch ought to introduce
get_current_library_filename().  There are other cases - e.g. for
launching background workers - where such a thing could be used, but
we haven't introduced it up until now.  It isn't a requirement to get
readfuncs support working.

-- 
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 for geqo tweaks

[Nathan Wagner]

On Wed, Nov 04, 2015 at 12:51:52PM -0500, Tom Lane wrote:
> Nathan Wagner  writes:
> > I have two patches to make the geqo initialization and mutation
> > slightly better.
> 
> > The first adjusts the mutation swaps to avoid having to re-pick
> > ties.  The second changes the initialization and shuffling algorithm
> > for the gene array to use an in-place Fisher-Yates shuffling
> > algorithm.
> 
> I took a quick look at this.
> 
> I'm not very impressed with the first patch: it might save a few
> geqo_randint() calls, but it seems to do so at the price of making the
> swap choices less random --- for instance it sure looks to me like the
> last array element is now less likely to participate in swaps than
> other elements.  Unless you can prove that actually the swapping is
> still unbiased, I'm inclined to reject this part.

If I have understood the original code correctly, we need to select two
different random integers between 0 and num_gene-1, inclusive.  That
happens to be num_gene possible results.

Having chosen the first one, which I will call "swap1", we now only have
num_gene-1 possible results, which need to range from either 0 to
swap1-1 or from swap1+1 to num_gene-1, which is num_gene-1 possible
results.  I treat this as a single range from 0 to num_gene-2 and
generate a number within that range, which I will call "swap2".

If swap2 is between 0 and swap1-1, it is in the first range, and no
adjustment is necessary.  If it is greater than or equal to swap1, then
it is in the second range.  However the generated swap2 in the second
range will be between swap1 and num_gene-2, whereas we need it to be
between swap1+1 and num_gene-1, so I add one to swap2, adjusting the
range to the needed range.

It would be equivalent to set swap2 to num_gene-1 in that case,
effectively remapping the first value to the last, but an increment was
more intuitive to me.

> As for the second part, I had to look up Fisher-Yates ;-) but after
> having read Wikipedia's entry about it I think this is a good change.
> The code's shorter and more efficient, and it should mathematically
> provide an equally-unbiased initial shuffle.  It could do with a
> better comment, and I'd be inclined to handle the first element
> outside the loop rather than uselessly computing geqo_randint(0,0),

I could do that.  It will make the code slightly harder to read.  I
wonder if it would be worth having geqo_randint() handle the special
case instead.

> Having said that, though, I believe that it's also probably a
> *different* initial shuffle, which may well mean that GEQO gives
> different plans in some cases.

Yes, I would expect it to be different in the general case.  I think my
commit message noted that, but perhaps it could be more explicit.

> That doesn't bother me as long as we only make the change in HEAD, but
> does anyone want to complain?

I don't see any need to backport either of these patches.

-- 
nw


-- 
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: multiple psql option -c

[Robert Haas]

On Tue, Nov 3, 2015 at 10:16 AM, Pavel Stehule  wrote:
>> The documentation included in this patch doesn't really make it clear
>> why -g is different from or better than -c.
>
> I wrote some text. But needs some work of native speaker.

It does.  It would be nice if some kind reviewer could help volunteer
to clean that up.

Upthread, it was suggested that this option be called -C rather than
-g, and personally I like that better.  I don't really think there's
anything "grouped" about the -g option; it's just an upgraded version
of -c that does what we probably should have had -C do from the
beginning, but now don't want to change out of a concern for
backward-compatibility.  I would propose to change not only the
user-visible option name but all of the internal things that call this
"group" or "grouped".  Maybe introduce ACT_COMMAND_LINE or similar
instead of ACT_GROUP_COMMANDS.

Whatever else we do here, -1 on having both _MainLoop and MainLoop as
function names.  That can't be anything but confusing.

-- 
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] Parallel Seq Scan

[Robert Haas]

On Thu, Nov 5, 2015 at 12:52 AM, Haribabu Kommi
 wrote:
>> Now instead of displaying Partial Seq Scan, if we just display Seq Scan,
>> then it might confuse user, so it is better to add some thing indicating
>> parallel node if we want to go this route.
>
> IMO, the change from Partial Seq Scan to Seq Scan may not confuse user,
> if we clearly specify in the documentation that all plans under a Gather node
> are parallel plans.
>
> This is possible for the execution nodes that executes fully under a
> Gather node.
> The same is not possible for parallel aggregates, so we have to mention the
> aggregate node below Gather node as partial only.
>
> I feel this suggestion arises as may be because of some duplicate code between
> Partial Seq Scan and Seq scan. By using Seq Scan node only if we display as
> Partial Seq Scan by storing some flag in the plan? This avoids the
> need of adding
> new plan nodes.

I was thinking about this idea:

1. Add a parallel_aware flag to each plan.

2. If the flag is set, have EXPLAIN print the word "Parallel" before
the node name.

-- 
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] Request: pg_cancel_backend variant that handles 'idle in transaction' sessions

[Joe Conway]

On 11/05/2015 10:09 AM, Pavel Stehule wrote:
> On 5.11.2015 19:02 Merlin Moncure wrote:
>> Thus, I think we have consensus that transaction_timeout is good -- it
>> would deprecate statement_timeout essentially.  Likewise,
>> pg_cancel_transaction is good and would deprecate pg_cancel_backend;
>> it's hard for me to imagine a scenario where a user would call
>> pg_cancel_backend if pg_cancel_transaction were to be available.
> 
> I am sorry, I see a consensus between you and Stephen only.

S
tC
a<>E
rA  B A  B  A   n
t d
|====---|

Currently we can set timeout and cancel for period B (). I can see
based on this discussion that there are legitimate use cases for wanting
timeout and cancel for any of the periods A, B, or C.

I guess the question then becomes how we provide that coverage. I think
for coverage of timeout you need three individual timeout settings.
However for cancel, it would seem that pg_cancel_transaction would cover
all three cases.

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature

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

[Joe Conway]

On 11/05/2015 10:48 AM, Pavel Stehule wrote:
> S
> tC
> a<>E
> rA  B A  B  A   n
> t d
> |====---|
> 
> Currently we can set timeout and cancel for period B (). I can see
> based on this discussion that there are legitimate use cases for wanting
> timeout and cancel for any of the periods A, B, or C.
> 
> I guess the question then becomes how we provide that coverage. I think
> for coverage of timeout you need three individual timeout settings.
> However for cancel, it would seem that pg_cancel_transaction would cover
> all three cases.
> 
> 
> It can be difficult to set it properly, because you don't know how much
> statements (cycles of A.B) will be in transaction. Respective for
> setting C, I have to know the number of A,B and it isn't possible everytime.

But you might have a limit you want to enforce regardless of the size or
quantity of A & B periods. That's why it needs to be a separate timeout
IMHO. Let's say I never want a transaction to be around more than 60
minutes no matter what. But I also don't want idle in transaction to
ever exceed 30 seconds, and I don't expect individual statements to
exceed 10 minutes.

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature

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

[Pavel Stehule]

2015-11-05 19:56 GMT+01:00 Joe Conway :

> On 11/05/2015 10:48 AM, Pavel Stehule wrote:
> > S
> > tC
> > a<>E
> > rA  B A  B  A   n
> > t d
> > |====---|
> >
> > Currently we can set timeout and cancel for period B (). I can
> see
> > based on this discussion that there are legitimate use cases for
> wanting
> > timeout and cancel for any of the periods A, B, or C.
> >
> > I guess the question then becomes how we provide that coverage. I
> think
> > for coverage of timeout you need three individual timeout settings.
> > However for cancel, it would seem that pg_cancel_transaction would
> cover
> > all three cases.
> >
> >
> > It can be difficult to set it properly, because you don't know how much
> > statements (cycles of A.B) will be in transaction. Respective for
> > setting C, I have to know the number of A,B and it isn't possible
> everytime.
>
> But you might have a limit you want to enforce regardless of the size or
> quantity of A & B periods. That's why it needs to be a separate timeout
> IMHO. Let's say I never want a transaction to be around more than 60
> minutes no matter what. But I also don't want idle in transaction to
> ever exceed 30 seconds, and I don't expect individual statements to
> exceed 10 minutes.
>

I am not sure due my wrong English if we are in agreement or not, I am
sorry :/ - Any mentioned timeouts are useful and covers little bit
different issues - and we need all.

Regards

Pavel


>
> Joe
>
> --
> Crunchy Data - http://crunchydata.com
> PostgreSQL Support for Secure Enterprises
> Consulting, Training, & Open Source Development
>
>

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

[Pavel Stehule]

Dne 5.11.2015 19:02 napsal uživatel "Merlin Moncure" :
>
> On Wed, Nov 4, 2015 at 4:15 PM, Stephen Frost  wrote:
> > * Joshua D. Drake (j...@commandprompt.com) wrote:
> >> On 11/04/2015 01:55 PM, Stephen Frost wrote:
> >> >* Joe Conway (m...@joeconway.com) wrote:
> >> >>On 11/04/2015 01:24 PM, Alvaro Herrera wrote:
> >> >>>I agree with Pavel.  Having a transaction timeout just does not
make any
> >> >>>sense.  I can see absolutely no use for it.  An idle-in-transaction
> >> >>>timeout, on the other hand, is very useful.
> >> >>
> >> >>+1 -- agreed
> >> >
> >> >I'm not sure of that.  I can certainly see a use for transaction
> >> >timeouts- after all, they hold locks and can be very disruptive in the
> >> >long run.  Further, there are cases where a transaction is normally
very
> >> >fast and in a corner case it becomes extremely slow and disruptive to
> >> >the rest of the system.  In those cases, having a timeout for it is
> >> >valuable.
> >>
> >> Yeah but anything holding a lock that long can be terminated via
> >> statement_timeout can it not?
> >
> > Well, no?  statement_timeout is per-statement, while transaction_timeout
> > is, well, per transaction.  If there's a process which is going and has
> > an open transaction and it's holding locks, that can be an issue.
> >
> > To be frank, my gut feeling is that transaction_timeout is actually more
> > useful than statement_timeout.
>
> Exactly.  statement_timeout is weak because it resets for every
> statement regardless of transaction.  Similarly, pg_cancel_backend is
> weak because it only works if a backend is actually in statement
> regardless of transaction state (reading this thread, it's clear that
> this is not widely known even among -hackers which further reinforces
> the point).
>
> Thus, I think we have consensus that transaction_timeout is good -- it
> would deprecate statement_timeout essentially.  Likewise,
> pg_cancel_transaction is good and would deprecate pg_cancel_backend;
> it's hard for me to imagine a scenario where a user would call
> pg_cancel_backend if pg_cancel_transaction were to be available.
>

I am sorry, I see a consensus between you and Stephen only.

Regards

Pavel
> merlin

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

[Robert Haas]

On Thu, Nov 5, 2015 at 1:29 PM, Tomas Vondra
 wrote:
>> Well, one thing is that you might leak information about
>> already-deleted rows, which could be a security vulnerability, or more
>> mundanely cause a function to error out when there are no actually
>> visible rows that could trigger such an error.  It would be surprising
>> if you could add a CHECK(b != 0) constraint to a table, query it for
>> rows where a/b>1, and get a division by zero error.
>
> Yes, I guess we don't have this problem when evaluating the expression on
> heap because we get to check visibility first, and after moving the
> expression to the index we can't do that.

Right.

> But then again, can we come up with a way to distinguish operators that are
> safe to evaluate on indexes - either automatic or manual? We already do that
> with the indexable operators with explicitly listing them in the opclass,
> and we also explicitly use the LEAKPROOF for a similar thing. I don't think
> extending the opclass is a really good solution, but why not to invent
> another *PROOF flag?

I think LEAKPROOF is probably fine for this.  How would the new thing
be different?

> But that is about heap, and we're discussing indexes here, right? I don't
> think you can break the index descriptor like this, because otherwise we'd
> already have problems with that.

Oh, right.

-- 
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] Request: pg_cancel_backend variant that handles 'idle in transaction' sessions

[Merlin Moncure]

On Wed, Nov 4, 2015 at 4:15 PM, Stephen Frost  wrote:
> * Joshua D. Drake (j...@commandprompt.com) wrote:
>> On 11/04/2015 01:55 PM, Stephen Frost wrote:
>> >* Joe Conway (m...@joeconway.com) wrote:
>> >>On 11/04/2015 01:24 PM, Alvaro Herrera wrote:
>> >>>I agree with Pavel.  Having a transaction timeout just does not make any
>> >>>sense.  I can see absolutely no use for it.  An idle-in-transaction
>> >>>timeout, on the other hand, is very useful.
>> >>
>> >>+1 -- agreed
>> >
>> >I'm not sure of that.  I can certainly see a use for transaction
>> >timeouts- after all, they hold locks and can be very disruptive in the
>> >long run.  Further, there are cases where a transaction is normally very
>> >fast and in a corner case it becomes extremely slow and disruptive to
>> >the rest of the system.  In those cases, having a timeout for it is
>> >valuable.
>>
>> Yeah but anything holding a lock that long can be terminated via
>> statement_timeout can it not?
>
> Well, no?  statement_timeout is per-statement, while transaction_timeout
> is, well, per transaction.  If there's a process which is going and has
> an open transaction and it's holding locks, that can be an issue.
>
> To be frank, my gut feeling is that transaction_timeout is actually more
> useful than statement_timeout.

Exactly.  statement_timeout is weak because it resets for every
statement regardless of transaction.  Similarly, pg_cancel_backend is
weak because it only works if a backend is actually in statement
regardless of transaction state (reading this thread, it's clear that
this is not widely known even among -hackers which further reinforces
the point).

Thus, I think we have consensus that transaction_timeout is good -- it
would deprecate statement_timeout essentially.  Likewise,
pg_cancel_transaction is good and would deprecate pg_cancel_backend;
it's hard for me to imagine a scenario where a user would call
pg_cancel_backend if pg_cancel_transaction were to be available.

merlin


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

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

[Pavel Stehule]

2015-11-05 19:31 GMT+01:00 Joe Conway :

> On 11/05/2015 10:09 AM, Pavel Stehule wrote:
> > On 5.11.2015 19:02 Merlin Moncure wrote:
> >> Thus, I think we have consensus that transaction_timeout is good -- it
> >> would deprecate statement_timeout essentially.  Likewise,
> >> pg_cancel_transaction is good and would deprecate pg_cancel_backend;
> >> it's hard for me to imagine a scenario where a user would call
> >> pg_cancel_backend if pg_cancel_transaction were to be available.
> >
> > I am sorry, I see a consensus between you and Stephen only.
>
> S
> tC
> a<>E
> rA  B A  B  A   n
> t d
> |====---|
>
> Currently we can set timeout and cancel for period B (). I can see
> based on this discussion that there are legitimate use cases for wanting
> timeout and cancel for any of the periods A, B, or C.
>
> I guess the question then becomes how we provide that coverage. I think
> for coverage of timeout you need three individual timeout settings.
> However for cancel, it would seem that pg_cancel_transaction would cover
> all three cases.
>

It can be difficult to set it properly, because you don't know how much
statements (cycles of A.B) will be in transaction. Respective for setting
C, I have to know the number of A,B and it isn't possible everytime.

Regards

Pavel


>
> Joe
>
> --
> Crunchy Data - http://crunchydata.com
> PostgreSQL Support for Secure Enterprises
> Consulting, Training, & Open Source Development
>
>

Re: [HACKERS] extend pgbench expressions with functions

[Robert Haas]

On Wed, Nov 4, 2015 at 4:06 AM, Fabien COELHO  wrote:
>> 1. I think there should really be two patches here, the first adding
>> functions, and the second adding doubles.  Those seem like separate
>> changes.  And offhand, the double stuff looks a lot less useful that
>> the function call syntax.
>
> I first submitted the infrastructure part, but I was asked to show how more
> functions could be included, especially random variants. As random
> gaussian/exponential functions require a double argument, there must be some
> support for double values.

Ugh, OK.

>> 2. ddebug and idebug seem like a lousy idea to me.
>
> It was really useful to me for debugging and testing.

That doesn't mean it belongs in the final patch.

>> 3. I'm perplexed by why you've replaced evaluateExpr() with evalInt()
>> and evalDouble().
>
> Basically, the code is significantly shorter and elegant with this option.

I find that pretty hard to swallow.  If the backend took this
approach, we've have a separate evaluate function for every datatype,
which would make it completely impossible to support the creation of
new datatypes.  And I find this code to be quite difficult to follow.

What I think we should have is a type like PgBenchValue that
represents a value which may be an integer or a double or whatever
else we want to support, but not an expression - specifically a value.
Then when you invoke a function or an operator it gets passed one or
more PgBenchValue objects and can do whatever it wants with those,
storing the result into an output PgBenchValue.  So add might look
like this:

void
add(PgBenchValue *result, PgBenchValue *x, PgBenchValue *y)
{
if (x->type == PGBT_INTEGER && y->type == PGBT_INTEGER)
{
result->type = PGBT_INTEGER;
result->u.ival = x->u.ival + y->u.ival;
return;
}
if (x->type == PGBT_DOUBLE && y->type == PGBT_DOUBLE)
{
result->type = PGBT_DOUBLE;
result->u.ival = x->u.dval + y->u.dval;
return;
   }
/* cross-type cases, if desired, go here */
/* if no case applies, somehow report an error */
}

The way you have it, the logic for every function and operator exists
in two copies: one in the integer function, and the other in the
double function.  As soon as we add another data type - strings,
dates, whatever - we'd need to have cases for all of these things in
those functions as well, and every time we add a function, we have to
update all the case statements for every datatype's evaluation
function.  That's just not a scalable model.

-- 
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] Bitmap index scans use of filters on available columns

[Tomas Vondra]

Hi,

On 11/05/2015 06:51 PM, Robert Haas wrote:

On Wed, Nov 4, 2015 at 9:15 PM, Tomas Vondra
 wrote:

I certainly understand there are cases that require care - like the
leakproof thing pointed out by Robert for example. I don't immediately see
why evaluation against dead rows would be a problem.


Well, one thing is that you might leak information about
already-deleted rows, which could be a security vulnerability, or more
mundanely cause a function to error out when there are no actually
visible rows that could trigger such an error.  It would be surprising
if you could add a CHECK(b != 0) constraint to a table, query it for
rows where a/b>1, and get a division by zero error.


Yes, I guess we don't have this problem when evaluating the expression 
on heap because we get to check visibility first, and after moving the 
expression to the index we can't do that.


But then again, can we come up with a way to distinguish operators that 
are safe to evaluate on indexes - either automatic or manual? We already 
do that with the indexable operators with explicitly listing them in the 
opclass, and we also explicitly use the LEAKPROOF for a similar thing. I 
don't think extending the opclass is a really good solution, but why not 
to invent another *PROOF flag?




But I think it's even worse: I believe there can be dead rows in the
heap whose tuple descriptor doesn't even match the current
pg_attribute contents.  Consider BEGIN; ALTER TABLE ... ADD COLUMN ...
int8; INSERT ...; ROLLBACK; ALTER TABLE .. ADD COLUMN .. text; SELECT
...  If the SELECT tries to decode one of the tuples added by the
failed transaction considering the new column as text when the dead
tuples in question had it as an int8, I suspect that you can crash the
server.  Nothing good will happen, anyway.


But that is about heap, and we're discussing indexes here, right? I 
don't think you can break the index descriptor like this, because 
otherwise we'd already have problems with that.


regards

--
Tomas Vondra  http://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] Note about comparation PL/SQL packages and our schema/extensions

[Oleg Bartunov]

On Thu, Nov 5, 2015 at 9:36 AM, Pavel Stehule 
wrote:

> Hi
>
> I had talk about possibility to implement PL/SQL packages in Postgres.
>
> The package concept is coming from ADA language and it is partially
> foreign/redundant element in SQL world. Oracle needs it for modularization,
> because schema plays different role there than in Postgres. My opinion
> about packages in Postgres is clean - the concept of schemas and extension
> is simple and just work. I don't see any big gap there. If we don't play
> Oracle compatibility game, then we don't need to implement class like
> Oracle package. But there are few features, that can help to PL/pgSQL
> developers - generally or with porting from Oracle.
>
> 1. The encapsulation and local scope - all objects in schema are
> accessible from other objects in schema  by default (can be rewritten by
> explicit granting). Local objects are visible only from objects in schema.
> This needs enhancing of our search_path mechanism.
>
> 2. The schema variables - a server side session (can be emulated now) and
> server side local schema session variables (doesn't exist) is pretty useful
> for storing some temp data or high frequent change data - and can
> significantly increase speed of some use cases. Now we emulate it via
> PLPerl shared array, but the encapsulation is missing.
>
> 3. The initialization routines - the routines called when any object from
> schema is used first time.
>
> All three features we can emulate relative simply in C, and probably for
> all mentioned points we have some workaround (less/more ugly) for PL/pgSQL.
> Can be nice do it cleanly in PLpgSQL too.
>

I'd say go ahead !  Packages support is the one of the most requested
feature of people migrating from Oracle.


>
> I don't think we need ADA/ | PL/SQL Syntax - we can enhance our extension
> mechanism to support mentioned points.
>
> Comments, notes?
>
> Regards
>
> Pavel
>

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

[Merlin Moncure]

On Thu, Nov 5, 2015 at 12:09 PM, Pavel Stehule  wrote:
>
> Dne 5.11.2015 19:02 napsal uživatel "Merlin Moncure" :
>>
>> On Wed, Nov 4, 2015 at 4:15 PM, Stephen Frost  wrote:
>> > * Joshua D. Drake (j...@commandprompt.com) wrote:
>> >> On 11/04/2015 01:55 PM, Stephen Frost wrote:
>> >> >* Joe Conway (m...@joeconway.com) wrote:
>> >> >>On 11/04/2015 01:24 PM, Alvaro Herrera wrote:
>> >> >>>I agree with Pavel.  Having a transaction timeout just does not make
>> >> >>> any
>> >> >>>sense.  I can see absolutely no use for it.  An idle-in-transaction
>> >> >>>timeout, on the other hand, is very useful.
>> >> >>
>> >> >>+1 -- agreed
>> >> >
>> >> >I'm not sure of that.  I can certainly see a use for transaction
>> >> >timeouts- after all, they hold locks and can be very disruptive in the
>> >> >long run.  Further, there are cases where a transaction is normally
>> >> > very
>> >> >fast and in a corner case it becomes extremely slow and disruptive to
>> >> >the rest of the system.  In those cases, having a timeout for it is
>> >> >valuable.
>> >>
>> >> Yeah but anything holding a lock that long can be terminated via
>> >> statement_timeout can it not?
>> >
>> > Well, no?  statement_timeout is per-statement, while transaction_timeout
>> > is, well, per transaction.  If there's a process which is going and has
>> > an open transaction and it's holding locks, that can be an issue.
>> >
>> > To be frank, my gut feeling is that transaction_timeout is actually more
>> > useful than statement_timeout.
>>
>> Exactly.  statement_timeout is weak because it resets for every
>> statement regardless of transaction.  Similarly, pg_cancel_backend is
>> weak because it only works if a backend is actually in statement
>> regardless of transaction state (reading this thread, it's clear that
>> this is not widely known even among -hackers which further reinforces
>> the point).
>>
>> Thus, I think we have consensus that transaction_timeout is good -- it
>> would deprecate statement_timeout essentially.  Likewise,
>> pg_cancel_transaction is good and would deprecate pg_cancel_backend;
>> it's hard for me to imagine a scenario where a user would call
>> pg_cancel_backend if pg_cancel_transaction were to be available.
>>
>
> I am sorry, I see a consensus between you and Stephen only.

:-). I guess then maybe not.  Note, I'm not taking a position on other
proposed settings.I'm not claiming they (noted above) solve every
problem, just that they are good.  IOW, the (narrow) claim
pg_cancel_backend and statement_timeout are broken and need to be
fixed.

If you disagree, then you are arguing that it's bad to give
administrator ability to cancel running transaction regardless of
execution state.

merlin


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

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

[Pavel Stehule]

2015-11-05 17:27 GMT+01:00 Robert Haas :

> On Tue, Nov 3, 2015 at 10:16 AM, Pavel Stehule 
> wrote:
> >> The documentation included in this patch doesn't really make it clear
> >> why -g is different from or better than -c.
> >
> > I wrote some text. But needs some work of native speaker.
>
> It does.  It would be nice if some kind reviewer could help volunteer
> to clean that up.
>
> Upthread, it was suggested that this option be called -C rather than
> -g, and personally I like that better.  I don't really think there's
> anything "grouped" about the -g option; it's just an upgraded version
> of -c that does what we probably should have had -C do from the
> beginning, but now don't want to change out of a concern for
> backward-compatibility.  I would propose to change not only the
> user-visible option name but all of the internal things that call this
> "group" or "grouped".  Maybe introduce ACT_COMMAND_LINE or similar
> instead of ACT_GROUP_COMMANDS.
>

-C is good, and if there will not by any objection, I am for it

>
> Whatever else we do here, -1 on having both _MainLoop and MainLoop as
> function names.  That can't be anything but confusing.
>

I'll have free time at weekend, and I'll check it.

Regards

Pavel

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

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

[Merlin Moncure]

On Thu, Nov 5, 2015 at 12:31 PM, Joe Conway  wrote:
> On 11/05/2015 10:09 AM, Pavel Stehule wrote:
>> On 5.11.2015 19:02 Merlin Moncure wrote:
>>> Thus, I think we have consensus that transaction_timeout is good -- it
>>> would deprecate statement_timeout essentially.  Likewise,
>>> pg_cancel_transaction is good and would deprecate pg_cancel_backend;
>>> it's hard for me to imagine a scenario where a user would call
>>> pg_cancel_backend if pg_cancel_transaction were to be available.
>>
>> I am sorry, I see a consensus between you and Stephen only.
>
> S
> tC
> a<>E
> rA  B A  B  A   n
> t d
> |====---|
>
> Currently we can set timeout and cancel for period B (). I can see
> based on this discussion that there are legitimate use cases for wanting
> timeout and cancel for any of the periods A, B, or C.
>
> I guess the question then becomes how we provide that coverage. I think
> for coverage of timeout you need three individual timeout settings.
> However for cancel, it would seem that pg_cancel_transaction would cover
> all three cases.

Agreed on all points.

Tom noted earlier some caveats with the 'idle' timeout in terms of
implementation.  Maybe that needs to be zeroed in on.

merlin


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

Re: [HACKERS] SortSupport for UUID type

[Peter Geoghegan]

On Thu, Oct 8, 2015 at 5:27 PM, Peter Geoghegan  wrote:
> This is more or less lifted from numeric_abbrev_convert_var(). Perhaps
> you should change it there too. The extra set of parenthesis are
> removed in the attached patch. The patch also mechanically updates
> things to be consistent with the text changes on the text thread [1]
> -- I had to rebase.

Attached is almost the same patch, but rebased. This was required
because the name of our new macro was changed to
DatumBigEndianToNative() at the last minute.


-- 
Peter Geoghegan
From faf55ae7682f1710017a7734f23864c16584886c Mon Sep 17 00:00:00 2001
From: Peter Geoghegan 
Date: Tue, 21 Jul 2015 16:18:25 -0700
Subject: [PATCH] Add SortSupport routine for UUID data type

This introduces a simple encoding scheme to produce abbreviated keys:
pack as many bytes of each UUID as will fit into a Datum.  On
little-endian machines, a byteswap is also performed; the abbreviated
comparator can therefore just consist of a simple 3-way unsigned integer
comparison.  This comports with regular UUID comparisons, because they
use memcmp() to compare authoritative UUID datums.
---
 src/backend/utils/adt/uuid.c| 187 
 src/include/catalog/pg_amproc.h |   1 +
 src/include/catalog/pg_proc.h   |   2 +
 src/include/utils/builtins.h|   1 +
 4 files changed, 191 insertions(+)

diff --git a/src/backend/utils/adt/uuid.c b/src/backend/utils/adt/uuid.c
index bb76b8d..6356229 100644
--- a/src/backend/utils/adt/uuid.c
+++ b/src/backend/utils/adt/uuid.c
@@ -14,8 +14,12 @@
 #include "postgres.h"
 
 #include "access/hash.h"
+#include "lib/hyperloglog.h"
 #include "libpq/pqformat.h"
+#include "port/pg_bswap.h"
 #include "utils/builtins.h"
+#include "utils/guc.h"
+#include "utils/sortsupport.h"
 #include "utils/uuid.h"
 
 /* uuid size in bytes */
@@ -27,8 +31,21 @@ struct pg_uuid_t
 	unsigned char data[UUID_LEN];
 };
 
+/* sortsupport for uuid */
+typedef struct
+{
+	int64		input_count;	/* number of non-null values seen */
+	bool		estimating;		/* true if estimating cardinality */
+
+	hyperLogLogState abbr_card; /* cardinality estimator */
+} uuid_sortsupport_state;
+
 static void string_to_uuid(const char *source, pg_uuid_t *uuid);
 static int	uuid_internal_cmp(const pg_uuid_t *arg1, const pg_uuid_t *arg2);
+static int	uuid_fast_cmp(Datum x, Datum y, SortSupport ssup);
+static int	uuid_cmp_abbrev(Datum x, Datum y, SortSupport ssup);
+static bool	uuid_abbrev_abort(int memtupcount, SortSupport ssup);
+static Datum	uuid_abbrev_convert(Datum original, SortSupport ssup);
 
 Datum
 uuid_in(PG_FUNCTION_ARGS)
@@ -222,6 +239,176 @@ uuid_cmp(PG_FUNCTION_ARGS)
 	PG_RETURN_INT32(uuid_internal_cmp(arg1, arg2));
 }
 
+/*
+ * Sort support strategy routine
+ */
+Datum
+uuid_sortsupport(PG_FUNCTION_ARGS)
+{
+	SortSupport	ssup = (SortSupport) PG_GETARG_POINTER(0);
+
+	ssup->comparator = uuid_fast_cmp;
+	ssup->ssup_extra = NULL;
+
+	if (ssup->abbreviate)
+	{
+		uuid_sortsupport_state	   *uss;
+		MemoryContextoldcontext;
+
+		oldcontext = MemoryContextSwitchTo(ssup->ssup_cxt);
+
+		uss = palloc(sizeof(uuid_sortsupport_state));
+		uss->input_count = 0;
+		uss->estimating = true;
+		initHyperLogLog(>abbr_card, 10);
+
+		ssup->ssup_extra = uss;
+
+		ssup->comparator = uuid_cmp_abbrev;
+		ssup->abbrev_converter = uuid_abbrev_convert;
+		ssup->abbrev_abort = uuid_abbrev_abort;
+		ssup->abbrev_full_comparator = uuid_fast_cmp;
+
+		MemoryContextSwitchTo(oldcontext);
+	}
+
+	PG_RETURN_VOID();
+}
+
+/*
+ * SortSupport comparison func
+ */
+static int
+uuid_fast_cmp(Datum x, Datum y, SortSupport ssup)
+{
+	pg_uuid_t  *arg1 = DatumGetUUIDP(x);
+	pg_uuid_t  *arg2 = DatumGetUUIDP(y);
+
+	return uuid_internal_cmp(arg1, arg2);
+}
+
+/*
+ * Abbreviated key comparison func
+ */
+static int
+uuid_cmp_abbrev(Datum x, Datum y, SortSupport ssup)
+{
+	if (x > y)
+		return 1;
+	else if (x == y)
+		return 0;
+	else
+		return -1;
+}
+
+/*
+ * Callback for estimating effectiveness of abbreviated key optimization.
+ *
+ * We pay no attention to the cardinality of the non-abbreviated data, because
+ * there is no equality fast-path within authoritative uuid comparator.
+ */
+static bool
+uuid_abbrev_abort(int memtupcount, SortSupport ssup)
+{
+	uuid_sortsupport_state	   *uss = ssup->ssup_extra;
+	double		abbr_card;
+
+	if (memtupcount < 1 || uss->input_count < 1 || !uss->estimating)
+		return false;
+
+	abbr_card = estimateHyperLogLog(>abbr_card);
+
+	/*
+	 * If we have >100k distinct values, then even if we were sorting many
+	 * billion rows we'd likely still break even, and the penalty of undoing
+	 * that many rows of abbrevs would probably not be worth it.  Stop even
+	 * counting at that point.
+	 */
+	if (abbr_card > 10.0)
+	{
+#ifdef TRACE_SORT
+		if (trace_sort)
+			elog(LOG,
+ "uuid_abbrev: estimation ends at cardinality %f"
+ " after " INT64_FORMAT " values (%d rows)",
+ abbr_card, uss->input_count, 

Re: [HACKERS] GIN data corruption bug(s) in 9.6devel

[Jeff Janes]

On Thu, Nov 5, 2015 at 2:18 PM, Tomas Vondra
 wrote:
> Hi,
>
> while repeating some full-text benchmarks on master, I've discovered
> that there's a data corruption bug somewhere. What happens is that while
> loading data into a table with GIN indexes (using multiple parallel
> connections), I sometimes get this:
>
> TRAP: FailedAssertion("!(((PageHeader) (page))->pd_special >=
> (__builtin_offsetof (PageHeaderData, pd_linp)))", File: "ginfast.c",
> Line: 537)
> LOG:  server process (PID 22982) was terminated by signal 6: Aborted
> DETAIL:  Failed process was running: autovacuum: ANALYZE messages
>
> The details of the assert are always exactly the same - it's always
> autovacuum and it trips on exactly the same check. And the backtrace
> always looks like this (full backtrace attached):
>
> #0  0x7f133b635045 in raise () from /lib64/libc.so.6
> #1  0x7f133b6364ea in abort () from /lib64/libc.so.6
> #2  0x007dc007 in ExceptionalCondition
> (conditionName=conditionName@entry=0x81a088 "!(((PageHeader)
> (page))->pd_special >= (__builtin_offsetof (PageHeaderData, pd_linp)))",
>errorType=errorType@entry=0x81998b "FailedAssertion",
> fileName=fileName@entry=0x83480a "ginfast.c",
> lineNumber=lineNumber@entry=537) at assert.c:54
> #3  0x004894aa in shiftList (stats=0x0, fill_fsm=1 '\001',
> newHead=26357, metabuffer=130744, index=0x7f133c0f7518) at ginfast.c:537
> #4  ginInsertCleanup (ginstate=ginstate@entry=0x7ffd98ac9160,
> vac_delay=vac_delay@entry=1 '\001', fill_fsm=fill_fsm@entry=1 '\001',
> stats=stats@entry=0x0) at ginfast.c:908
> #5  0x004874f7 in ginvacuumcleanup (fcinfo=) at
> ginvacuum.c:662
> ...

This looks like it is probably the same bug discussed here:

http://www.postgresql.org/message-id/CAMkU=1xalflhuuohfp5v33rzedlvb5aknnujceum9knbkrb...@mail.gmail.com

And here:

http://www.postgresql.org/message-id/56041b26.2040...@sigaev.ru

The bug theoretically exists in 9.5, but it wasn't until 9.6 (commit
e95680832854cf300e64c) that free pages were recycled aggressively
enough that it actually becomes likely to be hit.

There are some proposed patches in those threads, but discussion on
them seems to have stalled out.  Can you try one and see if it fixes
the problems you are seeing?

Cheers,

Jeff


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

Re: [HACKERS] Brain fade in gin_extract_jsonb_path()

[Tom Lane]

I wrote:
> I think the easiest way to fix this is to forget about the special
> initialization at outer level and just always initialize a new stack level
> to have the same hash as its parent.  That leads to the first patch below
> --- but once you look at that, you realize that we've got unnecessarily
> many copies of stack->parent->hash into stack->hash, so what I actually
> propose now is the second patch below.

Meh, that wasn't quite right either --- need to reset stack->hash after
processing a sub-object, not only after a scalar VALUE.  I think what I
committed is OK though.

regards, tom lane


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

Re: [HACKERS] [PATCH] Skip ALTER x SET SCHEMA if the schema didn't change

[David G. Johnston]

On Thu, Nov 5, 2015 at 2:15 PM, Robert Haas  wrote:

> On Thu, Nov 5, 2015 at 6:20 AM, Haribabu Kommi 
> wrote:
> > I went through the patch, following are my observations,
> >
> > Patch applied with hunks and compiled with out warnings.
> > Basic tests are passed.
>
> I'm interested in hearing opinions from multiple people about the
> following two questions:
>
> 1. Is the new behavior better than the old behavior?
> 2. Will breaking backward compatibility make too many people unhappy?
>
> My guess is that the answer to the first question is "yes" and that
> the answer to the second one is "no", but this is clearly a
> significant incompatibility, so I'd like to hear some more opinions
> before concluding that we definitely want to do this.
>

​For #2 I'm not that concerned about turning an error case into a non-error.

The rationale behind #1 makes sense to me.  Given all the recent work on
"IF NOT EXISTS" we obviously think that this general behavior is desirable
and we should correct this deviation from that norm.

David J.

Re: [HACKERS] GIN data corruption bug(s) in 9.6devel

[Tomas Vondra]

On 11/05/2015 11:44 PM, Jeff Janes wrote:
>

This looks like it is probably the same bug discussed here:

http://www.postgresql.org/message-id/CAMkU=1xalflhuuohfp5v33rzedlvb5aknnujceum9knbkrb...@mail.gmail.com

And here:

http://www.postgresql.org/message-id/56041b26.2040...@sigaev.ru

The bug theoretically exists in 9.5, but it wasn't until 9.6 (commit
e95680832854cf300e64c) that free pages were recycled aggressively
enough that it actually becomes likely to be hit.


I have only quickly skimmed the discussions, but my impression was that 
it's mostly about removing stuff that shouldn't be removed and such. But 
maybe there are race conditions that cause data corruption. I don't 
really want to dive too deeply into this, I've already spent too much 
time trying to reproduce it.




There are some proposed patches in those threads, but discussion on
them seems to have stalled out. Can you try one and see if it fixes
the problems you are seeing?


I can do that - I see there are three patches in the two threads:

  1) gin_pending_lwlock.patch (Jeff Janes)
  2) gin_pending_pagelock.patch (Jeff Janes)
  3) gin_alone_cleanup-2.patch (Teodor Sigaev)

Should I test all of them? Or is (1) obsoleted by (2) for example?

regards

--
Tomas Vondra  http://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] NOTIFY in Background Worker

[Haribabu Kommi]

On Fri, Nov 6, 2015 at 4:57 AM, Robert Haas  wrote:
> On Thu, Nov 5, 2015 at 12:34 AM, Haribabu Kommi
>  wrote:
>> I marked this patch as ready for committer.
>
> The patch says:
>
> If a background worker registers to receive asynchronous notifications
> with the LISTEN through SPI,
> there is currently no way for incoming notifications to be received.
>
> But wouldn't it be more correct to say:
>
> If a background worker registers to receive asynchronous notifications
> with the LISTEN through SPI, the
> worker will log those notifications, but there is no programmatic way
> for the worker to intercept and respond to those notifications.

Yes, the above description is good.

Regards,
Hari Babu
Fujitsu Australia


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

Re: [HACKERS] GIN data corruption bug(s) in 9.6devel

[Jeff Janes]

On Thu, Nov 5, 2015 at 3:50 PM, Tomas Vondra
 wrote:
>
>
> On 11/05/2015 11:44 PM, Jeff Janes wrote:
>>
>>
>> This looks like it is probably the same bug discussed here:
>>
>>
>> http://www.postgresql.org/message-id/CAMkU=1xalflhuuohfp5v33rzedlvb5aknnujceum9knbkrb...@mail.gmail.com
>>
>> And here:
>>
>> http://www.postgresql.org/message-id/56041b26.2040...@sigaev.ru
>>
>> The bug theoretically exists in 9.5, but it wasn't until 9.6 (commit
>> e95680832854cf300e64c) that free pages were recycled aggressively
>> enough that it actually becomes likely to be hit.
>
>
> I have only quickly skimmed the discussions, but my impression was that it's
> mostly about removing stuff that shouldn't be removed and such. But maybe
> there are race conditions that cause data corruption. I don't really want to
> dive too deeply into this, I've already spent too much time trying to
> reproduce it.
>
>>
>> There are some proposed patches in those threads, but discussion on
>> them seems to have stalled out. Can you try one and see if it fixes
>> the problems you are seeing?
>
>
> I can do that - I see there are three patches in the two threads:
>
>   1) gin_pending_lwlock.patch (Jeff Janes)
>   2) gin_pending_pagelock.patch (Jeff Janes)
>   3) gin_alone_cleanup-2.patch (Teodor Sigaev)
>
> Should I test all of them? Or is (1) obsoleted by (2) for example?

1 is obsolete.  Either 2 or 3 should fix the bug, provided this is the
bug you are seeing.  They have different performance side effects, but
as far as fixing the bug they should be equivalent.

Cheers,

Jeff


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

[HACKERS] Beta2 Next week

[Josh Berkus]

Folks,

We are going to try to get Beta2 wrapped on Monday for a release next
week.  So if you're currently working on a 9.5 Open Item, getting it
checked in tommorrow or this weekend would be great.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


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

Re: [HACKERS] Note about comparation PL/SQL packages and our schema/extensions

[Pavel Stehule]

2015-11-05 21:29 GMT+01:00 Pavel Stehule :

>
>
> 2015-11-05 13:31 GMT+01:00 Craig Ringer :
>
>> On 5 November 2015 at 14:36, Pavel Stehule 
>> wrote:
>>
>> > 1. The encapsulation and local scope - all objects in schema are
>> accessible
>> > from other objects in schema  by default (can be rewritten by explicit
>> > granting). Local objects are visible only from objects in schema. This
>> needs
>> > enhancing of our search_path mechanism.
>>
>> Yep. It's as if, within function packagename.funcname, packagename is
>> implicitly prefixed to search_path .
>>
>> I can see that being handy, but not especially important.
>>
>> > 2. The schema variables - a server side session (can be emulated now)
>> and
>> > server side local schema session variables (doesn't exist) is pretty
>> useful
>> > for storing some temp data or high frequent change data - and can
>> > significantly increase speed of some use cases. Now we emulate it via
>> PLPerl
>> > shared array, but the encapsulation is missing.
>>
>> This is the feature I feel we could really use.
>>
>> I see *lots* of people emulating session variables by (ab)using custom
>> GUCs. The missing-ok variant of current_setting helps with this to the
>> point where it's fairly OK now.
>>
>> The main advantage package variables have - IMO - are package
>> permissions. You can define a variable that is writeable only by
>> functions within a package. That's really handy for things like row
>> security since it lets you have variables you can only set via a
>> function that can do things like refuse to run again with different
>> args, validate input, etc. So you can do expensive work once, then
>> cheap row security checks against the preset variable. Or use it for
>> things like "current customer" settings when using pooled connections.
>>
>> It might make sense to extend custom GUCs for this rather than invent
>> a new mechanism, since GUCs have lots of useful properties like
>> global, db, user, session and transaction scoping, etc. I'm not really
>> sure... I just agree that it's a good idea to be able to have
>> something with similar capabilities to package variables. Especially
>> security properties.
>>
>
> I mentioned "local schema session variables", but I had to say "local
> schema variables", because I don't think using GUC is good idea.
>
> Personally I am inclined to use different mechanism than GUC - GUC is
> untyped and slow, and I don't prefer T-SQL syntax - it is foreign element -
> and it can do false believe about relation between T-SQL and Postgres.
>
> The local schema variables can be accessed only from PL functions - and it
> can have usual syntax for any specific PL language.
>
> So some extension can looks like
>
> DECLARE [ VARIABLE ] schema.myvar AS integer;
>
> CREATE LOCAL FUNCTION schema.init()
> RETURNS void AS $$
> BEGIN
>   myvar := 0;
> END;
>
> CREATE OR REPLACE FUNCTION schema.current_var()
> RETURNS integer AS $$
> BEGIN
>   RETURN myvar;
> END;
>
> CREATE OR REPLACE FUNCTION schema.set_var(myvar integer)
> RETURNS void AS $$
> BEGIN
>schema.myvar := var; -- using qualified name as name collision solution
> END;
>
> Outside schema the access should be via functions schema.current_var() and
> schema.set_var().
>
> The advantage of this design - we don't need to modify a SQL parser for
> DQL and DML, and we don't need to introduce any nonstandard behave (syntax)
> to SQL .
>

probably we can adopt concept ANSI/SQL MODULEs enhanced about the
variables. It is relative similar to proposed code.




>
>
>>
>> > 3. The initialization routines - the routines called when any object
>> from
>> > schema is used first time.
>>
>> ... which is somewhat similar to having an "on session start" trigger.
>> Also an oft-wanted feature.
>>
>> --
>>  Craig Ringer   http://www.2ndQuadrant.com/
>>  PostgreSQL Development, 24x7 Support, Training & Services
>>
>
>

[HACKERS] GIN data corruption bug(s) in 9.6devel

[Tomas Vondra]

Hi,

while repeating some full-text benchmarks on master, I've discovered
that there's a data corruption bug somewhere. What happens is that while
loading data into a table with GIN indexes (using multiple parallel
connections), I sometimes get this:

TRAP: FailedAssertion("!(((PageHeader) (page))->pd_special >=
(__builtin_offsetof (PageHeaderData, pd_linp)))", File: "ginfast.c",
Line: 537)
LOG:  server process (PID 22982) was terminated by signal 6: Aborted
DETAIL:  Failed process was running: autovacuum: ANALYZE messages

The details of the assert are always exactly the same - it's always
autovacuum and it trips on exactly the same check. And the backtrace
always looks like this (full backtrace attached):

#0  0x7f133b635045 in raise () from /lib64/libc.so.6
#1  0x7f133b6364ea in abort () from /lib64/libc.so.6
#2  0x007dc007 in ExceptionalCondition
(conditionName=conditionName@entry=0x81a088 "!(((PageHeader)
(page))->pd_special >= (__builtin_offsetof (PageHeaderData, pd_linp)))",
   errorType=errorType@entry=0x81998b "FailedAssertion",
fileName=fileName@entry=0x83480a "ginfast.c",
lineNumber=lineNumber@entry=537) at assert.c:54
#3  0x004894aa in shiftList (stats=0x0, fill_fsm=1 '\001',
newHead=26357, metabuffer=130744, index=0x7f133c0f7518) at ginfast.c:537
#4  ginInsertCleanup (ginstate=ginstate@entry=0x7ffd98ac9160,
vac_delay=vac_delay@entry=1 '\001', fill_fsm=fill_fsm@entry=1 '\001',
stats=stats@entry=0x0) at ginfast.c:908
#5  0x004874f7 in ginvacuumcleanup (fcinfo=) at
ginvacuum.c:662
...

It's not perfectly deterministic - sometimes I had repeat the whole load
multiple times (up to 5x, and each load takes ~30minutes).

I'm pretty sure this is not external issue, because I've reproduced it
on a different machine (different CPU / kernel / libraries / compiler).
It's however interesting that on the other machine I've also observed a
different kind of lockups, where the sessions get stuck on semop() in
gininsert (again, full backtrace attached):

#0  0x003f3d4eaf97 in semop () from /lib64/libc.so.6
#1  0x0067a41f in PGSemaphoreLock (sema=0x7f93290405d8) at
pg_sema.c:387
#2  0x006df613 in LWLockAcquire (lock=0x7f92a4dce900,
mode=LW_EXCLUSIVE) at lwlock.c:1049
#3  0x004878c6 in ginHeapTupleFastInsert
(ginstate=0x7ffd969c88f0, collector=0x7ffd969caeb0) at ginfast.c:250
#4  0x0047423a in gininsert (fcinfo=) at
gininsert.c:531
...

I'm not sure whether this is a different manifestation of the same issue
or another bug. The systems are not exactly the same - one has a single
socket (i5) while the other one has 2 (Xeon), the compilers and kernels
are different and so on.

I've also seen cases when the load seemingly completed OK, but trying to
dump the table to disk using COPY resulted in

   ERROR:  compressed data is corrupted

which I find rather strange as there was no previous error, and also
COPY should only dump table data (while the asserts were in GIN code
handling index pages, unless I'm mistaken). Seems like a case of
insufficient locking where two backends scribble on the same page
somehow, and then autovacuum hits the assert. Ot maybe not, not sure.

I've been unable to reproduce the issue on REL9_5_STABLE (despite
running the load ~20x on each machine), so that seems safe, and the
issue was introduced by some of the newer commits.

I've already spent too much CPU time on this, so perhaps someone with
better knowledge of the GIN code can take care of this. To reproduce it
you may use the same code I did - it's available here:

   https://bitbucket.org/tvondra/archie

it's a PoC of database with pgsql mailing lists with fulltext. It's a
bit messy, but it's rather simple

   1) clone the repo

  $ git clone https://bitbucket.org/tvondra/archie.git

   2) create a directory for downloading the mbox files

  $ mkdir archie-data

   3) download the mbox files (~4.5GB of data) using the download
  script (make sure archie/bin is on PATH)

  $ cd archie-data
  $ export PATH=../archie/bin:$PATH
  $ ../archie/download

   4) use "run" scipt (attached) to run the load n-times on a given
  commit

  $ run.sh master 10

  NOTICE: The run script is the messiest one, you'll have to
  edit it to fix paths etc.


regards

--
Tomas Vondra  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Core was generated by `postgres: autovacuum worker process   archie 
 '.
Program terminated with signal SIGABRT, Aborted.
#0  0x7f133b635045 in raise () from /lib64/libc.so.6
(gdb) bt
#0  0x7f133b635045 in raise () from /lib64/libc.so.6
#1  0x7f133b6364ea in abort () from /lib64/libc.so.6
#2  0x007dc007 in ExceptionalCondition 
(conditionName=conditionName@entry=0x81a088 "!(((PageHeader) 
(page))->pd_special >= (__builtin_offsetof (PageHeaderData, 

Re: [HACKERS] Brain fade in gin_extract_jsonb_path()

[Peter Geoghegan]

On Thu, Nov 5, 2015 at 12:39 PM, Tom Lane  wrote:
> The attached one-liner fixes it, but of course this means that on-disk
> jsonb_path_ops indexes are possibly broken and will need to be reindexed.
> I see no way around that ... does anybody else?

I think it's impossible to avoid having to recommend a reindex here.

-- 
Peter Geoghegan


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

Re: [HACKERS] [PATCH] Skip ALTER x SET SCHEMA if the schema didn't change

[Alvaro Herrera]

David G. Johnston wrote:
> On Thu, Nov 5, 2015 at 2:15 PM, Robert Haas  wrote:

> > I'm interested in hearing opinions from multiple people about the
> > following two questions:
> >
> > 1. Is the new behavior better than the old behavior?
> > 2. Will breaking backward compatibility make too many people unhappy?
> >
> > My guess is that the answer to the first question is "yes" and that
> > the answer to the second one is "no", but this is clearly a
> > significant incompatibility, so I'd like to hear some more opinions
> > before concluding that we definitely want to do this.
> 
> For #2 I'm not that concerned about turning an error case into a non-error.

Yeah, maybe I lack imagination but I don't see how the current behavior
is actually useful.  We already silently do nothing when appropriate in
many other DDL commands.

> The rationale behind #1 makes sense to me.  Given all the recent work on
> "IF NOT EXISTS" we obviously think that this general behavior is desirable
> and we should correct this deviation from that norm.

Agreed.

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

[Fabien COELHO]

Hello Robert,


2. ddebug and idebug seem like a lousy idea to me.


It was really useful to me for debugging and testing.


That doesn't mean it belongs in the final patch.


I think it is useful when debugging a script, not just for debugging the 
evaluation code itself.



3. I'm perplexed by why you've replaced evaluateExpr() with evalInt()
and evalDouble().


Basically, the code is significantly shorter and elegant with this option.


I find that pretty hard to swallow.


Hmmm, maybe with some more explanations?


If the backend took this approach,


Sure, I would never suggest to do anything like that in the backend!

we've have a separate evaluate function for every datatype, which would 
make it completely impossible to support the creation of new datatypes.


In the backend implementation is generic about types (one can add a type 
dynamically in the system), which is another abstraction level, it does 
not compare in any way.



And I find this code to be quite difficult to follow.


It is really the function *you* wrote, and there is just one version for
int and one for double.

What I think we should have is a type like PgBenchValue that represents 
a value which may be an integer or a double or whatever else we want to 
support, but not an expression - specifically a value. Then when you 
invoke a function or an operator it gets passed one or more PgBenchValue 
objects and can do whatever it wants with those, storing the result into 
an output PgBenchValue. So add might look like this:


Sure, I do know what it looks like, and I want to avoid it, because this 
is just a lot of ugly code which is useless for pgbench purpose.



void
add(PgBenchValue *result, PgBenchValue *x, PgBenchValue *y)
{
   if (x->type == PGBT_INTEGER && y->type == PGBT_INTEGER)
   {
   result->type = PGBT_INTEGER;
   result->u.ival = x->u.ival + y->u.ival;
   return;
   }
   if (x->type == PGBT_DOUBLE && y->type == PGBT_DOUBLE)
   {
   result->type = PGBT_DOUBLE;
   result->u.ival = x->u.dval + y->u.dval;
   return;
  }
   /* cross-type cases, if desired, go here */


Why reject 1 + 2.0 ? So the cross-type cases are really required for user 
sanity, which adds:


if (x->type == PGBT_DOUBLE && y->type == PGBT_INTEGER)
{
result->type = PGBT_DOUBLE;
result->u.ival = x->u.dval + y->u.ival;
return;
}
if (x->type == PGBT_INTEGER && y->type == PGBT_DOUBLE)
{
result->type = PGBT_DOUBLE;
result->u.ival = x->u.ival + y->u.dval;
   return;
   } 

}


For the '+' overload operator with conversions there are 4 cases (2 
arguments ** 2 types) to handle. For all 5 binary operators (+ - * / %). 
that makes 20 cases to handle. Then for every function, you have to deal 
with type conversion as well, each function times number of arguments ** 
number of types. That is 2 cases for abs, 4 cases for random, 8 cases for 
each random_exp*, 8 for random_gaus*, and so on. Some thinking would be 
required for n-ary functions (min & max).


Basically, it can be done, no technical issue, it is just a matter of 
writing a *lot* of repetitive code, hundreds of lines of them. As I think 
it does not bring any value for pgbench purpose, I used the other approach 
which reduces the code size by avoiding the combinatorial "cross-type" 
conversions.



The way you have it, the logic for every function and operator exists
in two copies: one in the integer function, and the other in the
double function.


Yep, but only 2 cases to handle, instead of 4 cases in the above example, 
that is the point.


As soon as we add another data type - strings, dates, whatever - we'd 
need to have cases for all of these things in those functions as well,
and every time we add a function, we have to update all the case 
statements for every datatype's evaluation function.  That's just not a 
scalable model.


On the contrary, it is reasonably scalable:

With the eval-per-type for every added type one should implement one eval 
function which handles all functions and operators, each eval function 
roughly the same size as the current ones.


With the above approach, the overloaded add function which handles 2 
operands with 3 types ('post' + 'gres' -> 'postgres') would have to deal 
with 2**3 = 8 types combinations instead of 4, so basically it would be 
doubling the code size. If you add dates on top of that, 2**4 = 16 cases 
just for one operator. No difficulty there, just a lot of lines...


Basically the code size complexity of the above approach is:

  #functions * (#args ** #types)

while for the approach in the submitted patch it is:

  #types * #functions

Now I obviously agree that the approach is not generic and should not be 
used in other contexts, it is just that it is simple/short and it fits the 
bill for pgbench.


Note that I do not really envision adding more types for pgbench scripts. 
The double type is just a side effect of the non uniform randoms. What I 
plan is 

Re: [HACKERS] Note about comparation PL/SQL packages and our schema/extensions

[Pavel Stehule]

2015-11-05 13:31 GMT+01:00 Craig Ringer :

> On 5 November 2015 at 14:36, Pavel Stehule 
> wrote:
>
> > 1. The encapsulation and local scope - all objects in schema are
> accessible
> > from other objects in schema  by default (can be rewritten by explicit
> > granting). Local objects are visible only from objects in schema. This
> needs
> > enhancing of our search_path mechanism.
>
> Yep. It's as if, within function packagename.funcname, packagename is
> implicitly prefixed to search_path .
>
> I can see that being handy, but not especially important.
>
> > 2. The schema variables - a server side session (can be emulated now) and
> > server side local schema session variables (doesn't exist) is pretty
> useful
> > for storing some temp data or high frequent change data - and can
> > significantly increase speed of some use cases. Now we emulate it via
> PLPerl
> > shared array, but the encapsulation is missing.
>
> This is the feature I feel we could really use.
>
> I see *lots* of people emulating session variables by (ab)using custom
> GUCs. The missing-ok variant of current_setting helps with this to the
> point where it's fairly OK now.
>
> The main advantage package variables have - IMO - are package
> permissions. You can define a variable that is writeable only by
> functions within a package. That's really handy for things like row
> security since it lets you have variables you can only set via a
> function that can do things like refuse to run again with different
> args, validate input, etc. So you can do expensive work once, then
> cheap row security checks against the preset variable. Or use it for
> things like "current customer" settings when using pooled connections.
>
> It might make sense to extend custom GUCs for this rather than invent
> a new mechanism, since GUCs have lots of useful properties like
> global, db, user, session and transaction scoping, etc. I'm not really
> sure... I just agree that it's a good idea to be able to have
> something with similar capabilities to package variables. Especially
> security properties.
>

I mentioned "local schema session variables", but I had to say "local
schema variables", because I don't think using GUC is good idea.

Personally I am inclined to use different mechanism than GUC - GUC is
untyped and slow, and I don't prefer T-SQL syntax - it is foreign element -
and it can do false believe about relation between T-SQL and Postgres.

The local schema variables can be accessed only from PL functions - and it
can have usual syntax for any specific PL language.

So some extension can looks like

DECLARE [ VARIABLE ] schema.myvar AS integer;

CREATE LOCAL FUNCTION schema.init()
RETURNS void AS $$
BEGIN
  myvar := 0;
END;

CREATE OR REPLACE FUNCTION schema.current_var()
RETURNS integer AS $$
BEGIN
  RETURN myvar;
END;

CREATE OR REPLACE FUNCTION schema.set_var(myvar integer)
RETURNS void AS $$
BEGIN
   schema.myvar := var; -- using qualified name as name collision solution
END;

Outside schema the access should be via functions schema.current_var() and
schema.set_var().

The advantage of this design - we don't need to modify a SQL parser for DQL
and DML, and we don't need to introduce any nonstandard behave (syntax) to
SQL .


>
> > 3. The initialization routines - the routines called when any object from
> > schema is used first time.
>
> ... which is somewhat similar to having an "on session start" trigger.
> Also an oft-wanted feature.
>
> --
>  Craig Ringer   http://www.2ndQuadrant.com/
>  PostgreSQL Development, 24x7 Support, Training & Services
>

Re: [HACKERS] Within CF app, "Bug Fixes" should be "Bug Fixes/Refactoring"

[Peter Geoghegan]

On Thu, Oct 29, 2015 at 1:29 PM, Peter Geoghegan  wrote:
>> "Refactoring" seems rather a narrow definition of what might show up
>> in such a category, btw.  Maybe "Code Beautification" would be a
>> suitable title?  I'm bikeshedding though.
>
> I think that there is value in limiting the number of topics. But I
> hardly but much weight on this. Any of the above are fine.

Can someone follow up and push this to the CF app? "Refactoring" seems
to be the consensus.

-- 
Peter Geoghegan


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

Re: [HACKERS] [patch] Proposal for \rotate in psql

[Pavel Stehule]

2015-11-05 17:17 GMT+01:00 Joe Conway :

> On 11/04/2015 10:46 PM, Pavel Stehule wrote:
> > 2015-11-05 7:39 GMT+01:00 Craig Ringer wrote:
> > I see constant confusion between \copy and COPY. It's a really good
> > reason NOT to overload other psql commands IMO.
> >
> > but crosstab is one old function from old extension with unfriendly
> > design.
>
> Hey, I resemble that remark ;-)
>

I am sorry, Joe - no any personal attack - I'll pay a beer for you if you
visit Prague :)

Pavel


>
> > When we support PIVOT/UNPIVOT - the crosstab function will be
> > obsolete. It is not often used command.
>
> But agreed, once we have proper support for PIVOT built into the
> grammar, the entire tablefunc extension becomes obsolete, so perhaps
> overloading \crosstab is not so bad.
>
> Joe
>
> --
> Crunchy Data - http://crunchydata.com
> PostgreSQL Support for Secure Enterprises
> Consulting, Training, & Open Source Development
>
>

Re: [HACKERS] [PATCH] Skip ALTER x SET SCHEMA if the schema didn't change

[Robert Haas]

On Thu, Nov 5, 2015 at 6:20 AM, Haribabu Kommi  wrote:
> I went through the patch, following are my observations,
>
> Patch applied with hunks and compiled with out warnings.
> Basic tests are passed.

I'm interested in hearing opinions from multiple people about the
following two questions:

1. Is the new behavior better than the old behavior?
2. Will breaking backward compatibility make too many people unhappy?

My guess is that the answer to the first question is "yes" and that
the answer to the second one is "no", but this is clearly a
significant incompatibility, so I'd like to hear some more opinions
before concluding that we definitely want to do this.

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


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

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

[David Steele]

On 11/5/15 10:10 AM, Alvaro Herrera wrote:

David Steele wrote:


The important thing about this implementation was that nothing was
terminated unless it had exceed a timeout AND was blocking another
process.


This seems a nice idea, but you need to take the effect on vacuum of
idle-in-xact sessions too.  If the operator left for the day and their
session doesn't block any other process, the next day you could find
some tables bloated to such extreme as to cause problems later on.
Surely the operator can review their terminal to re-do the work, in case
it was valuable.  (If it was valuable, why didn't they commit the
transaction?)


These particular databases were not subject to bloat since they were 
partitioned and append-only - no inserts or deletes whatsoever except to 
tiny dimension tables.  In general, though, you are correct.


An absolute transaction timeout would be a good first step but a 
blocking timeout would also be very handy.  It would be very applicable 
to data warehouse scenarios where bloat is controlled by other means and 
long transactions are the norm (and idle-in-transactions times can also 
be long).


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


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

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

[Catalin Iacob]

On Thu, Nov 5, 2015 at 5:27 PM, Robert Haas  wrote:
>> I wrote some text. But needs some work of native speaker.
>
> It does.  It would be nice if some kind reviewer could help volunteer
> to clean that up.

I'll give it a go sometime next week.


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

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

[Vik Fearing]

On 11/05/2015 09:01 PM, Merlin Moncure wrote:
> Tom noted earlier some caveats with the 'idle' timeout in terms of
> implementation.  Maybe that needs to be zeroed in on.

AFAIK, those issues have already been solved by Andres some time ago.
-- 
Vik Fearing  +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support


-- 
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] Proposal for \rotate in psql

[Joe Conway]

On 11/05/2015 12:56 PM, Pavel Stehule wrote:
> 2015-11-05 17:17 GMT+01:00 Joe Conway wrote:
> Hey, I resemble that remark ;-)
> 
> I am sorry, Joe - no any personal attack - I'll pay a beer for you if
> you visit Prague :)

No offense taken, but I might take you up on that beer someday ;-)

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature

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

[Adrian.Vondendriesch]

New patch attached and rebased on HEAD
(8c75ad436f75fc629b61f601ba884c8f9313c9af).

Am 03.11.2015 um 04:06 schrieb Robert Haas:
> On Sat, Oct 31, 2015 at 2:25 PM, Julien Rouhaud
>  wrote:
>> I just reviewed your patch, everything looks fine for me. Maybe some
>> minor cosmetic changes could be made to avoid declaring too many vars,
>> but I think a committer would have a better idea on this, so I mark
>> this patch as ready for committer.
> 
> I don't think we should define Sign(x) as a macro in c.h. c.h is
> really only supposed to contain stuff that's pretty generic and
> universal, and the fact that we haven't needed it up until now
> suggests that Sign(x) isn't.  I'd suggest instead defining something
> like:
> 
> #define half_rounded(x)   (((x) + (x) < 0 ? 0 : 1) / 2)

I removed the Sign macro and introduced half_rounded. It's placed it
in dbsize.c.  Please let me know if that's the wrong place.

> 
> Maybe rename numeric_divide_by_two to numeric_half_rounded.
> Generally, let's try to make the numeric and int64 code paths look as
> similar as possible.

I renamed numeric_divide_by_two.

> 
> Recomputing numeric_absolute(x) multiple times should be avoided.
> Compute it once and save the answer.

Because "size" is shifted multiple times within pg_size_pretty and
pg_size_pretty_numeric the absolute values needs to be recomputed.
Please let me know if I oversee something.

I changed the status back to "needs review".

Regards,
 - Adrian
From c06d1463cf21957260327c47217050e334058422 Mon Sep 17 00:00:00 2001
From: Adrian Vondendriesch 
Date: Sat, 19 Sep 2015 15:54:13 +0200
Subject: [PATCH] Make pg_size_pretty handle negative values.

Make pg_size_pretty(bigint) and pg_size_pretty(numeric) handle
negative values the same way as positive values are.

This commit introduces a new macro "Sign", which is used within
pg_size_pretty(bigint).  Also numeric_plus_one_over_two is renamed to
numeric_divide_by_two.  numeric_divide_by_two now handles negative
values the same way as positive values are handled.  To get the
absolute value of a Numeric variable, a new static function called
numeric_absolute is introduced.
---
 src/backend/utils/adt/dbsize.c | 67 ---
 .../regress/expected/pg_size_pretty_bigint.out | 39 +++
 .../regress/expected/pg_size_pretty_numeric.out| 75 ++
 src/test/regress/sql/pg_size_pretty_bigint.sql | 15 +
 src/test/regress/sql/pg_size_pretty_numeric.sql| 29 +
 5 files changed, 203 insertions(+), 22 deletions(-)
 create mode 100644 src/test/regress/expected/pg_size_pretty_bigint.out
 create mode 100644 src/test/regress/expected/pg_size_pretty_numeric.out
 create mode 100644 src/test/regress/sql/pg_size_pretty_bigint.sql
 create mode 100644 src/test/regress/sql/pg_size_pretty_numeric.sql

diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c
index 82311b4..84c67d3 100644
--- a/src/backend/utils/adt/dbsize.c
+++ b/src/backend/utils/adt/dbsize.c
@@ -31,6 +31,12 @@
 #include "utils/relmapper.h"
 #include "utils/syscache.h"
 
+/*
+ * half_rounded
+ * Return (x / 2) if x is smaller than 0
+ * ((x + 1) / 2) otherwise.
+ */
+#define half_rounded(x)   (((x) + ((x) < 0 ? 0 : 1)) / 2)
 
 /* Return physical size of directory contents, or 0 if dir doesn't exist */
 static int64
@@ -534,31 +540,31 @@ pg_size_pretty(PG_FUNCTION_ARGS)
 	int64		limit = 10 * 1024;
 	int64		limit2 = limit * 2 - 1;
 
-	if (size < limit)
+	if (Abs(size) < limit)
 		snprintf(buf, sizeof(buf), INT64_FORMAT " bytes", size);
 	else
 	{
 		size >>= 9;/* keep one extra bit for rounding */
-		if (size < limit2)
+		if (Abs(size) < limit2)
 			snprintf(buf, sizeof(buf), INT64_FORMAT " kB",
-	 (size + 1) / 2);
+	 half_rounded(size));
 		else
 		{
 			size >>= 10;
-			if (size < limit2)
+			if (Abs(size) < limit2)
 snprintf(buf, sizeof(buf), INT64_FORMAT " MB",
-		 (size + 1) / 2);
+		 half_rounded(size));
 			else
 			{
 size >>= 10;
-if (size < limit2)
+if (Abs(size) < limit2)
 	snprintf(buf, sizeof(buf), INT64_FORMAT " GB",
-			 (size + 1) / 2);
+			 half_rounded(size));
 else
 {
 	size >>= 10;
 	snprintf(buf, sizeof(buf), INT64_FORMAT " TB",
-			 (size + 1) / 2);
+			 half_rounded(size));
 }
 			}
 		}
@@ -593,16 +599,37 @@ numeric_is_less(Numeric a, Numeric b)
 }
 
 static Numeric
-numeric_plus_one_over_two(Numeric n)
+numeric_absolute(Numeric n)
+{
+	Datum		d = NumericGetDatum(n);
+	Datum		result;
+
+	result = DirectFunctionCall1(numeric_abs, d);
+	return DatumGetNumeric(result);
+}
+
+static Numeric
+numeric_half_rounded(Numeric n)
 {
 	Datum		d = NumericGetDatum(n);
+	Datum		zero;
 	Datum		one;
 	Datum		two;
 	Datum		result;
+	bool		greater_or_equal_zero;
 
+	zero = DirectFunctionCall1(int8_numeric, Int64GetDatum(0));
 	one = DirectFunctionCall1(int8_numeric, 

Re: [HACKERS] extend pgbench expressions with functions

[Robert Haas]

On Thu, Nov 5, 2015 at 3:33 PM, Fabien COELHO  wrote:
> For the '+' overload operator with conversions there are 4 cases (2
> arguments ** 2 types) to handle. For all 5 binary operators (+ - * / %).
> that makes 20 cases to handle. Then for every function, you have to deal
> with type conversion as well, each function times number of arguments **
> number of types. That is 2 cases for abs, 4 cases for random, 8 cases for
> each random_exp*, 8 for random_gaus*, and so on. Some thinking would be
> required for n-ary functions (min & max).
>
> Basically, it can be done, no technical issue, it is just a matter of
> writing a *lot* of repetitive code, hundreds of lines of them. As I think it
> does not bring any value for pgbench purpose, I used the other approach
> which reduces the code size by avoiding the combinatorial "cross-type"
> conversions.

Those can be avoided in other ways.  For example:

   if (x->type == PGBT_INTEGER && y->type == PGBT_INTEGER)
   {
   result->type = PGBT_INTEGER;
   result->u.ival = x->u.ival + y->u.ival;
   }
   else
   {
   result->type = PGBT_DOUBLE;
   result->u.ival = coerceToDouble(x) + coerceToDouble(y);
   }

coerceToDouble can re-used for every arithmetic operator and can throw
an error if the input type is not coercible.  Yet, we still return an
exact integer answer when 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

[HACKERS] Brain fade in gin_extract_jsonb_path()

[Tom Lane]

I looked into bug #13756,
http://www.postgresql.org/message-id/20151105171933.14035.25...@wrigleys.postgresql.org

The cause of the problem is that gin_extract_jsonb_path() computes
a different hash for the "2" in
{"a":[ ["b",{"x":1}], ["b",{"x":2}]]}
than it does for the "2" in
{"a":[[{"x":2}]]}
And the cause of that is that it supposes that after emitting a hash
for a VALUE, it can just leave stack->hash to be reinitialized by the
next KEY or ELEM.  But if the next thing is a sub-object, we propagate
the previous value's hash down into the sub-object, causing values
therein to receive different hashes than they'd have gotten without
the preceding outer-level VALUE.

The attached one-liner fixes it, but of course this means that on-disk
jsonb_path_ops indexes are possibly broken and will need to be reindexed.
I see no way around that ... does anybody else?

regards, tom lane

diff --git a/src/backend/utils/adt/jsonb_gin.c b/src/backend/utils/adt/jsonb_gin.c
index 204fb8b..b917ec2 100644
*** a/src/backend/utils/adt/jsonb_gin.c
--- b/src/backend/utils/adt/jsonb_gin.c
*** gin_extract_jsonb_path(PG_FUNCTION_ARGS)
*** 419,425 
  JsonbHashScalarValue(, >hash);
  /* and emit an index entry */
  entries[i++] = UInt32GetDatum(stack->hash);
! /* Note: we assume we'll see KEY before another VALUE */
  break;
  			case WJB_END_ARRAY:
  			case WJB_END_OBJECT:
--- 419,426 
  JsonbHashScalarValue(, >hash);
  /* and emit an index entry */
  entries[i++] = UInt32GetDatum(stack->hash);
! /* reset hash for next sub-object */
! stack->hash = stack->parent->hash;
  break;
  			case WJB_END_ARRAY:
  			case WJB_END_OBJECT:

-- 
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: multiple psql option -c

[Robert Haas]

On Thu, Nov 5, 2015 at 3:53 PM, Catalin Iacob  wrote:
> On Thu, Nov 5, 2015 at 5:27 PM, Robert Haas  wrote:
>>> I wrote some text. But needs some work of native speaker.
>>
>> It does.  It would be nice if some kind reviewer could help volunteer
>> to clean that up.
>
> I'll give it a go sometime next week.

Thanks, that would be great!

I recommend comparing the section on -c and the section on -C, and
probably updating the former as well as adjusting the wording of the
latter.  We don't want to repeat all the same details in both places,
but we hopefully want to give people a little clue that if they're
thinking about using -c, they may wish to instead consider -C.

-- 
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] Brain fade in gin_extract_jsonb_path()

[Tom Lane]

Peter Geoghegan  writes:
> On Thu, Nov 5, 2015 at 12:39 PM, Tom Lane  wrote:
>> The attached one-liner fixes it, but of course this means that on-disk
>> jsonb_path_ops indexes are possibly broken and will need to be reindexed.
>> I see no way around that ... does anybody else?

> I think it's impossible to avoid having to recommend a reindex here.

Good, because that's not the only bug here :-(

Extending the example with

insert into t (j) values ('[[14,2,3]]');
insert into t (j) values ('[1,[14,2,3]]');

we get this with seqscan on:

regression=# select * from t where j @> '[[14]]';
j
-
 [[14, 2, 3]]
 [1, [14, 2, 3]]
(2 rows)

which is the right answer, and this with seqscan off:

regression=# select * from t where j @> '[[14]]';
  j   
--
 [[14, 2, 3]]
(1 row)

which is not so much.

The reason for that is that when processing an outer-level WJB_BEGIN_XXX,
we initialize stack->hash to something that is different from
stack->parent->hash.  This is a waste of time in unnested cases because
when we hit the first WJB_KEY or WJB_ELEM, we'll reset stack->hash to
stack->parent->hash.  However, if we have a nested array, then processing
of elements inside that array treats stack->parent->hash as the reference
value, so that we compute different hash values than we would have at
outer level.  What's worse, it matters whether the nested array is the
first subobject or not, as shown by the above example; that's because once
we reset stack->hash to equal stack->parent->hash, hashes for later
elements will be inconsistent with the special-case initialization.
(Note: that explains why this is broken with my previous patch.  It fails
in much the same way with unmodified HEAD, but I think for slightly
different reasons, which I've not bothered to understand in detail.)

I think the easiest way to fix this is to forget about the special
initialization at outer level and just always initialize a new stack level
to have the same hash as its parent.  That leads to the first patch below
--- but once you look at that, you realize that we've got unnecessarily
many copies of stack->parent->hash into stack->hash, so what I actually
propose now is the second patch below.

I think that this only changes the hashes calculated for nested-array
cases, but it's still a larger change footprint than the previous patch.
Not that that affects much; a reindex is a reindex, and a bug is a bug,
so we don't have much wiggle room.

regards, tom lane

diff --git a/src/backend/utils/adt/jsonb_gin.c b/src/backend/utils/adt/jsonb_gin.c
index 204fb8b..9172268 100644
*** a/src/backend/utils/adt/jsonb_gin.c
--- b/src/backend/utils/adt/jsonb_gin.c
*** gin_extract_jsonb_path(PG_FUNCTION_ARGS)
*** 375,406 
  parent = stack;
  stack = (PathHashStack *) palloc(sizeof(PathHashStack));
  
! if (parent->parent)
! {
! 	/*
! 	 * We pass forward hashes from previous container nesting
! 	 * levels so that nested arrays with an outermost nested
! 	 * object will have element hashes mixed with the
! 	 * outermost key.  It's also somewhat useful to have
! 	 * nested objects' innermost values have hashes that are a
! 	 * function of not just their own key, but outer keys too.
! 	 *
! 	 * Nesting an array within another array will not alter
! 	 * innermost scalar element hash values, but that seems
! 	 * inconsequential.
! 	 */
! 	stack->hash = parent->hash;
! }
! else
! {
! 	/*
! 	 * At the outermost level, initialize hash with container
! 	 * type proxy value.  Note that this makes JB_FARRAY and
! 	 * JB_FOBJECT part of the on-disk representation, but they
! 	 * are that in the base jsonb object storage already.
! 	 */
! 	stack->hash = (r == WJB_BEGIN_ARRAY) ? JB_FARRAY : JB_FOBJECT;
! }
  stack->parent = parent;
  break;
  			case WJB_KEY:
--- 375,390 
  parent = stack;
  stack = (PathHashStack *) palloc(sizeof(PathHashStack));
  
! /*
!  * We pass forward hashes from outer nesting levels so that
!  * the hashes for nested values will include outer keys as
!  * well as their own keys.
!  *
!  * Nesting an array within another array will not alter
!  * innermost scalar element hash values, but that seems
!  * inconsequential.
!  */
! stack->hash = parent->hash;
  stack->parent = parent;
  break;
  			case WJB_KEY:
*** gin_extract_jsonb_path(PG_FUNCTION_ARGS)
*** 419,425 
  JsonbHashScalarValue(, >hash);
  /* and emit an index entry */
  entries[i++] = UInt32GetDatum(stack->hash);
! /* Note: we assume we'll see KEY before another VALUE */
  break;
  			case WJB_END_ARRAY:
  			case WJB_END_OBJECT:
--- 403,410 
  JsonbHashScalarValue(, >hash);
  /* and emit an index entry */
  entries[i++] = 

Re: [HACKERS] Within CF app, "Bug Fixes" should be "Bug Fixes/Refactoring"

[Robert Haas]

On Thu, Nov 5, 2015 at 4:53 PM, Peter Geoghegan  wrote:
> On Thu, Oct 29, 2015 at 1:29 PM, Peter Geoghegan  wrote:
>>> "Refactoring" seems rather a narrow definition of what might show up
>>> in such a category, btw.  Maybe "Code Beautification" would be a
>>> suitable title?  I'm bikeshedding though.
>>
>> I think that there is value in limiting the number of topics. But I
>> hardly but much weight on this. Any of the above are fine.
>
> Can someone follow up and push this to the CF app? "Refactoring" seems
> to be the consensus.

I guess I'm wondering whether there's really enough of this to need
its own category.

-- 
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] Some questions about the array.

[Robert Haas]

On Thu, Nov 5, 2015 at 9:57 AM, YUriy Zhuravlev
 wrote:
> Hello hackers.
> There are comments to my patch? Maybe I should create a separate thread?
> Thanks.

You should add this on commitfest.postgresql.org.

I think the first question that needs to be answered is "do we want
this?".  I'm sure I know your answer, but what do other people 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] NOTIFY in Background Worker

[Thomas Munro]

On Fri, Nov 6, 2015 at 12:08 PM, Haribabu Kommi 
wrote:

> On Fri, Nov 6, 2015 at 4:57 AM, Robert Haas  wrote:
> > On Thu, Nov 5, 2015 at 12:34 AM, Haribabu Kommi
> >  wrote:
> >> I marked this patch as ready for committer.
> >
> > The patch says:
> >
> > If a background worker registers to receive asynchronous notifications
> > with the LISTEN through SPI,
> > there is currently no way for incoming notifications to be received.
> >
> > But wouldn't it be more correct to say:
> >
> > If a background worker registers to receive asynchronous notifications
> > with the LISTEN through SPI, the
> > worker will log those notifications, but there is no programmatic way
> > for the worker to intercept and respond to those notifications.
>
> Yes, the above description is good.


+1

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

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

[Craig Ringer]

On 6 November 2015 at 13:34, Robert Haas  wrote:

>> But some options control how
>> next host should be choosen (i.e. use random order for load-balancing
>> or sequential order for high availability), so they should be specified
>> only once per connect string.
>
> But this seems like a point worthy of consideration.

This makes me think that trying to wedge this into the current API
using a funky connection string format might be a mistake.

Lots of these issues would go away if you could provide more than just
a connstring.

>> Third category of options are specified per-cluster much more often
>> than per node. But are quite often changed from compiled in default.
>
> This, too.

Like this. If you have a global set of connection options, then
per-connection options, it's a lot simpler.

I guess that can be hacked in with a more dramatic change in the
connstring format, otherwise, incorporating subsections or something.
I'm not keen on doing anything like that, but there are all sorts of
options...

"global:[user=fred port=5432] host1:[host=somehost user=user1]
host2:[host=localhost]"


(puts head in brown paper bag)

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


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

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

[Robert Haas]

On Wed, Nov 4, 2015 at 5:10 PM, Josh Berkus  wrote:
> On 11/04/2015 01:55 PM, Stephen Frost wrote:
>> * Joe Conway (m...@joeconway.com) wrote:
>>> On 11/04/2015 01:24 PM, Alvaro Herrera wrote:
 I agree with Pavel.  Having a transaction timeout just does not make any
 sense.  I can see absolutely no use for it.  An idle-in-transaction
 timeout, on the other hand, is very useful.
>>>
>>> +1 -- agreed
>>
>> I'm not sure of that.  I can certainly see a use for transaction
>> timeouts- after all, they hold locks and can be very disruptive in the
>> long run.  Further, there are cases where a transaction is normally very
>> fast and in a corner case it becomes extremely slow and disruptive to
>> the rest of the system.  In those cases, having a timeout for it is
>> valuable.
>
> I could see a use for both, having written scripts which do both.

+1.

-- 
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] Within CF app, "Bug Fixes" should be "Bug Fixes/Refactoring"

[Michael Paquier]

On Fri, Nov 6, 2015 at 1:47 PM, Robert Haas  wrote:
> On Thu, Nov 5, 2015 at 4:53 PM, Peter Geoghegan  wrote:
>> On Thu, Oct 29, 2015 at 1:29 PM, Peter Geoghegan  wrote:
 "Refactoring" seems rather a narrow definition of what might show up
 in such a category, btw.  Maybe "Code Beautification" would be a
 suitable title?  I'm bikeshedding though.
>>>
>>> I think that there is value in limiting the number of topics. But I
>>> hardly but much weight on this. Any of the above are fine.
>>
>> Can someone follow up and push this to the CF app? "Refactoring" seems
>> to be the consensus.
>
> I guess I'm wondering whether there's really enough of this to need
> its own category.

We have a category "Code comments" as well. Let's give it a shot so I
am adding it. We could always remove it later if necessary.
-- 
Michael


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

Re: [HACKERS] [PATCH] RFC: Add length parameterised dmetaphone functions

[Michael Paquier]

On Fri, Nov 6, 2015 at 2:00 PM, Christian Marie  wrote:
> This patch addresses this by adding and documenting an optional argument to
> dmetaphone and dmetaphone_alt that specifies the maximum output length. This
> makes it possible to use dmetaphone on much longer inputs.

It would be good if you could register your patch here so as other
folks can track it for review, and this way we are sure that it won't
fall into the void:
https://commitfest.postgresql.org/8/
It would be good as well if you could attach a version of this patch
directly to an email sent to this thread.

> Backwards compatibility is catered for by making the new argument optional,
> defaulting to the old, hard-coded value of four. We now have:

Preserving backward-compatibility is an essential piece. Thanks.
--
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] Beta2 Next week

[Michael Paquier]

On Fri, Nov 6, 2015 at 6:01 AM, Josh Berkus  wrote:
> We are going to try to get Beta2 wrapped on Monday for a release next
> week.  So if you're currently working on a 9.5 Open Item, getting it
> checked in tommorrow or this weekend would be great.

There is this vacuumdb issue with password handling that would be good
to get fixed:
http://www.postgresql.org/message-id/20151027193919.931.54...@wrigleys.postgresql.org
I am wrapping a patch for 9.5 and master, perhaps Alvaro (or somebody
else) could have a look at what I will send over the next couple of
days.

Another thing is this issue for pg_rewind regarding error handling:
http://www.postgresql.org/message-id/cab7npqsylo4jzp7-2hjh24yeu99tspkwj7vtj7nytgcxasw...@mail.gmail.com
This has stalled a bit though. I proposed a patch but Peter (E) did
not like it much visibly :)
Regards,
-- 
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] Some questions about the array.

[Craig Ringer]

On 6 November 2015 at 12:45, Robert Haas  wrote:
> On Thu, Nov 5, 2015 at 9:57 AM, YUriy Zhuravlev
>  wrote:
>> Hello hackers.
>> There are comments to my patch? Maybe I should create a separate thread?
>> Thanks.
>
> You should add this on commitfest.postgresql.org.
>
> I think the first question that needs to be answered is "do we want
> this?".  I'm sure I know your answer, but what do other people think?

Omitted bounds are common in other languages and would be handy. I
don't think they'd cause any issues with multi-dimensional arrays or
variable start-pos arrays.

I'd love negative indexes, but the variable-array-start (mis)feature
means we can't have those. I wouldn't shed a tear if
variable-start-position arrays were deprecated and removed, but that's
a multi-year process, and I'm not convinced negative indexes justify
it even though the moveable array start pos feature seems little-used.

Since the start-pos is recorded in the array, I wonder if it's worth
supporting negative indexing for arrays with the default 1-indexed
element numbering, and just ERRORing for others. Does anyone really
use anything else?

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


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

[HACKERS] Getting sorted data from foreign server for merge join

[Ashutosh Bapat]

Hi All,
PFA patch to get data sorted from the foreign server (postgres_fdw)
according to the pathkeys useful for merge join.

For a given base relation (extendable to join when that becomes available
in postgres_fdw), the patch tries to find merge joinable clauses. It then
adds paths with pathkeys extracted out of these merge joinable clauses. The
merge joinable clauses form equivalence classes. The patch searches in
root->eq_classes for equivalence members belonging to the given relation.
For every such expression it creates a single member pathkey list and
corresponding path. The test postgres_fdw.sql has an existing join which
uses merge join. With this patch the data is sorted on the foreign server
than locally.

While mergejoinable clauses can be obtained from rel->joininfo as well. But
rel->joininfo contains other clauses as well and we need extra efforts to
remove duplicates if the same expression appears in multiple merge joinable
clauses.

Two joining relations can have multiple merge joinable clauses, requiring
multi-member pathkeys so that merge join is efficient to the maximum
extent. The order in which the expressions appears in pathkeys can change
the costs of sorting the data according to the pathkeys, depending upon the
expressions and the presence of indexes containing those expressions. Thus
ideally we would need to club all the expressions appearing in all the
clauses for given two relations and create paths with pathkeys for every
order of these expressions.That explodes the number of possible paths. We
may restrict the number of paths created by considering only certain orders
like sort_inner_and_outer(). In any case, costing such paths increases the
planning time which may not be worth it. So, this patch uses a heuristic
approach of creating single member pathkeys path for every merge joinable
expression.

The pathkeys need to be canonicalised using make_canonical_pathkey(), which
is a static function. I have added a TODO and comments in the patch
explaining possible ways to avoid "extern"alization of this function.

Comments/suggestions are welcome.
-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 0159bf5..ddb05f2 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -3383,22 +3383,22 @@ select tableoid::regclass, * from bar order by 1,2;
  bar2 |  4 | 144
  bar2 |  7 |  77
 (6 rows)
 
 -- Check UPDATE with inherited target and an appendrel subquery
 explain (verbose, costs off)
 update bar set f2 = f2 + 100
 from
   ( select f1 from foo union all select f1+3 from foo ) ss
 where bar.f1 = ss.f1;
-  QUERY PLAN  
---
+   QUERY PLAN   
+
  Update on public.bar
Update on public.bar
Foreign Update on public.bar2
  Remote SQL: UPDATE public.loct2 SET f2 = $2 WHERE ctid = $1
->  Hash Join
  Output: bar.f1, (bar.f2 + 100), bar.ctid, (ROW(foo.f1))
  Hash Cond: (foo.f1 = bar.f1)
  ->  Append
->  Seq Scan on public.foo
  Output: ROW(foo.f1), foo.f1
@@ -3410,41 +3410,44 @@ where bar.f1 = ss.f1;
->  Foreign Scan on public.foo2 foo2_1
  Output: ROW((foo2_1.f1 + 3)), (foo2_1.f1 + 3)
  Remote SQL: SELECT f1 FROM public.loct1
  ->  Hash
Output: bar.f1, bar.f2, bar.ctid
->  Seq Scan on public.bar
  Output: bar.f1, bar.f2, bar.ctid
->  Merge Join
  Output: bar2.f1, (bar2.f2 + 100), bar2.f3, bar2.ctid, (ROW(foo.f1))
  Merge Cond: (bar2.f1 = foo.f1)
- ->  Sort
+ ->  Foreign Scan on public.bar2
Output: bar2.f1, bar2.f2, bar2.f3, bar2.ctid
-   Sort Key: bar2.f1
-   ->  Foreign Scan on public.bar2
- Output: bar2.f1, bar2.f2, bar2.f3, bar2.ctid
- Remote SQL: SELECT f1, f2, f3, ctid FROM public.loct2 FOR UPDATE
- ->  Sort
+   Remote SQL: SELECT f1, f2, f3, ctid FROM public.loct2 ORDER BY f1 ASC FOR UPDATE
+ ->  Materialize
Output: (ROW(foo.f1)), foo.f1
-   Sort Key: foo.f1
-   ->  Append
- ->  Seq Scan on public.foo
-   Output: ROW(foo.f1), foo.f1
+   ->  Merge Append
+ Sort Key: foo.f1
+ ->  Sort
+   Output: (ROW(foo.f1)), foo.f1
+   Sort Key: foo.f1
+  

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

[Robert Haas]

On Thu, Nov 5, 2015 at 10:12 AM, Victor Wagner  wrote:
> There are some drawbacks with this approach
>
> 1. You are suggesting usage of two nested loops, instead of one straight
> loop - outer loop over the URLs in the connect string, inner loop over
> the IP addresses each URL resolves into. (This inner loop already
> presents there for several versions of the postgres).

This isn't really a problem.

> 2. You are suggesting that it should be possible to specify all options
> separately for each of the fallback hosts.
>
> But some options control how
> next host should be choosen (i.e. use random order for load-balancing
> or sequential order for high availability), so they should be specified
> only once per connect string.

But this seems like a point worthy of consideration.

> Third category of options are specified per-cluster much more often
> than per node. But are quite often changed from compiled in default.
> Typically there are authentication credentials (even you use
> trigger-based replication, user information would probably be
> replicated), database name (if you use WAL-level replication), client
> encoding (which depends on more on the client than on server), etc.
>
> It would be pain if we would need specify them for each host separately.
> Syntax, implemented in the my patch allows even to specify
> nonstandard-port once for all hosts (using port= parameter in the query
> string) and override it for particular host in the list using colon
> syntax.

This, too.

> 3. Last, but not least. There is more than one postgresql client
> implementation. Apart of libpq we have jdbc, native GO language client
> etc. And I think that maintaining compatibility of the connect string
> between them is good thing. So, I've modelled syntax of multihost
> feature in the libpq URL-style syntax after jdbc syntax.

Also this.

I'm not sure what the right decision is here - I can see both sides of
it to some degree.  But I think your points deserve to be taken
seriously.

-- 
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] Request: pg_cancel_backend variant that handles 'idle in transaction' sessions

[Craig Ringer]

On 5 November 2015 at 23:10, Alvaro Herrera  wrote:
> David Steele wrote:
>
>> The important thing about this implementation was that nothing was
>> terminated unless it had exceed a timeout AND was blocking another
>> process.
>
> This seems a nice idea, but you need to take the effect on vacuum of
> idle-in-xact sessions too.  If the operator left for the day and their
> session doesn't block any other process, the next day you could find
> some tables bloated to such extreme as to cause problems later on.

The additional qualifier "and isn't pinning xmin" would probably be
useful there. Often it's pretty harmless to keep an xact open for
ages, the problem has, until the recent changes in pg_stat_activity,
been knowing when.

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


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

Re: [HACKERS] Some bugs in psql_complete of psql

[Kyotaro HORIGUCHI]

Hello, thank you for the comments.

The revised version of this patch is attached.

 - Prevent complete with ON to the sequence "[CREATE] [UNIQUE] INDEX ON".
 - Added TABLESPACE to the preposition list for SECURITY LABEL.

I think that the current completion mechanism is simple, fast and
maybe enough but lacks of flexibility for optional items and
requires extra attention for false match.


At Fri, 6 Nov 2015 00:26:44 +0900, Fujii Masao  wrote in 

> On Wed, Nov 4, 2015 at 5:27 PM, Kyotaro HORIGUCHI
>  wrote:
> > Hello, I found that a typo(?) in tab-complete.c.
> >
> >> /* ALTER TABLE,INDEX,MATERIALIZED VIEW xxx ALL IN TABLESPACE xxx OWNED BY 
> >> */
> >> else if (pg_strcasecmp(prev6_wd, "ALL") == 0 &&
> >>pg_strcasecmp(prev5_wd, "IN") == 0 &&
> >>pg_strcasecmp(prev4_wd, "TABLESPACE") == 0 &&
> >>pg_strcasecmp(prev2_wd, "OWNED") == 0 &&
> >>pg_strcasecmp(prev4_wd, "BY") == 0)
> >
> > "BY" is compared against the word in wrong position and it
> > prevents this completion from matching.
> >
> > I also found some other bugs in psql-completion. The attached
> > patch applied on master and fixes them all togher.
> 
> + /* If we have INDEX CONCURRENTLY , then add exiting indexes */
> + else if (pg_strcasecmp(prev2_wd, "INDEX") == 0 &&
> + pg_strcasecmp(prev_wd, "CONCURRENTLY") == 0)
> + COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_indexes, NULL);
> 
> Is this for DROP INDEX CONCURRENTLY case?
> If yes, we should check that prev3_wd is "DROP".

No. Both for CREATE/DROP, their syntax is as follows ingoring
optional "IF [NOT] EXITS" and "UNIQUE".  "ALTER INDEX" doesn't
take CONCURRENTLY.

 DROP INDEX [CONCURRENTLY] name ...
 CREATE INDEX [CONCURRENTLY] name ...

> + /* If we have CREATE|UNIQUE INDEX [CONCURRENTLY] , then add "ON" */
> + else if (((pg_strcasecmp(prev3_wd, "CREATE") == 0 ||
> +   pg_strcasecmp(prev3_wd, "UNIQUE") == 0) &&
> +  pg_strcasecmp(prev2_wd, "INDEX") == 0 &&
> +  pg_strcasecmp(prev_wd, "CONCURRENTLY") != 0) ||
> 
> The "!= 0" in the above last condition should be "== 0" ?

No. It intends to match the case where prev_wd is not
CONCURRENTLY but some name for the index to be created.  As
writing this answer, I noticed that the index name is
optional. For such case, this completion rule completes with ON
after "INDEX ON". It should be fixed as the following.

> + else if (((pg_strcasecmp(prev3_wd, "CREATE") == 0 ||
> +   pg_strcasecmp(prev3_wd, "UNIQUE") == 0) &&
> +  pg_strcasecmp(prev2_wd, "INDEX") == 0 &&
> +  pg_strcasecmp(prev_wd, "CONCURRENTLY") != 0 &&
> +  pg_strcasecmp(prev_wd, "ON") != 0) ||

The "CONCURRENTLY" case is matched by the condition after the
'||' at the tail in the codelet cited just above..

+   ((pg_strcasecmp(prev4_wd, "CREATE") == 0 ||
+ pg_strcasecmp(prev4_wd, "UNIQUE") == 0) &&
+pg_strcasecmp(prev3_wd, "INDEX") == 0 &&
+pg_strcasecmp(prev2_wd, "CONCURRENTLY") == 0))

> + {"TABLE", "COLUMN", "AGGREGATE", "DATABASE", "DOMAIN",
> + "EVENT TRIGGER", "FOREIGN TABLE", "FUNCTION", "LARGE OBJECT",
> + "MATERIALIZED VIEW", "LANGUAGE", "ROLE", "SCHEMA",
> + "SEQUENCE", "TYPE", "VIEW", NULL};
> 
> TABLESPACE also should be added to the list?

Mmm... I don't understand what the patch attached to my first
mail is.. Yes, the list is missing TABLESPACE which exists in my
branch. It is surely contained in the revised patch.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
>From 7b42a01556ef2ea2291e1d4748a9bf0f80046069 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi 
Date: Mon, 2 Nov 2015 14:10:53 +0900
Subject: [PATCH] Fix some tab-completino bugs.

  - 'ALTER [TABLE|xx] xxx ALL IN TABLE SPACE xx OWNED BY' cannot be completed.
  - CONCURRENTLY in CREATE INDEX is misplaced.
  - Preposition list of SECURITY LABEL is not terminated.
  - The target list of SECURITY LABEL ON misses some alternatives.
---
 src/bin/psql/tab-complete.c | 55 +++--
 1 file changed, 28 insertions(+), 27 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 0e8d395..5a3930f 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1040,7 +1040,7 @@ psql_completion(const char *text, int start, int end)
 			 pg_strcasecmp(prev5_wd, "IN") == 0 &&
 			 pg_strcasecmp(prev4_wd, "TABLESPACE") == 0 &&
 			 pg_strcasecmp(prev2_wd, "OWNED") == 0 &&
-			 pg_strcasecmp(prev4_wd, "BY") == 0)
+			 pg_strcasecmp(prev_wd, "BY") == 0)
 	{
 		COMPLETE_WITH_QUERY(Query_for_list_of_roles);
 	}
@@ -2320,35 +2320,35 @@ psql_completion(const char *text, int start, int end)
 		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_indexes,
    " UNION SELECT 'ON'"
    " UNION SELECT 'CONCURRENTLY'");
+	/* If we have INDEX CONCURRENTLY , then add exiting indexes */
+	else if 

Re: [HACKERS] Parallel Seq Scan

[Robert Haas]

On Thu, Nov 5, 2015 at 10:49 PM, Amit Kapila  wrote:
> On Thu, Nov 5, 2015 at 11:54 PM, Robert Haas  wrote:
>> I was thinking about this idea:
>>
>> 1. Add a parallel_aware flag to each plan.
>
> Okay, so shall we add it in generic Plan node or to specific plan nodes
> like SeqScan, IndexScan, etc.  To me, it appears that parallelism is
> a node specific property, so we should add it to specific nodes and
> for now as we are parallelising seq scan, so we can add this flag in
> SeqScan node.  What do you say?

I think it should go in the Plan node itself.  Parallel Append is
going to need a way to test whether a node is parallel-aware, and
there's nothing simpler than if (plan->parallel_aware).  That makes
life simple for EXPLAIN, too.

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


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

[HACKERS] [PATCH] RFC: Add length parameterised dmetaphone functions

[Christian Marie]

A developer I work with was trying to use dmetaphone to group people names into
equivalence classes. He found that many long names would be grouped together
when they shouldn't be, this turned out to be because dmetaphone has an
undocumented upper bound on its output length, of four. This is obviously
impractical for many use cases.

This patch addresses this by adding and documenting an optional argument to
dmetaphone and dmetaphone_alt that specifies the maximum output length. This
makes it possible to use dmetaphone on much longer inputs.

Backwards compatibility is catered for by making the new argument optional,
defaulting to the old, hard-coded value of four. We now have:

dmetaphone(text source) returns text
dmetaphone(text source, int max_output_length) returns text
dmetaphone_alt(text source) returns text
dmetaphone_alt(text source, int max_output_length) returns text

---
 contrib/fuzzystrmatch/Makefile|  2 +-
 contrib/fuzzystrmatch/dmetaphone.c| 46 ++--
 contrib/fuzzystrmatch/fuzzystrmatch--1.0--1.1.sql | 12 ++
 contrib/fuzzystrmatch/fuzzystrmatch--1.0.sql  | 44 ---
 contrib/fuzzystrmatch/fuzzystrmatch--1.1.sql  | 52 +++
 contrib/fuzzystrmatch/fuzzystrmatch.control   |  2 +-
 doc/src/sgml/fuzzystrmatch.sgml   | 18 ++--
 7 files changed, 115 insertions(+), 61 deletions(-)
 create mode 100644 contrib/fuzzystrmatch/fuzzystrmatch--1.0--1.1.sql
 delete mode 100644 contrib/fuzzystrmatch/fuzzystrmatch--1.0.sql
 create mode 100644 contrib/fuzzystrmatch/fuzzystrmatch--1.1.sql

diff --git a/contrib/fuzzystrmatch/Makefile b/contrib/fuzzystrmatch/Makefile
index 0327d9510a50..fee85ad1ec36 100644
--- a/contrib/fuzzystrmatch/Makefile
+++ b/contrib/fuzzystrmatch/Makefile
@@ -4,7 +4,7 @@ MODULE_big = fuzzystrmatch
 OBJS = fuzzystrmatch.o dmetaphone.o $(WIN32RES)
 
 EXTENSION = fuzzystrmatch
-DATA = fuzzystrmatch--1.0.sql fuzzystrmatch--unpackaged--1.0.sql
+DATA = fuzzystrmatch--1.1.sql fuzzystrmatch--unpackaged--1.0.sql 
fuzzystrmatch--1.0--1.1.sql
 PGFILEDESC = "fuzzystrmatch - similarities and distance between strings"
 
 ifdef USE_PGXS
diff --git a/contrib/fuzzystrmatch/dmetaphone.c 
b/contrib/fuzzystrmatch/dmetaphone.c
index 147c8501ee89..7afcf0aee841 100644
--- a/contrib/fuzzystrmatch/dmetaphone.c
+++ b/contrib/fuzzystrmatch/dmetaphone.c
@@ -115,10 +115,28 @@ The remaining code is authored by Andrew Dunstan 
 and
 #include 
 
 /* prototype for the main function we got from the perl module */
-static void DoubleMetaphone(char *, char **);
+static void DoubleMetaphone(char *, char **, int);
 
 #ifndef DMETAPHONE_MAIN
 
+/* Fetch and validate the optional second function argument, which should be a
+ * maximum length for output
+ *
+ * Defaults to 4.
+ *
+ * Introduced in 1.1
+ */
+inline int
+fetch_max_len_arg(PG_FUNCTION_ARGS)
+{
+   int max_len = (PG_NARGS() > 1) ? PG_GETARG_INT32(1) : 4;
+   if(max_len < 1)
+   ereport(ERROR,
+   (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+   errmsg("invalid max output length, should be >= 1")));
+   return max_len;
+}
+
 /*
  * The PostgreSQL visible dmetaphone function.
  */
@@ -128,6 +146,7 @@ PG_FUNCTION_INFO_V1(dmetaphone);
 Datum
 dmetaphone(PG_FUNCTION_ARGS)
 {
+   int max_len;
text   *arg;
char   *aptr,
   *codes[2],
@@ -139,8 +158,9 @@ dmetaphone(PG_FUNCTION_ARGS)
 #endif
arg = PG_GETARG_TEXT_P(0);
aptr = text_to_cstring(arg);
+   max_len = fetch_max_len_arg(fcinfo);
 
-   DoubleMetaphone(aptr, codes);
+   DoubleMetaphone(aptr, codes, max_len);
code = codes[0];
if (!code)
code = "";
@@ -157,6 +177,7 @@ PG_FUNCTION_INFO_V1(dmetaphone_alt);
 Datum
 dmetaphone_alt(PG_FUNCTION_ARGS)
 {
+   int max_len;
text   *arg;
char   *aptr,
   *codes[2],
@@ -168,8 +189,9 @@ dmetaphone_alt(PG_FUNCTION_ARGS)
 #endif
arg = PG_GETARG_TEXT_P(0);
aptr = text_to_cstring(arg);
+   max_len = fetch_max_len_arg(fcinfo);
 
-   DoubleMetaphone(aptr, codes);
+   DoubleMetaphone(aptr, codes, max_len);
code = codes[1];
if (!code)
code = "";
@@ -390,7 +412,7 @@ MetaphAdd(metastring *s, char *new_str)
 
 
 static void
-DoubleMetaphone(char *str, char **codes)
+DoubleMetaphone(char *str, char **codes, int max_len)
 {
int length;
metastring *original;
@@ -427,7 +449,7 @@ DoubleMetaphone(char *str, char **codes)
}
 
/* main loop */
-   while ((primary->length < 4) || (secondary->length < 4))
+   while ((primary->length < max_len) || (secondary->length < max_len))
{
if (current >= length)
break;
@@ -1410,11 +1432,11 @@ 

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

[Kyotaro HORIGUCHI]

Hello,

At Fri, 6 Nov 2015 09:49:30 +0530, Amit Kapila  wrote 
in 
> Apart from other problems discussed, I think it could also lead to
> a performance penality for the cases when the qual condition is
> costly as evaluating such a qual against lot of dead rows would be a
> time consuming operation.  I am not sure, but I think some of the
> other well know databases might not have any such problems as
> they store visibility info inside index due to which they don't need to
> fetch the heap tuple for evaluating such quals.

I was junst thinking of the same thing. Can we estimate the
degree of the expected penalty using heap statistics? Of couse
not so accurate though.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



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

Re: [HACKERS] NOTIFY in Background Worker

[Robert Haas]

On Thu, Nov 5, 2015 at 11:46 PM, Thomas Munro
 wrote:
>> Yes, the above description is good.
>
> +1

Committed that way, then.

-- 
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] Some questions about the array.

[David G. Johnston]

On Thursday, November 5, 2015, Craig Ringer  wrote:

> On 6 November 2015 at 12:45, Robert Haas  > wrote:
> > On Thu, Nov 5, 2015 at 9:57 AM, YUriy Zhuravlev
> > > wrote:
> >> Hello hackers.
> >> There are comments to my patch? Maybe I should create a separate thread?
> >> Thanks.
> >
> > You should add this on commitfest.postgresql.org.
> >
> > I think the first question that needs to be answered is "do we want
> > this?".  I'm sure I know your answer, but what do other people think?
>
> Omitted bounds are common in other languages and would be handy. I
> don't think they'd cause any issues with multi-dimensional arrays or
> variable start-pos arrays.
>
> I'd love negative indexes, but the variable-array-start (mis)feature
> means we can't have those. I wouldn't shed a tear if
> variable-start-position arrays were deprecated and removed, but that's
> a multi-year process, and I'm not convinced negative indexes justify
> it even though the moveable array start pos feature seems little-used.
>
> Since the start-pos is recorded in the array, I wonder if it's worth
> supporting negative indexing for arrays with the default 1-indexed
> element numbering, and just ERRORing for others. Does anyone really
> use anything else?
>
>
Does it have to be "negative"?

Would something like array[1:~1] as a syntax be acceptable to denote
backward counting?

David J.

Re: [HACKERS] pgbench gaussian/exponential docs improvements

[Robert Haas]

On Thu, Nov 5, 2015 at 10:36 AM, Fabien COELHO  wrote:
> After some more thoughts, ISTM that this is not exactly a CFD because of the
> truncations, so I just named it "f" to be on the safe side.

Was there supposed to be a patch attached here?

-- 
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 support on readfuncs.c

[Kouhei Kaigai]

> On Wed, Nov 4, 2015 at 10:13 AM, Kouhei Kaigai  wrote:
> > Sorry for my late.
> >
> > I confirmed that CustomScan support of readfuncs.c works fine using the
> > attached patch for the PG-Strom tree. It worked as expected - duplication,
> > serialization and deserialization by:
> >   plan_dup = stringToNode(nodeToString(copyObject(plan_orig)))
> >
> > So, the custom-scan-on-readfuncs.v1.path itself is good for me.
> 
> I don't think this is is what we want.  In particular, I like this:
> 
> Plan trees must be able to be duplicated using copyObject,
> -   so all the data stored within the custom fields must consist of
> -   nodes that that function can handle.  Furthermore, custom scan providers
> -   cannot substitute a larger structure that embeds
> -   a CustomScan for the structure itself, as would be possible
> -   for a CustomPath or CustomScanState.
> +   serialized using nodeToString and deserialized using
> +   stringToNode, so so all the data stored within
> +   the custom fields must consist of nodes that that function
> +   can handle.
> +   Furthermore, custom scan providers have to implement optional
> +   callbacks if it defines substitute a larger structure that embeds
> +   a CustomScan for the structure itself.
> +  
> 
> The purposes of this patch is supposedly to add readfuncs.c support to
> custom scans - I agree with that goal.  This documentation change is
> talking about something else, namely using a CustomScan as the fist
> field in a larger structure.  I'm not sure I agree with that goal, and
> in any case it seems like it ought to be a separate patch.  This patch
> would be a lot smaller if it weren't trying to make that change too.
>
Hmm. I might mix up two individual discussions here.
OK, I'll cut off the minimum portion for readfuncs.c support.
The other portion - to use a CustomScan as the first field in a larger
structure - shall be submitted as a separate one.

> I also don't think that this patch ought to introduce
> get_current_library_filename().  There are other cases - e.g. for
> launching background workers - where such a thing could be used, but
> we haven't introduced it up until now.  It isn't a requirement to get
> readfuncs support working.
>
Even though it isn't a minimum requirement (because extension can put
arbitrary cstring here), it can tell reasonably reliable pathname for
extensions if backend tracks filename of the library to be loaded.

Extension knows its name; its Makefile can embed module name somewhere,
for example. However, we cannot ensure use always install the modules
under the $libdir. Even if "$libdir/pg_strom" is expected, we have no
mechanism to prohibit users to install them under the "$libdir/buggy"
directory. In this case, "pg_strom" is not a right name to solve the
library path.

Even though glibc has dladdr(3) to know the filename that contains the
target symbol, it is not a portable way. So, I thought it is the best
way to inform extension by the core, on _PG_init() time where all the
extension get control once on loading.

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] patch for geqo tweaks

[Nathan Wagner]

On Wed, Nov 04, 2015 at 12:51:52PM -0500, Tom Lane wrote:

> As for the second part, I had to look up Fisher-Yates ;-) but after
> having read Wikipedia's entry about it I think this is a good change.
> The code's shorter and more efficient, and it should mathematically
> provide an equally-unbiased initial shuffle.  It could do with a
> better comment, and I'd be inclined to handle the first element
> outside the loop rather than uselessly computing geqo_randint(0,0),
> but those are trivial changes.

I see you committed a modified version of my patch in commit
59464bd6f928ad0da30502cbe9b54baec9ca2c69.

You changed the tour[0] to be hardcoded to 1, but it should be any of
the possible gene numbers from 0 to remainder.  If you want to pull the
geqo_randint(0,0) out of the loop, it would be the last element, not the
first (i.e. where remainder == 0).

We might be able to just skip the last swap, and the loop could be

for (i=0; i < num_gene-1; i++) {

but I'd need to re-read the details of the Fisher-Yates algorithm to be
sure.  It may be that the last swap needs to happen for the shuffle to
be fully random.  In any case, tour[0] certainly shouldn't be hardcoded
to 1.

-- 
nw


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

Re: [HACKERS] Foreign join pushdown vs EvalPlanQual

[Kyotaro HORIGUCHI]

Hello,

At Thu, 5 Nov 2015 01:58:00 +, Kouhei Kaigai  wrote 
in <9a28c8860f777e439aa12e8aea7694f801162...@bpxm15gp.gisp.nec.co.jp>
> > So, as the third way, I propose to resurrect the abandoned
> > ForeinJoinState seems to be for the unearthed requirements. FDW
> > returns ForeignJoinPath, not ForeignScanPath then finally it
> > becomes ForeignJoinState, which is handeled as a join node with
> > no doubt.
> > 
> > What do you think about this?
> >
> Apart from EPQ issues, it is fundamentally impossible to reflect
> the remote join tree on local side, because remote server runs
> the partial join in their best or arbitrary way.
> If this ForeignJoinState has just a compatible join sub-tree, what
> is the difference from the alternative local join sub-plan?

I think the ForeignJoinState don't have subnodes and might has no
difference in its structure from ForeignScanState. Its
significant difference from ForeignScanState would be that the
core can properly handle the return from the node as a joined
tuple in ordinary way. Executor no more calls ExecScan for joined
tuples again.

> Even if we have another node, the roles of FDW driver is unchanged.
> It eventually needs to do them:
>  1. Recheck scan-qualifier of base foreign table
>  2. Recheck join-clause of remote joins
>  3. Reconstruct a joined tuple

Yes, the most significant point of this proposal is in not FDW
side but core side. 

> I try to estimate your intention...
> You say that ForeignScan with scanrelid==0 is not a scan actually,
> so it is problematic to call ExecScan on ExecForeignScan always.
> Thus, individual ForeignJoin shall be defined.
> Right?

Definitely.

> In case of scanrelid==0, it performs like a scan on pseudo relation
> that has record type defined by fdw_scan_tlist. The rows generated
> with this node are consists of rows in underlying base relations.
> A significant point is, FDW driver is responsible to generate the
> rows according to the fdw_scan_tlist. Once FDW driver generates rows,
> ExecScan() runs remaining tasks - execution of host clauses (although
> it is not easy to image remote join includes host clause has cheaper
> cost than others) and projection.

Agreed. The role of FDW won't be changed by introducing
ForeignJoin.

> One thing I can agree is, ForeignScan is enforced to use ExecScan,
> thus some FDW driver may concern about this hard-wired logic.
> If we try to make ForeignScan unbound from the ExecScan, I like to
> suggest to revise ExecForeignScan, just invoke a callback; then
> FDW driver can choose whether ExecScan is best or not.

Agreed. Calling ExecScan unconditionally from ForeignScan is the
cause of the root(?) cause I mentioned. Since there'd be no
difference in data structure between Foreign(Join), calling
fdwroutine->ExecForeignScan() or something instaed of ExecScan()
from ExecForeignScan could be the alternative and most promising
solution for all problems in focus now.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



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

Re: [HACKERS] GIN data corruption bug(s) in 9.6devel

[Tomas Vondra]

Hi,

On 11/06/2015 01:05 AM, Jeff Janes wrote:

On Thu, Nov 5, 2015 at 3:50 PM, Tomas Vondra
 wrote:

...


I can do that - I see there are three patches in the two threads:

   1) gin_pending_lwlock.patch (Jeff Janes)
   2) gin_pending_pagelock.patch (Jeff Janes)
   3) gin_alone_cleanup-2.patch (Teodor Sigaev)

Should I test all of them? Or is (1) obsoleted by (2) for example?


1 is obsolete.  Either 2 or 3 should fix the bug, provided this is the
bug you are seeing.  They have different performance side effects, but
as far as fixing the bug they should be equivalent.


OK, I'll do testing with those two patches then, and I'll also note the 
performance difference (the data load was very stable). Of course, it's 
just one particular workload.


I'll post an update after the weekend.

regards

--
Tomas Vondra  http://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] psql completion for ids in multibyte string

[Kyotaro HORIGUCHI]

Hi. Thank you for the comments.

The revised version is attaced.

 - A typo is fixed in the comment for PQmblen().
 - Do the same fix to PQdsplen().



At Thu, 5 Nov 2015 18:32:59 +0900, Amit Langote  
wrote in <563b224b.3020...@lab.ntt.co.jp>
> On 2015/11/05 18:10, Kyotaro HORIGUCHI wrote:
> > Hello. I don't know whether this is a bug fix or improvement,
> 
> Would it be 50-50? :-)

Yeah, honestly saying, I doubt that this is valuable but feel
uneasy to see some of the candidates vanish as completon proceeds
for no persuasive reason.

> In the following change,
> 
> - * returns the byte length of the word beginning s, using the
> - * specified encoding.
> + * returns the byte length of the character beginning s, using the specified
> + * encoding.
> 
> 
> Just a minor nitpick -
> 
> ... character beginning *at* s ...?
> 
> If so, there would be one more instance to fix.

I think so.  I overlooked both of them. And as you mention,
PQdsplen has the same kind of typo. It returns display length of
the *character* beginning *at* s, too.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
>From 5a4bd365f6353f72198cb59bdf390d2968a03e36 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi 
Date: Mon, 2 Nov 2015 12:46:45 +0900
Subject: [PATCH] Fix identifier completion of multibyte characters.

_copletion_from_query took the byte length of the given text as its
character length. This should be counted in the environmental
encoding.

This patch also fixes a comment for PQmblen and PQdsplen.
---
 src/bin/psql/tab-complete.c| 13 ++---
 src/interfaces/libpq/fe-misc.c |  6 +++---
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 0e8d395..e6e22c4 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -4283,8 +4283,8 @@ complete_from_schema_query(const char *text, int state)
 static char *
 _complete_from_query(int is_schema_query, const char *text, int state)
 {
-	static int	list_index,
-string_length;
+	static int	list_index;
+	int 		string_length = 0;
 	static PGresult *result = NULL;
 
 	/*
@@ -4297,9 +4297,16 @@ _complete_from_query(int is_schema_query, const char *text, int state)
 		char	   *e_text;
 		char	   *e_info_charp;
 		char	   *e_info_charp2;
+		const char *pstr = text;
 
 		list_index = 0;
-		string_length = strlen(text);
+
+		/* count length as a multibyte text */
+		while (*pstr)
+		{
+			string_length++;
+			pstr += PQmblen(pstr, pset.encoding);
+		}
 
 		/* Free any prior result */
 		PQclear(result);
diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 0dbcf73..950634a 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -1,4 +1,4 @@
-/*-
+m/*-
  *
  *	 FILE
  *		fe-misc.c
@@ -1179,7 +1179,7 @@ pqSocketPoll(int sock, int forRead, int forWrite, time_t end_time)
  */
 
 /*
- * returns the byte length of the word beginning s, using the
+ * returns the byte length of the character beginning at s, using the
  * specified encoding.
  */
 int
@@ -1189,7 +1189,7 @@ PQmblen(const char *s, int encoding)
 }
 
 /*
- * returns the display length of the word beginning s, using the
+ * returns the display length of the character beginning at s, using the
  * specified encoding.
  */
 int
-- 
1.8.3.1


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

Re: [HACKERS] Parallel Seq Scan

[Amit Kapila]

On Thu, Nov 5, 2015 at 11:54 PM, Robert Haas  wrote:
>
> I was thinking about this idea:
>
> 1. Add a parallel_aware flag to each plan.
>

Okay, so shall we add it in generic Plan node or to specific plan nodes
like SeqScan, IndexScan, etc.  To me, it appears that parallelism is
a node specific property, so we should add it to specific nodes and
for now as we are parallelising seq scan, so we can add this flag in
SeqScan node.  What do you say?


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

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

[Amit Kapila]

On Thu, Nov 5, 2015 at 7:45 AM, Tomas Vondra 
wrote:

> Hi,
>
> On 11/04/2015 11:32 PM, Tom Lane wrote:
>
>> Jeff Janes  writes:
>>
>>> On Wed, Nov 4, 2015 at 7:14 AM, Tom Lane  wrote:
>>>
 You're missing my point: that is possible in an indexscan, but
 *not* in a bitmap indexscan, because the index AM APIs are
 totally different in the two cases. In a bitmap scan, nothing
 more than a TID bitmap is ever returned out to anyplace that
 could execute arbitrary expressions.

>>>
>> I had thought it must already be able to execute arbitrary
>>> expressions, due to the ability to already support user-defined
>>> btree ops (and ops of non-btree types in the case of other index
>>> types).
>>>
>>
>> No. An index AM is only expected to be able to evaluate clauses of
>> the form   , and the
>> key restriction there is that the operator is one that the AM has
>> volunteered to support. Well, actually, it's the opclass more than
>> the AM that determines this, but anyway it's not just some random
>> operator; more than likely, the AM and/or opclass has got special
>> logic about the operator.
>>
>
> Isn't that pretty much exactly the point made by Jeff and Simon, that
> index AM is currently only allowed to handle the indexable operators, i.e.
> operators that it can explicitly optimize (e.g. use to walk the btree and
> such), and completely ignores the other operators despite having all the
> columns in the index. Which means we'll have to do the heap fetch, which
> usually means a significant performance hit.
>
>
>> This also ties into Robert's point about evaluation of operators
>> against index entries for dead or invisible rows. Indexable operators
>> are much less likely than others to have unexpected side-effects.
>>
>
> I certainly understand there are cases that require care - like the
> leakproof thing pointed out by Robert for example. I don't immediately see
> why evaluation against dead rows would be a problem.
>
>
Apart from other problems discussed, I think it could also lead to
a performance penality for the cases when the qual condition is
costly as evaluating such a qual against lot of dead rows would be a
time consuming operation.  I am not sure, but I think some of the
other well know databases might not have any such problems as
they store visibility info inside index due to which they don't need to
fetch the heap tuple for evaluating such quals.



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

Re: [HACKERS] Foreign join pushdown vs EvalPlanQual

[Kyotaro HORIGUCHI]

Hello,

The attached small patch is what I have in mind now.

fdwroutine->ExecForeignScan may be unset if the FDW does nothing
special. And all the FDW routine needs is the node.

> Subject: [PATCH] Allow substitute ExecScan body for ExecForignScan
> 
> ForeignScan node may return joined tuple. This joined tuple cannot be
> handled properly by ExecScan during EQP recheck. This patch allows
> FDWs to give a special treat to such tuples.

regards,


> > One thing I can agree is, ForeignScan is enforced to use ExecScan,
> > thus some FDW driver may concern about this hard-wired logic.
> > If we try to make ForeignScan unbound from the ExecScan, I like to
> > suggest to revise ExecForeignScan, just invoke a callback; then
> > FDW driver can choose whether ExecScan is best or not.
> 
> Agreed. Calling ExecScan unconditionally from ForeignScan is the
> cause of the root(?) cause I mentioned. Since there'd be no
> difference in data structure between Foreign(Join), calling
> fdwroutine->ExecForeignScan() or something instaed of ExecScan()
> from ExecForeignScan could be the alternative and most promising
> solution for all problems in focus now.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
>From cddbb29bf09e33af38bc7690d1b78f4e20f363b3 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi 
Date: Fri, 6 Nov 2015 13:23:55 +0900
Subject: [PATCH] Allow substitute ExecScan body for ExecForignScan

ForeignScan node may return joined tuple. This joined tuple cannot be
handled properly by ExecScan during EQP recheck. This patch allows
FDWs to give a special treat to such tuples.
---
 src/backend/executor/nodeForeignscan.c | 3 +++
 src/include/foreign/fdwapi.h   | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/src/backend/executor/nodeForeignscan.c b/src/backend/executor/nodeForeignscan.c
index 6165e4a..f43a50b 100644
--- a/src/backend/executor/nodeForeignscan.c
+++ b/src/backend/executor/nodeForeignscan.c
@@ -100,6 +100,9 @@ ForeignRecheck(ForeignScanState *node, TupleTableSlot *slot)
 TupleTableSlot *
 ExecForeignScan(ForeignScanState *node)
 {
+	if (node->fdwroutine->ExecForeignScan)
+		return node->fdwroutine->ExecForeignScan(node);
+
 	return ExecScan((ScanState *) node,
 	(ExecScanAccessMtd) ForeignNext,
 	(ExecScanRecheckMtd) ForeignRecheck);
diff --git a/src/include/foreign/fdwapi.h b/src/include/foreign/fdwapi.h
index 69b48b4..564898d 100644
--- a/src/include/foreign/fdwapi.h
+++ b/src/include/foreign/fdwapi.h
@@ -41,6 +41,8 @@ typedef ForeignScan *(*GetForeignPlan_function) (PlannerInfo *root,
 typedef void (*BeginForeignScan_function) (ForeignScanState *node,
 	   int eflags);
 
+typedef TupleTableSlot *(*ExecForeignScan_function) (ForeignScanState *node);
+
 typedef TupleTableSlot *(*IterateForeignScan_function) (ForeignScanState *node);
 
 typedef void (*ReScanForeignScan_function) (ForeignScanState *node);
@@ -137,6 +139,7 @@ typedef struct FdwRoutine
 	GetForeignPaths_function GetForeignPaths;
 	GetForeignPlan_function GetForeignPlan;
 	BeginForeignScan_function BeginForeignScan;
+	ExecForeignScan_function ExecForeignScan;
 	IterateForeignScan_function IterateForeignScan;
 	ReScanForeignScan_function ReScanForeignScan;
 	EndForeignScan_function EndForeignScan;
-- 
1.8.3.1


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

Re: [HACKERS] CustomScan support on readfuncs.c

[Kouhei Kaigai]

> -Original Message-
> From: Kaigai Kouhei(海外 浩平)
> Sent: Friday, November 06, 2015 11:23 AM
> To: 'Robert Haas'
> Cc: Amit Kapila; Andres Freund; pgsql-hackers
> Subject: RE: [HACKERS] CustomScan support on readfuncs.c
> 
> > On Wed, Nov 4, 2015 at 10:13 AM, Kouhei Kaigai  wrote:
> > > Sorry for my late.
> > >
> > > I confirmed that CustomScan support of readfuncs.c works fine using the
> > > attached patch for the PG-Strom tree. It worked as expected - duplication,
> > > serialization and deserialization by:
> > >   plan_dup = stringToNode(nodeToString(copyObject(plan_orig)))
> > >
> > > So, the custom-scan-on-readfuncs.v1.path itself is good for me.
> >
> > I don't think this is is what we want.  In particular, I like this:
> >
> > Plan trees must be able to be duplicated using copyObject,
> > -   so all the data stored within the custom fields must consist 
> > of
> > -   nodes that that function can handle.  Furthermore, custom scan providers
> > -   cannot substitute a larger structure that embeds
> > -   a CustomScan for the structure itself, as would be 
> > possible
> > -   for a CustomPath or CustomScanState.
> > +   serialized using nodeToString and deserialized using
> > +   stringToNode, so so all the data stored within
> > +   the custom fields must consist of nodes that that function
> > +   can handle.
> > +   Furthermore, custom scan providers have to implement optional
> > +   callbacks if it defines substitute a larger structure that embeds
> > +   a CustomScan for the structure itself.
> > +  
> >
> > The purposes of this patch is supposedly to add readfuncs.c support to
> > custom scans - I agree with that goal.  This documentation change is
> > talking about something else, namely using a CustomScan as the fist
> > field in a larger structure.  I'm not sure I agree with that goal, and
> > in any case it seems like it ought to be a separate patch.  This patch
> > would be a lot smaller if it weren't trying to make that change too.
> >
> Hmm. I might mix up two individual discussions here.
> OK, I'll cut off the minimum portion for readfuncs.c support.
> The other portion - to use a CustomScan as the first field in a larger
> structure - shall be submitted as a separate one.
> 
> > I also don't think that this patch ought to introduce
> > get_current_library_filename().  There are other cases - e.g. for
> > launching background workers - where such a thing could be used, but
> > we haven't introduced it up until now.  It isn't a requirement to get
> > readfuncs support working.
> >
> Even though it isn't a minimum requirement (because extension can put
> arbitrary cstring here), it can tell reasonably reliable pathname for
> extensions if backend tracks filename of the library to be loaded.
> 
> Extension knows its name; its Makefile can embed module name somewhere,
> for example. However, we cannot ensure use always install the modules
> under the $libdir. Even if "$libdir/pg_strom" is expected, we have no
> mechanism to prohibit users to install them under the "$libdir/buggy"
> directory. In this case, "pg_strom" is not a right name to solve the
> library path.
> 
> Even though glibc has dladdr(3) to know the filename that contains the
> target symbol, it is not a portable way. So, I thought it is the best
> way to inform extension by the core, on _PG_init() time where all the
> extension get control once on loading.
>
I tried to split the previous version into two portions.

- custom-scan-on-readfuncs.v2.patch
It allows to serialize/deserialize CustomScan node as discussed upthread.
Regarding of get_current_library_filename(), I keep this feature as
the previous version right now, because I have no good alternatives.

In this patch, the role of TextReadCustomScan callback is to clean out
any tokens generated by TextOutCustomScan. The CustomScan node itself
can be reconstructed with common portion because we expect custom_exprs
and custom_private have objects which are safe to copyObject().


- custom-scan-on-copyfuncs.v2.patch
This is the portion that supports a larger structure with CustomScan
on its head.
The role of TextReadCustomScan is changed. It reconstruct the private
structure that contains CustomScan by palloc(), and fill-up its private
fields by tokens read. The common field of CustomScan shall be filled
up by the core.
One other callback I added is NodeCopyCustomScan; that enables extension
to allocate a structure larger than CustomScan and fill-up its private
fields. 
Anyway, I'll have further discussion for 2nd patch in another thread.

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



custom-scan-on-copyfuncs.v2.patch
Description: custom-scan-on-copyfuncs.v2.patch


custom-scan-on-readfuncs.v2.patch
Description: custom-scan-on-readfuncs.v2.patch

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

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

[Michael Paquier]

On Thu, Nov 5, 2015 at 3:00 PM, Michael Paquier
 wrote:
> On Wed, Nov 4, 2015 at 7:33 PM, Andres Freund wrote:
>> As soon as a single checkpoint ever happened the early-return logic in
>> CreateCheckPoint() will fail to take the LogStandbySnapshot() in
>> CreateCheckPoint() into account. The test is:
>> if (curInsert == ControlFile->checkPoint +
>> MAXALIGN(SizeOfXLogRecord + sizeof(CheckPoint)) &&
>> ControlFile->checkPoint == ControlFile->checkPointCopy.redo)
>> which obviously doesn't work if there's been a WAL record logged after
>> the redo pointer has been determined etc.
>
> Yes. If segment switches are enforced at a pace faster than
> checkpoint_timeout, this check considers that a checkpoint needs to
> happen because a SWITCH_XLOG record is in-between. I am a bit
> surprised that this should happen actually. The segment switch
> triggers a checkpoint record, and vice-versa, even for idle systems.
> Shouldn't we make this check a bit smarter then?

Ah, the documentation clearly explains that setting archive_timeout
will enforce a segment switch if any activity occurred, including a
checkpoint:
http://www.postgresql.org/docs/devel/static/runtime-config-wal.html#RUNTIME-CONFIG-WAL-ARCHIVING
I missed that, sorry.

I have as well thought a bit about adding a space-related constraint
on the standby snapshot generated by the bgwriter, so as to not rely
entirely on the interval of 15s. I finished with the attached that
uses a check based on CheckPointSegments / 8 to be sure that at least
this number of segments has been generated since the last checkpoint
before logging a new snapshot. I guess that's less brittle than the
last patch. Thoughts?
-- 
Michael
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 08d1682..3f410b7 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -526,6 +526,8 @@ typedef struct XLogCtlData
 	XLogwrtRqst LogwrtRqst;
 	XLogRecPtr	RedoRecPtr;		/* a recent copy of Insert->RedoRecPtr */
 	uint32		ckptXidEpoch;	/* nextXID & epoch of latest checkpoint */
+	XLogRecPtr	ckptPtrRec;		/* LSN position of latest checkpoint
+ * record */
 	TransactionId ckptXid;
 	XLogRecPtr	asyncXactLSN;	/* LSN of newest async commit/abort */
 	XLogRecPtr	replicationSlotMinLSN;	/* oldest LSN needed by any slot */
@@ -6316,6 +6318,7 @@ StartupXLOG(void)
 	 checkPoint.newestCommitTs);
 	XLogCtl->ckptXidEpoch = checkPoint.nextXidEpoch;
 	XLogCtl->ckptXid = checkPoint.nextXid;
+	XLogCtl->ckptPtrRec = checkPointLoc;
 
 	/*
 	 * Initialize replication slots, before there's a chance to remove
@@ -8476,10 +8479,11 @@ CreateCheckPoint(int flags)
 	UpdateControlFile();
 	LWLockRelease(ControlFileLock);
 
-	/* Update shared-memory copy of checkpoint XID/epoch */
+	/* Update shared-memory copy of checkpoint XID/epoch/LSN */
 	SpinLockAcquire(>info_lck);
 	XLogCtl->ckptXidEpoch = checkPoint.nextXidEpoch;
 	XLogCtl->ckptXid = checkPoint.nextXid;
+	XLogCtl->ckptPtrRec = recptr;
 	SpinLockRelease(>info_lck);
 
 	/*
@@ -9278,6 +9282,7 @@ xlog_redo(XLogReaderState *record)
 		SpinLockAcquire(>info_lck);
 		XLogCtl->ckptXidEpoch = checkPoint.nextXidEpoch;
 		XLogCtl->ckptXid = checkPoint.nextXid;
+		XLogCtl->ckptPtrRec = lsn;
 		SpinLockRelease(>info_lck);
 
 		/*
@@ -9328,6 +9333,7 @@ xlog_redo(XLogReaderState *record)
 		SpinLockAcquire(>info_lck);
 		XLogCtl->ckptXidEpoch = checkPoint.nextXidEpoch;
 		XLogCtl->ckptXid = checkPoint.nextXid;
+		XLogCtl->ckptPtrRec = lsn;
 		SpinLockRelease(>info_lck);
 
 		/* TLI should not change in an on-line checkpoint */
@@ -10623,6 +10629,21 @@ GetXLogWriteRecPtr(void)
 }
 
 /*
+ * Get latest WAL checkpoint pointer
+ */
+XLogRecPtr
+GetXLogCheckpointRecPtr(void)
+{
+	XLogRecPtr	chkptr;
+
+	SpinLockAcquire(>info_lck);
+	chkptr = XLogCtl->ckptPtrRec;
+	SpinLockRelease(>info_lck);
+
+	return chkptr;
+}
+
+/*
  * Returns the redo pointer of the last checkpoint or restartpoint. This is
  * the oldest point in WAL that we still need, if we have to restart recovery.
  */
diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c
index 65465d6..0706871 100644
--- a/src/backend/postmaster/bgwriter.c
+++ b/src/backend/postmaster/bgwriter.c
@@ -78,6 +78,12 @@ int			BgWriterDelay = 200;
 #define LOG_SNAPSHOT_INTERVAL_MS 15000
 
 /*
+ * Minimal number of WAL segments that need to be generated before logging
+ * a standby snapshot.
+ */
+#define LOG_SNAPSHOT_MIN_SEGS		(CheckPointSegments / 8)
+
+/*
  * LSN and timestamp at which we last issued a LogStandbySnapshot(), to avoid
  * doing so too often or repeatedly if there has been no other write activity
  * in the system.
@@ -310,16 +316,19 @@ BackgroundWriterMain(void)
 		{
 			TimestampTz timeout = 0;
 			TimestampTz now = GetCurrentTimestamp();
+			XLogRecPtr	checkpoint_lsn = GetXLogCheckpointRecPtr();
+			XLogRecPtr	insert_lsn = GetXLogInsertRecPtr();
 
 			timeout = 

Re: [HACKERS] pgbench gaussian/exponential docs improvements

[Fabien COELHO]

On Thu, Nov 5, 2015 at 10:36 AM, Fabien COELHO  wrote:

After some more thoughts, ISTM that this is not exactly a CFD because of the
truncations, so I just named it "f" to be on the safe side.


Was there supposed to be a patch attached here?


No, the actual patch is in the "add function to pgbench" thread as the 
documentation is reworked on the occasion and I tried to take into account 
Tomas suggestions while doing the editing.


--
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] pgbench gaussian/exponential docs improvements

[Fabien COELHO]

I've done some work on the documentation as part of adding functions to 
pgbench expression. You may have a look at:


http://www.postgresql.org/message-id/alpine.DEB.2.10.1511051256500.29177@sto


[...]
 CDF2(x) = PHI(2.0 * threshold * ...) / (2.0 * PHI(threshold) - 1.0)

and then the probability of "i" is

 P(X=i) = CDF2(i+0.5) - CDF2(i-0.5)


I agree that defining the shifted/scaled CDF and using it afterwards looks 
cleaner.


After some more thoughts, ISTM that this is not exactly a CFD because of 
the truncations, so I just named it "f" to be on the safe side.


--
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] Some questions about the array.

[YUriy Zhuravlev]

Hello hackers.
There are comments to my patch? Maybe I should create a separate thread?
Thanks.
-- 
YUriy Zhuravlev
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


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

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

[Victor Wagner]

On 2015.11.02 at 12:15:48 -0500, Peter Eisentraut wrote:

>> That's true, but doesn't allowing every parameter to be multiply
>> specified greatly increase the implementation complexity for a pretty
>> marginal benefit?  

>Well, the way I would have approached is that after all the parsing you
>end up with a list of PQconninfoOption arrays and you you loop over that
>list and call connectDBStart() until you have a connection that
>satisfies you.

>I see that the patch doesn't do that and it looks quite messy as a
>result.

There are some drawbacks with this approach

1. You are suggesting usage of two nested loops, instead of one straight
loop - outer loop over the URLs in the connect string, inner loop over
the IP addresses each URL resolves into. (This inner loop already
presents there for several versions of the postgres).

2. You are suggesting that it should be possible to specify all options 
separately for each of the fallback hosts. 

But some options control how
next host should be choosen (i.e. use random order for load-balancing
or sequential order for high availability), so they should be specified
only once per connect string.

Some other options are
connection level rather then node-level (i.e. if you are using
replication mode of the connection, there is no sense to specify it for
one host and not specify for other). These options should be better
specified such way, that it would be not possible to use conflicting
values.

Third category of options are specified per-cluster much more often
than per node. But are quite often changed from compiled in default.
Typically there are authentication credentials (even you use
trigger-based replication, user information would probably be
replicated), database name (if you use WAL-level replication), client
encoding (which depends on more on the client than on server), etc.

It would be pain if we would need specify them for each host separately.
Syntax, implemented in the my patch allows even to specify
nonstandard-port once for all hosts (using port= parameter in the query
string) and override it for particular host in the list using colon
syntax.

3. Last, but not least. There is more than one postgresql client
implementation. Apart of libpq we have jdbc, native GO language client
etc. And I think that maintaining compatibility of the connect string
between them is good thing. So, I've modelled syntax of multihost
feature in the libpq URL-style syntax after jdbc syntax.
-- 
   Victor Wagner 


-- 
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] Some bugs in psql_complete of psql

[Fujii Masao]

On Wed, Nov 4, 2015 at 5:27 PM, Kyotaro HORIGUCHI
 wrote:
> Hello, I found that a typo(?) in tab-complete.c.
>
>> /* ALTER TABLE,INDEX,MATERIALIZED VIEW xxx ALL IN TABLESPACE xxx OWNED BY */
>> else if (pg_strcasecmp(prev6_wd, "ALL") == 0 &&
>>pg_strcasecmp(prev5_wd, "IN") == 0 &&
>>pg_strcasecmp(prev4_wd, "TABLESPACE") == 0 &&
>>pg_strcasecmp(prev2_wd, "OWNED") == 0 &&
>>pg_strcasecmp(prev4_wd, "BY") == 0)
>
> "BY" is compared against the word in wrong position and it
> prevents this completion from matching.
>
> I also found some other bugs in psql-completion. The attached
> patch applied on master and fixes them all togher.

+ /* If we have INDEX CONCURRENTLY , then add exiting indexes */
+ else if (pg_strcasecmp(prev2_wd, "INDEX") == 0 &&
+ pg_strcasecmp(prev_wd, "CONCURRENTLY") == 0)
+ COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_indexes, NULL);

Is this for DROP INDEX CONCURRENTLY case?
If yes, we should check that prev3_wd is "DROP".

+ /* If we have CREATE|UNIQUE INDEX [CONCURRENTLY] , then add "ON" */
+ else if (((pg_strcasecmp(prev3_wd, "CREATE") == 0 ||
+   pg_strcasecmp(prev3_wd, "UNIQUE") == 0) &&
+  pg_strcasecmp(prev2_wd, "INDEX") == 0 &&
+  pg_strcasecmp(prev_wd, "CONCURRENTLY") != 0) ||

The "!= 0" in the above last condition should be "== 0" ?

+ {"TABLE", "COLUMN", "AGGREGATE", "DATABASE", "DOMAIN",
+ "EVENT TRIGGER", "FOREIGN TABLE", "FUNCTION", "LARGE OBJECT",
+ "MATERIALIZED VIEW", "LANGUAGE", "ROLE", "SCHEMA",
+ "SEQUENCE", "TYPE", "VIEW", NULL};

TABLESPACE also should be added to the list?

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] Request: pg_cancel_backend variant that handles 'idle in transaction' sessions

[Alvaro Herrera]

David Steele wrote:

> The important thing about this implementation was that nothing was
> terminated unless it had exceed a timeout AND was blocking another
> process.

This seems a nice idea, but you need to take the effect on vacuum of
idle-in-xact sessions too.  If the operator left for the day and their
session doesn't block any other process, the next day you could find
some tables bloated to such extreme as to cause problems later on.
Surely the operator can review their terminal to re-do the work, in case
it was valuable.  (If it was valuable, why didn't they commit the
transaction?)

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