Re: [HACKERS] Parallel worker error

2017-10-30 Thread Amit Kapila
On Mon, Oct 30, 2017 at 10:04 AM, Robert Haas wrote: > On Mon, Oct 30, 2017 at 8:25 AM, Amit Kapila wrote: >> Thanks. I have closed this entry in CF app, however, I am not sure >> what is the best way to deal with the entry present in PostgreSQL

Re: [HACKERS] Parallel worker error

2017-10-29 Thread Robert Haas
On Mon, Oct 30, 2017 at 8:25 AM, Amit Kapila wrote: > Thanks. I have closed this entry in CF app, however, I am not sure > what is the best way to deal with the entry present in PostgreSQL 10 > Open Items page[1]. The entry for this bug seems to be present in > Older

Re: [HACKERS] Parallel worker error

2017-10-29 Thread Amit Kapila
On Sun, Oct 29, 2017 at 1:15 PM, Robert Haas wrote: > On Sun, Oct 29, 2017 at 12:02 PM, Amit Kapila wrote: >> This patch no longer applies, so attached rebased patches. I have >> also created patches for v10 as we might want to backpatch the fix.

Re: [HACKERS] Parallel worker error

2017-10-29 Thread Robert Haas
On Sun, Oct 29, 2017 at 12:02 PM, Amit Kapila wrote: > This patch no longer applies, so attached rebased patches. I have > also created patches for v10 as we might want to backpatch the fix. > Added the patch in CF (https://commitfest.postgresql.org/15/1342/) Thanks. I

Re: [HACKERS] Parallel worker error

2017-10-29 Thread Amit Kapila
On Sat, Sep 9, 2017 at 7:56 AM, Amit Kapila wrote: > On Fri, Sep 8, 2017 at 3:13 PM, Robert Haas wrote: >> On Fri, Sep 8, 2017 at 1:17 AM, Amit Kapila wrote: >>> You are right. I have changed the ordering and passed

Re: [HACKERS] Parallel worker error

2017-09-08 Thread Amit Kapila
On Fri, Sep 8, 2017 at 3:13 PM, Robert Haas wrote: > On Fri, Sep 8, 2017 at 1:17 AM, Amit Kapila wrote: >> You are right. I have changed the ordering and passed OuterUserId via >> FixedParallelState. > > This looks a little strange: > > +

Re: [HACKERS] Parallel worker error

2017-09-08 Thread Robert Haas
On Fri, Sep 8, 2017 at 1:17 AM, Amit Kapila wrote: > You are right. I have changed the ordering and passed OuterUserId via > FixedParallelState. This looks a little strange: +SetCurrentRoleId(fps->outer_user_id, fps->is_current_user_superuser); The first argument

Re: [HACKERS] Parallel worker error

2017-09-07 Thread Amit Kapila
On Fri, Sep 8, 2017 at 3:18 AM, Robert Haas wrote: > On Mon, Sep 4, 2017 at 5:46 AM, Amit Kapila wrote: >> It seems like the consensus is to move forward with this approach. I >> have written a patch implementing the above idea. Note, that to use

Re: [HACKERS] Parallel worker error

2017-09-07 Thread Robert Haas
On Mon, Sep 4, 2017 at 5:46 AM, Amit Kapila wrote: > It seems like the consensus is to move forward with this approach. I > have written a patch implementing the above idea. Note, that to use > SetCurrentRoleId, we need the value of guc "is_superuser" for the > current

Re: [HACKERS] Parallel worker error

2017-09-04 Thread Amit Kapila
On Sat, Sep 2, 2017 at 12:21 PM, Noah Misch wrote: > On Thu, Aug 31, 2017 at 03:11:10PM -0400, Robert Haas wrote: >> On Wed, Aug 30, 2017 at 11:19 AM, Robert Haas wrote: >> > But since that's an established design fl^H^Hprinciple, maybe that >> > means

Re: [HACKERS] Parallel worker error

2017-09-03 Thread Pavan Deolasee
On Wed, Aug 30, 2017 at 8:49 PM, Robert Haas wrote: > > > But since that's an established design fl^H^Hprinciple, maybe that > means we should go with the approach of teaching SerializeGUCState() > to ignore role altogether and instead have ParallelWorkerMain call >

Re: [HACKERS] Parallel worker error

2017-09-02 Thread Robert Haas
On Sat, Sep 2, 2017 at 2:51 AM, Noah Misch wrote: > [Action required within three days. This is a generic notification.] > > The above-described topic is currently a PostgreSQL 10 open item. Robert, > since you committed the patch believed to have created it, you own this

Re: [HACKERS] Parallel worker error

2017-09-02 Thread Amit Kapila
On Sat, Sep 2, 2017 at 12:21 PM, Noah Misch wrote: > On Thu, Aug 31, 2017 at 03:11:10PM -0400, Robert Haas wrote: >> On Wed, Aug 30, 2017 at 11:19 AM, Robert Haas wrote: >> > But since that's an established design fl^H^Hprinciple, maybe that >> > means

Re: [HACKERS] Parallel worker error

2017-09-02 Thread Amit Kapila
On Wed, Aug 30, 2017 at 8:49 PM, Robert Haas wrote: > On Wed, Aug 30, 2017 at 9:58 AM, Tom Lane wrote: >> The problem here is exactly that we cannot transmit the leader's >> state to the worker. You can't blame it on SET ROLE, because >> I didn't do

Re: [HACKERS] Parallel worker error

2017-09-02 Thread Noah Misch
On Thu, Aug 31, 2017 at 03:11:10PM -0400, Robert Haas wrote: > On Wed, Aug 30, 2017 at 11:19 AM, Robert Haas wrote: > > But since that's an established design fl^H^Hprinciple, maybe that > > means we should go with the approach of teaching SerializeGUCState() > > to ignore

Re: [HACKERS] Parallel worker error

2017-08-31 Thread Robert Haas
On Wed, Aug 30, 2017 at 11:19 AM, Robert Haas wrote: > But since that's an established design fl^H^Hprinciple, maybe that > means we should go with the approach of teaching SerializeGUCState() > to ignore role altogether and instead have ParallelWorkerMain call >

Re: [HACKERS] Parallel worker error

2017-08-30 Thread Tom Lane
Alvaro Herrera writes: > Robert Haas wrote: >> In this case, >> I'll blame the fact that we allow a role to be dropped while there are >> users connected using that role. > Actually, my first comment when Pavan mentioned this on IM was that we > should look into fixing

Re: [HACKERS] Parallel worker error

2017-08-30 Thread Alvaro Herrera
Robert Haas wrote: > In this case, > I'll blame the fact that we allow a role to be dropped while there are > users connected using that role. That's about as sensible as allowing > a table to be dropped while there are users reading from it, or a > database to be dropped while there are users

Re: [HACKERS] Parallel worker error

2017-08-30 Thread Robert Haas
On Wed, Aug 30, 2017 at 9:58 AM, Tom Lane wrote: > The problem here is exactly that we cannot transmit the leader's > state to the worker. You can't blame it on SET ROLE, because > I didn't do one. Hmm, that's a good reason for holding it blameless. In this case, I'll blame

Re: [HACKERS] Parallel worker error

2017-08-30 Thread Tom Lane
Robert Haas writes: > On Wed, Aug 30, 2017 at 9:20 AM, Tom Lane wrote: >> I"m okay with a narrow solution if SET ROLE really is >> the only problem, but at this stage I'm not convinced of that. > I don't think the problem with role is that it's

Re: [HACKERS] Parallel worker error

2017-08-30 Thread Robert Haas
On Wed, Aug 30, 2017 at 9:20 AM, Tom Lane wrote: > Robert Haas writes: >> On Wed, Aug 30, 2017 at 8:04 AM, Tom Lane wrote: >>> We might need to redesign the GUC-propagation mechanism so it sends >>> the various internal

Re: [HACKERS] Parallel worker error

2017-08-30 Thread Tom Lane
Robert Haas writes: > On Wed, Aug 30, 2017 at 8:04 AM, Tom Lane wrote: >> We might need to redesign the GUC-propagation mechanism so it sends >> the various internal representations of GUC values, not the user-visible >> strings. > That would probably

Re: [HACKERS] Parallel worker error

2017-08-30 Thread Robert Haas
On Wed, Aug 30, 2017 at 8:04 AM, Tom Lane wrote: > We might need to redesign the GUC-propagation mechanism so it sends > the various internal representations of GUC values, not the user-visible > strings. That would probably be better in the long run, but I'm not keen to do

Re: [HACKERS] Parallel worker error

2017-08-30 Thread Tom Lane
Amit Kapila writes: > I am able to reproduce this without involving session authorization > guc as well. One needs to drop the newly created role from another > session, then also we can see the same error. Hm. I suspect the basic shape of what's happening here is "an

Re: [HACKERS] Parallel worker error

2017-08-30 Thread Pavan Deolasee
On Wed, Aug 30, 2017 at 5:19 PM, Amit Kapila wrote: > > > I am able to reproduce this without involving session authorization > guc as well. One needs to drop the newly created role from another > session, then also we can see the same error. > > Yeah, that's how I

Re: [HACKERS] Parallel worker error

2017-08-30 Thread Tom Lane
I wrote: > I can duplicate this in HEAD and v10, but not 9.6, so I've added it > as an open issue for v10. No idea what broke it. Oh, scratch that: the issue exists in 9.6, it's just that the particular test query you're using here does not generate a parallelized plan in 9.6, as shown by

Re: [HACKERS] Parallel worker error

2017-08-30 Thread Amit Kapila
On Wed, Aug 30, 2017 at 5:11 PM, Robert Haas wrote: > On Wed, Aug 30, 2017 at 6:58 AM, Pavan Deolasee > wrote: >> It's quite possible that I don't understand the differences in "role" and >> "session authorization", but it still looks like a bug

Re: [HACKERS] Parallel worker error

2017-08-30 Thread Tom Lane
Pavan Deolasee writes: > The last statement in this test fails with an error: > ERROR: role "testuser1" does not exist > CONTEXT: parallel worker I can duplicate this in HEAD and v10, but not 9.6, so I've added it as an open issue for v10. No idea what broke it.

Re: [HACKERS] Parallel worker error

2017-08-30 Thread Robert Haas
On Wed, Aug 30, 2017 at 6:58 AM, Pavan Deolasee wrote: > It's quite possible that I don't understand the differences in "role" and > "session authorization", but it still looks like a bug to me. May be > SerializeGUCState() should check if SetRoleIsActive is true and

[HACKERS] Parallel worker error

2017-08-30 Thread Pavan Deolasee
Hello, While investing an issue in Postgres-XL 10, I came across this rather surprisingly behaviour in PG master. See this test case: create role testuser1; set role to testuser1; show role; -- shows testuser1 -- reset back to default role reset session authorization ; show role; -- shows none