Re: [HACKERS] proposal: ANSI SQL 2011 syntax for named parameters

2013-02-04 Thread Pavel Stehule
2013/1/2 Robert Haas robertmh...@gmail.com:
 On Fri, Dec 28, 2012 at 11:22 AM, Pavel Stehule pavel.steh...@gmail.com 
 wrote:
 I am not sure, but maybe is time to introduce ANSI SQL syntax for
 functions' named parameters

 It is defined in ANSI SQL 2011

  CALL P (B = 1, A = 2)

 instead PostgreSQL syntax CALL ( B := 1, A := 2)

 Keep in mind that, as recently as PostgreSQL 9.1, we shipped hstore
 with a =(text, text) operator.  That operator was deprecated in 9.0,
 but it wasn't actually removed until PostgreSQL 9.2.  Whenever we do
 this, it's going to break things for anyone who hasn't yet upgraded
 from hstore v1.0 to hstore v1.1.  So I would prefer to wait one more
 release.  That way, anyone who does an upgrade, say, every other major
 release cycle should have a reasonably clean upgrade path.

 I realize that the 4+-year journey toward allowing = rather than :=
 probably seems tedious to many people by now, but I think the cautious
 path we've taken is entirely warranted.  As much as I want us to be
 standards-compliant in this area, I also want us to not break any more
 user applications than necessary along the way.

 Incidentally, I think there are two changes here which should be
 considered independently.  One, allowing = rather than := for
 specifying named parameters.  And two, adding a statement called CALL
 that can be used to invoke a function.  Maybe those are both good
 ideas and maybe they aren't, but they're independent.


can I recapitulate a plan?

* enabling '=' in 9.4
* we will support ':=' too

What we can (or have to) do now?

Regards

Pavel



 --
 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] cannot move relocatable extension out of pg_catalog schema

2013-02-04 Thread Hannu Krosing

On 02/04/2013 02:16 AM, Robert Haas wrote:

On Fri, Feb 1, 2013 at 5:13 PM, Peter Eisentraut pete...@gmx.net wrote:

I wonder whether it'd not be a better idea to forbid specifying
pg_catalog as the target schema for relocatable extensions.

But that would be important, I think.

I understand the temptation to forbid pg_catalog as the target schema
for relocatable extensions, or indeed for object creation in general.
The fact that you can't, for example, go back and drop the objects
later is a real downer. On the other hand, from a user perspective,
it's really tempting to want to create certain extensions (adminpack,
for example) in such a way that they appear to be part of the system
rather than something that lives in a user schema.  Had we some other
solution to that problem (a second schema that behaves like pg_catalog
but is empty by default and allows drops?) we might alleviate the need
to put stuff in pg_catalog per se.

+1

Having a standard schema for extensions (say pg_extensions) is
something I have wanted multiple times.

Hannu





--
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] sepgsql and materialized views

2013-02-04 Thread Kohei KaiGai
2013/2/3 Kevin Grittner kgri...@ymail.com:
 I'm hoping that someone familiar with sepgsql can review this portion of the 
 materialized view patch and comment on whether it is
 the best approach for dealing with the integration of these two
 features.  Basically, the patch as it stands treats a materialized
 view as a table for purposes of sepgsql labels.  I initially
 invented new lables, but Robert suggested that this would make
 materialized views unusable in an SE environment until the
 corresponding labels were added at the OS level.  It seems sane to
 me because a materialized view exists on disk the same as a table,
 but is populated differently -- from a view-like rule.

 The portion of the patch which affects the contrib/sepgsql/ tree is
 attached.

Hi Kevin,

Sorry, I have not been involved this discussion.
I briefly checked your patch. Indeed, it performs like a table, even though
it is named materialized-view.

Probably, we have two standpoints.

First, materialized-view shall have a security label corresponding to table,
and related checks handle references to materialized-views as if user
references regular-tables. This is an idea.
I briefly checked your latest patch. ExecRefreshMatView is a unique entry
point to update a particular materialized-view, and REFRESH MATERIALIZED
VIEW command is only way to kick this function. It also checks permissions to
reference underlying tables. So, it means update of materialized-view is a stuff
like writing a table with contents read from other tables by a particular users.

However, I'm worried whether this design continues forever, or not.
IIRC, you have a plan to refresh materialized-view asynchronously using
background worker stuff, don't you? Once we support an internal stuff (thus,
it can bypass valid security checks) to write out confidential contents into
unconfidential zone, it does not make sense to keep data confidentiality.

So, I'd like to suggest second way; that handles a materialized-view as a view.
SELinux checks db_view:{expand} permissions and relevant permissions
towards underlying tables. I don't think it is hard to implement because
relation-rd_rules holds Query tree to reference underlying tables.

Can you wait for a week? I'll adjust contrib/sepgsql portion to fit
materialized-
view with matter of existing view.

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.jp


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


Re: [HACKERS] pg_dump --pretty-print-views

2013-02-04 Thread Marko Tiikkaja

On 2/3/13 10:06 PM, Tom Lane wrote:

Applied with corrections.


Thank you.


The xml expected output was still wrong - to do that part right, you
need to update xml.out with an xml-enabled build and xml_1.out with a
non-xml-enabled build.


Ahh.  That explains a lot.



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] GetOldestXmin going backwards is dangerous after all

2013-02-04 Thread Andres Freund
On 2013-02-01 19:24:02 -0500, Tom Lane wrote:
 Robert Haas robertmh...@gmail.com writes:
  Having said that, I agree that a fix in GetOldestXmin() would be nice
  if we could find one, but since the comment describes at least three
  different ways the value can move backwards, I'm not sure that there's
  really a practical solution there, especially if you want something we
  can back-patch.
 
 Actually, wait a second.  As you say, the comment describes three known
 ways to make it go backwards.  It strikes me that all three are fixable:
 
  * if allDbs is FALSE and there are no transactions running in the current
  * database, GetOldestXmin() returns latestCompletedXid. If a transaction
  * begins after that, its xmin will include in-progress transactions in other
  * databases that started earlier, so another call will return a lower value.
 
 The reason this is a problem is that GetOldestXmin ignores XIDs of
 processes that are connected to other DBs.  It now seems to me that this
 is a flat-out bug.  It can ignore their xmins, but it should include
 their XIDs, because the point of considering those XIDs is that they may
 contribute to the xmins of snapshots computed in the future by processes
 in our own DB.  And snapshots never exclude any XIDs on the basis of
 which DB they're in.  (They can't really, since we can't know when the
 snap is taken whether it might be used to examine shared catalogs.)

  * The return value is also adjusted with vacuum_defer_cleanup_age, so
  * increasing that setting on the fly is another easy way to make
  * GetOldestXmin() move backwards, with no consequences for data integrity.

 And as for that, it's been pretty clear for awhile that allowing
 vacuum_defer_cleanup_age to change on the fly was a bad idea we'd
 eventually have to undo.  The day of reckoning has arrived: it needs
 to be PGC_POSTMASTER.

ISTM that the original problem can still occur, even after Simon's
commit.
1) start with -c vacuum_defer_cleanup_age=0
2) autovacuum vacuums test;
3) restart with -c vacuum_defer_cleanup_age=1
4) autovacuum vacuums test's toast table;

should result in about the same ERROR, shouldn't it?

Given that there seemingly isn't yet a way to fix that people agree on
and that it only result in a transient error I think the fix for this
should be pushed after the next point release.

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] Temporal features in PostgreSQL

2013-02-04 Thread Miroslav Šimulčík
Hi Vlad,

I'm also interested in this topic and work on system-time temporal
extension. Here I wrote down design of my solution few months ago
https://wiki.postgresql.org/wiki/SQL2011Temporal. The idea is basically the
same as in your solution with some minor differences. For example:
 - I use after triggers to store old versions of rows into historical
table, so the row is archived only if modification is actualy executed.
 - I don't need to deal with update conflicts, because I use
clock_timestamp() instead of current_timestamp.
 - Inheritence relation between historical and current table allows to
easily select whole history of rows.

Although my solution needs changes in parser to stick with SQL 2011
standard, maybe you can find something that can help you.

Regards,
Miro


2012/12/25 Vlad Arkhipov arhi...@dc.baikal.ru

 Hi all,

 Currently I'm working on a large enterprise project that heavily uses
 temporal features. We are using PostgreSQL database for data storage. Now
 we are using PL/pgSQL trigger-based and application-based solutions to
 handle with temporal data. However we would like to see this functionality
 in PostgreSQL core, especially in SQL 2011 syntax. There were some
 discussions several months ago on temporal support and audit logs:

 http://archives.postgresql.**org/pgsql-hackers/2012-05/**msg00765.phphttp://archives.postgresql.org/pgsql-hackers/2012-05/msg00765.php
 http://archives.postgresql.**org/pgsql-hackers/2012-08/**msg00680.phphttp://archives.postgresql.org/pgsql-hackers/2012-08/msg00680.php

 But currently it seems that there is no active work in this area (am I
 wrong?) Now I'm rewriting our temporal solutions into an extension that is
 based on C-language triggers to get a better sense of the problem space and
 various use cases. There are two aspects that temporal features usually
 include: system-time (aka transaction-time) and application-time (aka
 valid-time or business-time). The topics above discussed only the first
 one. However there is also another one, which includes application-time
 periods, partial updated/deletes queries, querying for a portion of
 application time etc. Details can be found here

 http://metadata-standards.org/**Document-library/Documents-by-**
 number/WG2-N1501-N1550/WG2_**N1536_koa046-Temporal-**
 features-in-SQL-standard.pdfhttp://metadata-standards.org/Document-library/Documents-by-number/WG2-N1501-N1550/WG2_N1536_koa046-Temporal-features-in-SQL-standard.pdf

 or in the SQL-2011 Standard Draft which is available freely on the
 network. It's hard to create a convenient extension for application-time
 periods because it needs the parser to be changed (however an extension may
 be useful for referential integrity checks for application-time period
 temporal tables).

 I created a simple solution for system-time period temporal tables, that
 consist of only one trigger (it resembles SPI/timetravel trigger but is
 based on new range types that were introduced in PostgreSQL 9.2 and it's
 closer to the SQL-2011 approach for implementation of temporal features).

 http://pgxn.org/dist/temporal_**tables/1.0.0/http://pgxn.org/dist/temporal_tables/1.0.0/

 I'm not a PostgreSQL expert, so I would appreciate if someone could review
 the code briefly. There are some places I'm not sure I use some functions
 properly. Also there are some slight problems with the design that I would
 like to discuss if anyone is interested in.


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



Re: [HACKERS] Streaming-only cascading replica won't come up without writes on the master

2013-02-04 Thread Josh Berkus

 Can you check if you still see that behavior with REL9_2_STABLE?

Will do.

-- 
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] [PATCH] pg_isready (was: [WIP] pg_ping utility)

2013-02-04 Thread Bruce Momjian
On Sat, Feb  2, 2013 at 09:55:29PM -0500, Phil Sorber wrote:
  I think that output is important as do others up thread. I think it'd
  be simpler to just disable dbname's ability to double as conninfo. If
  we are looking for easy, that is.
 
  I don't mind duplicating the behavior from conninfo_array_parse() or
  uri-regress.c either. I can just put a comment that at some point in
  the future we should add a libpq interface for it.
 
  I suggest duplicate the code for 9.3, and submit a patch to refactor
  into a new libpq function for CF2013-next.  If the patch is simple
  enough, we can consider putting it into 9.3.
 
  Agreed.
 
  Regards,
 
  --
  Fujii Masao
 
 OK, here is the patch that handles the connection string in dbname.
 I'll post the other patch under a different posting because I am sure
 it will get plenty of debate on it's own.

If we could run pg_isready on the patch, it would tell us if the patch
is ready.  Consider this a feature request.  ;-)

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

  + It's impossible for everything to be true. +


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


Re: [HACKERS] missing rename support

2013-02-04 Thread Ali Dar
The tweaks made by you seems fine. I'm good with it.

Regards,
Ali Dar


On Sun, Feb 3, 2013 at 8:04 PM, Dean Rasheed dean.a.rash...@gmail.comwrote:

 On 29 January 2013 15:34, Ali Dar ali.munir@gmail.com wrote:
  Please find attached the complete patch for alter rename rule. I have
  followed all the suggestions.

 This looks good. I've tested it, and it appears to work as intended.
 I'm happy with the code, and the new docs and regression tests look
 OK.

 I have a couple of minor tweaks (see attached):

 * On the new manual page, I replaced table with table or view.

 * In the new tab-completion code, I modified the query so that it
 completes with tables as well as views, and limited the results to
 just those relations that have a rule with the name specified,
 otherwise the list of completions could be very long.

 If you're happy with these changes, I think this is ready for committer
 review.

 Regards,
 Dean



Re: [HACKERS] [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

2013-02-04 Thread Alvaro Herrera

  On Feb 3, 2013 4:16 AM, Phil Sorber p...@omniti.com wrote:
 
  This patch came up from discussion about pg_isready.
 
  PQconninfoParseParams is similar to PQconninfoParse but takes two
  arrays like PQconnectdbParams. It essentially exposes
  conninfo_array_parse().
 
  PQconninfodefaultsMerge essentially exposes conninfo_add_defaults().
  It allows you to pass a PQconninfoOption struct and it adds defaults
  for all NULL values.

Uh, no existing code can use this new functionality?  That seems
disappointing.


-- 
Á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] json api WIP patch

2013-02-04 Thread Merlin Moncure
On Fri, Feb 1, 2013 at 4:08 PM, Robert Haas robertmh...@gmail.com wrote:
 But even leaving that aside, I'm surprised to hear so many people
 dismissing SQL standards compliance so blithely.  We've certainly
 spent a lot of blood, sweat, and tears on minor standards-compliance
 issues over they years - why is it OK to not care about this
 particular issue when we've spent so much effort caring about other
 ones?

Does the SQL Standard suggest you can't extend the language with
operators?  Or does it reserve certain characters for future use?  And
if so, is there a list?

merlin


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


Re: [HACKERS] ALTER command reworks

2013-02-04 Thread Alvaro Herrera
Kohei KaiGai escribió:
 2013/2/3 Tom Lane t...@sss.pgh.pa.us:
  Alvaro Herrera alvhe...@2ndquadrant.com writes:
  [ pgsql-v9.3-alter-reworks.3-rename.v10.patch.gz ]
 
  Say ... I hadn't been paying too close attention to this patch, but
  is there any particularly principled reason for it having unified
  only 14 of the 29 object types handled by ExecRenameStmt()?
  If so, how to tell which object types are supposed to be covered?
 
  The reason I'm asking is that it's very unclear to me whether
  https://commitfest.postgresql.org/action/patch_view?id=1043
  (ALTER RENAME RULE) is okay in more-or-less its current form,
  or whether it ought to be bounced back to be reworked for integration
  in this framework.
 
 Like trigger or constraint, rule is unavailable to integrate the generic
 rename logic using AlterObjectRename_internal().
 So, I don't think this patch needs to take much design change.

I did give that patch a glance last week, asked myself the same question
as Tom, and gave myself the same answer as KaiGai.  Sorry I didn't post
that.

-- 
Á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] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

2013-02-04 Thread Phil Sorber
On Mon, Feb 4, 2013 at 9:13 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote:

  On Feb 3, 2013 4:16 AM, Phil Sorber p...@omniti.com wrote:
 
  This patch came up from discussion about pg_isready.
 
  PQconninfoParseParams is similar to PQconninfoParse but takes two
  arrays like PQconnectdbParams. It essentially exposes
  conninfo_array_parse().
 
  PQconninfodefaultsMerge essentially exposes conninfo_add_defaults().
  It allows you to pass a PQconninfoOption struct and it adds defaults
  for all NULL values.

 Uh, no existing code can use this new functionality?  That seems
 disappointing.


I wrote this because I wanted to use it in pg_isready. I also wrote
something for pg_isready to get around not having this. I think adding
these two functions to libpq would be the better option, but wanted
something that could fix an existing issue without having to patch
libpq so late in the 9.3 development process. Actually, I think it was
you who suggested that approach.

So long answer short, there is existing code that can use this
functionality immediately.


 --
 Á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] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

2013-02-04 Thread Alvaro Herrera
Phil Sorber wrote:
 On Mon, Feb 4, 2013 at 9:13 AM, Alvaro Herrera alvhe...@2ndquadrant.com 
 wrote:

  Uh, no existing code can use this new functionality?  That seems
  disappointing.
 
 I wrote this because I wanted to use it in pg_isready. I also wrote
 something for pg_isready to get around not having this. I think adding
 these two functions to libpq would be the better option, but wanted
 something that could fix an existing issue without having to patch
 libpq so late in the 9.3 development process. Actually, I think it was
 you who suggested that approach.

Yes, I realize that (and yes, I did).  But is no code other than
pg_isready doing this?  Not even the libpq URI test program?

-- 
Á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] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

2013-02-04 Thread Phil Sorber
On Mon, Feb 4, 2013 at 10:16 AM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
 Phil Sorber wrote:
 On Mon, Feb 4, 2013 at 9:13 AM, Alvaro Herrera alvhe...@2ndquadrant.com 
 wrote:

  Uh, no existing code can use this new functionality?  That seems
  disappointing.

 I wrote this because I wanted to use it in pg_isready. I also wrote
 something for pg_isready to get around not having this. I think adding
 these two functions to libpq would be the better option, but wanted
 something that could fix an existing issue without having to patch
 libpq so late in the 9.3 development process. Actually, I think it was
 you who suggested that approach.

 Yes, I realize that (and yes, I did).  But is no code other than
 pg_isready doing this?  Not even the libpq URI test program?

I think it probably would be able to benefit from this. Are you
suggesting I patch that too? I thought it was usually frowned upon to
touch random bits of working code like that. I'd be more than happy to
do it if it helps build the case for getting this added.


 --
 Á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] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

2013-02-04 Thread Alvaro Herrera
Phil Sorber wrote:
 On Mon, Feb 4, 2013 at 10:16 AM, Alvaro Herrera
 alvhe...@2ndquadrant.com wrote:
  Phil Sorber wrote:
  On Mon, Feb 4, 2013 at 9:13 AM, Alvaro Herrera alvhe...@2ndquadrant.com 
  wrote:
 
   Uh, no existing code can use this new functionality?  That seems
   disappointing.
 
  I wrote this because I wanted to use it in pg_isready. I also wrote
  something for pg_isready to get around not having this. I think adding
  these two functions to libpq would be the better option, but wanted
  something that could fix an existing issue without having to patch
  libpq so late in the 9.3 development process. Actually, I think it was
  you who suggested that approach.
 
  Yes, I realize that (and yes, I did).  But is no code other than
  pg_isready doing this?  Not even the libpq URI test program?
 
 I think it probably would be able to benefit from this. Are you
 suggesting I patch that too? I thought it was usually frowned upon to
 touch random bits of working code like that. I'd be more than happy to
 do it if it helps build the case for getting this added.

Well, this qualifies as refactoring, so yeah, it helps the case.

-- 
Á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] Audit Logs WAS: temporal support patch

2013-02-04 Thread Miroslav Šimulčík
Hi Josh,

I was quite busy last few months and I didn't have time to devote to this
topic. I have read the feedbacks and want to make things move now.

I suggest, we can start by discussing the design of my solution:
https://wiki.postgresql.org/wiki/SQL2011Temporal

I expect any criticism and recommendations for making this solution
acceptable.

Regards,
Miro

First, note the change in topic.
 This whole discussion has gone rather far afield from Miroslav's
 original submission, which was for temporal tables, which is NOT the
 same thing as audit logs, although the use cases overlap significantly.
 Miroslav, I know this has been hard to follow, but you're getting a lot
 of feedback because people are really interested in the feature and
 related features.

 That sounds like a good way to start. Actually, even before the tool,
 how about just some really good examples of triggers for specific kinds
 of audit logs, and some ways to run queries on them? I think that might
 settle a lot of these details.

 Well, I'm not adverse to solving some problems in the core:
 1) That it's difficult/impossible to write a completely generic audit
 trigger which works with any table without utilizing an external SP
 language like Python.
 2) That there's no obvious way to handle audit triggers and FK
 relationships intelligently.
 3) That audit tables don't automatically track schema changes in the
 live table.
 4) Checking which columns have changed (see Craig Ringer's email)
 These seem like difficult enough challenges without getting more
 complicated.
 --
 Josh Berkus
 PostgreSQL Experts Inc.
 http://pgexperts.co


Re: [HACKERS] Turning auto-analyze off (was Re: [GENERAL] Unusually high IO for autovacuum worker)

2013-02-04 Thread Robert Haas
On Fri, Feb 1, 2013 at 12:33 PM, Pavan Deolasee
pavan.deola...@gmail.com wrote:
 I can write a patch in next couple of days if we are willing to accept
 for this release. I think it should be fairly easy and non-intrusive.

I think it's too late to consider this for 9.3, but I think we should
entertain it for 9.4.

The biggest problem I see is that the naming of the new reloptions
might end up being something kind of unintuitive, like
autovacuum_vacuum_enabled and autovacuum_analyze_enabled.  You need a
degree in PostgreSQLology to understand what that means, but I haven't
got a better idea.

-- 
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] logical changeset generation v4 - Heikki's thoughts about the patch state

2013-02-04 Thread Robert Haas
On Sat, Feb 2, 2013 at 4:38 PM, Andres Freund and...@2ndquadrant.com wrote:
 On 2013-01-28 16:55:52 -0500, Steve Singer wrote:
 If your using non-surragate /natural primary keys this tends to come up
 occasionally due to data-entry errors or renames.  I'm  looking at this from
 the point of view of what do I need to use this as a source for a production
 replication system with fewer sharp-edges compared to trigger source slony.
 My standard is a bit higher than 'first' version because I intent to use it
 in the version 3.0 of slony not 1.0.  If others feel I'm asking for too much
 they should speak up, maybe I am. Also the way things will fail if someone
 were to try and update a primary key value is pretty nasty (it will leave
 them with inconsistent databases).We could  install UPDATE triggers to
 try and detect this type of thing but I'd rather see us just log the old
 values so we can use them during replay.

 I pushed support for this. I am not yet 100% happy with this due to two
 issues:

 * it increases the xlog size logged by heap_update by 2 bytes even with
   wal_level  logical as it uses a variant of xl_heap_header that
   includes its lenght. Conditionally using xl_heap_header would make the
   code even harder to read. Is that acceptable?

I think it's important to avoid adding to DML WAL volume when
wal_level  logical.  I am not positive that 2 bytes is noticeable,
but I'm not positive that it isn't either: heap insert/update must be
our most commonly-used WAL records.  On the other hand, we also need
to keep in mind that branches in hot code paths aren't free either.  I
would be concerned more about the increased run-time cost of
constructing the correct WAL record than with the related code
complexity.  None of that code is simple anyway.

 * multi_insert should be converted to use xl_heap_header_len as well,
   instead of using xl_multi_insert_tuple, that would also reduce the
   amount of multi-insert specific code in decode.c
 * both for update and delete we should denote more explicitly that
   -oldtuple points to an index tuple, not to an full 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] json api WIP patch

2013-02-04 Thread Robert Haas
On Sun, Feb 3, 2013 at 9:05 PM, Andrew Dunstan and...@dunslane.net wrote:
 Yeah, this is surely not a workable policy unless we first move all
 those planner smarts to apply to functions not operators.  And rewrite
 all the index AM APIs to use functions not operators, too.  Now this is
 something that's been a wish-list item right along, but actually doing
 it has always looked like a great deal of work for rather small reward.

 Hmm.  Well, if the operators are going to be indexable, then I agree
 that's an issue, but isn't - just a key-extraction operator?  That
 wouldn't be something you could index anyway.

 Er, what? It gives you the value corresponding to a key (or the numbered
 array element).

That's what I figured.

 With the Json operators I provided you're more likely to use - in an
 index, because it returns de-escaped text rather than json, but I don't see
 any reason in principle why - couldn't be used.

The point is that if you're talking about indexing something like
myeav-'andrew' you could equally well index json_get(myeav,
'andrew').  So there's no real need for it to be an operator rather
than a function.

The case in which it would matter is if it were something that could
be used as an index predicate, like:

Index Scan
  - Index Cond: myeav-'andrew'

As of now, the query planner won't consider such a plan if it's only a
function and not an operator.  So if we had a case like that, the use
of operator notation could be justified on performance grounds.  If we
don't, I argue that it's better to stick with functional notation,
because the number of sensible function names is much larger than the
number of sensible operator names, and if we start using operator
notation every time someone thinks it will look nicer that way, we
will very quickly either run out of nice-looking operator names or
start overloading them heavily.

The SQL standards considerations seem worth thinking about, too.
We've certainly gone through a lot of pain working toward eliminating
= as an operator name, and if the SQL standard has commandeered -
for some purpose or other, I'd really rather not add to the headaches
involved should we ever decide to reclaim it.

-- 
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] Turning off hot_standby_feedback

2013-02-04 Thread Robert Haas
On Sun, Feb 3, 2013 at 5:25 PM, Simon Riggs si...@2ndquadrant.com wrote:
 There is a bug in hot_standby_feedback that causes the xmin of the
 walsender process to not be reset when hot_standby_feedback is turned
 off after it had previously sent at least one feedback message.

 Clearly, that is a bad thing and deserves backpatching to 9.1, unless
 I hear otherwise real soon.

+1.

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


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


Re: [HACKERS] sepgsql and materialized views

2013-02-04 Thread Kevin Grittner
Kohei KaiGai kai...@kaigai.gr.jp wrote:
 2013/2/3 Kevin Grittner kgri...@ymail.com:
 I'm hoping that someone familiar with sepgsql can review this
 portion of the materialized view patch and comment on whether it is
 the best approach for dealing with the integration of these two
 features.  Basically, the patch as it stands treats a materialized
 view as a table for purposes of sepgsql labels.  I initially
 invented new lables, but Robert suggested that this would make
 materialized views unusable in an SE environment until the
 corresponding labels were added at the OS level.  It seems sane to
 me because a materialized view exists on disk the same as a table,
 but is populated differently -- from a view-like rule.

 The portion of the patch which affects the contrib/sepgsql/ tree is
 attached.

 Hi Kevin,

 Sorry, I have not been involved this discussion.
 I briefly checked your patch. Indeed, it performs like a table, even though
 it is named materialized-view.

 Probably, we have two standpoints.

 First, materialized-view shall have a security label corresponding to table,
 and related checks handle references to materialized-views as if user
 references regular-tables. This is an idea.
 I briefly checked your latest patch. ExecRefreshMatView is a unique entry
 point to update a particular materialized-view, and REFRESH MATERIALIZED
 VIEW command is only way to kick this function. It also checks permissions to
 reference underlying tables. So, it means update of materialized-view is a 
 stuff
 like writing a table with contents read from other tables by a particular 
 users.

 However, I'm worried whether this design continues forever, or not.
 IIRC, you have a plan to refresh materialized-view asynchronously using
 background worker stuff, don't you?

The goal is to build on this to support other timings of updates to
the materialized view.  In general, I think this will take the form
of identifying, for the given rewrite rules associated with the
materialized view, what changes to the underlying data can affect
the view and determining whether incremental updates can be
supported for the MV.  If incremental update is possible, then
various timings can be chosen for applying them.  I think that all
timing should go through a queue of change requests, although that
is not yet designed, and I'm just waving my hands around and
speculating about any details.  Timings likely to be supported
range from on-demand incremental updates (of all accumlated
changes) to applying the changes from a transaction at COMMIT time,
or possibly as the underlying changes are made within a
transaction.  I suspect that the most popular timing will involve a
background process working from the queue asynchronously to keep
the MV updated asynchronously but without any artificial delay.

In all cases, a query against the view does not reach below the MV
table to the underlying tables or functions used to build the data
-- it will always read the materialized data, so Im not sure
that the normal concerns about leaky views apply here.

 Once we support an internal stuff (thus,
 it can bypass valid security checks) to write out confidential contents into
 unconfidential zone, it does not make sense to keep data confidentiality.

If the person who creates the materialized view has authority to
query the underlying tables, and grants access to the materialized
view as desired, and those selecting from the materialized view
only see the contents of the table which is being populated by the
underlying pg_rewrite rule, I'm not understanding your concern. 
Can you elaborate?

 So, I'd like to suggest second way; that handles a materialized-view as a
 view.

I don't see why that should apply to anyone selecting from the MV. 
Certainly it must apply to anyone creating the MV.  I'm not at all
clear why anything special would be required for REFRESH or
updating the underlying tables which will eventually generate
incremental changes.  I might be missing something, but could you
explain any risks you see?

 SELinux checks db_view:{expand} permissions and relevant permissions
 towards underlying tables. I don't think it is hard to implement because
 relation-rd_rules holds Query tree to reference underlying tables.

 Can you wait for a week? I'll adjust contrib/sepgsql portion to fit
 materialized-view with matter of existing view.

Before we code a different alternative, I would like to understand
why.  What risks do you see, exactly?

-Kevin



-- 
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] GetOldestXmin going backwards is dangerous after all

2013-02-04 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes:
 On 2013-02-01 19:24:02 -0500, Tom Lane wrote:
 And as for that, it's been pretty clear for awhile that allowing
 vacuum_defer_cleanup_age to change on the fly was a bad idea we'd
 eventually have to undo.  The day of reckoning has arrived: it needs
 to be PGC_POSTMASTER.

 ISTM that the original problem can still occur, even after Simon's
 commit.
 1) start with -c vacuum_defer_cleanup_age=0
 2) autovacuum vacuums test;
 3) restart with -c vacuum_defer_cleanup_age=1
 4) autovacuum vacuums test's toast table;

 should result in about the same ERROR, shouldn't it?

Hm ... yeah, you're right.  So that's still not bulletproof.

 Given that there seemingly isn't yet a way to fix that people agree on
 and that it only result in a transient error I think the fix for this
 should be pushed after the next point release.

Agreed, we can let this go until we have a more complete solution.

Simon, would you revert the vacuum_defer_cleanup_age changes?
We should wait till we have a complete fix before forcing that
redefinition on users.

regards, tom lane


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


Re: [HACKERS] json api WIP patch

2013-02-04 Thread Andrew Dunstan


On 02/04/2013 10:47 AM, Robert Haas wrote:


The SQL standards considerations seem worth thinking about, too.
We've certainly gone through a lot of pain working toward eliminating
= as an operator name, and if the SQL standard has commandeered -
for some purpose or other, I'd really rather not add to the headaches
involved should we ever decide to reclaim it.



OK, but I'd like to know what is going to be safe. There's no way to 
future-proof the language. I'm quite prepared to replace - with 
something else, and if I do then - will need to be adjusted 
accordingly, I think.


My suggestion would be ~ and ~. I know David Wheeler didn't like that 
on the ground that some fonts elevate ~ rather than aligning it in the 
middle as most monospaced fonts do, but I'm tempted just to say then 
use a different font. Other possibilities that come to mind are + and 
+, although I think they're less attractive. But I'll be guided by the 
consensus, assuming there is one ;-)


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] json api WIP patch

2013-02-04 Thread Benedikt Grundmann
On Mon, Feb 4, 2013 at 4:10 PM, Andrew Dunstan and...@dunslane.net wrote:


 On 02/04/2013 10:47 AM, Robert Haas wrote:


 The SQL standards considerations seem worth thinking about, too.
 We've certainly gone through a lot of pain working toward eliminating
 = as an operator name, and if the SQL standard has commandeered -
 for some purpose or other, I'd really rather not add to the headaches
 involved should we ever decide to reclaim it.



 OK, but I'd like to know what is going to be safe. There's no way to
 future-proof the language. I'm quite prepared to replace - with something
 else, and if I do then - will need to be adjusted accordingly, I think.

 My suggestion would be ~ and ~. I know David Wheeler didn't like that
 on the ground that some fonts elevate ~ rather than aligning it in the
 middle as most monospaced fonts do, but I'm tempted just to say then use a
 different font. Other possibilities that come to mind are + and +,
 although I think they're less attractive. But I'll be guided by the
 consensus, assuming there is one ;-)

 As a user I would be much in favor of just functions and no additional
operators if the sole difference is syntactical.  I think custom operators
are much harder to remember than function names (assuming reasonably well
chosen function names).

Now Robert seems to suggest that there will also be speed / planner
difference which seems sad (I would have expected operators to be just
syntactical sugar for specially named functions and once we are past the
parser there should be no difference).


Re: [HACKERS] [PATCH 4/5] Add pg_xlogdump contrib module

2013-02-04 Thread Alvaro Herrera

I didn't like this bit too much:

 diff --git a/contrib/pg_xlogdump/tables.c b/contrib/pg_xlogdump/tables.c
 new file mode 100644
 index 000..e947e0d
 --- /dev/null
 +++ b/contrib/pg_xlogdump/tables.c
 @@ -0,0 +1,78 @@

 +/*
 + * RmgrTable linked only to functions available outside of the backend.
 + *
 + * needs to be synced with src/backend/access/transam/rmgr.c
 + */
 +const RmgrData RmgrTable[RM_MAX_ID + 1] = {
 + {XLOG, NULL, xlog_desc, NULL, NULL, NULL},
 + {Transaction, NULL, xact_desc, NULL, NULL, NULL},

So I propose the following patch instead.  This lets pg_xlogdump compile
only the file it needs, and not concern itself with the rest of rmgr.c.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services
commit 6779c7ff81323699e82252a6e5b92a97319cb924
Author: Alvaro Herrera alvhe...@alvh.no-ip.org
Date:   Mon Feb 4 13:12:54 2013 -0300

Split out rm_desc pointers into their own table

This allows the split file to be compiled by frontend code

diff --git a/src/backend/access/transam/Makefile b/src/backend/access/transam/Makefile
index eb6cfc5..3f1a9e4 100644
--- a/src/backend/access/transam/Makefile
+++ b/src/backend/access/transam/Makefile
@@ -12,9 +12,9 @@ subdir = src/backend/access/transam
 top_builddir = ../../../..
 include $(top_builddir)/src/Makefile.global
 
-OBJS = clog.o transam.o varsup.o xact.o rmgr.o slru.o subtrans.o multixact.o \
-	timeline.o twophase.o twophase_rmgr.o xlog.o xlogarchive.o xlogfuncs.o \
-	xlogreader.o xlogutils.o
+OBJS = clog.o multixact.o rmgrdesc.o rmgr.o slru.o subtrans.o timeline.o \
+	transam.o twophase.o twophase_rmgr.o varsup.o xact.o xlogarchive.o \
+	xlogfuncs.o xlog.o xlogreader.o xlogutils.o
 
 include $(top_srcdir)/src/backend/common.mk
 
diff --git a/src/backend/access/transam/rmgr.c b/src/backend/access/transam/rmgr.c
index cc210a7..0c0a2d1 100644
--- a/src/backend/access/transam/rmgr.c
+++ b/src/backend/access/transam/rmgr.c
@@ -24,23 +24,24 @@
 #include storage/standby.h
 #include utils/relmapper.h
 
+/* See rmgrdesc.c, too */
 
 const RmgrData RmgrTable[RM_MAX_ID + 1] = {
-	{XLOG, xlog_redo, xlog_desc, NULL, NULL, NULL},
-	{Transaction, xact_redo, xact_desc, NULL, NULL, NULL},
-	{Storage, smgr_redo, smgr_desc, NULL, NULL, NULL},
-	{CLOG, clog_redo, clog_desc, NULL, NULL, NULL},
-	{Database, dbase_redo, dbase_desc, NULL, NULL, NULL},
-	{Tablespace, tblspc_redo, tblspc_desc, NULL, NULL, NULL},
-	{MultiXact, multixact_redo, multixact_desc, NULL, NULL, NULL},
-	{RelMap, relmap_redo, relmap_desc, NULL, NULL, NULL},
-	{Standby, standby_redo, standby_desc, NULL, NULL, NULL},
-	{Heap2, heap2_redo, heap2_desc, NULL, NULL, NULL},
-	{Heap, heap_redo, heap_desc, NULL, NULL, NULL},
-	{Btree, btree_redo, btree_desc, btree_xlog_startup, btree_xlog_cleanup, btree_safe_restartpoint},
-	{Hash, hash_redo, hash_desc, NULL, NULL, NULL},
-	{Gin, gin_redo, gin_desc, gin_xlog_startup, gin_xlog_cleanup, gin_safe_restartpoint},
-	{Gist, gist_redo, gist_desc, gist_xlog_startup, gist_xlog_cleanup, NULL},
-	{Sequence, seq_redo, seq_desc, NULL, NULL, NULL},
-	{SPGist, spg_redo, spg_desc, spg_xlog_startup, spg_xlog_cleanup, NULL}
+	{XLOG, xlog_redo, NULL, NULL, NULL},
+	{Transaction, xact_redo, NULL, NULL, NULL},
+	{Storage, smgr_redo, NULL, NULL, NULL},
+	{CLOG, clog_redo, NULL, NULL, NULL},
+	{Database, dbase_redo, NULL, NULL, NULL},
+	{Tablespace, tblspc_redo, NULL, NULL, NULL},
+	{MultiXact, multixact_redo, NULL, NULL, NULL},
+	{RelMap, relmap_redo, NULL, NULL, NULL},
+	{Standby, standby_redo, NULL, NULL, NULL},
+	{Heap2, heap2_redo, NULL, NULL, NULL},
+	{Heap, heap_redo, NULL, NULL, NULL},
+	{Btree, btree_redo, btree_xlog_startup, btree_xlog_cleanup, btree_safe_restartpoint},
+	{Hash, hash_redo, NULL, NULL, NULL},
+	{Gin, gin_redo, gin_xlog_startup, gin_xlog_cleanup, gin_safe_restartpoint},
+	{Gist, gist_redo, gist_xlog_startup, gist_xlog_cleanup, NULL},
+	{Sequence, seq_redo, NULL, NULL, NULL},
+	{SPGist, spg_redo, spg_xlog_startup, spg_xlog_cleanup, NULL}
 };
diff --git a/src/backend/access/transam/rmgrdesc.c b/src/backend/access/transam/rmgrdesc.c
new file mode 100644
index 000..20315c4
--- /dev/null
+++ b/src/backend/access/transam/rmgrdesc.c
@@ -0,0 +1,47 @@
+/*
+ * rmgrdesc.c
+ *
+ * Resource managers descriptor function definitions
+ *
+ * src/backend/access/transam/rmgrdesc.c
+ */
+#include postgres.h
+
+#include access/clog.h
+#include access/gin.h
+#include access/gist_private.h
+#include access/hash.h
+#include access/heapam_xlog.h
+#include access/multixact.h
+#include access/nbtree.h
+#include access/spgist.h
+#include access/xact.h
+#include access/xlog_internal.h
+#include catalog/storage_xlog.h
+#include commands/dbcommands.h
+#include commands/sequence.h
+#include commands/tablespace.h
+#include storage/standby.h
+#include utils/relmapper.h
+
+/* See rmgr.c, too */
+
+const RmgrDescData RmgrDescTable[RM_MAX_ID + 1] = {
+	{ xlog_desc },
+	{ xact_desc },
+	{ smgr_desc },

Re: [HACKERS] [PATCH 4/5] Add pg_xlogdump contrib module

2013-02-04 Thread Andres Freund
On 2013-02-04 13:22:26 -0300, Alvaro Herrera wrote:
 
 I didn't like this bit too much:
 
  diff --git a/contrib/pg_xlogdump/tables.c b/contrib/pg_xlogdump/tables.c
  new file mode 100644
  index 000..e947e0d
  --- /dev/null
  +++ b/contrib/pg_xlogdump/tables.c
  @@ -0,0 +1,78 @@
 
  +/*
  + * RmgrTable linked only to functions available outside of the backend.
  + *
  + * needs to be synced with src/backend/access/transam/rmgr.c
  + */
  +const RmgrData RmgrTable[RM_MAX_ID + 1] = {
  +   {XLOG, NULL, xlog_desc, NULL, NULL, NULL},
  +   {Transaction, NULL, xact_desc, NULL, NULL, NULL},
 
 So I propose the following patch instead.  This lets pg_xlogdump compile
 only the file it needs, and not concern itself with the rest of rmgr.c.

Hm. Not sure whats the advantage of that, duplicates about as much and
makes it harder to copy  paste between both files.
I am fine with it, I just don't see a clear advantage of either way.

 diff --git a/src/include/access/xlog_internal.h 
 b/src/include/access/xlog_internal.h
 index ce9957e..b751882 100644
 --- a/src/include/access/xlog_internal.h
 +++ b/src/include/access/xlog_internal.h
 @@ -231,15 +231,17 @@ typedef struct xl_end_of_recovery
  struct XLogRecord;
  
  /*
 - * Method table for resource managers.
 + * Method tables for resource managers.
   *
 - * RmgrTable[] is indexed by RmgrId values (see rmgr.h).
 + * RmgrDescData (for textual descriptor functions) is split so that the file 
 it
 + * lives in can be used by frontend programs.
 + *
 + * RmgrTable[] and RmgrDescTable[] are indexed by RmgrId values (see rmgr.h).
   */
  typedef struct RmgrData
  {
   const char *rm_name;
   void(*rm_redo) (XLogRecPtr lsn, struct XLogRecord *rptr);
 - void(*rm_desc) (StringInfo buf, uint8 xl_info, char *rec);
   void(*rm_startup) (void);
   void(*rm_cleanup) (void);
   bool(*rm_safe_restartpoint) (void);
 @@ -247,6 +249,14 @@ typedef struct RmgrData
  
  extern const RmgrData RmgrTable[];
  
 +typedef struct RmgrDescData
 +{
 + void(*rm_desc) (StringInfo buf, uint8 xl_info, char *rec);
 +} RmgrDescData;
 +
 +extern const RmgrDescData RmgrDescTable[];
 +

I think we would at least need to include rm_name here as well.

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] GetOldestXmin going backwards is dangerous after all

2013-02-04 Thread Simon Riggs
On 4 February 2013 16:07, Tom Lane t...@sss.pgh.pa.us wrote:

 Simon, would you revert the vacuum_defer_cleanup_age changes?

Will do

-- 
 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] GetOldestXmin going backwards is dangerous after all

2013-02-04 Thread Andres Freund
On 2013-02-04 11:07:17 -0500, Tom Lane wrote:
 Andres Freund and...@2ndquadrant.com writes:
  On 2013-02-01 19:24:02 -0500, Tom Lane wrote:
  And as for that, it's been pretty clear for awhile that allowing
  vacuum_defer_cleanup_age to change on the fly was a bad idea we'd
  eventually have to undo.  The day of reckoning has arrived: it needs
  to be PGC_POSTMASTER.
 
  ISTM that the original problem can still occur, even after Simon's
  commit.
  1) start with -c vacuum_defer_cleanup_age=0
  2) autovacuum vacuums test;
  3) restart with -c vacuum_defer_cleanup_age=1
  4) autovacuum vacuums test's toast table;
 
  should result in about the same ERROR, shouldn't it?
 
 Hm ... yeah, you're right.  So that's still not bulletproof.

ISTM that, unless we can guarantee that this won't cause GetOldestXmin
to backwards, there is no point in trying to make GetOldestXmin any
guarantees not to do so. Especially if that comes with a loss of
VACUUM's effectiveness.

I absolutely hate to suggest it, but what about storing the last
vacuum's xmin horizon in the main table's pg_class.options in the back
branches?
During heap_copy_data the assumed xmin horizon can be
Max(OldestXmin, main_table_last_vacuum_horizon, toast_table_last_vacuum_horizon)

That should solve the problem, right?

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] GetOldestXmin going backwards is dangerous after all

2013-02-04 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes:
 I absolutely hate to suggest it, but what about storing the last
 vacuum's xmin horizon in the main table's pg_class.options in the back
 branches?

Not workable.  This would require a non-in-place update of the table's
pg_class row (at least in cases where the option wasn't stored already).
Which would require VACUUM to have an XID, which would make a whole lot
of assumptions fall over.

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] sql_drop Event Trigger

2013-02-04 Thread Dimitri Fontaine
Robert Haas robertmh...@gmail.com writes:
 Taking a first look at this, I think the idea of pg_dropped_objects()
 is really pretty clever.  I like it.  I assure we will end up with
 several functions of this type eventually, so it might be good to
 adopt some kind of distinguishing naming convention for this type of
 function.  pg_event_trigger_context_dropped_objects() seems far too
 verbose, but that's the idea.

Thanks. Agreed that we will have more of them. In the attached version 3
of the patch, it got renamed to pg_event_trigger_dropped_objects().

 With this approach, there's no real need to introduce a new event
 type.  We could just make ddl_command_end triggers able to use this,
 and we're done.  The point of sql_drop was that it would have been
 called once *per dropped object*, not once per command.  But,

Well, from the beginning of the sql_drop discussion, it's been clear
that it's meant to allow for users to easily attach their function to
any drop that might appear, whatever the command at origin of that drop.

So in the attached, I've made the sql_drop an event triggers called once
per object, which means that currently you only know an object is
getting DROPed as part of a DROP TABLE command.

 actually, thinking about this, for something like Slony, exposing
 pg_dropped_objects() to ddl_command_end triggers should be just as
 good, and maybe a whole lot better, than what I was proposing.

It also changes the protocol to use for getting at the information
related to the objects. I think we will have to have the out parameters
of the function to grow to include the next information we're going to
make available to TG_* variables in the next patch of the series.

 Does it work for you to rip out sql_drop and just make
 pg_dropped_objects() - perhaps renamed - available in ddl_command_end?

I did rename pg_dropped_objects() and it's now available in
ddl_command_end, but I kept the new sql_drop event, which is now
called once per object dropped, as intended (but without any useful
information yet).

What do you think?
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support



sql_drop.2.patch.gz
Description: Binary data

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


Re: [HACKERS] GetOldestXmin going backwards is dangerous after all

2013-02-04 Thread Andres Freund
On 2013-02-04 11:52:05 -0500, Tom Lane wrote:
 Andres Freund and...@2ndquadrant.com writes:
  I absolutely hate to suggest it, but what about storing the last
  vacuum's xmin horizon in the main table's pg_class.options in the back
  branches?
 
 Not workable.  This would require a non-in-place update of the table's
 pg_class row (at least in cases where the option wasn't stored already).
 Which would require VACUUM to have an XID, which would make a whole lot
 of assumptions fall over.

Don't get me wrong, I hate that solution, but that specific issue seems
relatively easily solvable by pre-creating the option intially in
vacuum_rel() in a separate transaction before entering
lazy_vacuum_rel().

I unfortunately don't yet see a robust way without storing the last used
horizon :(.

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] GetOldestXmin going backwards is dangerous after all

2013-02-04 Thread Simon Riggs
On 4 February 2013 17:02, Andres Freund and...@2ndquadrant.com wrote:

 I unfortunately don't yet see a robust way without storing the last used
 horizon :(.

We can't go backwards, but we can go forwards.

We can move the next xid forwards by an amount equal to the increase
in vacuum_defer_cleanup_age.

-- 
 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] GetOldestXmin going backwards is dangerous after all

2013-02-04 Thread Andres Freund
On 2013-02-04 17:21:50 +, Simon Riggs wrote:
 On 4 February 2013 17:02, Andres Freund and...@2ndquadrant.com wrote:
 
  I unfortunately don't yet see a robust way without storing the last used
  horizon :(.
 
 We can't go backwards, but we can go forwards.
 
 We can move the next xid forwards by an amount equal to the increase
 in vacuum_defer_cleanup_age.

Don't think that helps, the problem is not new writes but already
removed rows in the toast table.

Besides, advancing the next xid by vacuum_defer_cleanup_age every
restart could get expensive. Unless we find a space to store the old
value which we haven't really got in the back branches (control file?)...

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] proposal: ANSI SQL 2011 syntax for named parameters

2013-02-04 Thread Gavin Flower

On 04/02/13 21:55, Pavel Stehule wrote:

2013/1/2 Robert Haas robertmh...@gmail.com:

On Fri, Dec 28, 2012 at 11:22 AM, Pavel Stehule pavel.steh...@gmail.com wrote:

I am not sure, but maybe is time to introduce ANSI SQL syntax for
functions' named parameters

It is defined in ANSI SQL 2011

  CALL P (B = 1, A = 2)

instead PostgreSQL syntax CALL ( B := 1, A := 2)

Keep in mind that, as recently as PostgreSQL 9.1, we shipped hstore
with a =(text, text) operator.  That operator was deprecated in 9.0,
but it wasn't actually removed until PostgreSQL 9.2.  Whenever we do
this, it's going to break things for anyone who hasn't yet upgraded
from hstore v1.0 to hstore v1.1.  So I would prefer to wait one more
release.  That way, anyone who does an upgrade, say, every other major
release cycle should have a reasonably clean upgrade path.

I realize that the 4+-year journey toward allowing = rather than :=
probably seems tedious to many people by now, but I think the cautious
path we've taken is entirely warranted.  As much as I want us to be
standards-compliant in this area, I also want us to not break any more
user applications than necessary along the way.

Incidentally, I think there are two changes here which should be
considered independently.  One, allowing = rather than := for
specifying named parameters.  And two, adding a statement called CALL
that can be used to invoke a function.  Maybe those are both good
ideas and maybe they aren't, but they're independent.


can I recapitulate a plan?

* enabling '=' in 9.4
* we will support ':=' too

What we can (or have to) do now?

Regards

Pavel




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



I prefer ':=', as I like the ALGOL justification of it.

But I won't even threaten to hold my breath if I'm not allowed to use 
':='!  :-)



Cheers,
Gavin


Re: [HACKERS] sepgsql and materialized views

2013-02-04 Thread Kohei KaiGai
2013/2/4 Kevin Grittner kgri...@ymail.com:
 Kohei KaiGai kai...@kaigai.gr.jp wrote:
 2013/2/3 Kevin Grittner kgri...@ymail.com:
 I'm hoping that someone familiar with sepgsql can review this
 portion of the materialized view patch and comment on whether it is
 the best approach for dealing with the integration of these two
 features.  Basically, the patch as it stands treats a materialized
 view as a table for purposes of sepgsql labels.  I initially
 invented new lables, but Robert suggested that this would make
 materialized views unusable in an SE environment until the
 corresponding labels were added at the OS level.  It seems sane to
 me because a materialized view exists on disk the same as a table,
 but is populated differently -- from a view-like rule.

 The portion of the patch which affects the contrib/sepgsql/ tree is
 attached.

 Hi Kevin,

 Sorry, I have not been involved this discussion.
 I briefly checked your patch. Indeed, it performs like a table, even though
 it is named materialized-view.

 Probably, we have two standpoints.

 First, materialized-view shall have a security label corresponding to table,
 and related checks handle references to materialized-views as if user
 references regular-tables. This is an idea.
 I briefly checked your latest patch. ExecRefreshMatView is a unique entry
 point to update a particular materialized-view, and REFRESH MATERIALIZED
 VIEW command is only way to kick this function. It also checks permissions to
 reference underlying tables. So, it means update of materialized-view is a 
 stuff
 like writing a table with contents read from other tables by a particular 
 users.

 However, I'm worried whether this design continues forever, or not.
 IIRC, you have a plan to refresh materialized-view asynchronously using
 background worker stuff, don't you?

 The goal is to build on this to support other timings of updates to
 the materialized view.  In general, I think this will take the form
 of identifying, for the given rewrite rules associated with the
 materialized view, what changes to the underlying data can affect
 the view and determining whether incremental updates can be
 supported for the MV.  If incremental update is possible, then
 various timings can be chosen for applying them.  I think that all
 timing should go through a queue of change requests, although that
 is not yet designed, and I'm just waving my hands around and
 speculating about any details.  Timings likely to be supported
 range from on-demand incremental updates (of all accumlated
 changes) to applying the changes from a transaction at COMMIT time,
 or possibly as the underlying changes are made within a
 transaction.  I suspect that the most popular timing will involve a
 background process working from the queue asynchronously to keep
 the MV updated asynchronously but without any artificial delay.

 In all cases, a query against the view does not reach below the MV
 table to the underlying tables or functions used to build the data
 -- it will always read the materialized data, so Im not sure
 that the normal concerns about leaky views apply here.

Let me confirm a significant point. Do you never plan a feature that
allows to update/refresh materialized-views out of user session?
I had an impression on asynchronous update of MV something like
a feature that moves data from regular tables to MV with batch-jobs
in mid-night, but under the privilege that bypass regular permission
checks.
It it is never planned, my concern was pointless.

 Once we support an internal stuff (thus,
 it can bypass valid security checks) to write out confidential contents into
 unconfidential zone, it does not make sense to keep data confidentiality.

 If the person who creates the materialized view has authority to
 query the underlying tables, and grants access to the materialized
 view as desired, and those selecting from the materialized view
 only see the contents of the table which is being populated by the
 underlying pg_rewrite rule, I'm not understanding your concern.
 Can you elaborate?

My concern is future development that allows to update/refresh MV
asynchronously, out of privilege control.
As long as all the update/refresh operation is under privilege control
with user-id/security label of the current session, here is no difference
from regular writer operation of tables with contents read from other
tables.
Can I explain where is my concern about well?


BTW, please clarify expected behavior in case when MV contains
WHERE clause that returns different result depending on privilege
of current session, such as:
  ... WHERE underlying_table.uname = CURRENT_USER

It seems to me this MV saves just a snapshot from a standpoint of
a particular user who refreshed this MV; typically its owner.
If bob has privilege to reference this MV, he will see rows to be visible
for alice. Of course, it does not contradictory, because all alice doing
is just writing data she can see into a table being visible 

Re: [HACKERS] json api WIP patch

2013-02-04 Thread David E. Wheeler
On Feb 4, 2013, at 8:10 AM, Andrew Dunstan and...@dunslane.net wrote:

 My suggestion would be ~ and ~. I know David Wheeler didn't like that on 
 the ground that some fonts elevate ~ rather than aligning it in the middle as 
 most monospaced fonts do, but I'm tempted just to say then use a different 
 font. Other possibilities that come to mind are + and +, although I think 
 they're less attractive. But I'll be guided by the consensus, assuming there 
 is one ;-)

On the contrary, I quite like ~. I've used it in pair.

  http://pgxn.org/extension/pair

But others have complained about the font issue when I've suggested it for 
things in the past.

My fonts don't suck. :-)

I can live with + and +.

David



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


Re: [HACKERS] proposal: ANSI SQL 2011 syntax for named parameters

2013-02-04 Thread Pavel Stehule
2013/2/4 Gavin Flower gavinflo...@archidevsys.co.nz:
 On 04/02/13 21:55, Pavel Stehule wrote:

 2013/1/2 Robert Haas robertmh...@gmail.com:

 On Fri, Dec 28, 2012 at 11:22 AM, Pavel Stehule pavel.steh...@gmail.com
 wrote:

 I am not sure, but maybe is time to introduce ANSI SQL syntax for
 functions' named parameters

 It is defined in ANSI SQL 2011

  CALL P (B = 1, A = 2)

 instead PostgreSQL syntax CALL ( B := 1, A := 2)

 Keep in mind that, as recently as PostgreSQL 9.1, we shipped hstore
 with a =(text, text) operator.  That operator was deprecated in 9.0,
 but it wasn't actually removed until PostgreSQL 9.2.  Whenever we do
 this, it's going to break things for anyone who hasn't yet upgraded
 from hstore v1.0 to hstore v1.1.  So I would prefer to wait one more
 release.  That way, anyone who does an upgrade, say, every other major
 release cycle should have a reasonably clean upgrade path.

 I realize that the 4+-year journey toward allowing = rather than :=
 probably seems tedious to many people by now, but I think the cautious
 path we've taken is entirely warranted.  As much as I want us to be
 standards-compliant in this area, I also want us to not break any more
 user applications than necessary along the way.

 Incidentally, I think there are two changes here which should be
 considered independently.  One, allowing = rather than := for
 specifying named parameters.  And two, adding a statement called CALL
 that can be used to invoke a function.  Maybe those are both good
 ideas and maybe they aren't, but they're independent.

 can I recapitulate a plan?

 * enabling '=' in 9.4
 * we will support ':=' too

 What we can (or have to) do now?

 Regards

 Pavel



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

 I prefer ':=', as I like the ALGOL justification of it.

:= is not in ANSI SQL, so we are sure about '='  (ADA wins :))

':=' can be supported as secondary form (and I don't plan remove it)

A timing is question now.

Regards

Pavel


 But I won't even threaten to hold my breath if I'm not allowed to use ':='!
 :-)


 Cheers,
 Gavin


-- 
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 4/5] Add pg_xlogdump contrib module

2013-02-04 Thread Alvaro Herrera
Andres Freund wrote:
 On 2013-02-04 13:22:26 -0300, Alvaro Herrera wrote:

  +typedef struct RmgrDescData
  +{
  +   void(*rm_desc) (StringInfo buf, uint8 xl_info, char *rec);
  +} RmgrDescData;
  +
  +extern const RmgrDescData RmgrDescTable[];
  +
 
 I think we would at least need to include rm_name here as well.

Not rm_name (that would be duplicative), but a comment with the name of
each rmgr's name in its line should be sufficient.

-- 
Á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] json api WIP patch

2013-02-04 Thread Merlin Moncure
On Mon, Feb 4, 2013 at 10:18 AM, Benedikt Grundmann
bgrundm...@janestreet.com wrote:



 On Mon, Feb 4, 2013 at 4:10 PM, Andrew Dunstan and...@dunslane.net wrote:


 On 02/04/2013 10:47 AM, Robert Haas wrote:


 The SQL standards considerations seem worth thinking about, too.
 We've certainly gone through a lot of pain working toward eliminating
 = as an operator name, and if the SQL standard has commandeered -
 for some purpose or other, I'd really rather not add to the headaches
 involved should we ever decide to reclaim it.



 OK, but I'd like to know what is going to be safe. There's no way to
 future-proof the language. I'm quite prepared to replace - with something
 else, and if I do then - will need to be adjusted accordingly, I think.

 My suggestion would be ~ and ~. I know David Wheeler didn't like that
 on the ground that some fonts elevate ~ rather than aligning it in the
 middle as most monospaced fonts do, but I'm tempted just to say then use a
 different font. Other possibilities that come to mind are + and +,
 although I think they're less attractive. But I'll be guided by the
 consensus, assuming there is one ;-)

 As a user I would be much in favor of just functions and no additional
 operators if the sole difference is syntactical.  I think custom operators
 are much harder to remember than function names (assuming reasonably well
 chosen function names).

couple quick observations:

*) just about all postgres extension types expose operators -- problem
is not specific to json (and therefore IMO irrelevant to 9.3 release
of enhancements)

*) hstore exposes -.  I use it all over the place.  I find operator
to be terse and readable -- much more so than function definition.
Ok, operator such as @-@ is pretty silly, but - for get is
natural.  The cat is out of the bag, so removing - for 9.3 for
production seems pretty fruitless.

*) Removing - (breaking all my and countless others' hstore dependent
code) should not happen until there is a very good reason.  This was
extensively discussed in development of hstore.   Breaking
compatibility sucks -- my company is just wrapping up a 12 month code
overhaul so we could get off 8.1.

*) it's bad enough that we drift from sql naming conventions and all
type manipulation functions (except in hstore) with type name.
json_get etc.   at least using operators allow avoiding some of that
unnecessary verbosity.  what's next: text_concatenate('foo', 'bar')?

merlin


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


Re: [HACKERS] [PATCH 4/5] Add pg_xlogdump contrib module

2013-02-04 Thread Andres Freund
On 2013-02-04 14:55:56 -0300, Alvaro Herrera wrote:
 Andres Freund wrote:
  On 2013-02-04 13:22:26 -0300, Alvaro Herrera wrote:
 
   +typedef struct RmgrDescData
   +{
   + void(*rm_desc) (StringInfo buf, uint8 xl_info, char *rec);
   +} RmgrDescData;
   +
   +extern const RmgrDescData RmgrDescTable[];
   +
  
  I think we would at least need to include rm_name here as well.
 
 Not rm_name (that would be duplicative), but a comment with the name of
 each rmgr's name in its line should be sufficient.

Maybe I am misunderstanding what you want to do, but xlogdump prints the
name of the rmgr and allows to filter by it, how would we do that
without rm_name?

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] [PATCH 4/5] Add pg_xlogdump contrib module

2013-02-04 Thread Alvaro Herrera
Andres Freund wrote:
 On 2013-02-04 14:55:56 -0300, Alvaro Herrera wrote:
  Andres Freund wrote:
   On 2013-02-04 13:22:26 -0300, Alvaro Herrera wrote:
  
+typedef struct RmgrDescData
+{
+   void(*rm_desc) (StringInfo buf, uint8 xl_info, char 
*rec);
+} RmgrDescData;
+
+extern const RmgrDescData RmgrDescTable[];
+
   
   I think we would at least need to include rm_name here as well.
  
  Not rm_name (that would be duplicative), but a comment with the name of
  each rmgr's name in its line should be sufficient.
 
 Maybe I am misunderstanding what you want to do, but xlogdump prints the
 name of the rmgr and allows to filter by it, how would we do that
 without rm_name?

Oh, I thought you wanted it for code documentation purposes only.  Okay,
that makes sense.  (Of course, we'd remove it from RmgrTable).

-- 
Á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] proposal: ANSI SQL 2011 syntax for named parameters

2013-02-04 Thread Simon Riggs
On 2 January 2013 22:51, Robert Haas robertmh...@gmail.com wrote:
 On Fri, Dec 28, 2012 at 11:22 AM, Pavel Stehule pavel.steh...@gmail.com 
 wrote:
 I am not sure, but maybe is time to introduce ANSI SQL syntax for
 functions' named parameters

 It is defined in ANSI SQL 2011

  CALL P (B = 1, A = 2)

 instead PostgreSQL syntax CALL ( B := 1, A := 2)

 Keep in mind that, as recently as PostgreSQL 9.1, we shipped hstore
 with a =(text, text) operator.  That operator was deprecated in 9.0,
 but it wasn't actually removed until PostgreSQL 9.2.  Whenever we do
 this, it's going to break things for anyone who hasn't yet upgraded
 from hstore v1.0 to hstore v1.1.  So I would prefer to wait one more
 release.  That way, anyone who does an upgrade, say, every other major
 release cycle should have a reasonably clean upgrade path.

I don't see why waiting 1 year makes this situation any better. We
just make upgrading to hstore 1.1 a prerequisite and we're done.

I doubt there are many people using hstore who haven't upgraded, and
fewer still that will upgrade yet can't follow simple instructions on
prerequisites. While hstore is reasonably popular, users are still in
the minority.

You can always override the operators using a different search_path if
you still see problems there.

We need to find ways forwards rather than block progress because of
obscure issues.

-- 
 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] json api WIP patch

2013-02-04 Thread Andrew Dunstan


On 02/04/2013 12:57 PM, Merlin Moncure wrote:


*) it's bad enough that we drift from sql naming conventions and all
type manipulation functions (except in hstore) with type name.
json_get etc.   at least using operators allow avoiding some of that
unnecessary verbosity.  what's next: text_concatenate('foo', 'bar')?



This names aren't set in stone either. I've been expecting some 
bikeshedding there, and I'm surprised there hasn't been more.


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


[HACKERS] USE_PGXS contrib build is broken

2013-02-04 Thread Alvaro Herrera
I just noticed that contrib programs don't build in USE_PGXS=1
environment:

$ pwd
/pgsql/build/HEAD/contrib/pg_upgrade
$ LC_ALL=C USE_PGXS=1 make 
make: *** No rule to make target `check.o', needed by `pg_upgrade'.  Stop.
$ LC_ALL=C make 
 [works]

Is this expected?

-- 
Á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] json api WIP patch

2013-02-04 Thread Robert Haas
On Mon, Feb 4, 2013 at 11:10 AM, Andrew Dunstan and...@dunslane.net wrote:
 On 02/04/2013 10:47 AM, Robert Haas wrote:


 The SQL standards considerations seem worth thinking about, too.
 We've certainly gone through a lot of pain working toward eliminating
 = as an operator name, and if the SQL standard has commandeered -
 for some purpose or other, I'd really rather not add to the headaches
 involved should we ever decide to reclaim it.

 OK, but I'd like to know what is going to be safe. There's no way to
 future-proof the language. I'm quite prepared to replace - with something
 else, and if I do then - will need to be adjusted accordingly, I think.

 My suggestion would be ~ and ~. I know David Wheeler didn't like that on
 the ground that some fonts elevate ~ rather than aligning it in the middle
 as most monospaced fonts do, but I'm tempted just to say then use a
 different font. Other possibilities that come to mind are + and +,
 although I think they're less attractive. But I'll be guided by the
 consensus, assuming there is one ;-)

I suspect both of those are pretty safe from an SQL standards point of
view.  Of course, as Tom is often wont to point out, the SQL standards
committee sometimes does bizarre things, so nothing's perfect, but I'd
be rather shocked if any of those got tapped to mean something else.

That having been said, I still don't see value in adding operators at
all.  Good old function call notation seems perfectly adequate from
where I sit.  Sure, it's a little more verbose, but when you try to
too hard make things concise then you end up having to explain to your
users why \ditS is a sensible thing for them to type into psql, or why
s@\W@sprintf%%%02x,ord($)@e in Perl.  I recognize that I may lose
this argument, but I've worked with a couple of languages where
operators can be overloaded (C++) or defined (ML) and it's just never
seemed to work out very well.  YMMV, of course.

-- 
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] proposal: ANSI SQL 2011 syntax for named parameters

2013-02-04 Thread Robert Haas
On Mon, Feb 4, 2013 at 1:06 PM, Simon Riggs si...@2ndquadrant.com wrote:
 On 2 January 2013 22:51, Robert Haas robertmh...@gmail.com wrote:
 On Fri, Dec 28, 2012 at 11:22 AM, Pavel Stehule pavel.steh...@gmail.com 
 wrote:
 I am not sure, but maybe is time to introduce ANSI SQL syntax for
 functions' named parameters

 It is defined in ANSI SQL 2011

  CALL P (B = 1, A = 2)

 instead PostgreSQL syntax CALL ( B := 1, A := 2)

 Keep in mind that, as recently as PostgreSQL 9.1, we shipped hstore
 with a =(text, text) operator.  That operator was deprecated in 9.0,
 but it wasn't actually removed until PostgreSQL 9.2.  Whenever we do
 this, it's going to break things for anyone who hasn't yet upgraded
 from hstore v1.0 to hstore v1.1.  So I would prefer to wait one more
 release.  That way, anyone who does an upgrade, say, every other major
 release cycle should have a reasonably clean upgrade path.

 I don't see why waiting 1 year makes this situation any better. We
 just make upgrading to hstore 1.1 a prerequisite and we're done.

 I doubt there are many people using hstore who haven't upgraded, and
 fewer still that will upgrade yet can't follow simple instructions on
 prerequisites. While hstore is reasonably popular, users are still in
 the minority.

 You can always override the operators using a different search_path if
 you still see problems there.

 We need to find ways forwards rather than block progress because of
 obscure issues.

This seems pretty close to an accusation of bad faith, which I don't
believe to be present.  Right now there is one and only one release in
the field that contains hstore 1.1.  If we go ahead and prohibit = as
an operator name now, we're going to require everyone who is on 9.1
and uses hstore and wants to get to 9.3 to either (a) first upgrade to
9.2, then update hstore, then upgrade to 9.3; or (b) dig the
hstore-1.1 update out of a future release, apply it to an earlier
release on which it did not ship, and then upgrade.  If they're
actually *using* the = operator (rather than just having it in the
DB), they'll also need to rewrite their application before doing any
of that.

Perhaps the users that you support won't mind that, but the users I
support will.  In fact, they're likely going to mind it even if we
have two releases where either version of hstore will work, but at
least it will ameliorate the problem somewhat.

I find your attitude toward backward compatibility to be astonishingly
inconsistent.  We haven't made any progress on overhauling
recovery.conf in two years because you've steadfastly stonewalled any
forward progress on backwards-compatibility grounds, even though a
change there can't possible break any working PostgreSQL
*application*, only the admin tools.  But now, on this change and on a
few others, which actually will break applications, you want to push
it forward faster.  That seems completely backwards to me.  In my
experience, it is far easier to get people to adjust their admin
scripts (which are usually controlled by the same team responsible for
the database upgrade anyway) than to get them to fix their
applications (which are usually written by a different team over which
the DBAs have no real control).

We've had customers postpone upgrades for *years* because of issues
like this.  Labeling it as obscure suggests that it is unimportant or
should be dismissed, a conclusion with which I respectfully disagree.

-- 
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] json api WIP patch

2013-02-04 Thread Merlin Moncure
On Mon, Feb 4, 2013 at 12:07 PM, Andrew Dunstan and...@dunslane.net wrote:

 On 02/04/2013 12:57 PM, Merlin Moncure wrote:

 *) it's bad enough that we drift from sql naming conventions and all
 type manipulation functions (except in hstore) with type name.
 json_get etc.   at least using operators allow avoiding some of that
 unnecessary verbosity.  what's next: text_concatenate('foo', 'bar')?


 This names aren't set in stone either. I've been expecting some bikeshedding
 there, and I'm surprised there hasn't been more.

Well -- heh (asked to bikeshed: joy!) -- I felt like my objections
were noted and am more interested in getting said functionality out
the door than splitting hairs over names.  Type prefix issue is under
the same umbrella as use of the - operator, that is, *not
specifically related to this patch, and not worth holding up this
patch over*.  In both cases it's essentially crying over spilt milk.

My only remaining nit with the proposal is with json_unnest().

SQL unnest() produces list of scalars regardless of dimensionality --
json unnest unwraps one level only (contrast: pl/pgsql array 'slice').
 So I think 'json_array_each', or perhaps json_slice() is a better fit
there.

merlin


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


Re: [HACKERS] json api WIP patch

2013-02-04 Thread Will Leinweber
On Mon, Feb 4, 2013 at 11:38 AM, Robert Haas robertmh...@gmail.com wrote:

 I suspect both of those are pretty safe from an SQL standards point of
 view.  Of course, as Tom is often wont to point out, the SQL standards
 committee sometimes does bizarre things, so nothing's perfect, but I'd
 be rather shocked if any of those got tapped to mean something else.

 That having been said, I still don't see value in adding operators at
 all.  Good old function call notation seems perfectly adequate from
 where I sit.  Sure, it's a little more verbose, but when you try to
 too hard make things concise then you end up having to explain to your
 users why \ditS is a sensible thing for them to type into psql, or why
 s@\W@sprintf%%%02x,ord($)@e in Perl.  I recognize that I may lose
 this argument, but I've worked with a couple of languages where
 operators can be overloaded (C++) or defined (ML) and it's just never
 seemed to work out very well.  YMMV, of course.



For what my opinion is worth I absolute agree with just having function
names. The - in hstore is kind of nice, but it lead me to a whole lot of
greif when I couldn't figure out how to create an index using it (turns out
you have to use _double_ parens, who knew?), but could create an index on
fetchval and assumed that postgres would figure it out.

Also a for quite a while it felt just like incantation of when I'd need
parens around those operatiors or not. Now that I sorta-kinda-not-really
understand the operation precedence rules in postgres/sql standard, I've
mostly given up on using cute operators because their much more of a pain
on a day-to-day basis.


Re: [HACKERS] json api WIP patch

2013-02-04 Thread Andrew Dunstan


On 02/04/2013 02:59 PM, Merlin Moncure wrote:

On Mon, Feb 4, 2013 at 12:07 PM, Andrew Dunstan and...@dunslane.net wrote:

On 02/04/2013 12:57 PM, Merlin Moncure wrote:


*) it's bad enough that we drift from sql naming conventions and all
type manipulation functions (except in hstore) with type name.
json_get etc.   at least using operators allow avoiding some of that
unnecessary verbosity.  what's next: text_concatenate('foo', 'bar')?


This names aren't set in stone either. I've been expecting some bikeshedding
there, and I'm surprised there hasn't been more.

Well -- heh (asked to bikeshed: joy!) -- I felt like my objections
were noted and am more interested in getting said functionality out
the door than splitting hairs over names.  Type prefix issue is under
the same umbrella as use of the - operator, that is, *not
specifically related to this patch, and not worth holding up this
patch over*.  In both cases it's essentially crying over spilt milk.

My only remaining nit with the proposal is with json_unnest().

SQL unnest() produces list of scalars regardless of dimensionality --
json unnest unwraps one level only (contrast: pl/pgsql array 'slice').
  So I think 'json_array_each', or perhaps json_slice() is a better fit
there.




how about json_array_elements()?

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] json api WIP patch

2013-02-04 Thread Daniel Farina
On Mon, Feb 4, 2013 at 11:38 AM, Robert Haas robertmh...@gmail.com wrote:
 On Mon, Feb 4, 2013 at 11:10 AM, Andrew Dunstan and...@dunslane.net wrote:
 On 02/04/2013 10:47 AM, Robert Haas wrote:


 The SQL standards considerations seem worth thinking about, too.
 We've certainly gone through a lot of pain working toward eliminating
 = as an operator name, and if the SQL standard has commandeered -
 for some purpose or other, I'd really rather not add to the headaches
 involved should we ever decide to reclaim it.

 OK, but I'd like to know what is going to be safe. There's no way to
 future-proof the language. I'm quite prepared to replace - with something
 else, and if I do then - will need to be adjusted accordingly, I think.

 My suggestion would be ~ and ~. I know David Wheeler didn't like that on
 the ground that some fonts elevate ~ rather than aligning it in the middle
 as most monospaced fonts do, but I'm tempted just to say then use a
 different font. Other possibilities that come to mind are + and +,
 although I think they're less attractive. But I'll be guided by the
 consensus, assuming there is one ;-)

 I suspect both of those are pretty safe from an SQL standards point of
 view.  Of course, as Tom is often wont to point out, the SQL standards
 committee sometimes does bizarre things, so nothing's perfect, but I'd
 be rather shocked if any of those got tapped to mean something else.

 That having been said, I still don't see value in adding operators at
 all.  Good old function call notation seems perfectly adequate from
 where I sit.  Sure, it's a little more verbose, but when you try to
 too hard make things concise then you end up having to explain to your
 users why \ditS is a sensible thing for them to type into psql, or why
 s@\W@sprintf%%%02x,ord($)@e in Perl.  I recognize that I may lose
 this argument, but I've worked with a couple of languages where
 operators can be overloaded (C++) or defined (ML) and it's just never
 seemed to work out very well.  YMMV, of course.

I also basically feel this way, although I know I tend more towards
notational brutalism than many.  I think we shouldn't kid ourselves
that non-default operators will be used, and for
current-implementation reasons (that maybe could be fixed by someone
determined) it's not really at the pleasure of the author to use them
via CREATE OPERATOR either.

So, I basically subscribe to view that we should investigate what
total reliance on prefix syntax looks like.  I guess it'll make nested
navigation horribly ugly, though...positively lisp-esque.  That' s one
consideration hstore doesn't have that may make use of infix notations
considerably more useful for json than hstore.

--
fdr


-- 
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] USE_PGXS contrib build is broken

2013-02-04 Thread Gurjeet Singh
On Mon, Feb 4, 2013 at 2:38 PM, Alvaro Herrera alvhe...@2ndquadrant.comwrote:

 I just noticed that contrib programs don't build in USE_PGXS=1
 environment:

 $ pwd
 /pgsql/build/HEAD/contrib/pg_upgrade
 $ LC_ALL=C USE_PGXS=1 make
 make: *** No rule to make target `check.o', needed by `pg_upgrade'.  Stop.


FWIW, I am able to build just fine using USE_PGXS!


Re: [HACKERS] proposal: ANSI SQL 2011 syntax for named parameters

2013-02-04 Thread Simon Riggs
On 4 February 2013 19:53, Robert Haas robertmh...@gmail.com wrote:

 This seems pretty close to an accusation of bad faith, which I don't
 believe to be present.

Robert, this is not an accusation of bad faith, just an observation
that we can move forwards more quickly.

I understand completely why you wish to make slow changes, I just
don't believe it is warranted in this case. I'm not sure how or why
that becomes some kind of accusation against you personally.

In this case, and others, moving forwards quickly can be achieved at
the same time as backwards compatibility with some thought and
documentation.

 I find your attitude toward backward compatibility to be astonishingly
 inconsistent.  We haven't made any progress on overhauling
 recovery.conf in two years because you've steadfastly stonewalled any
 forward progress on backwards-compatibility grounds, even though a
 change there can't possible break any working PostgreSQL
 *application*, only the admin tools.

I stonewalled nothing; what exactly do you think I hoped to gain by
such actions? In fact, I repeatedly encouraged change and showed how
we could have both backwards compatibility and progress. Any lack of
progress is not a result of my request for backwards compatibility,
made in the interests of avoiding data loss and very broken
applications.

-- 
 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] json api WIP patch

2013-02-04 Thread Andrew Dunstan


On 02/04/2013 03:16 PM, Daniel Farina wrote:

On Mon, Feb 4, 2013 at 11:38 AM, Robert Haas robertmh...@gmail.com wrote:

On Mon, Feb 4, 2013 at 11:10 AM, Andrew Dunstan and...@dunslane.net wrote:

On 02/04/2013 10:47 AM, Robert Haas wrote:


The SQL standards considerations seem worth thinking about, too.
We've certainly gone through a lot of pain working toward eliminating
= as an operator name, and if the SQL standard has commandeered -
for some purpose or other, I'd really rather not add to the headaches
involved should we ever decide to reclaim it.

OK, but I'd like to know what is going to be safe. There's no way to
future-proof the language. I'm quite prepared to replace - with something
else, and if I do then - will need to be adjusted accordingly, I think.

My suggestion would be ~ and ~. I know David Wheeler didn't like that on
the ground that some fonts elevate ~ rather than aligning it in the middle
as most monospaced fonts do, but I'm tempted just to say then use a
different font. Other possibilities that come to mind are + and +,
although I think they're less attractive. But I'll be guided by the
consensus, assuming there is one ;-)

I suspect both of those are pretty safe from an SQL standards point of
view.  Of course, as Tom is often wont to point out, the SQL standards
committee sometimes does bizarre things, so nothing's perfect, but I'd
be rather shocked if any of those got tapped to mean something else.

That having been said, I still don't see value in adding operators at
all.  Good old function call notation seems perfectly adequate from
where I sit.  Sure, it's a little more verbose, but when you try to
too hard make things concise then you end up having to explain to your
users why \ditS is a sensible thing for them to type into psql, or why
s@\W@sprintf%%%02x,ord($)@e in Perl.  I recognize that I may lose
this argument, but I've worked with a couple of languages where
operators can be overloaded (C++) or defined (ML) and it's just never
seemed to work out very well.  YMMV, of course.

I also basically feel this way, although I know I tend more towards
notational brutalism than many.  I think we shouldn't kid ourselves
that non-default operators will be used, and for
current-implementation reasons (that maybe could be fixed by someone
determined) it's not really at the pleasure of the author to use them
via CREATE OPERATOR either.

So, I basically subscribe to view that we should investigate what
total reliance on prefix syntax looks like.  I guess it'll make nested
navigation horribly ugly, though...positively lisp-esque.  That' s one
consideration hstore doesn't have that may make use of infix notations
considerably more useful for json than hstore.




We don't have the luxury of designing things like this in or out from 
scratch. Creation of operators has been a part of PostgreSQL for a good 
while longer than my involvement, and a great many people expect to be 
able to use it. I can just imagine the outrage at any suggestion of 
removing it.


So, please, let's get real. A total reliance on prefix syntax isn't 
going to happen, and investigating what it would look like seems to me 
just so much wasted time and effort.


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] USE_PGXS contrib build is broken

2013-02-04 Thread Alvaro Herrera
Gurjeet Singh wrote:
 On Mon, Feb 4, 2013 at 2:38 PM, Alvaro Herrera 
 alvhe...@2ndquadrant.comwrote:
 
  I just noticed that contrib programs don't build in USE_PGXS=1
  environment:
 
  $ pwd
  /pgsql/build/HEAD/contrib/pg_upgrade
  $ LC_ALL=C USE_PGXS=1 make
  make: *** No rule to make target `check.o', needed by `pg_upgrade'.  Stop.
 
 FWIW, I am able to build just fine using USE_PGXS!

Hmm.  Mine is a VPATH build; maybe that makes a difference.  Now I'm not
really certain that pgxs is supposed to work for VPATH.

-- 
Á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] USE_PGXS contrib build is broken

2013-02-04 Thread Gurjeet Singh
On Mon, Feb 4, 2013 at 3:37 PM, Alvaro Herrera alvhe...@2ndquadrant.comwrote:

 Gurjeet Singh wrote:
  On Mon, Feb 4, 2013 at 2:38 PM, Alvaro Herrera alvhe...@2ndquadrant.com
 wrote:
 
   I just noticed that contrib programs don't build in USE_PGXS=1
   environment:
  
   $ pwd
   /pgsql/build/HEAD/contrib/pg_upgrade
   $ LC_ALL=C USE_PGXS=1 make
   make: *** No rule to make target `check.o', needed by `pg_upgrade'.
  Stop.
 
  FWIW, I am able to build just fine using USE_PGXS!

 Hmm.  Mine is a VPATH build; maybe that makes a difference.  Now I'm not
 really certain that pgxs is supposed to work for VPATH.


Yup, looks like that. When I tried my (default) VPATH build, it did throw
that error! But a regular build went just fine.

My VPATH build that breaks:

USE_PGXS=1 pgmake -C contrib/pg_upgrade

Which gets translated into

USE_PGXS=1 make -C /home/gurjeet/dev/pgdbuilds/pg_master -C
contrib/pg_upgrade


Re: [HACKERS] cannot move relocatable extension out of pg_catalog schema

2013-02-04 Thread Dimitri Fontaine
Robert Haas robertmh...@gmail.com writes:
 On Fri, Feb 1, 2013 at 5:13 PM, Peter Eisentraut pete...@gmx.net wrote:
 I wonder whether it'd not be a better idea to forbid specifying
 pg_catalog as the target schema for relocatable extensions.

We should do that, yes. Rationale: it's only documenting an existing
restriction that you just explained we can't get rid of. Want me to send
a patch (tomorrow)?

 I understand the temptation to forbid pg_catalog as the target schema
 for relocatable extensions, or indeed for object creation in general.

Those two cases are not to be mixed.

 The fact that you can't, for example, go back and drop the objects
 later is a real downer. On the other hand, from a user perspective,
 it's really tempting to want to create certain extensions (adminpack,
 for example) in such a way that they appear to be part of the system
 rather than something that lives in a user schema.  Had we some other

It's easy to do that in the extension's control properties:

relocatable = false
schema = pg_catalog

And the adminpack extension is already set that way. It's then part of
the system and you can still remove it. The only think you can not do is
move its objects in another schema, and I don't much see the point.

 solution to that problem (a second schema that behaves like pg_catalog
 but is empty by default and allows drops?) we might alleviate the need
 to put stuff in pg_catalog per se.

We had extensive talks about that when cooking the extension patch, and
that almost killed it. I think it took about a full year to get back on
our feet again. The only thing I know about that search_path can of
worms is that I will stay away from it as much as possible, and
wholeheartedly advice anyone to do the same.

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support


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


[HACKERS] split rm_name and rm_desc out of rmgr.c

2013-02-04 Thread Alvaro Herrera
Hi,

pg_xlogdump needs access to the *_desc functions for each rmgr.  We
already moved forward quite a bit by splitting those functions out of
their containing files; so now they are compilable separately.  Good.
The remaining task is enabling the code to find those functions in the
first place; currently, the function pointers live in rmgr.c which is
not compilable by frontend code because it contains pointers to other
functions.  Hence the attached patch splits RmgrData into two; the names
and rm_desc functions go into a new file which can be compiled easily by
frontend.

Proposed patch attached.

This comes from 
http://www.postgresql.org/message-id/20130204180327.gh4...@alvh.no-ip.org
which is part of the pg_xlogdump patch in commitfest.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services
*** a/src/backend/access/transam/Makefile
--- b/src/backend/access/transam/Makefile
***
*** 12,20  subdir = src/backend/access/transam
  top_builddir = ../../../..
  include $(top_builddir)/src/Makefile.global
  
! OBJS = clog.o transam.o varsup.o xact.o rmgr.o slru.o subtrans.o multixact.o \
! 	timeline.o twophase.o twophase_rmgr.o xlog.o xlogarchive.o xlogfuncs.o \
! 	xlogreader.o xlogutils.o
  
  include $(top_srcdir)/src/backend/common.mk
  
--- 12,20 
  top_builddir = ../../../..
  include $(top_builddir)/src/Makefile.global
  
! OBJS = clog.o multixact.o rmgrdesc.o rmgr.o slru.o subtrans.o timeline.o \
! 	transam.o twophase.o twophase_rmgr.o varsup.o xact.o xlogarchive.o \
! 	xlogfuncs.o xlog.o xlogreader.o xlogutils.o
  
  include $(top_srcdir)/src/backend/common.mk
  
*** a/src/backend/access/transam/rmgr.c
--- b/src/backend/access/transam/rmgr.c
***
*** 24,46 
  #include storage/standby.h
  #include utils/relmapper.h
  
  
  const RmgrData RmgrTable[RM_MAX_ID + 1] = {
! 	{XLOG, xlog_redo, xlog_desc, NULL, NULL, NULL},
! 	{Transaction, xact_redo, xact_desc, NULL, NULL, NULL},
! 	{Storage, smgr_redo, smgr_desc, NULL, NULL, NULL},
! 	{CLOG, clog_redo, clog_desc, NULL, NULL, NULL},
! 	{Database, dbase_redo, dbase_desc, NULL, NULL, NULL},
! 	{Tablespace, tblspc_redo, tblspc_desc, NULL, NULL, NULL},
! 	{MultiXact, multixact_redo, multixact_desc, NULL, NULL, NULL},
! 	{RelMap, relmap_redo, relmap_desc, NULL, NULL, NULL},
! 	{Standby, standby_redo, standby_desc, NULL, NULL, NULL},
! 	{Heap2, heap2_redo, heap2_desc, NULL, NULL, NULL},
! 	{Heap, heap_redo, heap_desc, NULL, NULL, NULL},
! 	{Btree, btree_redo, btree_desc, btree_xlog_startup, btree_xlog_cleanup, btree_safe_restartpoint},
! 	{Hash, hash_redo, hash_desc, NULL, NULL, NULL},
! 	{Gin, gin_redo, gin_desc, gin_xlog_startup, gin_xlog_cleanup, gin_safe_restartpoint},
! 	{Gist, gist_redo, gist_desc, gist_xlog_startup, gist_xlog_cleanup, NULL},
! 	{Sequence, seq_redo, seq_desc, NULL, NULL, NULL},
! 	{SPGist, spg_redo, spg_desc, spg_xlog_startup, spg_xlog_cleanup, NULL}
  };
--- 24,47 
  #include storage/standby.h
  #include utils/relmapper.h
  
+ /* See rmgrdesc.c, too */
  
  const RmgrData RmgrTable[RM_MAX_ID + 1] = {
! 	{xlog_redo, NULL, NULL, NULL},
! 	{xact_redo, NULL, NULL, NULL},
! 	{smgr_redo, NULL, NULL, NULL},
! 	{clog_redo, NULL, NULL, NULL},
! 	{dbase_redo, NULL, NULL, NULL},
! 	{tblspc_redo, NULL, NULL, NULL},
! 	{multixact_redo, NULL, NULL, NULL},
! 	{relmap_redo, NULL, NULL, NULL},
! 	{standby_redo, NULL, NULL, NULL},
! 	{heap2_redo, NULL, NULL, NULL},
! 	{heap_redo, NULL, NULL, NULL},
! 	{btree_redo, btree_xlog_startup, btree_xlog_cleanup, btree_safe_restartpoint},
! 	{hash_redo, NULL, NULL, NULL},
! 	{gin_redo, gin_xlog_startup, gin_xlog_cleanup, gin_safe_restartpoint},
! 	{gist_redo, gist_xlog_startup, gist_xlog_cleanup, NULL},
! 	{seq_redo, NULL, NULL, NULL},
! 	{spg_redo, spg_xlog_startup, spg_xlog_cleanup, NULL}
  };
*** /dev/null
--- b/src/backend/access/transam/rmgrdesc.c
***
*** 0 
--- 1,47 
+ /*
+  * rmgrdesc.c
+  *
+  * Resource managers descriptor function definitions
+  *
+  * src/backend/access/transam/rmgrdesc.c
+  */
+ #include postgres.h
+ 
+ #include access/clog.h
+ #include access/gin.h
+ #include access/gist_private.h
+ #include access/hash.h
+ #include access/heapam_xlog.h
+ #include access/multixact.h
+ #include access/nbtree.h
+ #include access/spgist.h
+ #include access/xact.h
+ #include access/xlog_internal.h
+ #include catalog/storage_xlog.h
+ #include commands/dbcommands.h
+ #include commands/sequence.h
+ #include commands/tablespace.h
+ #include storage/standby.h
+ #include utils/relmapper.h
+ 
+ /* See rmgr.c, too */
+ 
+ const RmgrDescData RmgrDescTable[RM_MAX_ID + 1] = {
+ 	{XLOG, xlog_desc},
+ 	{Transaction, xact_desc},
+ 	{Storage, smgr_desc},
+ 	{CLOG, clog_desc},
+ 	{Database, dbase_desc},
+ 	{Tablespace, tblspc_desc},
+ 	{MultiXact, multixact_desc},
+ 	{RelMap, relmap_desc},
+ 	{Standby, standby_desc},
+ 	{Heap2, heap2_desc},
+ 	{Heap, heap_desc},
+ 	{Btree, btree_desc},
+ 	{Hash, 

Re: [HACKERS] issues with range types, btree_gist and constraints

2013-02-04 Thread Tomas Vondra
Hi,

I've managed to further simplify the test-case, and I've verified that
it's reproducible on current 9.2 and 9.3 branches.

This is the necessary table structure:

---
CREATE TABLE test (
idTEXT,
valid TSRANGE NOT NULL DEFAULT tsrange(NULL, NULL),
CONSTRAINT unique_ids EXCLUDE USING GIST (id WITH =, valid WITH )
);

CREATE OR REPLACE FUNCTION skip_existing() RETURNS trigger AS $$
DECLARE
v_exists BOOLEAN;
BEGIN

SELECT TRUE INTO v_exists FROM test WHERE id = NEW.id;
IF v_exists THEN
RETURN NULL;
END IF;

RETURN NEW;

END;
$$ LANGUAGE plpgsql;

CREATE TRIGGER skip_existing BEFORE INSERT ON test FOR EACH ROW EXECUTE
PROCEDURE skip_existing();
---

I've been unable to reproduce the bug with just a single text column.

The trigger simply skips existing records without throwing any error.

Now let's insert some data into the table - I'll use the same samples as
in the previous test-case (http://www.fuzzy.cz/tmp/samples.tgz).


test=# copy test(id) from '/tmp/sample-1.csv';
COPY 20001
test=# copy test(id) from '/tmp/sample-2.csv';
COPY 18590
test=# copy test(id) from '/tmp/sample-1.csv';
COPY 25
test=# copy test(id) from '/tmp/sample-2.csv';
COPY 45

The last two results are really suspicious - it means that some record
were inserted again, so let's verify that:

test=# select id, count(*) from test group by id having count(*)  1;
id| count
--+---
 0aab4791e1e41f62fd8452ae2c854a34 | 2
 0aa08441cd4526b972bb3451d9f8e4ea | 2
 0ab969a342837484ec0f81bf1e03 | 2
 0aea0c33c76b1fe5d123b18cec184dc0 | 2
 0af75a99b37be6dde08afaa69de36d29 | 2
 0af80d2c2931756b897b3ca5a0055820 | 2
 ... many more ...

On 9.3 the number of duplicates is much lower, and it's not stable - on
one run I get 6, on the very next one I get 2, then 5 and so on. That
leads me to a suspicion that it might be an uninitialized variable or
something like that.

Tomas


-- 
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] split rm_name and rm_desc out of rmgr.c

2013-02-04 Thread Simon Riggs
On 4 February 2013 20:57, Alvaro Herrera alvhe...@2ndquadrant.com wrote:

 pg_xlogdump needs access to the *_desc functions for each rmgr.  We
 already moved forward quite a bit by splitting those functions out of
 their containing files; so now they are compilable separately.  Good.
 The remaining task is enabling the code to find those functions in the
 first place; currently, the function pointers live in rmgr.c which is
 not compilable by frontend code because it contains pointers to other
 functions.  Hence the attached patch splits RmgrData into two; the names
 and rm_desc functions go into a new file which can be compiled easily by
 frontend.

 Proposed patch attached.

Not meaning to cause you extra work, but some kind of id as the first
field of each structure would allow a cross-check that there is no
misconfiguration.

-- 
 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] json api WIP patch

2013-02-04 Thread Merlin Moncure
On Mon, Feb 4, 2013 at 2:10 PM, Andrew Dunstan and...@dunslane.net wrote:

 My only remaining nit with the proposal is with json_unnest().

 SQL unnest() produces list of scalars regardless of dimensionality --
 json unnest unwraps one level only (contrast: pl/pgsql array 'slice').
   So I think 'json_array_each', or perhaps json_slice() is a better fit
 there.



 how about json_array_elements()?

that works (although it's a little verbose for my taste).  maybe
json_unwrap, json_array_unwrap, etc.

merlin


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


Re: [HACKERS] json api WIP patch

2013-02-04 Thread Daniel Farina
On Mon, Feb 4, 2013 at 12:37 PM, Andrew Dunstan and...@dunslane.net wrote:

 On 02/04/2013 03:16 PM, Daniel Farina wrote:

 On Mon, Feb 4, 2013 at 11:38 AM, Robert Haas robertmh...@gmail.com
 wrote:

 On Mon, Feb 4, 2013 at 11:10 AM, Andrew Dunstan and...@dunslane.net
 wrote:

 On 02/04/2013 10:47 AM, Robert Haas wrote:


 The SQL standards considerations seem worth thinking about, too.
 We've certainly gone through a lot of pain working toward eliminating
 = as an operator name, and if the SQL standard has commandeered -
 for some purpose or other, I'd really rather not add to the headaches
 involved should we ever decide to reclaim it.

 OK, but I'd like to know what is going to be safe. There's no way to
 future-proof the language. I'm quite prepared to replace - with
 something
 else, and if I do then - will need to be adjusted accordingly, I
 think.

 My suggestion would be ~ and ~. I know David Wheeler didn't like that
 on
 the ground that some fonts elevate ~ rather than aligning it in the
 middle
 as most monospaced fonts do, but I'm tempted just to say then use a
 different font. Other possibilities that come to mind are + and +,
 although I think they're less attractive. But I'll be guided by the
 consensus, assuming there is one ;-)

 I suspect both of those are pretty safe from an SQL standards point of
 view.  Of course, as Tom is often wont to point out, the SQL standards
 committee sometimes does bizarre things, so nothing's perfect, but I'd
 be rather shocked if any of those got tapped to mean something else.

 That having been said, I still don't see value in adding operators at
 all.  Good old function call notation seems perfectly adequate from
 where I sit.  Sure, it's a little more verbose, but when you try to
 too hard make things concise then you end up having to explain to your
 users why \ditS is a sensible thing for them to type into psql, or why
 s@\W@sprintf%%%02x,ord($)@e in Perl.  I recognize that I may lose
 this argument, but I've worked with a couple of languages where
 operators can be overloaded (C++) or defined (ML) and it's just never
 seemed to work out very well.  YMMV, of course.

 I also basically feel this way, although I know I tend more towards
 notational brutalism than many.  I think we shouldn't kid ourselves
 that non-default operators will be used, and for
 current-implementation reasons (that maybe could be fixed by someone
 determined) it's not really at the pleasure of the author to use them
 via CREATE OPERATOR either.

 So, I basically subscribe to view that we should investigate what
 total reliance on prefix syntax looks like.  I guess it'll make nested
 navigation horribly ugly, though...positively lisp-esque.  That' s one
 consideration hstore doesn't have that may make use of infix notations
 considerably more useful for json than hstore.



 We don't have the luxury of designing things like this in or out from
 scratch. Creation of operators has been a part of PostgreSQL for a good
 while longer than my involvement, and a great many people expect to be able
 to use it. I can just imagine the outrage at any suggestion of removing it.

I am only referring to referring the restriction that the planner
can't understand that fetchval() and '-' mean the same thing for,
say, hstore.  Hence, use of non-default CREATE OPERATOR may become
more useful some day, instead of basically being a pitfall when
someone reasonably thinks they could use either spelling of the same
functionality and the optimizer will figure it out.

I'm not suggesting removal of any feature.

My reference to total reliance of prefix syntax refers only to the
JSON operators, since the previous correspondence from Robert was
about how function call syntax alone may be sufficient.  This phrase
refers to the same idea he is proposing.

I also included a weakness to that idea, which is that nesting in JSON
makes the situation worse than the common compared case, hstore.

--
fdr


-- 
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] split rm_name and rm_desc out of rmgr.c

2013-02-04 Thread Alvaro Herrera
Simon Riggs wrote:
 On 4 February 2013 20:57, Alvaro Herrera alvhe...@2ndquadrant.com wrote:
 
  pg_xlogdump needs access to the *_desc functions for each rmgr.  We
  already moved forward quite a bit by splitting those functions out of
  their containing files; so now they are compilable separately.  Good.
  The remaining task is enabling the code to find those functions in the
  first place; currently, the function pointers live in rmgr.c which is
  not compilable by frontend code because it contains pointers to other
  functions.  Hence the attached patch splits RmgrData into two; the names
  and rm_desc functions go into a new file which can be compiled easily by
  frontend.
 
  Proposed patch attached.
 
 Not meaning to cause you extra work, but some kind of id as the first
 field of each structure would allow a cross-check that there is no
 misconfiguration.

Well, we could add the rmgr_id as the first struct member to both
tables and add an Assert() somewhere in xlog.c.  However, do we really
need that?  These tables are almost immutable, and failure to edit both
simultaneously should be promptly detected.

-- 
Á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] json api WIP patch

2013-02-04 Thread Andrew Dunstan


On 02/04/2013 04:19 PM, Daniel Farina wrote:

On Mon, Feb 4, 2013 at 12:37 PM, Andrew Dunstan and...@dunslane.net wrote:

On 02/04/2013 03:16 PM, Daniel Farina wrote:

On Mon, Feb 4, 2013 at 11:38 AM, Robert Haas robertmh...@gmail.com
wrote:

On Mon, Feb 4, 2013 at 11:10 AM, Andrew Dunstan and...@dunslane.net
wrote:

On 02/04/2013 10:47 AM, Robert Haas wrote:


The SQL standards considerations seem worth thinking about, too.
We've certainly gone through a lot of pain working toward eliminating
= as an operator name, and if the SQL standard has commandeered -
for some purpose or other, I'd really rather not add to the headaches
involved should we ever decide to reclaim it.

OK, but I'd like to know what is going to be safe. There's no way to
future-proof the language. I'm quite prepared to replace - with
something
else, and if I do then - will need to be adjusted accordingly, I
think.

My suggestion would be ~ and ~. I know David Wheeler didn't like that
on
the ground that some fonts elevate ~ rather than aligning it in the
middle
as most monospaced fonts do, but I'm tempted just to say then use a
different font. Other possibilities that come to mind are + and +,
although I think they're less attractive. But I'll be guided by the
consensus, assuming there is one ;-)

I suspect both of those are pretty safe from an SQL standards point of
view.  Of course, as Tom is often wont to point out, the SQL standards
committee sometimes does bizarre things, so nothing's perfect, but I'd
be rather shocked if any of those got tapped to mean something else.

That having been said, I still don't see value in adding operators at
all.  Good old function call notation seems perfectly adequate from
where I sit.  Sure, it's a little more verbose, but when you try to
too hard make things concise then you end up having to explain to your
users why \ditS is a sensible thing for them to type into psql, or why
s@\W@sprintf%%%02x,ord($)@e in Perl.  I recognize that I may lose
this argument, but I've worked with a couple of languages where
operators can be overloaded (C++) or defined (ML) and it's just never
seemed to work out very well.  YMMV, of course.

I also basically feel this way, although I know I tend more towards
notational brutalism than many.  I think we shouldn't kid ourselves
that non-default operators will be used, and for
current-implementation reasons (that maybe could be fixed by someone
determined) it's not really at the pleasure of the author to use them
via CREATE OPERATOR either.

So, I basically subscribe to view that we should investigate what
total reliance on prefix syntax looks like.  I guess it'll make nested
navigation horribly ugly, though...positively lisp-esque.  That' s one
consideration hstore doesn't have that may make use of infix notations
considerably more useful for json than hstore.



We don't have the luxury of designing things like this in or out from
scratch. Creation of operators has been a part of PostgreSQL for a good
while longer than my involvement, and a great many people expect to be able
to use it. I can just imagine the outrage at any suggestion of removing it.

I am only referring to referring the restriction that the planner
can't understand that fetchval() and '-' mean the same thing for,
say, hstore.  Hence, use of non-default CREATE OPERATOR may become
more useful some day, instead of basically being a pitfall when
someone reasonably thinks they could use either spelling of the same
functionality and the optimizer will figure it out.

I'm not suggesting removal of any feature.

My reference to total reliance of prefix syntax refers only to the
JSON operators, since the previous correspondence from Robert was
about how function call syntax alone may be sufficient.  This phrase
refers to the same idea he is proposing.

I also included a weakness to that idea, which is that nesting in JSON
makes the situation worse than the common compared case, hstore.




I see. OK, sorry for misunderstanding.


I suspect, BTW that mostly people will use get_path*() (or rather, its 
equivalent operator ;-) ) rather than operator chaining:


select myjson-'{authors,0,name}'::text[];

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] Turning off hot_standby_feedback

2013-02-04 Thread Simon Riggs
On 4 February 2013 15:52, Robert Haas robertmh...@gmail.com wrote:
 On Sun, Feb 3, 2013 at 5:25 PM, Simon Riggs si...@2ndquadrant.com wrote:
 There is a bug in hot_standby_feedback that causes the xmin of the
 walsender process to not be reset when hot_standby_feedback is turned
 off after it had previously sent at least one feedback message.

 Clearly, that is a bad thing and deserves backpatching to 9.1, unless
 I hear otherwise real soon.

 +1.

Patches for 9.1 and 9.2 different to the one for HEAD.

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


hs_feedback_off.92.v1.patch
Description: Binary data


hs_feedback_off.91.v1.patch
Description: Binary data

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


Re: [HACKERS] autovacuum not prioritising for-wraparound tables

2013-02-04 Thread Jeff Janes
On Sat, Feb 2, 2013 at 5:25 AM, Andres Freund and...@2ndquadrant.com wrote:
 On 2013-02-01 15:09:34 -0800, Jeff Janes wrote:
 On Fri, Feb 1, 2013 at 2:34 PM, Andres Freund and...@2ndquadrant.com wrote:
  On 2013-02-01 14:05:46 -0800, Jeff Janes wrote:

  As far as I can tell this bug kicks in when your cluster gets to be
  older than freeze_min_age, and then lasts forever after.  After that
  point pretty much every auto-vacuum inspired by update/deletion
  activity will get promoted to a full table scan.  (Which makes me
  wonder how much field-testing the vm-only vacuum has received, if it
  was rarely happening in practice due to this bug.)
 
  I think you're misreading the code. freezeTableLimit is calculated by

   limit = ReadNewTransactionId() - freezetable;

  which is always relative to the current xid. The bug was that
  freezetable had the wrong value in autovac due to freeze_min_age being
  used instead of freeze_table_age.

 Right.  Since freeze_min_age was mistakenly being used, the limit
 would be 50 million in the past (rather than 150 million) under
 defaults.  But since the last full-table vacuum, whenever that was,
 used freeze_min_age for its intended purpose, that means the 50
 million in the past *at the time of that last vacuum* is the highest
 that relfrozenxid can be.  And that is going to be further back than
 50 million from right now, so the vacuum will always be promoted to a
 full scan.

 Oh, wow. Youre right. I shouldn't answer emails after sport with cramped
 fingers on a friday night... And I should have thought about this
 scenario, because I essentially already explained it upthread, just with
 a different set of variables.

 This is rather scary. How come nobody noticed that this major
 performance improvement was effectively disabled for that long?

I'm not sure whom to address this to, but the just-committed release
notes for this issue reflect the original understanding that it only
applied when vacuum_freeze_min_age was lowered from its default.
Rather than the current understanding that it effects all old-enough
systems.

If the release notes are not already baked in, I would suggest this wording:

+  The main consequence of this mistake is that it
+  caused full-table vacuuming scans to occur much more frequently
+  than intended.


Cheers,

Jeff


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


Re: [HACKERS] autovacuum not prioritising for-wraparound tables

2013-02-04 Thread Tom Lane
Jeff Janes jeff.ja...@gmail.com writes:
 On Sat, Feb 2, 2013 at 5:25 AM, Andres Freund and...@2ndquadrant.com wrote:
 If the release notes are not already baked in, I would suggest this wording:

 +  The main consequence of this mistake is that it
 +  caused full-table vacuuming scans to occur much more frequently
 +  than intended.

It's baked ... sorry about that, but when making the notes there's
seldom time to go through the threads about every patch to see if the
commit messages are accurate or not.

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] split rm_name and rm_desc out of rmgr.c

2013-02-04 Thread Tom Lane
Alvaro Herrera alvhe...@2ndquadrant.com writes:
 pg_xlogdump needs access to the *_desc functions for each rmgr.  We
 already moved forward quite a bit by splitting those functions out of
 their containing files; so now they are compilable separately.  Good.
 The remaining task is enabling the code to find those functions in the
 first place; currently, the function pointers live in rmgr.c which is
 not compilable by frontend code because it contains pointers to other
 functions.  Hence the attached patch splits RmgrData into two; the names
 and rm_desc functions go into a new file which can be compiled easily by
 frontend.

 Proposed patch attached.

This seems pretty ugly to me.

Couldn't we do something similar to the design for SQL keyword constants,
wherein the actual data is in macros in a header file (providing exactly
one source of truth for each RM) and then various .c files can #include
that after #defining the macro as they need?  See
src/include/parser/kwlist.h and the files that include that.

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] issues with range types, btree_gist and constraints

2013-02-04 Thread Tom Lane
Tomas Vondra t...@fuzzy.cz writes:
 I've managed to further simplify the test-case, and I've verified that
 it's reproducible on current 9.2 and 9.3 branches.

Yeah, I've been able to reproduce it as well.  I found what I thought
might be the bug: gbt_var_bin_union() looks like it does the wrong thing
if both ends of the range need to be extended.  However, fixing that
doesn't fix your test case, so there's more to it.

 I've been unable to reproduce the bug with just a single text column.

Hm.  I noticed a lot of elog complaints about picksplit method
... doesn't support secondary split, so maybe the bug is in the code
that's trying to deal with that.  I've not had time to look closer.

regards, tom lane


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


[HACKERS] palloc unification

2013-02-04 Thread Alvaro Herrera
There was some discussion about unifying backend and frontend
code/headers for palloc et al, particularly so that programs that want
to mix both can be easily compiled; see 
http://www.postgresql.org/message-id/20130118150629.gc29...@alap2.anarazel.de
for what I believe to be the latest and greatest patch in that area.
The good news from that patch is that there is a reported (small)
performance increase from the approach suggested there, on Intel and
non-Intel platforms.  So it seems we're okay on that front.

On the source code organization front, however, I'm not too happy with
that particular proposal.  There would be two files named palloc.h, for
one thing, and also the changes to utils/palloc.h leave way too much of
the backend-only implementation exposed to the frontend world.

I propose to have a new subdirectory src/include/shared, and two
header files:

src/include/utils/palloc.h (same as today)
src/include/shared/fe_memutils.h
(pg_malloc, pg_free, pg_strdup decls)

utils/palloc.h would be modified so that #ifdef FRONTEND, only the basic
function declarations (palloc, pfree, pstrdup, pnstrdup) are exposed;
frontend environment would be safe from CurrentMemoryContext etc.

The frontend (pg_malloc) function definitions would live somewhere in,
say, src/shared/fe_memutils.c.  For the palloc() implementation, I think
we should create another file, so that frontend-only programs that do
not require those symbols (most of them) are not unnecessarily bloated.

Opinions on having a new subdir?
We could eventually move some stuff from timestamp.c in there, so that
pg_xlogdump could get timestamp_to_str from there; we could perhaps
remove the ecpg implementation of that as well and make it use this one.

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


[HACKERS] get_progname() should not be const char *?

2013-02-04 Thread Phil Sorber
get_progname() returns a strdup()'d value. Shouldn't it then be simply
char * and not const char *? Otherwise free() complains loudly without
a cast.

diff --git a/src/include/port.h b/src/include/port.h
new file mode 100644
index 99d3a9b..2d6a435
*** a/src/include/port.h
--- b/src/include/port.h
*** extern void make_native_path(char *path)
*** 45,51 
  extern bool path_contains_parent_reference(const char *path);
  extern bool path_is_relative_and_below_cwd(const char *path);
  extern bool path_is_prefix_of_path(const char *path1, const char *path2);
! extern const char *get_progname(const char *argv0);
  extern void get_share_path(const char *my_exec_path, char *ret_path);
  extern void get_etc_path(const char *my_exec_path, char *ret_path);
  extern void get_include_path(const char *my_exec_path, char *ret_path);
--- 45,51 
  extern bool path_contains_parent_reference(const char *path);
  extern bool path_is_relative_and_below_cwd(const char *path);
  extern bool path_is_prefix_of_path(const char *path1, const char *path2);
! extern char *get_progname(const char *argv0);
  extern void get_share_path(const char *my_exec_path, char *ret_path);
  extern void get_etc_path(const char *my_exec_path, char *ret_path);
  extern void get_include_path(const char *my_exec_path, char *ret_path);
diff --git a/src/port/path.c b/src/port/path.c
new file mode 100644
index 72ca24c..ebfc94c
*** a/src/port/path.c
--- b/src/port/path.c
*** path_is_prefix_of_path(const char *path1
*** 411,417 
   * Extracts the actual name of the program as called -
   * stripped of .exe suffix if any
   */
! const char *
  get_progname(const char *argv0)
  {
const char *nodir_name;
--- 411,417 
   * Extracts the actual name of the program as called -
   * stripped of .exe suffix if any
   */
! char *
  get_progname(const char *argv0)
  {
const char *nodir_name;


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


Re: [HACKERS] json api WIP patch

2013-02-04 Thread Hannu Krosing

On 01/31/2013 11:20 PM, Andrew Dunstan wrote:


I'm happy to take opinions about this, and I expected
some bikeshedding, but your reaction is contrary to
everything others have told me. Mostly they love the operators.

What I would really like is if we extended postgresql core and made
a few more constructs definable as overloadable operator:

1) array / dictionary element lookup
  a[b]
 CREATE OPERATOR [] (...)

2) attribute lookup
  a.b
 CREATE OPERATOR . (...)

then you could make json lookups either step-by-step using

CREATE OPERATOR [] (
PROCEDURE = json_array_lookup, LEFTARG = json, RIGHTARG = int)

and

CREATE OPERATOR [] (
PROCEDURE = json_dict_lookup, LEFTARG = json, RIGHTARG = text)

fourthname = myjson[4]['name']


or perhaps a single


CREATE OPERATOR [] (
PROCEDURE = json_deep_lookup, LEFTARG = json, RIGHTARG = VARIADIC 
any)


fourthname = myjson[4, 'name']


though I suspect that we do not support type VARIADIC any in operator 
definitions


-
Hannu




I guess that '~' and '~' would work as well as '-' and '-'.


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] split rm_name and rm_desc out of rmgr.c

2013-02-04 Thread Andres Freund
On 2013-02-04 17:27:39 -0500, Tom Lane wrote:
 Alvaro Herrera alvhe...@2ndquadrant.com writes:
  pg_xlogdump needs access to the *_desc functions for each rmgr.  We
  already moved forward quite a bit by splitting those functions out of
  their containing files; so now they are compilable separately.  Good.
  The remaining task is enabling the code to find those functions in the
  first place; currently, the function pointers live in rmgr.c which is
  not compilable by frontend code because it contains pointers to other
  functions.  Hence the attached patch splits RmgrData into two; the names
  and rm_desc functions go into a new file which can be compiled easily by
  frontend.
 
  Proposed patch attached.
 
 This seems pretty ugly to me.
 
 Couldn't we do something similar to the design for SQL keyword constants,
 wherein the actual data is in macros in a header file (providing exactly
 one source of truth for each RM) and then various .c files can #include
 that after #defining the macro as they need?  See
 src/include/parser/kwlist.h and the files that include that.

My original patch just surrounded the backend-only functions by a macro
which was defined to NULL in #ifdef FRONTEND. I still think thats the
best way to go. People objected to that though...
Other than that I find duplicating the whole table by far the best
approach. Splitting parts off seems to be more complex than warranted
without actually getting rid of duplication.

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] split rm_name and rm_desc out of rmgr.c

2013-02-04 Thread Alvaro Herrera
Tom Lane wrote:
 Alvaro Herrera alvhe...@2ndquadrant.com writes:
  pg_xlogdump needs access to the *_desc functions for each rmgr.  We
  already moved forward quite a bit by splitting those functions out of
  their containing files; so now they are compilable separately.  Good.
  The remaining task is enabling the code to find those functions in the
  first place; currently, the function pointers live in rmgr.c which is
  not compilable by frontend code because it contains pointers to other
  functions.  Hence the attached patch splits RmgrData into two; the names
  and rm_desc functions go into a new file which can be compiled easily by
  frontend.
 
  Proposed patch attached.
 
 This seems pretty ugly to me.
 
 Couldn't we do something similar to the design for SQL keyword constants,
 wherein the actual data is in macros in a header file (providing exactly
 one source of truth for each RM) and then various .c files can #include
 that after #defining the macro as they need?  See
 src/include/parser/kwlist.h and the files that include that.

Meh.  I proposed this months ago and was shot down.  I still like it
better than what I propose here, so I will resurrect it.

-- 
Á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] sepgsql and materialized views

2013-02-04 Thread Kevin Grittner
Kohei KaiGai kai...@kaigai.gr.jp wrote:

 Let me confirm a significant point. Do you never plan a feature
 that allows to update/refresh materialized-views out of user
 session?

Currently only the owner of the MV or a database superuser can
refresh it, and the refresh is run with the permissions of the MV
owner.  The main change to that I see is that the owner could
establish a policy of automatic updates to the MV based on changes
to the underlying tables, with a timing established by the MV owner
or a database superuser.

 I had an impression on asynchronous update of MV something like
 a feature that moves data from regular tables to MV with
 batch-jobs in mid-night, but under the privilege that bypass
 regular permission checks.

I would expect it to be more a matter of being based on the
authority of the MV owner.  That raises interesting questions about
what happens if a permission which allowed the MV to be defined is
revoked from the owner, or if the MV is altered to have an owner
without permission to access the underlying data.  With the current
patch, if the owner is changed to a user who does not have rights
to access the underlying table, a permission denied error is
generated when that new owner tries to refresh the MV.

 It it is never planned, my concern was pointless.

I think these are issues require vigilance.  I hadn't really
thought through all of this, but since the creation and refresh
work off of the existing VIEW code, and the querying works off of
the existing TABLE code, the existing security mechanisms tend to
come into play by default.

 My concern is future development that allows to update/refresh MV
 asynchronously, out of privilege control.

While it has not yet been defined, my first reaction is that it
should happen under privileges of the MV owner.

 As long as all the update/refresh operation is under privilege
 control with user-id/security label of the current session, here
 is no difference from regular writer operation of tables with
 contents read from other tables.

Again, it's just my first impression, but I think that the
permissions of the current session would control whether the
operation would be allowed on the underlying tables, but the
permissions of the MV owner would control replication to the MV.

 BTW, please clarify expected behavior in case when MV contains
 WHERE clause that returns different result depending on privilege
 of current session, such as:
   ... WHERE underlying_table.uname = CURRENT_USER

Ah, good question.  Something else I hadn't thought about.  When I
read that I was afraid that the current patch left a security hole
where if the owner didn't have rights to populate the MV with
something, it could still get there if a database superuser ran
REFRESH, but it seems to do what I would think is the right thing. 
With kgrittn as a superuser and bob as a normal user:

test=# set role bob;
SET
test= select * from t;
ERROR:  permission denied for relation t
test= reset role;
RESET
test=# alter materialized view tm owner to bob;
ALTER MATERIALIZED VIEW
test=# set role bob;
SET
test= refresh MATERIALIZED VIEW tm;
ERROR:  permission denied for relation t
test= reset role;
RESET
test=# refresh MATERIALIZED VIEW tm;
ERROR:  permission denied for relation t
test=# alter materialized view tm owner to kgrittn;
ALTER MATERIALIZED VIEW
test=# refresh MATERIALIZED VIEW tm;
REFRESH MATERIALIZED VIEW

So it seems to run the refresh as the owner, not under authority of
the session.

 It seems to me this MV saves just a snapshot from a standpoint of
 a particular user who refreshed this MV; typically its owner.

Exactly.  Typically a summary of a snapshot of underlying detail.

 If bob has privilege to reference this MV, he will see rows to be
 visible for alice. Of course, it does not contradictory, because
 all alice doing is just writing data she can see into a table
 being visible for public.

Right.  From a security perspective it is rather like alice running
CREATE TABLE AS.  And again, it is worth remembering that the usual
reason for creating one of these is to summarize data, to support
quick generation of statistical reports, for example.

 Even if MV's contents were moved in out of privilege controls,
 we can ensure the current user has rights to reference data of
 MV, as long as he has privileges to reference underlying data
 source.

I don't think it should have anything to do with authority to the
underlying tables from which the data is selected.

 On the other hand, it can make problems if some internal stuff
 moves data from regular tables with confidential label into MV
 with unconfidential label; that works official information leak
 channel.

I don't see that it opens any new vulnerabilities compared to
INSERT ... SELECT or CREATE TABLE AS.

 Only point I'm concerned about is whether we will support a
 feature that refresh materialized-view without appropriate
 privilege control, or not.

I guess the question is whether it makes sense 

[HACKERS] Reminder to committers: we're done with 8.3 branch

2013-02-04 Thread Tom Lane
Per the support policy at
http://www.postgresql.org/support/versioning/
the 8.3.x branch has an EOL date of February 2013.

That means that, barring discovery of a bug so bad that we decide to
make a fresh set of update releases within the month, the just-wrapped
8.3.23 is the last release in the 8.3.x series.

There is therefore no more need to back-patch into REL8_3_STABLE.

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] get_progname() should not be const char *?

2013-02-04 Thread Tom Lane
Phil Sorber p...@omniti.com writes:
 get_progname() returns a strdup()'d value. Shouldn't it then be simply
 char * and not const char *? Otherwise free() complains loudly without
 a cast.

I don't believe that callers should be trying to free() the result.
Whether it's been strdup'd or not is not any of their business.

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] get_progname() should not be const char *?

2013-02-04 Thread Phil Sorber
On Mon, Feb 4, 2013 at 10:52 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Phil Sorber p...@omniti.com writes:
 get_progname() returns a strdup()'d value. Shouldn't it then be simply
 char * and not const char *? Otherwise free() complains loudly without
 a cast.

 I don't believe that callers should be trying to free() the result.
 Whether it's been strdup'd or not is not any of their business.

Is that just because of the nature of this specific function?


 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] proposal: ANSI SQL 2011 syntax for named parameters

2013-02-04 Thread Pavel Stehule
2013/2/4 Robert Haas robertmh...@gmail.com:
 On Mon, Feb 4, 2013 at 1:06 PM, Simon Riggs si...@2ndquadrant.com wrote:
 On 2 January 2013 22:51, Robert Haas robertmh...@gmail.com wrote:
 On Fri, Dec 28, 2012 at 11:22 AM, Pavel Stehule pavel.steh...@gmail.com 
 wrote:
 I am not sure, but maybe is time to introduce ANSI SQL syntax for
 functions' named parameters

 It is defined in ANSI SQL 2011

  CALL P (B = 1, A = 2)

 instead PostgreSQL syntax CALL ( B := 1, A := 2)

 Keep in mind that, as recently as PostgreSQL 9.1, we shipped hstore
 with a =(text, text) operator.  That operator was deprecated in 9.0,
 but it wasn't actually removed until PostgreSQL 9.2.  Whenever we do
 this, it's going to break things for anyone who hasn't yet upgraded
 from hstore v1.0 to hstore v1.1.  So I would prefer to wait one more
 release.  That way, anyone who does an upgrade, say, every other major
 release cycle should have a reasonably clean upgrade path.

 I don't see why waiting 1 year makes this situation any better. We
 just make upgrading to hstore 1.1 a prerequisite and we're done.

 I doubt there are many people using hstore who haven't upgraded, and
 fewer still that will upgrade yet can't follow simple instructions on
 prerequisites. While hstore is reasonably popular, users are still in
 the minority.

 You can always override the operators using a different search_path if
 you still see problems there.

 We need to find ways forwards rather than block progress because of
 obscure issues.

 This seems pretty close to an accusation of bad faith, which I don't
 believe to be present.  Right now there is one and only one release in
 the field that contains hstore 1.1.  If we go ahead and prohibit = as
 an operator name now, we're going to require everyone who is on 9.1
 and uses hstore and wants to get to 9.3 to either (a) first upgrade to
 9.2, then update hstore, then upgrade to 9.3; or (b) dig the
 hstore-1.1 update out of a future release, apply it to an earlier
 release on which it did not ship, and then upgrade.  If they're
 actually *using* the = operator (rather than just having it in the
 DB), they'll also need to rewrite their application before doing any
 of that.

 Perhaps the users that you support won't mind that, but the users I
 support will.  In fact, they're likely going to mind it even if we
 have two releases where either version of hstore will work, but at
 least it will ameliorate the problem somewhat.

 I find your attitude toward backward compatibility to be astonishingly
 inconsistent.  We haven't made any progress on overhauling
 recovery.conf in two years because you've steadfastly stonewalled any
 forward progress on backwards-compatibility grounds, even though a
 change there can't possible break any working PostgreSQL
 *application*, only the admin tools.  But now, on this change and on a
 few others, which actually will break applications, you want to push
 it forward faster.  That seems completely backwards to me.  In my
 experience, it is far easier to get people to adjust their admin
 scripts (which are usually controlled by the same team responsible for
 the database upgrade anyway) than to get them to fix their
 applications (which are usually written by a different team over which
 the DBAs have no real control).

 We've had customers postpone upgrades for *years* because of issues
 like this.  Labeling it as obscure suggests that it is unimportant or
 should be dismissed, a conclusion with which I respectfully disagree.

| know so GUC is not popular but we can introduce new syntax controlled by GUC.

Probably there are not simple best solution. People that use hstore
will prefer compatibility and slower progress, people that wrote more
stored procedures will prefer faster progress - because will write lot
of code with possible obsolete (in future) syntax. GUC can introduce
this feature early - and don't create or don't solve any problems that
we (or users) have to solve in following versions.

Regards

Pavel



 --
 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] json api WIP patch

2013-02-04 Thread Pavel Stehule
2013/2/5 Hannu Krosing ha...@krosing.net:
 On 01/31/2013 11:20 PM, Andrew Dunstan wrote:


 I'm happy to take opinions about this, and I expected
 some bikeshedding, but your reaction is contrary to
 everything others have told me. Mostly they love the operators.

 What I would really like is if we extended postgresql core and made
 a few more constructs definable as overloadable operator:

 1) array / dictionary element lookup
   a[b]
  CREATE OPERATOR [] (...)

 2) attribute lookup
   a.b
  CREATE OPERATOR . (...)

 then you could make json lookups either step-by-step using

 CREATE OPERATOR [] (
 PROCEDURE = json_array_lookup, LEFTARG = json, RIGHTARG = int)

 and

 CREATE OPERATOR [] (
 PROCEDURE = json_dict_lookup, LEFTARG = json, RIGHTARG = text)

 fourthname = myjson[4]['name']


 or perhaps a single


 CREATE OPERATOR [] (
 PROCEDURE = json_deep_lookup, LEFTARG = json, RIGHTARG = VARIADIC any)

 fourthname = myjson[4, 'name']


it is near to full collection implementation - and can be nice to have
it. For this moment we should to return to this topic.

My preference is using well named functions (prefer it against
operator) and operator that are not in collision with current ANSI SQL

I don't see any nice on design   select
myjson-'{authors,0,name}'::text[]; - more it is ugly as
dinosaurs

better and more usual

myjson['authors']['0']['name']

or

myjson['authors/0/name']

Regards

Pavel


 though I suspect that we do not support type VARIADIC any in operator
 definitions

 -
 Hannu



 I guess that '~' and '~' would work as well as '-' and '-'.


 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


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