Re: [HACKERS] Stating the significance of Lehman Yao in the nbtree README

2014-09-27 Thread Amit Kapila
On Fri, Sep 12, 2014 at 10:54 PM, Peter Geoghegan p...@heroku.com wrote:

 It isn't. It's a minor point, originally raised by Amit.

I have observed that this patch is in 'Needs Review' state for
next CF.  Do you expect any further review from myside?  I think
we can use text recommended by Heikki and after that if you
feel something more is still required, then you can update the same
and send an updated patch.  I believe it should be in 'Waiting On Author'
state, please do let me know if you feel otherwise.


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


Re: [HACKERS] Stating the significance of Lehman Yao in the nbtree README

2014-09-27 Thread Peter Geoghegan
On Fri, Sep 26, 2014 at 11:34 PM, Amit Kapila amit.kapil...@gmail.com wrote:
 I have observed that this patch is in 'Needs Review' state for
 next CF.  Do you expect any further review from myside?  I think
 we can use text recommended by Heikki and after that if you
 feel something more is still required, then you can update the same
 and send an updated patch.  I believe it should be in 'Waiting On Author'
 state, please do let me know if you feel otherwise.

I don't think so. Thanks for the review!

-- 
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] LIMIT for UPDATE and DELETE

2014-09-27 Thread Heikki Linnakangas

On 09/24/2014 05:22 AM, Etsuro Fujita wrote:

(2014/09/17 1:58), Robert Haas wrote:

On Tue, Sep 16, 2014 at 11:31 AM, Kevin Grittner kgri...@ymail.com wrote:

Etsuro Fujita fujita.ets...@lab.ntt.co.jp wrote:

(2014/08/15 6:18), Rukh Meski wrote:

Based on the feedback on my previous patch, I've separated only the
LIMIT part into its own feature.  This version plays nicely with
inheritance.  The intended use is splitting up big UPDATEs and DELETEs
into batches more easily and efficiently.


IIUC, the patch doesn't support OFFSET with UPDATE/DELETE ... LIMIT.  Is
that OK?  When we support ORDER BY ... LIMIT/OFFSET, we will also be
allowing for OFFSET with UPDATE/DELETE ... LIMIT.  So, ISTM it would be
better for the patch to support OFFSET at this point.  No?


Without ORDER BY you really would have no idea *which* rows the
OFFSET would be skipping.  Even more dangerously, you might *think*
you do, and get a surprise when you see the results (if, for
example, a seqscan starts at a point other than the start of the
heap, due to a concurrent seqscan from an unrelated query).  It
might be better not to provide an illusion of a degree of control
you don't have, especially for UPDATE and DELETE operations.


Fair point, but I'd lean toward including it.  I think we all agree
the end goal is ORDER BY .. LIMIT, and there OFFSET certainly has
meaning.  If we don't include it now, we'll just end up adding it
later.  It makes for fewer patches, and fewer changes for users, if we
do it all at once.


I agree with Robert.

Rukh, what do you think as an author?


I have marked this as returned with feedback in the commitfest app. 
What I'd like to see happen next is:


Rewrite how UPDATEs work, per Tom's suggestion here: 
http://www.postgresql.org/message-id/1598.1399826...@sss.pgh.pa.us. Then 
implement ORDER BY ... LIMIT on top of that.



A lot of people would also be willing to settle for just implementing 
LIMIT without ORDER BY, as a stopgap measure. But the UPDATE rewrite is 
what would make everyone most happy.


- Heikki


--
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] Turning off HOT/Cleanup sometimes

2014-09-27 Thread Heikki Linnakangas
This patch has gotten a fair amount of review, and has been rewritten 
once during the commitfest. I think it's pretty close to being 
committable, the only remaining question seems to be what to do with 
system catalogs. I'm marking this as Returned with feedback, I take it 
that Simon can proceed from here, outside the commitfest.


- Heikki



--
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] Scaling shared buffer eviction

2014-09-27 Thread Heikki Linnakangas
Part of this patch was already committed, and the overall patch has had 
its fair share of review for this commitfest, so I'm marking this as 
Returned with feedback. The benchmark results for the bgreclaimer 
showed a fairly small improvement, so it doesn't seem like anyone's 
going to commit the rest of the patch soon, not without more discussion 
and testing anyway. Let's not hold up the commitfest for that.


- Heikki



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


Re: [HACKERS] pg_receivexlog --status-interval add fsync feedback

2014-09-27 Thread Heikki Linnakangas

On 09/05/2014 08:51 AM, furu...@pm.nttdata.co.jp wrote:

Thanks for the review!

I understand the attention message wasn't appropriate.

To report the write location, even If you do not specify a replication

slot.

So the fix only appended messages.

There was a description of the flush location section of '-S' option,
but I intended to catch eye more and added a message.

Is it better to make specification of the -S option indispensable?


The patch cannot be applied to HEAD cleanly. Could you update the patch?


Thank you for pointing out.
Updated the patch.


I don't understand what this patch does. When would you want to use the 
new --reply-fsync option? Is there any reason *not* to use it? In other 
words, do we need an option for this, couldn't you just always send the 
feedback message after fsync?


- Heikki


--
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] Sloppy thinking about leakproof properties of opclass co-members

2014-09-27 Thread Dean Rasheed
On 26 September 2014 15:48, Stephen Frost sfr...@snowman.net wrote:
 * Robert Haas (robertmh...@gmail.com) wrote:
 On Wed, Sep 24, 2014 at 5:34 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  ISTM that the most appropriate solution here is to insist that all or none
  of the members of an operator class be marked leakproof.  (Possibly we
  could restrict that to btree opclasses, but I'm not sure any exception is
  needed for other index types.)  I looked for existing violations of this
  precept, and unfortunately found a *lot*.  For example, texteq is marked
  leakproof but its fellow text comparison operators aren't.  Is that really
  sane?

 Not really.  Fortunately, AFAICT, most if not all of these are in the
 good direction: there are some things not marked leakproof that can be
 so marked.  The reverse direction would be a hard-to-fix security
 hole.  I think at some point somebody went through and tried to mark
 all of the same-type equality operators as leakproof, and it seems
 like that got expanded somewhat without fully rationalizing what we
 had in pg_proc... mostly because I think nobody had a clear idea how
 to do that.

 We'll need to investigate and see if there are any which *aren't* safe
 and probably fix those to be safe rather than trying to deal with this
 risk in some other way.  In other words, I hope it's really all of
 these rather than just most.  In general, I've been hoping that we have
 more leakproof functions than not as, while it's non-trivial to write
 them and ensure they're correct, it's better for us to take that on than
 to ask our users to do so (or have them get some set that someone else
 wrote off of a website or even the extension network).  We can't cover
 everything, of course, but hopefully we'll cover all reasonable use
 cases for types we ship.


Looking at other functions, ISTM that there's an entire class of
functions that can trivially be marked leakproof, and that's no-arg
functions which can't possibly leak. There are quite a few no-arg
builtin functions and none of them are currently marked leakproof.

Rather than (or perhaps as well as) marking all these leakproof,
perhaps we should teach contain_leaky_functions() to automatically
treat any no-arg function as leakproof, so that we allow user-defined
functions too. Taking that one step further, perhaps what it should
really be looking for is Vars in the argument list of a leaky
function, i.e., contain_leaked_vars() rather than
contain_leaky_functions().

Regards,
Dean


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


[HACKERS] Last Commitfest patches waiting review

2014-09-27 Thread Heikki Linnakangas
We are down to 13 patch in Needs Review state. Many of these are 
stragglers that no-one's really even looked at yet.


If we want to finish the commitfest, we'll soon have to decide what to 
do to patches that no-one cares enough about to review them.




pg_receivexlog: addition of --create/--drop to create/drop repslots
---

Magnus has promised to review this, but hasn't had the time for weeks. 
Does anyone else want to pick this up? I'm OK to wait for Magnus to 
handle this - I expect it to be a quick review and commit - but we 
should not hold up the commitfest for this.



Grouping Sets
---

There has been a lot of discussion, but no decisions. See 
http://www.postgresql.org/message-id/87vbodhtvb@news-spur.riddles.org.uk.


Would a committer be interested to take responsibility of this? If not, 
this will just linger indefinitely.



Sort support for text with strxfrm() poor man's keys
---

Peter: Are you waiting for Robert to review this? Robert, could you 
review the latest patch, please? Peter: Could you try to get rid of the 
extra SortSupport object that Robert didn't like? 
(http://www.postgresql.org/message-id/ca+tgmobde+ydfnhts0gwpt54-er8bpt3vx8rpshd+98ctdo...@mail.gmail.com). 
I think it would speed up the process if you did that, instead of 
waiting for Robert to find the time.



Compression of Full Page Writes
---

This has been a ridiculously long thread, with diversions into different 
compression and CRC algorithms. Given that, the latest patch is 
surprisingly small. I don't think this is going to get any better with 
further discussion, and the patch looks OK at a quick glance, so I've 
now marked this as Ready for Committer.



hash join - dynamic bucket count
---

Kevin: Could you review the latest patch, please?


INNER JOIN removals
---

The latest patch is significantly different from what was originally 
submitted for the commitfest, so I wouldn't feel bad just bumping this 
to the next one. I'll do that unless someone picks this up soon.


Tom: I know you're busy with the more urgent jsonb patch..  Do you think 
you would find the time to review this anytime soon? Anyone else?



event triggers: more DROP info
---

Adam: Are you planning to do more review on this? Alvaro: do you feel 
comfortable proceeding with the review you got so far?



Selectivity estimation for inet operators
---

I think there's a bug in the estimation of semi-joins, see 
http://www.postgresql.org/message-id/5423dc8d.3080...@vmware.com. But I 
think we could split this patch into two, and commit the non-join 
selectivity estimator first, as that seems OK. If no-one else steps up 
to the plate, I can do that..



add latency limit to pgbench throttling (--rate)
---

Rukh: Are you planning to review the latest patch version?


Escaping from blocked send() by pg_terminate_backend().
---

I've had a look, but I'd like to have a second opinion on this.


Fix local_preload_libraries to work as expected.
---

Peter: Could you move this forward, please?


pg_dump refactor to remove global variables
---

Peter: Can you review the latest patch, please?

Fix xpath() to return namespace definitions (fixes the issue with nested 
or repeated xpath())

---

No-one's volunteered to review this. I don't understand XML enough to 
review this, and Abhijit who was earlier signed up as reviewer said the 
same thing.


Peter: Could you take a look at this too? You've dabbled into XML stuff 
before..


- Heikki


--
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] Turning off HOT/Cleanup sometimes

2014-09-27 Thread Andres Freund
On 2014-09-27 10:23:33 +0300, Heikki Linnakangas wrote:
 This patch has gotten a fair amount of review, and has been rewritten once
 during the commitfest. I think it's pretty close to being committable, the
 only remaining question seems to be what to do with system catalogs. I'm
 marking this as Returned with feedback, I take it that Simon can proceed
 from here, outside the commitfest.

FWIW, I don't think it is, even with that. As is it seems very likely
that it's going to regress a fair share of workloads. At the very least
it needs a fair amount of benchmarking beforehand.

Greetings,

Andres Freund

-- 
 Andres Freund 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] Last Commitfest patches waiting review

2014-09-27 Thread Andres Freund
On 2014-09-27 11:18:26 +0300, Heikki Linnakangas wrote:
 pg_receivexlog: addition of --create/--drop to create/drop repslots
 ---
 
 Magnus has promised to review this, but hasn't had the time for weeks. Does
 anyone else want to pick this up? I'm OK to wait for Magnus to handle this -
 I expect it to be a quick review and commit - but we should not hold up the
 commitfest for this.

I'll take that one, sometime next week, if Magnus hasn't gotten to it by then.

 Compression of Full Page Writes
 ---
 
 This has been a ridiculously long thread, with diversions into different
 compression and CRC algorithms. Given that, the latest patch is surprisingly
 small. I don't think this is going to get any better with further
 discussion, and the patch looks OK at a quick glance, so I've now marked
 this as Ready for Committer.

Did you like the patch? On a quick pass I wasn't very enthusiastic about
it in its current state.

Greetings,

Andres Freund

-- 
 Andres Freund 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] Last Commitfest patches waiting review

2014-09-27 Thread Heikki Linnakangas

On 09/27/2014 11:31 AM, Andres Freund wrote:

On 2014-09-27 11:18:26 +0300, Heikki Linnakangas wrote:

pg_receivexlog: addition of --create/--drop to create/drop repslots
---

Magnus has promised to review this, but hasn't had the time for weeks. Does
anyone else want to pick this up? I'm OK to wait for Magnus to handle this -
I expect it to be a quick review and commit - but we should not hold up the
commitfest for this.


I'll take that one, sometime next week, if Magnus hasn't gotten to it by then.


Thanks!


Compression of Full Page Writes
---

This has been a ridiculously long thread, with diversions into different
compression and CRC algorithms. Given that, the latest patch is surprisingly
small. I don't think this is going to get any better with further
discussion, and the patch looks OK at a quick glance, so I've now marked
this as Ready for Committer.


Did you like the patch? On a quick pass I wasn't very enthusiastic about
it in its current state.


Now that I look at it closer, there's some ugly things there like 
putting the xl_compress field to XLogRecord. I guess I should post to 
the thread with more detailed comments.


- Heikki



--
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] Selectivity estimation for inet operators

2014-09-27 Thread Emre Hasegeli
 Thanks. Overall, my impression of this patch is that it works very
 well. But damned if I understood *how* it works :-). There's a lot
 of statistics involved, and it's not easy to see why something is
 multiplied by something else. I'm adding comments as I read through
 it.

Thank you for looking at it.  I tried to add more comments to
the multiplications.  New version attached.  It also fixes a bug
caused by wrong operator order used on histogram to histogram
selectivity estimation for inner join.

 I've gotten to the inet_semi_join_selec function:
 
  [function]
 
 This desperately needs comment at the top of the function explaining
 what it does. Let me try to explain what I think it does:
 
 [explanation]

I used your explanation on the new version.

 Now, I think that last step is wrong. Firstly, the Do not bother if
 histogram weight is smaller than 0.1 rule seems bogus. The
 his2_weight is the total number of rows represented by the
 histogram, so surely it can't be less than 1. It can't really be
 less than the statistics target. Unless maybe if the histogram was
 collected when the table was large, but it has since shrunk to
 contain only a few rows, but that seems like a very bizarre corner
 case. At least it needs more comments explaining what the test is
 all about, but I think we should just always use the histogram (if
 it's available).

It was an unnecessary check.  I put an assert instead of it.

 Secondly, if we estimate that there is on average 1.0 matching row
 in the table, it does not follow that the probability that at least
 one row matches is 1.0. Assuming a gaussian distribution with mean
 1.0, the probability that at least one row matches is 0.5. Assuming
 a gaussian distribution here isn't quite right - I guess a Poisson
 distribution would be more accurate - but it sure doesn't seem right
 as it is.

 The error isn't very big, and perhaps you don't run into that very
 often, so I'm not sure what the best way to fix that would be. My
 statistics skills are a bit rusty, but I think the appropriate way
 would be to apply the Poisson distribution, with the estimated
 number of matched rows as the mean. The probability of at least one
 match would be the cumulative distribution function at k=1. It
 sounds like overkill, if this is case occurs only rarely. But then
 again, perhaps it's not all that rare.

A function of his_weight and his_selec could be a better option
than just multiplying them.  I am not sure about the function or
it worths the trouble.  Join selectivity estimation function for
equality doesn't even bother to look at the histograms.  Others
only return constant values.

 That said, I can't immediately find a test case where that error
 would matter. I tried this:
 
 create table inettbl1 (a inet);
 insert into inettbl1 select '10.0.0.' || (g % 255) from
 generate_series(1, 10) g;
 analyze inettbl1;
 explain analyze select count(*) from inettbl1 where a = ANY
 (SELECT a from inettbl1);
 
 The estimate for that is pretty accurate, 833 rows estimated vs 1000
 actual, with the current patch. I'm afraid if we fixed
 inet_semi_join_selec the way I suggest, the estimate would be
 smaller, i.e. more wrong. Is there something else in the estimates
 that accidentally compensates for this currently?

The partial bucket match on inet_his_inclusion_selec() causes low
estimates.  Which also effects non join estimation but not as much as
it effects join estimations.  If that works more correctly, semi
join estimation can be higher than it should be.

network_selfuncs.c:602:
   /* Partial bucket match. */

   left_divider = inet_his_match_divider(left, query, 
 opr_order);
   right_divider = inet_his_match_divider(right, query, 
 opr_order);

   if (left_divider = 0 || right_divider = 0)
   match += 1.0 / pow(2, Max(left_divider, 
 right_divider));

I think this calculation can benefit from a statistical function
more than the semi join.  Using the different bit count as power
of two is the best I could find.  It works quite well on most of
the cases.
diff --git a/src/backend/utils/adt/network_selfuncs.c b/src/backend/utils/adt/network_selfuncs.c
index d0d806f..e9f9696 100644
--- a/src/backend/utils/adt/network_selfuncs.c
+++ b/src/backend/utils/adt/network_selfuncs.c
@@ -3,7 +3,8 @@
  * network_selfuncs.c
  *	  Functions for selectivity estimation of inet/cidr operators
  *
- * Currently these are just stubs, but we hope to do better soon.
+ * Estimates are based on null fraction, most common values, and
+ * histogram of inet/cidr datatypes.
  *
  * Portions Copyright (c) 1996-2014, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
@@ -16,17 +17,864 @@
  */
 #include postgres.h
 
+#include math.h
+
+#include access/htup_details.h
+#include catalog/pg_collation.h
+#include catalog/pg_operator.h
+#include 

Re: [HACKERS] json (b) and null fields

2014-09-27 Thread Stephen Frost
Andrew, all,

* Andrew Dunstan (and...@dunslane.net) wrote:
 I should have been paying a bit more attention to the recent work on
 adding an ignore_nulls option to row_to_json(). Here are some
 belated thought. I apologize to Pavel and Stephen for not having
 commented earlier.

No problem at all and thanks for continuing to think about it!  We
certainly still have quite a bit of time til 9.5 to get this right.

 I think this is really a bandaid, and it will fail to catch lots of
 cases. Several examples:

As discussed on IRC- I agree.  I tend to think of JSON objects as
relatively simple hstore-like structures and so hadn't considered the
complex structure case (as I'm guessing Pavel hadn't either).

 I think a much more comprehensive solution would be preferable. What
 I have in mind is something like
 
 json_strip_null_fields(json) - json
 
 and a similar function for jsonb.

Right, this makes sense to me.

 These would operate recursively. There is a downside, in that they
 would be required to reprocess the json/jsonb. But adding an option
 like this to all the json generator functions would be seriously
 ugly, especially since they are mostly aggregate functions or
 variadic functions. At least in the jsonb case the cost of
 reprocessing is likely to be fairly low.

Yeah, I don't see adding this option to all json generator functions as
making a lot of sense but rather just to the select few things which it
really makes sense for and then having a function which can be used by
users to do the same for results from other operations.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] WITH CHECK and Column-Level Privileges

2014-09-27 Thread Stephen Frost
* Stephen Frost (sfr...@snowman.net) wrote:
  Looks like there is an issue here with CHECK constraints and NOT NULL
  constraints, yes.  The uniqueness check complains about the key already
  existing and returns the key, but I don't think that's actually a
  problem- to get that to happen you have to specify the new key and
  that's what is returned.
 
 Yeah, I take that back.  If there is a composite key involved then you
 can run into the same issue- you update one of the columns to a
 conflicting value and get back the entire key, including columns you
 shouldn't be allowed to see.

Alright, attached is a patch which addresses this by checking if the
user has SELECT rights on the relation first and, if so, the existing
error message is returned with the full row as usual, but if not, then
the row data is omitted.

Also attached is a patch for 9.4 which does the same, but also removes
the row reporting (completely) from the WITH CHECK case.  It could be
argued that it would be helpful to have it there also, perhaps, but I'm
not convinced at this point that it's really valuable- and we'd have to
think about how this would work with views (permission on the view?  or
on the table underneath?  what if there is more than one?, etc).

Thanks,

Stephen
diff --git a/src/backend/access/nbtree/nbtinsert.c b/src/backend/access/nbtree/nbtinsert.c
new file mode 100644
index 17d9765..f9a8bff
*** a/src/backend/access/nbtree/nbtinsert.c
--- b/src/backend/access/nbtree/nbtinsert.c
***
*** 21,26 
--- 21,27 
  #include miscadmin.h
  #include storage/lmgr.h
  #include storage/predicate.h
+ #include utils/acl.h
  #include utils/inval.h
  #include utils/tqual.h
  
*** _bt_check_unique(Relation rel, IndexTupl
*** 387,400 
  
  		index_deform_tuple(itup, RelationGetDescr(rel),
  		   values, isnull);
! 		ereport(ERROR,
! (errcode(ERRCODE_UNIQUE_VIOLATION),
!  errmsg(duplicate key value violates unique constraint \%s\,
! 		RelationGetRelationName(rel)),
!  errdetail(Key %s already exists.,
! 		   BuildIndexValueDescription(rel,
! 			values, isnull)),
!  errtableconstraint(heapRel,
  			 RelationGetRelationName(rel;
  	}
  }
--- 388,420 
  
  		index_deform_tuple(itup, RelationGetDescr(rel),
  		   values, isnull);
! 		/*
! 		 * When reporting a failure, it can be handy to have
! 		 * the key which failed reported.  Unfortunately, when
! 		 * using column-level privileges, this could expose
! 		 * a column the user does not have rights for.  Instead,
! 		 * only report the key if the user has full table-level
! 		 * SELECT rights on the relation.
! 		 */
! 		if (pg_class_aclcheck(RelationGetRelid(rel),
! 			  GetUserId(), ACL_SELECT) ==
! ACLCHECK_OK)
! 			ereport(ERROR,
! 	(errcode(ERRCODE_UNIQUE_VIOLATION),
! 	 errmsg(duplicate key value violates unique constraint \%s\,
! 			RelationGetRelationName(rel)),
! 	 errdetail(Key %s already exists.,
! 			   BuildIndexValueDescription(rel,
! values,
! isnull)),
! 	 errtableconstraint(heapRel,
! 			 RelationGetRelationName(rel;
! 		else
! 			ereport(ERROR,
! 	(errcode(ERRCODE_UNIQUE_VIOLATION),
! 	 errmsg(duplicate key value violates unique constraint \%s\,
! 			RelationGetRelationName(rel)),
! 	 errtableconstraint(heapRel,
  			 RelationGetRelationName(rel;
  	}
  }
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
new file mode 100644
index 85d1c63..fe98599
*** a/src/backend/executor/execMain.c
--- b/src/backend/executor/execMain.c
*** ExecConstraints(ResultRelInfo *resultRel
*** 1600,1614 
  		{
  			if (tupdesc-attrs[attrChk - 1]-attnotnull 
  slot_attisnull(slot, attrChk))
! ereport(ERROR,
! 		(errcode(ERRCODE_NOT_NULL_VIOLATION),
! 		 errmsg(null value in column \%s\ violates not-null constraint,
! 			  NameStr(tupdesc-attrs[attrChk - 1]-attname)),
! 		 errdetail(Failing row contains %s.,
!    ExecBuildSlotValueDescription(slot,
!  tupdesc,
!  64)),
! 		 errtablecol(rel, attrChk)));
  		}
  	}
  
--- 1600,1631 
  		{
  			if (tupdesc-attrs[attrChk - 1]-attnotnull 
  slot_attisnull(slot, attrChk))
! 			{
! /*
!  * When reporting failure, it's handy to have the whole row
!  * which failed returned, but we can only show it if the user
!  * has full SELECT rights on the relation, otherwise we might
!  * end up showing fields the user does not have access to, due
!  * to column-level privileges.
!  */
! if (pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
! 	  ACL_SELECT) == ACLCHECK_OK)
! 	ereport(ERROR,
! 			

Re: [HACKERS] Sloppy thinking about leakproof properties of opclass co-members

2014-09-27 Thread Tom Lane
Dean Rasheed dean.a.rash...@gmail.com writes:
 Rather than (or perhaps as well as) marking all these leakproof,
 perhaps we should teach contain_leaky_functions() to automatically
 treat any no-arg function as leakproof, so that we allow user-defined
 functions too. Taking that one step further, perhaps what it should
 really be looking for is Vars in the argument list of a leaky
 function, i.e., contain_leaked_vars() rather than
 contain_leaky_functions().

+1, but that's a totally independent question from what I'm on about
at the moment.

regards, tom lane


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


Re: [HACKERS] Last Commitfest patches waiting review

2014-09-27 Thread Tom Lane
Heikki Linnakangas hlinnakan...@vmware.com writes:
 [ unreviewed patches ]

 Grouping Sets

 There has been a lot of discussion, but no decisions. See 
 http://www.postgresql.org/message-id/87vbodhtvb@news-spur.riddles.org.uk.

 Would a committer be interested to take responsibility of this? If not, 
 this will just linger indefinitely.

I should and will take this, but not in this commitfest: I've been just
horribly busy lately with both work and personal issues.  If we can punt
it to the next fest, I will promise to work on it then.


 INNER JOIN removals

 The latest patch is significantly different from what was originally 
 submitted for the commitfest, so I wouldn't feel bad just bumping this 
 to the next one. I'll do that unless someone picks this up soon.
 Tom: I know you're busy with the more urgent jsonb patch..  Do you think 
 you would find the time to review this anytime soon? Anyone else?

Same deal here.


 Selectivity estimation for inet operators

 I think there's a bug in the estimation of semi-joins, see 
 http://www.postgresql.org/message-id/5423dc8d.3080...@vmware.com. But I 
 think we could split this patch into two, and commit the non-join 
 selectivity estimator first, as that seems OK. If no-one else steps up 
 to the plate, I can do that..

And I'd like to look this one over too ...


 Escaping from blocked send() by pg_terminate_backend().

 I've had a look, but I'd like to have a second opinion on this.

I concur with your opinion that this is scary as heck.  We need multiple
pairs of eyeballs if we're going to do anything in this area.  In the long
run though, I think pushing functionality into signal handlers is exactly
backwards; we ought to be trying to go the other way.

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] Last Commitfest patches waiting review

2014-09-27 Thread Andres Freund
On 2014-09-27 10:12:03 -0400, Tom Lane wrote:
  Escaping from blocked send() by pg_terminate_backend().
 
  I've had a look, but I'd like to have a second opinion on this.
 
 I concur with your opinion that this is scary as heck.  We need multiple
 pairs of eyeballs if we're going to do anything in this area.  In the long
 run though, I think pushing functionality into signal handlers is exactly
 backwards; we ought to be trying to go the other way.

I very much agree with that concern. The interactions are just
complicated.

I'm now refreshing my work to do all waiting in client communication via
latches. While far from trivial, I'm pretty sure that's the better
course in the long run.

I guess I'll post it in the other thread.

Greetings,

Andres Freund

-- 
 Andres Freund 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] Replication identifiers, take 3

2014-09-27 Thread Steve Singer

On 09/26/2014 06:05 PM, Andres Freund wrote:

On 2014-09-26 14:57:12 -0400, Robert Haas wrote:
Sure, it'll possibly not be trivial to move them elsewhere. On the other
hand, the padding bytes have been unused for 8+ years without somebody
laying claim on them but me. I don't think it's a good idea to leave
them there unused when nobody even has proposed another use for a long
while. That'll just end up with them continuing to be unused.  And
there's actually four more consecutive bytes on 64bit systems that are
unused.

Should there really be a dire need after that, we can simply bump the
record size. WAL volume wise it'd not be too bad to make the record a
tiny bit bigger - the header is only a relatively small fraction of the
entire content.


If we were now increasing the WAL record size anyway for some unrelated 
reason, would we be willing to increase it by a further 2 bytes for the 
node identifier?
If the answer is 'no' then I don't think we can justify using the 2 
padding bytes just because they are there and have been unused for 
years.  But if the answer is yes, we feel this important enough to 
justfiy a slightly (2 byte) larger WAL record header then we shouldn't 
use the excuse of maybe needing those 2 bytes for something else.   When 
something else comes along that needs the WAL space we'll have to 
increase the record size.


To say that if some other patch comes along that needs the space we'll 
redo this feature to use the method Robert describes is unrealistic.  If 
we think that the replication identifier  isn't 
general/important/necessary to justify 2 bytes of WAL header space then 
we should start out with something that doesn't use the WAL header,




Steve


I've also wondered about that. Perhaps we simply should have an
additional 'name' column indicating the replication solution?

Yeah, maybe, but there's still the question of substructure within the
non-replication-solution part of the name.  Not sure if we can assume
that a bipartite identifier, specifically, is right, or whether some
solutions will end up with different numbers of components.

Ah. I thought you only wanted to suggest a separator between the
replication solution and it's internal dat. But you actually want to
suggest an internal separator to be used in the solution's namespace?
I'm fine with that. I don't think we can suggest much beyond that -
different solutions will have fundamentally differing requirements about
which information to store.

Agreed.

So, let's recommend underscores as that separator?

Greetings,

Andres Freund





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


Re: [HACKERS] Replication identifiers, take 3

2014-09-27 Thread Steve Singer

On 09/26/2014 10:21 AM, Andres Freund wrote:

On 2014-09-26 09:53:09 -0400, Robert Haas wrote:

On Fri, Sep 26, 2014 at 5:05 AM, Andres Freund and...@2ndquadrant.com wrote:

Let me try to summarize the information requirements for each of these
things.  For #1, you need to know, after crash recovery, for each
standby, the last commit LSN which the client has confirmed via a
feedback message.

I'm not sure I understand what you mean here? This is all happening on
the *standby*. The standby needs to know, after crash recovery, the
latest commit LSN from the primary that it has successfully replayed.

Ah, sorry, you're right: so, you need to know, after crash recovery,
for each machine you are replicating *from*, the last transaction (in
terms of LSN) from that server that you successfully replayed.

Precisely.


I don't think a solution which logs the change of origin will be
simpler. When the origin is in every record, you can filter without keep
track of any state. That's different if you can switch the origin per
tx. At the very least you need a in memory entry for the origin.

But again, that complexity pertains only to logical decoding.
Somebody who wants to tweak the WAL format for an UPDATE in the future
doesn't need to understand how this works, or care.

I agree that that's a worthy goal. But I don't see how this isn't the
case with what I propose? This isn't happening on the level of
individual rmgrs/ams - there've been two padding bytes after 'xl_rmid'
in struct XLogRecord for a long time.

There's also the significant advantage that not basing this on the xid
allows it to work correctly with records not tied to a
transaction. There's not that much of that happening yet, but I've
several features in mind:

* separately replicate 2PC commits. 2PC commits don't have an xid
   anymore... With some tooling on top replication 2PC in two phases
   allow for very cool stuff. Like optionally synchronous multimaster
   replication.
* I have a pending patch that allows to send 'messages' through logical
   decoding - yielding a really fast and persistent queue. For that it's
   useful have transactional *and* nontransactional messages.
* Sanely replicating CONCURRENTLY stuff gets harder if you tie things to
   the xid.


At what point in the decoding stream should something related to a 
CONCURRENTLY command show up?
Also, for a logical message queue why couldn't you have a xid associated 
with the message that had nothing else in the transaction?


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] WITH CHECK and Column-Level Privileges

2014-09-27 Thread Dean Rasheed
On 27 September 2014 14:33, Stephen Frost sfr...@snowman.net wrote:
 Yeah, I take that back.  If there is a composite key involved then you
 can run into the same issue- you update one of the columns to a
 conflicting value and get back the entire key, including columns you
 shouldn't be allowed to see.

 Alright, attached is a patch which addresses this by checking if the
 user has SELECT rights on the relation first and, if so, the existing
 error message is returned with the full row as usual, but if not, then
 the row data is omitted.


Seems reasonable.

 Also attached is a patch for 9.4 which does the same, but also removes
 the row reporting (completely) from the WITH CHECK case.  It could be
 argued that it would be helpful to have it there also, perhaps, but I'm
 not convinced at this point that it's really valuable- and we'd have to
 think about how this would work with views (permission on the view?  or
 on the table underneath?  what if there is more than one?, etc).


Well by that point in the code, the query has been rewritten and the
row being reported is for the underlying table, so it's the table's
ACLs that should apply. In fact not all the values from the table are
even necessarily exported in the view, so its ACLs are not appropriate
to the values being reported. I suspect that in many/most real-world
cases, the user will only have permissions on the view, not on the
underlying table, in which case it won't work anyway. So +1 for just
removing it.

Regards,
Dean


-- 
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] json (b) and null fields

2014-09-27 Thread Andrew Dunstan


On 09/27/2014 08:00 AM, Stephen Frost wrote:

Andrew, all,

* Andrew Dunstan (and...@dunslane.net) wrote:

I should have been paying a bit more attention to the recent work on
adding an ignore_nulls option to row_to_json(). Here are some
belated thought. I apologize to Pavel and Stephen for not having
commented earlier.

No problem at all and thanks for continuing to think about it!  We
certainly still have quite a bit of time til 9.5 to get this right.


I think this is really a bandaid, and it will fail to catch lots of
cases. Several examples:

As discussed on IRC- I agree.  I tend to think of JSON objects as
relatively simple hstore-like structures and so hadn't considered the
complex structure case (as I'm guessing Pavel hadn't either).


I think a much more comprehensive solution would be preferable. What
I have in mind is something like

 json_strip_null_fields(json) - json

and a similar function for jsonb.

Right, this makes sense to me.


These would operate recursively. There is a downside, in that they
would be required to reprocess the json/jsonb. But adding an option
like this to all the json generator functions would be seriously
ugly, especially since they are mostly aggregate functions or
variadic functions. At least in the jsonb case the cost of
reprocessing is likely to be fairly low.

Yeah, I don't see adding this option to all json generator functions as
making a lot of sense but rather just to the select few things which it
really makes sense for and then having a function which can be used by
users to do the same for results from other operations.





I guess I'm questioning the wisdom of keeping it for row_to_json given 
that it doesn't operate recursively anyway (and making it do so would be 
difficult and ugly).


The counter argument for this is that nested composites and arrays of 
composites are relatively rare in records, so providing a fast 
non-recursive stripping of nulls for the common case is reasonable.


If we're going to keep this, I think we also need to provide it 
(non-recursive) for json_agg via an optional second argument. This 
should be a fairly simple change: just steer the result via 
composite_to_json if it's a record, rather than to datum_to_json.


cheers

andrew


--
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] Escaping from blocked send() reprised.

2014-09-27 Thread Andres Freund
On 2014-09-03 15:09:54 +0300, Heikki Linnakangas wrote:
 On 09/03/2014 12:23 AM, Andres Freund wrote:
 On 2014-09-02 17:21:03 -0400, Tom Lane wrote:
 Heikki Linnakangas hlinnakan...@vmware.com writes:
 I was going to suggest using WaitLatchOrSocket instead of sleeping in 1
 second increment, but I see that WaitLatchOrSocket() doesn't currently
 support waiting for a socket to become writeable, without also waiting
 for it to become readable. I wonder how difficult it would be to lift
 that restriction.
 
 My recollection is that there was a reason for that, but I don't recall
 details any more.
 
 http://git.postgresql.org/pg/commitdiff/e42a21b9e6c9b9e6346a34b62628d48ff2fc6ddf
 
 In my prototype I've changed the API that errors set both
 READABLE/WRITABLE. Seems to work
 
 Andres, would you mind posting the WIP patch you have? That could be a
 better foundation for this patch.

Sorry, I missed this message and only cought up when reading your CF
status mail. I've attached three patches:

0001: Allows WaitLatchOrSocket(WL_WRITABLE) without WL_READABLE. I've
  tested the poll() and select() implementations on linux and
  blindly patched up windows.
0002: Put the socket the backend uses to communicate with the client
  into nonblocking mode as soon as latches are ready and use latches
  to wait. This probably doesn't work correctly without 0003, but
  seems easier to review separately.
0003: Don't do sinval catchup and notify processing in signal
  handlers. It's quite cool that it worked that well so far, but it
  requires some complicated code and is rather fragile. 0002 allows
  to move that out of signal handlers and just use a latch
  there. This seems remarkably simpler:
   4 files changed, 69 insertions(+), 229 deletions(-)

These aren't ready for commit, especially not 0003, but I think they are
quite a good foundation for getting rid of the blocking in send(). I
haven't added any interrupt processing after interrupted writes, but
marked the relevant places with XXXs.

With regard to 0002, I dislike the current need to do interrupt
processing both in be-secure.c and be-secure-openssl.c. I guess we could
solve that by returning something like EINTR from the ssl routines when
they need further reads/writes and do all the processing in one place in
be-secure.c.

There's also some cleanup in 0002/0003 needed:
prepare_for_client_read()/client_read_ended() aren't needed in that form
anymore and should probably rather be something like
CHECK_FOR_READ_INTERRUPT() or similar.  Similarly the
EnableCatchupInterrupt()/DisableCatchupInterrupt() in autovacuum.c is
pretty ugly.

Btw, be-secure.c is really not a good name anymore...

What do you think?

Greetings,

Andres Freund

-- 
 Andres Freund 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] HEADS UP: PGCon 2015 is in June

2014-09-27 Thread Dan Langille
HEADS UP.

PGCon 2015 will be in June.  That’s a few weeks later than in previous years.

— 
Dan Langille



signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: [HACKERS] pgcrypto: PGP armor headers

2014-09-27 Thread Marko Tiikkaja

Hi,

On 9/25/14, 3:56 PM, I wrote:

On 9/25/14 3:50 PM, Heikki Linnakangas wrote:

Are you planning to post the main patch rebased on top of this soon? As
in the next day or two? Otherwise I'll mark this as Returned with
feedback for this commitfest.


Yes.  With good luck I'll get you a rebased one today, otherwise it'll
have to wait until tomorrow.


Missed that promise by a day since something unexpected came up 
yesterday.  Attached is v3 of the patch.  The changes are:


  - Rebased on top of the current master
  - Added a function pgp_armor_header_keys() to list all keys present 
in an armor

  - Changed pgp_armor_header() to use a StringInfo instead of an mbuf
  - Fixed the error is returned problem in the documentation pointed 
out earlier



.marko
*** a/contrib/pgcrypto/Makefile
--- b/contrib/pgcrypto/Makefile
***
*** 26,32  MODULE_big   = pgcrypto
  OBJS  = $(SRCS:.c=.o) $(WIN32RES)
  
  EXTENSION = pgcrypto
! DATA = pgcrypto--1.1.sql pgcrypto--1.0--1.1.sql pgcrypto--unpackaged--1.0.sql
  PGFILEDESC = pgcrypto - cryptographic functions
  
  REGRESS = init md5 sha1 hmac-md5 hmac-sha1 blowfish rijndael \
--- 26,32 
  OBJS  = $(SRCS:.c=.o) $(WIN32RES)
  
  EXTENSION = pgcrypto
! DATA = pgcrypto--1.2.sql pgcrypto--1.1--1.2.sql pgcrypto--1.0--1.1.sql 
pgcrypto--unpackaged--1.0.sql
  PGFILEDESC = pgcrypto - cryptographic functions
  
  REGRESS = init md5 sha1 hmac-md5 hmac-sha1 blowfish rijndael \
*** a/contrib/pgcrypto/expected/pgp-armor.out
--- b/contrib/pgcrypto/expected/pgp-armor.out
***
*** 102,104  em9va2E=
--- 102,423 
  -END PGP MESSAGE-
  ');
  ERROR:  Corrupt ascii-armor
+ -- corrupt
+ select pgp_armor_header('
+ -BEGIN PGP MESSAGE-
+ foo:
+ 
+ em9va2E=
+ =
+ -END PGP MESSAGE-
+ ', 'foo');
+ ERROR:  Corrupt ascii-armor
+ -- empty
+ select pgp_armor_header('
+ -BEGIN PGP MESSAGE-
+ foo: 
+ 
+ em9va2E=
+ =
+ -END PGP MESSAGE-
+ ', 'foo');
+  pgp_armor_header 
+ --
+  
+ (1 row)
+ 
+ -- simple
+ select pgp_armor_header('
+ -BEGIN PGP MESSAGE-
+ foo: bar
+ 
+ em9va2E=
+ =
+ -END PGP MESSAGE-
+ ', 'foo');
+  pgp_armor_header 
+ --
+  bar
+ (1 row)
+ 
+ -- uninteresting keys, part 1
+ select pgp_armor_header('
+ -BEGIN PGP MESSAGE-
+ foo: found
+ bar: ignored
+ 
+ em9va2E=
+ =
+ -END PGP MESSAGE-
+ ', 'foo');
+  pgp_armor_header 
+ --
+  found
+ (1 row)
+ 
+ -- uninteresting keys, part 2
+ select pgp_armor_header('
+ -BEGIN PGP MESSAGE-
+ bar: ignored
+ foo: found
+ 
+ em9va2E=
+ =
+ -END PGP MESSAGE-
+ ', 'foo');
+  pgp_armor_header 
+ --
+  found
+ (1 row)
+ 
+ -- uninteresting keys, part 3
+ select pgp_armor_header('
+ -BEGIN PGP MESSAGE-
+ bar: ignored
+ foo: found
+ bar: ignored
+ 
+ em9va2E=
+ =
+ -END PGP MESSAGE-
+ ', 'foo');
+  pgp_armor_header 
+ --
+  found
+ (1 row)
+ 
+ -- insane keys, part 1
+ select pgp_armor_header('
+ -BEGIN PGP MESSAGE-
+ insane:key : 
+ 
+ em9va2E=
+ =
+ -END PGP MESSAGE-
+ ', 'insane:key ');
+  pgp_armor_header 
+ --
+  
+ (1 row)
+ 
+ -- insane keys, part 2
+ select pgp_armor_header('
+ -BEGIN PGP MESSAGE-
+ insane:key : text value here
+ 
+ em9va2E=
+ =
+ -END PGP MESSAGE-
+ ', 'insane:key ');
+  pgp_armor_header 
+ --
+  text value here
+ (1 row)
+ 
+ -- long value
+ select pgp_armor_header('
+ -BEGIN PGP MESSAGE-
+ long: this value is more than 76 characters long, but it should still parse 
correctly as that''s permitted by RFC 4880
+ 
+ em9va2E=
+ =
+ -END PGP MESSAGE-
+ ', 'long');
+ pgp_armor_header  
   
+ 
-
+  this value is more than 76 characters long, but it should still parse 
correctly as that's permitted by RFC 4880
+ (1 row)
+ 
+ -- long value, split up
+ select pgp_armor_header('
+ -BEGIN PGP MESSAGE-
+ long: this value is more than 76 characters long, but it should still 
+ long: parse correctly as that''s permitted by RFC 4880
+ 
+ em9va2E=
+ =
+ -END PGP MESSAGE-
+ ', 'long');
+ pgp_armor_header  
   
+ 
-
+  this value is more than 76 characters long, but it should still parse 
correctly as that's permitted by RFC 4880
+ (1 row)
+ 
+ -- long value, split up, part 2
+ select pgp_armor_header('
+ -BEGIN PGP MESSAGE-
+ long: this value is more than 
+ long: 76 characters long, but it should still 
+ long: parse correctly as that''s permitted by RFC 4880
+ 
+ 

[HACKERS] trivial patch for dynahash logging

2014-09-27 Thread Jeff Janes
under HASH_STATISTICS, the output reporting this HTAB upon destruction is
pretty useless. Which HTAB would this one be?  It is not necessarily the
most recently created one.

This makes it output the %p to the actual HTAB, so it can be matched up
with the logging of the creation.

I'm not sure why it bypasses elog.  Is that so it can run during start-up
before elog is active?  I'd like to make it go through elog so that
log_line_prefix are applied, but then it would no longer be a trivial patch.

Cheers,

Jeff
diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c
new file mode 100644
index 2b99e4b..89c3a9d
*** a/src/backend/utils/hash/dynahash.c
--- b/src/backend/utils/hash/dynahash.c
*** void
*** 733,740 
  hash_stats(const char *where, HTAB *hashp)
  {
  #if HASH_STATISTICS
! 	fprintf(stderr, %s: this HTAB -- accesses %ld collisions %ld\n,
! 			where, hashp-hctl-accesses, hashp-hctl-collisions);
  
  	fprintf(stderr, hash_stats: entries %ld keysize %ld maxp %u segmentcount %ld\n,
  			hashp-hctl-nentries, (long) hashp-hctl-keysize,
--- 737,744 
  hash_stats(const char *where, HTAB *hashp)
  {
  #if HASH_STATISTICS
! 	fprintf(stderr, %s: HTAB %p -- accesses %ld collisions %ld\n,
! 			where, hashp, hashp-hctl-accesses, hashp-hctl-collisions);
  
  	fprintf(stderr, hash_stats: entries %ld keysize %ld maxp %u segmentcount %ld\n,
  			hashp-hctl-nentries, (long) hashp-hctl-keysize,

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


Re: [HACKERS] trivial patch for dynahash logging

2014-09-27 Thread Tom Lane
Jeff Janes jeff.ja...@gmail.com writes:
 under HASH_STATISTICS, the output reporting this HTAB upon destruction is
 pretty useless. Which HTAB would this one be?  It is not necessarily the
 most recently created one.

 This makes it output the %p to the actual HTAB, so it can be matched up
 with the logging of the creation.

Seems reasonable, although I wonder why neither of them print the tabname
string.  More generally, I see no calls to hash_stats() anywhere except
in hash_destroy(), so wouldn't you need a rather larger patch to make it
actually do anything useful?

 I'm not sure why it bypasses elog.  Is that so it can run during start-up
 before elog is active?  I'd like to make it go through elog so that
 log_line_prefix are applied, but then it would no longer be a trivial patch.

A quick dig in our git history shows that it was like that in Postgres95,
so the answer is most likely this code predates elog().  I would think
it'd be a safe assumption that elog is initialized before any hashtables
are created, but certainly you could test that pretty quickly ...

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] INSERT ... ON CONFLICT {UPDATE | IGNORE}

2014-09-27 Thread Peter Geoghegan
On Thu, Sep 25, 2014 at 1:48 PM, Simon Riggs si...@2ndquadrant.com wrote:
 I hate the fact
 that you have written no user facing documentation for this feature.

Attached patch adds a commit to the existing patchset. For the
convenience of reviewers, I've uploaded and made publicly accessible a
html build of the documentation. This page is of most interest:

http://postgres-benchmarks.s3-website-us-east-1.amazonaws.com/on-conflict-docs/sql-insert.html

See also:

http://postgres-benchmarks.s3-website-us-east-1.amazonaws.com/on-conflict-docs/transaction-iso.html
http://postgres-benchmarks.s3-website-us-east-1.amazonaws.com/on-conflict-docs/ddl-inherit.html
http://postgres-benchmarks.s3-website-us-east-1.amazonaws.com/on-conflict-docs/sql-createrule.html
http://postgres-benchmarks.s3-website-us-east-1.amazonaws.com/on-conflict-docs/trigger-definition.html
http://postgres-benchmarks.s3-website-us-east-1.amazonaws.com/on-conflict-docs/sql-createtrigger.html
http://postgres-benchmarks.s3-website-us-east-1.amazonaws.com/on-conflict-docs/index-unique-checks.html
http://postgres-benchmarks.s3-website-us-east-1.amazonaws.com/on-conflict-docs/sql-createview.html
http://postgres-benchmarks.s3-website-us-east-1.amazonaws.com/on-conflict-docs/postgres-fdw.html

-- 
Peter Geoghegan
From 005eb2760b356c7383c591bb92294cc9626baabe Mon Sep 17 00:00:00 2001
From: Peter Geoghegan p...@heroku.com
Date: Fri, 26 Sep 2014 20:59:04 -0700
Subject: [PATCH 6/6] User-visible documentation for INSERT ... ON CONFLICT
 {UPDATE | IGNORE}

INSERT ... ON CONFLICT {UPDATE | IGNORE} is documented as a new clause
of the INSERT command.  Some potentially surprising interactions with
triggers are noted -- statement level UPDATE triggers will not fire when
INSERT ... ON CONFLICT UPDATE is executed, for example.

All the existing features that INSERT ... ON CONFLICT {UPDATE | IGNORE}
fails to completely play nice with have those limitations noted.  (Notes
are added to the existing documentation for those other features,
although some of these cases will need to be revisited).  This includes
postgres_fdw, updatable views and table inheritance.  In principle it is
the responsibility of writable foreign data wrapper authors to provide
appropriate support for this new clause (although it's hard to see how
the optional WITHIN `unique_index` clause could work there).

Finally, a user-level description of the new MVCC violation that
INSERT ... ON CONFLICT {UPDATE | IGNORE} sometimes requires has been
added to Chapter 13 - Concurrency Control, beside existing commentary
on Read Committed mode's special handling of concurrent updates, and the
implications for snapshot isolation (i.e.  what is internally referred
to as the EvalPlanQual() mechanism).  The new MVCC violation
introduced seems somewhat distinct from the existing one, because in
Read Committed mode it is no longer necessary for any row version to be
conventionally visible to the command's MVCC snapshot for an UPDATE of
the row to occur (or for the row to be locked).
---
 doc/src/sgml/ddl.sgml|  14 +++
 doc/src/sgml/mvcc.sgml   |  43 ++--
 doc/src/sgml/plpgsql.sgml|  14 +--
 doc/src/sgml/postgres-fdw.sgml   |   8 ++
 doc/src/sgml/ref/create_rule.sgml|   6 +-
 doc/src/sgml/ref/create_trigger.sgml |   5 +-
 doc/src/sgml/ref/create_view.sgml|  15 ++-
 doc/src/sgml/ref/insert.sgml | 203 ---
 doc/src/sgml/trigger.sgml|  30 +-
 9 files changed, 302 insertions(+), 36 deletions(-)

diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index c07f5a2..2910890 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -2292,6 +2292,20 @@ VALUES ('Albany', NULL, NULL, 'NY');
but in the meantime considerable care is needed in deciding whether
inheritance is useful for your application.
   /para
+  para
+   !-- XXX:  This limitation can probably be removed, at least for
+the simple case where a unique index constrains an attribute
+or attributes that are effectively contained within the
+(implied) partitioning key.  Clearly doing anything more
+advanced than that would require a large overhaul of
+partitioning in PostgreSQL.
+--
+   Since unique indexes do not constrain every child table in an
+   inheritance hierarchy, inheritance is not supported for
+   commandINSERT/command statements that contain an literalON
+   CONFLICT UPDATE/ clause, or an literalON CONFLICT IGNORE/
+   clause.
+  /para
 
/sect2
   /sect1
diff --git a/doc/src/sgml/mvcc.sgml b/doc/src/sgml/mvcc.sgml
index cd55be8..dd05cfe 100644
--- a/doc/src/sgml/mvcc.sgml
+++ b/doc/src/sgml/mvcc.sgml
@@ -326,14 +326,41 @@
/para
 
para
-Because of the above rule, it is possible for an updating command to see an
-inconsistent snapshot: it can see the effects of concurrent updating
-commands on the same rows it is trying to update, but it
-does not 

Re: [HACKERS] json (b) and null fields

2014-09-27 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes:
 On 09/27/2014 08:00 AM, Stephen Frost wrote:
 Yeah, I don't see adding this option to all json generator functions as
 making a lot of sense but rather just to the select few things which it
 really makes sense for and then having a function which can be used by
 users to do the same for results from other operations.

 I guess I'm questioning the wisdom of keeping it for row_to_json given 
 that it doesn't operate recursively anyway (and making it do so would be 
 difficult and ugly).

IMO, adding it to row_to_json was really ugly to start with, independently
of whether it's difficult or not.  That function had one too many optional
arguments already, and in its current form it's nothing but a loaded gun
pointed at users' feet.  (Quick, which bool argument is which?)

 If we're going to keep this, I think we also need to provide it 
 (non-recursive) for json_agg via an optional second argument. This 
 should be a fairly simple change: just steer the result via 
 composite_to_json if it's a record, rather than to datum_to_json.

Unfortunately, any such thing will fall foul of an established project
policy.  I quote the opr_sanity regression test that will complain:

-- Check that there are not aggregates with the same name and different
-- numbers of arguments.  While not technically wrong, we have a project policy
-- to avoid this because it opens the door for confusion in connection with
-- ORDER BY: novices frequently put the ORDER BY in the wrong place.
-- See the fate of the single-argument form of string_agg() for history.

So my vote is for a separate function and no optional arguments.

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] Last Commitfest patches waiting review

2014-09-27 Thread Peter Geoghegan
On Sat, Sep 27, 2014 at 1:18 AM, Heikki Linnakangas
hlinnakan...@vmware.com wrote:
 Sort support for text with strxfrm() poor man's keys
 ---

 Peter: Are you waiting for Robert to review this? Robert, could you review
 the latest patch, please? Peter: Could you try to get rid of the extra
 SortSupport object that Robert didn't like?
 (http://www.postgresql.org/message-id/ca+tgmobde+ydfnhts0gwpt54-er8bpt3vx8rpshd+98ctdo...@mail.gmail.com).
 I think it would speed up the process if you did that, instead of waiting
 for Robert to find the time.

I am not waiting on Robert to spend the time, FWIW. The question that
resolving if we should not have an extra sortsupport object is
blocking on is the need to have a consistent sorttuple.datum1
representation for the benefit of having comparetup_heap() know that
it's either always abbreviated keys or always pointers to text. My
view is that it's not worth going back to fix up datum1 to always be a
pointer to text when we abort abbreviation - I think we should just
forget about datum1 on the rare occasion that happens (due to the
costs involved, as well as the complexity implied).

I think that it will be necessary for me to rigorously prove that
view, as with the memcmp() == 0 thing. So I'm looking at that.

-- 
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] json (b) and null fields

2014-09-27 Thread Andrew Dunstan


On 09/27/2014 06:27 PM, Tom Lane wrote:

Andrew Dunstan and...@dunslane.net writes:

On 09/27/2014 08:00 AM, Stephen Frost wrote:

Yeah, I don't see adding this option to all json generator functions as
making a lot of sense but rather just to the select few things which it
really makes sense for and then having a function which can be used by
users to do the same for results from other operations.

I guess I'm questioning the wisdom of keeping it for row_to_json given
that it doesn't operate recursively anyway (and making it do so would be
difficult and ugly).

IMO, adding it to row_to_json was really ugly to start with, independently
of whether it's difficult or not.  That function had one too many optional
arguments already, and in its current form it's nothing but a loaded gun
pointed at users' feet.  (Quick, which bool argument is which?)


If we're going to keep this, I think we also need to provide it
(non-recursive) for json_agg via an optional second argument. This
should be a fairly simple change: just steer the result via
composite_to_json if it's a record, rather than to datum_to_json.

Unfortunately, any such thing will fall foul of an established project
policy.  I quote the opr_sanity regression test that will complain:

-- Check that there are not aggregates with the same name and different
-- numbers of arguments.  While not technically wrong, we have a project policy
-- to avoid this because it opens the door for confusion in connection with
-- ORDER BY: novices frequently put the ORDER BY in the wrong place.
-- See the fate of the single-argument form of string_agg() for history.

So my vote is for a separate function and no optional arguments.






You mean like row_to_json_no_nulls() and json_agg_no_nulls()?

cheers

andrew


--
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: rounding up time value less than its unit.

2014-09-27 Thread Gregory Smith

On 9/26/14, 3:22 PM, Tom Lane wrote:
We could alternatively try to split up these cases into multiple GUCs, 
which I guess is what you're imagining as a multi-year battle.


No, I was just pointing out that even the cleanest major refactoring 
possible here is going to result in broken config files complaints for 
years.  And no matter how well that goes, a second rewrite in the next 
major version, addressing whatever things nobody saw coming in the first 
redesign, is a very real possibility.  The minute the GUC box is opened 
that far, it will not close up again in a release, or likely even two, 
and the griping from the field is going to take 3+ years to settle.


I have no desire to waste time on partial measures either though.  I 
think I've been clear that's my theme on this--if we're gonna mess with 
this significantly, let's do it right and in a big way.  If there's a 
really trivial fix to apply, that's fine too.  Throw an error when the 
value is between the special one and the useful minimum, no other 
changes; that I could see slipping into a single release without much 
pain.  Might even be possible to write as a useful sub-step toward a 
bigger plan too.  Wouldn't bother breaking that out as a goal unless it 
just happened to fall out of the larger design though.


The rest of the rounding and error handling ideas wandering around seem 
in this uncomfortable middle ground to me.  They are neither large 
enough to feel like a major improvement happened, nor trivial enough to 
apply with minimal work/risk.  And this is not a bug that must be fixed 
ASAP--it's an unfriendly UI surprise.


--
Greg Smith greg.sm...@crunchydatasolutions.com
Chief PostgreSQL Evangelist - http://crunchydatasolutions.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] Last Commitfest patches waiting review

2014-09-27 Thread Gregory Smith

On 9/27/14, 4:18 AM, Heikki Linnakangas wrote:

add latency limit to pgbench throttling (--rate)
---
Rukh: Are you planning to review the latest patch version?


Rukh is free to jump onto this ASAP, but if it's still there next week I 
already had my eye on picking that up and taking it out for a spin.  I 
already updated my pgbench-tools set to incorporate the rate limit for 
9.4, and I use that part all the time now.  I think I understand 
Fabien's entire game plan for this now, and I want the remainder of the 
set to land in 9.5.


--
Greg Smith greg.sm...@crunchydatasolutions.com
Chief PostgreSQL Evangelist - http://crunchydatasolutions.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] INSERT ... ON CONFLICT {UPDATE | IGNORE}

2014-09-27 Thread Peter Geoghegan
On Thu, Sep 25, 2014 at 1:48 PM, Simon Riggs si...@2ndquadrant.com wrote:
 At this stage, poll the Django and Rails communities for acceptance
 and early warning of these features. Listen.

FYI, I have asked for input from the Django developers here:

https://groups.google.com/forum/#!topic/django-developers/hdzkoLYVjBY

-- 
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] json (b) and null fields

2014-09-27 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes:
 On 09/27/2014 06:27 PM, Tom Lane wrote:
 So my vote is for a separate function and no optional arguments.

 You mean like row_to_json_no_nulls() and json_agg_no_nulls()?

I thought you were proposing that we should revert the committed patch
lock-stock-n-barrel, and instead invent json_strip_null_fields().
That's instead, not in addition to.  Even if you weren't saying that
exactly, that's where my vote goes.

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] json (b) and null fields

2014-09-27 Thread Stephen Frost
All,

On Saturday, September 27, 2014, Andrew Dunstan and...@dunslane.net wrote:


 On 09/27/2014 10:52 PM, Tom Lane wrote:

 Andrew Dunstan and...@dunslane.net writes:

 On 09/27/2014 06:27 PM, Tom Lane wrote:

 So my vote is for a separate function and no optional arguments.

 You mean like row_to_json_no_nulls() and json_agg_no_nulls()?

 I thought you were proposing that we should revert the committed patch
 lock-stock-n-barrel, and instead invent json_strip_null_fields().
 That's instead, not in addition to.  Even if you weren't saying that
 exactly, that's where my vote goes.



 I was just exploring alternatives. But I think that's where my vote goes
 too.


I'm fine with that. I'd like the strip-Nulls capability, but seems like
it'd be better off as an independent function (or functions) instead.

Thanks,

Stephen


Re: [HACKERS] Patch to support SEMI and ANTI join removal

2014-09-27 Thread David Rowley
On Fri, Sep 26, 2014 at 12:36 AM, Heikki Linnakangas 
hlinnakan...@vmware.com wrote:

 On 09/16/2014 01:20 PM, David Rowley wrote:

 +   /*
 +* We mustn't allow any joins to be removed if there are any
 pending
 +* foreign key triggers in the queue. This could happen if we are
 planning
 +* a query that has been executed from within a volatile function
 and the
 +* query which called this volatile function has made some
 changes to a
 +* table referenced by a foreign key. The reason for this is that
 any
 +* updates to a table which is referenced by a foreign key
 constraint will
 +* only have the referencing tables updated after the command is
 complete,
 +* so there is a window of time where records may violate the
 foreign key
 +* constraint.
 +*
 +* Currently this code is quite naive, as we won't even attempt
 to remove
 +* the join if there are *any* pending foreign key triggers, on
 any
 +* relation. It may be worthwhile to improve this to check if
 there's any
 +* pending triggers for the referencing relation in the join.
 +*/
 +   if (!AfterTriggerQueueIsEmpty())
 +   return false;



Hi Heikki,

Thanks for having a look at the patch.

Hmm. This code runs when the query is planned. There is no guarantee that
 there won't be after triggers pending when the query is later *executed*.


Please correct anything that sounds wrong here, but my understanding is
that we'll always plan a query right before we execute it, with the
exception of PREPARE statements where PostgreSQL will cache the query plan
when the prepare statement is first executed. So I think you may have a
point here regarding PREPARE'd statements, but I think that it is isolated
to those.

I think in all other cases we'll plan right before we execute. So if we
happen to be planning an UPDATE statement which has a sub-query that
perform some INNER JOINs, I think we're safe to remove INNER JOINs when
possible, as the UPDATE statement won't get visibility of its own changes.

We can see that here:

create table updatetest (id int primary key, value int, value2 int);

create or replace function getvalue2(p_id int) returns int
as $$select value2 from updatetest where id = p_id$$
language sql volatile;


insert into updatetest values(0,0,0);
insert into updatetest values(1,10,10);
insert into updatetest values(2,20,20);
insert into updatetest values(3,30,30);

update updatetest set value = COALESCE((select value from updatetest u2
where updatetest.id - 1 = u2.id) + 1,0);

update updatetest set value2 = COALESCE(getvalue2(id - 1) + 1,0);

select * from updatetest;
 id | value | value2
+---+
  0 | 0 |  0
  1 | 1 |  1
  2 |11 |  2
  3 |21 |  3

The value column appears to have been set based on the value that was
previously in the value column, and has not come from the newly set value.
The behaviour is different for the value2 column as the value for this has
been fetched from another query, which *does* see the newly updated value
stored in the value2 column.

My understanding of foreign keys is that any pending foreign key triggers
will be executed just before the query completes, so we should only ever
encounter pending foreign key triggers during planning when we're planning
a query that's being executed from somewhere like a volatile function or
trigger function, if the outer query has updated or deleted some records
which are referenced by a foreign key.

So I think with the check for pending triggers at planning time this is
safe at least for queries being planned right before they're executed, but
you've caused me to realise that I'll probably need to do some more work on
this for when it comes to PREPARE'd queries, as it looks like if we
executed a prepared query from inside a volatile function or trigger
function that was called from a DELETE or UPDATE statement that caused
foreign key triggers to be queued, and we'd happened to have removed some
INNER JOINs when we originally planned that prepare statement, then that
would be wrong.

The only thing that comes to mind to fix that right now is to tag something
maybe in PlannerInfo to say if we've removed any INNER JOINs in planning,
then when we execute a prepared statement we could void the cached plan we
see that some INNER JOINs were removed, but only do this if the foreign key
trigger queue has pending triggers. (which will hopefully not be very
often).

Another thing that comes to mind which may be similar is how we handle
something like:

PREPARE a AS SELECT * from tbl WHERE name LIKE $1;

Where, if $1 is 'foo' or 'foo%' we might want to use an index scan, but if
$1 was '%foo' then we'd probably not.
I've not yet looked into great detail of what happens here, but from some
quick simple tests it seems to replan each time!? But perhaps that'd due to
the parameter, where with my other tests the 

[HACKERS] Missing newlines in verbose logs of pg_dump, introduced by RLS patch

2014-09-27 Thread Michael Paquier
Hi all,

Recent commit 491c029 introducing RLS has broken a bit the verbose logs of
pg_dump, one message missing a newline:
+   if (g_verbose)
+   write_msg(NULL, reading row-security enabled for
table \%s\,
+ tbinfo-dobj.name);
The patch attached corrects that.
Regards,
-- 
Michael


20140928_rls_pgdump_fix.patch
Description: Binary data

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