Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

[Amit Kapila]

On Sat, May 6, 2017 at 10:41 PM, Robert Haas  wrote:
> On Sat, May 6, 2017 at 12:55 PM, Robert Haas  wrote:
>> Oh!  Good catch.  Given that the behavior in question is intentional
>> there and intended to provide backward compatibility, changing it in
>> back-branches doesn't seem like a good plan.  I'll adjust the patch so
>> that it continues to ignore errors in that case.
>
> Updated patch attached.
>

Patch looks good to me.


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


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

Re: [HACKERS] PG 10 release notes

[Bruce Momjian]

On Thu, May  4, 2017 at 07:01:17PM -0300, Alvaro Herrera wrote:
> Thanks for doing this, looks great.  A few notes:
> 
>   
>
>   
>Add the ability to compute a correlation ratio and the number of
>distinct values on several columns (Tomas Vondra, David Rowley)
>   
> 
> I think this should be worded in terms of "extended data statistics" or
> such.  I think your proposed wording is a bit obscure.  How about
> something like "Add extended statistics to improve query planning".
> Also, I'd add myself as co-author, with Tomas' permission.

Already done.

>   
>
>
> Cause BRIN index summarization to happen more
> aggressively (lvaro Herrera)
>
> 
>
> Specifically, summarize the previous page range when a new page
> range is created.
>
>   
> 
> I think it should be pointed out that this is optional and defaults to
> off.  Maybe start with "add option to ..."  (I wonder if it's worth
> creating a linkable area in the docs that explain this feature, so that
> we could have a link in the relnotes entry).

Wow, looking at the patch, there is a lot more going on.  I have updated
the docs but please let me know if there is more needed.

Also, can ALTER INDEX change the index summariziation?  Should it?

Also, when you remove summarization, when is it re-added?  Why would you
remove summarization rather than just re-add it.  I must be missing
something.

>   
>
>
> Add function brin_desummarize_range() to remove
> BRIN summarization of a specified range (lvaro
> Herrera)
>
> 
>
> This allows future BRIN index summarization to be
> more compact.  CLARIFY
>
>   
> 
> The point of this change is that if data has changed (delete, update) in
> a way that could use tighter min/max limits, it would be worthwhile to
> remove the previous BRIN tuple so that a new one is created so that
> future scans can skip scanning the range.  Maybe word it something like
> "This is useful if UPDATE and DELETE have changed the data to tighter
> limits; a future BRIN index entry will be more accurate"?

OK, updated, but please let me know if more changes are needed.

>  
>   
>   
>Add support for converting XML-formatted data into a row
>set (Pavel Stehule, lvaro Herrera)
>   
> XMLTABLE is a sql-standard-specified construct, so we should mention it
> by name in the leading paragraph more prominently ("add support for the
> XMLTABLE standard feature" or something like that); also, I think it

Done.

> should be in the Queries section, not Functions.

Done.


>
> Add GUC 
> to limit the number of worker processes that can be used for
> parallelism (Julien Rouhaud)
>
> 
>
> This can be set lower than  linkend="guc-max-worker-processes"> to reserve worker processes
> for non-parallel purposes.
>
> 
> Strange last phrase.  I suggest "...to reserve worker processes for
> purposes other than parallel queries".  Also in the leading para, "for
> parallelism" should probably be "for query parallelism".

Done.

> I think commit e306df7f9 ("Full Text Search support for JSON
> and JSONB") does definitely not belong to section Indexes; I
> think Data Types is a better fit.

Moved.

> I think commits 67dc4ccbb and 1753b1b02 (pg_sequence and pg_sequences)
> should be listed in the opposite order.  Maybe merge them into one?

Merged.

>
> This proves better security than the existing md5
> negotiation and storage method.
>
> You probably mean "provides" not "proves".

Fixed.

>
> Allow SSL configuration to be updated at
> SIGHUP (Andreas Karlsson, Tom Lane)
>
> SIGHUP seems much too technical.  How about "... to be updated during
> configurating reload"

Yes, updated.

> I think the entry for commits a734fd5d1 and dafa0848d does not belong in
> the reliability section.  In fact I wonder if it's necessary to list
> this one at all.

Yes, I removed it.  We can always re-add it later.

> 
>  Increase the maximum configurable WAL size to 1
>  gigabyte (Beena Emerson)
> 
> "WAL segment size" perhaps, and note that this is useful to reduce
> frequency of archive_command?

Yes, updated.

>
> Also add -nosync option to disable fsync.
>
> Misspelled --no-sync.

Fixed.

>
> Add log options for pg_ctl wait (--wait)
> and no-wait (--no-wait) (Vik Fearing)
>
> Mispelled "long options".

Fixed.

Applied patch attached.

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

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +
diff --git a/doc/src/sgml/release-10.sgml 

Re: [HACKERS] Row Level Security UPDATE Confusion

[Stephen Frost]

Rod,

* Rod Taylor (rod.tay...@gmail.com) wrote:
> UPDATE seems to work as described (unable to create records you cannot
> select); both the single rule version and multi-rule version seem to work
> the same.

Thanks for reporting this and helping to test the fix.  I've now pushed
the fix and it'll be included in the next round of patch releases.

I'd definitely like to chat further at some point regarding what really
makes sense here and if we should be considering a change, and, further,
how we can improve the documentation to be more clear.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] MSVC odd TAP test problem

[Andrew Dunstan]

On 05/06/2017 07:41 PM, Craig Ringer wrote:
>
>
> On 7 May 2017 4:24 am, "Andrew Dunstan"
>  > wrote:
>
>
> I have been working on enabling the remaining TAP tests on MSVC
> build in
> the buildfarm client, but I have come across an odd problem. The bin
> tests all run fine, but the recover tests crash and in such a way
> as to
> crash the buildfarm client itself and require some manual cleanup.
> This
> happens at some stage after the tests have run (the final "ok" is
> output) but before the END handler in PostgresNode.pm (I put some
> traces
> in there to see if I could narrow down where there were problems).
>
> The symptom is that this appears at the end of the output when the
> client calls "vcregress.pl  taptest
> src/test/recover":
>
> Terminating on signal SIGBREAK(21)
> Terminating on signal SIGBREAK(21)
> Terminate batch job (Y/N)?
>
> And at that point there is nothing at all apparently running,
> according
> to Sysinternals Process Explorer, including the buildfarm client.
>
> It's 100% repeatable on bowerbird, and I'm a bit puzzled about how to
> fix it.
>
>
> Anyone have any clues?
>
>
> That looks like we've upset CMD.exe its self. I'm not sure how ...
> leaking a signal to the parent proc?
>
> I suspect this could be something to do with console process groups.
>
> Bowerbird is win8 . So this isn't going to be related to the support
> for ANSI escapes added in win10.
>
> A serach for the error turns up a complaint about IPC::Run as the
> first hit. Probably not coincidence.
>
>
> http://stackoverflow.com/q/40924750
>
> See this bug
>
> https://rt.cpan.org/Public/Bug/Display.html?id=101093
>
>
>



Actually, it's Win10, looks like I forgot to update the personality, my bad.

I had a feeling it was probably something to do with timeout. That RT
ticket looks like it's on the money.

cheers

andrew

-- 
Andrew Dunstanhttps://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] tuplesort_gettuple_common() and *should_free argument

[Andres Freund]

On 2017-04-06 14:57:56 -0700, Peter Geoghegan wrote:
> On Thu, Apr 6, 2017 at 2:50 PM, Andres Freund  wrote:
> > Pushed with very minor wording changes.
> 
> This had a typo:
> 
> + * If copy is true, the slot receives a copied tuple that'll that will stay

Belatedly fixed.


-- 
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] Change GetLastImportantRecPtr's definition? (wasSkip checkpoints, archiving on idle systems.)

[Andres Freund]

Hi,

On 2017-05-04 18:24:47 -0700, Andres Freund wrote:
> Hi,
> 
> On 2016-12-22 19:33:30 +, Andres Freund wrote:
> > Skip checkpoints, archiving on idle systems.
> 
> As part of an independent bugfix I noticed that Michael & I appear to
> have introduced an off-by-one here. A few locations do comparisons like:
> /*
>  * Only log if enough time has passed and interesting records have
>  * been inserted since the last snapshot.
>  */
> if (now >= timeout &&
> last_snapshot_lsn < GetLastImportantRecPtr())
> {
> last_snapshot_lsn = LogStandbySnapshot();
> ...
> 
> which looks reasonable on its face.  But LogStandbySnapshot (via XLogInsert())
>  * Returns XLOG pointer to end of record (beginning of next record).
>  * This can be used as LSN for data pages affected by the logged action.
>  * (LSN is the XLOG point up to which the XLOG must be flushed to disk
>  * before the data page can be written out.  This implements the basic
>  * WAL rule "write the log before the data".)
> 
> and GetLastImportantRecPtr
>  * GetLastImportantRecPtr -- Returns the LSN of the last important record
>  * inserted. All records not explicitly marked as unimportant are considered
>  * important.
> 
> which means that we'll e.g. not notice if there's exactly a *single* WAL
> record since the last logged snapshot (and likely similar in the other
> users of GetLastImportantRecPtr()), because XLogInsert() will return
> where the next record will most of the time be inserted, and
> GetLastImportantRecPtr() returns the beginning of said record.
> 
> This is trivially fixable by replacing < with <=.  But I wonder if the
> better fix would be to redefine GetLastImportantRecPtr() to point to the
> end of the record, too?  I don't quite see any upcoming user that'd need
> the beginning, and this is a bit failure prone for likely users.

Turns out this isn't the better fix, because the checkpoint code
compares with the actual record LSN (rather than the end+1 that
XLogInsert() returns).  We'd start having to do more bookkeeping or more
complicated computations (subtracting the checkpoint record's size).
Therefore I've pushed the simple fix mentioned above instead.

- Andres


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

[HACKERS] Google Summer Of Code 2017 & PostgreSQL

[Stephen Frost]

Greetings!

I am very pleased to be able to announce that the PostgreSQL project has
been approved for 4 GSoC projects this year!

The four projects are:

- Add errors handling and parallel execution to COPY

  Student: Alexey Kondratov
  Mentors: Anastasia Lubennikova, Alexander Korotkov, Alvaro Herrera

- Eliminate O(N^2) scaling from rw-conflict tracking in serializable
  transactions

  Student: Mengxing Liu
  Mentors: Alvaro Herrera, Kevin Grittner

- Explicitly support predicate locks in index AMs besides btree

  Student: Shubham Barai
  Mentors: Andrey Borodin, Kevin Grittner

- Foreign Keys for Array Elements

  Student: Mark Rofail
  Mentors: Alexander Korotkov, Stephen Frost, Alvaro Herrera

Please be welcoming to our students as they learn about our community,
project, code, and how to communicate on the mailing lists and begin
their hacking on PostgreSQL!

Feel free to reach out to me or David Steele, as the co-administrator,
with any questions regarding the GSoC program.

Thanks!

Stephen
PostgreSQL GSoC 2017 Administrator


signature.asc
Description: Digital signature

Re: [HACKERS] MSVC odd TAP test problem

[Craig Ringer]

On 7 May 2017 4:24 am, "Andrew Dunstan" 
wrote:


I have been working on enabling the remaining TAP tests on MSVC build in
the buildfarm client, but I have come across an odd problem. The bin
tests all run fine, but the recover tests crash and in such a way as to
crash the buildfarm client itself and require some manual cleanup. This
happens at some stage after the tests have run (the final "ok" is
output) but before the END handler in PostgresNode.pm (I put some traces
in there to see if I could narrow down where there were problems).

The symptom is that this appears at the end of the output when the
client calls "vcregress.pl taptest src/test/recover":

Terminating on signal SIGBREAK(21)
Terminating on signal SIGBREAK(21)
Terminate batch job (Y/N)?

And at that point there is nothing at all apparently running, according
to Sysinternals Process Explorer, including the buildfarm client.

It's 100% repeatable on bowerbird, and I'm a bit puzzled about how to
fix it.


Anyone have any clues?


That looks like we've upset CMD.exe its self. I'm not sure how ... leaking
a signal to the parent proc?

I suspect this could be something to do with console process groups.

Bowerbird is win8 . So this isn't going to be related to the support for
ANSI escapes added in win10.

A serach for the error turns up a complaint about IPC::Run as the first
hit. Probably not coincidence.


http://stackoverflow.com/q/40924750

See this bug

https://rt.cpan.org/Public/Bug/Display.html?id=101093

Re: [HACKERS] CTE inlining

[Vik Fearing]

On 05/03/2017 07:33 PM, Alvaro Herrera wrote:
> 1) we switch unmarked CTEs as inlineable by default in pg11.  What seems
> likely to happen for a user that upgrades to pg11 is that 5 out of 10
> CTE-using queries are going to become faster than with pg10, and they
> are going to be happy; 4 out of five are going to see no difference, but
> they didn't have to do anything about it; and the remaining query is
> going to become slower, either indistinguishably so (in which case they
> don't care and they remain happy because of the other improvements) or
> notably so, in which case they can easily figure where to add the
> MATERIALIZED option and regain the original performance.

I am overwhelmingly in favor of this option.

I am in favor of an enable_inlinedcte guc in the same spirit as the
other enable_* gucs, but violently opposed to any "compatibility" guc.

-- 
Vik Fearing  +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support



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

[HACKERS] MSVC odd TAP test problem

[Andrew Dunstan]

I have been working on enabling the remaining TAP tests on MSVC build in
the buildfarm client, but I have come across an odd problem. The bin
tests all run fine, but the recover tests crash and in such a way as to
crash the buildfarm client itself and require some manual cleanup. This
happens at some stage after the tests have run (the final "ok" is
output) but before the END handler in PostgresNode.pm (I put some traces
in there to see if I could narrow down where there were problems).

The symptom is that this appears at the end of the output when the
client calls "vcregress.pl taptest src/test/recover":

Terminating on signal SIGBREAK(21)
Terminating on signal SIGBREAK(21)
Terminate batch job (Y/N)?

And at that point there is nothing at all apparently running, according
to Sysinternals Process Explorer, including the buildfarm client.

It's 100% repeatable on bowerbird, and I'm a bit puzzled about how to
fix it.


Anyone have any clues?


cheers


andrew

-- 
Andrew Dunstanhttps://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] "CURRENT_ROLE" is not documented

[Fabien COELHO]

 functions with the attribute SECURITY DEFINER.
 In Unix parlance, the session user is the real user and
 the current user is the effective user.
+ current_role and user are
+ synonyms for current_user.  (The SQL standard draws
+ a distinction between current_role
+ and current_user, but PostgreSQL
+ does not, since it unifies users and roles into a single kind of entity.)


Looks simple and good to me. Thanks for the wording!

--
Fabien.


--
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] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

[Noah Misch]

On Wed, May 03, 2017 at 11:29:29PM -0400, Peter Eisentraut wrote:
> On 4/30/17 04:05, Noah Misch wrote:
> > The above-described topic is currently a PostgreSQL 10 open item.  Peter,
> > since you committed the patch believed to have created it, you own this open
> > item.  If some other commit is more relevant or if this does not belong as a
> > v10 open item, please let us know.  Otherwise, please observe the policy on
> > open item ownership[1] and send a status update within three calendar days 
> > of
> > this message.  Include a date for your subsequent status update.  Testers 
> > may
> > discover new open items at any time, and I want to plan to get them all 
> > fixed
> > well in advance of shipping v10.  Consequently, I will appreciate your 
> > efforts
> > toward speedy resolution.  Thanks.
> 
> I'm working on this and will report on Friday.

This PostgreSQL 10 open item is past due for your status update.  Kindly send
a status update within 24 hours, and include a date for your subsequent status
update.  Refer to the policy on open item ownership:
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com


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

Re: [HACKERS] Transition tables for triggers on foreign tables and views

[Noah Misch]

On Tue, May 02, 2017 at 06:06:47PM -0500, Kevin Grittner wrote:
> On Fri, Apr 28, 2017 at 10:56 PM, Tom Lane  wrote:
> 
> > They will fire if you have an INSTEAD OF row-level trigger; the existence
> > of that trigger is what determines whether we implement DML on a view
> > through the view's own triggers or through translation to an action on
> > the underlying table.
> >
> > I do not think it'd be reasonable to throw an error for creation of
> > a statement-level view trigger when there's no row-level trigger,
> > because that just imposes a hard-to-deal-with DDL ordering dependency.
> >
> > You could make a case for having the updatable-view translation code
> > print a WARNING if it notices that there are statement-level triggers
> > that cannot be fired due to the translation.
> 
> Oh, I see -- you can add all the AFTER ... FOR EACH STATEMENT
> triggers you want for an updatable view and they will quietly sit
> there without firing no matter how many statements perform the
> supposedly triggering action, but as soon as you add a INSTEAD OF
> ... FOR EACH ROW trigger they spring to life.  On the face of it that
> seems to me to violate the POLA, but I kinda see how it evolved.
> 
> I need to look at this and the rather similar odd behavior under
> inheritance.  I hope to post something Friday.

This PostgreSQL 10 open item is past due for your status update.  Kindly send
a status update within 24 hours, and include a date for your subsequent status
update.  Refer to the policy on open item ownership:
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com


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

[HACKERS] Re: transition table behavior with inheritance appears broken (was: Declarative partitioning - another take)

[Noah Misch]

On Mon, May 01, 2017 at 11:10:52AM -0500, Kevin Grittner wrote:
> On Mon, May 1, 2017 at 10:01 AM, Robert Haas  wrote:
> 
> > It seems pretty clear to me that this is busted.
> 
> I don't think you actually tested anything that is dependent on any
> of my patches there.
> 
> > Adding this as an open item.  Kevin?
> 
> It will take some time to establish what legacy behavior is and how
> the new transition tables are impacted.  My first reaction is that a
> trigger on the parent should fire for any related action on a child
> (unless maybe the trigger is defined with an ONLY keyword???) using
> the TupleDesc of the parent.  Note that the SQL spec mandates that
> even in a AFTER EACH ROW trigger the transition tables must
> represent all rows affected by the STATEMENT.  I think that this
> should be independent of triggers fired at the row level.  I think
> the rules should be similar for updateable views.
> 
> This will take some time to investigate, discuss and produce a
> patch.  I think best case is Friday.

[Action required within three days.  This is a generic notification.]

The above-described topic is currently a PostgreSQL 10 open item.  Kevin,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within three calendar days of
this message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10.  Consequently, I will appreciate your efforts
toward speedy resolution.  Thanks.

[1] 
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com


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

Re: [HACKERS] SUBSCRIPTIONS and pg_upgrade

[Noah Misch]

On Fri, May 05, 2017 at 11:01:57AM -0400, Peter Eisentraut wrote:
> On 5/2/17 21:44, Noah Misch wrote:
> >> I wonder if we should have an --no-subscriptions option, now that they
> >> are dumped by default, just like we have --no-blobs, --no-owner,
> >> --no-password, --no-privileges, --no-acl, --no-tablespaces, and
> >> --no-security-labels.  It seems like there is probably a fairly large
> >> use case for excluding subscriptions even if you have sufficient
> >> permissions to dump them.
> > 
> > [Action required within three days.  This is a generic notification.]
> > 
> > The above-described topic is currently a PostgreSQL 10 open item.  Peter,
> > since you committed the patch believed to have created it, you own this open
> > item.  If some other commit is more relevant or if this does not belong as a
> > v10 open item, please let us know.  Otherwise, please observe the policy on
> > open item ownership[1] and send a status update within three calendar days 
> > of
> > this message.  Include a date for your subsequent status update.  Testers 
> > may
> > discover new open items at any time, and I want to plan to get them all 
> > fixed
> > well in advance of shipping v10.  Consequently, I will appreciate your 
> > efforts
> > toward speedy resolution.  Thanks.
> 
> I consider this item low priority and don't plan to work on it before
> all the other open items under logical replication are addressed.
> 
> (Here, working on it would include thinking further about whether it is
> necessary at all or what alternatives might look like.)

That's informative, but it's not a valid status update.  This PostgreSQL 10
open item is past due for your status update.  Kindly send a valid status
update within 24 hours.  Refer to the policy on open item ownership:
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com


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

Re: [HACKERS] "CURRENT_ROLE" is not documented

[Tom Lane]

Fabien COELHO  writes:
>> I agree we ought to document this, but we likely need to mention
>> the discrepancy from the spec, too.

> Yep. A little subtle, though. Maybe it is enough to just say that for pg a 
> user is a role, which is not the case in the standard?

I did it like this:

*** 15943,15948 
--- 15956,15966 
  functions with the attribute SECURITY DEFINER.
  In Unix parlance, the session user is the real user and
  the current user is the effective user.
+ current_role and user are
+ synonyms for current_user.  (The SQL standard draws
+ a distinction between current_role
+ and current_user, but PostgreSQL
+ does not, since it unifies users and roles into a single kind of entity.)
 
  
 

I stole the "unifies..." language out of the CREATE ROLE page.

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] "CURRENT_ROLE" is not documented

[Fabien COELHO]

  psql> SELECT CURRENT_ROLE;
current_user -- not a typo, it really says "current_user"


Not as of HEAD ;-)


Good:-) I was connecting to a 9.6.2 server from a pg10dev client.


Is there a special reason why it does not appear in the documentation?


Oversight, evidently.


Ok.


Also, there is a SESSION_USER, but no SESSION_ROLE. Not sure of the
rationale.


SQL standard says so, basically.  The standard draws a hard line between
"role" and "user", and says that only "users" can be the initiators of
sessions, so that the initial privilege identifier is always a user name
not a role name; hence no need for SESSION_ROLE.


Hmmm... why not. I'm in the pg context where a USER is a ROLE, as you 
point out below.



PG doesn't draw such a hard line; for us, roles and users are the same
kind of entity, with the distinction being a can-login privilege that's
really only a minor attribute.  So I think it's sensible for us to
treat these functions as synonyms.


Yep.


I agree we ought to document this, but we likely need to mention
the discrepancy from the spec, too.


Yep. A little subtle, though. Maybe it is enough to just say that for pg a 
user is a role, which is not the case in the standard?


Thanks for the explanation!

--
Fabien.


--
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] Draft release notes for next week's back-branch releases

[Tom Lane]

Petr Jelinek  writes:
> On 06/05/17 19:15, Tom Lane wrote:
>> (Or, wait a minute.  That documentation only applies to v10, but we
>> need to be writing this relnote for 9.6 users.  What terminology should
>> we be using anyway?)

> Yeah we need to somehow mention that it only affects 3rd party tools
> using logical decoding.

> "The initial snapshot created for a logical decoding slot was
> potentially incorrect.  This could allow the 3rd party tools using
> the logical decoding to copy incomplete existing(?) data.  This was
> more likely to happen if the source server was busy at the time of
> slot creation, or if two slots were created concurrently."

>> Also, do we need to recommend that people not trust any logical replicas
>> at this point, but recreate them after installing the update?

> Yes, but only if there was preexisting data *and* there was concurrent
> activity on the server when the "replication" was setup.

OK, I can work with this.  Thanks for the help!

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] Draft release notes for next week's back-branch releases

[Petr Jelinek]

On 06/05/17 19:15, Tom Lane wrote:
> Petr Jelinek  writes:
>> On 06/05/17 18:16, Tom Lane wrote:
>>> Hmm, I'm hoping for something more user-oriented.  Is the corruption
>>> time-limited?  What's an "exported snapshot" anyway, is it the same
>>> thing as pg_export_snapshot(), and if so what's that got to do with
>>> logical replication?
> 
>> Okay to explain what's happening. When you create logical replication
>> slot via walsender, it exports snapshot (like the one exported by
>> pg_export_snapshot() indeed) which corresponds to exact point in time
>> where the decoding will start for the slot. You can import this snapshot
>> to another backend and use it to copy any existing data before starting
>> the replication. The bugs cause that these snapshots would be corrupted
>> and you'd have inconsistent data (some tuples missing). One of them
>> would export snapshot that's only valid for system catalogs but not user
>> tables (the ondisk snapshot, this needs at least one preexisting slot).
>> The other would not guarantee that tuples needed by the snapshot weren't
>> removed by vacuum of HOT pruning (the window being only the time it took
>> to generate the snapshot).
> 
> OK, that's better.  But I'm still not really seeing a reason to treat
> these as two separate items for release-note purposes: I don't think users

Sure my main issue was that the text combined the info from two commits
in a way that made the statement no longer correct.

> will care.  Now that I've read section 31.4, I'd suggest that we phrase
> the release notes in the terms it uses.  How about saying something like
> "The initial snapshot created for a logical replication slot was
> incorrect.  This could allow the apply process to copy incomplete or
> inconsistent data.  This was more likely to happen if the source server
> was busy at the time of slot creation, or if two slots were created
> concurrently" ?
> 
> (Or, wait a minute.  That documentation only applies to v10, but we
> need to be writing this relnote for 9.6 users.  What terminology should
> we be using anyway?)

Yeah we need to somehow mention that it only affects 3rd party tools
using logical decoding.

"The initial snapshot created for a logical decoding slot was
potentially incorrect.  This could allow the 3rd party tools using
the logical decoding to copy incomplete existing(?) data.  This was
more likely to happen if the source server was busy at the time of
slot creation, or if two slots were created concurrently."

> 
> Also, do we need to recommend that people not trust any logical replicas
> at this point, but recreate them after installing the update?
> 

Yes, but only if there was preexisting data *and* there was concurrent
activity on the server when the "replication" was setup.

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


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

Re: [HACKERS] Draft release notes for next week's back-branch releases

[Tom Lane]

Petr Jelinek  writes:
> On 06/05/17 18:16, Tom Lane wrote:
>> Hmm, I'm hoping for something more user-oriented.  Is the corruption
>> time-limited?  What's an "exported snapshot" anyway, is it the same
>> thing as pg_export_snapshot(), and if so what's that got to do with
>> logical replication?

> Okay to explain what's happening. When you create logical replication
> slot via walsender, it exports snapshot (like the one exported by
> pg_export_snapshot() indeed) which corresponds to exact point in time
> where the decoding will start for the slot. You can import this snapshot
> to another backend and use it to copy any existing data before starting
> the replication. The bugs cause that these snapshots would be corrupted
> and you'd have inconsistent data (some tuples missing). One of them
> would export snapshot that's only valid for system catalogs but not user
> tables (the ondisk snapshot, this needs at least one preexisting slot).
> The other would not guarantee that tuples needed by the snapshot weren't
> removed by vacuum of HOT pruning (the window being only the time it took
> to generate the snapshot).

OK, that's better.  But I'm still not really seeing a reason to treat
these as two separate items for release-note purposes: I don't think users
will care.  Now that I've read section 31.4, I'd suggest that we phrase
the release notes in the terms it uses.  How about saying something like
"The initial snapshot created for a logical replication slot was
incorrect.  This could allow the apply process to copy incomplete or
inconsistent data.  This was more likely to happen if the source server
was busy at the time of slot creation, or if two slots were created
concurrently" ?

(Or, wait a minute.  That documentation only applies to v10, but we
need to be writing this relnote for 9.6 users.  What terminology should
we be using anyway?)

Also, do we need to recommend that people not trust any logical replicas
at this point, but recreate them after installing the update?

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] statement_timeout is not working as expected with postgres_fdw

[Robert Haas]

On Sat, May 6, 2017 at 12:55 PM, Robert Haas  wrote:
> Oh!  Good catch.  Given that the behavior in question is intentional
> there and intended to provide backward compatibility, changing it in
> back-branches doesn't seem like a good plan.  I'll adjust the patch so
> that it continues to ignore errors in that case.

Updated patch attached.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index c6e3d44..2243ff8 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -14,6 +14,8 @@
 
 #include "postgres_fdw.h"
 
+#include "access/htup_details.h"
+#include "catalog/pg_user_mapping.h"
 #include "access/xact.h"
 #include "mb/pg_wchar.h"
 #include "miscadmin.h"
@@ -21,6 +23,7 @@
 #include "storage/latch.h"
 #include "utils/hsearch.h"
 #include "utils/memutils.h"
+#include "utils/syscache.h"
 
 
 /*
@@ -49,6 +52,7 @@ typedef struct ConnCacheEntry
  * one level of subxact open, etc */
 	bool		have_prep_stmt; /* have we prepared any stmts in this xact? */
 	bool		have_error;		/* have any subxacts aborted in this xact? */
+	bool		abort_cleanup_incomplete;		/* (sub)abort cleanup pending */
 } ConnCacheEntry;
 
 /*
@@ -74,6 +78,12 @@ static void pgfdw_subxact_callback(SubXactEvent event,
 	   SubTransactionId mySubid,
 	   SubTransactionId parentSubid,
 	   void *arg);
+static void pgfdw_reject_if_abort_cleanup_incomplete(ConnCacheEntry *entry);
+static bool pgfdw_cancel_query(PGconn *conn);
+static bool pgfdw_exec_cleanup_query(PGconn *conn, const char *query,
+		 bool ignore_errors);
+static bool pgfdw_get_cleanup_result(PGconn *conn, TimestampTz endtime,
+		 PGresult **result);
 
 
 /*
@@ -139,8 +149,12 @@ GetConnection(UserMapping *user, bool will_prep_stmt)
 		entry->xact_depth = 0;
 		entry->have_prep_stmt = false;
 		entry->have_error = false;
+		entry->abort_cleanup_incomplete = false;
 	}
 
+	/* Reject further use of connections which failed abort cleanup. */
+	pgfdw_reject_if_abort_cleanup_incomplete(entry);
+
 	/*
 	 * We don't check the health of cached connection here, because it would
 	 * require some overhead.  Broken connection will be detected when the
@@ -604,6 +618,8 @@ pgfdw_xact_callback(XactEvent event, void *arg)
 		/* If it has an open remote transaction, try to close it */
 		if (entry->xact_depth > 0)
 		{
+			bool		abort_cleanup_failure = false;
+
 			elog(DEBUG3, "closing remote transaction on connection %p",
  entry->conn);
 
@@ -611,6 +627,13 @@ pgfdw_xact_callback(XactEvent event, void *arg)
 			{
 case XACT_EVENT_PARALLEL_PRE_COMMIT:
 case XACT_EVENT_PRE_COMMIT:
+
+	/*
+	 * If abort cleanup previously failed for this connection,
+	 * we can't issue any more commands against it.
+	 */
+	pgfdw_reject_if_abort_cleanup_incomplete(entry);
+
 	/* Commit all remote transactions during pre-commit */
 	do_sql_command(entry->conn, "COMMIT TRANSACTION");
 
@@ -660,6 +683,24 @@ pgfdw_xact_callback(XactEvent event, void *arg)
 	break;
 case XACT_EVENT_PARALLEL_ABORT:
 case XACT_EVENT_ABORT:
+
+	/*
+	 * Don't try to clean up the connection if we're already
+	 * in error recursion trouble.
+	 */
+	if (in_error_recursion_trouble())
+		entry->abort_cleanup_incomplete = true;
+
+	/*
+	 * If connection is already unsalvageable, don't touch it
+	 * further.
+	 */
+	if (entry->abort_cleanup_incomplete)
+		break;
+
+	/* Remember that abort cleanup is in progress. */
+	entry->abort_cleanup_incomplete = true;
+
 	/* Assume we might have lost track of prepared statements */
 	entry->have_error = true;
 
@@ -670,40 +711,35 @@ pgfdw_xact_callback(XactEvent event, void *arg)
 	 * command is still being processed by the remote server,
 	 * and if so, request cancellation of the command.
 	 */
-	if (PQtransactionStatus(entry->conn) == PQTRANS_ACTIVE)
+	if (PQtransactionStatus(entry->conn) == PQTRANS_ACTIVE &&
+		!pgfdw_cancel_query(entry->conn))
 	{
-		PGcancel   *cancel;
-		char		errbuf[256];
-
-		if ((cancel = PQgetCancel(entry->conn)))
-		{
-			if (!PQcancel(cancel, errbuf, sizeof(errbuf)))
-ereport(WARNING,
-		(errcode(ERRCODE_CONNECTION_FAILURE),
-  errmsg("could not send cancel request: %s",
-		 errbuf)));
-			PQfreeCancel(cancel);
-		}
+		/* Unable to cancel running query. */
+		abort_cleanup_failure = true;
+	}
+	else if (!pgfdw_exec_cleanup_query(entry->conn,
+	   "ABORT TRANSACTION",
+	   false))
+	{
+		/* Unable to abort remote transaction. */
+		abort_cleanup_failure = true;
+	}
+	else if (entry->have_prep_stmt && entry->have_error &&
+			 !pgfdw_exec_cleanup_query(entry->conn,
+	   "DEALLOCATE ALL",
+	   

Re: [HACKERS] "CURRENT_ROLE" is not documented

[Tom Lane]

Fabien COELHO  writes:
> While trying to understand whether there was any difference, I noticed 
> that CURRENT_ROLE is an undocumented synonymous for CURRENT_USER:

>   psql> SELECT CURRENT_ROLE;
> current_user -- not a typo, it really says "current_user"

Not as of HEAD ;-)

> Is there a special reason why it does not appear in the documentation?

Oversight, evidently.

> Also, there is a SESSION_USER, but no SESSION_ROLE. Not sure of the 
> rationale.

SQL standard says so, basically.  The standard draws a hard line between
"role" and "user", and says that only "users" can be the initiators of
sessions, so that the initial privilege identifier is always a user name
not a role name; hence no need for SESSION_ROLE.

It looks to me like according to the spec, when the current privilege
identifier is a role name, then CURRENT_ROLE returns that name and
CURRENT_USER returns NULL; when the current privilege identifier is a
user name, the opposite is true.

PG doesn't draw such a hard line; for us, roles and users are the same
kind of entity, with the distinction being a can-login privilege that's
really only a minor attribute.  So I think it's sensible for us to
treat these functions as synonyms.

Perhaps we could satisfy the letter of the spec by having one of these
functions return NULL depending on the current role's can-login attribute,
but I frankly cannot see a reason why that would be a good thing to do.
It would mostly be a foot-gun for SQL queries --- I think you'd basically
always have to write "coalesce(current_user, current_role)" to avoid
having your code break in unexpected contexts.

I agree we ought to document this, but we likely need to mention
the discrepancy from the spec, too.

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] Draft release notes for next week's back-branch releases

[Petr Jelinek]

On 06/05/17 18:16, Tom Lane wrote:
> Petr Jelinek  writes:
>> On 06/05/17 17:25, Tom Lane wrote:
>>> OK, can you suggest better wording?
> 
>> Something like the attached (it requires some polishing of English
>> probably).
> 
> Hmm, I'm hoping for something more user-oriented.  Is the corruption
> time-limited?  What's an "exported snapshot" anyway, is it the same
> thing as pg_export_snapshot(), and if so what's that got to do with
> logical replication?
> 

Well user does not care unless they use something like pglogical, or
bottledwater, or wal2json, etc.

Okay to explain what's happening. When you create logical replication
slot via walsender, it exports snapshot (like the one exported by
pg_export_snapshot() indeed) which corresponds to exact point in time
where the decoding will start for the slot. You can import this snapshot
to another backend and use it to copy any existing data before starting
the replication. The bugs cause that these snapshots would be corrupted
and you'd have inconsistent data (some tuples missing). One of them
would export snapshot that's only valid for system catalogs but not user
tables (the ondisk snapshot, this needs at least one preexisting slot).
The other would not guarantee that tuples needed by the snapshot weren't
removed by vacuum of HOT pruning (the window being only the time it took
to generate the snapshot).

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


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

Re: [HACKERS] statement_timeout is not working as expected with postgres_fdw

[Robert Haas]

On Fri, May 5, 2017 at 7:11 PM, Amit Kapila  wrote:
> There is a comment in the code which explains why currently we ignore
> errors from DEALLOCATE ALL.
>
> * DEALLOCATE ALL only exists in 8.3 and later, so this
> * constrains how old a server postgres_fdw can
> * communicate with.  We intentionally ignore errors in
> * the DEALLOCATE, so that we can hobble along to some
> * extent with older servers (leaking prepared statements
> * as we go; but we don't really support update operations
> * pre-8.3 anyway).
>
> It is not entirely clear to me whether it is only for Commit case or
> in general.  From the code, it appears that the comment holds true for
> both commit and abort.  If it is for both, then after this patch, the
> above comment will not be valid and you might want to update it in
> case we want to proceed with your proposed changes for DEALLOCATE ALL
> statement.

Oh!  Good catch.  Given that the behavior in question is intentional
there and intended to provide backward compatibility, changing it in
back-branches doesn't seem like a good plan.  I'll adjust the patch so
that it continues to ignore errors in that case.

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


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

Re: [HACKERS] Draft release notes for next week's back-branch releases

[Tom Lane]

Petr Jelinek  writes:
> On 06/05/17 17:25, Tom Lane wrote:
>> OK, can you suggest better wording?

> Something like the attached (it requires some polishing of English
> probably).

Hmm, I'm hoping for something more user-oriented.  Is the corruption
time-limited?  What's an "exported snapshot" anyway, is it the same
thing as pg_export_snapshot(), and if so what's that got to do with
logical replication?

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] Draft release notes for next week's back-branch releases

[Petr Jelinek]

On 06/05/17 17:25, Tom Lane wrote:
> Petr Jelinek  writes:
>> Hmm, the 2bef06d51 and 56e19d938 are fixes for different bugs, we can
>> keep them together since result of both is corrupted snapshot, but the
>> description can't just mangle pieces of text from different commits
>> together like this as that's misleading.
> 
> OK, can you suggest better wording?
> 

Something like the attached (it requires some polishing of English
probably).

-- 
  Petr Jelinek  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/doc/src/sgml/release-9.6.sgml b/doc/src/sgml/release-9.6.sgml
index cdff084..195cbb1 100644
--- a/doc/src/sgml/release-9.6.sgml
+++ b/doc/src/sgml/release-9.6.sgml
@@ -92,17 +92,24 @@ Branch: REL9_5_STABLE [54270d7eb] 2017-04-27 15:29:43 -0700
 Branch: REL9_4_STABLE [b6ecf26cc] 2017-04-27 15:29:52 -0700
 -->
  
-  Fix possibly-corrupt initial snapshot during logical decoding
-  (Petr Jelinek, Andres Freund)
+  Fix two bugs resulting in possibly-corrupt initial exported snapshot
+  during logical decoding (Petr Jelinek, Andres Freund)
  
 
  
   If a logical decoding replication slot was created while another slot
-  already exists, it was initialized with a potentially-corrupted
-  snapshot, allowing wrong data to be returned during decoding.
-  The time window during which this snapshot continued to be used
-  depended on how busy the server was; under low load it would be hard
-  to see any problem.
+  already exists, it would exported a potentially-corrupted snapshot,
+  allowing wrong data to be returned when such snapshot was imported to
+  another backend.
+ 
+
+ 
+  Logical decoding normally preserves all required historical catalog
+  tuples, it didn't guarantee that the non-catalog tuples also
+  were preserved for exported snapshot. This could have resulted in
+  concurrent VACUUM or HOT pruning to remove tuples needed by the
+  snapshot. The time window for this was very small and the problem
+  required server to be very busy during slot creation.
  
 
 

-- 
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] Draft release notes for next week's back-branch releases

[Tom Lane]

Petr Jelinek  writes:
> Hmm, the 2bef06d51 and 56e19d938 are fixes for different bugs, we can
> keep them together since result of both is corrupted snapshot, but the
> description can't just mangle pieces of text from different commits
> together like this as that's misleading.

OK, can you suggest better wording?

regards, tom lane


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

[HACKERS] Atomics for heap_parallelscan_nextpage()

[David Rowley]

Hi,

A while back I did some benchmarking on a big 4 socket machine to
explore a bit around the outer limits of parallel aggregates.  I
discovered along the way that, given enough workers, and a simple
enough task, that seq-scan workers were held up waiting for the lock
to be released in heap_parallelscan_nextpage().

I've since done a little work in this area to try to improve things. I
ended up posting about it yesterday in [1]. My original patch used
batching to solve the issue; instead of allocating 1 block at a time,
the batching patch allocated a range of 10 blocks for the worker to
process. However, the implementation still needed a bit of work around
reporting sync-scan locations.

Andres mentioned in [2] that it might be worth exploring using atomics
to do the same job. So I went ahead and did that, and came up with the
attached, which is a slight variation on what he mentioned in the
thread.

To keep things a bit more simple, and streamline, I ended up pulling
out the logic for setting the startblock into another function, which
we only call once before the first call to
heap_parallelscan_nextpage().  I also ended up changing phs_cblock and
replacing it with a counter that always starts at zero. The actual
block is calculated based on that + the startblock modulo nblocks.
This makes things a good bit more simple for detecting when we've
allocated all the blocks to the workers, and also works nicely when
wrapping back to the start of a relation when we started somewhere in
the middle due to piggybacking with a synchronous scan.

Performance:

With parallel_workers=71, it looks something like:

Query 1: 881 GB, ~6 billion row TPC-H lineitem table.

tpch=# select count(*) from lineitem;
   count

 589709
(1 row)

-- Master
Time: 123421.283 ms (02:03.421)
Time: 118895.846 ms (01:58.896)
Time: 118632.546 ms (01:58.633)

-- Atomics patch
Time: 74038.813 ms (01:14.039)
Time: 73166.200 ms (01:13.166)
Time: 72492.338 ms (01:12.492)

-- Batching Patch: Batching 10 pages at a time in heap_parallelscan_nextpage()
Time: 76364.215 ms (01:16.364)
Time: 75808.900 ms (01:15.809)
Time: 74927.756 ms (01:14.928)

Query 2: Single int column table with 2 billion rows.

tpch=# select count(*) from a;
   count

 20
(1 row)

-- Master
Time: 5853.918 ms (00:05.854)
Time: 5925.633 ms (00:05.926)
Time: 5859.223 ms (00:05.859)

-- Atomics patch
Time: 5825.745 ms (00:05.826)
Time: 5849.139 ms (00:05.849)
Time: 5815.818 ms (00:05.816)

-- Batching Patch: Batching 10 pages at a time in heap_parallelscan_nextpage()
Time: 5789.237 ms (00:05.789)
Time: 5837.395 ms (00:05.837)
Time: 5821.492 ms (00:05.821)

I've also attached a text file with the perf report for the lineitem
query. You'll notice that the heap_parallelscan_nextpage() is very
visible in master, but not on each of the two patches.

With the 2nd query, heap_parallelscan_nextpage is fairly insignificant
on master's profile, it's only showing up as 0.48%. Likely this must
be due to more tuples being read from the page, and more aggregation
work getting done before the next page is needed. I'm uncertain why I
previously saw a speed up in this case in [1].

I've also noticed that both the atomics patch and unpatched master do
something that looks a bit weird with synchronous seq-scans. If the
parallel seq-scan piggybacked on another scan, then subsequent
parallel scans will start at the same non-zero block location, even
when no other concurrent scans exist. I'd have expected this should go
back to block 0 again, but maybe I'm just failing to understand the
reason for reporting the startblock to ss_report_location() at the end
of the scan.

I'll now add this to the first commitfest of pg11. I just wanted to
note that I've done this, so that it's less likely someone else goes
and repeats the same work.

[1] 
https://www.postgresql.org/message-id/CAKJS1f-XhfQ2-%3D85wgYo5b3WtEs%3Dys%3D2Rsq%3DNuvnmaV4ZsM1XQ%40mail.gmail.com

[2] 
https://www.postgresql.org/message-id/20170505023646.3uhnmf2hbwtm63lc%40alap3.anarazel.de

-- 
 David Rowley   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
-- Unpatched select count(*) from lineitem; 71 workers
  Children  Self  Command   Shared Object Symbol
+   99.83% 0.00%  postgres  libpthread-2.17.so[.] __restore_rt
+   99.83% 0.00%  postgres  postgres  [.] sigusr1_handler
+   99.83% 0.00%  postgres  postgres  [.] maybe_start_bgworkers
+   99.83% 0.00%  postgres  postgres  [.] do_start_bgworker
+   99.83% 0.93%  postgres  postgres  [.] ExecProcNode
+   99.83% 0.00%  postgres  postgres  [.] StartBackgroundWorker
+   99.83% 0.00%  postgres  postgres  [.] ParallelWorkerMain
+   99.83% 0.00%  postgres  postgres  [.] ParallelQueryMain
+   99.83% 0.00%  postgres  postgres  [.] ExecutorRun
+   99.83% 0.00%  

Re: [HACKERS] Draft release notes for next week's back-branch releases

[Petr Jelinek]

On 06/05/17 01:37, Tom Lane wrote:
> I've written $SUBJECT ... you can find them at
> 
> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=54dbd4dc78b045ffcc046b9a43681770c3992dd4
> 

Hmm, the 2bef06d51 and 56e19d938 are fixes for different bugs, we can
keep them together since result of both is corrupted snapshot, but the
description can't just mangle pieces of text from different commits
together like this as that's misleading.

The 2bef06d51 is about not preserving global xmin correctly, window for
that is indeed small. But the 56e19d938 was about using ondisk snapshot
(which only contain transactions changing catalogs) as full snapshots,
it's more about creating multiple logical replication slots while
concurrent writes happen.

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


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

Re: [HACKERS] Not getting error if ALTER SUBSCRIPTION syntax is wrong.

[Petr Jelinek]

On 05/05/17 19:51, Petr Jelinek wrote:
> On 05/05/17 14:40, tushar wrote:
>> Hi,
>>
>> While testing 'logical replication' against v10 , i encountered one
>> issue where data stop migrating after ALTER PUBLICATION.
>>
>> X Server
>> \\ Make sure wal_level is set to logical in postgresql.conf file
>> \\create table/Insert 1 row -> create table test(n int); insert into t
>> values (1);
>> \\create publication for all -> create publication pub for ALL TABLES ;
>>
>>
>> Y server
>>
>> \\ Make sure wal_level is set to logical in postgresql.conf file
>> \\create table -> create table test(n int);
>>
>> \\create Subscription
>>
>> CREATE SUBSCRIPTION sub CONNECTION 'host=localhost dbname=postgres
>> port=5432 ' PUBLICATION pub;
>>
>> [...]
>>
>> I think probably syntax of alter subscription is not correct but
>> surprisingly it is not throwing an error.
>>
> 
> Syntax of ALTER command is correct, syntax of the connection string is
> not, you are probably getting errors in log from the replication worker.
> 
> We could check validity of the connection string though to complain
> immediately like we do in CREATE.
> 

The attached does exactly that.

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


Check-connection-info-in-ALTER-SUBSCRIPTION.patch
Description: binary/octet-stream

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

Re: [HACKERS] [PROPOSAL] Use SnapshotAny in get_actual_variable_range

[Amit Kapila]

On Fri, May 5, 2017 at 1:28 PM, Dmitriy Sarafannikov
 wrote:
> Amit, thanks for comments!
>
>> 1.
>> +#define InitNonVacuumableSnapshot(snapshotdata)  \
>> + do { \
>> + (snapshotdata).satisfies = HeapTupleSatisfiesNonVacuumable; \
>> + (snapshotdata).xmin = RecentGlobalDataXmin; \
>> + } while(0)
>> +
>>
>> Can you explain and add comments why you think RecentGlobalDataXmin is
>> the right to use it here?  As of now, it seems to be only used for
>> pruning non-catalog tables.
>
> Can you explain me, what value for xmin should be used there?
>

I think we can use RecentGlobalDataXmin for non-catalog relations and
RecentGlobalXmin for catalog relations (probably a check similar to
what we have in heap_page_prune_opt).


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


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

Re: [HACKERS] password_encryption, default and 'plain' support

[Gavin Flower]

On 06/05/17 22:44, Vik Fearing wrote:

On 05/05/2017 02:42 PM, Michael Paquier wrote:

+This option is obsolete but still accepted for backwards
+compatibility.
Isn't that incorrect English?

No.


It seems to me that this be non-plural,
as "for backward compatibility".

"Backwards" is not plural, it's a regional variation of "backward" (or
vice versa depending on which region you come from).  Both are correct.


I am English, born & bred, and 'Backwards' feels a lot more natural to me.


Cheers,
Gavin



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

Re: [HACKERS] password_encryption, default and 'plain' support

[Vik Fearing]

On 05/05/2017 02:42 PM, Michael Paquier wrote:
> +This option is obsolete but still accepted for backwards
> +compatibility.
> Isn't that incorrect English?

No.

> It seems to me that this be non-plural,
> as "for backward compatibility".

"Backwards" is not plural, it's a regional variation of "backward" (or
vice versa depending on which region you come from).  Both are correct.

-- 
Vik Fearing  +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support



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

Re: [HACKERS] logical replication and PANIC during shutdown checkpoint in publisher

[Michael Paquier]

On Fri, May 5, 2017 at 11:50 PM, Peter Eisentraut
 wrote:
> On 5/5/17 01:26, Michael Paquier wrote:
>> The only code path doing HOT-pruning and generating WAL is
>> heap_page_prune(). Do you think that we need to worry about FPWs as
>> well?
>>
>> Attached is an updated patch, which also forbids the run of any
>> replication commands when the stopping state is reached.
>
> I have committed this without the HOT pruning change.  That can be
> considered separately, and I think it could use another round of
> thinking about it.

Agreed. Just adding an ERROR message in XLogInsert() is not going to
help much as this leads also to PANIC for critical sections :(
So a patch really needs to be a no-op for all WAL-related operations
within the WAL sender, and that will be quite invasive I am afraid.

> I will move the open item to "Older Bugs" now, because the user
> experience regression, so to speak, in version 10 has been addressed.
> (This could be a backpatching candidate, but I am not planning on it for
> next week's releases in any case.)

No issues with all that.
-- 
Michael


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

[HACKERS] "CURRENT_ROLE" is not documented

[Fabien COELHO]

While trying to understand whether there was any difference, I noticed 
that CURRENT_ROLE is an undocumented synonymous for CURRENT_USER:


 psql> SELECT CURRENT_ROLE;
   current_user -- not a typo, it really says "current_user"
   calvin

 sh> grep -i CURRENT_ROLE doc/src/sgml/*/*.sgml doc/src/sgml/*.sgml
  doc/src/sgml/keywords.sgml: CURRENT_ROLE

Is there a special reason why it does not appear in the documentation? If 
not, I would suggest to apply the attached minimal documentation patch.


Also, there is a SESSION_USER, but no SESSION_ROLE. Not sure of the 
rationale.


--
Fabien.diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index f06d0a9..9ce1dd0 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -15736,6 +15736,12 @@ SELECT * FROM pg_ls_dir('.') WITH ORDINALITY AS t(ls,n);
   
 
   
+   current_role
+   name
+   equivalent to current_user
+  
+
+  
current_schema[()]
name
name of current schema

-- 
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] Should pg_current_wal_location() become pg_current_wal_lsn()

[Tom Lane]

Noah Misch  writes:
> On Fri, May 05, 2017 at 03:36:39AM +1200, David Rowley wrote:
>> Do any of the committers who voted for this change feel inclined to
>> pick this patch up?

> I'll echo that question.  This open item lacks a clear owner.  One might argue
> that 806091c caused it by doing the backward-compatibility breaks that
> inspired this patch, but that's a link too tenuous to create ownership.

If no one else takes this, I will pick it up --- but I don't anticipate
having any time for it until after Monday's back-branch release wraps.

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