Re: [HACKERS] factor out encoding dependent json/jsonb regression tests

2015-10-05 Thread Tom Lane
Andrew Dunstan  writes:
> I'm rather tired of having to update two sets of output files every time 
> we change the json and jsonb regression tests. Also, other committers 
> occasionally fall over this trap and fail to update json_1.out and 
> jsonb_1.out, as do almost all other patch authors. I propose that we 
> remove those tests to quite small json_utf8.sql and jsonb_utf8.sql 
> (which will each have 2 output files) and that will allow us to get rid 
> of json_1.out and jsonb_1.out. I'd like to backpatch this to 9.4 for 
> jsonb and 9.2 for json. Alternatively, for >= 9.4 we could just add the 
> jsonb encoding-specific tests to json_utf8.sql, to avoid a greater 
> proliferation of tests.

I'd vote for your alternative plan (ie, just one new regression .sql
file).  It will be so small that having two even-smaller ones seems silly.

Is "json_utf8" really the best name, or should it be "json_encoding"?

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] Less than ideal error reporting in pg_stat_statements

2015-10-05 Thread Peter Geoghegan
On Mon, Oct 5, 2015 at 8:50 AM, Tom Lane  wrote:
> That's certainly something worth looking at, but I think it's probably
> more complicated than that.  If you just write "WHERE x IN (1,2,3,4)",
> that gets folded to a ScalarArrayOp with a single array constant, which
> the existing code would deal with just fine.  We need to identify
> situations where that's not the case but yet we shouldn't distinguish.

I have a certain amount of sympathy for doing that kind of thing, but
significantly less than I have for cases with many failed queries,
which is why I focused on that.

> In any case, that's just a marginal tweak for one class of query.

Sometimes narrow cases are also important and representative cases. I
don't care if someone has thrashing type issues with
pg_stat_statements when they're doing something really odd that calls
into question the purpose of using it to begin with [1]. The two
classes of queries we talked about (1. Many aborted data integration
transactions, and 2. A variable number of constants) are interesting
because a reasonable person could have those cases, and run into
trouble with pg_stat_statements as a consequence.

> I suspect the right fix for the core problem is the one Peter mentioned
> in passing earlier, namely make it possible to do garbage collection
> without having to slurp the entire file into memory at once.  It'd be
> slower, without a doubt, but we could continue to use the existing code
> path unless the file gets really large.

While it would be nice to not lose query texts on OOM, that's not my
primary concern. My primary concern is infrequent garbage collection.
This fix certainly isn't going to help with the fact that garbage
collection can be stalled for far too long in at least
quasi-reasonable cases.

[1] http://www.postgresql.org/message-id/52e9d98a.4000...@lab.ntt.co.jp
-- 
Peter Geoghegan


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


Re: [HACKERS] factor out encoding dependent json/jsonb regression tests

2015-10-05 Thread Peter Geoghegan
On Mon, Oct 5, 2015 at 10:32 AM, Andrew Dunstan  wrote:
> I'm rather tired of having to update two sets of output files every time we
> change the json and jsonb regression tests. Also, other committers
> occasionally fall over this trap and fail to update json_1.out and
> jsonb_1.out, as do almost all other patch authors. I propose that we remove
> those tests to quite small json_utf8.sql and jsonb_utf8.sql (which will each
> have 2 output files) and that will allow us to get rid of json_1.out and
> jsonb_1.out. I'd like to backpatch this to 9.4 for jsonb and 9.2 for json.
> Alternatively, for >= 9.4 we could just add the jsonb encoding-specific
> tests to json_utf8.sql, to avoid a greater proliferation of tests.

While I figured out a way of dealing with that that is minimally
annoying, it is a little confusing.

+1 to doing this, and +1 to backpatching. I don't think that there is
actually very much different between each output.

-- 
Peter Geoghegan


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


Re: [HACKERS] factor out encoding dependent json/jsonb regression tests

2015-10-05 Thread Andrew Dunstan



On 10/05/2015 01:53 PM, Tom Lane wrote:

Andrew Dunstan  writes:

I'm rather tired of having to update two sets of output files every time
we change the json and jsonb regression tests. Also, other committers
occasionally fall over this trap and fail to update json_1.out and
jsonb_1.out, as do almost all other patch authors. I propose that we
remove those tests to quite small json_utf8.sql and jsonb_utf8.sql
(which will each have 2 output files) and that will allow us to get rid
of json_1.out and jsonb_1.out. I'd like to backpatch this to 9.4 for
jsonb and 9.2 for json. Alternatively, for >= 9.4 we could just add the
jsonb encoding-specific tests to json_utf8.sql, to avoid a greater
proliferation of tests.

I'd vote for your alternative plan (ie, just one new regression .sql
file).  It will be so small that having two even-smaller ones seems silly.

Is "json_utf8" really the best name, or should it be "json_encoding"?



I'm fine with "json_enccoding". I'll do it that way.

cheers

andrew



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


Re: [HACKERS] factor out encoding dependent json/jsonb regression tests

2015-10-05 Thread Alvaro Herrera
Andrew Dunstan wrote:
> I'm rather tired of having to update two sets of output files every time we
> change the json and jsonb regression tests. Also, other committers
> occasionally fall over this trap and fail to update json_1.out and
> jsonb_1.out, as do almost all other patch authors. I propose that we remove
> those tests to quite small json_utf8.sql and jsonb_utf8.sql (which will each
> have 2 output files) and that will allow us to get rid of json_1.out and
> jsonb_1.out. I'd like to backpatch this to 9.4 for jsonb and 9.2 for json.

+1.

> Alternatively, for >= 9.4 we could just add the jsonb encoding-specific
> tests to json_utf8.sql, to avoid a greater proliferation of tests.

shrug

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


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


Re: [HACKERS] run pg_rewind on an uncleanly shut down cluster.

2015-10-05 Thread Oleksii Kliukin

> On 05 Oct 2015, at 18:04, Bruce Momjian  wrote:
> 
> On Mon, Oct  5, 2015 at 05:41:07PM +0200, Oleksii Kliukin wrote:
>> Hello,
>> 
>> I'm trying to find out how to rewind a cluster that was not shut down
>> cleanly, in order to implement pg_rewind support in patroni (an
>> automated failover system, https://github.com/zalando/patroni).
>> 
>> At the moment, pg_rewind limits itself to only cleanly shut down
>> clusters. This works nicely in the case of a split brain caused by the
>> network partitioning. However, it doesn't cover the important case of a
>> suddenly crashed master: the crashed cluster cannot be rewound to the
>> new master. 
> 
> Did you read this thread convering the same topic from a few weeks ago?
> 
>   
> http://www.postgresql.org/message-id/flat/55fa2537.4070...@gmx.net#55fa2537.4070...@gmx.net
>  
> 

Thanks, I saw it. The problem being discussed there is different from mine: I 
need to rewind a crashed master, not a replica being shut down in recovery. And 
I’m looking for something that be achieved with 9.3 or 9.4, and there are 
evidences (at least suggestions in the thread linked from my previous post) 
that it should work.

Kind regards,
--
Oleksii



[HACKERS] factor out encoding dependent json/jsonb regression tests

2015-10-05 Thread Andrew Dunstan
I'm rather tired of having to update two sets of output files every time 
we change the json and jsonb regression tests. Also, other committers 
occasionally fall over this trap and fail to update json_1.out and 
jsonb_1.out, as do almost all other patch authors. I propose that we 
remove those tests to quite small json_utf8.sql and jsonb_utf8.sql 
(which will each have 2 output files) and that will allow us to get rid 
of json_1.out and jsonb_1.out. I'd like to backpatch this to 9.4 for 
jsonb and 9.2 for json. Alternatively, for >= 9.4 we could just add the 
jsonb encoding-specific tests to json_utf8.sql, to avoid a greater 
proliferation of tests.


Thoughts?

cheers

andrew


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


[HACKERS] Tsvector editing functions

2015-10-05 Thread Stas Kelvich
Hello.

There is patch that adds some editing routines for tsvector type.

tsvector delete(tsvector, text)
removes entry from tsvector by lexeme name
set unnest(tsvector)
expands a tsvector to a set of rows. Each row has following columns: 
lexeme, postings, weights.
text[] to_array(tsvector)
converts tsvector to array of lexemes
tsvector to_tsvector(text[])
converts array of lexemes to tsvector




tsvector_funcs.diff
Description: Binary data


Stas Kelvich
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company



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


[HACKERS] Obsolete comment in tidpath.c

2015-10-05 Thread Etsuro Fujita
I think "best_inner_indexscan()" in the following comment in tidpath.c
is obsolete.

 * There is currently no special support for joins involving CTID; in
 * particular nothing corresponding to best_inner_indexscan().  Since it's
 * not very useful to store TIDs of one table in another table, there
 * doesn't seem to be enough use-case to justify adding a lot of code
 * for that.

How about s/best_inner_indexscan()/parameterized scans/?

Best regards,
Etsuro Fujita



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


Re: [HACKERS] No Issue Tracker - Say it Ain't So!]

2015-10-05 Thread Oleg Bartunov
On Mon, Oct 5, 2015 at 3:08 AM, Nathan Wagner  wrote:

> On Sun, Oct 04, 2015 at 04:30:49PM -0700, Josh Berkus wrote:
> > That would be the key part, wouldn't it?  Nice that you have [code to
> > store and parse email messages].
>
> Yeah.  It actually made most of the work pretty easy.  It's available
> with a bunch of other code at https://pd.if.org/git/ if anyone wants it.
> I did find a bug in my header processing though, so I'll need to commit
> that fix.
>
> > We'd also want a way to link a bug fix to a commit, and probably a way
> > to give the bug a list of searchable keywords (and add to that list).
>
> I've been thinking of hooking it up to the fti machinery and providing
> a search box.  I've never really used fti before, so this might be a
> good opportunity to learn it for real.
>

Nathan, there are many options in our fts, which can be useful, so +1 to
help you.



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


Re: [HACKERS] No Issue Tracker - Say it Ain't So!]

2015-10-05 Thread Magnus Hagander
On Mon, Oct 5, 2015 at 2:08 AM, Nathan Wagner  wrote:

> On Sun, Oct 04, 2015 at 04:30:49PM -0700, Josh Berkus wrote:
> > That would be the key part, wouldn't it?  Nice that you have [code to
> > store and parse email messages].
>
> Yeah.  It actually made most of the work pretty easy.  It's available
> with a bunch of other code at https://pd.if.org/git/ if anyone wants it.
> I did find a bug in my header processing though, so I'll need to commit
> that fix.
>

Note that we already have all the emails in a database, as parsed out for
archives.postgresql.org. There is also an API for this, but it's only
available internally. This one is used for example by the commitfest app,
which is in many ways doing things that are very similar to this one. If we
were to proceed down this road, I would strongly suggest that we utilize
this API (at least then we will have a consistent set of MIME parsing
bugs..)



> > We'd also want a way to link a bug fix to a commit, and probably a way
> > to give the bug a list of searchable keywords (and add to that list).
>
> I've been thinking of hooking it up to the fti machinery and providing
> a search box.  I've never really used fti before, so this might be a
> good opportunity to learn it for real.
>


Again, we already have an API for searching the archives that can do this
for us. No need to re-invent the wheel there. (And it's based on the
PostgreSQL fts functionality - of course)



>
> > > it probably makes sense to just close up front bugs that are marked
> > > against unsupported pg versions, or haven't had any activity for too
> > > long, perhaps two years.
>
> > I'm reluctant to close all of those unexamined, since part of the
> > purpose of this is to find bugs which were never fixed.  Probably we
> > should organize a posse to comb trhough all of the old bugs and
> > hand-close them.
>
> I'm doing some of that as I poke at the bugs pages.  Perhaps it would
> make sense to have a closed status of 'stale' or the like (perhaps
> that's what you meant by 'timed out') which could be used to get bugs
> out of the main list but still be marked as 'not human reviewed'.  These
> could then be reviewed by the posse.
>


I think this is definitely a state that's needed, no matter what it's
called.  In particular in the beginning.

But if looking at the bugtrackers at other projects, this is a state that
often holds a *lot* of bugs. And having an explicit one like that would
make it easier for all the volunteers to know what to look at immediately -
it would be a good goal for such a group to ensure that no bugs remain in
that state.




> > Yeah, fixing this [email's without a bug id] would probably be tied to
> > the possible change to mailman.  Unless someone already has a way to
> > get majordomo to append a bug ID.
>
> How are bug ids assigned now?  From the evidence, I assume there is a
> sequence in a database that the web submission form queries to format a
> resulting email to the bugs mailing list.  Do the mailing lists live on
> the same server?  If so, perhaps it would be easy to assign a new bug id
> to a new thread on -bugs.  But perhaps that's too aggressive in creating
> bugs.
>


Correct, that's exactly what it does.

I don't think we want to assign a new bug id to a new thread immediately.
Given how many people post a new thread referencing an old one.

I think we'd want an interface that lets you either pick an existing thread
on bugs that has no bug id and create one for it, or pick a thread and
attach it to an existing thread. Note that we also need to cover things
like threads on hackers (it's very common that a thread is opened on
hackers about the same point), as well as the enentual commit message.

Again, this is fairly similar to what the commitfest app does, by allowing
you to attach multiple threads.

I'm not saying it's a good idea to use the CF app as-is, but the fact is
that much of what it does is very similar - it adds a bunch of metadata to
mailthreads and tracks that metadata. Which is pretty much what this tool
would/should do. But it's an almost completely different set of metadata,
so I don't think merging them is a good idea.




>
> > > 5: How can we use email to update the status of a bug?
> > >
> > > I suggest using email headers to do this.  'X-PGBug-Fixed:
> > > ' and the like.  I assume here that everyone who might
> > > want to do such a thing uses an MUA that would allow this, and they
> > > know how.
> >
> > I guess that depends on who we expect to use this, at least for
> > closing stuff.
>
> I could certainly support more than one mechanism.  A web interface for
> those who would prefer such and emails would seem to be the basic
> requirements.
>

Using email headers I think is a no-go. Way too many of the people who
would be doing it don' use MUAs that let them do that. I think the way to
go is in-message keywords at the start of a line. And in that case
everybody should use those, to make things 

Re: [HACKERS] Foreign join pushdown vs EvalPlanQual

2015-10-05 Thread Etsuro Fujita

On 2015/09/29 16:36, Etsuro Fujita wrote:

For the foreign table case (scanrelid>0), I imagined an approach
different than yours.  In that case, I thought the issue would be
probably addressed by just modifying the remote query performed in
RefetchForeignRow, which would be of the form "SELECT ctid, * FROM
remote table WHERE ctid = $1", so that the modified query would be of
the form "SELECT ctid, * FROM remote table WHERE ctid = $1 AND *remote
quals*".


Sorry, I was wrong.  I noticed that the modifieid query (that will be 
sent to the remote server during postgresRefetchForeignRow) should be of 
the form "SELECT * FROM (SELECT ctid, * FROM remote table WHERE ctid = 
$1) ss WHERE *remote quals*".  (I think the query of the form "SELECT 
ctid, * FROM remote table WHERE ctid = $1 AND *remote quals*" would be 
okay if using a TID scan on the remote side, though.)


Best regards,
Etsuro Fujita



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


Re: [HACKERS] Shouldn't CREATE TABLE LIKE copy the relhasoids property?

2015-10-05 Thread Bruce Momjian
On Wed, Apr 29, 2015 at 03:48:04PM -0400, Bruce Momjian wrote:
> On Tue, Apr 28, 2015 at 09:15:49AM -0400, Bruce Momjian wrote:
> > > It seems to me that waiting for 9.6 for what's arguably a bug fix is too
> > > much.  It's not like this is a new feature.  Why don't we just make sure
> > > it is as correct as possible and get it done for 9.5?  It's not even in
> > > beta yet, nor feature freeze.
> > 
> > Well, I applied what I thought would work, but did not handle three
> > cases:
> > 
> > *  checking of hasoids by index specifications
> > *  queries with multiple LIKE'ed tables
> > *  matching inheritance behavior
> > 
> > I am unclear if I should be addressing such complex issues at this point
> > in the development cycle.  I can certainly apply this patch, but I need
> > someone else to tell me it is good and should be applied.  I am also
> > thinking such review time would be better spent on patches submitted
> > long before mine.
> 
> I have added regression tests to the patch, attached.  I have included
> Tom's test that doesn't directly use LIKE.

Patch applied.

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

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription +


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


Re: [HACKERS] Multi-tenancy with RLS

2015-10-05 Thread Haribabu Kommi
On Fri, Sep 11, 2015 at 7:50 AM, Joe Conway  wrote:
> On 09/01/2015 11:25 PM, Haribabu Kommi wrote:
>> If any user is granted any permissions on that object then that user
>> can view it's meta data of that object from the catalog tables.
>> To check the permissions of the user on the object, instead of
>> checking each and every available option, I just added a new
>> privilege check option called "any". If user have any permissions on
>> the object, the corresponding permission check function returns
>> true. Patch attached for the same.
>>
>> Any thoughts/comments?
>
> Thanks for working on this! Overall I like the concept and know of use
> cases where it is critical and should be supported. Some comments:

Here I attached an updated version of the patch with the following changes.

Two options to the user to create catalog security on system catalog tables.

./initdb -C -D data

With the above option during initdb, the catalog security is enabled
on all shared system catalog
tables. With this way the user can achieve the catalog security at
database level. For some users
this may be enough. Currently enabling catalog security is supported
only at initdb.

ALTER DATABASE  WITH CATALOG SECURITY=true;
ALTER DATABASE  WITH CATALOG SECURITY=false;

With the above commands, user can enable/disable catalog security on a
database system catalog
tables if multi-tenancy requires at table level.

Currently setting catalog security at create database command is not
supported. And also with
alter database command to enable/disable to database where the backend
is not connected.
This is because of a restriction to execute the policy commands
without connecting to a database.


Pending things needs to be taken care:

1. select * from tenancy_user1_tbl1;
ERROR:  permission denied for relation tenancy_user1_tbl1

As we are not able to see the above user table in any catalog relation
because of the multi-tenancy policies,
but if user tries to select the data of the table directly, The error
message comes as permission denied, I feel
instead of the permission denied error, in case of multi-tenancy is
enabled, the error message should be
"relation doesn't exist".

2. Correct all catalog relation policies
3. Add regression tests for all system catalog relations and views.
4. Documentation changes

Any comments?

Regards,
Hari Babu
Fujitsu Australia


multi-tenancy_with_rls_poc_3.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] CREATE POLICY and RETURNING

2015-10-05 Thread Zhaomo Yang
Stephen,

I just tried a little bit your patch for applying SELECT policies to
DELETE/UPDATE. It is consistent with the GRANT system so it looks
really good. I'll test it more thoroughly later.

Also, I guess we don't need to worry about the syntax of "restrictive
policies" you mentioned in the upthread since SELECT policies are
essentially restrictive now. Since that work has already been done,
I'm wondering if I can take the task of allowing policies to reference
both the 'old' and 'new' versions of the row. I understand that this
feature won't be considered for 9.5 but I'd like to implement it and
hopefully get it incorporated into 9.6.

Thanks,
Zhaomo


On Wed, Sep 23, 2015 at 11:54 AM, Stephen Frost  wrote:
> * Zhaomo Yang (zmp...@gmail.com) wrote:
>> > Just a side-note, but your mail client doesn't seem to get the quoting
>> > quite right sometimes, which can be confusing.  Not sure if there's
>> > anything you can do about it but wanted to let you know in case there
>> > is.
>>
>> Sorry about this. From now on I'll use the plain text mode for msgs I
>> send to the mailing list.
>> Please let me know if this happens also in this email.
>
> Looks like this one has all of the quoting correct- thanks!
>
>> > Regarding this, specifically, we'd need to first decide on what the
>> > syntax/grammar should be.
>>
>> I'll think about it. Also, thanks for the pointers.
>
> Sure, no problem.
>
>> > Right, and we adressed the concerns with RETURNING.  Regarding the
>> > non-RETURNING case, The same concerns about blind updates and deletes
>> > already exist with the GRANT permission system; it's not anything new.
>>
>> I think they are different. In the current GRANT permission system,
>> one can do blind updates but he
>> cannot refer to any existing values in either the expressions or the
>> condition if he doesn't have
>> SELECT privilege on the table (or the columns), thus the tricks like
>> divide-by-zero cannot be used and a malicious
>> user cannot get information out of blind updates.
>
> Ok, I see what you're getting at with that and I believe it'll be a
> pretty straight-forward change, thanks to Dean's recent rework.  I'll
> take a look at making that happens.
>
> Thanks!
>
> Stephen


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


Re: [HACKERS] CREATE POLICY and RETURNING

2015-10-05 Thread Stephen Frost
Zhaomo,

* Zhaomo Yang (zmp...@gmail.com) wrote:
> I just tried a little bit your patch for applying SELECT policies to
> DELETE/UPDATE. It is consistent with the GRANT system so it looks
> really good. I'll test it more thoroughly later.

Great!  Glad to hear it.

> Also, I guess we don't need to worry about the syntax of "restrictive
> policies" you mentioned in the upthread since SELECT policies are
> essentially restrictive now.

They are when it comes to applying them on top of other policies to
match the permissions system, but what I believe we'd like is the
ability to *explicitly* make policies both restrictive and permissive.
That would allow a user to create a set of permissive SELECT policies
and than a set of restrictive SELECT policies, which might be much
simpler to manage for their particular use-case.

> Since that work has already been done,
> I'm wondering if I can take the task of allowing policies to reference
> both the 'old' and 'new' versions of the row. I understand that this
> feature won't be considered for 9.5 but I'd like to implement it and
> hopefully get it incorporated into 9.6.

I'd love to see a patch for that for 9.6.  Feel free to work on it and
ping me with any questions you have.  Once you have a patch, please make
sure to add it to the appropriate commitfest (via
http://commitfest.postgresql.org), so it won't be lost.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] [Proposal] Table partition + join pushdown

2015-10-05 Thread Taiki Kondo
Hello, KaiGai-san.

Thank you for your comment, and sorry for late response.

> If inner-scan of the join under the append node has param_info, its qualifier 
> shall be implicitly attached to the scan node. So, if it is legal, I'd like 
> to have this approach because it is less invasive than enhancement of Hash 
> node.
> 
> You mention about copyObject() to make a duplication of underlying scan-path.
> Actually, copyObject() support is not minimum requirement, because all you 
> need to do here is flat copy of the original path-node, then put param_info.
> (Be careful to check whether the original path is not parametalized.)

OK. I'll try implementation by a method you mentioned.


Best regards,
--
Taiki Kondo

NEC Solution Innovators, Ltd.



-Original Message-
From: Kaigai Kouhei(海外 浩平) [mailto:kai...@ak.jp.nec.com] 
Sent: Wednesday, September 30, 2015 11:19 PM
To: Kondo Taiki(近藤 太樹)
Cc: Iwaasa Akio(岩浅 晃郎); pgsql-hackers@postgresql.org
Subject: RE: [HACKERS] [Proposal] Table partition + join pushdown

> > * Suppose we focus on only HashJoin in the first version?
> > This patch also add support on NestLoop and MergeJoin, however, 
> > NestLoop has no valuable scenario without parallel execution 
> > capability, and the most valuable scenario on MergeJoin is reduction of 
> > rows prior to Sort.
> > Once input rows gets sorted, it is less attractive to filter out rows.
> 
> I agree that handling for NestLoop doesn't make sense in this timing.
> But I think that handling for MergeJoin still makes sense in this timing.
> 
> In my v1 patch, I implemented that the additional filter is used for 
> qualification at same place as join filter, same as NestLoop.
> It is not useful indeed. I agree with you at this point.
> 
> I think, and you also mentioned, large factor of cost estimation for 
> MergeJoin is Sort under MergeJoin, so I believe additional filtering 
> at Sort is a good choice for this situation, as same way at Hash under 
> HashJoin.
>
> Furthermore, I think it is better way that the additional filtering 
> shall be "added" to Scan node under each child (pushed-down) Join 
> nodes, because we don't have to implement additional qualification at 
> Join nodes and we only have to implement simply concatenating original 
> and additional RestrictInfos for filtering.
> 
> As a mere idea, for realizing this way, I think we have to implement 
> copyObject() for Scan path nodes and use ppi_clauses for this usage.
> 
> What is your opinion?
>

You are saying this part at create_scan_plan(), aren't you.

/*
 * If this is a parameterized scan, we also need to enforce all the join
 * clauses available from the outer relation(s).
 *
 * For paranoia's sake, don't modify the stored baserestrictinfo list.
 */
if (best_path->param_info)
scan_clauses = list_concat(list_copy(scan_clauses),
   best_path->param_info->ppi_clauses);

If inner-scan of the join under the append node has param_info, its qualifier 
shall be implicitly attached to the scan node. So, if it is legal, I'd like to 
have this approach because it is less invasive than enhancement of Hash node.

You mention about copyObject() to make a duplication of underlying scan-path.
Actually, copyObject() support is not minimum requirement, because all you need 
to do here is flat copy of the original path-node, then put param_info.
(Be careful to check whether the original path is not parametalized.)

ParamPathInfo is declared as below:

typedef struct ParamPathInfo
{
NodeTag type;

Relids  ppi_req_outer;  /* rels supplying parameters used by path */
double  ppi_rows;   /* estimated number of result tuples */
List   *ppi_clauses;/* join clauses available from outer rels */
} ParamPathInfo;

You may need to set the additional filter on ppi_clauses, number of rows after 
the filtering on ppi_rows and NULL on ppi_req_outer.
However, I'm not 100% certain whether NULL is legal value on ppi_req_outer.

If somebody can comment on, it is helpful.

> > * MultiExecHash() once put slot on outer_slot then move it to 
> > inner_slot This patch add set_hash_references() to replace varno in 
> > the expression of Hash->filterqual to OUTER_VAR. Why not INNER_VAR?
> > If Var nodes would be initialized t oreference inner_slot, you don't 
> > need to re-assign slot.
> 
> The node under Hash node is connected as the OUTER node. This 
> implementation may be from implementation of 
> set_dummy_tlist_references() commonly used by Material, Sort, Unique, 
> SetOp, and Hash.
> 
> And I was faced a problem when I was implementing EXPLAIN for the additional 
> filter.
> I implemented same as you mentioned above, then error occurred in running 
> EXPLAIN.
> I think EXPLAIN expects expression's varno is same as the position 
> that the under node is connected to; i.e. if it is connected to OUTER, 
> varno must be OUTER_VAR.
>
Ah, OK. It is a 

Re: [HACKERS] Rename withCheckOptions to insertedCheckClauses

2015-10-05 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Stephen Frost  writes:
> > * Dean Rasheed (dean.a.rash...@gmail.com) wrote:
> >> Also, these were added in 9.4, so introducing this many differences
> >> between 9.4 and 9.5+ will make back-patching harder.
> 
> > That's certainly true, but we don't want current or future hackers to be
> > confused either.
> 
> Yes.  I do not think that we should stick with badly chosen names just
> because of back-patching concerns.  By that argument, we should never
> fix any erroneous comments either.
> 
> Whether these particular names are improvements is, of course, a fit
> subject for debate.  I have to agree that I don't feel like we've quite
> hit on le mot juste yet.

I've gone ahead and at least removed the withCheckOptions empty-list
from being written out as part of Query for 9.5 and HEAD, and bumped
catversion accordingly.

I came to realize that ModifyTable actually is planned to be used for
parallel query and therefore the list for that needs to stay, along with
support for reading the WithCheckOption node in.

Thanks!

Stephen


signature.asc
Description: Digital signature


[HACKERS] Use EVP API pgcrypto encryption, dropping support for OpenSSL 0.9.6 and older

2015-10-05 Thread Heikki Linnakangas
pgcrypto uses the old, deprecated, "low-level" functions for symmetric 
encryption, with algorithm-specific functions like AES_ecb_encrypt(), 
DES_ecb3_encrypt() and so forth. The recommended new API is the 
so-called EVP API, which has functions for initializing a "context" 
using a specific algorithm, and then that context is passed around to 
EVP_Encrypt*/Decrypt* functions. The EVP API has been around for ages, 
at least since OpenSSL 0.9.6.


We should switch to the new API. Aside from being nicer, the low-level 
functions don't (necessarily) use hardware acceleration, while the EVP 
functions do. I could see a significant boost to pgcrypto AES encryption 
on my laptop, which has an Intel CPU that supports the special AES-NI 
instructions. That said, AES encryption is pretty fast anyway, so you 
need very large inputs to see any difference and it's actually pretty 
difficult to come up with a test case where the gains are not lost in 
the noise of e.g. toasting/detoasting the data. Nevertheless, it's a 
nice bonus. Test case is attached (aes-speedtest.sql). It runs in about 
1.7s with the old API, and 1.3s with the new API.


The real reason I started digging this, though, is that Pivotal was 
trying to use the FIPS-validated version of OpenSSL with PostgreSQL, and 
it turns out that the low-level APIs are disabled in "FIPS mode", and 
trip an assertion inside OpenSSL (that changed some time between 0.9.8 
and 1.0.2, not sure when exactly). Switching to the EVP functions will 
avoid that problem. There is obviously a lot more you'd need to do 
before you could actually FIPS-certify PostgreSQL and pgcrypto, but this 
is one unnecessary hurdle.


There was prior discussion on the EVP API in this old thread from 2007: 
http://www.postgresql.org/message-id/flat/46a5e284.7030...@sun.com#46a5e284.7030...@sun.com


In short, pgcrypto actually used to use the EVP functions, but was 
changed to *not* use them, because in older versions of OpenSSL, some 
key lengths and/or padding options that pgcrypto supports were not 
supported by the EVP API. That was fixed in OpenSSL 0.9.7, however. The 
consensus in 2007 was that we could drop support for OpenSSL 0.9.6 and 
below, so that should definitely be OK by now, if we haven't already 
done that elsewhere in the code.


Any objections to the attached two patches?

- Heikki


aes-speedtest.sql
Description: application/sql
From 9cbf787e22b06b56b2cd05d871feb09c684094b3 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas 
Date: Mon, 5 Oct 2015 15:43:44 +0300
Subject: [PATCH 1/2] Remove support for OpenSSL versions before 0.9.7

This makes the code simpler.
---
 contrib/pgcrypto/openssl.c | 116 +
 1 file changed, 1 insertion(+), 115 deletions(-)

diff --git a/contrib/pgcrypto/openssl.c b/contrib/pgcrypto/openssl.c
index 976af70..bd257b0 100644
--- a/contrib/pgcrypto/openssl.c
+++ b/contrib/pgcrypto/openssl.c
@@ -34,6 +34,7 @@
 #include "px.h"
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -47,121 +48,6 @@
 #define MAX_IV		(128/8)
 
 /*
- * Compatibility with OpenSSL 0.9.6
- *
- * It needs AES and newer DES and digest API.
- */
-#if OPENSSL_VERSION_NUMBER >= 0x00907000L
-
-/*
- * Nothing needed for OpenSSL 0.9.7+
- */
-
-#include 
-#else			/* old OPENSSL */
-
-/*
- * Emulate OpenSSL AES.
- */
-
-#include "rijndael.c"
-
-#define AES_ENCRYPT 1
-#define AES_DECRYPT 0
-#define AES_KEY		rijndael_ctx
-
-static int
-AES_set_encrypt_key(const uint8 *key, int kbits, AES_KEY *ctx)
-{
-	aes_set_key(ctx, key, kbits, 1);
-	return 0;
-}
-
-static int
-AES_set_decrypt_key(const uint8 *key, int kbits, AES_KEY *ctx)
-{
-	aes_set_key(ctx, key, kbits, 0);
-	return 0;
-}
-
-static void
-AES_ecb_encrypt(const uint8 *src, uint8 *dst, AES_KEY *ctx, int enc)
-{
-	memcpy(dst, src, 16);
-	if (enc)
-		aes_ecb_encrypt(ctx, dst, 16);
-	else
-		aes_ecb_decrypt(ctx, dst, 16);
-}
-
-static void
-AES_cbc_encrypt(const uint8 *src, uint8 *dst, int len, AES_KEY *ctx, uint8 *iv, int enc)
-{
-	memcpy(dst, src, len);
-	if (enc)
-	{
-		aes_cbc_encrypt(ctx, iv, dst, len);
-		memcpy(iv, dst + len - 16, 16);
-	}
-	else
-	{
-		aes_cbc_decrypt(ctx, iv, dst, len);
-		memcpy(iv, src + len - 16, 16);
-	}
-}
-
-/*
- * Emulate DES_* API
- */
-
-#define DES_key_schedule des_key_schedule
-#define DES_cblock des_cblock
-#define DES_set_key(k, ks) \
-		des_set_key((k), *(ks))
-#define DES_ecb_encrypt(i, o, k, e) \
-		des_ecb_encrypt((i), (o), *(k), (e))
-#define DES_ncbc_encrypt(i, o, l, k, iv, e) \
-		des_ncbc_encrypt((i), (o), (l), *(k), (iv), (e))
-#define DES_ecb3_encrypt(i, o, k1, k2, k3, e) \
-		des_ecb3_encrypt((des_cblock *)(i), (des_cblock *)(o), \
-*(k1), *(k2), *(k3), (e))
-#define DES_ede3_cbc_encrypt(i, o, l, k1, k2, k3, iv, e) \
-		des_ede3_cbc_encrypt((i), (o), \
-(l), *(k1), *(k2), *(k3), (iv), (e))
-
-/*
- * Emulate newer digest API.
- */
-
-static void
-EVP_MD_CTX_init(EVP_MD_CTX *ctx)
-{
-	memset(ctx, 0, 

Re: [HACKERS] Connection string parameter 'replication' in documentation

2015-10-05 Thread Kyotaro HORIGUCHI
Hello,

At Fri, 2 Oct 2015 23:13:24 +0900, Takashi Ohnishi  wrote 
in 

Re: [HACKERS] No Issue Tracker - Say it Ain't So!]

2015-10-05 Thread Magnus Hagander
On Mon, Oct 5, 2015 at 12:42 AM, Nathan Wagner 
wrote:

> I don't have the original message for this thread, so I arbitrarily picked
> a
> message to reply to.
>
> Since what has been asked for is a bug *tracker*, and we already have a
> bugs
> mailing list, I put together something.
>


FWIW, I think this is a good approach in general. This makes it a  bug
*tracker*, rather than a "workflow enforcer". That depends on what we want
of course, but those are two very different things and many of the other
tools suggested are more workflow enforcers.

Something like debbugs falls in the same category though, I think, as being
mainly a tracker. Keeping the mailinglists as the first-class way of
communications, which is what we prefer.

It's dirt simple.  If the system sees a message with 'Bug #(\d+)' in the
> subject line, it creates an entry in a bugs table with that bug number (if
> needed), and then marks the message as belonging to that bug.  If there
> seems
> to be metadata about the bug in the format of the (unquoted)
>
> Bug reference:
> Logged by:
> Email address:
> PostgreSQL version:
> Operating system:
> Description:
> Details:
>


FWIW, if we end up going with this method and it makes things easier, we
could easily add such metadata as X- headers to the original bug email,
thereby making it even easier (and safer) to parse out.


4: What should I do with emails that don't reference a bug id but seem to be
> talking about a bug?
>
> I suggest we do nothing with them as far as the bug tracker is concerned.
> If
> people want to mark their message as pertaining to a bug, they can put
> that in
> the subject line.  However, I don't think a bug id can be assigned via
> email,
> that is, I think you have to use a web form to create a bug report with a
> bug
> id.  Presumably that could change if whoever runs the bug counter wants it
> to.
>

It cannot now, but we can fix that. And if we want to use a tool like this,
we should fix it. Using something like submit...@postgresql.org. But we
should also make it possible to assign a bug post-email. Someone might
email -general with a bug report, and it's a lot more friendly to just
assign ita bug than to say "hey take this thing you just wrote and re-paste
it into a form over here instead", just to get a duplicate posted into our
archivse.


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


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

2015-10-05 Thread Masahiko Sawada
On Sat, Oct 3, 2015 at 3:41 AM, Robert Haas  wrote:
> On Fri, Oct 2, 2015 at 11:23 AM, Alvaro Herrera
>  wrote:
>>> + /* all-frozen information is also cleared at the same time */
>>>   PageClearAllVisible(page);
>>> + PageClearAllFrozen(page);
>>
>> I wonder if it makes sense to have a macro to clear both in unison,
>> which seems a very common pattern.
>
> I think PageClearAllVisible should clear both, and there should be no
> other macro.  There is no event that causes a page to cease being
> all-visible that does not also cause it to cease being all-frozen.
> You might think that deleting or locking a tuple would fall into that
> category - but nope, XMAX needs to be cleared or the tuple pruned, or
> there will be problems after wraparound.
>

Thank you for your advice.
I understood.

I changed the patch so that PageClearAllVisible clear both bits, and
removed ClearAllFrozen.
Attached the latest v16 patch which contains draft version documentation patch.

Regards,

--
Masahiko Sawada
diff --git a/contrib/pgstattuple/pgstatapprox.c b/contrib/pgstattuple/pgstatapprox.c
index 22c5f7a..b1b6a06 100644
--- a/contrib/pgstattuple/pgstatapprox.c
+++ b/contrib/pgstattuple/pgstatapprox.c
@@ -87,7 +87,7 @@ statapprox_heap(Relation rel, output_type *stat)
 		 * If the page has only visible tuples, then we can find out the free
 		 * space from the FSM and move on.
 		 */
-		if (visibilitymap_test(rel, blkno, ))
+		if (visibilitymap_test(rel, blkno, , VISIBILITYMAP_ALL_VISIBLE))
 		{
 			freespace = GetRecordedFreeSpace(rel, blkno);
 			stat->tuple_len += BLCKSZ - freespace;
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 97ef618..f8aa18b 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -1841,6 +1841,18 @@
  
 
  
+   relallfrozen
+   int4
+   
+   
+Number of pages that are marked all-frozen in the tables's
+visibility map. It is updated by VACUUM.
+ANALYZE, and a few DDL coomand such as
+CREATE INDEX.
+   
+ 
+
+ 
   reltoastrelid
   oid
   pg_class.oid
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 5081da0..6bd4d57 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -5900,7 +5900,7 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
   
   

-VACUUM performs a whole-table scan if the table's
+VACUUM performs a aggressive freezing if the table's
 pg_class.relfrozenxid field has reached
 the age specified by this setting.  The default is 150 million
 transactions.  Although users can set this value anywhere from zero to
@@ -5944,7 +5944,7 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
   
   

-VACUUM performs a whole-table scan if the table's
+VACUUM performs a aggressive freezing if the table's
 pg_class.relminmxid field has reached
 the age specified by this setting.  The default is 150 million multixacts.
 Although users can set this value anywhere from zero to two billions,
diff --git a/doc/src/sgml/maintenance.sgml b/doc/src/sgml/maintenance.sgml
index b5d4050..c8ad27f 100644
--- a/doc/src/sgml/maintenance.sgml
+++ b/doc/src/sgml/maintenance.sgml
@@ -352,9 +352,9 @@
 Vacuum maintains a visibility map for each
 table to keep track of which pages contain only tuples that are known to be
 visible to all active transactions (and all future transactions, until the
-page is again modified).  This has two purposes.  First, vacuum
-itself can skip such pages on the next run, since there is nothing to
-clean up.
+page is again modified), and pages contain only tuples that are marked as
+frozen.  This has two purposes.  First, vacuum itself can skip such pages
+on the next run, since there is nothing to clean up.

 

@@ -438,23 +438,22 @@

 

-VACUUM normally skips pages that don't have any dead row
-versions, but those pages might still have row versions with old XID
-values.  To ensure all old row versions have been frozen, a
-scan of the whole table is needed.
+VACUUM skips pages that don't have any dead row
+versions, and pages that have only frozen rows.
+To ensure all old row versions have been frozen, a scan of all pages that
+are not marked as frozen is needed.
  controls when
-VACUUM does that: a whole table sweep is forced if
-the table hasn't been fully scanned for vacuum_freeze_table_age
-minus vacuum_freeze_min_age transactions. Setting it to 0
-forces VACUUM to always scan all pages, effectively ignoring
-the visibility map.
+VACUUM does that: a table sweep is forced if
+the table hasn't been ensured all row versions are frozen for
+vacuum_freeze_table_age minus vacuum_freeze_min_age
+

Re: [HACKERS] Parallel Seq Scan

2015-10-05 Thread Amit Kapila
On Sun, Oct 4, 2015 at 10:21 AM, Amit Kapila 
wrote:
>
> On Sat, Oct 3, 2015 at 11:35 PM, Robert Haas 
wrote:
> >
> > On Fri, Oct 2, 2015 at 11:44 PM, Amit Kapila 
wrote:
> > > On Thu, Oct 1, 2015 at 7:41 PM, Robert Haas 
wrote:
> > >> Does heap_parallelscan_nextpage really need a pscan_finished output
> > >> parameter, or can it just return InvalidBlockNumber to indicate end
of
> > >> scan?  I think the latter can be done and would be cleaner.
> > >
> > > I think we can do that way as well, however we have to keep the check
> > > of page == InvalidBlockNumber at all the callers to indicate finish
> > > of scan which made me write the function as it exists in patch.
However,
> > > I don't mind changing it the way you have suggested if you feel that
would
> > > be cleaner.
> >
> > I think it would.  I mean, you just end up testing the other thing
instead.
> >
>
> No issues, will change in next version of patch.
>

Changed as per suggestion.

> > >>  I think that heap_parallelscan_initialize() should taken an
> > >> additional Boolean argument, allow_sync.  It should decide whether to
> > >> actually perform a syncscan using the logic from initscan(), and then
> > >> it should store a phs_syncscan flag into the ParallelHeapScanDesc.
> > >> heap_beginscan_internal should set rs_syncscan based on phs_syncscan,
> > >> regardless of anything else.
> > >
> > > I think this will ensure that any future change in this area won't
break the
> > > guarantee for sync scans for parallel workers, is that the reason you
> > > prefer to implement this function in the way suggested by you?
> >
> > Basically.  It seems pretty fragile the way you have it now.
> >
>
> Okay, in that case I will change it as per your suggestion.
>

Changed as per suggestion.

> > >> I think heap_parallel_rescan() is an unworkable API.  When rescanning
> > >> a synchronized scan, the existing logic keeps the original
start-block
> > >> so that the scan's results are reproducible,
> > >
> > > It seems from the code comments in initscan, the reason for keeping
> > > previous startblock is to allow rewinding the cursor which doesn't
hold for
> > > parallel scan.  We might or might not want to support such cases with
> > > parallel query, but even if we want to there is a way we can do with
> > > current logic (as mentioned in one of my replies below).
> >
> > You don't need to rewind a cursor; you just need to restart the scan.
> > So for example if we were on the inner side of a NestLoop, this would
> > be a real issue.
> >
>
> Sorry, but I am not able to see the exact issue you have in mind for
NestLoop,
> if we don't preserve the start block position for parallel scan.


For now, I have fixed this by not preserving the startblock incase of rescan
for parallel scan. Note that, I have created a separate patch
(parallel_seqscan_heaprescan_v1.patch) for support of rescan (for parallel
scan).



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


parallel_seqscan_heapscan_v2.patch
Description: Binary data


parallel_seqscan_heaprescan_v1.patch
Description: Binary data

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


Re: [HACKERS] [COMMITTERS] pgsql: Lower *_freeze_max_age minimum values.

2015-10-05 Thread Andres Freund
On 2015-09-24 12:39:54 -0300, Alvaro Herrera wrote:
> Andres Freund wrote:
> > On 2015-09-24 10:37:33 -0400, Tom Lane wrote:
> > > Andres Freund  writes:
> 
> > > Should this patch not have also touched the per-table limits in
> > > reloptions.c?
> > 
> > Hm. I guess that'd make sense. It's not really related to the goal of
> > making it realistic to test multixact/clog truncation, but it's less
> > confusing if consistent.
> 
> Yeah, agreed.

Pushed. I actually noticed that the lower limit reloption
multixact_freeze_max_age in reloptions was wrong independent of recent
commits.

> > > and I found places in create_table.sgml that claim these variables can be
> > > set to zero.  You didn't break that with this patch, but it's still wrong.
> > 
> > Seems to have been "broken" back in 834a6da4f7 - the old table based
> > approach doesn't seem to have imposed lower limits. I'm not really sure
> > whether making the limits consistent and updating the docs or removing
> > them alltogether is the better approach.
> 
> I'm surprised the error has survived this long.  Without checking I
> can't say what's the best solution either, but I would opt for
> documenting the limits we have -- if we want to change them back to 0 I
> say that merits its own discussion.

How about simply removing that sentence? I.e. something like
  autovacuum_freeze_max_age larger than the system-wide setting
- (it can only be set smaller). Note that while you can set
- autovacuum_freeze_max_age very small, or even zero, this is
- usually unwise since it will force frequent vacuuming.
+ (it can only be set smaller).
  

Greetings,

Andres Freund


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


Re: [HACKERS] ON CONFLICT issues around whole row vars,

2015-10-05 Thread Stephen Frost
Peter, all,

* Peter Geoghegan (p...@heroku.com) wrote:
> I see now that commit 4f3b2a8883 changed things for UPDATE and DELETE
> statements, but not INSERT statements. I guess my unease is because
> that isn't entirely consistent with INSERT + RETURNING and the GRANT
> system. Logically, the only possible justification for our long
> standing INSERT and RETURNING behavior with GRANT (the fact that it
> requires SELECT privilege for rows returned, just like UPDATE and
> DELETE) is that before row insert triggers could do something secret
> (e.g. they could be security definer). It doesn't seem to be too much
> of a stretch to suppose the same should apply with RLS.

I had intended to address with policies what is addressed through
permissions with 7d8db3e, but the coverage for INSERT+RETURNING was only
done when ON CONFLICT was in use.

I've fixed that by applying the SELECT policies as WCOs for both the
INSERT and UPDATE RETURNING cases.  This matches the permissions system,
where we require SELECT rights on the table for an INSERT RETURNING
query.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] ON CONFLICT issues around whole row vars,

2015-10-05 Thread Andres Freund
On 2015-10-05 08:01:00 -0400, Stephen Frost wrote:
> Peter, all,
> I had intended to address with policies what is addressed through
> permissions with 7d8db3e, but the coverage for INSERT+RETURNING was only
> done when ON CONFLICT was in use.
> 
> I've fixed that by applying the SELECT policies as WCOs for both the
> INSERT and UPDATE RETURNING cases.  This matches the permissions system,
> where we require SELECT rights on the table for an INSERT RETURNING
> query.

This really needs tests verifying the behaviour...

Greetings,

Andres Freund


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


Re: [HACKERS] ON CONFLICT issues around whole row vars,

2015-10-05 Thread Stephen Frost
Andres,

On Monday, October 5, 2015, Andres Freund  wrote:

> On 2015-10-05 08:01:00 -0400, Stephen Frost wrote:
> > Peter, all,
> > I had intended to address with policies what is addressed through
> > permissions with 7d8db3e, but the coverage for INSERT+RETURNING was only
> > done when ON CONFLICT was in use.
> >
> > I've fixed that by applying the SELECT policies as WCOs for both the
> > INSERT and UPDATE RETURNING cases.  This matches the permissions system,
> > where we require SELECT rights on the table for an INSERT RETURNING
> > query.
>
> This really needs tests verifying the behaviour...
>

Good point, will add.

Thanks!

Stephen


Re: [HACKERS] Odd query execution behavior with extended protocol

2015-10-05 Thread Shay Rojansky
Thanks for the help Tom and the others, I'll modify my sequence and report
if I encounter any further issues.

On Sun, Oct 4, 2015 at 7:36 PM, Tom Lane  wrote:

> Shay Rojansky  writes:
> >> To my mind there is not a lot of value in performing Bind until you
> >> are ready to do Execute.  The only reason the operations are separated
> >> in the protocol is so that you can do multiple Executes with a row limit
> >> on each one, to retrieve a large query result in chunks.
>
> > So you would suggest changing my message chain to send Bind right after
> > Execute, right? This would yield the following messages:
>
> > P1/P2/D1/B1/E1/D2/B2/E2/S (rather than the current
> > P1/D1/B1/P2/D2/B2/E1/C1/E2/C2/S)
>
> > This would mean that I would switch to using named statements and the
> > unnamed portal, rather than the current unnamed statement
> > and named portals. If I recall correctly, I was under the impression that
> > there are some PostgreSQL performance benefits to using the
> > unnamed statement over named statements, although I admit I can't find
> any
> > documentation backing that. Can you confirm that the two
> > are equivalent performance-wise?
>
> Hmm.  I do not recall exactly what performance optimizations apply to
> those two cases; they're probably not "equivalent", though I do not think
> the difference is major in either case.  TBH I was a bit surprised on
> reading your message to hear that the system would take that sequence at
> all; it's not obvious that it should be allowed to replace a statement,
> named or not, while there's an open portal that depends on it.
>
> I think you might have more issues with lifespans, since portals go away
> at commit whereas named statements don't.
>
> regards, tom lane
>


Re: [HACKERS] Use EVP API pgcrypto encryption, dropping support for OpenSSL 0.9.6 and older

2015-10-05 Thread Alvaro Herrera
Heikki Linnakangas wrote:

> In short, pgcrypto actually used to use the EVP functions, but was changed
> to *not* use them, because in older versions of OpenSSL, some key lengths
> and/or padding options that pgcrypto supports were not supported by the EVP
> API. That was fixed in OpenSSL 0.9.7, however. The consensus in 2007 was
> that we could drop support for OpenSSL 0.9.6 and below, so that should
> definitely be OK by now, if we haven't already done that elsewhere in the
> code.

I think we already effectively dropped support for < 0.9.7 with the
renegotiation fixes; see
https://www.postgresql.org/message-id/20130712203252.GH29206%40eldon.alvh.no-ip.org

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


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


[HACKERS] Odd query execution behavior with extended protocol

2015-10-05 Thread Shay Rojansky
Hi hackers, some odd behavior has been reported with Npgsql and I wanted to
get your help.

Npgsql supports sending multiple SQL statements in a single packet via the
extended protocol. This works fine, but when the second query SELECTs a
value modified by the first's UPDATE, I'm getting a result as if the UPDATE
hasn't yet occurred.

The exact messages send by Npgsql are:

Parse (UPDATE data SET name='foo' WHERE id=1), statement=unnamed
Describe (statement=unnamed)
Bind (statement=unnamed, portal=MQ0)
Parse (SELECT * FROM data WHERE id=1), statement=unnamed
Describe (statement=unnamed)
Bind (statement=unnamed, portal=MQ1)
Execute (portal=MQ0)
Close (portal=MQ0)
Execute (portal=MQ1)
Close (portal=MQ1)
Sync

Instead of returning the expected 'foo' value set in the first command's
UPDATE, I'm getting whatever value was previously there.
Note that this happen regardless of whether a transaction is already set
and of the isolation level.

Is this the expected behavior, have I misunderstood the protocol specs?

Thanks for your help, and please let me know if you need any more info.

Shay


Re: [HACKERS] [COMMITTERS] pgsql: Lower *_freeze_max_age minimum values.

2015-10-05 Thread Tom Lane
Andres Freund  writes:
> On 2015-09-24 12:39:54 -0300, Alvaro Herrera wrote:
>> I'm surprised the error has survived this long.  Without checking I
>> can't say what's the best solution either, but I would opt for
>> documenting the limits we have -- if we want to change them back to 0 I
>> say that merits its own discussion.

> How about simply removing that sentence? I.e. something like
>   autovacuum_freeze_max_age larger than the system-wide 
> setting
> - (it can only be set smaller). Note that while you can set
> - autovacuum_freeze_max_age very small, or even zero, this is
> - usually unwise since it will force frequent vacuuming.
> + (it can only be set smaller).
>   

How about "Setting autovacuum_freeze_max_age to very small values
is unwise since it will force frequent vacuuming."

regards, tom lane


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


Re: [HACKERS] [COMMITTERS] pgsql: Lower *_freeze_max_age minimum values.

2015-10-05 Thread Tom Lane
Andres Freund  writes:
> On 2015-10-05 09:39:58 -0400, Tom Lane wrote:
>> How about "Setting autovacuum_freeze_max_age to very small values
>> is unwise since it will force frequent vacuuming."

> Well, you still can't really set it to a very small value - the lower
> limits are still 100k/10k for xids/mxids. To me that sentence mostly
> made sense with the old logic where no lower limits existed.

Good point.  Agreed, let's just flush it.

regards, tom lane


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


Re: [HACKERS] ON CONFLICT issues around whole row vars,

2015-10-05 Thread Stephen Frost
* Andres Freund (and...@anarazel.de) wrote:
> On 2015-10-05 08:01:00 -0400, Stephen Frost wrote:
> > Peter, all,
> > I had intended to address with policies what is addressed through
> > permissions with 7d8db3e, but the coverage for INSERT+RETURNING was only
> > done when ON CONFLICT was in use.
> > 
> > I've fixed that by applying the SELECT policies as WCOs for both the
> > INSERT and UPDATE RETURNING cases.  This matches the permissions system,
> > where we require SELECT rights on the table for an INSERT RETURNING
> > query.
> 
> This really needs tests verifying the behaviour...

Added.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] No Issue Tracker - Say it Ain't So!]

2015-10-05 Thread Merlin Moncure
On Mon, Oct 5, 2015 at 5:32 AM, Magnus Hagander  wrote:
>
>
> On Mon, Oct 5, 2015 at 12:42 AM, Nathan Wagner 
> wrote:
>>
>> I don't have the original message for this thread, so I arbitrarily picked
>> a
>> message to reply to.
>>
>> Since what has been asked for is a bug *tracker*, and we already have a
>> bugs
>> mailing list, I put together something.
>
> FWIW, I think this is a good approach in general. This makes it a  bug
> *tracker*, rather than a "workflow enforcer". That depends on what we want
> of course, but those are two very different things and many of the other
> tools suggested are more workflow enforcers.

+1

The key points is how people interact with the tool; as long as the
interaction is basically "one way" from email to the tracking tool
(either debbugs or something hand rolled) it should work as long as it
provides value.  The main output of the tool is to do thing like
making qualified searches of bug fixes by version easier and perhaps
supporting release note generation.

Personally I think a hand-rolled tool is a better choice for this
project given that the requirements are so specific; it can be thought
of as an extension of the commit fest framework.

merlin


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


Re: [HACKERS] Connection string parameter 'replication' in documentation

2015-10-05 Thread Takashi Ohnishi
Thanks for your answer:)

>This is introduced by the commit 5a991ef, which allows logical
>decoding via walsender interface. The paramter replication has
>been there far from the commit intentionary as an 'undocumented
>paramter'. This is, I suppose, because it is treated as a part of
>the replication ptorocol, not a matter of ordinary clients at
>all.

Ah, this is intentional!

>Since it has no meaning out of the context of replication agents,
>it seems reasonable that the parameter is not documented in the
>section.

I see.
But, if this parameter is for logical decoding,  I think it is better that
"46.3. Streaming Replication Protocol Interface" tells about this.


>Instead, the comment for libpqrcv_connect@libpqwalreceiver.c has
>become outedated by the commit.
>
>> * We use the expand_dbname parameter to process the connection string (or
>> * URI), and pass some extra options. The deliberately undocumented
>> * parameter "replication=true" makes it a replication connection. The
>
>This should be synced with the document.

Agreed.

--
Takashi Ohnishi

2015-10-05 19:07 GMT+09:00 Kyotaro HORIGUCHI <
horiguchi.kyot...@lab.ntt.co.jp>:

> Hello,
>
> At Fri, 2 Oct 2015 23:13:24 +0900, Takashi Ohnishi 
> wrote in 

Re: [HACKERS] No Issue Tracker - Say it Ain't So!

2015-10-05 Thread Nathan Wagner
I don't have the original message for this thread, so I arbitrarily picked a
message to reply to.

Since what has been asked for is a bug *tracker*, and we already have a bugs
mailing list, I put together something.

I downloaded the archives for pgsql-bugs, and fed them into a database.  This
part was easy, since I have already written a pg backed usenet server and had
the code hand for storing and parsing out bits of rfc 2822 messages.

It's dirt simple.  If the system sees a message with 'Bug #(\d+)' in the
subject line, it creates an entry in a bugs table with that bug number (if
needed), and then marks the message as belonging to that bug.  If there seems
to be metadata about the bug in the format of the (unquoted)

Bug reference:
Logged by:  
Email address:
PostgreSQL version:
Operating system:
Description:
Details:

it pulls that out and puts it in the bugs table.  There's also an "open"
boolean in the table, defaulting to true.

The results can be found at https://granicus.if.org/pgbugs/

Ok.  So now we have a bug tracker, but...

Some open questions that I don't think have really been addressed, with my
commentary interspersed:

1: Can a bug be more than "open" or "closed"?

I think yes.  At least we probably want to know why a bug is closed.  Is it not
a bug at all, not our bug, a duplicate submission, a duplicate of another bug,
something we won't fix for some reason (e.g. a bug against version 7)

2: Who can declare a bug closed.

Ugh.  I'm going to close some of them if it seems obvious to me that they
should be closed.  But what if it's not obvious?  I could probably maintain it
to some extent, but I don't know how much time that would actually take.

Related to the next point, it probably makes sense to just close up front
bugs that are marked against unsupported pg versions, or haven't had
any activity for too long, perhaps two years.  Just closing bugs with no
mailing list activity for two years closes 5280 of 6376 bugs.

3: How far back should I actually import data from the bugs list?

I have imported each archived month from December of 1998.  It looks like the
bug sequence was started at 1000 in December of 2003.  Emails with no bug id in
the subject line don't get associated with any bug, they're in the DB bug not
really findable.

4: What should I do with emails that don't reference a bug id but seem to be
talking about a bug?

I suggest we do nothing with them as far as the bug tracker is concerned.  If
people want to mark their message as pertaining to a bug, they can put that in
the subject line.  However, I don't think a bug id can be assigned via email,
that is, I think you have to use a web form to create a bug report with a bug
id.  Presumably that could change if whoever runs the bug counter wants it to.

5: How can we use email to update the status of a bug?

I suggest using email headers to do this.  'X-PGBug-Fixed: ' and the
like.  I assume here that everyone who might want to do such a thing uses an
MUA that would allow this, and they know how.

6: Does there need to be any security on updating the status?

Probably not.  I don't think it's the sort of thing that would attract
malicious adjustments.  If I'm wrong, I'd need to rethink this.  I realize I'm
making security an afterthought, which makes my teeth itch, but I think layers
of security would make it much less likely to be actually adopted.

Just to be clear, this is both a work in progress and a proof of concept.  It's
slow, it's ugly.  I haven't created any indexes, written any css or javascript,
or implemented any caching.  I may work on that before you read this though.

Comments are welcome, and no, I don't really expect that this will be what gets
adopted, mainly I wanted to show that we can probably just build something
rather effective off our existing infrastructure, and I had Saturday free (as
of this writing, I've got maybe 5 hours into it total, albeit with lots of
code re-use from other projects).  There are some obvious todo items,
filtering and searching being the most salient.

Some data import issues:

March 10, 2003, bad Date header, looked like junk anyway, so I deleted it.

Time zone offsets of --0400 and -0500 (at least), I took these as being -0400
(or whathaveyou).

Date header of Sat, 31 May 2008 12:12:18 +1930, judging by the name on the
email, this is probably posted from Venezuela, which switched to time one -0430
in 2007, which could also be thought of as +1930 if you ignore the implied date
change.

And, by way of some statistics, since I've got the archives in a database:

Emails: 43870
Bugs: 6376
Distinct 'From' headers: 10643

The bugs have 3.5 messages each on average, with 2 being the most common
number, and 113 at the most, for bug 12990.  1284 bugs have only one message
associated with them.

-- 
nw


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


Re: [HACKERS] Use EVP API pgcrypto encryption, dropping support for OpenSSL 0.9.6 and older

2015-10-05 Thread Joe Conway
On 10/05/2015 06:02 AM, Heikki Linnakangas wrote:
> There was prior discussion on the EVP API in this old thread from 2007:
> http://www.postgresql.org/message-id/flat/46a5e284.7030...@sun.com#46a5e284.7030...@sun.com
> 
> 
> In short, pgcrypto actually used to use the EVP functions, but was
> changed to *not* use them, because in older versions of OpenSSL, some
> key lengths and/or padding options that pgcrypto supports were not
> supported by the EVP API. That was fixed in OpenSSL 0.9.7, however. The
> consensus in 2007 was that we could drop support for OpenSSL 0.9.6 and
> below, so that should definitely be OK by now, if we haven't already
> done that elsewhere in the code.
> 
> Any objections to the attached two patches?

I haven't studied that patches themselves yet, but +1 for the concept.

Joe

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



signature.asc
Description: OpenPGP digital signature


Re: [HACKERS] ON CONFLICT issues around whole row vars,

2015-10-05 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Stephen Frost  writes:
> > I had intended to address with policies what is addressed through
> > permissions with 7d8db3e, but the coverage for INSERT+RETURNING was only
> > done when ON CONFLICT was in use.
> 
> > I've fixed that by applying the SELECT policies as WCOs for both the
> > INSERT and UPDATE RETURNING cases.  This matches the permissions system,
> > where we require SELECT rights on the table for an INSERT RETURNING
> > query.
> 
> What of DELETE RETURNING?

That was handled in 7d8db3e.

Per previous discussion, UPDATE and DELETE RETURNING apply SELECT 
policies as security quals, meaning only the records visible through the
SELECT policy are eligible for consideration.  INSERT+RETURNING has only
WithCheckOptions, no security quals, which is what makes it different
from the other cases.  The INSERT+ON CONFLICT+RETURNING case had been
covered already and I had mistakenly thought it was also covering
INSERT+RETURNING.  In fixing that, I realized that Peter makes a good
point that UPDATE+RETURNING should also have SELECT policies applied as
WithCheckOptions.

I'm about to push updated regression tests, as suggested by Andres.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Less than ideal error reporting in pg_stat_statements

2015-10-05 Thread Tom Lane
Peter Geoghegan  writes:
> I'm annoyed and disappointed that the patch committed does not even
> begin to address the underlying problem -- it just adds an escape
> hatch, and fixes another theoretical issue that no one was affected
> by. Honestly, next time I won't bother.

The problem as I see it is that what you submitted is a kluge that will
have weird and unpredictable side effects.  Moreover, it seems to be
targeting an extremely narrow problem case, ie large numbers of queries
that (a) have long query texts and (b) are distinct to the fingerprinting
code and (c) fail.  It seems to me that you could get into equal trouble
with situations where (c) is not satisfied, and what then?

I'm certainly amenable to doing further work on this problem.  But I do
not think that what we had was well-enough-thought-out to risk pushing
it just hours before a release deadline.  Let's arrive at a more
carefully considered fix in a leisurely fashion.

regards, tom lane


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


Re: [HACKERS] ON CONFLICT issues around whole row vars,

2015-10-05 Thread Tom Lane
Stephen Frost  writes:
> I had intended to address with policies what is addressed through
> permissions with 7d8db3e, but the coverage for INSERT+RETURNING was only
> done when ON CONFLICT was in use.

> I've fixed that by applying the SELECT policies as WCOs for both the
> INSERT and UPDATE RETURNING cases.  This matches the permissions system,
> where we require SELECT rights on the table for an INSERT RETURNING
> query.

What of DELETE RETURNING?

regards, tom lane


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


Re: [HACKERS] [COMMITTERS] pgsql: Lower *_freeze_max_age minimum values.

2015-10-05 Thread Andres Freund
On 2015-10-05 09:39:58 -0400, Tom Lane wrote:
> Andres Freund  writes:
> > How about simply removing that sentence? I.e. something like
> >   autovacuum_freeze_max_age larger than the system-wide 
> > setting
> > - (it can only be set smaller). Note that while you can set
> > - autovacuum_freeze_max_age very small, or even zero, this 
> > is
> > - usually unwise since it will force frequent vacuuming.
> > + (it can only be set smaller).
> >   
> 
> How about "Setting autovacuum_freeze_max_age to very small values
> is unwise since it will force frequent vacuuming."

Well, you still can't really set it to a very small value - the lower
limits are still 100k/10k for xids/mxids. To me that sentence mostly
made sense with the old logic where no lower limits existed.

Andres


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


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

2015-10-05 Thread Simon Riggs
On 10 September 2015 at 01:58, Andres Freund  wrote:

> On 2015-09-04 23:35:42 +0100, Simon Riggs wrote:
> > This looks OK. You saw that I was proposing to solve this problem a
> > different way ("Summary of plans to avoid the annoyance of Freezing"),
> > suggesting that we wait for a few CFs to see if a patch emerges for that
> -
> > then fall back to this patch if it doesn't? So I am moving this patch to
> > next CF.
>
> As noted on that other thread I don't think that's a good policy, and it
> seems like Robert agrees with me. So I think we should move this back to
> "Needs Review".
>

I also agree. Andres and I spoke at PostgresOpen and persuaded me, I've
just been away.

Am happy to review and commit in next few days/weeks, once I catch up on
the thread.

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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

2015-10-05 Thread Fujii Masao
On Fri, Oct 2, 2015 at 8:14 PM, Masahiko Sawada  wrote:
>> +#define Anum_pg_class_relallfrozen12
>> Why is pg_class.relallfrozen necessary? ISTM that there is no user of it now.
>
> The relallfrozen would be useful for user to estimate time to vacuum
> freeze or anti-wrapping vacuum before being done them actually.
> (Also this value is used on regression test.)
> But this information is not used on planning like relallvisible, so it
> would be good to move this information to another system view like
> pg_stat_*_tables.

Or make pgstattuple and pgstattuple_approx report even the number
of frozen tuples?

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] Use EVP API pgcrypto encryption, dropping support for OpenSSL 0.9.6 and older

2015-10-05 Thread Alvaro Herrera
Andres Freund wrote:

> But more seriously: Given the upstream support policies from
> https://www.openssl.org/policies/releasestrat.html :
> "
> Support for version 0.9.8 will cease on 2015-12-31. No further releases of 
> 0.9.8 will be made after that date. Security fixes only will be applied to 
> 0.9.8 until then.
> Support for version 1.0.0 will cease on 2015-12-31. No further releases of 
> 1.0.0 will be made after that date. Security fixes only will be applied to 
> 1.0.0 until then.
> 
> We may designate a release as a Long Term Support (LTS) release. LTS
> releases will be supported for at least five years and we will specify
> one at least every four years. Non-LTS releases will be supported for at
> least two years.
> "
> and the amount of security fixes regularly required for openssl, I don't
> think we'd do anybody a favor by trying to continue supporting older
> versions for a long while.
> 
> Note that openssl's security releases are denoted by a letter after the
> numeric version, not by the last digit. 0.9.7 was released 30 Dec 2002.

Yeah.  Last of the 0.9.7 line (0.9.7m) was in 2007:

commit 10626fac1569ea37839c37b105681cd08dbe6658
Author: cvs2svn 
AuthorDate: Fri Feb 23 12:49:10 2007 +
CommitDate: Fri Feb 23 12:49:10 2007 +

This commit was manufactured by cvs2svn to create tag 'OpenSSL_0_9_7m'.


Current 0.9.8 is 0.9.8zg, in June this year:

commit 0823ddc56e9aaa1de6c4f57bb45457d5eeca404d
Author: Matt Caswell 
AuthorDate: Thu Jun 11 15:20:22 2015 +0100
CommitDate: Thu Jun 11 15:20:22 2015 +0100

Prepare for 0.9.8zg release

Reviewed-by: Stephen Henson 

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


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


Re: [HACKERS] No Issue Tracker - Say it Ain't So!]

2015-10-05 Thread Alvaro Herrera
Nathan Wagner wrote:

> 1: Can a bug be more than "open" or "closed"?
> 
> I think yes.  At least we probably want to know why a bug is closed.  Is it 
> not
> a bug at all, not our bug, a duplicate submission, a duplicate of another bug,
> something we won't fix for some reason (e.g. a bug against version 7)

Not only that -- is the bug closed in all branches or only some of them?
If there's also some data about when a bug appeared (commit ID), then
it's easy to get a report of which minor releases have the bug.  For
instance, see bug #8470 which I fixed by commits in 9.5 and master, but
is yet unfixed in 9.3 and 9.4.  At least one other multixact bug was
fixed in 9.5/master only.


What debbugs allows you to do, is that you write to the bug address and
in the first few lines of the body it looks for commands such as
"close", "merge", "reassign".  I for one am open fo commandeering a bug
tracker in this way, as well as adding fixed-format tags to commit
messages indicating "Fixes: #xyz" so that it can be picked up
automatically.

One of our policies in backpatching fixes is that we use the same commit
in all branches so that they become grouped as a single element in the
output of the src/tools/git_changelog script.  Maybe that can be useful
to the bug tracker as well, in some form.

> 2: Who can declare a bug closed.
> 
> Ugh.  I'm going to close some of them if it seems obvious to me that they
> should be closed.  But what if it's not obvious?  I could probably maintain it
> to some extent, but I don't know how much time that would actually take.

I think at least committers should be able to close bugs.

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


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


Re: [HACKERS] Less than ideal error reporting in pg_stat_statements

2015-10-05 Thread Alvaro Herrera
Andrew Dunstan wrote:
> 
> On 10/05/2015 11:15 AM, Tom Lane wrote:
> >Peter Geoghegan  writes:
> >>I'm annoyed and disappointed that the patch committed does not even
> >>begin to address the underlying problem -- it just adds an escape
> >>hatch, and fixes another theoretical issue that no one was affected
> >>by. Honestly, next time I won't bother.
> >The problem as I see it is that what you submitted is a kluge that will
> >have weird and unpredictable side effects.  Moreover, it seems to be
> >targeting an extremely narrow problem case, ie large numbers of queries
> >that (a) have long query texts and (b) are distinct to the fingerprinting
> >code and (c) fail.  It seems to me that you could get into equal trouble
> >with situations where (c) is not satisfied, and what then?

> FWIW, (a) and (b) but not (c) is probably the right description for my
> client who has been seeing problems here.

I think the fact that long IN lists are fingerprinted differently
according to the number of elements in the list makes the scenario
rather very likely -- not particularly narrow.

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


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


Re: [HACKERS] [COMMITTERS] pgsql: Apply SELECT policies in INSERT/UPDATE+RETURNING

2015-10-05 Thread Stephen Frost
Peter,

On Monday, October 5, 2015, Peter Geoghegan  wrote:

> On Mon, Oct 5, 2015 at 4:55 AM, Stephen Frost  > wrote:
> > Apply SELECT policies in INSERT/UPDATE+RETURNING
>
> If we've changed this now, where does that leave the excluded.*
> pseudo-relation?
>

Away from my keyboard at the moment, and I'm sure Andres can comment, but I
didn't see this commit as changing anything regarding how excluded should
be handled (which Andres and I had at least seemed to come to an agreement
on).

Thanks!

Stephen


Re: [HACKERS] Less than ideal error reporting in pg_stat_statements

2015-10-05 Thread Andrew Dunstan



On 10/05/2015 11:15 AM, Tom Lane wrote:

Peter Geoghegan  writes:

I'm annoyed and disappointed that the patch committed does not even
begin to address the underlying problem -- it just adds an escape
hatch, and fixes another theoretical issue that no one was affected
by. Honestly, next time I won't bother.

The problem as I see it is that what you submitted is a kluge that will
have weird and unpredictable side effects.  Moreover, it seems to be
targeting an extremely narrow problem case, ie large numbers of queries
that (a) have long query texts and (b) are distinct to the fingerprinting
code and (c) fail.  It seems to me that you could get into equal trouble
with situations where (c) is not satisfied, and what then?

I'm certainly amenable to doing further work on this problem.  But I do
not think that what we had was well-enough-thought-out to risk pushing
it just hours before a release deadline.  Let's arrive at a more
carefully considered fix in a leisurely fashion.





FWIW, (a) and (b) but not (c) is probably the right description for my 
client who has been seeing problems here.


cheers

andrew


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


Re: [HACKERS] Less than ideal error reporting in pg_stat_statements

2015-10-05 Thread Tom Lane
Alvaro Herrera  writes:
> Andrew Dunstan wrote:
>> FWIW, (a) and (b) but not (c) is probably the right description for my
>> client who has been seeing problems here.

> I think the fact that long IN lists are fingerprinted differently
> according to the number of elements in the list makes the scenario
> rather very likely -- not particularly narrow.

That's certainly something worth looking at, but I think it's probably
more complicated than that.  If you just write "WHERE x IN (1,2,3,4)",
that gets folded to a ScalarArrayOp with a single array constant, which
the existing code would deal with just fine.  We need to identify
situations where that's not the case but yet we shouldn't distinguish.

In any case, that's just a marginal tweak for one class of query.
I suspect the right fix for the core problem is the one Peter mentioned
in passing earlier, namely make it possible to do garbage collection
without having to slurp the entire file into memory at once.  It'd be
slower, without a doubt, but we could continue to use the existing code
path unless the file gets really large.

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] Use EVP API pgcrypto encryption, dropping support for OpenSSL 0.9.6 and older

2015-10-05 Thread Andres Freund
On 2015-10-05 12:16:05 -0300, Alvaro Herrera wrote:
> Heikki Linnakangas wrote:
> 
> > In short, pgcrypto actually used to use the EVP functions, but was changed
> > to *not* use them, because in older versions of OpenSSL, some key lengths
> > and/or padding options that pgcrypto supports were not supported by the EVP
> > API. That was fixed in OpenSSL 0.9.7, however. The consensus in 2007 was
> > that we could drop support for OpenSSL 0.9.6 and below, so that should
> > definitely be OK by now, if we haven't already done that elsewhere in the
> > code.
> 
> I think we already effectively dropped support for < 0.9.7 with the
> renegotiation fixes; see
> https://www.postgresql.org/message-id/20130712203252.GH29206%40eldon.alvh.no-ip.org

9.5+ do again then :P

But more seriously: Given the upstream support policies from
https://www.openssl.org/policies/releasestrat.html :
"
Support for version 0.9.8 will cease on 2015-12-31. No further releases of 
0.9.8 will be made after that date. Security fixes only will be applied to 
0.9.8 until then.
Support for version 1.0.0 will cease on 2015-12-31. No further releases of 
1.0.0 will be made after that date. Security fixes only will be applied to 
1.0.0 until then.

We may designate a release as a Long Term Support (LTS) release. LTS
releases will be supported for at least five years and we will specify
one at least every four years. Non-LTS releases will be supported for at
least two years.
"

and the amount of security fixes regularly required for openssl, I don't
think we'd do anybody a favor by trying to continue supporting older
versions for a long while.

Note that openssl's security releases are denoted by a letter after the
numeric version, not by the last digit. 0.9.7 was released 30 Dec 2002.

Greetings,

Andres Freund


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


[HACKERS] run pg_rewind on an uncleanly shut down cluster.

2015-10-05 Thread Oleksii Kliukin
Hello,

I'm trying to find out how to rewind a cluster that was not shut down
cleanly, in order to implement pg_rewind support in patroni (an
automated failover system, https://github.com/zalando/patroni).

At the moment, pg_rewind limits itself to only cleanly shut down
clusters. This works nicely in the case of a split brain caused by the
network partitioning. However, it doesn't cover the important case of a
suddenly crashed master: the crashed cluster cannot be rewound to the
new master. 

One idea to overcome this limitation is to start the former master for a
short time, just to let automatic recovery do its job, and stop it
cleanly afterwards. There are some indications on the list that it
works:
http://www.postgresql.org/message-id/79f6ceb4-f519-40fa-9c72-167def1eb...@simply.name

However, in our testing we had an issue with a missing WAL segment on a
former master, which prevented pg_rewind from bringing it up to date
with the current master:

Suppose, the current XLOG segment right before we crash the master is:

postgres=# select * from pg_xlogfile_name(pg_current_xlog_location());
 pg_xlogfile_name
--
 00010003
(1 row)

(the master is configured to archive all segments into the external
directory).

The latest checkpoint location right before the crash is:

Latest checkpoint's REDO location:0/228
Latest checkpoint's REDO WAL file:00010002

and pg_xlog contains the following data
$ ls -R  postgresql0/pg_xlog/
00010001
00010002.0028.backup archive_status
00010002 00010003

postgresql0/pg_xlog//archive_status:
00010001.done
00010002.done
00010002.0028.backup.done

Now, if we crash the master by sending it SIGKILL, and then start it
again with:

$ postgres  -D data/postgresql0 -c "max_replication_slots=5" -c
"wal_level=hot_standby" -c "wal_log_hints=on"
LOG:  database system was interrupted; last known up at 2015-10-05
17:28:04 CEST
LOG:  database system was not properly shut down; automatic recovery in
progress
LOG:  redo starts at 0/228
LOG:  invalid record length at 0/360
LOG:  redo done at 0/328

we'll get the following contents of postgresql0/pg_xlog:

$ ls -R  postgresql0/pg_xlog/
00010002.0028.backup 00010004   
 archive_status
00010003 00010005

postgresql0/pg_xlog//archive_status:
00010002.0028.backup.done

Note, that at some moment the master removed the segment
00010002 from its pg_xlog.

In the pg_controldata, I get:

Latest checkpoint's REDO location:0/3000108
Latest checkpoint's REDO WAL file:00010003

When I try to run pg_rewind, I'm getting:

pg_rewind -D postgresql0 --source-server="host=127.0.0.1 port=5433
dbname=postgres"
The servers diverged at WAL position 0/360 on timeline 1.
could not open file "data/postgresql0/pg_xlog/00010002":
No such file or directory

Note that this problem happens not 100% of time during the tests,
sometimes pg_rewind can actually rewind the former master.

I know I can copy the segment back from the archive, but I'd like to
avoid putting this logic into the failover tool if possible. Is there
anything we can do to avoid the problem described above, or is there a
better way to bring up the former master after the crash with pg_rewind?

Kind regards,
--
Oleksii Kliukin


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


Re: [HACKERS] run pg_rewind on an uncleanly shut down cluster.

2015-10-05 Thread Bruce Momjian
On Mon, Oct  5, 2015 at 05:41:07PM +0200, Oleksii Kliukin wrote:
> Hello,
> 
> I'm trying to find out how to rewind a cluster that was not shut down
> cleanly, in order to implement pg_rewind support in patroni (an
> automated failover system, https://github.com/zalando/patroni).
> 
> At the moment, pg_rewind limits itself to only cleanly shut down
> clusters. This works nicely in the case of a split brain caused by the
> network partitioning. However, it doesn't cover the important case of a
> suddenly crashed master: the crashed cluster cannot be rewound to the
> new master. 

Did you read this thread convering the same topic from a few weeks ago?


http://www.postgresql.org/message-id/flat/55fa2537.4070...@gmx.net#55fa2537.4070...@gmx.net

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

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription +


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


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

2015-10-05 Thread Masahiko Sawada
On Mon, Oct 5, 2015 at 11:03 PM, Fujii Masao  wrote:
> On Fri, Oct 2, 2015 at 8:14 PM, Masahiko Sawada  wrote:
>>> +#define Anum_pg_class_relallfrozen12
>>> Why is pg_class.relallfrozen necessary? ISTM that there is no user of it 
>>> now.
>>
>> The relallfrozen would be useful for user to estimate time to vacuum
>> freeze or anti-wrapping vacuum before being done them actually.
>> (Also this value is used on regression test.)
>> But this information is not used on planning like relallvisible, so it
>> would be good to move this information to another system view like
>> pg_stat_*_tables.
>
> Or make pgstattuple and pgstattuple_approx report even the number
> of frozen tuples?
>

But we cannot know the number of frozen pages without installation of
pageinspect module.
I'm a bit concerned about that the all projects cannot install
extentension module into postgresql on production environment.
I think we need to provide such feature at least into core.
Thought?

Regards,

--
Masahiko Sawada


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


Re: [HACKERS] Odd query execution behavior with extended protocol

2015-10-05 Thread Shay Rojansky
>
> > So you would suggest changing my message chain to send Bind right after
> > Execute, right? This would yield the following messages:
>
> > P1/P2/D1/B1/E1/D2/B2/E2/S (rather than the current
> > P1/D1/B1/P2/D2/B2/E1/C1/E2/C2/S)
>
> > This would mean that I would switch to using named statements and the
> > unnamed portal, rather than the current unnamed statement
> > and named portals. If I recall correctly, I was under the impression that
> > there are some PostgreSQL performance benefits to using the
> > unnamed statement over named statements, although I admit I can't find
> any
> > documentation backing that. Can you confirm that the two
> > are equivalent performance-wise?
>
> Hmm.  I do not recall exactly what performance optimizations apply to
> those two cases; they're probably not "equivalent", though I do not think
> the difference is major in either case.  TBH I was a bit surprised on
> reading your message to hear that the system would take that sequence at
> all; it's not obvious that it should be allowed to replace a statement,
> named or not, while there's an open portal that depends on it.
>

One more important piece of information...

The reason Npgsql currently sends P1/D1/B1/P2/D2/B2/E1/C1/E2/C2/S is to
avoid deadlocks, I've already discussed this with you in
http://www.postgresql.org/message-id/cadt4rqb+fbtqpte5ylz0hkb-2k-ngzhm2ybvj0tmc8rqbgf...@mail.gmail.com
.

Unfortunately, the alternative I proposed above, P1/P2/D1/B1/E1/D2/B2/E2/S,
suffers from the same issue: any sequence in which a Bind is sent after a
previous Execute is deadlock-prone - Execute causes PostgreSQL to start
writing a potentially large dataset, while Bind means the client may be
writing a potentially large parameter value.

In other words, unless I'm mistaken it seems there's no alternative but to
implement non-blocking I/O at the client side - write until writing would
block, switching to reading when that happens. This adds some substantial
complexity, especially with .NET's SSL/TLS implementation layer.

Or does anyone see some sort of alternative which I've missed?