date:20140318


On 03/18/2014 11:57 AM, KONDO Mitsumasa wrote:

I think that this feature will be also useful for survey new buffer-replace
algorithm and checkpoint strategy, so on.


Sure. No doubt about that.


If we remove this option, it is really dissapointed..


As long as we get the \setrandom changes in, you can easily do these 
tests using a custom script. There's nothing wrong with using a custom 
script, it will be just as useful for exploring buffer replacement 
algorithms, checkpoints etc. as a built-in option.


- 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_dump without explicit table locking

2014-03-18 Thread Jürgen Strobel

On 18.03.14 00:15, Tom Lane wrote:
 Jim Nasby j...@nasby.net writes:
 On 3/17/14, 8:47 AM, Tom Lane wrote:
 (Note that this is only one of assorted O(N^2) behaviors in older versions
 of pg_dump; we've gradually stamped them out over time.)
 
 On that note, it's recommended that when you are taking a backup to restore 
 into a newer version of Postgres you create the dump using the NEWER version 
 of pg_dump, not the old one.
 
 Right.  IIRC, the OP said he *did* use a recent pg_dump ... but this
 particular issue got fixed server-side, so the new pg_dump didn't help
 against an 8.1 server :-(


Yes, I did use 9.3's pg_dump against my 8.1 DB initially.

The patch was created against github's master.

-Jürgen


-- 
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] ALTER TABLE lock strength reduction patch is unsafe Reply-To:

On 8 March 2014 11:14, Simon Riggs si...@2ndquadrant.com wrote:
 On 7 March 2014 09:04, Simon Riggs si...@2ndquadrant.com wrote:

 The right thing to do here is to not push to the extremes. If we mess
 too much with the ruleutil stuff it will just be buggy. A more
 considered analysis in a later release is required for a full and
 complete approach. As I indicated earlier, an 80/20 solution is better
 for this release.

 Slimming down the patch, I've removed changes to lock levels for
 almost all variants. The only lock levels now reduced are those for
 VALIDATE, plus setting of relation and attribute level options.

 VALIDATE is implemented by calling pg_get_constraintdef_mvcc(), a
 slightly modified variant of pg_get_constraintdef that uses the
 transaction snapshot. I propose this rather than Noah's solution
 solely because this will allow any user to request the MVCC data,
 rather than implement a hack that only works for pg_dump. I will post
 the patch later today.

 Implemented in attached patch, v22

 The following commands (only) are allowed with
 ShareUpdateExclusiveLock, patch includes doc changes.

 ALTER TABLE ... VALIDATE CONSTRAINT constraint_name
 covered by isolation test, plus verified manually with pg_dump

 ALTER TABLE ... ALTER COLUMN ... SET STATISTICS
 ALTER TABLE ... ALTER COLUMN ... SET (...)
 ALTER TABLE ... ALTER COLUMN ... RESET (...)

 ALTER TABLE ... CLUSTER ON ...
 ALTER TABLE ... SET WITHOUT CLUSTER
 ALTER TABLE ... SET (...)
 covered by isolation test

 ALTER TABLE ... RESET (...)

 ALTER INDEX ... SET (...)
 ALTER INDEX ... RESET (...)

 All other ALTER commands take AccessExclusiveLock

 I commend this patch to you for final review; I would like to commit
 this in a few days.

I'm planning to commit this today at 1500UTC barring objections or
negative reviews.

-- 
 Simon Riggs   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] GSoC question

2014-03-18 Thread Artur Gadelshin

Hi, PostgreSQL community!

My name is Artur Gadelshin, I'm from Saint-Petersburg, Russia. I'm 4th year
student in Computer Scince.

I haven't enough experience in C/C++, but I really in love with PostgreSQL
(but, yes, haven't experience to develop this). My dream is to be computer
scientist or data scientist. So, I should read a lot of books about data
structures, mathematics, algorithms and so on.

Is there any chance to participate in your community and get some help in
envolving, may be simple tasks, advices about books which I should read or
smth else.

If I will be useless in GSoC projects, I glad to participate without GSoC
with your help and advices.

What I already know:

Programming languages: Erlang, Python, basic knowledge of
Clojure/Java/Scala/C

OS: advanced knowledge of Linux, I'm very interested in learning, how
operating systems works and Linux kernel development.DBMS: SQL (MariaDB,
MySQL), Riak, CouchDB, PostgreSQL

Experience: RESTful web service development: API for cloud storage, VDS
service and internal hosting company backend with Python, Erlang,
cowboy_rest, MySQL/Riak.

Re: [HACKERS] GSoC question

2014-03-18 Thread Stephen Frost

Artur,

* Artur Gadelshin (ar.gadels...@gmail.com) wrote:
 Is there any chance to participate in your community and get some help in
 envolving, may be simple tasks, advices about books which I should read or
 smth else.

There are a number of GSoC project ideas here:

https://wiki.postgresql.org/wiki/GSoC_2014

 If I will be useless in GSoC projects, I glad to participate without GSoC
 with your help and advices.

It's up to you to submit a proposal for GSoC, if you're interested in
participating and meet the requirements.  We'd certainly welcome new
members to the community even if you don't want to participate in GSoC.

Thanks,

Stephen


signature.asc
Description: Digital signature

[HACKERS] B-tree descend for insertion locking

When inserting into a B-tree index, all the pages are read-locked when 
descending the tree. When we reach the leaf page, the read-lock is 
exchanged for a write-lock.


There's nothing wrong with that, but why don't we just directly grab a 
write-lock on the leaf page? When descending, we know the level we're 
on, and what level the child page is. The only downside I can see is 
that we would unnecessarily hold a write-lock when a read-lock would 
suffice, if the page was just split and we have to move right. But that 
seems like a really bad bet - hitting the page when it was just split is 
highly unlikely.


Grabbing the write-lock directly would obviously save one buffer 
unlock+lock sequence, for what it's worth, but I think it would also 
slightly simplify the code. Am I missing something?


See attached patch. The new contract of _bt_getroot is a bit weird: it 
locks the returned page in the requested lock-mode, shared or exclusive, 
if the root page was also the leaf page. Otherwise it's locked in shared 
mode regardless off the requested lock mode. But OTOH, the new contract 
for _bt_search() is more clear now: it actually locks the returned page 
in the requested mode, where it used to only use the access parameter to 
decide whether to create a root page if the index was empty.


- Heikki
diff --git a/src/backend/access/nbtree/nbtinsert.c b/src/backend/access/nbtree/nbtinsert.c
index 5f7953f..3cb6404 100644
--- a/src/backend/access/nbtree/nbtinsert.c
+++ b/src/backend/access/nbtree/nbtinsert.c
@@ -113,24 +113,11 @@ _bt_doinsert(Relation rel, IndexTuple itup,
 	itup_scankey = _bt_mkscankey(rel, itup);
 
 top:
-	/* find the first page containing this key */
+	/* find the first page containing this key, and write-lock it */
 	stack = _bt_search(rel, natts, itup_scankey, false, buf, BT_WRITE);
 
 	offset = InvalidOffsetNumber;
 
-	/* trade in our read lock for a write lock */
-	LockBuffer(buf, BUFFER_LOCK_UNLOCK);
-	LockBuffer(buf, BT_WRITE);
-
-	/*
-	 * If the page was split between the time that we surrendered our read
-	 * lock and acquired our write lock, then this page may no longer be the
-	 * right place for the key we want to insert.  In this case, we need to
-	 * move right in the tree.	See Lehman and Yao for an excruciatingly
-	 * precise description.
-	 */
-	buf = _bt_moveright(rel, buf, natts, itup_scankey, false, BT_WRITE);
-
 	/*
 	 * If we're not allowing duplicates, make sure the key isn't already in
 	 * the index.
diff --git a/src/backend/access/nbtree/nbtpage.c b/src/backend/access/nbtree/nbtpage.c
index 14e422a..d6a4933 100644
--- a/src/backend/access/nbtree/nbtpage.c
+++ b/src/backend/access/nbtree/nbtpage.c
@@ -78,12 +78,13 @@ _bt_initmetapage(Page page, BlockNumber rootbknum, uint32 level)
  *		standard class of race conditions exists here; I think I covered
  *		them all in the Hopi Indian rain dance of lock requests below.
  *
- *		The access type parameter (BT_READ or BT_WRITE) controls whether
- *		a new root page will be created or not.  If access = BT_READ,
- *		and no root page exists, we just return InvalidBuffer.	For
- *		BT_WRITE, we try to create the root page if it doesn't exist.
- *		NOTE that the returned root page will have only a read lock set
- *		on it even if access = BT_WRITE!
+ *		The access type parameter (BT_READ or BT_WRITE) indicates whether
+ *		we're fetching the root for a search or an insertion.  If access =
+ *		BT_READ, and no root page exists, we just return InvalidBuffer.
+ *		For BT_WRITE, we try to create the root page if it doesn't exist.
+ *		If the root page is also a leaf-page, it is locked in the mode
+ *		specified.  NOTE that otherwise the returned root page will have
+ *		only a read lock on it,	even if access = BT_WRITE!
  *
  *		The returned page is not necessarily the true root --- it could be
  *		a fast root (a page that is alone in its level due to deletions).
@@ -127,7 +128,8 @@ _bt_getroot(Relation rel, int access)
 		Assert(rootblkno != P_NONE);
 		rootlevel = metad-btm_fastlevel;
 
-		rootbuf = _bt_getbuf(rel, rootblkno, BT_READ);
+		rootbuf = _bt_getbuf(rel, rootblkno,
+			 (rootlevel == 0) ? access : BT_READ);
 		rootpage = BufferGetPage(rootbuf);
 		rootopaque = (BTPageOpaque) PageGetSpecialPointer(rootpage);
 
@@ -253,14 +255,6 @@ _bt_getroot(Relation rel, int access)
 
 		END_CRIT_SECTION();
 
-		/*
-		 * swap root write lock for read lock.	There is no danger of anyone
-		 * else accessing the new root page while it's unlocked, since no one
-		 * else knows where it is yet.
-		 */
-		LockBuffer(rootbuf, BUFFER_LOCK_UNLOCK);
-		LockBuffer(rootbuf, BT_READ);
-
 		/* okay, metadata is correct, release lock on it */
 		_bt_relbuf(rel, metabuf);
 	}
@@ -285,7 +279,8 @@ _bt_getroot(Relation rel, int access)
 
 		for (;;)
 		{
-			rootbuf = _bt_relandgetbuf(rel, rootbuf, rootblkno, BT_READ);
+			rootbuf = _bt_relandgetbuf(rel, rootbuf, rootblkno,
+	   (rootlevel == 0) ? access : BT_READ);
 			rootpage =

Re: [HACKERS] Triggers on foreign tables

On Mon, Mar 17, 2014 at 11:54 PM, Kouhei Kaigai kai...@ak.jp.nec.com wrote:
 I hacked on this for awhile, but there remain two matters on which I'm
 uncertain about the right way forward.

 (1) To acquire the old tuple for UPDATE/DELETE operations, the patch closely
 parallels our handling for INSTEAD OF triggers on views.  It adds a wholerow
 resjunk attribute, from which it constructs a HeapTuple before calling a
 trigger function.  This loses the system columns, an irrelevant
 consideration for views but meaningful for foreign tables.  postgres_fdw
 maintains the ctid system column (t_self), but triggers will always see
 an invalid t_self for the old tuple.  One way to fix this is to pass around
 the old tuple data as ROW(ctid, oid, xmin, cmin, xmax, cmax, tableoid,
 wholerow).  That's fairly close to sufficient, but it doesn't cover t_ctid.
 Frankly, I would like to find a principled excuse to not worry about making
 foreign table system columns accessible from triggers.  Supporting system
 columns dramatically affects the mechanism, and what trigger is likely to
 care?  Unfortunately, that argument seems too weak.  Does anyone have a
 cleaner idea for keeping track of the system column values or a stronger
 argument for not bothering?

 Regarding to the first suggestion,
 I think, it is better not to care about system columns on foreign tables,
 because it fully depends on driver's implementation whether FDW fetches
 ctid from its data source, or not.
 Usually, system columns of foreign table, except for tableoid, are nonsense.
 Because of implementation reason, postgres_fdw fetches ctid of remote
 tables on UPDATE / DELETE, it is not a common nature for all FDW drivers.
 For example, we can assume an implementation that uses primary key of remote
 table to identify the record to be updated or deleted. In this case, local
 ctid does not have meaningful value.
 So, fundamentally, we cannot guarantee FDW driver returns meaningful ctid
 or other system columns.

I'm not sure I particularly agree with this reasoning - after all,
just because some people might not find a feature useful isn't a
reason not to have it.  On the other hand, I don't think it's a very
useful feature, and I don't feel like we have to have it.  Most system
columns can't be updated or indexed, and none of them can be dropped
or renamed, so it's not like they aren't second-class citizens to some
degree already.

By way of comparison, the first version of the index-only scan patch
gave up when it saw an expression index, instead of making an effort
to figure out whether a matching expression was present in the query.
Somebody could have looked at that patch and said, oh, well, that's an
ugly and unacceptable limitation, and we ought to reject the patch
until it's fixed.  Well, instead, Tom committed the patch, and we
still have that limitation today, and it's still something we really
ought to fix some day, but in the meantime we have the feature.

Obviously, the line between your patch is only part of a feature,
please finish it and try again and your patch implements a nice
self-contained feature and there are some more things that we could
build on top of it later is to some extent a matter of judgement; but
for what it's worth, I can't get too excited about this particular
limitation of this particular patch.  I just don't think that very
many people are going to miss the functionality in question.

-- 
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] Replication slots and footguns

On Thu, Mar 13, 2014 at 9:07 PM, Josh Berkus j...@agliodbs.com wrote:
 On 03/13/2014 05:28 PM, Robert Haas wrote:
 Well we may have kind of hosed ourselves, because the in-memory data
 structures that represent the data structure have an in_use flag that
 indicates whether the structure is allocated at all, and then an
 active flag that indicates whether some backend is using it.  I never
 liked that naming much.  Maybe we should go through and let in_use -
 allocated and active - in_use.

 Wait, which one of those does pg_drop_replication_slot() care about?

Well... the slots that aren't in_use can't be dropped because they
don't exist in the first place.  The ones that aren't active can't be
dropped because somebody else is using them.  So both, sorta, I guess?

-- 
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] on_exit_reset fails to clear DSM-related exit actions

On Mon, Mar 17, 2014 at 1:41 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 After mulling over a few possible approaches, I came up with the
 attached, which seems short and to the point.

 Looks reasonable in principle.  I didn't run through all the existing
 PGSharedMemoryDetach calls to see if there are any other places to
 call dsm_detach_all, but it's an easy fix if there are any.

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] plpgsql.warn_shadow



On 14/03/14 13:12, Simon Riggs wrote:

On 14 March 2014 11:10, Pavel Stehule pavel.steh...@gmail.com wrote:



2014-03-14 12:02 GMT+01:00 Marko Tiikkaja ma...@joh.to:


On 3/14/14 10:56 AM, Simon Riggs wrote:

The patch looks fine, apart from some non-guideline code formatting
issues.


I'm not sure what you're referring to.  I thought it looked fine.



Having looked at gcc and clang, I have a proposal for naming/API

We just have two variables

plpgsql.compile_warnings = 'list'default = 'none'


+1


plpgsql.compile_errors = 'list'default = 'none'

Only possible values in 9.4 are 'shadow', 'all', 'none'


what is compile_errors ? We don't allow to ignore any error now.

How about

plpgsql.additional_warnings = 'list'
plpgsql.additional_errors = 'list'



Agree that compile_errors dos not make sense, additional_errors and 
additional_warnings seems better, maybe plpgsql.extra_warnings and 
plpgsql.extra_errors?


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



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

Re: [HACKERS] pg_dump without explicit table locking

2014-03-18 Thread Jürgen Strobel

On 18.03.14 02:32, Joe Conway wrote:
 On 03/17/2014 05:55 PM, Jeff Janes wrote:
 On Mon, Mar 17, 2014 at 5:48 PM, Craig Ringer
 cr...@2ndquadrant.com I wonder if doing large batches of
 
 LOCK TABLE table1, table2, table3, ...
 
 would help, instead of doing individual statements?
 
 If I recall correctly, someone did submit a patch to do that. It
 helped when dumping schema only, but not much when dumping data.
 
 Not surprising at all. The huge time is incurred in taking the locks,
 but if you are trying to use pg_upgrade in link mode to speed your
 upgrade, you are totally hosed by the time it takes to grab those locks.
 
 This patch applied to 9.3 substantially fixes the issue:
 8---
 commit eeb6f37d89fc60c6449ca12ef9e91491069369cb
 Author: Heikki Linnakangas heikki.linnakan...@iki.fi
 Date:   Thu Jun 21 15:01:17 2012 +0300
 
 Add a small cache of locks owned by a resource owner in ResourceOwner.
 8---
 
 On my 8.4 database, with 500,000 tables there were about 2.5 million
 locks taken including toast tables and indexes during the schema dump.
 Without the patch grabbing locks took many, many days with that many
 objects to lock. With a backported version of the patch, one hour.
 
 So if you have a problem due to many tables on an older than 9.3
 version of Postgres, this is the direction to head (a custom patch
 applied to your old version just long enough to get successfully
 upgraded).
 

In a testing environment I restored my 8.1 DB with 300,000 tables to a
9.3 server (using my patched pg_dump).

Then I ran the original 9.3 pg_dump against the 9.3 DB again, and it
works reasonably well. So I can confirm the server side improvements in
9.3 do to work for my test case.

Still when I finally get around to do this on production I plan to use
my patched pg_dump rather than backporting the server fix to 8.1, as I'd
rather not touch our already-patched-for-something-else 8.1 server.

I can't wait to get my hand on 9.x replication features and other stuff :-)

-Jürgen








-- 
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] HEAD seems to generate larger WAL regarding GIN index

2014-03-18 Thread Fujii Masao

On Mon, Mar 17, 2014 at 10:44 PM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
 Fujii Masao escribió:
 On Sun, Mar 16, 2014 at 7:15 AM, Alexander Korotkov
 aekorot...@gmail.com wrote:

  That could be optimized, but I figured we can live with it, thanks to the
  fastupdate feature. Fastupdate allows amortizing that cost over several
  insertions. But of course, you explicitly disabled that...
 
  Let me know if you want me to write patch addressing this issue.

 Yeah, I really want you to address this problem! That's definitely useful
 for every users disabling FASTUPDATE option for some reasons.

 Users that disable FASTUPDATE, in my experience, do so because their
 stock work_mem is way too high and GIN searches become too slow due to
 having to scan too large a list.

Yes. Another reason that I've heard from users so far is that
the size of GIN index with FASTUPDATE=off is likely to be smaller
than that with FASTUPDATE=on.

 I think it might make sense to invest
 a modest amount of time in getting FASTUPDATE to be sized completely
 differently from today -- perhaps base it on a hardcoded factor of
 BLCKSZ, rather than work_mem.  Or, if we really need to make it
 configurable, then let it have its own parameter.

I prefer to have the parameter. When users create multiple GIN indexes
for various uses, they might want to use different thresholds of the pending
list for each index. So, GIN index parameter might be better than GUC one.

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] [COMMITTERS] pgsql: Make it easy to detach completely from shared memory.

On 18 March 2014 11:59, Robert Haas rh...@postgresql.org wrote:
 Make it easy to detach completely from shared memory.

 The new function dsm_detach_all() can be used either by postmaster
 children that don't wish to take any risk of accidentally corrupting
 shared memory; or by forked children of regular backends with
 the same need.  This patch also updates the postmaster children that
 already do PGSharedMemoryDetach() to do dsm_detach_all() as well.

 Per discussion with Tom Lane.

I think we need to document exactly why dsm_detach_all() isn't simply
part of PGSharedMemoryDetach() ?

Having two calls seems like a recipe for error in core and extensions.

Perhaps we should consider a parameter for PGSharedMemoryDetach() ?

-- 
 Simon Riggs   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] plpgsql.warn_shadow

2014-03-18 13:23 GMT+01:00 Petr Jelinek p...@2ndquadrant.com:


 On 14/03/14 13:12, Simon Riggs wrote:

 On 14 March 2014 11:10, Pavel Stehule pavel.steh...@gmail.com wrote:



 2014-03-14 12:02 GMT+01:00 Marko Tiikkaja ma...@joh.to:

  On 3/14/14 10:56 AM, Simon Riggs wrote:

 The patch looks fine, apart from some non-guideline code formatting
 issues.


 I'm not sure what you're referring to.  I thought it looked fine.


  Having looked at gcc and clang, I have a proposal for naming/API

 We just have two variables

 plpgsql.compile_warnings = 'list'default = 'none'


 +1

  plpgsql.compile_errors = 'list'default = 'none'

 Only possible values in 9.4 are 'shadow', 'all', 'none'


 what is compile_errors ? We don't allow to ignore any error now.

 How about

 plpgsql.additional_warnings = 'list'
 plpgsql.additional_errors = 'list'


 Agree that compile_errors dos not make sense, additional_errors and
 additional_warnings seems better, maybe plpgsql.extra_warnings and
 plpgsql.extra_errors?


extra* sounds better

Pavel



 --
  Petr Jelinek   http://www.2ndQuadrant.com/

  PostgreSQL Development, 24x7 Support, Training  Services

Re: [HACKERS] plpgsql.warn_shadow

On 18 March 2014 12:23, Petr Jelinek p...@2ndquadrant.com wrote:

 Agree that compile_errors dos not make sense, additional_errors and
 additional_warnings seems better, maybe plpgsql.extra_warnings and
 plpgsql.extra_errors?

That sems better to me also.

-- 
 Simon Riggs   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] [COMMITTERS] pgsql: Make it easy to detach completely from shared memory.

On Tue, Mar 18, 2014 at 8:41 AM, Simon Riggs si...@2ndquadrant.com wrote:
 On 18 March 2014 11:59, Robert Haas rh...@postgresql.org wrote:
 Make it easy to detach completely from shared memory.

 The new function dsm_detach_all() can be used either by postmaster
 children that don't wish to take any risk of accidentally corrupting
 shared memory; or by forked children of regular backends with
 the same need.  This patch also updates the postmaster children that
 already do PGSharedMemoryDetach() to do dsm_detach_all() as well.

 Per discussion with Tom Lane.

 I think we need to document exactly why dsm_detach_all() isn't simply
 part of PGSharedMemoryDetach() ?

 Having two calls seems like a recipe for error in core and extensions.

 Perhaps we should consider a parameter for PGSharedMemoryDetach() ?

Yeah, maybe.  It seems like a possible modularity violation, because
the PGSharedMemory... stuff has heretofore not needed to know anything
about DSM, and apart from this one function, it still wouldn't.  On
the other hand, your argument is good, 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

Re: [HACKERS] Is this a bug?

On Mon, Mar 17, 2014 at 10:27 PM, Michael Paquier
michael.paqu...@gmail.com wrote:
 On Tue, Mar 18, 2014 at 10:24 AM, Fabrízio de Royes Mello
 fabriziome...@gmail.com wrote:

 On Thu, Mar 13, 2014 at 10:22 AM, Robert Haas robertmh...@gmail.com wrote:
 Well, it's fairly harmless, but it might not be a bad idea to tighten that
 up.
 The attached patch tighten that up.
 Hm... It might be interesting to include it in 9.4 IMO, somewhat
 grouping with what has been done in a6542a4 for SET and ABORT.

Meh.  There will always be another thing we could squeeze in; I don't
think this is particularly urgent, and it's late to the party.

-- 
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] GSoC proposal. Index-only scans for GIST

2014-03-18 Thread Anastasia Lubennikova

Hello!
Here is the text of my proposal which I've applied to GSoC.
(and link
http://www.google-melange.com/gsoc/proposal/public/google/gsoc2014/lubennikovaav/5629499534213120)
Any suggestions and comments are welcome.

*Project name*

Support for index-only scans for GIST index

*Brief review*

Currently GiST index don't have index-only scan functionality. Index-only
scans are a major performance feature added to Postgres 9.2. They allow
certain types of queries to be satisfied just by retrieving data from
indexes, and not from tables. This feature for B-trees (implemented since
PostgreSQL-9.2) allows doing certain types of queries significantly faster.
This is achieved by reduction in the amount of I/O necessary to satisfy
queries. I think it will be useful to implement this feature for user
defined data types that use GiST index.

*Benefits to the PostgreSQL Community*

Faster GiST index search would be actual for many PostgreSQL applications
(for example some GIS systems).

*Quantifiable results*

Acceleration of GiST index search.

*Project details*

1. The GiST is a balanced tree structure like a B-tree, containing key,
pointer pairs. GiST key is a member of a user-defined class, and
represents some property that is true of all data items reachable from the
pointer associated with the key. The GiST provides a possibility to create
custom data types with indexed access methods and extensible set of queries.

There are seven methods that an index operator class for GiST must provide,
and an eighth that is optional.

-consistent

-union

-compress

-decompress

-penalty

-picksplit

-equal

-distance (optional)

I'm going to create new optional method fetch() in addition to existing.
Thus user can create a method of retrieving data from the index without
loss. It will be used when performing search queries to speed data
retrieving.

2. gistget algorithm (several parts omitted):

Check the key
gistindex_keytest() - does this index tuple satisfy the scan key(s)?

Scan all items on the GiST index page and insert them into the queue (or
directly to output areas)

plain scan

Heap tuple TIDs are returned into so-pageData[]

ordered scan

Heap tuple TIDs are pushed into individual search queue items

If the fetch() is specified by the developer, then using it, algorithm can
retrieve the data directly to output areas at this stage, without reference
to the heap.

3. Create function gistcanreturn to check whether fetch() is specified by
user.

Amcanreturn - Function to check whether index supports index-only scans, or
zero if none

There is the question of support index-only scans for multicolumn indexes.
Probably it would require extend the interface to add separate columns
checking.

To solve this question I need the community's help.

4. Add details for index only scans into gistcostestimate function

*Links*

1) Hellerstein J. M., Naughton J. F., Pfeffer A. Generalized search
trees for database systems. - September, 1995.

2) http://www.sai.msu.su/~megera/postgres/gist/

3) PostgreSQL 9.3.3 Documentation: chapters 11. Indexes, 55. GiST
Indexes.

--
Best regards,
Lubennikova Anastasia

Re: [HACKERS] pg_archivecleanup bug

On Tue, Mar 18, 2014 at 11:25:46AM +0530, Amit Kapila wrote:
 On Thu, Mar 13, 2014 at 11:18 AM, Bruce Momjian br...@momjian.us wrote:
 
  I have developed the attached patch which fixes all cases where
  readdir() wasn't checking for errno, and cleaned up the syntax in other
  cases to be consistent.
 
 
 1. One common thing missed wherever handling for errno is added
 is below check which is present in all existing cases where errno
 is used (initdb.c, pg_resetxlog.c, ReadDir, ..)
 
 #ifdef WIN32
 /*
 * This fix is in mingw cvs (runtime/mingwex/dirent.c rev 1.4), but not in
 * released version
 */
 if (GetLastError() == ERROR_NO_MORE_FILES)
 errno = 0;
 #endif

Very good point.  I have modified the patch to add this block in all
cases where it was missing.  I started to wonder about the comment and
if the Mingw fix was released.  Based on some research, I see this as
fixed in mingw-runtime-3.2, released 2003-10-10.  That's pretty old. 
(What I don't know is when that was paired with Msys in a bundled
release.)  Here is the Mingw fixed code:

http://ftp.ntua.gr/mirror/mingw/OldFiles/mingw-runtime-3.2-src.tar.gz
{
  /* Get the next search entry. */
  if (_tfindnext (dirp-dd_handle, (dirp-dd_dta)))
{
  /* We are off the end or otherwise error.
 _findnext sets errno to ENOENT if no more file
 Undo this. */
  DWORD winerr = GetLastError();
  if (winerr == ERROR_NO_MORE_FILES)
errno = 0;

The current code has a better explanation:


http://sourceforge.net/p/mingw/mingw-org-wsl/ci/master/tree/src/libcrt/tchar/dirent.c
if( dirp-dd_private.dd_stat++  0 )
{
/* Otherwise...
*
* Get the next search entry. POSIX mandates that this must
* return NULL after the last entry has been read, but that it
* MUST NOT change errno in this case. MS-Windows _findnext()
* DOES change errno (to ENOENT) after the last entry has been
* read, so we must be prepared to restore it to its previous
* value, when no actual error has occurred.
*/
int prev_errno = errno;
if( DIRENT_UPDATE( dirp-dd_private ) != 0 )
{
/* May be an error, or just the case described above...
*/
if( GetLastError() == ERROR_NO_MORE_FILES )
/*
* ...which requires us to reset errno.
*/
errno = prev_errno;

but it is basically doing the same thing.  I am wondering if we should
back-patch the PG code block where it was missing, and remove it from
head in all places on the logic that everyone running 9.4 will have a
post-3.1 version of Mingw.  Postgres 8.4 was released in 2009 and it is
possible some people are still using pre-3.2 Mingw versions with that PG
release.

 2.
 ! if (errno || closedir(chkdir) == -1)
   result = -1; /* some kind of I/O error? */
 
 Is there a special need to check return value of closedir in this
 function, as all other uses (initdb.c, pg_resetxlog.c, pgfnames.c)
 of it in similar context doesn't check the same?
 
 One thing I think for which this code needs change is to check
 errno before closedir as is done in initdb.c or pg_resetxlog.c

Yes, good point.  Patch adjusted to add this.

  While I am not a fan of backpatching, the fact we are ignoring errors in
  some critical cases seems the non-cosmetic parts should be backpatched.
 
 +1

The larger the patch gets, the more worried I am about backpatching.

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

  + Everyone has their own god. +


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

[HACKERS] pg_basebackup --slot=SLOTNAME -X stream

2014-03-18 Thread Fujii Masao

Hi,

Is there the explicit reason why --slot option that pg_receivexlog
already has has not been added into pg_basebackup? If not,
I'd like to support that option to eliminate the need to increase
wal_keep_segments for pg_basebackup. Thought?

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] pg_basebackup --slot=SLOTNAME -X stream

2014-03-18 Thread Andres Freund

Hi,

On 2014-03-18 22:25:05 +0900, Fujii Masao wrote:
 Is there the explicit reason why --slot option that pg_receivexlog
 already has has not been added into pg_basebackup? If not,
 I'd like to support that option to eliminate the need to increase
 wal_keep_segments for pg_basebackup. Thought?

I think to make the option really interesting for pg_basebackup we'd
need to make pg_basebackup create the slot, use it for streaming, and
then dump it afterwards. At the moment the walsender interface isn't
expressive enough to do that safely, without any chance of leaving a
slot behind on error. I don't have time to implement the necessary
scaffolding for 9.4.
I wouldn't mind a raw --slot argument, that requires the user to
create/drop the slot outside the calls to pg_basebackup. I just am not
sure how interesting it is to the majority of users.

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] pg_basebackup --slot=SLOTNAME -X stream


On 03/18/2014 03:25 PM, Fujii Masao wrote:

Hi,

Is there the explicit reason why --slot option that pg_receivexlog
already has has not been added into pg_basebackup? If not,
I'd like to support that option to eliminate the need to increase
wal_keep_segments for pg_basebackup. Thought?


That seems rather cumbersome. pg_basebackup is a run once, and then it 
dies. The slot would have to be created before running pg_basebackup, 
and destroyed manually.


It would make sense to provide a new option that would tell the server 
to recycle segments until they've been sent to this client, or until the 
client disconnects. But that would look quite different from 
pg_receivexlog's --slot option.


- 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_basebackup --slot=SLOTNAME -X stream

2014-03-18 Thread Magnus Hagander

On Tue, Mar 18, 2014 at 2:32 PM, Heikki Linnakangas hlinnakan...@vmware.com
 wrote:

 On 03/18/2014 03:25 PM, Fujii Masao wrote:

 Hi,

 Is there the explicit reason why --slot option that pg_receivexlog
 already has has not been added into pg_basebackup? If not,
 I'd like to support that option to eliminate the need to increase
 wal_keep_segments for pg_basebackup. Thought?


 That seems rather cumbersome. pg_basebackup is a run once, and then it
 dies. The slot would have to be created before running pg_basebackup, and
 destroyed manually.

 It would make sense to provide a new option that would tell the server to
 recycle segments until they've been sent to this client, or until the
 client disconnects. But that would look quite different from
 pg_receivexlog's --slot option.


I started working on that at some point long ago, but never got far enough.

Right now, it seems the right way to do that is to somehow reimplement it
on top of a lightweight slot that gets automatically destroyed when
pg_basebackup stops.

But as Andres said, there's a little more to it. You'd want the slot to be
created in the connection that does BASE_BACKUP, then used by the
replication client side, and then destroyed by BASE_BACKUP. But you also
want it auto-destroyed if the BASE_BACKUP connection gets terminated for
example..

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

Re: [HACKERS] pg_archivecleanup bug

Bruce Momjian br...@momjian.us writes:
 Very good point.  I have modified the patch to add this block in all
 cases where it was missing.  I started to wonder about the comment and
 if the Mingw fix was released.  Based on some research, I see this as
 fixed in mingw-runtime-3.2, released 2003-10-10.  That's pretty old. 

Yeah.  I would vote for removing that code in all branches.  There is no
reason to suppose somebody is going to install 8.4.22 on a machine that
they haven't updated mingw on since 2003.  Or, if you prefer, just remove
it in HEAD --- but going around and *adding* more copies seems like
make-work.  The fact that we've not heard complaints about the omissions
is good evidence that nobody's using the buggy mingw versions anymore.

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] [COMMITTERS] pgsql: Make it easy to detach completely from shared memory.

Robert Haas robertmh...@gmail.com writes:
 On Tue, Mar 18, 2014 at 8:41 AM, Simon Riggs si...@2ndquadrant.com wrote:
 Perhaps we should consider a parameter for PGSharedMemoryDetach() ?

 Yeah, maybe.  It seems like a possible modularity violation, because
 the PGSharedMemory... stuff has heretofore not needed to know anything
 about DSM, and apart from this one function, it still wouldn't.

That was exactly the reason we rejected that design upthread.
PGSharedMemoryDetach is specific to the main shmem segment, and in fact
has multiple OS-dependent implementations.

You could make an argument for inventing some new wrapper function that
calls both PGSharedMemoryDetach and dsm_detach_all, but I don't believe
that the existing flavors of that function should know about DSM.

regards, tom lane


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

Re: [HACKERS] pg_archivecleanup bug

On Tue, Mar 18, 2014 at 9:56 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Bruce Momjian br...@momjian.us writes:
 Very good point.  I have modified the patch to add this block in all
 cases where it was missing.  I started to wonder about the comment and
 if the Mingw fix was released.  Based on some research, I see this as
 fixed in mingw-runtime-3.2, released 2003-10-10.  That's pretty old.

 Yeah.  I would vote for removing that code in all branches.  There is no
 reason to suppose somebody is going to install 8.4.22 on a machine that
 they haven't updated mingw on since 2003.  Or, if you prefer, just remove
 it in HEAD --- but going around and *adding* more copies seems like
 make-work.  The fact that we've not heard complaints about the omissions
 is good evidence that nobody's using the buggy mingw versions anymore.

I don't think it is.  Right now we're not checking errno *at all* in a
bunch of these places, so we're sure not going to get complaints about
doing it incorrectly in those places.  Or do I need more caffeine?

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


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

Re: [HACKERS] pg_archivecleanup bug

On Tue, Mar 18, 2014 at 10:03:46AM -0400, Robert Haas wrote:
 On Tue, Mar 18, 2014 at 9:56 AM, Tom Lane t...@sss.pgh.pa.us wrote:
  Bruce Momjian br...@momjian.us writes:
  Very good point.  I have modified the patch to add this block in all
  cases where it was missing.  I started to wonder about the comment and
  if the Mingw fix was released.  Based on some research, I see this as
  fixed in mingw-runtime-3.2, released 2003-10-10.  That's pretty old.
 
  Yeah.  I would vote for removing that code in all branches.  There is no
  reason to suppose somebody is going to install 8.4.22 on a machine that
  they haven't updated mingw on since 2003.  Or, if you prefer, just remove
  it in HEAD --- but going around and *adding* more copies seems like
  make-work.  The fact that we've not heard complaints about the omissions
  is good evidence that nobody's using the buggy mingw versions anymore.
 
 I don't think it is.  Right now we're not checking errno *at all* in a
 bunch of these places, so we're sure not going to get complaints about
 doing it incorrectly in those places.  Or do I need more caffeine?

You are correct.  This code is seriously broken and I am susprised we
have not gotten more complaints.  Good thing readdir/closedir rarely
fail.

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

  + Everyone has their own god. +


-- 
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] [COMMITTERS] pgsql: Make it easy to detach completely from shared memory.

On 18 March 2014 13:51, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Tue, Mar 18, 2014 at 8:41 AM, Simon Riggs si...@2ndquadrant.com wrote:
 Perhaps we should consider a parameter for PGSharedMemoryDetach() ?

 Yeah, maybe.  It seems like a possible modularity violation, because
 the PGSharedMemory... stuff has heretofore not needed to know anything
 about DSM, and apart from this one function, it still wouldn't.

 That was exactly the reason we rejected that design upthread.
 PGSharedMemoryDetach is specific to the main shmem segment, and in fact
 has multiple OS-dependent implementations.

 You could make an argument for inventing some new wrapper function that
 calls both PGSharedMemoryDetach and dsm_detach_all, but I don't believe
 that the existing flavors of that function should know about DSM.

I'm not bothered which we choose, as long as its well documented to
ensure people that use those calls don't detach from just one when
they really would wish to detach from both.

-- 
 Simon Riggs   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] pg_basebackup --slot=SLOTNAME -X stream

On Tue, Mar 18, 2014 at 9:46 AM, Magnus Hagander mag...@hagander.net wrote:
 I started working on that at some point long ago, but never got far enough.

 Right now, it seems the right way to do that is to somehow reimplement it on
 top of a lightweight slot that gets automatically destroyed when
 pg_basebackup stops.

 But as Andres said, there's a little more to it. You'd want the slot to be
 created in the connection that does BASE_BACKUP, then used by the
 replication client side, and then destroyed by BASE_BACKUP. But you also
 want it auto-destroyed if the BASE_BACKUP connection gets terminated for
 example..

The slot machinery already has provision for drop-on-release slots.
But I don't think we expose that at the protocol level in any way just
yet.   CREATE_REPLICATION_SLOT ... TEMPORARY or something might not be
that hard, but at this point I think it's too late to invent new
things for 9.4.  I am hoping for a raft of improvements to the slot
stuff for next release, but trying to do that now seems like pushing
our luck.

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


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

Re: [HACKERS] pg_archivecleanup bug

Bruce Momjian escribió:
 On Tue, Mar 18, 2014 at 10:03:46AM -0400, Robert Haas wrote:
  On Tue, Mar 18, 2014 at 9:56 AM, Tom Lane t...@sss.pgh.pa.us wrote:
   Bruce Momjian br...@momjian.us writes:
   Very good point.  I have modified the patch to add this block in all
   cases where it was missing.  I started to wonder about the comment and
   if the Mingw fix was released.  Based on some research, I see this as
   fixed in mingw-runtime-3.2, released 2003-10-10.  That's pretty old.
  
   Yeah.  I would vote for removing that code in all branches.  There is no
   reason to suppose somebody is going to install 8.4.22 on a machine that
   they haven't updated mingw on since 2003.  Or, if you prefer, just remove
   it in HEAD --- but going around and *adding* more copies seems like
   make-work.  The fact that we've not heard complaints about the omissions
   is good evidence that nobody's using the buggy mingw versions anymore.
  
  I don't think it is.  Right now we're not checking errno *at all* in a
  bunch of these places, so we're sure not going to get complaints about
  doing it incorrectly in those places.  Or do I need more caffeine?
 
 You are correct.  This code is seriously broken and I am susprised we
 have not gotten more complaints.  Good thing readdir/closedir rarely
 fail.

I think we need to keep the check for old mingw runtime in older
branches; it seems reasonable to keep updating Postgres when new
versions come out but keep mingw the same if it doesn't break.  A good
criterion here, to me, is: would we make it a runtime error if an old
mingw version is detected?  If we would, then let's go and remove all
those errno checks.  Then we force everyone to update to a sane mingw.
But if we're not adding such a check, then we might cause subtle trouble
just because we think running old mingw is unlikely.


On another note, please let's not make the code dissimilar in some
branches just because of source code embellishments are not back-ported
out of fear.  I mean, if we want them in master, let them be in older
branches as well.  Otherwise we end up with slightly different versions
that make back-patching future fixes a lot harder, for no gain.

-- 
Álvaro Herrerahttp://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] Portability issues in shm_mq

On Mon, Mar 17, 2014 at 11:09 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 OK, I tried this out.  The major complication that cropped up was
 that, if we make the length word always a Size but align the buffer to
 MAXIMUM_ALIGNOF, then the length word might get split if sizeof(Size)
  MAXIMUM_ALIGNOF.

 Hmm ... do we support any platforms where that's actually the case?
 It's possible I guess but I don't know of any offhand.  The reverse
 case is real, but I'm not sure about this one.

Man, I don't have a clue what exists.  I certainly wouldn't want to
discourage someone from making a 64-bit machine that only requires
32-bit alignment for 8-bit values, but color me clueless as to whether
such things are really out there.

 That doesn't look too bad, but required changing a
 couple of if statements into while loops, and changing around the
 structure of a shm_mq_handle a bit.  See attached.

 Would it get noticeably simpler or faster if you omitted support for
 the sizeof(Size)  MAXIMUM_ALIGNOF case?  It looks like perhaps not,
 but if we were paying anything much I'd be tempted to just put in
 a static assert to the contrary and see if anyone complains.

Not really.  I installed a fast path into the receive code for the
common case where the length word isn't split, which will always be
true on platforms where sizeof(Size) = MAXIMUM_ALIGNOF and usually
true otherwise.  We could ditch the slow path completely by ignoring
that case, but it's not all that much code.  On the sending side, the
logic is pretty trivial, so I definitely don't feel bad about carrying
that.

The thing I kind of like about this approach is that it makes the code
fully independent of the relationship between MAXIMUM_ALIGNOF and
sizeof(Size).  If the former is smaller, we'll write the length word
in chunks if needed; if the latter is smaller, we'll insert useless
padding bytes.  In the perhaps-common case where they're identical, it
all works as before, except for minor space savings on 32-bit
platforms.  I was a little annoyed by having to write the extra code
and thought about objecting to this whole line of attack yet again,
but I think it's actually likely for the best.  If we start persuading
ourselves that certain cases don't need to work, and rely on that
throughout the backend, and then such machines crop up and we want to
support them, we'll have a deep hole to climb out of.  With this
approach, there might be bugs, of course, but it's a lot easier to fix
a bug that only occurs on a new platform than it is to reconsider the
whole design in light of a new platform.

 BTW, can't we drop the MAXALIGN64 stuff again?

It's still used by xlog.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] Portability issues in shm_mq

Robert Haas robertmh...@gmail.com writes:
 On Mon, Mar 17, 2014 at 11:09 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Would it get noticeably simpler or faster if you omitted support for
 the sizeof(Size)  MAXIMUM_ALIGNOF case?  It looks like perhaps not,
 but if we were paying anything much I'd be tempted to just put in
 a static assert to the contrary and see if anyone complains.

 Not really.  I installed a fast path into the receive code for the
 common case where the length word isn't split, which will always be
 true on platforms where sizeof(Size) = MAXIMUM_ALIGNOF and usually
 true otherwise.  We could ditch the slow path completely by ignoring
 that case, but it's not all that much code.  On the sending side, the
 logic is pretty trivial, so I definitely don't feel bad about carrying
 that.

Works for me.

 The thing I kind of like about this approach is that it makes the code
 fully independent of the relationship between MAXIMUM_ALIGNOF and
 sizeof(Size).

Yeah.  If it's not costing us much to support both cases, let's do so.

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] GSoC proposal. Index-only scans for GIST

On Tue, Mar 18, 2014 at 9:12 AM, Anastasia Lubennikova
lubennikov...@gmail.com wrote:
 Support for index-only scans for GIST index

This is a cool idea, if it can be made to work.

 If the fetch() is specified by the developer, then using it, algorithm can
 retrieve the data directly to output areas at this stage, without reference
 to the heap.

This seems to be the crux of your proposal, but it seems vague: what
exactly do you mean by retrieve the data directly to output areas?
What data are you going to retrieve and where are you going to put it?

Another question to consider is: which operator classes do you
anticipate that this will work for and which ones do you anticipate
that it will not work for?  Any operator class that lossifies that
input data before storing it in the index is presumably doomed, but
which ones do that, and which do not?

Tom Lane previously proposed extending SP-GiST to support index-only
scans.  You might find that discussing worth reading, or perhaps
consider it as an alternative if GiST doesn't work out:

http://www.postgresql.org/message-id/10839.1323885...@sss.pgh.pa.us

-- 
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] GSoC proposal. Index-only scans for GIST

Robert Haas robertmh...@gmail.com writes:
 Tom Lane previously proposed extending SP-GiST to support index-only
 scans.  You might find that discussing worth reading, or perhaps
 consider it as an alternative if GiST doesn't work out:
 http://www.postgresql.org/message-id/10839.1323885...@sss.pgh.pa.us

That wasn't just a proposal, see commits
3695a555136a6d179cac8ae48d5f90171d5b30e9 and
92203624934095163f8b57b5b3d7bbd2645da2c8.  But yeah, that might be a
useful reference for what is likely to be involved with making GIST
do it.

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] Minimum supported version of Python?

2014-03-18 Thread Andrew Dunstan



On 03/17/2014 10:31 PM, Peter Eisentraut wrote:

On Sun, 2014-03-16 at 22:34 -0400, Tom Lane wrote:

As for 2.4 vs 2.5, I don't have a lot of faith that we're really
supporting anything that's not represented in the buildfarm...

There are many other features that the build farm doesn't test and that
I don't have a lot of faith in, but I'm not proposing to remove those.
I don't control what the build farm tests, I only control my own work.






Actually, if you run a buildfarm animal you have considerable control 
over what it tests.


It's a very loosely coupled distributed system.

The whole basis on which it was constructed was that people who were 
interested in certain combinations of hardware and infrastructure would 
create animals to test those combinations.


If that's too much trouble for people, I have often offered to set up an 
animal for people if they give me ssh access. If that's too much trouble 
for people I tend to conclude that they aren't actually all that interested.



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] Portability issues in shm_mq

On Tue, Mar 18, 2014 at 10:44 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 The thing I kind of like about this approach is that it makes the code
 fully independent of the relationship between MAXIMUM_ALIGNOF and
 sizeof(Size).

 Yeah.  If it's not costing us much to support both cases, let's do so.

OK, committed as posted.

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


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

Re: [HACKERS] pg_archivecleanup bug

On 18 March 2014 14:15, Alvaro Herrera alvhe...@2ndquadrant.com wrote:
 Bruce Momjian escribió:
 On Tue, Mar 18, 2014 at 10:03:46AM -0400, Robert Haas wrote:
  On Tue, Mar 18, 2014 at 9:56 AM, Tom Lane t...@sss.pgh.pa.us wrote:
   Bruce Momjian br...@momjian.us writes:
   Very good point.  I have modified the patch to add this block in all
   cases where it was missing.  I started to wonder about the comment and
   if the Mingw fix was released.  Based on some research, I see this as
   fixed in mingw-runtime-3.2, released 2003-10-10.  That's pretty old.
  
   Yeah.  I would vote for removing that code in all branches.  There is no
   reason to suppose somebody is going to install 8.4.22 on a machine that
   they haven't updated mingw on since 2003.  Or, if you prefer, just remove
   it in HEAD --- but going around and *adding* more copies seems like
   make-work.  The fact that we've not heard complaints about the omissions
   is good evidence that nobody's using the buggy mingw versions anymore.
 
  I don't think it is.  Right now we're not checking errno *at all* in a
  bunch of these places, so we're sure not going to get complaints about
  doing it incorrectly in those places.  Or do I need more caffeine?

 You are correct.  This code is seriously broken and I am susprised we
 have not gotten more complaints.  Good thing readdir/closedir rarely
 fail.

 back-patching

Some commentary on this...

Obviously, all errors are mine.

If pg_archivecleanup is a problem, then so is pg_standby a problem.

Given the above, this means we've run for about 7 years without a
reported issue on this. If we are going to make this better by
actually having it throw errors in places that didn't throw errors
before, are we sure that is going to make people happier? The archive
cleanup isn't exactly critical in most cases, so dynamic errors don't
matter much.

Also, the programs were originally written to work as standalone
program as well as an archive_cleanup_command. So we can't use
PostgreSQL infrastructure (can we?). That aspect is needed to allow
testing the program before it goes live.

-- 
 Simon Riggs   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] Minimum supported version of Python?

2014-03-18 Thread Peter Eisentraut

On 3/17/14, 10:55 PM, Tom Lane wrote:
 It doesn't pass the regression tests.  Do you need more of a bug report
 than that?

It does pass the tests for me and others.  If you are seeing something
different, then we need to see some details, like what platform, what
version, what Python version, how installed, what PostgreSQL version,
how installed, actual diffs, etc.



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

On Tue, Mar 18, 2014 at 11:36 AM, Simon Riggs si...@2ndquadrant.com wrote:
 Given the above, this means we've run for about 7 years without a
 reported issue on this. If we are going to make this better by
 actually having it throw errors in places that didn't throw errors
 before, are we sure that is going to make people happier? The archive
 cleanup isn't exactly critical in most cases, so dynamic errors don't
 matter much.

We report errors returned by system calls in many other places.  I
can't see why this place should be any different.

-- 
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: show relation and tuple infos of a lock to acquire

On Tue, Mar 18, 2014 at 3:42 AM, Amit Kapila amit.kapil...@gmail.com wrote:
 The message for exclusive lock on tuple print the database information.

It is true that it is possible to have a deadlock or lock chains that
involves locks on other databases.
In this example the table test is not in the database that just
logged the deadlock.

STATEMENT:  create role test;
ERROR:  deadlock detected
DETAIL:  Process 8968 waits for ShareLock on transaction 1067; blocked
by process 8973.
Process 8973 waits for ShareLock on transaction 1064; blocked
by process 8971.
Process 8971 waits for ShareLock on transaction 1062; blocked
by process 8968.
Process 8968: create role test;
Process 8973: insert into test values (2);
Process 8971: create role test2;


-- 
greg


-- 
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] Minimum supported version of Python?

2014-03-18 Thread Peter Eisentraut

On 3/17/14, 10:47 PM, Joshua D. Drake wrote:
 We shouldn't be supporting anything the community doesn't support.
 Python 2.3 is dead. We shouldn't actively support it nor suggest that we
 could or should via the docs.

The information that is available to me about this issue is lacking
details, but as far as I can tell, we are partially talking about
backbranches, which were released at a time when Python 2.3 and 2.4 were
not unreasonable targets.  We're not going to desupport build
dependencies in the middle of a stable release branch unless we have a
solid reason.



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

On 18 March 2014 15:50, Robert Haas robertmh...@gmail.com wrote:
 On Tue, Mar 18, 2014 at 11:36 AM, Simon Riggs si...@2ndquadrant.com wrote:
 Given the above, this means we've run for about 7 years without a
 reported issue on this. If we are going to make this better by
 actually having it throw errors in places that didn't throw errors
 before, are we sure that is going to make people happier? The archive
 cleanup isn't exactly critical in most cases, so dynamic errors don't
 matter much.

 We report errors returned by system calls in many other places.  I
 can't see why this place should be any different.

Sure. Just wanted to make sure it's a conscious, explicit choice to do so.

-- 
 Simon Riggs   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] Minimum supported version of Python?

2014-03-18 Thread Peter Eisentraut

On 3/18/14, 11:22 AM, Andrew Dunstan wrote:
 Actually, if you run a buildfarm animal you have considerable control
 over what it tests.

I appreciate that.  My problem here isn't time or ideas or coding, but
lack of hardware resources.  If I had hardware, I could set up tests for
every build dependency under the sun.



-- 
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] Portability issues in shm_mq

All right.

On Fri, Mar 14, 2014 at 4:43 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Whilst setting up a buildfarm member on an old, now-spare Mac, I was
 somewhat astonished to discover that contrib/test_shm_mq crashes thus:
 TRAP: FailedAssertion(!(rb = sizeof(uint64)), File: shm_mq.c, Line: 429)
 but only in UTF8 locales, not in C locale.  You'd have bet your last
 dollar that that code was locale-independent, right?

First, can you retest this with the latest code?

 Recommendations:

 1. Reduce the random() multiplier from 96 to 95.  In multibyte encodings
 other than UTF8, chr() would flat out reject values of 128, so this test
 case is unportable.

 2. Why in the world is the test case testing exactly one message length
 that happens to be a multiple of 8?  Put some randomness into that,
 instead.

 3. Either you need to work a bit harder at forcing alignment, or you need
 to fix shm_mq_receive to cope with split message length words.

 4. The header comment for shm_mq_receive_bytes may once have described its
 API accurately, but that appears to have been a long long time ago in a
 galaxy far far away.  Please fix.

On these recommendations, I believe that #3 and #4 are now dealt with.
 That leaves #1 and #2.  #1 is of course easy, but I think we should
do them both together.

If we want to inject some randomness into the test, which parameters
do we want to randomize and over what ranges?  Also, if a buildfarm
critter falls over, how will we know what value triggered the failure?
 It's tempting to instead add one or more tests that we specifically
choose to have values we think are likely to exercise
platform-specific differences or otherwise find bugs - e.g. just add a
second test where the queue size and message length are both odd.  And
maybe at a test where the queue is smaller than the message size, so
that every message wraps (multiple times?).

Thoughts?

-- 
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: show relation and tuple infos of a lock to acquire

On Tue, Mar 18, 2014 at 11:59 AM, Greg Stark st...@mit.edu wrote:
 On Tue, Mar 18, 2014 at 3:42 AM, Amit Kapila amit.kapil...@gmail.com wrote:
 The message for exclusive lock on tuple print the database information.

 It is true that it is possible to have a deadlock or lock chains that
 involves locks on other databases.
 In this example the table test is not in the database that just
 logged the deadlock.

 STATEMENT:  create role test;
 ERROR:  deadlock detected
 DETAIL:  Process 8968 waits for ShareLock on transaction 1067; blocked
 by process 8973.
 Process 8973 waits for ShareLock on transaction 1064; blocked
 by process 8971.
 Process 8971 waits for ShareLock on transaction 1062; blocked
 by process 8968.
 Process 8968: create role test;
 Process 8973: insert into test values (2);
 Process 8971: create role test2;

This is a good point, but I think it's acceptable to leave out the
database name as Tom proposes.  The context message applies to what
the current backend was doing when the message got printed, and that's
always relative to the current database.

-- 
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] Portability issues in shm_mq

Robert Haas robertmh...@gmail.com writes:
 First, can you retest this with the latest code?

Yeah, on it now.

 If we want to inject some randomness into the test, which parameters
 do we want to randomize and over what ranges?

I think the message length is the only particularly interesting
parameter.  It'd be nice if the length varied *within* a test, but
that would take rather considerable restructuring, so maybe it's
not worth the trouble.

 Also, if a buildfarm
 critter falls over, how will we know what value triggered the failure?

Maybe we won't, but I think knowing that it does fail on platform X is
likely to be enough to find the problem.

  It's tempting to instead add one or more tests that we specifically
 choose to have values we think are likely to exercise
 platform-specific differences or otherwise find bugs - e.g. just add a
 second test where the queue size and message length are both odd.

Meh.  I think you're putting a bit too much faith in your ability to
predict the locus of bugs that you think aren't there.

 maybe at a test where the queue is smaller than the message size, so
 that every message wraps (multiple times?).

Does the code support messages larger than the queue size?  If so, yes,
that case clearly oughta be tested.

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: show relation and tuple infos of a lock to acquire

On Tue, Mar 18, 2014 at 12:00 AM, Amit Kapila amit.kapil...@gmail.com wrote:
 Therefore I think the only case worth considering here is when
 database/relation/TID are all defined.  The other cases where there is
 partial information are dead code; and the case where there is nothing
 defined (such as the one in SnapBuildFindSnapshot) is already handled by
 simply not setting up a context at all.

 Right. So I think we should just keep one version of message.

Well, if we're back to one version of the message, and I'm glad we
are, can we go back to saying:

CONTEXT:  while updating tuple (0,2) in relation public.foo of
database postgres

Instead of:

CONTEXT:  while operating on tuple (0,2) in relation public.foo of
database postgres: updating tuple

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


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

Re: [HACKERS] pg_archivecleanup bug

On 13 March 2014 05:48, Bruce Momjian br...@momjian.us wrote:
 On Mon, Dec  9, 2013 at 11:27:28AM -0500, Robert Haas wrote:
 On Thu, Dec 5, 2013 at 6:15 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  But the other usages seem to be in assorted utilities, which
  will need to do it right for themselves.  initdb.c's walkdir() seems to
  have it right and might be a reasonable model to follow.  Or maybe we
  should invent a frontend-friendly version of ReadDir() rather than
  duplicating all the error checking code in ten-and-counting places?

 If there's enough uniformity in all of those places to make that
 feasible, it certainly seems wise to do it that way.  I don't know if
 that's the case, though - e.g. maybe some callers want to exit and
 others do not.  pg_resetxlog wants to exit; pg_archivecleanup and
 pg_standby most likely want to print an error and carry on.

 I have developed the attached patch which fixes all cases where
 readdir() wasn't checking for errno, and cleaned up the syntax in other
 cases to be consistent.

 While I am not a fan of backpatching, the fact we are ignoring errors in
 some critical cases seems the non-cosmetic parts should be backpatched.

pg_resetxlog was not an offender here; its coding was sound.

We shouldn't be discussing backpatching a patch that contains changes
to coding style.

ISTM we should change the code with missing checks to adopt the coding
style of pg_resetxlog, not the other way around.

I assume you or Kevin have this in hand and you don't want me to apply
the patch? (Since it was originally my bug)

-- 
 Simon Riggs   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] Minimum supported version of Python?

2014-03-18 Thread David Johnston

Peter Eisentraut-2 wrote
 On 3/18/14, 11:22 AM, Andrew Dunstan wrote:
 Actually, if you run a buildfarm animal you have considerable control
 over what it tests.
 
 I appreciate that.  My problem here isn't time or ideas or coding, but
 lack of hardware resources.  If I had hardware, I could set up tests for
 every build dependency under the sun.

I don't imagine there is enough churn in this area that having a constantly
testing buildfarm animal is a strong need; but a wiki page dedicated to
Python Support in PostgreSQL where we can publicly and officially release
testing results and commentary would be an improvement.

As it sounds like the only caveat to supporting 2.3 is that we don't
technically (or do we - is Decimal mandatory?) support an un-modified core
installation but require that at one add-on module be installed.  Assuming
that clears up all the errors Tom is seeing then saying that plpythonu works
with a slightly modified 2.3 on all current releases of PostgreSQL isn't a
stretch nor does it commit us to fixing bugs in the unlikely event that any
are discovered in two extremely stable environments.





--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/Minimum-supported-version-of-Python-tp5796175p5796615.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.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] Portability issues in shm_mq

I wrote:
 Robert Haas robertmh...@gmail.com writes:
 First, can you retest this with the latest code?

 Yeah, on it now.

Early returns not good:

*** 
/Users/buildfarm/bf-data/HEAD/pgsql.93630/contrib/test_shm_mq/expected/test_shm_mq.out
  Tue Mar 18 12:00:18 2014
--- 
/Users/buildfarm/bf-data/HEAD/pgsql.93630/contrib/test_shm_mq/results/test_shm_mq.out
   Tue Mar 18 12:17:04 2014
***
*** 5,18 
  -- internal sanity tests fail.
  --
  SELECT test_shm_mq(32768, (select string_agg(chr(32+(random()*96)::int), '') 
from generate_series(1,400)), 1, 1);
!  test_shm_mq 
! -
!  
! (1 row)
! 
  SELECT test_shm_mq_pipelined(16384, (select 
string_agg(chr(32+(random()*96)::int), '') from generate_series(1,27)), 
200, 3);
!  test_shm_mq_pipelined 
! ---
!  
! (1 row)
! 
--- 5,12 
  -- internal sanity tests fail.
  --
  SELECT test_shm_mq(32768, (select string_agg(chr(32+(random()*96)::int), '') 
from generate_series(1,400)), 1, 1);
! ERROR:  message corrupted
! DETAIL:  The original message was 400 bytes but the final message is 
7492059346764176 bytes.
  SELECT test_shm_mq_pipelined(16384, (select 
string_agg(chr(32+(random()*96)::int), '') from generate_series(1,27)), 
200, 3);
! ERROR:  message corrupted
! DETAIL:  The original message was 27 bytes but the final message is 
7492059347033776 bytes.


This is C locale on a 32-bit machine, so you'll likely be seeing the same
complaint in already-online buildfarm members.

Note that size_t is definitely not int64 on this machine, so it looks to
me like your int64-ectomy was incomplete.  Those message lengths should
be impossible no matter what on this hardware.

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: show relation and tuple infos of a lock to acquire

Robert Haas escribió:
 On Tue, Mar 18, 2014 at 12:00 AM, Amit Kapila amit.kapil...@gmail.com wrote:
  Therefore I think the only case worth considering here is when
  database/relation/TID are all defined.  The other cases where there is
  partial information are dead code; and the case where there is nothing
  defined (such as the one in SnapBuildFindSnapshot) is already handled by
  simply not setting up a context at all.
 
  Right. So I think we should just keep one version of message.
 
 Well, if we're back to one version of the message, and I'm glad we
 are, can we go back to saying:
 
 CONTEXT:  while updating tuple (0,2) in relation public.foo of
 database postgres

That isn't easy.  The callers would need to pass the whole message in
order for it to be translatable; and generating a format string in one
module and having the arguments be stapled on top in a separate module
doesn't seem a very good idea to me.  Currently we have this:

/* wait for regular transaction to end */
XactLockTableWait(xwait, relation, tp.t_data-t_ctid,
/* translator: string is XactLockTableWait operation 
name */
  deleting tuple);

And if we go with what you propose, we would have this:

/* wait for regular transaction to end */
XactLockTableWait(xwait, relation, tp.t_data-t_ctid,
while updating tuple (%u,%u) in relation \%s\)

which doesn't seem an improvement to me.

Now another idea would be to pass a code or something; so callers would
do
XactLockTableWait(xwait, relation, TID, XLTW_OPER_UPDATE);

and the callback would select the appropriate message.  Not really
excited about that, though.

In the current version of the patch, attached, I've reduced the callback
to this:

if (ItemPointerIsValid(info-ctid)  RelationIsValid(info-rel))
{
errcontext(while operating on tuple (%u,%u) in relation 
\%s\: %s,
   ItemPointerGetBlockNumber(info-ctid),
   ItemPointerGetOffsetNumber(info-ctid),
   RelationGetRelationName(info-rel),
   _(info-opr_name));
}

Note that:
1. the database name is gone, as discussed
2. the schema name is gone too, because it requires a syscache lookup

Now we can go back to having a schema name, but I think we would have to
add an IsTransactionState() check first or something like that.  Or save
the schema name when setting up the callback, but this doesn't sound so
good (particularly considering that lock waits might end without
actually waiting at all, so this'd be wasted effort.)


If you have a better idea, I'm all ears.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services
*** a/src/backend/access/heap/heapam.c
--- b/src/backend/access/heap/heapam.c
***
*** 105,115  static void GetMultiXactIdHintBits(MultiXactId multi, uint16 *new_infomask,
  	   uint16 *new_infomask2);
  static TransactionId MultiXactIdGetUpdateXid(TransactionId xmax,
  		uint16 t_infomask);
! static void MultiXactIdWait(MultiXactId multi, MultiXactStatus status,
! int *remaining, uint16 infomask);
! static bool ConditionalMultiXactIdWait(MultiXactId multi,
! 		   MultiXactStatus status, int *remaining,
! 		   uint16 infomask);
  static XLogRecPtr log_heap_new_cid(Relation relation, HeapTuple tup);
  static HeapTuple ExtractReplicaIdentity(Relation rel, HeapTuple tup, bool key_modified,
  		bool *copy);
--- 105,116 
  	   uint16 *new_infomask2);
  static TransactionId MultiXactIdGetUpdateXid(TransactionId xmax,
  		uint16 t_infomask);
! static void MultiXactIdWait(MultiXactId multi, MultiXactStatus status, uint16 infomask,
! Relation rel, ItemPointer ctid, const char *opr,
! int *remaining);
! static bool ConditionalMultiXactIdWait(MultiXactId multi, MultiXactStatus status,
! 		   uint16 infomask, Relation rel, ItemPointer ctid,
! 		   const char *opr, int *remaining);
  static XLogRecPtr log_heap_new_cid(Relation relation, HeapTuple tup);
  static HeapTuple ExtractReplicaIdentity(Relation rel, HeapTuple tup, bool key_modified,
  		bool *copy);
***
*** 2714,2721  l1:
  		if (infomask  HEAP_XMAX_IS_MULTI)
  		{
  			/* wait for multixact */
! 			MultiXactIdWait((MultiXactId) xwait, MultiXactStatusUpdate,
! 			NULL, infomask);
  			LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
  
  			/*
--- 2715,2724 
  		if (infomask  HEAP_XMAX_IS_MULTI)
  		{
  			/* wait for multixact */
! 			MultiXactIdWait((MultiXactId) xwait, MultiXactStatusUpdate, infomask,
! 			/* translator: string is XactLockTableWait operation name */
! 			relation, tp.t_data-t_ctid, deleting tuple,
! 			NULL);

Re: [HACKERS] Patch: show relation and tuple infos of a lock to acquire

Robert Haas robertmh...@gmail.com writes:
 Well, if we're back to one version of the message, and I'm glad we
 are, can we go back to saying:

 CONTEXT:  while updating tuple (0,2) in relation public.foo of
 database postgres

If I end up being the one who commits this, it's going to say

while updating tuple (0,2) in table foo

Not more, and not less.  It is not project style to include schema names
(much less database names) in error messages where they're not central to
the meaning.

One reason why not is that schema and database names are generally not
available without an extra lookup step, which you don't really want to do
in an error-reporting code path.  Every extra action you take increases
the risk of a cascading failure, so that the user will get something
unhelpful like out of memory rather than the oh-so-extra-helpful message
you wanted to print.  The added utility of the extra information, for most
cases, is insufficient to justify that risk.

Even without that argument, it's still not project style; why should this
message be randomly different from many hundreds of others?  If you want
to start a push to include schema names anywhere a table name is given,
that should be debated separately and then done in a reasonably uniform
fashion.

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: show relation and tuple infos of a lock to acquire

Tom Lane escribió:
 Robert Haas robertmh...@gmail.com writes:
  Well, if we're back to one version of the message, and I'm glad we
  are, can we go back to saying:
 
  CONTEXT:  while updating tuple (0,2) in relation public.foo of
  database postgres
 
 If I end up being the one who commits this, it's going to say
 
 while updating tuple (0,2) in table foo
 
 Not more, and not less.

Please see my reply to Robert.  My proposal (in form of a patch) is
  while operating on tuple (0,2) in table foo: updating tuple
Would this work for you?

-- 
Álvaro Herrerahttp://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] Patch: show relation and tuple infos of a lock to acquire

Alvaro Herrera alvhe...@2ndquadrant.com writes:
 Please see my reply to Robert.  My proposal (in form of a patch) is
   while operating on tuple (0,2) in table foo: updating tuple
 Would this work for you?

It's pretty lousy from a readability standpoint, even in English;
I shudder to think what it might come out as after translation.

I think the enum idea you floated is probably worth doing.  It's
certainly no more complex than passing a string, no?

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] Planner hints in Postgresql

2014-03-18 Thread Claudio Freire

On Mon, Mar 17, 2014 at 8:47 PM, Tom Lane t...@sss.pgh.pa.us wrote:

 Claudio Freire klaussfre...@gmail.com writes:
  On Mon, Mar 17, 2014 at 7:01 PM, Jim Nasby j...@nasby.net wrote:
  Even better would be if the planner could estimate how bad a plan will
  become if we made assumptions that turn out to be wrong.

  That's precisely what risk estimation was about.

 Yeah.  I would like to see the planner's cost estimates extended to
 include some sort of uncertainty estimate, whereupon risk-averse people
 could ask it to prefer low-uncertainty plans over high-uncertainty ones
 (the plans we typically choose for ORDER BY ... LIMIT queries being great
 examples of the latter).  But it's a long way from wishing that to making
 it so.  Right now it's not even clear (to me anyway) how we'd measure or
 model such uncertainty.


Well, currently, selectivity estimates based on MCV should be pretty
low-uncertainty, whereas certainty of other estimates could be modeled as a
random variable if ANALYZE gathered a few statistical moments (for
variables that are prone to that kind of statistical analysis).

That alone could improve things considerably, and statistical info could be
propagated along expressions to make it possible to model uncertainty in
complex expressions as well.

[HACKERS] json/jsonb/hstore operator precedence

Fwiw I'm finding myself repeatedly caught up by the operator
precedence rules when experimenting with jsonb:

stark=***# select  segment-'id' as id from flight_segments where
segment-'marketing_airline_code' 
segment-'operating_airline_code' ;
ERROR:  42883: operator does not exist: text  jsonb
LINE 2: ...segments where segment-'marketing_airline_code'  segment...
 ^
HINT:  No operator matches the given name and argument type(s). You
might need to add explicit type casts.
LOCATION:  op_error, parse_oper.c:722
Time: 0.407 ms
stark=***# select  segment-'id' as id from flight_segments where
(segment-'marketing_airline_code') 
(segment-'operating_airline_code') ;
 id
-
 45866185
 95575359


I don't think this is related to the jsonb patch -- json and hstore
have the same behaviour so jsonb is obviously going to follow suit.
The only option right now would be to use a higher precedence operator
like % or ^ for all of these data types which I'm not for. I suspect
it's a pipe dream to think we might be able to override the '.' and
changing the precedence of - and - would be fraught...

I think the best we can do is to highlight it in the docs.

Incidentally it's a good thing there wasn't an implicit cast
text-jsonb. In this case it would have resulted in just a confusing
error of jsonb-boolean not existing.


-- 
greg


-- 
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] GSoC proposal. Index-only scans for GIST

Alexander,

Can you comment on the below proposal? I'd like your opinion on how
difficult it will be.

On 03/18/2014 06:12 AM, Anastasia Lubennikova wrote:
Hello!
Here is the text of my proposal which I've applied to GSoC.
(and link
http://www.google-melange.com/gsoc/proposal/public/google/gsoc2014/lubennikovaav/5629499534213120)
Any suggestions and comments are welcome.

*Project name*

Support for index-only scans for GIST index

*Brief review*

*Benefits to the PostgreSQL Community*

Faster GiST index search would be actual for many PostgreSQL applications
(for example some GIS systems).

*Quantifiable results*

Acceleration of GiST index search.

*Project details*

There are seven methods that an index operator class for GiST must provide,
and an eighth that is optional.

-consistent

-union

-compress

-decompress

-penalty

-picksplit

-equal

-distance (optional)

2. gistget algorithm (several parts omitted):

Check the key
gistindex_keytest() - does this index tuple satisfy the scan key(s)?

Scan all items on the GiST index page and insert them into the queue (or
directly to output areas)

plain scan

Heap tuple TIDs are returned into so-pageData[]

ordered scan

Heap tuple TIDs are pushed into individual search queue items

If the fetch() is specified by the developer, then using it, algorithm can
retrieve the data directly to output areas at this stage, without reference
to the heap.

3. Create function gistcanreturn to check whether fetch() is specified by
user.

Amcanreturn - Function to check whether index supports index-only scans, or
zero if none

There is the question of support index-only scans for multicolumn indexes.
Probably it would require extend the interface to add separate columns
checking.

To solve this question I need the community's help.

4. Add details for index only scans into gistcostestimate function

*Links*

1) Hellerstein J. M., Naughton J. F., Pfeffer A. Generalized search
trees for database systems. - September, 1995.

2) http://www.sai.msu.su/~megera/postgres/gist/

3) PostgreSQL 9.3.3 Documentation: chapters 11. Indexes, 55. GiST
Indexes.

--
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] Portability issues in shm_mq

I wrote:
 Early returns not good:

Also, these compiler messages are probably relevant:

ccache gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith 
-Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute 
-Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -g 
-fpic -I. -I. -I../../src/include -D_GNU_SOURCE -I/usr/include/libxml2  
-I/usr/include/et  -c -o test.o test.c
test.c: In function 'test_shm_mq':
test.c:89:3: warning: passing argument 2 of 'shm_mq_receive' from incompatible 
pointer type [enabled by default]
In file included from test_shm_mq.h:18:0,
 from test.c:19:
../../src/include/storage/shm_mq.h:61:22: note: expected 'Size *' but argument 
is of type 'uint64 *'
test.c: In function 'test_shm_mq_pipelined':
test.c:198:4: warning: passing argument 2 of 'shm_mq_receive' from incompatible 
pointer type [enabled by default]
In file included from test_shm_mq.h:18:0,
 from test.c:19:
../../src/include/storage/shm_mq.h:61:22: note: expected 'Size *' but argument 
is of type 'uint64 *'
ccache gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith 
-Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute 
-Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -g 
-fpic -I. -I. -I../../src/include -D_GNU_SOURCE -I/usr/include/libxml2  
-I/usr/include/et  -c -o setup.o setup.c
ccache gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith 
-Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute 
-Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -g 
-fpic -I. -I. -I../../src/include -D_GNU_SOURCE -I/usr/include/libxml2  
-I/usr/include/et  -c -o worker.o worker.c
worker.c: In function 'copy_messages':
worker.c:193:3: warning: passing argument 2 of 'shm_mq_receive' from 
incompatible pointer type [enabled by default]
In file included from worker.c:25:0:
../../src/include/storage/shm_mq.h:61:22: note: expected 'Size *' but argument 
is of type 'uint64 *'

I'm thinking maybe you just forgot to update the contrib module.

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] Portability issues in shm_mq

On Tue, Mar 18, 2014 at 1:16 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 I wrote:
 Early returns not good:

 Also, these compiler messages are probably relevant:

 ccache gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith 
 -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute 
 -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -g 
 -fpic -I. -I. -I../../src/include -D_GNU_SOURCE -I/usr/include/libxml2  
 -I/usr/include/et  -c -o test.o test.c
 test.c: In function 'test_shm_mq':
 test.c:89:3: warning: passing argument 2 of 'shm_mq_receive' from 
 incompatible pointer type [enabled by default]
 In file included from test_shm_mq.h:18:0,
  from test.c:19:
 ../../src/include/storage/shm_mq.h:61:22: note: expected 'Size *' but 
 argument is of type 'uint64 *'
 test.c: In function 'test_shm_mq_pipelined':
 test.c:198:4: warning: passing argument 2 of 'shm_mq_receive' from 
 incompatible pointer type [enabled by default]
 In file included from test_shm_mq.h:18:0,
  from test.c:19:
 ../../src/include/storage/shm_mq.h:61:22: note: expected 'Size *' but 
 argument is of type 'uint64 *'
 ccache gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith 
 -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute 
 -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -g 
 -fpic -I. -I. -I../../src/include -D_GNU_SOURCE -I/usr/include/libxml2  
 -I/usr/include/et  -c -o setup.o setup.c
 ccache gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith 
 -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute 
 -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -g 
 -fpic -I. -I. -I../../src/include -D_GNU_SOURCE -I/usr/include/libxml2  
 -I/usr/include/et  -c -o worker.o worker.c
 worker.c: In function 'copy_messages':
 worker.c:193:3: warning: passing argument 2 of 'shm_mq_receive' from 
 incompatible pointer type [enabled by default]
 In file included from worker.c:25:0:
 ../../src/include/storage/shm_mq.h:61:22: note: expected 'Size *' but 
 argument is of type 'uint64 *'

 I'm thinking maybe you just forgot to update the contrib module.

Well, I definitely forgot that.  I'll count myself lucky if that's the
only problem.

-- 
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] Portability issues in shm_mq

2014-03-18 Thread Andres Freund

On 2014-03-18 13:31:47 -0400, Robert Haas wrote:
 Well, I definitely forgot that.  I'll count myself lucky if that's the
 only problem.

One minor thing missing, patch attached.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services
diff --git a/contrib/test_shm_mq/test.c b/contrib/test_shm_mq/test.c
index dba5e69..5ff1e9a 100644
--- a/contrib/test_shm_mq/test.c
+++ b/contrib/test_shm_mq/test.c
@@ -254,12 +254,12 @@ verify_message(Size origlen, char *origdata, Size newlen, char *newdata)
 	if (origlen != newlen)
 		ereport(ERROR,
 (errmsg(message corrupted),
- errdetail(The original message was  UINT64_FORMAT  bytes but the final message is  UINT64_FORMAT  bytes.,
+ errdetail(The original message was %zu bytes but the final message is %zu bytes.,
 	 origlen, newlen)));
 
 	for (i = 0; i  origlen; ++i)
 		if (origdata[i] != newdata[i])
 			ereport(ERROR,
 	(errmsg(message corrupted),
-	 errdetail(The new and original messages differ at byte  UINT64_FORMAT  of  UINT64_FORMAT ., i, origlen)));
+	 errdetail(The new and original messages differ at byte %zu of %zu., i, origlen)));
 }

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

On Tue, Mar 18, 2014 at 04:17:53PM +, Simon Riggs wrote:
  While I am not a fan of backpatching, the fact we are ignoring errors in
  some critical cases seems the non-cosmetic parts should be backpatched.
 
 pg_resetxlog was not an offender here; its coding was sound.
 
 We shouldn't be discussing backpatching a patch that contains changes
 to coding style.

I was going to remove the coding style changes to pg_resetxlog from the
backpatched portion.

 ISTM we should change the code with missing checks to adopt the coding
 style of pg_resetxlog, not the other way around.
 
 I assume you or Kevin have this in hand and you don't want me to apply
 the patch? (Since it was originally my bug)

I know the email subject says pg_archivecleanup but the problem is all
over our code.

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

  + Everyone has their own god. +


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

On 18 March 2014 18:01, Bruce Momjian br...@momjian.us wrote:
 On Tue, Mar 18, 2014 at 04:17:53PM +, Simon Riggs wrote:
  While I am not a fan of backpatching, the fact we are ignoring errors in
  some critical cases seems the non-cosmetic parts should be backpatched.

 pg_resetxlog was not an offender here; its coding was sound.

 We shouldn't be discussing backpatching a patch that contains changes
 to coding style.

 I was going to remove the coding style changes to pg_resetxlog from the
 backpatched portion.

Why make style changes at all? A patch with no style changes would
mean backpatch and HEAD versions would be the same.

 ISTM we should change the code with missing checks to adopt the coding
 style of pg_resetxlog, not the other way around.

 I assume you or Kevin have this in hand and you don't want me to apply
 the patch? (Since it was originally my bug)

 I know the email subject says pg_archivecleanup but the problem is all
 over our code.

Yes, understood.

-- 
 Simon Riggs   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] Risk Estimation WAS: Planner hints in Postgresql


 On Mon, Mar 17, 2014 at 8:47 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Yeah.  I would like to see the planner's cost estimates extended to
 include some sort of uncertainty estimate, whereupon risk-averse people
 could ask it to prefer low-uncertainty plans over high-uncertainty ones
 (the plans we typically choose for ORDER BY ... LIMIT queries being great
 examples of the latter).  But it's a long way from wishing that to making
 it so.  Right now it's not even clear (to me anyway) how we'd measure or
 model such uncertainty.

This is not a model, but here's some starting thoughts:

A high risk plan has two components:

a) our statistical data is out-of-date or inadequate

b) the potential execution time if our estimates of selectivity are
wrong is high

c) the cost ratio of certain operations is wrong.

Factor (a) can be modeled two ways:

1. If last_analyze is a long time ago, we have increased the risk.
   (Ideally, we'd have some idea of the change rate on the table vs.
the last analyze time; right now we don't have those stats)

2. Certain patterns, such as multi-column selectivity and GIN/GiST
selectivity are known to have poor estimates, and be higher risk.
Certainly selectivity functions which have been programmed with a flat
coefficient (like default 0.05 selectivity for gist_ops) could also
return a risk factor which is fairly high.

Factor (b) can be modeled simply by estimating the cost of a plan where
all row estimates are changed by 10X, or even better by a calculation on
the risk factor calculated in (a).  This would then give us the failure
cost of the bad plan.  Note that we need to estimate in both
directions, both for higher estimates and lower ones; abort early
plans fail because the rows returned are lower than expected, for example.

(b) estimation would be expensive if we did every combination of the
entire plan with wrong estimates, so I'm wondering if it would be
adequate to just estimate the node selectivity being off on a per-node
basis.

(c) we can't realistically estimate for at all (i.e. if we knew the cost
factor was wrong, we'd fix it) so I suggest ignoring it for risk estimation.

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

On Tue, Mar 18, 2014 at 06:11:30PM +, Simon Riggs wrote:
 On 18 March 2014 18:01, Bruce Momjian br...@momjian.us wrote:
  On Tue, Mar 18, 2014 at 04:17:53PM +, Simon Riggs wrote:
   While I am not a fan of backpatching, the fact we are ignoring errors in
   some critical cases seems the non-cosmetic parts should be backpatched.
 
  pg_resetxlog was not an offender here; its coding was sound.
 
  We shouldn't be discussing backpatching a patch that contains changes
  to coding style.
 
  I was going to remove the coding style changes to pg_resetxlog from the
  backpatched portion.
 
 Why make style changes at all? A patch with no style changes would
 mean backpatch and HEAD versions would be the same.

The old style had errno set in two places in the loop, while the new
style has it set in only one place.

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

  + Everyone has their own god. +


-- 
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] Portability issues in shm_mq

and...@anarazel.de (Andres Freund) writes:
 On 2014-03-18 13:31:47 -0400, Robert Haas wrote:
 Well, I definitely forgot that.  I'll count myself lucky if that's the
 only problem.

 One minor thing missing, patch attached.

setup_dynamic_shared_memory needed some more hacking too.  Committed.

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] Risk Estimation WAS: Planner hints in Postgresql

2014-03-18 Thread Atri Sharma

On Tue, Mar 18, 2014 at 11:43 PM, Josh Berkus j...@agliodbs.com wrote:


  On Mon, Mar 17, 2014 at 8:47 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  Yeah.  I would like to see the planner's cost estimates extended to
  include some sort of uncertainty estimate, whereupon risk-averse people
  could ask it to prefer low-uncertainty plans over high-uncertainty ones
  (the plans we typically choose for ORDER BY ... LIMIT queries being
 great
  examples of the latter).  But it's a long way from wishing that to
 making
  it so.  Right now it's not even clear (to me anyway) how we'd measure or
  model such uncertainty.



I have been thinking of some ways to have a risk estimate of each
selectivity that our planner gives. I think a way to do it is as follows:

One of the factors that leads to bad estimates is that the histogram of the
values of a column maintained by the planner gets old by time and the data
in the column changes. So, the histogram is no longer a quite accurate view
of the data and it leads to bad selectivity.

One thing we can try to do is to add a factor of error that we feel the
selectivity given can have. This allows us to factor in the probability
that the data changed and the estimate of the difference of the current
histogram and the histogram of the actual data currently present in the
column in the table.

We can use Central Limit Theorem (
http://en.wikipedia.org/wiki/Central_limit_theorem). Essentially, what the
theorem says is that given a distribution that has finite variance and
finite mean, we can take random independent samples from the data and
calculate the standard deviation and the mean of the sample. If we have
large enough number of samples and if we plot the mean and SD, they would
follow a normal distribution.

What is interesting is that this can allow us to predict the SD of a given
dataset from the curve and the SD should be directly proportional to the
deviation it has from the given planner histogram.

I am no mathematician hence its hard for me to explain. I think this link
[1] will be more helpful.

So, we can have a probability value for the random variable and that shall
model the confidence we have in our estimate.

I may be wrong in some parts but I hope I have been able to convey the
general idea.

If this idea develops, I shall be happy to work on this but my hands are
full in ROLLUPS right now, so for my part it shall take time. I just want
to float the idea and get a general feel about the idea right now.

Please let me know your comments and feedback on this.

Regards,

Atri

[1]: http://www.theriac.org/DeskReference/viewDocument.php?id=177
-- 
Regards,

Atri
*l'apprenant*

Re: [HACKERS] Risk Estimation WAS: Planner hints in Postgresql

Atri Sharma atri.j...@gmail.com writes:
 One of the factors that leads to bad estimates is that the histogram of the
 values of a column maintained by the planner gets old by time and the data
 in the column changes. So, the histogram is no longer a quite accurate view
 of the data and it leads to bad selectivity.

TBH, this is so far down the list of problems that it'll be a long time
before we need to worry about it.  It's certainly not the number one
priority for any project to model risk in the planner.

The thing that I think is probably the number one problem is estimates
that depend on an assumption of uniform distribution of sought-after rows
among those encountered by a scan.  This is usually where bad plans for
LIMIT queries are coming from.  We could certainly add some sort of fudge
factor to those costs, but I'd like to have a more-or-less principled
framework for doing so.

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] Wiki Page Draft for upcoming release

On 03/17/2014 05:49 PM, Josh Berkus wrote:
 All,
 
 https://wiki.postgresql.org/wiki/20140320UpdateIssues
 
 I'm sure my explanation of the data corruption issue is not correct, so
 please fix it.  Thanks!
 

Anyone?  We're going public with this in 36 hours, so I'd love for it to
actually be correct.

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

Bruce Momjian escribió:
 On Tue, Mar 18, 2014 at 06:11:30PM +, Simon Riggs wrote:
  On 18 March 2014 18:01, Bruce Momjian br...@momjian.us wrote:
   On Tue, Mar 18, 2014 at 04:17:53PM +, Simon Riggs wrote:
While I am not a fan of backpatching, the fact we are ignoring errors 
in
some critical cases seems the non-cosmetic parts should be backpatched.
  
   pg_resetxlog was not an offender here; its coding was sound.
  
   We shouldn't be discussing backpatching a patch that contains changes
   to coding style.
  
   I was going to remove the coding style changes to pg_resetxlog from the
   backpatched portion.
  
  Why make style changes at all? A patch with no style changes would
  mean backpatch and HEAD versions would be the same.
 
 The old style had errno set in two places in the loop, while the new
 style has it set in only one place.

I think it makes more sense to have all callsites look the same in all
supported branches.

-- 
Álvaro Herrerahttp://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] pg_archivecleanup bug

On 18 March 2014 18:18, Bruce Momjian br...@momjian.us wrote:
 On Tue, Mar 18, 2014 at 06:11:30PM +, Simon Riggs wrote:
 On 18 March 2014 18:01, Bruce Momjian br...@momjian.us wrote:
  On Tue, Mar 18, 2014 at 04:17:53PM +, Simon Riggs wrote:
   While I am not a fan of backpatching, the fact we are ignoring errors in
   some critical cases seems the non-cosmetic parts should be backpatched.
 
  pg_resetxlog was not an offender here; its coding was sound.
 
  We shouldn't be discussing backpatching a patch that contains changes
  to coding style.
 
  I was going to remove the coding style changes to pg_resetxlog from the
  backpatched portion.

 Why make style changes at all? A patch with no style changes would
 mean backpatch and HEAD versions would be the same.

 The old style had errno set in two places in the loop, while the new
 style has it set in only one place.

Seems better to leave the previously-good coding in place. ISTM to be
clearer to use simple C.

You're doing this anyway for the backpatch, so I'm not causing you any
more work. Better one patch than two.

-- 
 Simon Riggs   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] pg_archivecleanup bug

Simon Riggs escribió:
 On 18 March 2014 18:18, Bruce Momjian br...@momjian.us wrote:
  On Tue, Mar 18, 2014 at 06:11:30PM +, Simon Riggs wrote:

  Why make style changes at all? A patch with no style changes would
  mean backpatch and HEAD versions would be the same.
 
  The old style had errno set in two places in the loop, while the new
  style has it set in only one place.
 
 Seems better to leave the previously-good coding in place. ISTM to be
 clearer to use simple C.

If you're saying we should use that style in all readdir loops, with the
errno=0 before the loop and at the bottom of it, I don't disagree.
Let's just make sure they're all safe though (i.e. watch out for
continue for instance).

That said, I don't find comma expression to be particularly not
simple.

-- 
Álvaro Herrerahttp://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] plpgsql.warn_shadow



On 18/03/14 13:43, Pavel Stehule wrote:
2014-03-18 13:23 GMT+01:00 Petr Jelinek p...@2ndquadrant.com 
mailto:p...@2ndquadrant.com



Agree that compile_errors dos not make sense, additional_errors
and additional_warnings seems better, maybe plpgsql.extra_warnings
and plpgsql.extra_errors?


extra* sounds better


Ok, so I took the liberty of rewriting the patch so that it uses 
plpgsql.extra_warnings and plpgsql.extra_errors configuration variables 
with possible values none, all and shadow (none being the default).
Updated doc and regression tests to reflect the code changes, everything 
builds and tests pass just fine.


I did one small change (that I think was agreed anyway) from Marko's 
original patch in that warnings are only emitted during function 
creation, no runtime warnings and no warnings for inline (DO) plpgsql 
code either as I really don't think these optional warnings/errors 
during runtime are a good idea.


Note that the patch does not really handle the list of values as list, 
basically all and shadow are translated to true and proper handling 
of this is left to whoever will want to implement additional checks. I 
think this approach is fine as I don't see the need to build frameworks 
here and it was same in the original patch.


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

diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index bddd458..56ee2b3 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -4711,6 +4711,51 @@ a_output := a_output || $$ if v_$$ || referrer_keys.kind || $$ like '$$
   /variablelist
 
   /sect2
+  sect2 id=plpgsql-extra-checks
+   titleAdditional compile-time checks/title
+
+   para
+To aid the user in finding instances of simple but common problems before
+they cause harm, applicationPL/PgSQL/ provides a number of additional
+replaceablechecks/. When enabled, depending on the configuration, they
+can be used to emit a literalWARNING/ or an literalERROR/
+during the compilation of a function.
+   /para
+
+ para
+  These additional checks are enabled through the configuration variables
+  varnameplpgsql.extra_warnings/ for warnings and 
+  varnameplpgsql.extra_errors/ for errors. Both can be set either to
+  a comma-separated list of checks, literalnone/ or literalall/.
+  The default is literalnone/. Currently the list of available checks
+  includes only one:
+  variablelist
+   varlistentry
+termvarnameshadow/varname/term
+listitem
+ para
+  Checks if a declaration shadows a previously defined variable. For
+  example (with varnameplpgsql.extra_warnings/ set to
+  varnameshadow/varname):
+programlisting
+CREATE FUNCTION foo(f1 int) RETURNS int AS $$
+DECLARE
+f1 int;
+BEGIN
+RETURN f1;
+END
+$$ LANGUAGE plpgsql;
+WARNING:  variable f1 shadows a previously defined variable
+LINE 3: f1 int;
+^
+CREATE FUNCTION
+/programlisting
+ /para
+/listitem
+   /varlistentry
+  /variablelist
+ /para
+ /sect2
  /sect1
 
   !--  Porting from Oracle PL/SQL  --
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index 5afc2e5..8732efc 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -352,6 +352,9 @@ do_compile(FunctionCallInfo fcinfo,
 	function-out_param_varno = -1;		/* set up for no OUT param */
 	function-resolve_option = plpgsql_variable_conflict;
 	function-print_strict_params = plpgsql_print_strict_params;
+	/* only promote extra warnings and errors at CREATE FUNCTION time */
+	function-extra_warnings = plpgsql_extra_warnings  forValidator;
+	function-extra_errors = plpgsql_extra_errors  forValidator;
 
 	if (is_dml_trigger)
 		function-fn_is_trigger = PLPGSQL_DML_TRIGGER;
@@ -849,6 +852,9 @@ plpgsql_compile_inline(char *proc_source)
 	function-out_param_varno = -1;		/* set up for no OUT param */
 	function-resolve_option = plpgsql_variable_conflict;
 	function-print_strict_params = plpgsql_print_strict_params;
+	/* don't do extra validation for inline code as we don't want to add spam at runtime */
+	function-extra_warnings = false;
+	function-extra_errors = false;
 
 	plpgsql_ns_init();
 	plpgsql_ns_push(func_name);
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index c0cb585..98f7ddd 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -727,6 +727,20 @@ decl_varname	: T_WORD
 			  $1.ident, NULL, NULL,
 			  NULL) != NULL)
 			yyerror(duplicate declaration);
+
+		if (plpgsql_curr_compile-extra_warnings || plpgsql_curr_compile-extra_errors)
+		{
+			PLpgSQL_nsitem *nsi;
+			nsi = plpgsql_ns_lookup(plpgsql_ns_top(), false,
+	$1.ident, NULL, NULL, NULL);
+			if (nsi != NULL)
+ereport(plpgsql_curr_compile-extra_errors ? ERROR : WARNING,
+		(errcode(ERRCODE_DUPLICATE_ALIAS),
+		 errmsg(variable \%s\

Re: [HACKERS] Portability issues in shm_mq

After the last round of changes, I can confirm that my original test with
UTF8 locale works, and my HPPA box is happy too.  We could still stand
to improve the regression test 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] Failure while inserting parent tuple to B-tree is not fun


On 02/06/2014 06:42 AM, Peter Geoghegan wrote:

I'm not sure about this:


*** _bt_findinsertloc(Relation rel,
*** 675,680 
--- 701,707 
static void
_bt_insertonpg(Relation rel,
   Buffer buf,
+  Buffer cbuf,
   BTStack stack,
   IndexTuple itup,
   OffsetNumber newitemoff,


Wouldn't lcbuf be a better name for the new argument? After all, in
relevant contexts 'buf' is the parent of both of the pages post-split,
but there are two children in play here. You say:


+  *When inserting to a non-leaf page, 'cbuf' is the left-sibling 
of the
+  *page we're inserting the downlink for.  This function will 
clear the
+  *INCOMPLETE_SPLIT flag on it, and release the buffer.


I suggest also noting in comments at this point that cbuf/the
left-sibling is the old half from the split.

Even having vented about cbuf's name, I can kind of see why you didn't
call it lcbuf: we already have an BTPageOpaque lpageop variable for
the special 'buf' metadata within the _bt_insertonpg() function; so
there might be an ambiguity between the possibly-soon-to-be-left page
(the target page) and the *child* left page that needs to have its
flag cleared. Although I still think you should change the name of
lpageop (further details follow).

Consider this:


/* release buffers */
+   if (!P_ISLEAF(lpageop))
+   _bt_relbuf(rel, cbuf);


(Rebased, so this looks a bit different to your original version IIRC).

This is checking that the _bt_insertonpg() target page (whose special
data lpageop points to) is not a leaf page, and releasing the *child*
left page if it isn't. This is pretty confusing. So I suggest naming
lpageop tpageop ('t' for target, a terminology already used in the
comments above the function).


I don't know, the buf/page/lpageop variable names are used in many 
functions in nbtinsert.c. Perhaps they should all be renamed, but I 
think it's fine as it is, and not this patch's responsibility anyway. 
(The lpageop name makes sense when the page has to be split, and the 
page becomes the left page. Otherwise, admittedly, not so much)



Also, I suggest you change this existing code comment:

  * On entry, we must have the right buffer in which to do the
  * insertion, and the buffer must be pinned and write-locked.  
On return,
  * we will have dropped both the pin and the lock on the buffer.

Change right to correct here. Minor point, but right is a loaded word.


Fixed.


But why are you doing new things while checking P_ISLEAF(lpageop) in
_bt_insertonpg() to begin with? Would you not be better off doing
things when there is a child buffer passed (e.g. if
(BufferIsValid(cbuf))...), and only then asserting
!P_ISLEAF(lpageop)? That seems to me to more naturally establish the
context of _bt_insertonpg() is called to insert downlink after
split. Although maybe that conflates things within XLOG stuff. Hmm.


Changed that way for the places where we deal with the incomplete-split 
flag.


I committed the patch now. Thanks for the review!

Before committing, I spotted a bug in the way the new page-deletion code 
interacts with this: there was a check that the page that's about to be 
deleted was not marked with the INCOMPLETE_SPLIT flag, it was possible 
that the page was the right half of an incomplete split, and so didn't 
have a downlink. Vacuuming that failed with an failed to re-find parent 
key error. I fixed that by checking that the left sibling is also not 
marked with the flag.


It would be fairly easy to delete a page that is the right half of an 
incomplete split. Such a page doesn't have a downlink pointing to it, so 
just skip removing it, and instead clear the INCOMPLETE_SPLIT flag in 
the left sibling. But deleting incompletely split pages isn't important 
from a disk-space point of view, they should be extremely rare, so I 
decided to just punt.


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

On 18 March 2014 18:55, Alvaro Herrera alvhe...@2ndquadrant.com wrote:

 That said, I don't find comma expression to be particularly not
 simple.

Maybe, but we've not used it before anywhere in Postgres, so I don't
see a reason to start now. Especially since C is not the native
language of many people these days and people just won't understand
it.

-- 
 Simon Riggs   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] plpgsql.warn_shadow

2014-03-18 19:56 GMT+01:00 Petr Jelinek p...@2ndquadrant.com:


 On 18/03/14 13:43, Pavel Stehule wrote:

 2014-03-18 13:23 GMT+01:00 Petr Jelinek p...@2ndquadrant.com


  Agree that compile_errors dos not make sense, additional_errors and
 additional_warnings seems better, maybe plpgsql.extra_warnings and
 plpgsql.extra_errors?


  extra* sounds better


 Ok, so I took the liberty of rewriting the patch so that it uses
 plpgsql.extra_warnings and plpgsql.extra_errors configuration variables
 with possible values none, all and shadow (none being the default).
 Updated doc and regression tests to reflect the code changes, everything
 builds and tests pass just fine.


I don't think so only shadow is good name for some check. Maybe
shadow-variables-check

??



 I did one small change (that I think was agreed anyway) from Marko's
 original patch in that warnings are only emitted during function creation,
 no runtime warnings and no warnings for inline (DO) plpgsql code either as
 I really don't think these optional warnings/errors during runtime are a
 good idea.

 Note that the patch does not really handle the list of values as list,
 basically all and shadow are translated to true and proper handling of
 this is left to whoever will want to implement additional checks. I think
 this approach is fine as I don't see the need to build frameworks here and
 it was same in the original patch.

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

Re: [HACKERS] Wiki Page Draft for upcoming release

On Tue, Mar 18, 2014 at 6:41 PM, Josh Berkus j...@agliodbs.com wrote:

 Anyone?  We're going public with this in 36 hours, so I'd love for it to
 actually be correct.

I'll do a first pass now.


-- 
greg


-- 
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] GSoC proposal. Index-only scans for GIST

2014-03-18 Thread Alexander Korotkov

On Tue, Mar 18, 2014 at 5:12 PM, Anastasia Lubennikova 
lubennikov...@gmail.com wrote:

 2. gistget algorithm (several parts omitted):

 Check the key
 gistindex_keytest() - does this index tuple satisfy the scan key(s)?

 Scan all items on the GiST index page and insert them into the queue (or
 directly to output areas)

 plain scan

 Heap tuple TIDs are returned into so-pageData[]

 ordered scan

 Heap tuple TIDs are pushed into individual search queue items

 If the fetch() is specified by the developer, then using it, algorithm can
 retrieve the data directly to output areas at this stage, without reference
 to the heap.


I think there are following changes to be made to GiST code:
1) When next consistent IndexTuple is found extract original Datums using
fetch method.
2) Put those original Datums to queue.
3) When returning next ItemPointer from queue put original Datums to
IndexScanDesc (into xs_itup).

 3. Create function gistcanreturn to check whether fetch() is specified by
 user.

 Amcanreturn - Function to check whether index supports index-only scans,
 or zero if none

 There is the question of support index-only scans for multicolumn indexes.
 Probably it would require extend the interface to add separate columns
 checking.

 To solve this question I need the community's help.


 4. Add details for index only scans into gistcostestimate function


Also, another part of work to be mentioned is to implement fetch function
for all suitable opclasses.


With best regards,
Alexander Korotkov.

Re: [HACKERS] Portability issues in shm_mq

On Tue, Mar 18, 2014 at 12:15 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  It's tempting to instead add one or more tests that we specifically
 choose to have values we think are likely to exercise
 platform-specific differences or otherwise find bugs - e.g. just add a
 second test where the queue size and message length are both odd.

 Meh.  I think you're putting a bit too much faith in your ability to
 predict the locus of bugs that you think aren't there.

Well, I'm open to suggestions.

 maybe at a test where the queue is smaller than the message size, so
 that every message wraps (multiple times?).

 Does the code support messages larger than the queue size?  If so, yes,
 that case clearly oughta be tested.

Yep.  You should be able to send and receive any message that fits
within MaxAllocSize on even the smallest possible queue. Performance
may suck, but if that's an issue for you then don't use such a blasted
small queue.  The intended use of this is to stream (potentially long)
error messages or (potentially long and numerous) tuples between
cooperating backends without having to preallocate space for all the
data you want to send (which won't be any more feasible in a DSM than
it would be in the main segment).

Actually, you should be able to send or receive arbitrarily large
messages if you change the MemoryContextAlloc call in shm_mq.c to
MemoryContextAllocHuge, but I can't see any compelling reason to do
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] GSoC proposal. Index-only scans for GIST

2014-03-18 Thread Alexander Korotkov

Josh,

Anastasia has already consulted to me in person. It is not big proposal.
But for newbie who is not familiar with PostgreSQL code base and especially
GiST it seems fair enough.

With best regards,
Alexander Korotkov.

On Tue, Mar 18, 2014 at 9:16 PM, Josh Berkus j...@agliodbs.com wrote:

Alexander,

Can you comment on the below proposal? I'd like your opinion on how
difficult it will be.

On 03/18/2014 06:12 AM, Anastasia Lubennikova wrote:
Hello!
Here is the text of my proposal which I've applied to GSoC.
(and link

http://www.google-melange.com/gsoc/proposal/public/google/gsoc2014/lubennikovaav/5629499534213120
)
Any suggestions and comments are welcome.

*Project name*

Support for index-only scans for GIST index

*Brief review*

Currently GiST index don't have index-only scan functionality. Index-only
scans are a major performance feature added to Postgres 9.2. They allow
certain types of queries to be satisfied just by retrieving data from
indexes, and not from tables. This feature for B-trees (implemented since
PostgreSQL-9.2) allows doing certain types of queries significantly
faster.
This is achieved by reduction in the amount of I/O necessary to satisfy
queries. I think it will be useful to implement this feature for user
defined data types that use GiST index.

*Benefits to the PostgreSQL Community*

Faster GiST index search would be actual for many PostgreSQL applications
(for example some GIS systems).

*Quantifiable results*

Acceleration of GiST index search.

*Project details*

1. The GiST is a balanced tree structure like a B-tree, containing key,
pointer pairs. GiST key is a member of a user-defined class, and
represents some property that is true of all data items reachable from
the
pointer associated with the key. The GiST provides a possibility to
create
custom data types with indexed access methods and extensible set of
queries.

There are seven methods that an index operator class for GiST must
provide,
and an eighth that is optional.

-consistent

-union

-compress

-decompress

-penalty

-picksplit

-equal

-distance (optional)

2. gistget algorithm (several parts omitted):

Check the key
gistindex_keytest() - does this index tuple satisfy the scan key(s)?

Scan all items on the GiST index page and insert them into the queue (or
directly to output areas)

plain scan

Heap tuple TIDs are returned into so-pageData[]

ordered scan

Heap tuple TIDs are pushed into individual search queue items

If the fetch() is specified by the developer, then using it, algorithm
can
retrieve the data directly to output areas at this stage, without
reference
to the heap.

3. Create function gistcanreturn to check whether fetch() is specified by
user.

Amcanreturn - Function to check whether index supports index-only scans,
or
zero if none

There is the question of support index-only scans for multicolumn
indexes.
Probably it would require extend the interface to add separate columns
checking.

To solve this question I need the community's help.

4. Add details for index only scans into gistcostestimate function

*Links*

1) Hellerstein J. M., Naughton J. F., Pfeffer A. Generalized search
trees for database systems. - September, 1995.

2) http://www.sai.msu.su/~megera/postgres/gist/

3) PostgreSQL 9.3.3 Documentation: chapters 11. Indexes, 55. GiST
Indexes.

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

Re: [HACKERS] GSoC proposal. Index-only scans for GIST

On 03/18/2014 12:11 PM, Alexander Korotkov wrote:
 Josh,
 
 Anastasia has already consulted to me in person. It is not big proposal.
 But for newbie who is not familiar with PostgreSQL code base and especially
 GiST it seems fair enough.
 

Thanks, that's what I wanted to know.

-- 
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] B-tree descend for insertion locking

2014-03-18 Thread Peter Geoghegan

On Tue, Mar 18, 2014 at 4:12 AM, Heikki Linnakangas
hlinnakan...@vmware.com wrote:
 See attached patch. The new contract of _bt_getroot is a bit weird: it locks
 the returned page in the requested lock-mode, shared or exclusive, if the
 root page was also the leaf page. Otherwise it's locked in shared mode
 regardless off the requested lock mode. But OTOH, the new contract for
 _bt_search() is more clear now: it actually locks the returned page in the
 requested mode, where it used to only use the access parameter to decide
 whether to create a root page if the index was empty.

I had considered this myself, and am under the impression that the
only reason things work this way is because it makes the interface of
_bt_getroot() clearer. I think what you propose is logical, and you
should pursue it, but fwiw I'm not convinced that you've really
simplified _bt_search(). However, you have kind of made _bt_search()
into something that succinctly represents what is useful about Lehman
and Yao as compared to earlier concurrent locking techniques, and
that's a good thing. I would probably have remarked on that in the
comments if I were you. I certainly would not have left out some
reference to LY. You've removed an existing one where the lock
escalation occurs, emphasizing that it's safe, and I'd like to see you
at least compensate for 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] pg_archivecleanup bug


On 03/18/2014 09:04 PM, Simon Riggs wrote:

On 18 March 2014 18:55, Alvaro Herrera alvhe...@2ndquadrant.com wrote:


That said, I don't find comma expression to be particularly not
simple.


Maybe, but we've not used it before anywhere in Postgres, so I don't
see a reason to start now. Especially since C is not the native
language of many people these days and people just won't understand
it.


Agreed. The psqlODBC code is littered with comma expressions, and the 
first time I saw it, it took me a really long time to figure out what 
if (foo = malloc(...), foo) { }  meant. And I consider myself quite 
experienced with C.


- 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] plpgsql.warn_shadow

On 18 March 2014 19:04, Pavel Stehule pavel.steh...@gmail.com wrote:

 I don't think so only shadow is good name for some check. Maybe
 shadow-variables-check

shadowed-variables would be a better name.

-- 
 Simon Riggs   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] plpgsql.warn_shadow

2014-03-18 20:14 GMT+01:00 Simon Riggs si...@2ndquadrant.com:

 On 18 March 2014 19:04, Pavel Stehule pavel.steh...@gmail.com wrote:

  I don't think so only shadow is good name for some check. Maybe
  shadow-variables-check

 shadowed-variables would be a better name.


+1



 --
  Simon Riggs   http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training  Services

Re: [HACKERS] plpgsql.warn_shadow



On 18/03/14 20:15, Pavel Stehule wrote:


2014-03-18 20:14 GMT+01:00 Simon Riggs si...@2ndquadrant.com 
mailto:si...@2ndquadrant.com:


On 18 March 2014 19:04, Pavel Stehule pavel.steh...@gmail.com
mailto:pavel.steh...@gmail.com wrote:

 I don't think so only shadow is good name for some check. Maybe
 shadow-variables-check

shadowed-variables would be a better name.


+1


Attached V4 uses shadowed-variables instead of shadow.

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

diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index bddd458..d6709fd 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -4711,6 +4711,51 @@ a_output := a_output || $$ if v_$$ || referrer_keys.kind || $$ like '$$
   /variablelist
 
   /sect2
+  sect2 id=plpgsql-extra-checks
+   titleAdditional compile-time checks/title
+
+   para
+To aid the user in finding instances of simple but common problems before
+they cause harm, applicationPL/PgSQL/ provides a number of additional
+replaceablechecks/. When enabled, depending on the configuration, they
+can be used to emit either a literalWARNING/ or an literalERROR/
+during the compilation of a function.
+   /para
+
+ para
+  These additional checks are enabled through the configuration variables
+  varnameplpgsql.extra_warnings/ for warnings and 
+  varnameplpgsql.extra_errors/ for errors. Both can be set either to
+  a comma-separated list of checks, literalnone/ or literalall/.
+  The default is literalnone/. Currently the list of available checks
+  includes only one:
+  variablelist
+   varlistentry
+termvarnameshadowed-variables/varname/term
+listitem
+ para
+  Checks if a declaration shadows a previously defined variable. For
+  example (with varnameplpgsql.extra_warnings/ set to
+  varnameshadowed-variables/varname):
+programlisting
+CREATE FUNCTION foo(f1 int) RETURNS int AS $$
+DECLARE
+f1 int;
+BEGIN
+RETURN f1;
+END
+$$ LANGUAGE plpgsql;
+WARNING:  variable f1 shadows a previously defined variable
+LINE 3: f1 int;
+^
+CREATE FUNCTION
+/programlisting
+ /para
+/listitem
+   /varlistentry
+  /variablelist
+ /para
+ /sect2
  /sect1
 
   !--  Porting from Oracle PL/SQL  --
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index 5afc2e5..8732efc 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -352,6 +352,9 @@ do_compile(FunctionCallInfo fcinfo,
 	function-out_param_varno = -1;		/* set up for no OUT param */
 	function-resolve_option = plpgsql_variable_conflict;
 	function-print_strict_params = plpgsql_print_strict_params;
+	/* only promote extra warnings and errors at CREATE FUNCTION time */
+	function-extra_warnings = plpgsql_extra_warnings  forValidator;
+	function-extra_errors = plpgsql_extra_errors  forValidator;
 
 	if (is_dml_trigger)
 		function-fn_is_trigger = PLPGSQL_DML_TRIGGER;
@@ -849,6 +852,9 @@ plpgsql_compile_inline(char *proc_source)
 	function-out_param_varno = -1;		/* set up for no OUT param */
 	function-resolve_option = plpgsql_variable_conflict;
 	function-print_strict_params = plpgsql_print_strict_params;
+	/* don't do extra validation for inline code as we don't want to add spam at runtime */
+	function-extra_warnings = false;
+	function-extra_errors = false;
 
 	plpgsql_ns_init();
 	plpgsql_ns_push(func_name);
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index c0cb585..98f7ddd 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -727,6 +727,20 @@ decl_varname	: T_WORD
 			  $1.ident, NULL, NULL,
 			  NULL) != NULL)
 			yyerror(duplicate declaration);
+
+		if (plpgsql_curr_compile-extra_warnings || plpgsql_curr_compile-extra_errors)
+		{
+			PLpgSQL_nsitem *nsi;
+			nsi = plpgsql_ns_lookup(plpgsql_ns_top(), false,
+	$1.ident, NULL, NULL, NULL);
+			if (nsi != NULL)
+ereport(plpgsql_curr_compile-extra_errors ? ERROR : WARNING,
+		(errcode(ERRCODE_DUPLICATE_ALIAS),
+		 errmsg(variable \%s\ shadows a previously defined variable,
+$1.ident),
+		 parser_errposition(@1)));
+		}
+
 	}
 | unreserved_keyword
 	{
@@ -740,6 +754,20 @@ decl_varname	: T_WORD
 			  $1, NULL, NULL,
 			  NULL) != NULL)
 			yyerror(duplicate declaration);
+
+		if (plpgsql_curr_compile-extra_warnings || plpgsql_curr_compile-extra_errors)
+		{
+			PLpgSQL_nsitem *nsi;
+			nsi = plpgsql_ns_lookup(plpgsql_ns_top(), false,
+	$1, NULL, NULL, NULL);
+			if (nsi != NULL)
+ereport(plpgsql_curr_compile-extra_errors ? ERROR : WARNING,
+		(errcode(ERRCODE_DUPLICATE_ALIAS),
+		 errmsg(variable \%s\ shadows a previously defined variable,
+$1),
+		 parser_errposition(@1)));
+		}
+
 	}

Re: [HACKERS] Wiki Page Draft for upcoming release

On Tue, Mar 18, 2014 at 7:05 PM, Greg Stark st...@mit.edu wrote:
 I'll do a first pass now.

I fixed the Causes section. I haven't touched the other sections
which are pretty reasonable. It would be nice to have more suggestions
for what people should do other than dump/restore.

It would be nice to be able to tell people that if they vacuum, then
reindex and check all their foreign key constraints then they should
be ok. I think that's almost true except I'm not sure how to tell that
they've waited long enough before vacuuming and I'm not 100% sure
it'll fix the problem. (Also they could have lost a row which has
since had all all it's referencing rows deleted. The database won't be
able to help them find that.)

-- 
greg


-- 
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] Wiki Page Draft for upcoming release

2014-03-18 Thread Andres Freund

On 2014-03-18 19:28:53 +, Greg Stark wrote:
 It would be nice to be able to tell people that if they vacuum, then
 reindex and check all their foreign key constraints then they should
 be ok.

I don't think so:
http://archives.postgresql.org/message-id/20140317233919.GS16438%40awork2.anarazel.de

I still think a rewriting noop ALTER TABLE ... ALTER COLUMN col TYPE
old_type USING (col); is the only real thing to do.

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] plpgsql.warn_shadow

Hello


Tomorrow I'll do a review

fast check



   para
+To aid the user in finding instances of simple but common problems
before
+they cause harm, applicationPL/PgSQL/ provides a number of
additional
+replaceablechecks/. When enabled, depending on the configuration,
they
+can be used to emit either a literalWARNING/ or an
literalERROR/
+during the compilation of a function.
+   /para

provides a number of additional

There will be only one check in 9.4, so this sentence is confusing and
should be reformulated.

Regards

Pavel



2014-03-18 20:29 GMT+01:00 Petr Jelinek p...@2ndquadrant.com:


 On 18/03/14 20:15, Pavel Stehule wrote:


 2014-03-18 20:14 GMT+01:00 Simon Riggs si...@2ndquadrant.com:

 On 18 March 2014 19:04, Pavel Stehule pavel.steh...@gmail.com wrote:

  I don't think so only shadow is good name for some check. Maybe
  shadow-variables-check

  shadowed-variables would be a better name.


 +1


 Attached V4 uses shadowed-variables instead of shadow.

 --
  Petr Jelinek  http://www.2ndQuadrant.com/

  PostgreSQL Development, 24x7 Support, Training  Services

Re: [HACKERS] plpgsql.warn_shadow

2014-03-18 Thread Marko Tiikkaja


On 3/18/14, 7:56 PM, Petr Jelinek wrote:

Ok, so I took the liberty of rewriting the patch so that it uses
plpgsql.extra_warnings and plpgsql.extra_errors configuration variables
with possible values none, all and shadow (none being the default).
Updated doc and regression tests to reflect the code changes, everything
builds and tests pass just fine.


Cool, thanks!


I did one small change (that I think was agreed anyway) from Marko's
original patch in that warnings are only emitted during function
creation, no runtime warnings and no warnings for inline (DO) plpgsql
code either as I really don't think these optional warnings/errors
during runtime are a good idea.


Not super excited, but I can live with that.


Note that the patch does not really handle the list of values as list,
basically all and shadow are translated to true and proper handling
of this is left to whoever will want to implement additional checks. I
think this approach is fine as I don't see the need to build frameworks
here and it was same in the original patch.


Yeah, I don't think rushing all that logic into 9.4 would be such a good 
idea.  Especially since it's not necessary at all.



Regards,
Marko Tiikkaja


--
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: show relation and tuple infos of a lock to acquire

On Tue, Mar 18, 2014 at 12:53 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Alvaro Herrera alvhe...@2ndquadrant.com writes:
 Please see my reply to Robert.  My proposal (in form of a patch) is
   while operating on tuple (0,2) in table foo: updating tuple
 Would this work for you?

 It's pretty lousy from a readability standpoint, even in English;
 I shudder to think what it might come out as after translation.

 I think the enum idea you floated is probably worth doing.  It's
 certainly no more complex than passing a string, no?

+1.  We've done similar things in tablecmds.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] plpgsql.warn_shadow



On 18/03/14 20:36, Pavel Stehule wrote:


   para
+To aid the user in finding instances of simple but common 
problems before
+they cause harm, applicationPL/PgSQL/ provides a number of 
additional
+replaceablechecks/. When enabled, depending on the 
configuration, they
+can be used to emit either a literalWARNING/ or an 
literalERROR/

+during the compilation of a function.
+   /para

provides a number of additional

There will be only one check in 9.4, so this sentence is confusing and 
should be reformulated.


Thanks, yeah I left that sentence unchanged from original patch, maybe I 
should remove the word number there as it implies that there are a lot 
of them, but I don't really want to change everything to singular when 
the input is specified as list.


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



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

Re: [HACKERS] plpgsql.warn_shadow