Re: [HACKERS] psql: show only failed queries

2014-06-25 Thread Samrat Revagade
> I am sending updated patch - buggy statement is printed via more logical
psql_error function instead printf

Thank you for updating patch, I really appreciate your efforts.

Now, everything is good from my side.
* it apply cleanly to the current git master
* includes necessary docs
* I think It is very good  and necessary feature.

If Kumar Rajeev Rastogi do not have any extra comments, then I think patch
is  ready for committer.


Re: [HACKERS] [PATCH] log_{directory,filename} doc fixes

2014-06-25 Thread Fujii Masao
On Wed, Jun 25, 2014 at 11:38 PM, Christoph Berg
 wrote:
> Hi,
>
> The defaults for log_directory and log_filename were undocumented, and
> the log_filename docs still refered to a config example that was
> removed with 8.4 deprecation of epoch-based logfilenames.

Thanks! Committed.

Regards,

-- 
Fujii Masao


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


Re: [HACKERS] idle_in_transaction_timeout

2014-06-25 Thread Fujii Masao
On Thu, Jun 26, 2014 at 12:40 AM, Tom Lane  wrote:
> Fujii Masao  writes:
>> Why is IIT timeout turned on only when send_ready_for_query is true?
>> I was thinking it should be turned on every time a message is received.
>> Imagine the case where the session is in idle-in-transaction state and
>> a client gets stuck after sending Parse message and before sending Bind
>> message. This case would also cause long transaction problem and should
>> be resolved by IIT timeout. But IIT timeout that this patch adds cannot
>> handle this case because it's enabled only when send_ready_for_query is
>> true. Thought?
>
> I think you just moved the goalposts to the next county.
>
> The point of this feature, AFAICS, is to detect clients that are failing
> to issue another query or close their transaction as a result of bad
> client logic.  It's not to protect against network glitches.

If so, the document should explain that cleanly. Otherwise users may
misunderstand this parameter and try to use it to protect even long transaction
generated by network glitches or client freeze.

> Moreover, there would be no way to implement a timeout like that without
> adding a gettimeofday() call after every packet receipt, which is overhead
> we do not need and cannot afford.

Hmm.. right. I was thinking to just call enable_timeout_after() every time
message is received. But it calls gettimeofday() and causes overhead.

>  I don't think this feature should add
> *any* gettimeofday's beyond the ones that are already there.

But, ISTM that the patch adds enable_timeout_after() which calls
GetCurrentTimestamp()->gettimeofday() before receiving new message
when send_ready_for_query is true. When send_ready_for_query is true,
it's expected that next message takes time to arrive at server, so
calling gettimeofday() in that case might not be so harmful, though.

Regards,

-- 
Fujii Masao


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


Re: [HACKERS] Re: [BUGS] BUG #8673: Could not open file "pg_multixact/members/xxxx" on slave during hot_standby

2014-06-25 Thread Alvaro Herrera
Andres Freund wrote:
> On 2014-06-20 17:38:16 -0400, Alvaro Herrera wrote:

> > It seems to me that we need to keep the offsets files around until a
> > checkpoint has written the "oldest" number to WAL.  In other words we
> > need additional state in shared memory: (a) what we currently store
> > which is the oldest number as computed by vacuum (not safe to delete,
> > but it's the number that the next checkpoint must write), and (b) the
> > oldest number that the last checkpoint wrote (the safe deletion point).
> 
> Why not just WAL log truncations? If we'd emit the WAL record after
> determining the offsets page we should be safe I think? That seems like
> easier and more robust fix? And it's what e.g. the clog does.

Yes, I think this whole thing would be simpler if we just wal-logged the
truncations, like pg_clog does.  But I would like to avoid doing that
for now, and do it in 9.5 only in the future.  As a backpatchable (to
9.4/9.3) fix, I propose we do the following:

1. have vacuum update MultiXactState->oldestMultiXactId based on the
minimum value of pg_database->datminmxid.  Since this value is saved in
pg_control, it is restored from checkpoint replay during recovery.

2. Keep track of a new value, MultiXactState->lastCheckpointedOldest.
This value is updated by CreateCheckPoint in a primary server after the
checkpoint record has been flushed, and by xlog_redo in a hot standby, to
be the MultiXactState->oldestMultiXactId value that was last flushed.

3. TruncateMultiXact() no longer receives a parameter.  Files are
removed based on MultiXactState->lastCheckpointedOldest instead.  

4. call TruncateMultiXact at checkpoint time, after the checkpoint WAL
record has been flushed, and at restartpoint time (just like today).
This means we only remove files that a prior checkpoint has already
registered as being no longer necessary.  Also, if a recovery is
interrupted before end of WAL (recovery target), the files are still
present.  So we no longer truncate during vacuum.

Another consideration for (4) is that right now we're only invoking
multixact truncation in a primary when we're able to advance
pg_database.datminmxid (see vac_update_datfrozenxid).  The problem is
that after a crash and subsequent recovery, pg_database might be updated
without removing pg_multixact files; this would mean that the next
opportunity to remove files would be far in the future, when the minimum
datminmxid is advanced again.  One way to fix that would be to have
every single call to vac_update_datfrozenxid() attempt multixact
truncation, but that seems wasteful since I expect vacuuming is more
frequent than checkpointing.

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

2014-06-25 Thread Amit Kapila
On Mon, Jun 9, 2014 at 9:33 AM, Amit Kapila  wrote:
> On Sun, Jun 8, 2014 at 7:21 PM, Kevin Grittner  wrote:
> > Backend processes related to user connections still
> > performed about 30% of the writes, and this work shows promise
> > toward bringing that down, which would be great; but please don't
> > eliminate the ability to prevent write stalls in the process.
>
>
> I am planing to take some more performance data, part of which will
> be write load as well, but I am now sure if that can anyway show the
> need as mentioned by you.

After taking the performance data for write load using tpc-b with the
patch, I found that there is a regression in it.  So I went ahead and
tried to figure out the reason for same and found that after patch,
Bgwriter started flushing buffers which were required by backends
and reason was that *nextVictimBuffer* was not getting updated
properly while we are running clock sweep kind of logic (decrement
the usage count when number of buffers on freelist fall below low
threshhold value) in Bgwriter.  In HEAD, I noticed that at default
settings, BGwriter was not at all flushing any buffers which is at least
better than what my patch was doing (flushing buffers required by
backend).

So I tried to fix the issue by updating *nextVictimBuffer* in new
BGWriter logic and results are positive.

sbe - scalable buffer eviction

Select only Data
Client count/TPS64128Un-patched4523217310sbe_v3111468114521sbe_v4153137
160752
TPC-B

Client count/TPS
64128Un-patched825784sbe_v4814845

For Select Data, I am quite confident that it will improve if we introduce
nextVictimBuffer increments in BGwriter and rather it scales much better
with that change, however for TPC-B, I am getting fluctuation in data,
so not sure it has eliminated the problem.  The main difference is that in
HEAD, BGwriter never increments nextVictimBuffer during syncing the
buffers, it just notes down the current setting before start and then
proceeds sequentially.

I think it will be good if we can have a new process for moving buffers to
free list due to below reasons:

a. while trying to move buffers to freelist, it should not block due
to in between write activity.
b. The writer should not increment nextVictimBuffer and maintain
the current logic.

One significant change in this version of patch is to use a separate
spin lock to protect nextVictimBuffer rather than using BufFreelistLock.

Suggestions?

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


scalable_buffer_eviction_v4.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] postgresql.auto.conf and reload

2014-06-25 Thread Amit Kapila
On Wed, Jun 25, 2014 at 7:52 PM, Christoph Berg  wrote:
> Re: Amit Kapila 2014-06-25 <
caa4ek1+f9ztogvvw-wyj2+vt0k8_jxtziqhp8ivb7wdo1w1...@mail.gmail.com>
>
> > I think maintaining values both in postgresql.conf and by Alter System
> > is not advisable.
>
> Possibly, but then the system should be warning about all options, not
> just the restart-only ones. And it should warn at startup, not at
> reload time.

How about adding a note in Alter System so that users are aware of
such behaviour and can ensure that they don't have duplicate entries?

Clearly such warnings indicate that there are conflicting settings, so
user can take appropriate action to avoid it.

> > I am not sure if this addresses your concern completely, but I thinking
> > changing some existing mechanism (maintaining duplicate entries during
> > processing of config files) at this point might be risky.
>
> Not sure how intrusive a fix would be - collect all settings during
> config parse, and only warn once everything has been seen, instead of
> emitting warnings line-by-line.

As per my understanding, it is more appropriate to eliminate duplicate
entries.

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


Re: [HACKERS] ALTER SYSTEM RESET?

2014-06-25 Thread Amit Kapila
On Wed, Jun 25, 2014 at 9:56 PM, Vik Fearing  wrote:
> On 06/25/2014 03:04 PM, Amit Kapila wrote:
> > Currently you can achieve that by
> > "ALTER SYSTEM RESET guc = Default;".
> > However it will be good to have support for RESET as well.  I think it
> > should not be too complicated to implement that syntax, I personally
> > don't have bandwidth to it immediately, but I would like to take care
> > of it unless you or someone wants to do it by the time I get some
> > bandwidth.
>
> Would something like this suffice?

I think it will make sense if we support RESET ALL as well similar
to Alter Database .. RESET ALL syntax.  Do you see any reason
why we shouldn't support RESET ALL syntax for Alter System?

About patch:

+ | ALTER SYSTEM_P RESET var_name
+ {
+ AlterSystemStmt *n = makeNode(AlterSystemStmt);
+ n->setstmt = makeNode(VariableSetStmt);
+ n->setstmt->kind = VAR_RESET;
+ n->setstmt->name = $4;
+ $ = (Node *)n;
+ }

I think it would be better to have ALTER SYSTEM_P as generic and
then SET | RESET as different versions, something like below:

| SET reloptions
{
AlterTableCmd *n = makeNode(AlterTableCmd);
n->subtype = AT_SetRelOptions;
n->def = (Node *)$2;
$$ = (Node *)n;
}
/* ALTER TABLE  RESET (...) */
| RESET reloptions
{
AlterTableCmd *n = makeNode(AlterTableCmd);
n->subtype = AT_ResetRelOptions;
n->def = (Node *)$2;
$$ = (Node *)n;
}

Another point is that if we decide to support RESET ALL syntax, then
we might want reuse VariableResetStmt, may be by breaking into
generic and specific like we have done for generic_set.


Thanks for working on patch.


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


Re: [HACKERS] "RETURNING PRIMARY KEY" syntax extension

2014-06-25 Thread Ian Barwick

On 25/06/14 16:04, Ian Barwick wrote:

Hi

On 14/06/25 15:13, Rushabh Lathia wrote:

Hello All,

I assigned my self as reviewer of the patch. I gone through the
mail chain discussion and in that question has been raised about
the feature and its implementation, so would like to know what is
the current status of this project/patch.

Regards,


I'll be submitting a revised version of this patch very shortly.


Revised version of the patch attached, which implements the expansion
of "primary key" in the rewrite phase per Tom Lane's suggestion upthread [*]

[*] http://www.postgresql.org/message-id/28583.1402325...@sss.pgh.pa.us


Regards

Ian Barwick

--
 Ian Barwick   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/doc/src/sgml/ref/delete.sgml b/doc/src/sgml/ref/delete.sgml
index 74ea907..45295d1 100644
--- a/doc/src/sgml/ref/delete.sgml
+++ b/doc/src/sgml/ref/delete.sgml
@@ -25,7 +25,7 @@ PostgreSQL documentation
 DELETE FROM [ ONLY ] table_name [ * ] [ [ AS ] alias ]
 [ USING using_list ]
 [ WHERE condition | WHERE CURRENT OF cursor_name ]
-[ RETURNING * | output_expression [ [ AS ] output_name ] [, ...] ]
+[ RETURNING * | output_expression [ [ AS ] output_name ] [, ...] | PRIMARY KEY ]
 
  
 
@@ -182,6 +182,17 @@ DELETE FROM [ ONLY ] table_name [ *
  
 

+
+   
+PRIMARY KEY
+
+ 
+  Returns the table's primary key column(s) after each row is deleted.
+  Cannot be combined with an output_expression.
+ 
+
+   
+
   
  
 
@@ -208,7 +219,9 @@ DELETE count
clause, the result will be similar to that of a SELECT
statement containing the columns and values defined in the
RETURNING list, computed over the row(s) deleted by the
-   command.
+   command. PRIMARY KEY can be specified to return the
+   primary key value(s) for each deleted row. An error will be raised
+   if the table does not have a primary key.
   
  
 
diff --git a/doc/src/sgml/ref/insert.sgml b/doc/src/sgml/ref/insert.sgml
index a3cccb9..9fbd859 100644
--- a/doc/src/sgml/ref/insert.sgml
+++ b/doc/src/sgml/ref/insert.sgml
@@ -24,7 +24,7 @@ PostgreSQL documentation
 [ WITH [ RECURSIVE ] with_query [, ...] ]
 INSERT INTO table_name [ ( column_name [, ...] ) ]
 { DEFAULT VALUES | VALUES ( { expression | DEFAULT } [, ...] ) [, ...] | query }
-[ RETURNING * | output_expression [ [ AS ] output_name ] [, ...] ]
+[ RETURNING * | output_expression [ [ AS ] output_name ] [, ...] | PRIMARY KEY ]
 
  
 
@@ -65,7 +65,9 @@ INSERT INTO table_name [ ( RETURNING list is identical to that of the output list
-   of SELECT.
+   of SELECT. Alternatively, PRIMARY KEY will
+   return the  primary key value(s) for each inserted row. An error will
+   be raised if the table does not have a primary key.
   
 
   
@@ -186,6 +188,17 @@ INSERT INTO table_name [ ( 
 

+
+   
+PRIMARY KEY
+
+ 
+  Returns the table's primary key column(s) after each row is inserted.
+  Cannot be combined with an output_expression.
+ 
+
+   
+
   
  
 
diff --git a/doc/src/sgml/ref/update.sgml b/doc/src/sgml/ref/update.sgml
index 35b0699..27c49c4 100644
--- a/doc/src/sgml/ref/update.sgml
+++ b/doc/src/sgml/ref/update.sgml
@@ -29,7 +29,7 @@ UPDATE [ ONLY ] table_name [ * ] [
 } [, ...]
 [ FROM from_list ]
 [ WHERE condition | WHERE CURRENT OF cursor_name ]
-[ RETURNING * | output_expression [ [ AS ] output_name ] [, ...] ]
+[ RETURNING * | output_expression [ [ AS ] output_name ] [, ...] | PRIMARY KEY ]
 
  
 
@@ -58,7 +58,9 @@ UPDATE [ ONLY ] table_name [ * ] [
tables mentioned in FROM, can be computed.
The new (post-update) values of the table's columns are used.
The syntax of the RETURNING list is identical to that of the
-   output list of SELECT.
+   output list of SELECT. Alternatively, PRIMARY KEY
+   will return the  primary key value(s) for each updated row. An error will
+   be raised if the table does not have a primary key.
   
 
   
@@ -228,6 +230,17 @@ UPDATE [ ONLY ] table_name [ * ] [
  
 

+
+   
+PRIMARY KEY
+
+ 
+  Returns the table's primary key column(s) after each row is updated.
+  Cannot be combined with an output_expression.
+ 
+
+   
+
   
  
 
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 8d3d5a7..ae604e7 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -2517,6 +2517,7 @@ _copyInsertStmt(const InsertStmt *from)
 	COPY_NODE_FIELD(cols);
 	COPY_NODE_FIELD(selectStmt);
 	COPY_NODE_FIELD(returningList);
+	COPY_SCALAR_FIELD(returningPK);
 	COPY_NODE_FIELD(withClause);
 
 	return newnode;
@@ -2531,6 +2532,7 @@ _copyDeleteStmt(const DeleteStmt *from)
 	COPY_NODE_FIELD(usingClause);
 	COPY_NODE_FIELD(whereClause);
 	COPY_NODE_FIELD(returningList);
+	COPY_SCALAR_FIELD(returningPK);
 	COPY_NODE_FIELD(withClause);
 
 	return newnode;
@@ -2546,6 +2548,7 @@ _copy

Re: [HACKERS] sorting a union over inheritance vs pathkeys

2014-06-25 Thread Tom Lane
I wrote:
> Michael Glaesemann  writes:
>> -- ERROR:  could not find pathkey item to sort

> Hm ... I can reproduce that in 9.3 but it seems fine in 9.4 and HEAD.
> Don't know what's going on exactly.

Interesting --- it appears that commit
a87c729153e372f3731689a7be007bc2b53f1410 is why it works in 9.4.  I had
thought that was just improving plan quality, but it seems to also prevent
this problem.  I guess we'd better back-patch it.

I find that 9.1 through 9.3 fail with this example; it may be that it was
the addition of MergeAppend support that exposed the issue.

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] Inaccuracy in VACUUM's tuple count estimates

2014-06-25 Thread tim_wilson
Given that this seems to have slipped off the hackers radar (or in too hard
basket) I have constructed a horrible solution.

I will stop using autovacuum for this relation , I will use our own system
to monitor the relation, and I will reset pgclass.reltuples on this relation
after vacuum is done to the correct value.

I note that vacuum.c has comments in vac_update_relstat that changes to
pg_class are done without a transaction. Are there dangers of my doing an 
update pg_class set reltuples=6 where relkind='r' and
relname='my_hot_table' ? 







--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/Inaccuracy-in-VACUUM-s-tuple-count-estimates-tp5806367p5809273.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.


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


Re: [HACKERS] Set new system identifier using pg_resetxlog

2014-06-25 Thread Michael Paquier
On Thu, Jun 26, 2014 at 2:43 AM, Sawada Masahiko 
wrote:

> Hi,
>
> I send you review comment about thie patch.
>
> I found no error/warning with compling and installation.
> I have executed pg_resetxlog with some input pattern.
>
> $ initdb -D data -E UTF8 --no-locale
> $ pg_controldata data | grep "Database system identifier"
> Database system identifier:   6028907917695471865
>
> --
> $ pg_resetxlog -s -n  data | grep "Database system identifier"
> Database system identifier:   6028907917695471865
>
> The -s option does not works fine with -n option.
>
> --
> $ pg_resetxlog
> -s602890791769547186511
> data
> Transaction log reset
> $ pg_controldata data | grep "Database system identifier"
> Database system identifier:   18446744073709551615
>
> pg_resetxlog is finished successfully, but system identifier was not
> changed.
> Also I think that checking data about number of digits is needed.
>

Yep, system_identifier is a uint64, and the input you are giving here is
incompatible with that.
-- 
Michael


Re: [HACKERS] better atomics - v0.5

2014-06-25 Thread Robert Haas
On Wed, Jun 25, 2014 at 5:42 PM, Heikki Linnakangas
 wrote:
> I think having a separate file for each architecture is nice. I totally
> agree that they don't belong in src/include/storage, though. s_lock.h has
> always been misplaced there, but we've let it be for historical reasons, but
> now that we're adding a dozen new files, it's time to move them out.

I find the current organization pretty confusing, but maybe that could
be solved by better documentation of what's supposed to go in each
architecture or compiler-dependent file.

-- 
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] better atomics - v0.5

2014-06-25 Thread Robert Haas
On Wed, Jun 25, 2014 at 4:36 PM, Andres Freund  wrote:
> Since it better be legal to manipulate a atomic variable while holding a
> spinlock we cannot simply use an arbitrary spinlock as backing for
> atomics. That'd possibly cause us to wait on ourselves or cause
> deadlocks.

I think that's going to fall afoul of Tom's previously-articulated "no
loops inside spinlocks" rule.  Most atomics, by nature, are
loop-until-it-works.

>> How much would we lose if we supported compiler intrinsics on
>> versions of GCC and MSVC that have it and left everything else to
>> future patches?
>
> The discussion so far seemed pretty clear that we can't regress somewhat
> frequently used platforms. And dropping support for all but msvc and gcc
> would end up doing that. We're going to have to do the legword for the
> most common platforms... Note that I didn't use assembler for those, but
> relied on intrinsics...

We can't *regress* somewhat-frequently used platforms, but that's
different from saying we've got to support *new* facilities on those
platforms.  Essentially the entire buildfarm is running either gcc,
some Microsoft compiler, or a compiler that's supports the same
atomics intrinsics as gcc i.e. icc or clang.  Some of those compilers
may be too old to support the atomics intrinsics, and there's one Sun
Studio animal, but I don't know that we need to care about those
things in this patch...

...unless of course the atomics fallbacks in this patch are
sufficiently sucky that anyone who ends up using those is going to be
sad.  Then the bar is a lot higher.  But if that's the case then I
wonder if we're really on the right course here.

> I added the x86 inline assembler because a fair number of buildfarm
> animals use ancient gcc's and because I could easily test it. It's also
> more efficient for gcc < 4.6. I'm not wedded to keeping it.

Hmm. gcc 4.6 is only just over a year old, so if pre-4.6
implementations aren't that good, that's a pretty good argument for
keeping our own implementations around.  :-(

-- 
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] makeAndExpr(), etc. confined to gram.y?

2014-06-25 Thread Amit Langote
On Thu, Jun 26, 2014 at 12:46 AM, Tom Lane  wrote:
> Amit Langote  writes:
>> Yeah, that is true. Sorry, I am unaware as to how generic make*
>> functions in gram.y are and how they differ from those in makefuncs.c.
>
>> So, use of make* family of functions outside parser is their abuse in
>> some way? Anything that needs to use these functions should somehow be
>> accomplished in parser perhaps. For example, duplicate/redundant CHECK
>> expressions elimination and such?
>
> Well, the larger point here is that those functions are specific to
> gram.y's problem of constructing multi-AND(OR) structures during a series
> of binary production actions.  I don't see that there's any use for them
> elsewhere, and the way that they modify the input structures wouldn't
> necessarily be safe anywhere else either.
>

I see. Thanks for clarifying.

--
Amit


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


Re: [HACKERS] Extended Prefetching using Asynchronous IO - proposal and patch

2014-06-25 Thread John Lumby
My cut'n'pasting failed me at one point corrected below.


> discussion about what is the difference between a synchronous read
> versus an asynchronous read as far as non-originator waiting on it is 
> concerned.
>
> I thought a bit more about this.   There are currently two differences,
> one of which can easily be changed and one not so easy.
>
> 1) The current code,  even with sigevent,  still makes the non-originator 
> waiter
>  call aio_error on the originator's aiocb to get the completion code.
>  For sigevent variation,  easily changed to have the originator 
> always call aio_error
>  (from its CHECK_INTERRUPTS or from FIleCompleteaio)
>  and store that in the BAiocb.
>  My idea of why not to do that  was that,  by having the 
> non-originator check the aiocb,
> this would allow the waiter to proceed sooner.   But for a different 
> reason it actually
>  doesn't.   (The non-originator must still wait for the LWlock 
> release)
>
>   2)   Buffer pinning and  returning the BufferAiocb to the free list
> With synchronous IO,each backend that calls a ReadBuffer must pin 
> the buffer
> early in the process.
> With asynchronous IO,initially only the originator gets the pin
> (and that is during PrefetchBuffer,  not Readbuffer)
>  When the aio completes and some backend checks that completion,
> then the backend has various responsibilities:
>
>.   pin the buffer if it did not already have one (from 
> prefetch)
>.  if it was the last such backend to make that check
>   (amongst the cohort waiting on it)
>then XXpin the buffer if it did not already have one 
> (from prefetch)

then return the BufferAiocb to the free list


  

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


Re: [HACKERS] sorting a union over inheritance vs pathkeys

2014-06-25 Thread Tom Lane
Michael Glaesemann  writes:
> I’ve come across an issue when creating a union over tables which includes 
> inheritance:
> CREATE TABLE events (event_id INT NOT NULL);
> -- CREATE TABLE
> CREATE UNIQUE INDEX events_event_id_key ON events (event_id);
> -- CREATE INDEX

> CREATE TABLE legacy_events (event_id INT NOT NULL);
> -- CREATE TABLE
> CREATE UNIQUE INDEX legacy_events_event_id_key ON legacy_events (event_id);
> -- CREATE INDEX

> CREATE TABLE events_2 () INHERITS (events);
> -- CREATE TABLE
> -- this index isn't necessary to reproduce the error
> CREATE UNIQUE INDEX events_2_event_id_key ON events_2 (event_id);
> -- CREATE INDEX

> SELECT event_id
>  FROM (SELECT event_id
>  FROM events
>UNION ALL
>SELECT event_id
>  FROM legacy_events) _
>  ORDER BY event_id;
> -- ERROR:  could not find pathkey item to sort

Hm ... I can reproduce that in 9.3 but it seems fine in 9.4 and HEAD.
Don't know what's going on exactly.

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] better atomics - v0.5

2014-06-25 Thread Alvaro Herrera
Heikki Linnakangas wrote:
> On 06/25/2014 11:36 PM, Andres Freund wrote:
> >
> >>>- I completely loathe the file layout you've proposed in
> >>>src/include/storage.  If we're going to have separate #include files
> >>>for each architecture (and I'd rather we didn't go that route, because
> >>>it's messy and unlike what we have now), then shouldn't that stuff go
> >>>in src/include/port or a new directory src/include/port/atomics?
> >>>Polluting src/include/storage with a whole bunch of files called
> >>>atomics-whatever.h strikes me as completely ugly, and there's a lot of
> >>>duplicate code in there.
> >I don't mind moving it somewhere else and it's easy enough to change.
> 
> I think having a separate file for each architecture is nice. I
> totally agree that they don't belong in src/include/storage, though.
> s_lock.h has always been misplaced there, but we've let it be for
> historical reasons, but now that we're adding a dozen new files,
> it's time to move them out.

We also have a bunch of files in src/backend/storage/lmgr, both current
and introduced by this patch, which would be good to relocate.  (Really,
how did lmgr/ end up in storage/ in the first place?)

-- 
Á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] sorting a union over inheritance vs pathkeys

2014-06-25 Thread Michael Glaesemann
I’ve come across an issue when creating a union over tables which includes 
inheritance:

CREATE TABLE events (event_id INT NOT NULL);
-- CREATE TABLE
CREATE UNIQUE INDEX events_event_id_key ON events (event_id);
-- CREATE INDEX

CREATE TABLE legacy_events (event_id INT NOT NULL);
-- CREATE TABLE
CREATE UNIQUE INDEX legacy_events_event_id_key ON legacy_events (event_id);
-- CREATE INDEX

CREATE TABLE events_2 () INHERITS (events);
-- CREATE TABLE
-- this index isn't necessary to reproduce the error
CREATE UNIQUE INDEX events_2_event_id_key ON events_2 (event_id);
-- CREATE INDEX

SELECT event_id
 FROM (SELECT event_id
 FROM events
   UNION ALL
   SELECT event_id
 FROM legacy_events) _
 ORDER BY event_id;
-- ERROR:  could not find pathkey item to sort

It’ll work if the indexes are removed. Using PRIMARY KEY in lieu of NOT NULL 
and UNIQUE indexes still exhibits the issue.

I’ve seen this in 9.2.8 and 9.3.4. I haven’t tested this in 9.4 or earlier than 
9.2.

Any thoughts?

Michael Glaesemann
grzm seespotcode net



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


Re: [HACKERS] better atomics - v0.5

2014-06-25 Thread Heikki Linnakangas

On 06/25/2014 11:36 PM, Andres Freund wrote:



>- I completely loathe the file layout you've proposed in
>src/include/storage.  If we're going to have separate #include files
>for each architecture (and I'd rather we didn't go that route, because
>it's messy and unlike what we have now), then shouldn't that stuff go
>in src/include/port or a new directory src/include/port/atomics?
>Polluting src/include/storage with a whole bunch of files called
>atomics-whatever.h strikes me as completely ugly, and there's a lot of
>duplicate code in there.

I don't mind moving it somewhere else and it's easy enough to change.


I think having a separate file for each architecture is nice. I totally 
agree that they don't belong in src/include/storage, though. s_lock.h 
has always been misplaced there, but we've let it be for historical 
reasons, but now that we're adding a dozen new files, it's time to move 
them out.


- Heikki


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


Re: [HACKERS] Extended Prefetching using Asynchronous IO - proposal and patch

2014-06-25 Thread Andres Freund
On 2014-06-26 00:08:48 +0300, Heikki Linnakangas wrote:
> LWLocks are implemented with semaphores, so if you can increment a semaphore
> in the signal handler / callback thread, then in theory you should be able
> to release a LWLock.

I don't think that's a convincing argument even if semop et al were
signal safe. LWLocks also use spinlocks and it's a fairly bad idea to do
anything with the same spinlock both inside and outside a signal
handler.
I don't think we're going to get lwlock.c/LWLockRelease to work
reasonably from a signal handler without a lot of work.

> On Linux at least we use System V semaphores, which are (unsurpisingly) not
> listed as safe for using in a signal handler. See list Async-signal-safe
> functions in signal(7) man page. The function used to increment a POSIX
> semaphore, sem_post(), is in the list, however.

Heh, just wrote the same after reading the beginning of your email ;)

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] Extended Prefetching using Asynchronous IO - proposal and patch

2014-06-25 Thread Heikki Linnakangas

On 06/25/2014 09:20 PM, Claudio Freire wrote:

On Tue, Jun 24, 2014 at 12:08 PM, John Lumby  wrote:

The question is, if you receive the notification of the I/O completion
using a signal or a thread, is it safe to release the lwlock from the
signal handler or a separate thread?


In the forthcoming  new version of the patch that uses sigevent,
the originator locks a LWlock associated with that BAaiocb eXclusive,
and ,   when signalled,  in the signal handler it places that LWlock
on a process-local queue of LWlocks awaiting release.
(No, It cannot be safely released inside the signal handler or in a
separate thread). Whenever the mainline passes a CHECK_INTERRUPTS macro
and at a few additional points in bufmgr,  the backend walks this process-local
queue and releases those LWlocks.This is also done if the originator
itself issues a ReadBuffer,  which is the most frequent case in which it
is released.


I suggest using a semaphore instead.

Semaphores are supposed to be incremented/decremented from multiple
threads or processes already. So, in theory, the callback itself
should be able to do it.


LWLocks are implemented with semaphores, so if you can increment a 
semaphore in the signal handler / callback thread, then in theory you 
should be able to release a LWLock. You'll need some additional 
synchronization within the same process, to make sure you don't release 
a lock in signal handler while you're just doing the same thing in the 
main thread. I'm not sure I want to buy into the notion that 
LWLockRelease must be generally safe to call from a signal handler, but 
maybe it's possible to have a variant of it that is.


On Linux at least we use System V semaphores, which are (unsurpisingly) 
not listed as safe for using in a signal handler. See list 
Async-signal-safe functions in signal(7) man page. The function used to 
increment a POSIX semaphore, sem_post(), is in the list, however.


- Heikki



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


Re: [HACKERS] RLS Design

2014-06-25 Thread Dean Rasheed
On 25 June 2014 16:44, Robert Haas  wrote:
> On Tue, Jun 24, 2014 at 8:49 PM, Stephen Frost  wrote:
>> Let's try to outline what this would look like then.
>>
>> Taking your approach, we'd have:
>>
>> CREATE POLICY p1;
>> CREATE POLICY p2;
>>
>> ALTER TABLE t1 SET POLICY p1 TO t1_p1_quals;
>> ALTER TABLE t1 SET POLICY p2 TO t1_p2_quals;
>
> This seems like a very nice, flexible framework.
>
>> GRANT SELECT ON TABLE t1 TO role1 USING p1;
>> GRANT SELECT ON TABLE t1 TO role2 USING p2;
>
> Instead of doing it this way, we could instead do:
>
> ALTER ROLE role1 ADD POLICY p1;
> ALTER ROLE role2 ADD POLICY p2;
>
> We could possibly allow multiple policies to be set for the same user,
> but given an error (or OR the quals together) if there are conflicting
> policies for the same table.  A user with no policies would see
> everything to which they've been granted access.
>

I'm a bit uneasy about allowing overlapping policies like this,
because I think it is more likely to lead to unintended consequences
than solve real use cases. For example, suppose you define policies p1
and p2 and set them up on table t1, and you grant role1 permissions on
t1 and allow role1 the use of policy p1. Then you set up policy p2 on
another table t2, and decide you want to allow role1 access to t2
using this policy. The only way to do it is to add p2 to role1, but
doing so also then gives role1 access to t1 using p2, which might not
have been what you intended.

With the GRANT ... USING policy syntax, you have greater flexibility
to pick and choose which policies each user has permission to use with
each table. To me at least, that seems much less error prone, since
you are being much more explicit about exactly what privileges you are
granting. The ALTER ROLE ... ADD POLICY syntax is potentially adding a
whole bunch of extra privileges to the role, and you have to work
quite hard to see exactly what it's adding.


> To support different policies on different operations, you could have
> something like:
>
> ALTER TABLE t1 SET POLICY p1 ON INSERT TO t1_p1_quals;
>
> Without the ON clause, it would establish the given policy for all operations.
>

Yes, that makes sense. But as I was arguing above, I think the ACLs
should be attached to the specific RLS policy identified uniquely by
(table, policy, command). So, for example, if you did

ALTER TABLE t1 SET POLICY p1 ON SELECT TO t1_p1_sel_quals;
ALTER TABLE t1 SET POLICY p1 ON UPDATE TO t1_p1_upd_quals;

you could also do

GRANT SELECT ON TABLE t1 TO role1 USING p1;
GRANT UPDATE ON TABLE t1 TO role1 USING p1;

but it would be an error to do

GRANT DELETE ON TABLE t1 TO role1 USING p1;

because there is no p1 delete policy for t1;

Regards,
Dean


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


Re: [HACKERS] better atomics - v0.5

2014-06-25 Thread Andres Freund
Hi,

On 2014-06-25 15:54:37 -0400, Robert Haas wrote:
> - The new argument to s_init_lock_sema() isn't used.

It's used when atomics fallback to spinlocks which fall back to
semaphores. c.f. atomics.c.

Since it better be legal to manipulate a atomic variable while holding a
spinlock we cannot simply use an arbitrary spinlock as backing for
atomics. That'd possibly cause us to wait on ourselves or cause
deadlocks.

> - Are the regression tests in regress.c likely to catch anything?
> Really?

They already cought several bugs. If the operations were immediately
widely used they probably wouldn't be necessary.

> - The patch adds a test for PGAC_HAVE_GCC__SYNC_INT32_ATOMICS, which
> seems to be testing for __builtin_constant_p.  I don't see that being
> used in the patch anywhere, though; and also, why doesn't the naming
> match?

Uh. that's a rebase screweup. Should entirely be gone.

> - s_lock.h adds some fallback code to use atomics to define TAS on
> platforms where GCC supports atomics but where we do not have a
> working TAS implementation.  However, this supposed fallback code
> defines HAS_TEST_AND_SET unconditionally, so I think it will get used
> even if we don't have (or have disabled via configure) atomics.

Hm. Good point. It should protect against that.

> - I completely loathe the file layout you've proposed in
> src/include/storage.  If we're going to have separate #include files
> for each architecture (and I'd rather we didn't go that route, because
> it's messy and unlike what we have now), then shouldn't that stuff go
> in src/include/port or a new directory src/include/port/atomics?
> Polluting src/include/storage with a whole bunch of files called
> atomics-whatever.h strikes me as completely ugly, and there's a lot of
> duplicate code in there.

I don't mind moving it somewhere else and it's easy enough to change.

> - What's the point of defining pg_read_barrier() to be
> pg_read_barrier_impl() and pg_write_barrier() to be
> pg_write_barrier_impl() and so forth?  That seems like unnecessary
> notation.

We could forgo it for pg_read_barrier() et al, but for the actual
atomics it's useful because it allows to put common checks in the !impl
routines.

> - Should SpinlockSemaInit() be assigning SpinlockSemas() to a local
> variable instead of re-executing it every time?  Granted, the compiler
> might inline this somehow, but...

I sure hope it will, but still a good idea.

> More generally, I'm really wondering if you're making the
> compare-and-swap implementation a lot more complicated than it needs
> to be.

Possible.

> How much would we lose if we supported compiler intrinsics on
> versions of GCC and MSVC that have it and left everything else to
> future patches?

The discussion so far seemed pretty clear that we can't regress somewhat
frequently used platforms. And dropping support for all but msvc and gcc
would end up doing that. We're going to have to do the legword for the
most common platforms... Note that I didn't use assembler for those, but
relied on intrinsics...

> I suspect this patch could be about 20% of its current size and give
> us everything that we need.

I think the only way to do that would be to create a s_lock.h like
maze. Which imo is a utterly horrible idea. I'm pretty sure there'll be
more assembler implementations for 9.5 and unless we think ahead of how
to structure that we'll create something bad.
I also think that a year or so down the road we're going to support more
operations. E.g. optional support for a 16byte CAS can allow for some
awesome things when you want to do lockless stuff on a 64bit platform.

I think there's chances for reducing the size by merging i386/amd64,
that looked better earlier than it does now (due to the addition of the
inline assembler).

> I've previously
> opposed discarding s_lock.h on the grounds that it's extremely well
> battle-tested, and if we change it, we might introduce subtle bugs
> that are dependent on GCC versions and such.

I think we should entirely get rid of s_lock.h. It's an unmaintainable
mess. That's what I'd done in an earlier version of the patch, but you'd
argued against it. I think you're right in that it shouldn't be done in
the same patch and possibly not even in the same cycle, but I think we
better do it sometime not too far away.
I e.g. don't see how we can guarantee sane barrier semantics with the
current implementation.

> But now that compiler
> intrinsics are relatively common, I don't really see any reason for us
> to provide our own assembler versions of *new* primitives we want to
> use.  As long as we have some kind of wrapper functions or macros, we
> retain the *option* to add workarounds for compiler bugs or lack of
> compiler support on platforms, but it seems an awful lot simpler to me
> to start by assuming that that the compiler will DTRT, and only roll
> our own if that proves to be necessary.  It's not like our hand-rolled
> implementations couldn't have bugs - which is differen

Re: [HACKERS] PostgreSQL for VAX on NetBSD/OpenBSD

2014-06-25 Thread John Klos

Well, the fact that initdb didn't produce a working configuration and
that make installcheck failed to work properly are bad.  But, yeah,
it's not totally broken.


I think it did create a working configuration (with the exception of 
postgresql.conf), because I can run psql and do stuff on the command line:


psql --username=pgsql postgres
psql (9.3.4)
Type "help" for help.

postgres=# CREATE DATABASE test;
CREATE DATABASE
postgres=# CREATE USER testuser WITH PASSWORD 'test';
CREATE ROLE
postgres=# GRANT ALL PRIVILEGES ON DATABASE test to testuser;
GRANT
postgres=# CREATE SCHEMA testschema;
CREATE SCHEMA
postgres=# CREATE TABLE testschema.testtable (testserial serial PRIMARY 
KEY, testchar varchar (100) NOT NULL);

CREATE TABLE

I don't know enough to really test this. Can you recommend a simple script 
to do some PostgreSQL testing?


John


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


Re: [HACKERS] psql: show only failed queries

2014-06-25 Thread Alvaro Herrera
ECHO_HIDDEN?

-- 
Á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] PostgreSQL for VAX on NetBSD/OpenBSD

2014-06-25 Thread Robert Haas
On Wed, Jun 25, 2014 at 1:58 PM, John Klos  wrote:
>> Well, the fact that initdb didn't produce a working configuration and
>> that make installcheck failed to work properly are bad.  But, yeah,
>> it's not totally broken.
>
> I think it did create a working configuration (with the exception of
> postgresql.conf), because I can run psql and do stuff on the command line:

Yeah, but postgresql.conf should not have require manual tweaking...

> I don't know enough to really test this. Can you recommend a simple script
> to do some PostgreSQL testing?

Well, this is what 'make installcheck' is for...

-- 
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] better atomics - v0.5

2014-06-25 Thread Robert Haas
On Wed, Jun 25, 2014 at 1:14 PM, Andres Freund  wrote:
> [sorry for the second copy Robert]
>
> Attached is a new version of the atomic operations patch. Lots has
> changed since the last post:
>
> * gcc, msvc work. acc, xlc, sunpro have blindly written support which
>   should be relatively easy to fix up. All try to implement TAS, 32 bit
>   atomic add, 32 bit compare exchange; some do it for 64bit as well.
>
> * x86 gcc inline assembly means that we support every gcc version on x86
>   >= i486; even when the __sync APIs aren't available yet.
>
> * 'inline' support isn't required anymore. We fall back to
>   ATOMICS_INCLUDE_DEFINITIONS/STATIC_IF_INLINE etc. trickery.
>
> * When the current platform doesn't have support for atomic operations a
>   spinlock backed implementation is used. This can be forced using
>   --disable-atomics.
>
>   That even works when semaphores are used to implement spinlocks (a
>   separate array is used there to avoid nesting problems). It contrast
>   to an earlier implementation this even works when atomics are mapped
>   to different addresses in individual processes (think dsm).
>
> * s_lock.h falls back to the atomics.h provided APIs iff it doesn't have
>   native support for the current platform. This can be forced by
>   defining USE_ATOMICS_BASED_SPINLOCKS. Due to generic compiler
>   intrinsics based implementations that should make it easier to bring
>   up postgres on different platfomrs.
>
> * I tried to improve the documentation of the facilities in
>   src/include/storage/atomics.h. Including documentation of the various
>   barrier semantics.
>
> * There's tests in regress.c that get call via a SQL function from the
>   regression tests.
>
> * Lots of other details changed, but that's the major pieces.

Review:

- The changes to spin.c include unnecessary whitespace adjustments.
- The new argument to s_init_lock_sema() isn't used.
- "on top" is two words, not one ("ontop").
- The changes to pg_config_manual.h add a superfluous blank line.
- Are the regression tests in regress.c likely to catch anything?  Really?
- The patch adds a test for PGAC_HAVE_GCC__SYNC_INT32_ATOMICS, which
seems to be testing for __builtin_constant_p.  I don't see that being
used in the patch anywhere, though; and also, why doesn't the naming
match?
- s_lock.h adds some fallback code to use atomics to define TAS on
platforms where GCC supports atomics but where we do not have a
working TAS implementation.  However, this supposed fallback code
defines HAS_TEST_AND_SET unconditionally, so I think it will get used
even if we don't have (or have disabled via configure) atomics.
- I completely loathe the file layout you've proposed in
src/include/storage.  If we're going to have separate #include files
for each architecture (and I'd rather we didn't go that route, because
it's messy and unlike what we have now), then shouldn't that stuff go
in src/include/port or a new directory src/include/port/atomics?
Polluting src/include/storage with a whole bunch of files called
atomics-whatever.h strikes me as completely ugly, and there's a lot of
duplicate code in there.
- What's the point of defining pg_read_barrier() to be
pg_read_barrier_impl() and pg_write_barrier() to be
pg_write_barrier_impl() and so forth?  That seems like unnecessary
notation.
- Should SpinlockSemaInit() be assigning SpinlockSemas() to a local
variable instead of re-executing it every time?  Granted, the compiler
might inline this somehow, but...

More generally, I'm really wondering if you're making the
compare-and-swap implementation a lot more complicated than it needs
to be.  How much would we lose if we supported compiler intrinsics on
versions of GCC and MSVC that have it and left everything else to
future patches?  I suspect this patch could be about 20% of its
current size and give us everything that we need.  I've previously
opposed discarding s_lock.h on the grounds that it's extremely well
battle-tested, and if we change it, we might introduce subtle bugs
that are dependent on GCC versions and such.  But now that compiler
intrinsics are relatively common, I don't really see any reason for us
to provide our own assembler versions of *new* primitives we want to
use.  As long as we have some kind of wrapper functions or macros, we
retain the *option* to add workarounds for compiler bugs or lack of
compiler support on platforms, but it seems an awful lot simpler to me
to start by assuming that that the compiler will DTRT, and only roll
our own if that proves to be necessary.  It's not like our hand-rolled
implementations couldn't have bugs - which is different from s_lock.h,
where we have evidence that the exact coding in that file does or did
work on those platforms.

Similarly, I would rip out - or separate into a separate patch - the
code that tries to use atomic operations to implement TAS if we don't
already have an implementation in s_lock.h.  That might be worth
doing, if there are any common architectures that do

Re: [Fwd: Re: [HACKERS] proposal: new long psql parameter --on-error-stop]

2014-06-25 Thread Pavel Stehule
Hello

Here is next update


2014-06-25 17:17 GMT+02:00 MauMau :

> OK, let me help you, though I'm only a Japanese who is never confident in
> my English.
>
> (1)
> As Fujii-san pointed out, could you add explanation for --help-variables
> in doc/src/sgml/ref/psqlref.sgml?
>
>
> (2)
>
> + printf(_("  --help-variables list of available configuration
> variables (options), then exit\n"));
>
> should better be:
>
> + printf(_("  --help-variables show A list of all specially
> treated variables, then exit\n"));
>
> This follows the psql manual page.  Similarly,
>
> + printf(_("List of variables (options) for use from command line.\n"));
>
> should be:
>
> + printf(_("List of specially treated variables.\n"));
>
>
> (3)
> + printf(_("  ECHO   control what input can be writtent to
> standard output [all, queries]\n"));
>
> "writtent" should be "written".  "controls" should be "control" like other
> options.
>
>
> (4)
> + printf(_("  ECHO_HIDDENdisplay internal queries (same as -E
> option)\n"));
>
> should better be:
>
> + printf(_("  ECHO_HIDDENdisplay internal queries executed by
> backslash commands\n"));
>
> I think "(same as ...)" can be omitted to keep the description short.  If
> you want to retain it, other variables should also accompany similar
> description, such as -a for ECHO.
>
>
> (5)
> + printf(_("  FETCH_COUNTfetch many rows at a time (use less
> memory) (default 0 unlimited)\n"));
>
> should better be:
>
> + printf(_("  FETCH_COUNTthe number of result rows to fetch and
> display at a time (default: 0=unlimited)\n"));
>
>
> (6)
> + printf(_("  HISTCONTROLwhen set, controls history list
> [ignorespace, ignoredups, ignoreboth]\n"));
>
> should better be:
>
> + printf(_("  HISTCONTROLcontrol history list [ignorespace,
> ignoredups, ignoreboth]\n"));
>
>
> (7)
> + printf(_("  USER   the database user currently
> connected\n"));
>
> should add "as" at the end:
>
> + printf(_("  USER   the database user currently connected
> as\n"));
>
>
> (8)
> "Printing options" section lack the following ones described in psql
> manual:
>
> columns
> expanded (or x)
> footer
> numericlocale
> tableattr (or T)
>

fixed


>
>
> (9)
> + printf(_("\nEnvironment options:\n"));
>
> should be ""Environment variables".  And this section lacks description
> for Windows, such as:
>
> + printf(_("  NAME=VALUE [NAME=VALUE] psql ...\n  or \\setenv NAME [VALUE]
> in interactive mode\n\n"));
>
> + printf(_("  PGPASSFILE password file (default ~/.pgpass)\n"));
>

??? -

Regards

Pavel


>
> Regards
> MauMau
>
>
commit 4d77ccf19ac77dc0f43a58f4d2b74d4aebed871d
Author: Pavel Stehule 
Date:   Mon Jun 23 19:38:41 2014 +0200

without comments

diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index ee6ec3a..6a172dc 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -556,6 +556,15 @@ EOF
   
 
 
+
+  --help-variables
+  
+  
+  Show help about psql variables,
+  and exit.
+  
+  
+
   
  
 
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 3aa3c16..1304e1a 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -78,12 +78,13 @@ usage(void)
 	printf(_("  -f, --file=FILENAME  execute commands from file, then exit\n"));
 	printf(_("  -l, --list   list available databases, then exit\n"));
 	printf(_("  -v, --set=, --variable=NAME=VALUE\n"
-			 "   set psql variable NAME to VALUE\n"));
+			 "   set psql variable NAME to VALUE e.g.: -v ON_ERROR_STOP=1\n"));
 	printf(_("  -V, --versionoutput version information, then exit\n"));
 	printf(_("  -X, --no-psqlrc  do not read startup file (~/.psqlrc)\n"));
 	printf(_("  -1 (\"one\"), --single-transaction\n"
 			 "   execute as a single transaction (if non-interactive)\n"));
 	printf(_("  -?, --help   show this help, then exit\n"));
+	printf(_("  --help-variables show a list of all specially treated variables, then exit\n"));
 
 	printf(_("\nInput and output options:\n"));
 	printf(_("  -a, --echo-all   echo all input from script\n"));
@@ -279,6 +280,92 @@ slashUsage(unsigned short int pager)
 }
 
 
+/*
+ * show list of available variables (options) from command line
+ */
+void
+help_variables(void)
+{
+	printf(_("List of specially treated variables.\n"));
+
+	printf(_("psql variables:\n"));
+	printf(_("Usage:\n"));
+	printf(_("  psql --set=NAME=VALUE\n  or \\set NAME VALUE in interactive mode\n\n"));
+
+	printf(_("  AUTOCOMMIT successful SQL commands are automatically committed\n"));
+	printf(_("  COMP_KEYWORD_CASE  determines which letter case to use when completing an SQL key word\n"));
+	printf(_("  DBNAME name of currently connected database\n"));
+	printf(_("  ECHO   control what input can be written 

Re: [HACKERS] RLS Design

2014-06-25 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote:
> On Tue, Jun 24, 2014 at 8:49 PM, Stephen Frost  wrote:
> > Let's try to outline what this would look like then.
> >
> > Taking your approach, we'd have:
> >
> > CREATE POLICY p1;
> > CREATE POLICY p2;
> >
> > ALTER TABLE t1 SET POLICY p1 TO t1_p1_quals;
> > ALTER TABLE t1 SET POLICY p2 TO t1_p2_quals;
> 
> This seems like a very nice, flexible framework.
> 
> > GRANT SELECT ON TABLE t1 TO role1 USING p1;
> > GRANT SELECT ON TABLE t1 TO role2 USING p2;
> 
> Instead of doing it this way, we could instead do:
> 
> ALTER ROLE role1 ADD POLICY p1;
> ALTER ROLE role2 ADD POLICY p2;
> 
> We could possibly allow multiple policies to be set for the same user,
> but given an error (or OR the quals together) if there are conflicting
> policies for the same table.  A user with no policies would see
> everything to which they've been granted access.

Ok, I like that as it means that "normal" GRANTs, etc, remain the same
and are just constrained by RLS when there is an RLS policy on the
table, which I believe is really the right approach.

> To support different policies on different operations, you could have
> something like:
> 
> ALTER TABLE t1 SET POLICY p1 ON INSERT TO t1_p1_quals;
> 
> Without the ON clause, it would establish the given policy for all operations.

Right, this makes sense also and is similar to what we were angling
towards initially.  I'll think further on this and propose a catalog
structure and try to delve into the semantics of query operations, etc.

One issue that occurs to me is trying to think through how to address
the plancache invalidation, such that we are not invalidating constantly
but also setting the correct quals for the current query.  We had gone
down a road where we saved a plan for each role under which a query was
run but then ripped that out because the RLS policy would handle the
per-role issues (modulo whether RLS should be enabled or not).  This
approach means that we'd have to bring back the notion of per-role plan
cacheing.  I'm not against that, just making note of it.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] ALTER SYSTEM RESET?

2014-06-25 Thread Fabrízio de Royes Mello
On Wed, Jun 25, 2014 at 1:26 PM, Vik Fearing  wrote:

> On 06/25/2014 03:04 PM, Amit Kapila wrote:
> > On Wed, Jun 25, 2014 at 6:20 PM, Christoph Berg  > > wrote:
> >>
> >> Hi,
> >>
> >> is there a reason there's no ALTER SYSTEM RESET?
> >>
> >> The natural idiom to reset SET statements is "RESET guc;", I don't
> >> think "SET guc = default;" is in use much, so "ALTER SYSTEM RESET guc;"
> >> would be the natural way to try.
> >
> > Currently you can achieve that by
> > "ALTER SYSTEM RESET guc = Default;".
> > However it will be good to have support for RESET as well.  I think it
> > should not be too complicated to implement that syntax, I personally
> > don't have bandwidth to it immediately, but I would like to take care
> > of it unless you or someone wants to do it by the time I get some
> > bandwidth.
>
> Would something like this suffice?
>
>
Is fine to me...

-- 
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello


Re: [HACKERS] pg_filedump for 9.4?

2014-06-25 Thread Tom Lane
=?UTF-8?Q?Fabr=C3=ADzio_de_Royes_Mello?=  writes:
> I'm thinking in run "pgindent" to better organize the source code... What
> do you think?

Would like that, but I'm not sure what pgindent will do with the //
comments.  It's been on my to-do list to switch all the comments to C89
style and then pgindent it, but I don't see myself getting to that in this
decade :-(

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] psql: show only failed queries

2014-06-25 Thread Pavel Stehule
2014-06-25 12:32 GMT+02:00 Samrat Revagade :

> Hi Pavel,
>
> After applying patch, on error condition it displays error message two
> times as follows:
>
> ERROR:  column "abc" does not exist at character 23
> STATEMENT:  insert into ax
> values(abc);
> psql:a.sql:7: ERROR:  column "abc" does not exist
> LINE 2: values(abc);
>
>
> user may confuse because of repeated error messages. so I think its better
> to display only one message, one of the possible ways is as follows:
>
> ERROR:  column "abc" does not exist at character 23
> STATEMENT:  insert into ax
> values(abc);
>
>
> Am I missing something ?
>

LINE info is a part of error message and should be eliminated by terse mode


[pavel@localhost ~]$ psql -v ECHO=error  -f test.sql postgres > /dev/null
psql:test.sql:4: ERROR:  syntax error at or near ";"
LINE 2: 10 + ;
 ^
psql:test.sql:4: STATEMENT:  select
10 + ;
psql:test.sql:8: ERROR:  syntax error at end of input
LINE 2: 30 +
 ^
psql:test.sql:8: STATEMENT:  select
30 +

but you can switch to terse mode:

[pavel@localhost ~]$ psql -v ECHO=error -v VERBOSITY=terse -f test.sql
postgres > /dev/null
psql:test.sql:4: ERROR:  syntax error at or near ";" at character 13
psql:test.sql:4: STATEMENT:  select
10 + ;
psql:test.sql:8: ERROR:  syntax error at end of input at character 13
psql:test.sql:8: STATEMENT:  select
30 +

What is what you would

I am sending updated patch - buggy statement is printed via more logical
psql_error function instead printf

Regards

Pavel


>
>
>
> On Wed, Jun 4, 2014 at 9:52 PM, Pavel Stehule 
> wrote:
>
>>
>>
>>
>> 2014-06-04 18:16 GMT+02:00 Peter Eisentraut :
>>
>> On 6/4/14, 11:54 AM, Pavel Stehule wrote:
>>> > updated patch - only one change: query is prefixed by "QUERY:  "
>>>
>>> In the backend server log, this is called "STATEMENT:  ".
>>>
>>
>> good idea
>>
>> updated patch
>>
>> Pavel
>>
>>
>> --
>> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgsql-hackers
>>
>>
>
>
> --
> Regards,
>
> Samrat Revgade
>
commit b269613e0b261a7a5cfea35594299a0dd093682d
Author: Pavel Stehule 
Date:   Wed Jun 25 20:45:30 2014 +0200

initial

diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index ee6ec3a..cf0e78b 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -2812,7 +2812,8 @@ bar
 queries,
 psql merely prints all queries as
 they are sent to the server. The switch for this is
--e.
+-e. If set to error then only
+failed queries are displayed.
 
 
   
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 60169a2..69860d7 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -995,6 +995,9 @@ SendQuery(const char *query)
 		results = NULL;			/* PQclear(NULL) does nothing */
 	}
 
+	if (!OK && pset.echo == PSQL_ECHO_ERROR)
+		psql_error("STATEMENT:  %s\n", query);
+
 	/* If we made a temporary savepoint, possibly release/rollback */
 	if (on_error_rollback_savepoint)
 	{
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 0a60e68..e4a18f0 100644
--- a/src/bin/psql/settings.h
+++ b/src/bin/psql/settings.h
@@ -31,6 +31,7 @@ typedef enum
 {
 	PSQL_ECHO_NONE,
 	PSQL_ECHO_QUERIES,
+	PSQL_ECHO_ERROR,
 	PSQL_ECHO_ALL
 } PSQL_ECHO;
 
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 45653a1..b59bd59 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -720,6 +720,8 @@ echo_hook(const char *newval)
 		pset.echo = PSQL_ECHO_NONE;
 	else if (strcmp(newval, "queries") == 0)
 		pset.echo = PSQL_ECHO_QUERIES;
+	else if (strcmp(newval, "error") == 0)
+		pset.echo = PSQL_ECHO_ERROR;
 	else if (strcmp(newval, "all") == 0)
 		pset.echo = PSQL_ECHO_ALL;
 	else
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index be5c3c5..8611dd2 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3591,6 +3591,23 @@ psql_completion(const char *text, int start, int end)
 	{
 		matches = complete_from_variables(text, "", "");
 	}
+	else if (strcmp(prev2_wd, "\\set") == 0)
+	{
+		if (strcmp(prev_wd, "ECHO") == 0)
+		{
+			static const char *const my_list[] =
+			{"none", "error", "queries", "all", NULL};
+
+			COMPLETE_WITH_LIST_CS(my_list);
+		}
+		else if (strcmp(prev_wd, "ECHO_HIDDEN") == 0)
+		{
+			static const char *const my_list[] =
+			{"noexec", "off", "on", NULL};
+
+			COMPLETE_WITH_LIST_CS(my_list);
+		}
+	}
 	else if (strcmp(prev_wd, "\\sf") == 0 || strcmp(prev_wd, "\\sf+") == 0)
 		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_functions, NULL);
 	else if (strcmp(prev_wd, "\\cd") == 0 ||

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


Re: [HACKERS] pg_filedump for 9.4?

2014-06-25 Thread Fabrízio de Royes Mello
On Wed, Jun 25, 2014 at 1:28 PM, Tom Lane  wrote:
>
> =?UTF-8?Q?Fabr=C3=ADzio_de_Royes_Mello?=  writes:
> > On Wed, Jun 25, 2014 at 12:31 PM, Tom Lane  wrote:
> >> Devrim =?ISO-8859-1?Q?G=FCnd=FCz?=  writes:
> >>> Will there be a pg_filedump for 9.4? I'd like to finish package tests
> >>> before we release 9.4.0.
>
> >> Probably, but I have no time for it right now.
> >> FWIW, I believe the current "9.3" sources still work with HEAD/9.4.
>
> > I'll check it and post the results.
>
> I forgot to mention that thanks to Christoph Berg, the pg_filedump sources
> have gotten off pgfoundry and onto git.postgresql.org.  If anyone's got
> bandwidth for working on it (in particular, merging the checksum-testing
> code that was posted awhile back), I'd be happy to grant commit access.
>

Merged...

I'm thinking in run "pgindent" to better organize the source code... What
do you think?

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
diff --git a/README.pg_filedump b/README.pg_filedump
index c9104e4..3a04d59 100644
--- a/README.pg_filedump
+++ b/README.pg_filedump
@@ -59,7 +59,7 @@ not require any manual adjustments of the Makefile.
 
 Invocation:
 
-pg_filedump [-abcdfhixy] [-R startblock [endblock]] [-S blocksize] file
+pg_filedump [-abcdfhikxy] [-R startblock [endblock]] [-S blocksize] file
 
 Defaults are: relative addressing, range of the entire file, block size
   as listed on block 0 in the file
@@ -74,6 +74,7 @@ The following options are valid for heap and index files:
   -f  Display formatted block content dump along with interpretation
   -h  Display this information
   -i  Display interpreted item details
+  -k  Verify block checksums
   -R  Display specific block ranges within the file (Blocks are
   indexed from 0)
 [startblock]: block to start at
diff --git a/pg_filedump.c b/pg_filedump.c
index abcdb1e..34ce9d6 100644
--- a/pg_filedump.c
+++ b/pg_filedump.c
@@ -26,6 +26,13 @@
 
 #include "utils/pg_crc_tables.h"
 
+// checksum_impl.h uses Assert, which doesn't work outside the server
+#undef Assert
+#define Assert(X)
+
+#include "storage/checksum.h"
+#include "storage/checksum_impl.h"
+
 // Global variables for ease of use mostly
 static FILE *fp = NULL;		// File to dump or format
 static char *fileName = NULL;	// File name for display
@@ -40,12 +47,12 @@ static unsigned int blockVersion = 0;	// Block version number
 static void DisplayOptions (unsigned int validOptions);
 static unsigned int ConsumeOptions (int numOptions, char **options);
 static int GetOptionValue (char *optionString);
-static void FormatBlock ();
+static void FormatBlock (BlockNumber blkno);
 static unsigned int GetBlockSize ();
 static unsigned int GetSpecialSectionType (Page page);
 static bool IsBtreeMetaPage(Page page);
 static void CreateDumpFileHeader (int numOptions, char **options);
-static int FormatHeader (Page page);
+static int FormatHeader (Page page, BlockNumber blkno);
 static void FormatItemBlock (Page page);
 static void FormatItem (unsigned int numBytes, unsigned int startIndex,
 			unsigned int formatAs);
@@ -68,7 +75,7 @@ DisplayOptions (unsigned int validOptions)
FD_VERSION, FD_PG_VERSION);
 
   printf
-("\nUsage: pg_filedump [-abcdfhixy] [-R startblock [endblock]] [-S blocksize] file\n\n"
+("\nUsage: pg_filedump [-abcdfhikxy] [-R startblock [endblock]] [-S blocksize] file\n\n"
  "Display formatted contents of a PostgreSQL heap/index/control file\n"
  "Defaults are: relative addressing, range of the entire file, block\n"
  "   size as listed on block 0 in the file\n\n"
@@ -82,6 +89,7 @@ DisplayOptions (unsigned int validOptions)
  "  -f  Display formatted block content dump along with interpretation\n"
  "  -h  Display this information\n"
  "  -i  Display interpreted item details\n"
+ "  -k  Verify block checksums\n"
  "  -R  Display specific block ranges within the file (Blocks are\n"
  "  indexed from 0)\n" "[startblock]: block to start at\n"
  "[endblock]: block to end at\n"
@@ -288,6 +296,11 @@ ConsumeOptions (int numOptions, char **options)
 		  SET_OPTION (itemOptions, ITEM_DETAIL, 'i');
 		  break;
 
+		  // Verify block checksums
+		case 'k':
+		  SET_OPTION (blockOptions, BLOCK_CHECKSUMS, 'k');
+		  break;
+
 		  // Interpret items as standard index values
 		case 'x':
 		  SET_OPTION (itemOptions, ITEM_INDEX, 'x');
@@ -555,7 +568,7 @@ CreateDumpFileHeader (int numOptions, char **options)
 
 // Dump out a formatted block header for the requested block
 static int
-FormatHeader (Page page)
+FormatHeader (Page page, BlockNumber blkno)
 {
   int rc = 0;
   unsigned int headerBytes;
@@ -647,6 +660,14 @@ FormatHeader

[HACKERS] What's the point of json_extract_path_op etc?

2014-06-25 Thread Tom Lane
Why do we have essentially duplicate pg_proc entries for json_extract_path
and json_extract_path_op?  The latter is undocumented and seems only to be
used as the infrastructure for the #> operator.  I see that only the
former is marked variadic, but AFAIK the operator machinery couldn't care
less about that, so it seems to me we could get rid of the
json_extract_path_op entry and point the operator at json_extract_path.

Likewise for json_extract_path_text_op, jsonb_extract_path_op, and
jsonb_extract_path_text_op.

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] Extended Prefetching using Asynchronous IO - proposal and patch

2014-06-25 Thread Claudio Freire
On Tue, Jun 24, 2014 at 12:08 PM, John Lumby  wrote:
>> The question is, if you receive the notification of the I/O completion
>> using a signal or a thread, is it safe to release the lwlock from the
>> signal handler or a separate thread?
>
> In the forthcoming  new version of the patch that uses sigevent,
> the originator locks a LWlock associated with that BAaiocb eXclusive,
> and ,   when signalled,  in the signal handler it places that LWlock
> on a process-local queue of LWlocks awaiting release.
> (No, It cannot be safely released inside the signal handler or in a
> separate thread). Whenever the mainline passes a CHECK_INTERRUPTS macro
> and at a few additional points in bufmgr,  the backend walks this 
> process-local
> queue and releases those LWlocks.This is also done if the originator
> itself issues a ReadBuffer,  which is the most frequent case in which it
> is released.

I suggest using a semaphore instead.

Semaphores are supposed to be incremented/decremented from multiple
threads or processes already. So, in theory, the callback itself
should be able to do it.

The problem with the process-local queue is that it may take time to
be processed (the time it takes to get to a CHECK_INTERRUPTS macro,
which as it happened with regexes, it can be quite high).


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


Re: [HACKERS] TODO : Allow parallel cores to be used by vacuumdb [ WIP ]

2014-06-25 Thread Sawada Masahiko
Hi,

I got following FAILED when I patched v3 to HEAD.

$ patch -d. -p1 < ../patch/vacuumdb_parallel_v3.patch
patching file doc/src/sgml/ref/vacuumdb.sgml
Hunk #1 succeeded at 224 (offset 20 lines).
patching file src/bin/scripts/Makefile
Hunk #2 succeeded at 65 with fuzz 2 (offset -1 lines).
patching file src/bin/scripts/vac_parallel.c
patching file src/bin/scripts/vac_parallel.h
patching file src/bin/scripts/vacuumdb.c
Hunk #3 succeeded at 61 with fuzz 2.
Hunk #4 succeeded at 87 (offset 2 lines).
Hunk #5 succeeded at 143 (offset 2 lines).
Hunk #6 succeeded at 158 (offset 5 lines).
Hunk #7 succeeded at 214 with fuzz 2 (offset 5 lines).
Hunk #8 FAILED at 223.
Hunk #9 succeeded at 374 with fuzz 1 (offset 35 lines).
Hunk #10 FAILED at 360.
Hunk #11 FAILED at 387.
3 out of 11 hunks FAILED -- saving rejects to file
src/bin/scripts/vacuumdb.c.rej

---
Sawada Masahiko

On Friday, March 21, 2014, Dilip kumar  wrote:

> On 16 January 2014 19:53, Euler Taveira Wrote,
>
> > >
> > >> For the case where you have tables of varying size this would lead
> > to a reduced overall processing time as it prevents large (read: long
> > processing time) tables to be processed in the last step. While
> > processing large tables at first and filling up "processing slots/jobs"
> > when they get free with smaller tables one after the other would safe
> > overall execution time.
> > > Good point, I have made the change and attached the modified patch.
> > >
> > Don't you submit it for a CF, do you? Is it too late for this CF?
>
>  Attached the latest updated patch
>  1. Rebased the patch to current GIT head.
>  2. Doc is updated.
>  3. Supported parallel execution for all db option also.
>
> Same I will add to current open commitfest..
>
> Regards,
> Dilip
>


-- 
Regards,

---
Sawada Masahiko


Re: [HACKERS] PostgreSQL for VAX on NetBSD/OpenBSD

2014-06-25 Thread Tom Lane
Robert Haas  writes:
> On Wed, Jun 25, 2014 at 1:05 PM, John Klos  wrote:
>> While I wouldn't be surprised if you remove the VAX code because not many
>> people are going to be running PostgreSQL, I'd disagree with the assessment
>> that this port is broken. It compiles, it initializes databases, it runs, et
>> cetera, albeit not with the default postgresql.conf.

> Well, the fact that initdb didn't produce a working configuration and
> that make installcheck failed to work properly are bad.  But, yeah,
> it's not totally broken.

>> I'm actually rather impressed at how well PostgreSQL can be adjusted to
>> lower memory systems. I deploy a lot of embedded systems with 128 megs (a
>> lot for an embedded system, but nothing compared with what everyone else
>> assumes), so I'll be checking out PostgreSQL for other uses.

> I agree, that's cool.

I think we'd be happy to keep the VAX port of PG going as long as we
get regular feedback on it, ie closed-loop maintenance not open-loop ;-)

Is there anyone in the NetBSD/VAX community who would be willing to
host a PG buildfarm member?
http://buildfarm.postgresql.org/index.html

The requirements for this beyond what it takes to build from source
are basically just working git and Perl (ccache helps a lot too),
and enough cycles to build the code at least once a day or so.
Once you've got the thing set up it seldom needs human attention.

If we had a buildfarm member to tell us when we break things, it
would be a lot easier to promise continued support.

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] Set new system identifier using pg_resetxlog

2014-06-25 Thread Sawada Masahiko
Hi,

I send you review comment about thie patch.

I found no error/warning with compling and installation.
I have executed pg_resetxlog with some input pattern.

$ initdb -D data -E UTF8 --no-locale
$ pg_controldata data | grep "Database system identifier"
Database system identifier:   6028907917695471865

--
$ pg_resetxlog -s -n  data | grep "Database system identifier"
Database system identifier:   6028907917695471865

The -s option does not works fine with -n option.

--
$ pg_resetxlog
-s602890791769547186511
data
Transaction log reset
$ pg_controldata data | grep "Database system identifier"
Database system identifier:   18446744073709551615

pg_resetxlog is finished successfully, but system identifier was not
changed.
Also I think that checking data about number of digits is needed.

regards
--
Sawada Masahiko

On Thursday, June 19, 2014, Petr Jelinek  wrote:

> On 18/06/14 19:26, Robert Haas wrote:
>
>> On Wed, Jun 18, 2014 at 12:54 PM, Andres Freund 
>> wrote:
>>
>>> I don't see how the proposed ability makes it more dangerous. It
>>> *already* has the ability to reset the timelineid. That's the case where
>>> users are much more likely to think about resetting it because that's
>>> much more plausible than taking a unrelated cluster and resetting its
>>> sysid, timeline and LSN.
>>>
>>
>> All right, well, I've said my piece.  I don't have anything to add to
>> that that isn't sheer repetition.  My vote is to hold off on this
>> until we've talked about replication identifiers and other related
>> topics in more depth.  But if that position doesn't garner majority
>> support ... so be it!
>>
>>
> I am not sure I get what does this have to do with replication
> identifiers. The patch has several use-cases, one of them has to do that
> you can know the future system id before you set it, which is useful for
> automating some things...
>
> --
>  Petr Jelinek  http://www.2ndQuadrant.com/
>  PostgreSQL Development, 24x7 Support, Training & Services
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>


-- 
Regards,

---
Sawada Masahiko


Re: [HACKERS] PostgreSQL for VAX on NetBSD/OpenBSD

2014-06-25 Thread Greg Stark
On Wed, Jun 25, 2014 at 10:17 AM, Robert Haas  wrote:
> Well, the fact that initdb didn't produce a working configuration and
> that make installcheck failed to work properly are bad.  But, yeah,
> it's not totally broken.

Yeah it seems to me that these kinds of autoconf and initdb tests
failing are different from a platform where the spinlock code doesn't
work. It's actually valuable to have a platform where people routinely
trigger those configuration values. If they're broken there's not much
value in carrying them.


-- 
greg


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


Re: [HACKERS] PostgreSQL for VAX on NetBSD/OpenBSD

2014-06-25 Thread John Klos

That's all I have time for tonight. Is there an easier way to run a
testsuite?


I think you're doing it right, but apparently configure is
mis-identifying which flags are needed for thread-safety on your
platform.  It's possible configuring with --disable-thread-safety
would help, or you could manually edit the Makefile.


I'll play with it some more in a little bit. This is why I tend to trust 
the pkgsrc framework - it just works.



In any case I'm coming to the conclusion that there's little point in
us keeping the VAX-specific code in our source tree, because in fact,
this port is broken and doesn't work.  Based on your results thus far,
I doubt that it would be a huge amount of work to fix that, but unless
somebody from the VAX community wants to volunteer to be a PostgreSQL
maintainer for that platform, straighten out the things that have
gotten broken since this port was originally added, and keep it
working on an ongoing basis, it's probably not going to happen.


While I wouldn't be surprised if you remove the VAX code because not many 
people are going to be running PostgreSQL, I'd disagree with the 
assessment that this port is broken. It compiles, it initializes 
databases, it runs, et cetera, albeit not with the default postgresql.conf.


I'm actually rather impressed at how well PostgreSQL can be adjusted to 
lower memory systems. I deploy a lot of embedded systems with 128 megs (a 
lot for an embedded system, but nothing compared with what everyone else 
assumes), so I'll be checking out PostgreSQL for other uses.


NetBSD's VAX port does lots to help ensure code portability and code 
correctness, so it's not going anywhere any time soon. It certainly is a 
good sign that PostgreSQL can run on a VAX with only 20 MB or so of 
resident memory.


Thanks,
John


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


Re: [HACKERS] better atomics - v0.5

2014-06-25 Thread Andres Freund
On 2014-06-25 10:39:53 -0700, Josh Berkus wrote:
> On 06/25/2014 10:14 AM, Andres Freund wrote:
> > Hi,
> > 
> > [sorry for the second copy Robert]
> > 
> > Attached is a new version of the atomic operations patch. Lots has
> > changed since the last post:
> 
> Is this at a state where we can performance-test it yet?

Well, this patch itself won't have a performance impact at all. It's
about proving the infrastructure to use atomic operations in further
patches in a compiler and platform independent way.
I'll repost a new version of the lwlocks patch (still fighting an issue
I introduced while rebasing ntop of Heikki's xlog scalability/lwlock
merger) soon. Addressing the review comments Amit has made since.

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] better atomics - v0.5

2014-06-25 Thread Josh Berkus
On 06/25/2014 10:14 AM, Andres Freund wrote:
> Hi,
> 
> [sorry for the second copy Robert]
> 
> Attached is a new version of the atomic operations patch. Lots has
> changed since the last post:

Is this at a state where we can performance-test it yet?

-- 
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] PostgreSQL for VAX on NetBSD/OpenBSD

2014-06-25 Thread Dave McGuire
On 06/25/2014 05:30 AM, John Klos wrote:
>> What value did it select for shared_buffers?  How much memory does a
>> high-end VAX have?  These days, we try to set shared_buffers = 128MB
>> if the platform will support it, but it's supposed to fall back to
>> smaller values if that doesn't work.  It will try allocating that much
>> though, at least for a moment, to see whether it can.
> 
> A high end VAX, such as a 4000 Model 108, can have 512 megs (as can an
> 11/780, at least in theory), but most of the VAXen used here are
> VAXstations such as the 4000/60 or 4000/90, 90a or 96, which have either
> 104 megs or 128 megs.

  My VAX-7000 has 1.5GB. B-)

-Dave

-- 
Dave McGuire, AK4HZ/3
New Kensington, PA


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


Re: [HACKERS] PostgreSQL for VAX on NetBSD/OpenBSD

2014-06-25 Thread David Brownlee
On 25 June 2014 12:38, Toby Thain  wrote:
> On 24/06/14 10:16 PM, John Klos wrote:
>>
>> Hi,
>>
>>> Has anyone tried to build PostgreSQL for VAX lately?  If so, did it
>>> compile?  Did you have to use --disable-spinlocks to get it to
>>> compile? If it did compile, can you actually run it, and does it pass
>>> the regression tests and work as expected?  Would you be willing to
>>> work with the PostgreSQL to ensure continuing support for this
>>> platform, or does that seem not worthwhile for whatever reason?
>>
>>
>> I've compiled postgresql93-client and postgresql93-server from pkgsrc on
>> a VAX running NetBSD 6.1.4. ...
>>
>> It does compile and initialize, so the VAX code does work. However,
>> considering how much memory it uses, I wonder how many people would
>> actually use it. I did run Apache / MySQL / PHP on a VAXstation 4000/60
>
> I guess I shan't expect to run PgSQL on a MicroVAX II (9MB), NetBSD 1.4.1. I
> did get Apache 1.3.x built on it.

I suspect it might technically be possible to run PgSQL on that
hardware - probably best done with an app on another box (maybe a
second uVAX II :) which is not in a particular hurry for query
responses

>> not long ago, but MySQL takes way too much memory, too. Don't even get
>> me started on how memory PHP uses - someone has to write some good
>> weblog software in C one of these days...
>
> If only C and PHP weren't the only options!

Tsk, how could we forget VAX MACRO assembler :-p


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


Re: [HACKERS] PostgreSQL for VAX on NetBSD/OpenBSD

2014-06-25 Thread Toby Thain

On 24/06/14 10:16 PM, John Klos wrote:

Hi,


Has anyone tried to build PostgreSQL for VAX lately?  If so, did it
compile?  Did you have to use --disable-spinlocks to get it to
compile? If it did compile, can you actually run it, and does it pass
the regression tests and work as expected?  Would you be willing to
work with the PostgreSQL to ensure continuing support for this
platform, or does that seem not worthwhile for whatever reason?


I've compiled postgresql93-client and postgresql93-server from pkgsrc on
a VAX running NetBSD 6.1.4. ...
It does compile and initialize, so the VAX code does work. However,
considering how much memory it uses, I wonder how many people would
actually use it. I did run Apache / MySQL / PHP on a VAXstation 4000/60


I guess I shan't expect to run PgSQL on a MicroVAX II (9MB), NetBSD 
1.4.1. I did get Apache 1.3.x built on it.



not long ago, but MySQL takes way too much memory, too. Don't even get
me started on how memory PHP uses - someone has to write some good
weblog software in C one of these days...


If only C and PHP weren't the only options!

--T



John





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


Re: [HACKERS] PostgreSQL for VAX on NetBSD/OpenBSD

2014-06-25 Thread John Klos

Hi,


What value did it select for shared_buffers?  How much memory does a
high-end VAX have?  These days, we try to set shared_buffers = 128MB
if the platform will support it, but it's supposed to fall back to
smaller values if that doesn't work.  It will try allocating that much
though, at least for a moment, to see whether it can.


A high end VAX, such as a 4000 Model 108, can have 512 megs (as can an 
11/780, at least in theory), but most of the VAXen used here are 
VAXstations such as the 4000/60 or 4000/90, 90a or 96, which have either 
104 megs or 128 megs.


I was trying it just using the default postgresql.conf. I changed it:

< max_connections = 10  # (change requires restart)

max_connections = 40  # (change requires restart)

< shared_buffers = 16MB # min 128kB

shared_buffers = 128MB# min 128kB

< temp_buffers = 2MB# min 800kB
< max_prepared_transactions = 0 # zero disables the feature

#temp_buffers = 8MB   # min 800kB
#max_prepared_transactions = 0# zero disables the feature

< maintenance_work_mem = 8MB# min 1MB
< max_stack_depth = 1MB # min 100kB

#maintenance_work_mem = 16MB  # min 1MB
#max_stack_depth = 2MB# min 100kB

< max_files_per_process = 100   # min 25

#max_files_per_process = 1000 # min 25



and it launched fine. I then tried to run:

gmake MAX_CONNECTIONS=5 installcheck

in 
/usr/pkgsrc/databases/postgresql93-server/work/postgresql-9.3.4/src/test/regress,
but it failed with:

...
gmake[2]: Leaving directory 
'/usr/pkgsrc/databases/postgresql93-server/work/postgresql-9.3.4/src/backend'
gcc -O1 -fgcse -fstrength-reduce -fgcse-after-reload -I/usr/include 
-I/usr/local/include -Wall -Wmissing-prototypes -Wpointer-arith 
-Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute 
-Wformat-security -fno-strict-aliasing -fwrapv -pthread  -mt -D_REENTRANT 
-D_THREAD_SAFE -D_POSIX_PTHREAD_SEMANTICS -I../../src/port -DFRONTEND 
-I../../src/include -I/usr/include -I/usr/local/include   -c -o thread.o 
thread.c
cc1: error: unrecognized 
command line option "-mt" : recipe for target 'thread.o' failed

gmake[1]: *** [thread.o] Error 1
gmake[1]: Leaving directory 
'/usr/pkgsrc/databases/postgresql93-server/work/postgresql-9.3.4/src/port'
../../../src/Makefile.global:423: recipe for target 'submake-libpgport' 
failed

gmake: *** [submake-libpgport] Error 2

That's all I have time for tonight. Is there an easier way to run a 
testsuite?


Thanks,
John


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


Re: [HACKERS] PostgreSQL for VAX on NetBSD/OpenBSD

2014-06-25 Thread John Klos

Hi,

Has anyone tried to build PostgreSQL for VAX lately?  If so, did it 
compile?  Did you have to use --disable-spinlocks to get it to compile? 
If it did compile, can you actually run it, and does it pass the 
regression tests and work as expected?  Would you be willing to work 
with the PostgreSQL to ensure continuing support for this platform, or 
does that seem not worthwhile for whatever reason?


I've compiled postgresql93-client and postgresql93-server from pkgsrc on a 
VAX running NetBSD 6.1.4. The initial launch didn't like the default stack 
limit:


/etc/rc.d/pgsql start
Initializing PostgreSQL databases.
LOG:  invalid value for parameter "max_stack_depth": 100
DETAIL:  "max_stack_depth" must not exceed 0kB.
HINT:  Increase the platform's stack depth limit via "ulimit -s" or local 
equivalent.

FATAL:  failed to initialize max_stack_depth to 100
child process exited with exit code 1
initdb: removing data directory "/usr/local/pgsql/data"
pg_ctl: database system initialization failed


I unlimited and tried again. The pgsql process showed it was using 146 
megabytes of memory while initializing, then got as far as:


/etc/rc.d/pgsql start
Initializing PostgreSQL databases.

WARNING: enabling "trust" authentication for local connections
You can change this by editing pg_hba.conf or using the option -A, or
--auth-local and --auth-host, the next time you run initdb.
Starting pgsql.

Then the machine paniced. The serial console showed:

panic: usrptmap space leakage
cpu0: Begin traceback...
panic: usrptmap space leakage
Stack traceback :
 Process is executing in user space.
cpu0: End traceback...

dump to dev 9,1 not possible


It does compile and initialize, so the VAX code does work. However, 
considering how much memory it uses, I wonder how many people would 
actually use it. I did run Apache / MySQL / PHP on a VAXstation 4000/60 
not long ago, but MySQL takes way too much memory, too. Don't even get me 
started on how memory PHP uses - someone has to write some good weblog 
software in C one of these days...


John


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


Re: [HACKERS] PostgreSQL for VAX on NetBSD/OpenBSD

2014-06-25 Thread Anders Magnusson

John Klos skrev 2014-06-25 04:16:

Then the machine paniced. The serial console showed:

panic: usrptmap space leakage
cpu0: Begin traceback...
panic: usrptmap space leakage
Stack traceback :
 Process is executing in user space.
cpu0: End traceback...

Hm, can you add info about this panic to PR #28379 ?  I will try to hunt 
this down soon, so I need some test cases.


-- Ragge


Re: [HACKERS] PostgreSQL for VAX on NetBSD/OpenBSD

2014-06-25 Thread Robert Haas
On Wed, Jun 25, 2014 at 1:05 PM, John Klos  wrote:
>> In any case I'm coming to the conclusion that there's little point in
>> us keeping the VAX-specific code in our source tree, because in fact,
>> this port is broken and doesn't work.  Based on your results thus far,
>> I doubt that it would be a huge amount of work to fix that, but unless
>> somebody from the VAX community wants to volunteer to be a PostgreSQL
>> maintainer for that platform, straighten out the things that have
>> gotten broken since this port was originally added, and keep it
>> working on an ongoing basis, it's probably not going to happen.
>
> While I wouldn't be surprised if you remove the VAX code because not many
> people are going to be running PostgreSQL, I'd disagree with the assessment
> that this port is broken. It compiles, it initializes databases, it runs, et
> cetera, albeit not with the default postgresql.conf.

Well, the fact that initdb didn't produce a working configuration and
that make installcheck failed to work properly are bad.  But, yeah,
it's not totally broken.

> I'm actually rather impressed at how well PostgreSQL can be adjusted to
> lower memory systems. I deploy a lot of embedded systems with 128 megs (a
> lot for an embedded system, but nothing compared with what everyone else
> assumes), so I'll be checking out PostgreSQL for other uses.

I agree, that's cool.

> NetBSD's VAX port does lots to help ensure code portability and code
> correctness, so it's not going anywhere any time soon. It certainly is a
> good sign that PostgreSQL can run on a VAX with only 20 MB or so of resident
> memory.

Yeah!

-- 
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: Bug in spg_range_quad_inner_consistent for adjacent operator (was Re: [HACKERS] Add a filed to PageHeaderData)

2014-06-25 Thread Heikki Linnakangas

On 06/24/2014 11:22 PM, Heikki Linnakangas wrote:

The real bug is in spg_range_quad_inner_consistent(), for the adjacent
operator. Things go wrong when:

The scan key is [100, 500)
The prev centroid is [500, 510)
The current centroid is [544, 554).

The row that should match but isn't returned, [500, 510) is equal to the
previous centroid. It's in quadrant 3 from the current centroid, but
spg_range_quad_inner_consistent() incorrectly concludes that it doesn't
need to scan that quadrant.

The function compares the scan key's upper bound with the the previous
centroid's lower bound and the current centroid's lower bound:


/*
  * Check if upper bound of argument is not in a
  * quadrant we visited in the previous step.
  */
cmp1 = range_cmp_bounds(typcache, &upper, &prevLower);
cmp2 = range_cmp_bounds(typcache, ¢roidLower,
&prevLower);
if ((cmp2 < 0 && cmp1 > 0) || (cmp2 > 0 && cmp1 < 0))
 which2 = 0;


The idea is that if the scan key's upper bound doesn't fall between the
prev and current centroid's lower bounds, there is no match.

*   **
   PL   XCL

X = scan key's upper bound: 500)
PL = prev centroid's lower bound [500
CL = current centroid's lower bound [500

This is wrong. X < PL, but it's still nevertheless adjacent to it.

I'll take a closer look tomorrow...

(The "if (which2) ..." block after the code I quoted above also looks
wrong - it seems to be comparing the argument's lower bound when it
should be comparing the upper bound according to the comment. )


I came up with the attached. There were several bugs:

* The "if (which2) { ... }"  block was broken. It compared the 
argument's lower bound against centroid's upper bound, while it was 
supposed to compare the argument's upper bound against the centroid's 
lower bound (the comment was right). Also, it clear bits in the "which1" 
variable, while it was supposed to clear bits in "which2". ISTM it was 
copy-pasted from the if (which1) { ... }" block just before it, but not 
modified.


* If the argument's upper bound was equal to the centroid's lower bound, 
we descended to both halves (= all quadrants). That's unnecessary. 
Imagine that the argument is (x, 500), and the centroid is (500, y), so 
that the bounds are equal. The adjacent ranges that we're trying to find 
would be of form [500, z), which are to the right of the centroid. There 
is no need to visit the left quadrants. This won't lead to incorrect 
query results, but slows down queries unnecessarily.


* In the case that argument's lower bound is adjacent to the centroid's 
upper bound, we also don't need to visit all quadrants. Per similar 
reasoning as previous point.


* The code where we compare the previous centroid with the current 
centroid should match the code where we compare the current centroid 
with the argument. The point of that code is to redo the calculation 
done in the previous level, to see if we were supposed to traverse left 
or right (or up or down), and if we actually did. If we moved in the 
different direction, then we know there are no matches for bound.


Those could be fixed with quite small adjustments, but I think the code 
needs some refactoring. It's complicated as it is, it's very difficult 
to understand all the cases and comparisons. Case in point: the patch 
was written by Alexander, reviewed by Jeff, and committed by me, and we 
all missed the above bugs. So, I propose the attached.


I also wrote the attached little white-box testing tool for this. It 
allows you to call spg_range_quad_inner_consistent with the adjacent 
strategy, and pass the exact argument, centroid and prev centroid ranges 
you want. It prints out the result of which quadrants to visit.


- Heikki

diff --git a/src/backend/utils/adt/rangetypes_spgist.c b/src/backend/utils/adt/rangetypes_spgist.c
index a55cffa..1b83941 100644
--- a/src/backend/utils/adt/rangetypes_spgist.c
+++ b/src/backend/utils/adt/rangetypes_spgist.c
@@ -54,6 +54,12 @@ static int16 getQuadrant(TypeCacheEntry *typcache, RangeType *centroid,
 			RangeType *tst);
 static int	bound_cmp(const void *a, const void *b, void *arg);
 
+static int adjacent_inner_consistent(TypeCacheEntry *typcache,
+		  RangeBound *arg, RangeBound *centroid,
+		  RangeBound *prev);
+static int adjacent_cmp_bounds(TypeCacheEntry *typcache, RangeBound *arg,
+	RangeBound *centroid);
+
 /*
  * SP-GiST 'config' interface function.
  */
@@ -441,6 +447,11 @@ spg_range_quad_inner_consistent(PG_FUNCTION_ARGS)
 			bool		empty;
 			RangeType  *range = NULL;
 
+			RangeType  *prevCentroid = NULL;
+			RangeBound	prevLower,
+		prevUpper;
+			bool		prevEmpty;
+
 			/* Restrictions on range bounds according to scan strategy */
 			RangeBound *minLower = NULL,
 	   *maxLower = NULL,
@@ -550,109 +561,53 @@ spg_range_quad_inner_consistent(PG_FUNCTION_ARGS)
 		break;	/* Skip to strictEmpty check. */
 
 	/*
-	 * which1 is bitmask for possibility to be adjacent with
-	 * lower bo

Re: [HACKERS] pg_filedump for 9.4?

2014-06-25 Thread Tom Lane
=?UTF-8?Q?Fabr=C3=ADzio_de_Royes_Mello?=  writes:
> On Wed, Jun 25, 2014 at 12:31 PM, Tom Lane  wrote:
>> Devrim =?ISO-8859-1?Q?G=FCnd=FCz?=  writes:
>>> Will there be a pg_filedump for 9.4? I'd like to finish package tests
>>> before we release 9.4.0.

>> Probably, but I have no time for it right now.
>> FWIW, I believe the current "9.3" sources still work with HEAD/9.4.

> I'll check it and post the results.

I forgot to mention that thanks to Christoph Berg, the pg_filedump sources
have gotten off pgfoundry and onto git.postgresql.org.  If anyone's got
bandwidth for working on it (in particular, merging the checksum-testing
code that was posted awhile back), I'd be happy to grant commit access.

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] ALTER SYSTEM RESET?

2014-06-25 Thread Vik Fearing
On 06/25/2014 03:04 PM, Amit Kapila wrote:
> On Wed, Jun 25, 2014 at 6:20 PM, Christoph Berg  > wrote:
>>
>> Hi,
>>
>> is there a reason there's no ALTER SYSTEM RESET?
>>
>> The natural idiom to reset SET statements is "RESET guc;", I don't
>> think "SET guc = default;" is in use much, so "ALTER SYSTEM RESET guc;"
>> would be the natural way to try.
> 
> Currently you can achieve that by
> "ALTER SYSTEM RESET guc = Default;".
> However it will be good to have support for RESET as well.  I think it
> should not be too complicated to implement that syntax, I personally
> don't have bandwidth to it immediately, but I would like to take care
> of it unless you or someone wants to do it by the time I get some
> bandwidth.

Would something like this suffice?
-- 
Vik
*** a/doc/src/sgml/ref/alter_system.sgml
--- b/doc/src/sgml/ref/alter_system.sgml
***
*** 22,27  PostgreSQL documentation
--- 22,28 
   
  
  ALTER SYSTEM SET configuration_parameter { TO | = } { value | 'value' | DEFAULT }
+ ALTER SYSTEM RESET configuration_parameter
  
   
  
***
*** 30,37  ALTER SYSTEM SET configuration_parameter
 ALTER SYSTEM writes the configuration parameter
!values to the postgresql.auto.conf file. With
!DEFAULT, it removes a configuration entry from
 postgresql.auto.conf file. The values will be
 effective after reload of server configuration (SIGHUP) or in next 
 server start based on the type of configuration parameter modified.
--- 31,38 
  

 ALTER SYSTEM writes the configuration parameter
!values to the postgresql.auto.conf file. Setting the parameter to
!DEFAULT, or using the RESET variant, removes the configuration entry from
 postgresql.auto.conf file. The values will be
 effective after reload of server configuration (SIGHUP) or in next 
 server start based on the type of configuration parameter modified.
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***
*** 8486,8491  AlterSystemStmt:
--- 8486,8499 
  	n->setstmt = $4;
  	$$ = (Node *)n;
  }
+ 			| ALTER SYSTEM_P RESET var_name
+ {
+ 	AlterSystemStmt *n = makeNode(AlterSystemStmt);
+ 	n->setstmt = makeNode(VariableSetStmt);
+ 	n->setstmt->kind = VAR_RESET;
+ 	n->setstmt->name = $4;
+ 	$$ = (Node *)n;
+ }
  		;
  
  
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***
*** 6720,6725  AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt)
--- 6720,6726 
  			break;
  
  		case VAR_SET_DEFAULT:
+ 		case VAR_RESET:
  			value = NULL;
  			break;
  		default:

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


Re: [HACKERS] 9.3 minor release soon?

2014-06-25 Thread Alvaro Herrera
Bruce Momjian wrote:
> Are we nearing a minor release deadline for 9.3?  There will need to be
> fix-up query in the minor release notes for pg_upgrade:
> 
>   
> http://www.postgresql.org/message-id/20140530121631.ge25...@alap3.anarazel.de

There's one more multixact bugfix pending, actually, so please hold on
for a bit.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


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


Re: [HACKERS] pg_filedump for 9.4?

2014-06-25 Thread Fabrízio de Royes Mello
On Wed, Jun 25, 2014 at 12:31 PM, Tom Lane  wrote:
>
> Devrim =?ISO-8859-1?Q?G=FCnd=FCz?=  writes:
> > Will there be a pg_filedump for 9.4? I'd like to finish package tests
> > before we release 9.4.0.
>
> Probably, but I have no time for it right now.
>
> FWIW, I believe the current "9.3" sources still work with HEAD/9.4.
>

I'll check it and post the results.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello


Re: [HACKERS] ALTER SYSTEM RESET?

2014-06-25 Thread Fabrízio de Royes Mello
On Wed, Jun 25, 2014 at 10:04 AM, Amit Kapila 
wrote:
>
> On Wed, Jun 25, 2014 at 6:20 PM, Christoph Berg  wrote:
> >
> > Hi,
> >
> > is there a reason there's no ALTER SYSTEM RESET?
> >
> > The natural idiom to reset SET statements is "RESET guc;", I don't
> > think "SET guc = default;" is in use much, so "ALTER SYSTEM RESET guc;"
> > would be the natural way to try.
>
> Currently you can achieve that by
> "ALTER SYSTEM RESET guc = Default;".
> However it will be good to have support for RESET as well.  I think it
> should not be too complicated to implement that syntax, I personally
> don't have bandwidth to it immediately, but I would like to take care
> of it unless you or someone wants to do it by the time I get some
> bandwidth.
>

I have some time then I can provide a patch in a few days... Is ok for you
guys?

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello


Re: [HACKERS] wrapping in extended mode doesn't work well with default pager

2014-06-25 Thread Pavel Stehule
2014-06-24 19:45 GMT+02:00 Sergey Muraviov :

> Hi.
>
> Is there any problem with the patch?
>

I tested it and I had not any issue with last version

So, please, commit it

Regards

Pavel


>
>
> 2014-06-17 0:21 GMT+04:00 Greg Stark :
>
> On Mon, Jun 16, 2014 at 9:05 PM, Robert Haas 
>> wrote:
>> > So, it seems like we need to do something about this one way or
>> > another.  Who's working on that?
>>
>> So I'm fine finishing what I started. I've just been a bit busy this past
>> week.
>>
>> My inclination is to try to push forward and commit this patch,
>> document the changes and make sure we check for any consequences of
>> them.
>>
>> The alternate plan is to revert it for 9.4 and commit the changes to
>> 9.5 and that gives us more time to be sure we're ok with them.
>>
>>
>> --
>> greg
>>
>
>
>
> --
> Best regards,
> Sergey Muraviov
>


Re: [HACKERS] Allowing NOT IN to use ANTI joins

2014-06-25 Thread Tom Lane
Simon Riggs  writes:
> To be clearer, what I mean is we use only the direct proof approach,
> for queries like this

>   SELECT * FROM a WHERE id NOT IN(SELECT unknown_col FROM b WHERE
> unknown_col IS NOT NULL);

> and we don't try to do it for queries like this

>   SELECT * FROM a WHERE id NOT IN(SELECT not_null_column FROM b);

> because we don't know the full provenance of "not_null_column" in all
> cases and that info is (currently) unreliable.

FWIW, I think that would largely cripple the usefulness of the patch.
If you can tell people to rewrite their queries, you might as well
tell them to rewrite into NOT EXISTS.  The real-world queries that
we're trying to improve invariably look like the latter case not the
former, because people who are running into this problem usually
aren't even thinking about the possibility of NULLs.

I would actually say that if we only have the bandwidth to get one of
these cases done, it should be the second one not the first.  It's
unclear to me that checking for the first case would even be worth
the planner cycles it'd cost.

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] makeAndExpr(), etc. confined to gram.y?

2014-06-25 Thread Tom Lane
Amit Langote  writes:
> On Wed, Jun 25, 2014 at 1:27 PM, Tom Lane  wrote:
>> Amit Langote  writes:
>>> Is there a reason why they've been left out of
>>> makefuncs.h/makefuncs.c? Perhaps they are not supposed to be used
>>> outside gram.y at all? For example, previously a caller (potentially)
>>> outside parser could do a makeA_Expr(AEXPR_AND, ...). I guess this is
>>> no longer possible with AEXPR_AND gone?

>> What would be the purpose?  There is noplace except gram.y that builds
>> raw parse trees.

> Yeah, that is true. Sorry, I am unaware as to how generic make*
> functions in gram.y are and how they differ from those in makefuncs.c.

> So, use of make* family of functions outside parser is their abuse in
> some way? Anything that needs to use these functions should somehow be
> accomplished in parser perhaps. For example, duplicate/redundant CHECK
> expressions elimination and such?

Well, the larger point here is that those functions are specific to
gram.y's problem of constructing multi-AND(OR) structures during a series
of binary production actions.  I don't see that there's any use for them
elsewhere, and the way that they modify the input structures wouldn't
necessarily be safe anywhere else either.

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] RLS Design

2014-06-25 Thread Robert Haas
On Tue, Jun 24, 2014 at 8:49 PM, Stephen Frost  wrote:
> Let's try to outline what this would look like then.
>
> Taking your approach, we'd have:
>
> CREATE POLICY p1;
> CREATE POLICY p2;
>
> ALTER TABLE t1 SET POLICY p1 TO t1_p1_quals;
> ALTER TABLE t1 SET POLICY p2 TO t1_p2_quals;

This seems like a very nice, flexible framework.

> GRANT SELECT ON TABLE t1 TO role1 USING p1;
> GRANT SELECT ON TABLE t1 TO role2 USING p2;

Instead of doing it this way, we could instead do:

ALTER ROLE role1 ADD POLICY p1;
ALTER ROLE role2 ADD POLICY p2;

We could possibly allow multiple policies to be set for the same user,
but given an error (or OR the quals together) if there are conflicting
policies for the same table.  A user with no policies would see
everything to which they've been granted access.

To support different policies on different operations, you could have
something like:

ALTER TABLE t1 SET POLICY p1 ON INSERT TO t1_p1_quals;

Without the ON clause, it would establish the given policy for all operations.

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

2014-06-25 Thread Tom Lane
Fujii Masao  writes:
> Why is IIT timeout turned on only when send_ready_for_query is true?
> I was thinking it should be turned on every time a message is received.
> Imagine the case where the session is in idle-in-transaction state and
> a client gets stuck after sending Parse message and before sending Bind
> message. This case would also cause long transaction problem and should
> be resolved by IIT timeout. But IIT timeout that this patch adds cannot
> handle this case because it's enabled only when send_ready_for_query is
> true. Thought?

I think you just moved the goalposts to the next county.

The point of this feature, AFAICS, is to detect clients that are failing
to issue another query or close their transaction as a result of bad
client logic.  It's not to protect against network glitches.

Moreover, there would be no way to implement a timeout like that without
adding a gettimeofday() call after every packet receipt, which is overhead
we do not need and cannot afford.  I don't think this feature should add
*any* gettimeofday's beyond the ones that are already there.

regards, tom lane


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


Re: [HACKERS] pg_filedump for 9.4?

2014-06-25 Thread Tom Lane
Devrim =?ISO-8859-1?Q?G=FCnd=FCz?=  writes:
> Will there be a pg_filedump for 9.4? I'd like to finish package tests
> before we release 9.4.0.

Probably, but I have no time for it right now.

FWIW, I believe the current "9.3" sources still work with HEAD/9.4.

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: [Fwd: Re: [HACKERS] proposal: new long psql parameter --on-error-stop]

2014-06-25 Thread MauMau
OK, let me help you, though I'm only a Japanese who is never confident in my 
English.


(1)
As Fujii-san pointed out, could you add explanation for --help-variables in 
doc/src/sgml/ref/psqlref.sgml?



(2)
+ printf(_("  --help-variables list of available configuration 
variables (options), then exit\n"));


should better be:

+ printf(_("  --help-variables show A list of all specially treated 
variables, then exit\n"));


This follows the psql manual page.  Similarly,

+ printf(_("List of variables (options) for use from command line.\n"));

should be:

+ printf(_("List of specially treated variables.\n"));


(3)
+ printf(_("  ECHO   control what input can be writtent to 
standard output [all, queries]\n"));


"writtent" should be "written".  "controls" should be "control" like other 
options.



(4)
+ printf(_("  ECHO_HIDDENdisplay internal queries (same as -E 
option)\n"));


should better be:

+ printf(_("  ECHO_HIDDENdisplay internal queries executed by 
backslash commands\n"));


I think "(same as ...)" can be omitted to keep the description short.  If 
you want to retain it, other variables should also accompany similar 
description, such as -a for ECHO.



(5)
+ printf(_("  FETCH_COUNTfetch many rows at a time (use less memory) 
(default 0 unlimited)\n"));


should better be:

+ printf(_("  FETCH_COUNTthe number of result rows to fetch and 
display at a time (default: 0=unlimited)\n"));



(6)
+ printf(_("  HISTCONTROLwhen set, controls history list 
[ignorespace, ignoredups, ignoreboth]\n"));


should better be:

+ printf(_("  HISTCONTROLcontrol history list [ignorespace, 
ignoredups, ignoreboth]\n"));



(7)
+ printf(_("  USER   the database user currently connected\n"));

should add "as" at the end:

+ printf(_("  USER   the database user currently connected 
as\n"));



(8)
"Printing options" section lack the following ones described in psql manual:

columns
expanded (or x)
footer
numericlocale
tableattr (or T)


(9)
+ printf(_("\nEnvironment options:\n"));

should be ""Environment variables".  And this section lacks description for 
Windows, such as:


+ printf(_("  NAME=VALUE [NAME=VALUE] psql ...\n  or \\setenv NAME [VALUE] 
in interactive mode\n\n"));

+ printf(_("  PGPASSFILE password file (default ~/.pgpass)\n"));

Regards
MauMau



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


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-06-25 Thread Stephen Frost
* Abhijit Menon-Sen (a...@2ndquadrant.com) wrote:
> At 2014-06-25 00:10:55 -0400, sfr...@snowman.net wrote:
> > For my part, the nexts steps might be to consider how you'd migrate
> > what you've provided for configuration into catalog tables
> 
> I must confess that I do not understand what needs to be migrated into
> the catalog tables, or why. Of course, pgaudit.log must be renamed, but
> why can't it continue to be a GUC setting? (Fujii-san suggested that it
> be integrated with log_statement. I'm not sure what I think of that, but
> it's certainly one possibility.)

What I was getting at are things like "what tables are to be audited,
and for what users?".  I thought pg_audit had this capability today
(isn't that part of the request for the rlsextension field..?) and I'd
want to see something like

AUDIT SELECT ON table1 FOR role1;

> > and how we'd address the concerns raised elsewhere regarding catalog
> > access in cases where we're not in a transaction
> 
> …by not putting things into the catalog?

I just don't see that as viable.

> If we implement per-object auditing configuration in-core, it can use a
> real reloption. Apart from that, I don't see a really good reason yet to
> put more things into the database.

I don't think the auditing will be so simple as being able to be
supported by a single additional field or with just reloption.  Consider
what's happening on the RLS thread.

> > We'd also end up re-working the code to be called as part of PG core
> > rather than through hook functions, of course, but I don't think those
> > changes would be too bad compared to figuring out the other issues.
> 
> You're right (but we'd still want to use event triggers). Maybe it would
> make sense to have an auditing hook that we can sprinkle calls to in all
> the interesting places, though.

Not against using event triggers if they make sense..  I'm not 100% sure
that I agree that they do, but I haven't looked at it closely enough to
really form an opinion.

> > Additionally, thought towards what the SQL-level syntax would be is
> > another key point- would the main command be 'ALTER AUDIT'?
> 
> (I have some thoughts about that, but I'll discuss them later when I
> have a bit more time to present them properly.)

I find it really helps to try and define an SQL syntax as it can help
build an understanding of what's going to be in the catalogs and what
features are expected.

Thanks,

Stephen


signature.asc
Description: Digital signature


[HACKERS] [PATCH] log_{directory,filename} doc fixes

2014-06-25 Thread Christoph Berg
Hi,

The defaults for log_directory and log_filename were undocumented, and
the log_filename docs still refered to a config example that was
removed with 8.4 deprecation of epoch-based logfilenames.

Christoph
-- 
c...@df7cb.de | http://www.df7cb.de/
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
new file mode 100644
index 09e6dfc..e3d1c62
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
*** local0.*/var/log/postgresql
*** 3667,3672 
--- 3667,3673 
  cluster data directory.
  This parameter can only be set in the postgresql.conf
  file or on the server command line.
+ The default is pg_log.
 

   
*** local0.*/var/log/postgresql
*** 3693,3698 
--- 3694,3700 
   specification.
  Note that the system's strftime is not used
  directly, so platform-specific (nonstandard) extensions do not work.
+ The default is postgresql-%Y-%m-%d_%H%M%S.log.
 
 
  If you specify a file name without escapes, you should plan to
*** local0.*/var/log/postgresql
*** 3709,3716 
  log file name to create the file name for CSV-format output.
  (If log_filename ends in .log, the suffix is
  replaced instead.)
- In the case of the example above, the CSV
- file name will be server_log.1093827753.csv.
 
 
  This parameter can only be set in the postgresql.conf
--- 3711,3716 

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


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-06-25 Thread Stephen Frost
Alvaro,

* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> Stephen Frost wrote:
> > * Abhijit Menon-Sen (a...@2ndquadrant.com) wrote:
> > > We have some time available to work on it, but not so much that I want
> > > to write any more code without a clearer idea of what might be accepted
> > > eventually for inclusion.
> > 
> > You and me both... (see nearby discussion regarding the redesign of
> > RLS..).  For my part, the nexts steps might be to consider how you'd
> > migrate what you've provided for configuration into catalog tables and
> > how we'd address the concerns raised elsewhere regarding catalog access
> > in cases where we're not in a transaction (or at least addressing those
> > areas and working out what the logging would do in those situations..).
> 
> I think the whole idea of storing audit info in catalogs should go away
> entirely.  There are, it seems to me, too many problems with that.

I'm completely against the notion of managing auditing requirements and
configurations which reference tables, users, and other objects which
exist in the catalog by using flat files.  To me, that's ridiculous on
the face of it.  Other databases have had this kind of capability as a
matter of course for decades- we are far behind in this area.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] RLS Design the rewriter into the planner?

2014-06-25 Thread Stephen Frost
Robert, all,

Changing the thread topic to match the other one, and adding Dean in
explicitly since we're talking about the design discussed with him.

* Robert Haas (robertmh...@gmail.com) wrote:
> I think role is good enough.  That's the primary identifier for all
> access-control related decisions, so it should be good enough here,
> too.

Alright.  That works for me.

> I don't really understand your concerns about overlapping policies
> being complex.  If you've got a couple of WHERE clauses, combining
> them with OR is not hard.  Now, the query optimizer may have trouble
> with it, but on the whole I expect to win more than we lose, by
> entirely excluding some branches of an OR for users for whom entirely
> policies can be excluded.

On the thread with Dean we're proposing some specific catalog designs
and part of that included (fleshing it out a bit more) something like:

CREATE TABLE pg_relrlspolicy (-- relation RLS policy table
  ptblrelid  oid, -- Relation/table
  ptblaction text,-- SELECT, INSERT, UPDATE, DELETE
  ptblpolid  oid, -- Policy
  ptblquals  text,-- Quals to add
  ptblaclaclitem[],   -- Rights to use this policy on the table

  primary key (ptblrelid, ptblaction)
);

And note that I had expected aclitem to only include one entry per role.

To support overlapping policies, we could add 'ptblpolid' into the
primary key and then simply extract out all of the entries for the
relation and action that we're currently running and step through each
to find which of the policies apply to the current_role...?

If a role has policyA with 'INSERT' rights, but no rights to SELECT, but
they also have an entry for policyB with 'SELECT' rights, we would use
only policyB for a SELECT query?  Does that approach mean we don't need
'ptblaction' after all?  I'm thinking this approach would also toss out
the "pick your policy" concept that Dean had proposed up-thread.

How would these interact with the existing table-level rights?  For
column-level rights, if you have access to SELECT the column then you
don't need any table-level rights (and the table-level rights mean you
can SELECT from any column), are we thinking the same would apply here,
such that having 'USING POLICY' rights means you can SELECT from the
table and the table-level rights end up being the 'DIRECT' rights which
had been discussed up-thread?  Not sure that I like that approach,
though I understand some others might find it appealing..  As we're
integrating this with the GRANT command, perhaps it'd be alright.

> > Overall, while I'm interested in defining where this is going in a way
> > which allows us implement an initial RLS capability while avoiding
> > future upgrade issues, I am perfectly happy to say that the 9.5 RLS
> > implementation may not be exactly syntax-compatible with 9.6 or 10.0.
> 
> Again, I think that's completely non-viable.  Are you going to tell
> people they can't pg_upgrade, and they can't dump-and-reload either,
> without manual fiddling?  There's no way that's going to be accepted.

I don't understand what you're getting at here.  We dump the catalog
using the newer version of pg_dump for pg_upgrade and we could handle
any *syntax* change required during that process to ensure that the same
access is granted in the new cluster as existed in the old cluster.

We do the exact same thing every time we add a new reserved keyword-
anything which used that keyword before ends up getting double-quoted by
the new version of pg_dump and both pg_dump and pg_upgrade work just
fine.  We routinly break some syntax compatibility between major
versions, address those changes in the newer version of pg_dump, and
move on.

I am not proposing that users won't be able to upgrade from 9.5 to 9.6
if they have RLS and agree that it'd be a non-starter.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] postgresql.auto.conf and reload

2014-06-25 Thread Christoph Berg
Re: Amit Kapila 2014-06-25 

> The main reason behind such such a behaviour after restart is
> that there are duplicate entries, one in postgresql.conf and
> another in postgresql.conf.  It always first read postgresql.conf
> and then .auto file and applies the changed parameters one by one,
> so when it reads a different value than current setting, it can lead
> to such messages.  During processing of config files it doesn't try
> to eliminate duplicate entries.
> 
> You can observe same behaviour incase you have another conf
> file (special.conf, containing conflicting settings) and include that
> in postgresql.conf.

Or even two statements for the same guc in postgresql.conf itself.

> One way could be that while processing if we could eliminate
> duplicate entries, then it will not lead to such messages, but I think
> that is existing mechanism and not introduced by Alter System,
> so changing just for Alter System might impact some existing users.

Yes, it's the same mechanism as before, but now we have a tool that
was specifically meant to be used to set and override postgresql.conf
parameters, so duplicate entries in postgresql.conf and
postgresql.auto.conf are no longer user error (or weirdness), but
expected. The system should deal with it.

> I think maintaining values both in postgresql.conf and by Alter System
> is not advisable.

Possibly, but then the system should be warning about all options, not
just the restart-only ones. And it should warn at startup, not at
reload time.

> I am not sure if this addresses your concern completely, but I thinking
> changing some existing mechanism (maintaining duplicate entries during
> processing of config files) at this point might be risky.

Not sure how intrusive a fix would be - collect all settings during
config parse, and only warn once everything has been seen, instead of
emitting warnings line-by-line.

Christoph
-- 
c...@df7cb.de | http://www.df7cb.de/


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


Re: [HACKERS] postgresql.auto.conf and reload

2014-06-25 Thread Amit Kapila
On Wed, Jun 25, 2014 at 7:45 PM, Amit Kapila 
wrote:
>
> On Wed, Jun 25, 2014 at 6:47 PM, Christoph Berg  wrote:
> > Re: Amit Kapila 2014-06-25 <
caa4ek1log98jvfov9wztqpcdewja+5jr54ttpkiz3xbngjy...@mail.gmail.com>
> > > This will happen without Alter System as well, if you change
> > > the value of port in postgresql.conf and try to load conf file with
SIGHUP.
> > > You cannot reload PGC_POSTMASTER parameters without server restart.
> >
> > That's not the issue. I did restart the server, which didn't log any
> > problems, yet reload keeps complaining indefinitely. This makes ALTER
> > SYSTEM not-so-nice-to-use to override parameters already set in
> > postgresql.conf.
>
> The main reason behind such such a behaviour after restart is
> that there are duplicate entries, one in postgresql.conf and
> another in postgresql.conf.

Sorry, I mean to say one in postgresql.conf and another in
postgresql.auto.conf

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


Re: [HACKERS] postgresql.auto.conf and reload

2014-06-25 Thread Amit Kapila
On Wed, Jun 25, 2014 at 6:47 PM, Christoph Berg  wrote:
> Re: Amit Kapila 2014-06-25 <
caa4ek1log98jvfov9wztqpcdewja+5jr54ttpkiz3xbngjy...@mail.gmail.com>
> > This will happen without Alter System as well, if you change
> > the value of port in postgresql.conf and try to load conf file with
SIGHUP.
> > You cannot reload PGC_POSTMASTER parameters without server restart.
>
> That's not the issue. I did restart the server, which didn't log any
> problems, yet reload keeps complaining indefinitely. This makes ALTER
> SYSTEM not-so-nice-to-use to override parameters already set in
> postgresql.conf.

The main reason behind such such a behaviour after restart is
that there are duplicate entries, one in postgresql.conf and
another in postgresql.conf.  It always first read postgresql.conf
and then .auto file and applies the changed parameters one by one,
so when it reads a different value than current setting, it can lead
to such messages.  During processing of config files it doesn't try
to eliminate duplicate entries.

You can observe same behaviour incase you have another conf
file (special.conf, containing conflicting settings) and include that
in postgresql.conf.

One way could be that while processing if we could eliminate
duplicate entries, then it will not lead to such messages, but I think
that is existing mechanism and not introduced by Alter System,
so changing just for Alter System might impact some existing users.

I think maintaining values both in postgresql.conf and by Alter System
is not advisable.

I am not sure if this addresses your concern completely, but I thinking
changing some existing mechanism (maintaining duplicate entries during
processing of config files) at this point might be risky.

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


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-06-25 Thread Abhijit Menon-Sen
At 2014-06-25 00:10:55 -0400, sfr...@snowman.net wrote:
>
> For my part, the nexts steps might be to consider how you'd migrate
> what you've provided for configuration into catalog tables

I must confess that I do not understand what needs to be migrated into
the catalog tables, or why. Of course, pgaudit.log must be renamed, but
why can't it continue to be a GUC setting? (Fujii-san suggested that it
be integrated with log_statement. I'm not sure what I think of that, but
it's certainly one possibility.)

> and how we'd address the concerns raised elsewhere regarding catalog
> access in cases where we're not in a transaction

…by not putting things into the catalog?

If we implement per-object auditing configuration in-core, it can use a
real reloption. Apart from that, I don't see a really good reason yet to
put more things into the database.

> We'd also end up re-working the code to be called as part of PG core
> rather than through hook functions, of course, but I don't think those
> changes would be too bad compared to figuring out the other issues.

You're right (but we'd still want to use event triggers). Maybe it would
make sense to have an auditing hook that we can sprinkle calls to in all
the interesting places, though.

> Additionally, thought towards what the SQL-level syntax would be is
> another key point- would the main command be 'ALTER AUDIT'?

(I have some thoughts about that, but I'll discuss them later when I
have a bit more time to present them properly.)

-- Abhijit


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


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-06-25 Thread Alvaro Herrera
Stephen Frost wrote:
> Abhijit,
> 
> * Abhijit Menon-Sen (a...@2ndquadrant.com) wrote:
> > At 2014-06-24 14:02:11 -0400, sfr...@snowman.net wrote:
> > >
> > > Will you (collectively) be working in this direction for 9.5?
> > 
> > We have some time available to work on it, but not so much that I want
> > to write any more code without a clearer idea of what might be accepted
> > eventually for inclusion.
> 
> You and me both... (see nearby discussion regarding the redesign of
> RLS..).  For my part, the nexts steps might be to consider how you'd
> migrate what you've provided for configuration into catalog tables and
> how we'd address the concerns raised elsewhere regarding catalog access
> in cases where we're not in a transaction (or at least addressing those
> areas and working out what the logging would do in those situations..).

I think the whole idea of storing audit info in catalogs should go away
entirely.  There are, it seems to me, too many problems with 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] Wait free LW_SHARED acquisition - v0.2

2014-06-25 Thread Amit Kapila
On Tue, Jun 24, 2014 at 9:33 AM, Amit Kapila 
wrote:
> On Mon, Jun 23, 2014 at 9:12 PM, Andres Freund 
wrote:
> > On 2014-06-23 19:59:10 +0530, Amit Kapila wrote:
> > > 7.
> > > LWLockWaitForVar()
> > > {
> > > ..
> > > /*
> > >  * Add myself to wait queue. Note that this is racy, somebody else
> > >  * could wakeup before we're finished queuing.
> > >  * NB: We're using nearly the same twice-in-a-row lock acquisition
> > >  * protocol as LWLockAcquire(). Check its comments for details.
> > >  */
> > > LWLockQueueSelf(l, LW_WAIT_UNTIL_FREE);
> > >
> > > /* we're now guaranteed to be woken up if necessary */
> > >  mustwait = LWLockAttemptLock(l, LW_EXCLUSIVE, false,
> > > &potentially_spurious);
> > > }
> > >
> > > Why is it important to Attempt lock after queuing in this case, can't
> > > we just re-check exclusive lock as done before queuing?
> >
> > Well, that's how Heikki designed LWLockWaitForVar().
>
> In that case I might be missing some point here, un-patched code of
> LWLockWaitForVar() never tries to acquire the lock, but the new code
> does so.  Basically I am not able to think what is the problem if we just
> do below after queuing:
> mustwait = pg_atomic_read_u32(&lock->lockcount) != 0;
>
> Could you please explain what is the problem in just rechecking?


I have further reviewed the lwlock related changes and thought
its good to share my findings with you. This completes my initial
review for lwlock related changes and below are my findings:

1.
LWLockRelease()
{
..
TRACE_POSTGRESQL_LWLOCK_RELEASE(T_NAME(l), T_ID(l));
}

Dynamic tracing macro seems to be omitted from LWLockRelease()
call.

2.
LWLockWakeup()
{
..
#ifdef LWLOCK_STATS
lwstats->spin_delay_count += SpinLockAcquire(&lock->mutex);
#else
SpinLockAcquire(&lock->mutex);
#endif
..
}

Earlier while releasing lock, we don't count it towards LWLock stats
spin_delay_count.  I think if we see other places in lwlock.c, it only gets
counted when we try to acquire it in a loop.

3.
LWLockRelease()
{
..
/* grant permission to run, even if a spurious share lock increases
lockcount */
else if (mode == LW_EXCLUSIVE && have_waiters)
check_waiters = true;
/* nobody has this locked anymore, potential exclusive lockers get a chance
*/
else if (lockcount == 0 && have_waiters)
check_waiters = true;
..
}

It seems comments have been reversed in above code.

4.
LWLockWakeup()
{
..
dlist_foreach_modify(iter, (dlist_head *) &l->waiters)
..
}

Shouldn't we need to use volatile variable in above loop (lock instead of
l)?

5.
LWLockWakeup()
{
..
dlist_foreach_modify(iter, (dlist_head *) &wakeup)
{
PGPROC *waiter = dlist_container(PGPROC, lwWaitLink, iter.cur);
LOG_LWDEBUG("LWLockRelease", l, mode, "release waiter");
dlist_delete(&waiter->lwWaitLink);
pg_write_barrier();
waiter->lwWaiting = false;
PGSemaphoreUnlock(&waiter->sem);
}
..
}

Why can't we decrement the nwaiters after waking up? I don't think
there is any major problem even if callers do that themselves, but
in some rare cases LWLockRelease() might spuriously assume that
there are some waiters and tries to call LWLockWakeup().  Although
this doesn't create any problem, keeping the value sane is good unless
there is some problem in doing so.

6.
LWLockWakeup()
{
..
dlist_foreach_modify(iter, (dlist_head *) &l->waiters)
{
..
if (wokeup_somebody && waiter->lwWaitMode == LW_EXCLUSIVE)
continue;
..
if (waiter->lwWaitMode != LW_WAIT_UNTIL_FREE)
{
..
wokeup_somebody = true;
}
..
}
..
}

a.
IIUC above logic, if the waiter queue is as follows:
(S-Shared; X-Exclusive) S X S S S X S S

it can skip the exclusive waiters and release shared waiter.

If my understanding is right, then I think instead of continue, there
should be *break* in above logic.

b.
Consider below sequence of waiters:
(S-Shared; X-Exclusive) S S X S S

I think as per un-patched code, it will wakeup waiters uptill (including)
first Exclusive, but patch will wake up uptill (*excluding*) first
Exclusive.

7.
LWLockWakeup()
{
..
dlist_foreach_modify(iter, (dlist_head *) &l->waiters)
{
..
dlist_delete(&waiter->lwWaitLink);
dlist_push_tail(&wakeup, &waiter->lwWaitLink);
..
}
..
}

Use of dlist has simplified the code, but I think there might be a slight
overhead of maintaining wakeup queue as compare to un-patched
mechanism especially when there is a long waiter queue.

8.
LWLockConditionalAcquire()
{
..
/*
 * We ran into an exclusive lock and might have blocked another
 * exclusive lock from taking a shot because it took a time to back
 * off. Retry till we are either sure we didn't block somebody (because
 * somebody else certainly has the lock) or till we got it.
 *
 * We cannot rely on the two-step lock-acquisition protocol as in
 * LWLockAcquire because we're not using it.
 */
if (potentially_spurious)
{
SPIN_DELAY();
goto retry;
}
..
}

Due to above logic, I think it can keep on retrying for long time before
it actually concludes whether it got lock or not incase other backend/'s
takes Exclusive lock after *double_check* and release before
un

Re: [HACKERS] RLS Design

2014-06-25 Thread Stephen Frost
* Dean Rasheed (dean.a.rash...@gmail.com) wrote:
> On 25 June 2014 01:49, Stephen Frost  wrote:
> > I can't recall a system where managers have to request access to their
> > manager role.  Having another way of changing the permissions which are
> > applied to a session (the existing one being 'set role') doesn't strike
> > me as a great idea either.
> >
> 
> Actually I think it's quite common to build applications where more
> privileged users might want to initially log in with normal
> privileges, and then only escalate to a higher privilege level if
> needed (much like only being root on a machine when absolutely
> necessary). But as you say, that can be done through 'set role' so I
> don't think being able to choose between policies is as important as
> being able to define different policies for different roles.

For those kinds of applications (eg: sudo), yes.  I was, perhaps,
looking at your example a bit too literally- I was thinking of HR
management type systems (timecard systems, etc). 

> > You mention:
> >
> > GRANT SELECT (col1, col2), UPDATE (col1) ON t1 TO bob USING policy1;
> >
> > but, to be clear, there would be no option for policies to be
> > column-specific, right?  The policy would apply to the whole row and
> > just the SELECT/UPDATE privileges would be on the specific columns (as
> > exists today).
> >
> 
> I think that would be OK for the first release. It could be extended
> in a future release to support column-specific policy ACLs, as long as
> we don't preclude that in the syntax we choose now. The syntax
> 
> GRANT  [,] ON table TO role USING policy
> 
> works because columns can be added to it later.

What would per-column RLS policies mean..?  Would we have to work out
which columns are being updated vs. select'd on before being able to
choose the policy/quals to include?  Seems like that's probably workable
but I've not thought about it very hard.

> > From this what I'm gathering is that we'd need catalog tables along
> > these lines:
> >
> > rls_policy
> >   oid, polname name, polowner oid, polnamespace oid, polacl aclitme[]
> >   (oid, policy name, policy owner, policy namespace, ACL, eg: usage?)
> >
> > rls_policy_table
> >   ptblpolid oid, ptblrelid oid, ptblquals text(?), ptblacl aclitem[]?
> >   (policy oid, table/relation oid, quals, ACL)
> >
> > pg_class
> >   relhasrls boolean ?
> 
> Seems about right.
> 
> > An extension to the existing ACLs which are for GRANT to include a
> > policy OID, eg:
> >
> > typedef struct AclItem
> > {
> > Oid ai_grantee;
> > Oid ai_grantor;
> > AclMode ai_privs;
> > Oid rls_policy;
> > }
> >
> 
> Alternatively, use the ACLs on rls_policy_table - i.e., to SELECT from
> a table using a particular policy, you would need to have the SELECT
> bit assigned to you in the corresponding rls_policy_table entry's
> ACLs. That seems like it would be a less invasive change, but I don't
> know if there are other problems with that approach.

Ah, that's a good thought.  My original thinking for that column was
some kind of privilege structure around who is allowed to modify the
quals for a given policy+table, but using that as the definition of who
has what policies does make sense and means we can leave AclItem
more-or-less alone, which is very nice.  The relhasrls boolean would
allow us to only query that catalog in cases where a policy exists,
hopefully minimizing the impact for users who are not using RLS.

> > and further:
> >
> > role1=r|p1/postgres
> > role2=r|p2/postgres
> 
> Or just
> 
> table1:
>   role1=rw/grantor
> table1 using policy1:
>   role2=rw/grantor
> 
> to avoid changing the privilege display pattern. That's also more in
> keeping with the model of storing the per-policy ACLs in
> rls_policy_table.

I like that output, but do we expect any pushback from users who parse
out that field?  Admittedly, they really shouldn't be doing that, but
I'm sure most actually do, and "table1 using policy1" isn't terribly
nice to parse.

> > or even:
> >
> > bob=|policy1/postgres
> >
> > with no table-level privileges and only column-level privileges granted
> > to role3 for this table.
> 
> I don't get that last one. If there are no table-level privileges,
> would it not just be empty?

No, as there could be column-level privileges.  Note that table-level
privileges get you access to all columns, and column level privileges
(as defined by SQL spec) give you access even if you don't have any
table-level privileges.  As I was trying to exclude the notion of
column-level policies, I figured policies would always show up in the
"table" level ACL fields, but if there aren't any table-level rights,
what would that look like?  With your proposal, it'd be:

table1 using policy1:
  bob=/grantor

?

> > The plan cache would include what policy OID a given plan was run under
> > (with InvalidOid indicating an "everything-allowed" policy).
> >
> > This doesn't address the concern raised 

Re: [HACKERS] postgresql.auto.conf and reload

2014-06-25 Thread Christoph Berg
Re: Amit Kapila 2014-06-25 

> On Wed, Jun 25, 2014 at 6:11 PM, Christoph Berg  wrote:
> >
> > I've just run into this:
> >
> > $ psql -p 5433   (that port is configured in postgresql.conf)
> > # alter system set port = 5494;
> >
> > ... restart the server
> >
> > $ psql -p 5494
> > # select pg_reload_conf();
> >
> > 2014-06-25 14:22:07 CEST [11297-4] LOG:  received SIGHUP, reloading
> configuration files
> > 2014-06-25 14:22:07 CEST [11297-5] LOG:  parameter "port" cannot be
> changed without restarting the server
> > 2014-06-25 14:22:07 CEST [11297-6] LOG:  configuration file
> "/etc/postgresql/9.4/main/postgresql.conf" contains errors; unaffected
> changes were applied
> 
> This will happen without Alter System as well, if you change
> the value of port in postgresql.conf and try to load conf file with SIGHUP.
> You cannot reload PGC_POSTMASTER parameters without server restart.

That's not the issue. I did restart the server, which didn't log any
problems, yet reload keeps complaining indefinitely. This makes ALTER
SYSTEM not-so-nice-to-use to override parameters already set in
postgresql.conf.

Christoph
-- 
c...@df7cb.de | http://www.df7cb.de/


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


Re: [HACKERS] postgresql.auto.conf and reload

2014-06-25 Thread Devrim Gündüz

Hi,

On Wed, 2014-06-25 at 18:42 +0530, Amit Kapila wrote:
> This will happen without Alter System as well, if you change
> the value of port in postgresql.conf and try to load conf file with
> SIGHUP. You cannot reload PGC_POSTMASTER parameters without server
> restart.

Ok, but Christoph already started the server (successfully) with port
5494, so the pg_reload_conf() should ignore it in the next attempt.

Regards,
-- 
Devrim GÜNDÜZ
Principal Systems Engineer @ EnterpriseDB: http://www.enterprisedb.com
PostgreSQL Danışmanı/Consultant, Red Hat Certified Engineer
Twitter: @DevrimGunduz , @DevrimGunduzTR



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


Re: [HACKERS] API change advice: Passing plan invalidation info from the rewriter into the planner?

2014-06-25 Thread Robert Haas
On Tue, Jun 24, 2014 at 12:27 PM, Stephen Frost  wrote:
> I feel like we are getting to the point of simply talking past each
> other and so I'll try anew, and I'll include my understanding of how the
> different approaches would address the specific use-case you outlined
> up-thread.

Thanks, you're right, and this is a good write-up.

> Single policy
> -
> The current implementation approach only allows a single policy to be
> included.
> [...snip...]
> For the case where a sales rep isn't also a partner, you could simplify
> this to:
>
> WHERE
>   sales_rep_id = 16384
>
> but I'm not sure that really buys you much?  With the bitmap heap
> scan, if one side of the OR ends up not returning anything then it
> doesn't contribute to the blocks which have to be scanned.  The index
> might still need to be scanned, although I think you could avoid even
> that with an EXISTS check to see if the user is a partner at all.
> That's not to say that a bitmap scan is equivilant to an index scan, but
> it's certainly likely to be far better than a sequential scan.

True, but the wins could be much larger if one policy is WHERE
sales_rep_id = (SELECT oid FROM pg_roles WHERE rolname = current_user)
and the other policy is WHERE complexfn().  I'll also throw out a +1
for Dean's comments on this topic.

> Multiple, Non-overlapping policies
> --
> Preventing the overlap of policies ends up being very complicated if
> many dimensions are allowed.  For the simple case, perhaps only the
> 'current role' dimension is useful.  I expect that going down that
> route would very quickly lead to requests for other dimensions (client
> IP, etc) which is why I'm not a big fan of it, but if that's the
> concensus then let's work out the syntax and update the patch and move
> on.

I think role is good enough.  That's the primary identifier for all
access-control related decisions, so it should be good enough here,
too.

I don't really understand your concerns about overlapping policies
being complex.  If you've got a couple of WHERE clauses, combining
them with OR is not hard.  Now, the query optimizer may have trouble
with it, but on the whole I expect to win more than we lose, by
entirely excluding some branches of an OR for users for whom entirely
policies can be excluded.

> Overall, while I'm interested in defining where this is going in a way
> which allows us implement an initial RLS capability while avoiding
> future upgrade issues, I am perfectly happy to say that the 9.5 RLS
> implementation may not be exactly syntax-compatible with 9.6 or 10.0.

Again, I think that's completely non-viable.  Are you going to tell
people they can't pg_upgrade, and they can't dump-and-reload either,
without manual fiddling?  There's no way that's going to be accepted.

-- 
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] postgresql.auto.conf and reload

2014-06-25 Thread Amit Kapila
On Wed, Jun 25, 2014 at 6:11 PM, Christoph Berg  wrote:
>
> I've just run into this:
>
> $ psql -p 5433   (that port is configured in postgresql.conf)
> # alter system set port = 5494;
>
> ... restart the server
>
> $ psql -p 5494
> # select pg_reload_conf();
>
> 2014-06-25 14:22:07 CEST [11297-4] LOG:  received SIGHUP, reloading
configuration files
> 2014-06-25 14:22:07 CEST [11297-5] LOG:  parameter "port" cannot be
changed without restarting the server
> 2014-06-25 14:22:07 CEST [11297-6] LOG:  configuration file
"/etc/postgresql/9.4/main/postgresql.conf" contains errors; unaffected
changes were applied

This will happen without Alter System as well, if you change
the value of port in postgresql.conf and try to load conf file with SIGHUP.
You cannot reload PGC_POSTMASTER parameters without server restart.



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


[HACKERS] pg_filedump for 9.4?

2014-06-25 Thread Devrim Gündüz

Hi,

Will there be a pg_filedump for 9.4? I'd like to finish package tests
before we release 9.4.0.

Regards,
-- 
Devrim GÜNDÜZ
Principal Systems Engineer @ EnterpriseDB: http://www.enterprisedb.com
PostgreSQL Danışmanı/Consultant, Red Hat Certified Engineer
Twitter: @DevrimGunduz , @DevrimGunduzTR



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


Re: [HACKERS] ALTER SYSTEM RESET?

2014-06-25 Thread Amit Kapila
On Wed, Jun 25, 2014 at 6:20 PM, Christoph Berg  wrote:
>
> Hi,
>
> is there a reason there's no ALTER SYSTEM RESET?
>
> The natural idiom to reset SET statements is "RESET guc;", I don't
> think "SET guc = default;" is in use much, so "ALTER SYSTEM RESET guc;"
> would be the natural way to try.

Currently you can achieve that by
"ALTER SYSTEM RESET guc = Default;".
However it will be good to have support for RESET as well.  I think it
should not be too complicated to implement that syntax, I personally
don't have bandwidth to it immediately, but I would like to take care
of it unless you or someone wants to do it by the time I get some
bandwidth.


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


Re: [HACKERS] PostgreSQL for VAX on NetBSD/OpenBSD

2014-06-25 Thread Robert Haas
On Wed, Jun 25, 2014 at 5:30 AM, John Klos  wrote:
> A high end VAX, such as a 4000 Model 108, can have 512 megs (as can an
> 11/780, at least in theory), but most of the VAXen used here are VAXstations
> such as the 4000/60 or 4000/90, 90a or 96, which have either 104 megs or 128
> megs.

Hmm, not bad for old hardware.

> and it launched fine. I then tried to run:
>
> gmake MAX_CONNECTIONS=5 installcheck
>
> in
> /usr/pkgsrc/databases/postgresql93-server/work/postgresql-9.3.4/src/test/regress,
> but it failed with:
>
> ...
> gmake[2]: Leaving directory
> '/usr/pkgsrc/databases/postgresql93-server/work/postgresql-9.3.4/src/backend'
> gcc -O1 -fgcse -fstrength-reduce -fgcse-after-reload -I/usr/include
> -I/usr/local/include -Wall -Wmissing-prototypes -Wpointer-arith
> -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute
> -Wformat-security -fno-strict-aliasing -fwrapv -pthread  -mt -D_REENTRANT
> -D_THREAD_SAFE -D_POSIX_PTHREAD_SEMANTICS -I../../src/port -DFRONTEND
> -I../../src/include -I/usr/include -I/usr/local/include   -c -o thread.o
> thread.c
> cc1: error: unrecognized command line option "-mt" : recipe for
> target 'thread.o' failed
> gmake[1]: *** [thread.o] Error 1
> gmake[1]: Leaving directory
> '/usr/pkgsrc/databases/postgresql93-server/work/postgresql-9.3.4/src/port'
> ../../../src/Makefile.global:423: recipe for target 'submake-libpgport'
> failed
> gmake: *** [submake-libpgport] Error 2
>
> That's all I have time for tonight. Is there an easier way to run a
> testsuite?

I think you're doing it right, but apparently configure is
mis-identifying which flags are needed for thread-safety on your
platform.  It's possible configuring with --disable-thread-safety
would help, or you could manually edit the Makefile.

In any case I'm coming to the conclusion that there's little point in
us keeping the VAX-specific code in our source tree, because in fact,
this port is broken and doesn't work.  Based on your results thus far,
I doubt that it would be a huge amount of work to fix that, but unless
somebody from the VAX community wants to volunteer to be a PostgreSQL
maintainer for that platform, straighten out the things that have
gotten broken since this port was originally added, and keep it
working on an ongoing basis, it's probably not going to happen.

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


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


[HACKERS] ALTER SYSTEM RESET?

2014-06-25 Thread Christoph Berg
Hi,

is there a reason there's no ALTER SYSTEM RESET?

The natural idiom to reset SET statements is "RESET guc;", I don't
think "SET guc = default;" is in use much, so "ALTER SYSTEM RESET guc;"
would be the natural way to try.

Also, ALTER SYSTEM SET/RESET seems to be what oracle does:
http://docs.oracle.com/cd/E11882_01/server.112/e40402/initparams004.htm#REFRN00102

Christoph
-- 
c...@df7cb.de | http://www.df7cb.de/


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


[HACKERS] postgresql.auto.conf and reload

2014-06-25 Thread Christoph Berg
I've just run into this:

$ psql -p 5433   (that port is configured in postgresql.conf)
# alter system set port = 5494;

... restart the server

$ psql -p 5494
# select pg_reload_conf();

2014-06-25 14:22:07 CEST [11297-4] LOG:  received SIGHUP, reloading 
configuration files
2014-06-25 14:22:07 CEST [11297-5] LOG:  parameter "port" cannot be changed 
without restarting the server
2014-06-25 14:22:07 CEST [11297-6] LOG:  configuration file 
"/etc/postgresql/9.4/main/postgresql.conf" contains errors; unaffected changes 
were applied

I think reloading shouldn't be nagging me about parameters in
postgresql.conf that are going to be overwritten in
postgresql.auto.conf. Reloading again will issue the warnings again...

It's true that my config has two contradicting settings in there now,
but if that's not an issue at server start, it shouldn't be one for
reloading either.

Christoph
-- 
c...@df7cb.de | http://www.df7cb.de/


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


Re: [HACKERS] RLS Design

2014-06-25 Thread Dean Rasheed
On 25 June 2014 01:49, Stephen Frost  wrote:
> Dean, all,
>
> Changing the subject of this thread (though keeping it threaded) as
> we've really moved on to a much broader discussion.
>
> * Dean Rasheed (dean.a.rash...@gmail.com) wrote:
>> On 24 June 2014 17:27, Stephen Frost  wrote:
>> > Single policy vs Multiple, Overlapping policies vs Multiple, 
>> > Non-overlapping policies
>>
>> What I was describing upthread was multiple non-overlapping policies.
>
> Ok.
>
>> I disagree that this will be more complicated to use. It's a strict
>> superset of the single policy functionality, so if you want to do it
>> all using a single policy then you can. But I think that once the ACLs
>> reach a certain level of complexity, you probably will want to break
>> it up into multiple policies, and I think doing so will make things
>> simpler, not more complicated.
>
> If we keep it explicitly to per-role only, with only one policy ever
> being applied, then perhaps it would be, but I'm not convinced..
>
>> Taking a specific, simplistic example, suppose you had 2 groups of
>> users - some are normal users who should only be able to access their
>> own records. For these users, you might have a policy like
>>
>>   WHERE person_id = current_user
>>
>> which would be highly selective, and probably use an index scan. Then
>> there might be another group of users who are managers with access to
>> the records of, say, everyone in their department. This might then be
>> a more complex qual along the lines of
>>
>>   WHERE person_id IN (SELECT ... FROM person_department
>>WHERE mgr_id = current_user AND ...)
>>
>> which might end up being a hash or merge join, depending on any
>> user-supplied quals.
>
> Certainly my experience with such a setup is that it includes at least 4
> levels (self, manager, director, officer).  Now, officer you could
> perhaps exclude as being simply RLS-exempt but with such a structure I
> would think we'd just make that a special kind of policy (and not chew
> up those last 4 bits).  As for this example, it's quite naturally done
> with a recursive query as it's a tree structure, but if you want to keep
> the qual simple and fast, you'd materialize the results of such a query
> and simply have:
>
> WHERE EXISTS (SELECT 1 from org_chart
>WHERE current_user = emp_id
>  AND person_id = org_chart.id)
>
>> You _could_ combine those into a single policy, but I think it would
>> be much better to have 2 distinct policies, since they're 2 very
>> different queries, for different use cases. Normal users would only be
>> granted permission to use the normal_user_policy. Managers might be
>> granted permission to use either the normal_user_policy or the
>> manager_policy (but not both at the same time).
>
> I can't recall a system where managers have to request access to their
> manager role.  Having another way of changing the permissions which are
> applied to a session (the existing one being 'set role') doesn't strike
> me as a great idea either.
>

Actually I think it's quite common to build applications where more
privileged users might want to initially log in with normal
privileges, and then only escalate to a higher privilege level if
needed (much like only being root on a machine when absolutely
necessary). But as you say, that can be done through 'set role' so I
don't think being able to choose between policies is as important as
being able to define different policies for different roles.


>> That's a very simplified example. In more realistic situations there
>> are likely to be many more classes of users, and trying to enforce all
>> the logic in a single WHERE clause is likely to get unmanageable, or
>> inefficient if it involves lots of logic hidden away in functions.
>
> Functions and external security systems are exactly the real-world
> use-case which users I've talked to are looking for.  All of this
> discussion is completely orthogonal to their requirements.  I understand
> that there are simpler use-cases than those and we may be able to
> provide an approach which performs better for those.
>

OK.


>> Allowing multiple, non-overlapping policies allows the problem to be
>> broken up into more manageable pieces, which also makes the planner's
>> job easier, since only a single, simpler policy is in effect in any
>> given query.
>
> Let's try to outline what this would look like then.
>
> Taking your approach, we'd have:
>
> CREATE POLICY p1;
> CREATE POLICY p2;
>
> ALTER TABLE t1 SET POLICY p1 TO t1_p1_quals;
> ALTER TABLE t1 SET POLICY p2 TO t1_p2_quals;
>
> GRANT SELECT ON TABLE t1 TO role1 USING p1;
> GRANT SELECT ON TABLE t1 TO role2 USING p2;
>
> I'm guessing we would need to further support:
>
> GRANT INSERT ON TABLE t1 TO role1 USING p2;
>
> as we've already discussed being able to support per-action (SELECT,
> INSERT, UPDATE, DELETE) policies.  I'm not quite sure how to address
> that though.
>
> Further, as you mention, use

[HACKERS] 9.3 minor release soon?

2014-06-25 Thread Bruce Momjian
Are we nearing a minor release deadline for 9.3?  There will need to be
fix-up query in the minor release notes for pg_upgrade:


http://www.postgresql.org/message-id/20140530121631.ge25...@alap3.anarazel.de

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

  + Everyone has their own god. +


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


Re: [HACKERS] Allowing NOT IN to use ANTI joins

2014-06-25 Thread Simon Riggs
On 24 June 2014 23:22, Simon Riggs  wrote:

>> On a more positive or even slightly exciting note I think I've managed to
>> devise a way that ANTI JOINS can be used for NOT IN much more often. It
>> seems that find_nonnullable_vars will analyse a quals list to find
>> expressions that mean that the var cannot be NULL. This means we can perform
>> ANTI JOINS for NOT IN with queries like:
>>
>> SELECT * FROM a WHERE id NOT IN(SELECT nullable_col FROM b WHERE
>> nullable_col = 1);
>> or
>> SELECT * FROM a WHERE id NOT IN(SELECT nullable_col FROM b WHERE
>> nullable_col IS NOT NULL);
>>
>> (The attached patch implements this)
>>
>> the nullable_col =1 will mean that nullable_col cannot be NULL, so the ANTI
>> JOIN can be performed safely. I think this combined with the NOT NULL check
>> will cover probably just about all valid uses of NOT IN with a subquery...
>> unless of course I've assumed something wrongly about find_nonnullable_vars.
>> I just need the correct RangeTblEntry in order to determine if the
>> TargetEntry is from an out join.
>
> This is the better way to go. It's much better to have explicit proof
> its not null than a possibly long chain of metadata that might be
> buggy.
>
>> The attached patch is a broken implemention that still needs the lookup code
>> fixed to reference the correct RTE. The failing regression tests show where
>> the problems lie.
>>
>> Any help on this would be really appreciated.
>
> I'd suggest we just drop the targetlist approach completely.

To be clearer, what I mean is we use only the direct proof approach,
for queries like this

  SELECT * FROM a WHERE id NOT IN(SELECT unknown_col FROM b WHERE
unknown_col IS NOT NULL);

and we don't try to do it for queries like this

  SELECT * FROM a WHERE id NOT IN(SELECT not_null_column FROM b);

because we don't know the full provenance of "not_null_column" in all
cases and that info is (currently) unreliable.

Once we have the transform working for one case, we can try to extend
the cases covered.

-- 
 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] Quantify small changes to predicate evaluation

2014-06-25 Thread Marti Raudsepp
On Tue, Jun 17, 2014 at 5:23 PM, Dennis Butterstein  wrote:
> I tried the proposed tweaks and
> see some differences regarding the measurements.
> Unfortunately the variance between the runs seems to remain high.

Using these techniques I managed to get standard deviation below 1.5%
in my read-only tests (and most below 1%). Not all workloads may be
able to achieve that, but your should be able to do better than your
results.

Maybe your cooling is not sufficient? It seems your 2nd run is always
slower than the first one, maybe the CPU is doing thermal throttling?
Lowering the max frequency might be something to try to resolve that,
like "cpupower frequency-set --max 2GHz"

How do you run your benchmark, are you using pgbench? Single threaded?
Is the client locked to the same CPU core?

Regards,
Marti


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


Re: [HACKERS] psql: show only failed queries

2014-06-25 Thread Samrat Revagade
Hi Pavel,

After applying patch, on error condition it displays error message two
times as follows:

ERROR:  column "abc" does not exist at character 23
STATEMENT:  insert into ax
values(abc);
psql:a.sql:7: ERROR:  column "abc" does not exist
LINE 2: values(abc);


user may confuse because of repeated error messages. so I think its better
to display only one message, one of the possible ways is as follows:

ERROR:  column "abc" does not exist at character 23
STATEMENT:  insert into ax
values(abc);


Am I missing something ?



On Wed, Jun 4, 2014 at 9:52 PM, Pavel Stehule 
wrote:

>
>
>
> 2014-06-04 18:16 GMT+02:00 Peter Eisentraut :
>
> On 6/4/14, 11:54 AM, Pavel Stehule wrote:
>> > updated patch - only one change: query is prefixed by "QUERY:  "
>>
>> In the backend server log, this is called "STATEMENT:  ".
>>
>
> good idea
>
> updated patch
>
> Pavel
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>


-- 
Regards,

Samrat Revgade


Re: [HACKERS] pg_receivexlog add synchronous mode

2014-06-25 Thread Fujii Masao
On Wed, Jun 25, 2014 at 3:50 AM, Fujii Masao  wrote:
> On Tue, Jun 24, 2014 at 3:18 PM,   wrote:
>>> I found that this patch breaks --status-interval option of
>>> pg_receivexlog when -m option which the patch introduced is supplied.
>>> When -m is set, pg_receivexlog tries to send the feedback message as soon
>>> as it flushes WAL file even if status interval timeout has not been passed
>>> yet. If you want to send the feedback as soon as WAL is written or flushed,
>>> like walreceiver does, you need to extend --status-interval option, for
>>> example, so that it accepts the value "-1" which means enabling that
>>> behavior.
>>>
>>> Including this change in your original patch would make it more difficult
>>> to review. I think that you should implement this as separate patch.
>>> Thought?
>> As your comments, the current specification to ignore the --status-intarvall.
>> It is necessary to respond immediately to synchronize.
>>
>> It is necessary to think about specifications the --status-intarvall.
>> So I revised it to a patch of flushmode which performed flush by a timing 
>> same as walreceiver.
>
> I'm not sure if it's good idea to call the feature which you'd like to
> add as 'flush mode'.
> ISTM that 'flush mode' is vague and confusion for users. Instead, what
> about adding
> something like --fsync-interval which pg_recvlogical supports?
>
>> A changed part deletes the feedback message after flush, and transmitted the 
>> feedback message according to the status interval.
>> Change to flushmode from syncmode the mode name, and fixed the document.
>
> + * Receive a message available from XLOG stream, blocking for
> + * maximum of 'timeout' ms.
>
> The above comment seems incorrect because 'timeout' is boolean argument.
>
> +FD_ZERO(&input_mask);
> +FD_SET(PQsocket(conn), &input_mask);
> +if (standby_message_timeout)
>
> Why did you get rid of the check of 'still_sending' flag here? Originally the
> flag was checked but not in the patch.
>
> +r = rcv_receive(true , ©buf, conn,
> standby_message_timeout, last_status, now);
>
> When the return value is -2 (i.e., an error happend), we should go to
> the 'error' label.
>
> ISTM that stream_stop() should be called every time a message is
> processed. But the
> patch changes pg_receivexlog so that it keeps processing the received
> data without
> calling stream_stop(). This seems incorrect.
>
> 'copybuf' needs to be free'd every time new message is received. But you seem 
> to
> have forgotten to do that when rcv_receive() with no timeout is called.

The patch looks somewhat complicated and bugs can be easily introduced
because it tries to not only add new feature but also reorganize
the main loop in HandleCopyStream at the same time. To keep the patch
simple, I'm thinking to firstly apply the attached patch which just
refactors the main loop. Then we can apply the main patch, i.e., add new
feature. Thought?

Regards,

-- 
Fujii Masao
diff --git a/src/bin/pg_basebackup/receivelog.c b/src/bin/pg_basebackup/receivelog.c
index d76e605..1182dc7 100644
--- a/src/bin/pg_basebackup/receivelog.c
+++ b/src/bin/pg_basebackup/receivelog.c
@@ -35,6 +35,8 @@ static PGresult *HandleCopyStream(PGconn *conn, XLogRecPtr startpos,
  uint32 timeline, char *basedir,
 			   stream_stop_callback stream_stop, int standby_message_timeout,
  char *partial_suffix, XLogRecPtr *stoppos);
+static int CopyStreamPoll(PGconn *conn, long timeout_ms);
+static int CopyStreamReceive(PGconn *conn, long timeout, char **buffer);
 
 static bool ReadEndOfStreamingResult(PGresult *res, XLogRecPtr *startpos,
 		 uint32 *timeline);
@@ -744,12 +746,7 @@ HandleCopyStream(PGconn *conn, XLogRecPtr startpos, uint32 timeline,
 		int			bytes_written;
 		int64		now;
 		int			hdr_len;
-
-		if (copybuf != NULL)
-		{
-			PQfreemem(copybuf);
-			copybuf = NULL;
-		}
+		long		sleeptime;
 
 		/*
 		 * Check if we should continue streaming, or abort at this point.
@@ -784,67 +781,34 @@ HandleCopyStream(PGconn *conn, XLogRecPtr startpos, uint32 timeline,
 			last_status = now;
 		}
 
-		r = PQgetCopyData(conn, ©buf, 1);
-		if (r == 0)
+		/*
+		 * Compute how long send/receive loops should sleep
+		 */
+		if (standby_message_timeout && still_sending)
 		{
-			/*
-			 * No data available. Wait for some to appear, but not longer than
-			 * the specified timeout, so that we can ping the server.
-			 */
-			fd_set		input_mask;
-			struct timeval timeout;
-			struct timeval *timeoutptr;
-
-			FD_ZERO(&input_mask);
-			FD_SET(PQsocket(conn), &input_mask);
-			if (standby_message_timeout && still_sending)
-			{
-int64		targettime;
-long		secs;
-int			usecs;
-
-targettime = last_status + (standby_message_timeout - 1) * ((int64) 1000);
-feTimestampDifference(now,
-	  targettime,
-	  &secs,
-	  &usecs);
-if (secs <= 0)
-	timeout.tv_sec = 1; /* Always sleep at least 1 sec */
-else
-	timeout.tv_sec = secs;
-			

Re: [HACKERS] makeAndExpr(), etc. confined to gram.y?

2014-06-25 Thread Amit Langote
On Wed, Jun 25, 2014 at 1:27 PM, Tom Lane  wrote:
> Amit Langote  writes:
>> Is there a reason why they've been left out of
>> makefuncs.h/makefuncs.c? Perhaps they are not supposed to be used
>> outside gram.y at all? For example, previously a caller (potentially)
>> outside parser could do a makeA_Expr(AEXPR_AND, ...). I guess this is
>> no longer possible with AEXPR_AND gone?
>
> What would be the purpose?  There is noplace except gram.y that builds
> raw parse trees.
>

Yeah, that is true. Sorry, I am unaware as to how generic make*
functions in gram.y are and how they differ from those in makefuncs.c.

So, use of make* family of functions outside parser is their abuse in
some way? Anything that needs to use these functions should somehow be
accomplished in parser perhaps. For example, duplicate/redundant CHECK
expressions elimination and such?

--
Amit


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


Re: [HACKERS] Cluster name in ps output

2014-06-25 Thread Fujii Masao
>> * cluster_name moved to config group CONN_AUTH_SETTINGS, on the
>>   basis that it's similar to bonjour_name, but it isn't
>>   really... open to suggestions for a better config_group!

Categorizing this parameter to CONN_AUTH_SETTINGS looks strange to me
because it's not directly related to connection and authentication.
LOGGING_WHAT is better if we allow log_line_prefix to include the
cluster_name. Or STATS_COLLECTOR which update_process_title is
categorized to? STATS_COLLECTOR also looks a bit strange not only for
cluster_name but also update_process_title, though...

On Wed, Jun 25, 2014 at 1:29 PM, Abhijit Menon-Sen  wrote:
> The patch looks OK, and works as advertised (I tested on Linux). If we
> want the feature (I like it), this patch is a good enough way to get it.

I got the following compiler warning.

guc.c:3099: warning: initialization from incompatible pointer type

monitoring.sgml explains PS display. Isn't it better to update monitoring.sgml
so that it refers to cluster_name?

Regards,

-- 
Fujii Masao


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


Re: [HACKERS] "RETURNING PRIMARY KEY" syntax extension

2014-06-25 Thread Ian Barwick
Hi

On 14/06/25 15:13, Rushabh Lathia wrote:
> Hello All,
> 
> I assigned my self as reviewer of the patch. I gone through the
> mail chain discussion and in that question has been raised about
> the feature and its implementation, so would like to know what is
> the current status of this project/patch.
> 
> Regards,

I'll be submitting a revised version of this patch very shortly.


Regards

Ian Barwick

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