Re: [HACKERS] TEXT vs VARCHAR join qual push down diffrence, bug or expected?

[Jeevan Chalke]

Hi Tom,

On Tue, Sep 22, 2015 at 12:31 AM, Tom Lane  wrote:

>
> I think you're blaming the wrong code; RelabelType is handled basically
> the same as most other cases.
>
> It strikes me that this function is really going about things the wrong
> way.  Rather than trying to determine the output collation per se, what
> we ought to be asking is "does every operator in the proposed expression
> have an input collation that can be traced to some foreign Var further
> down in the expression"?  That is, given the example in hand,
>
> RelabelType(ForeignVar) = RelabelType(LocalVar)
>
> the logic ought to be like "the ForeignVar has collation X, and that
> bubbles up without change through the RelabelType, and then the equals
> operator's inputcollation matches that, so accept it --- regardless of
> where the other operand's collation came from exactly".  The key point
> is that we want to validate operator input collations, not output
> collations, as having something to do with what the remote side would do.
>
> This would represent a fairly significant rewrite of foreign_expr_walker's
> collation logic; although I think the end result would be no more
> complicated, possibly simpler, than it is now.
>
> regards, tom lane
>


IIUC, you are saying that collation check for output collation is not
necessary for all OpExpr/FuncExpr/ArrayRef etc.
Should we remove code blocks like
collation = r->resultcollid;
if (collation == InvalidOid)
state = FDW_COLLATE_NONE;
else if (inner_cxt.state == FDW_COLLATE_SAFE &&
 collation == inner_cxt.collation)
state = FDW_COLLATE_SAFE;
else
state = FDW_COLLATE_UNSAFE;

and just bubble up the collation and state to the next level?

Here I see that, in the result collation validation, we missed the case when
result collation is default collation. For foreign var, we return collation
as is in inner context with the state set to SAFE. But in case of local var,
we are only allowing InvalidOid or Default oid for collation, however while
returning back, we set collation to InvalidOid and state to NONE even for
default collation. I think we are missing something here.

To handle this case, we need to either,
1. allow local var to set inner_cxt collation to what var actually has
(which
will be either Invalid or DEFAULT) and set state to NONE if non collable or
set state to SAFE if default collation. Like:
  In T_Var, local var case
collation = var->varcollid;
state = OidIsValid(collation) ? FDW_COLLATE_SAFE : FDW_COLLATE_NONE;

OR
2. In above code block, which checks result collation, we need to handle
default collation. Like:
else if (collation == DEFAULT_COLLATION_OID)
state = inner_cxt.state;

Let me know if I missed any.

Thanks

-- 
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company

Re: [HACKERS] Parallel Seq Scan

[Amit Kapila]

On Wed, Sep 23, 2015 at 6:42 AM, Kouhei Kaigai  wrote:
>
> > > On Thu, Sep 17, 2015 at 4:44 PM, Robert Haas 
wrote:
>
> > To verify the patch, I have done 2 things, first I have added elog to
> > the newly supported read funcs and then in planner, I have used
> > nodeToString and stringToNode on planned_stmt and then used the
> > newly generated planned_stmt for further execution.  After making these
> > changes, I have ran make check-world and ensures that it covers all the
> > newly added nodes.
> >
> > Note, that as we don't populate funcid's in expressions during read, the
> > same has to be updated by traversing the tree and updating in different
> > expressions based on node type.  Attached patch (read_funcs_test_v1)
> > contains the changes required for testing the patch.  I am not very sure
> > about what do about some of the ForeignScan fields (fdw_private) in
order
> > to update the funcid as the data in those expressions could be FDW
specific.
> > This is anyway for test, so doesn't matter much, but the same will be
> > required to support read of ForeignScan node by worker.
> >
> Because of interface contract, it is role of FDW driver to put nodes which
> are safe to copyObject on fdw_exprs and fdw_private field. Unless FDW
driver
> does not violate, fdw_exprs and fdw_private shall be reproduced on the
worker
> side. (Of course, we cannot guarantee nobody has local pointer on private
> field...)
> Sorry, I cannot understand the sentence of funcid population. It seems to
me
> funcid is displayed as-is, and _readFuncExpr() does nothing special...?
>

Here I am referring to operator's funcid (refer function _readOpExpr()).
It is
hard-coded to InvalidOid during read.  Currently fix_opfuncids() is used to
fill the funcid for expressions, now I am not sure if it can be used for
fdw_private data (I have tried it, but it was failing, may be it is not at
all
required to fix any funcid for it) or other fields in Foreign Scan.

I have observed that in out functions, we output fdw_private field which
indicates that ideally we should be able to read it.

>
> Robert Haas said:
> > I think it would be worth doing something like this:
> >
> > #define READ_ATTRNUMBER_ARRAY(fldname, len) \
> > token = pg_strtok(); \
> > local_node->fldname = readAttrNumberCols(len);
> >
> > And similarly for READ_OID_ARRAY, READ_BOOL_ARRAY, READ_INT_ARRAY.
> >
> Like this?
>
https://github.com/kaigai/sepgsql/blob/readfuncs/src/backend/nodes/readfuncs.c#L133
>
> I think outfuncs.c also have similar macro to centralize the format of
array.
> Actually, most of boolean array are displayed using booltostr(), however,
only
> _outMergeJoin() uses "%d" format to display boolean as integer.
> It is a bit inconsistent manner.
>

Yes, I have also noticed the same and thought of sending a patch which I
have done just before sending this mail.


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

Re: [HACKERS] Calculage avg. width when operator = is missing

[Tom Lane]

"Shulgin, Oleksandr"  writes:
> On Tue, Sep 22, 2015 at 11:56 PM, Alvaro Herrera 
> wrote:
>> It looks like a bug to me, but I think it might destabilize approved
>> execution plans(*), so it may not be such a great idea to back patch
>> branches that are already released.  I think HEAD + 9.5 is good.
>> 
>> (*) I hear there are even applications where queries and their approved
>> execution plans are kept in a manifest, and plans that deviate from that
>> raise all kinds of alarms.  I have never seen such a thing ...

> Ugh.  Anyway, do you expect any plans to change only due to avg. width
> estimation being different?  Why would that be so?

Certainly, eg it could affect a decision about whether to use a hash join
or hash aggregation through changing the planner's estimate of the
required hashtable size.  We wouldn't be bothering to track that data if
it didn't affect plans.

Personally I think Alvaro's position is unduly conservative: to the extent
that plans change it'd likely be for the better.  But I'm not excited
enough to fight hard about 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] 9.5: Can't connect with PGSSLMODE=require on Windows

[Michael Paquier]

On Wed, Sep 23, 2015 at 2:15 AM, Robert Haas  wrote:

> On Tue, Sep 22, 2015 at 11:23 AM, Andrew Dunstan 
> wrote:
> > "git bisect" is your friend.
>
> Yeah, but finding someone who has a working Windows build environment
> and a lot of time to run this down is my enemy.  We're trying, but if
> anyone else has a clue, that would be much appreciated.
>

That's not cool. I have added this problem in the list of open items for
9.5.
-- 
Michael

Re: [HACKERS] Rework the way multixact truncations work

[Alvaro Herrera]

> @@ -1210,8 +1211,14 @@ restart:;
>   (void) SlruScanDirectory(ctl, SlruScanDirCbDeleteCutoff, );
>  }
>  
> -void
> -SlruDeleteSegment(SlruCtl ctl, char *filename)
> +/*
> + * Delete an individual SLRU segment, identified by the filename.
> + *
> + * NB: This does not touch the SLRU buffers themselves, callers have to 
> ensure
> + * they either can't yet contain anything, or have already been cleaned out.
> + */
> +static void
> +SlruInternalDeleteSegment(SlruCtl ctl, char *filename)
>  {
>   charpath[MAXPGPATH];
>  
> @@ -1222,6 +1229,64 @@ SlruDeleteSegment(SlruCtl ctl, char *filename)
>  }
>  
>  /*
> + * Delete an individual SLRU segment, identified by the segment number.
> + */
> +void
> +SlruDeleteSegment(SlruCtl ctl, int segno)

Is it okay to change the ABI of SlruDeleteSegment?

-- 
Á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] a funnel by any other name

[Simon Riggs]

On 22 September 2015 at 20:34, Robert Haas  wrote:

> On Tue, Sep 22, 2015 at 10:34 AM, Simon Riggs 
> wrote:
> > Robert, thanks for asking. We'll be stuck with these words for some time,
> > user visible via EXPLAIN so this is important.
>
> I agree, thanks for taking an interest.
>
> > The main operations are the 3 mentioned by Nicolas:
> > 1. Send data from many to one - which has subtypes for Unsorted, Sorted
> and
> > Evenly balanced (but unsorted)
> > 2. Send data from one process to many
> > 3. Send data from many to many
> >
> > My preferences for this would be
> > 1. Gather (but not Gather Motion) e.g. Gather, Gather Sorted
> > 2. Scatter (since Broadcast only makes sense in the context of a
> distributed
> > query, it sounds weird for intra-node query)
> > 3. Redistribution - which implies the description of how we spread data
> > across nodes is "Distribution" (or DISTRIBUTED BY)
>
> "Scatter" isn't one of the things that I mentioned in my original
> email.  Not sure where we'd use that, although there might be
> somewhere.


Understood. Thought it best to cover all the phrases we'll use in the
future now in one discussion.


> > For 3 we should definitely use Redistribute, since this is what Teradata
> has
> > been calling it for 30 years, which is where Greenplum got it from.
>
> That's a reasonable option.  We can bikeshed it some more when we get that
> far.


Sure


> > For 1, Gather makes most sense.
>
> Yeah, I'm leaning that way myself.  Amit argued for "Parallel Gather"
> but I think that's overkill.  There can't be a non-parallel gather,
> and long names are a pain.


Agreed

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

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

Re: [HACKERS] unclear about row-level security USING vs. CHECK

[Robert Haas]

On Tue, Sep 22, 2015 at 10:36 PM, Charles Clavadetscher
 wrote:
> Since the policy is defined for ALL commands and no WITH CHECK is specified 
> then the same condition defined in USING takes effect for all commands, i.e. 
> including INSERT.
>
> From the docs 
> (http://www.postgresql.org/docs/9.5/static/sql-createpolicy.html): "Further, 
> for commands which can have both USING and WITH CHECK policies (ALL and 
> UPDATE), if no WITH CHECK policy is defined then the USING policy will be 
> used for both what rows are visible (normal USING case) and which rows will 
> be allowed to be added (WITH CHECK case)."
>
> If you want e.g. to allow users to insert rows without the restriction of 
> being the current_user in column entered_by then you would need separate 
> policies for each command. If you define a policy for INSERT, USING does not 
> make sense. In the thread above there is a similar example to this as well as 
> in the documentation:
>
> http://www.postgresql.org/docs/9.5/static/ddl-rowsecurity.html
>
>> (Btw., what's the meaning of a policy for DELETE?)
>
> In your example it means that users can delete only the rows where entered_by 
> = current_user. A WITH CHECK policy does not make sense in this case.

Gosh, I think it would have been better to have a cleaner separation
of USING and WITH CHECK.  That sounds far too unnecessarily magical.

-- 
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] unclear about row-level security USING vs. CHECK

[Robert Haas]

On Wed, Sep 23, 2015 at 11:05 AM, Stephen Frost  wrote:
>> Gosh, I think it would have been better to have a cleaner separation
>> of USING and WITH CHECK.  That sounds far too unnecessarily magical.
>
> That the USING policy is used if WITH CHECK isn't defined?  That was
> simply done to make policy management simple as in quite a few cases
> only one policy is needed.  If a WITH CHECK was always required then
> you'd be constantly writing:
>
> CREATE POLICY p1 ON t1
> USING (entered_by = current_user)
> WITH CHECK (entered_by = current_user);
>
> With potentially quite lengthy expressions.
>
> I'm not against changing that if people feel strongly about it, but I
> certainly find it extremely handy.
>
> If that wasn't what you were referring to then please clarify as I
> didn't follow.

No, that's what I was talking about.  Maybe it is the most useful
behavior, but it seems to have surprised Peter, and it surprised me,
too.

-- 
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] [PATCH] Refactoring of LWLock tranches

[Robert Haas]

On Tue, Sep 22, 2015 at 5:16 AM, Ildus Kurbangaliev
 wrote:
> Yes, probably.
> I'm going to change API calls as you suggested earlier.
> How you do think the tranches registration after initialization should
> look like?

I don't see any need to change anything there.  The idea there is that
an extension allocates a tranche ID and are responsible for making
sure that every backend that uses that tranche finds out about the ID
that was chosen and registers a matching tranche definition.  How to
do that is the extension's problem.  Maybe eventually we'll provide
some tools to make that easier, but that's separate from the work
we're trying to do here.

-- 
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] [PATCH] Refactoring of LWLock tranches

[Alvaro Herrera]

Robert Haas wrote:
> On Tue, Sep 22, 2015 at 5:16 AM, Ildus Kurbangaliev
>  wrote:
> > Yes, probably.
> > I'm going to change API calls as you suggested earlier.
> > How you do think the tranches registration after initialization should
> > look like?
> 
> I don't see any need to change anything there.  The idea there is that
> an extension allocates a tranche ID and are responsible for making
> sure that every backend that uses that tranche finds out about the ID
> that was chosen and registers a matching tranche definition.  How to
> do that is the extension's problem.  Maybe eventually we'll provide
> some tools to make that easier, but that's separate from the work
> we're trying to do here.

FWIW I had assumed, when you created the tranche stuff, that SLRU users
would all allocate their lwlocks from a tranche provided by slru.c
itself, and the locks would be stored in the slru Ctl struct.  Does that
not work for some reason?

-- 
Á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] Calculage avg. width when operator = is missing

[Alvaro Herrera]

Tom Lane wrote:

> Personally I think Alvaro's position is unduly conservative: to the extent
> that plans change it'd likely be for the better.  But I'm not excited
> enough to fight hard about it.

I don't really care enough.  We have received some complaints about
keeping plans stable, but maybe it's okay.

-- 
Á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] Rework the way multixact truncations work

[Andres Freund]

On 2015-09-23 10:29:09 -0300, Alvaro Herrera wrote:
> >  /*
> > + * Delete an individual SLRU segment, identified by the segment number.
> > + */
> > +void
> > +SlruDeleteSegment(SlruCtl ctl, int segno)
> 
> Is it okay to change the ABI of SlruDeleteSegment?

I think so. Any previous user of the API is going to be currently broken
anyway due to the missing flushing of buffers.

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] hot_standby_feedback default and docs

[Robert Haas]

On Tue, Sep 22, 2015 at 3:35 PM, Peter Eisentraut  wrote:
> On 9/16/15 5:52 PM, Simon Riggs wrote:
>> IMHO the default is the best one at the current time.
>> See recovery_min_apply_delay.
>
> The applications of recovery_min_apply_delay are likely to be varied and
> specific, so there might not be a general answer to this, but wouldn't
> you want hot_standby_feedback on with it?  Because the longer you wait
> on the standby, the more likely it is that the primary will clean stuff
> away.

If min_recovery_apply_delay was set to 1 hour, and if the standby had
hot standby feedback turned on, wouldn't that mean that the master had
to not do any HOT pruning or vacuuming of tuples until they had been
dead for at least an hour?  That seems like it would be bad.

-- 
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] TEXT vs VARCHAR join qual push down diffrence, bug or expected?

[Tom Lane]

Jeevan Chalke  writes:
> On Tue, Sep 22, 2015 at 12:31 AM, Tom Lane  wrote:
>> It strikes me that this function is really going about things the wrong
>> way.  Rather than trying to determine the output collation per se, what
>> we ought to be asking is "does every operator in the proposed expression
>> have an input collation that can be traced to some foreign Var further
>> down in the expression"?

> IIUC, you are saying that collation check for output collation is not
> necessary for all OpExpr/FuncExpr/ArrayRef etc.
> Should we remove code blocks like
> collation = r->resultcollid;
> if (collation == InvalidOid)
> state = FDW_COLLATE_NONE;
> else if (inner_cxt.state == FDW_COLLATE_SAFE &&
>  collation == inner_cxt.collation)
> state = FDW_COLLATE_SAFE;
> else
> state = FDW_COLLATE_UNSAFE;

> and just bubble up the collation and state to the next level?

Removing that entirely would be quite incorrect, because then you'd be
lying to the parent node about what collation your node outputs.

After thinking a bit more about the existing special case for non-foreign
Vars, I wonder if what we should do is change these code blocks to look
like

collation = r->resultcollid;
if (collation == InvalidOid)
state = FDW_COLLATE_NONE;
else if (inner_cxt.state == FDW_COLLATE_SAFE &&
 collation == inner_cxt.collation)
state = FDW_COLLATE_SAFE;
+   else if (collation == DEFAULT_COLLATION_OID)
+   state = FDW_COLLATE_NONE;
else
state = FDW_COLLATE_UNSAFE;

That is, only explicit introduction of a non-default collation causes
a subexpression to get labeled FDW_COLLATE_UNSAFE.  Default collations
would lose out when getting merged with a nondefault collation from a
foreign Var, so they should work all right.

The core point here is that we're going to send the expression to the
remote without any COLLATE clauses, so the remote's parser has to
come to the same conclusions we did about which collation to apply.
We assume that default-collation-throughout will work all right.
Nondefault collations will work as long as they originate from foreign
Vars, because then the remote parser should see the equivalent far-end
collations originating from those Vars --- and those collations win when
combined with default collations.

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] a funnel by any other name

[Simon Riggs]

On 22 September 2015 at 21:14, Alvaro Herrera 
wrote:

> Robert Haas wrote:
> > On Tue, Sep 22, 2015 at 10:34 AM, Simon Riggs 
> wrote:
>
> > > For 1, Gather makes most sense.
> >
> > Yeah, I'm leaning that way myself.  Amit argued for "Parallel Gather"
> > but I think that's overkill.  There can't be a non-parallel gather,
> > and long names are a pain.
>
> "Gather" seems a pretty decent choice to me too, even if we only have a
> single worker (your "1").  I don't think there's much need to
> distinguish 1 from 2, is there?
>

I think so. 1 is Many->1 and the other is 1->Many.

You may wish to do an operation like a parallel merge join.

Parallel Sort -> Scatter -> Parallel Merge

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

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

Re: [HACKERS] unclear about row-level security USING vs. CHECK

[Stephen Frost]

* Robert Haas (robertmh...@gmail.com) wrote:
> On Tue, Sep 22, 2015 at 10:36 PM, Charles Clavadetscher
>  wrote:
> > Since the policy is defined for ALL commands and no WITH CHECK is specified 
> > then the same condition defined in USING takes effect for all commands, 
> > i.e. including INSERT.
> >
> > From the docs 
> > (http://www.postgresql.org/docs/9.5/static/sql-createpolicy.html): 
> > "Further, for commands which can have both USING and WITH CHECK policies 
> > (ALL and UPDATE), if no WITH CHECK policy is defined then the USING policy 
> > will be used for both what rows are visible (normal USING case) and which 
> > rows will be allowed to be added (WITH CHECK case)."
> >
> > If you want e.g. to allow users to insert rows without the restriction of 
> > being the current_user in column entered_by then you would need separate 
> > policies for each command. If you define a policy for INSERT, USING does 
> > not make sense. In the thread above there is a similar example to this as 
> > well as in the documentation:
> >
> > http://www.postgresql.org/docs/9.5/static/ddl-rowsecurity.html
> >
> >> (Btw., what's the meaning of a policy for DELETE?)
> >
> > In your example it means that users can delete only the rows where 
> > entered_by = current_user. A WITH CHECK policy does not make sense in this 
> > case.
> 
> Gosh, I think it would have been better to have a cleaner separation
> of USING and WITH CHECK.  That sounds far too unnecessarily magical.

That the USING policy is used if WITH CHECK isn't defined?  That was
simply done to make policy management simple as in quite a few cases
only one policy is needed.  If a WITH CHECK was always required then
you'd be constantly writing:

CREATE POLICY p1 ON t1
USING (entered_by = current_user)
WITH CHECK (entered_by = current_user);

With potentially quite lengthy expressions.

I'm not against changing that if people feel strongly about it, but I
certainly find it extremely handy.

If that wasn't what you were referring to then please clarify as I
didn't follow.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] TEXT vs VARCHAR join qual push down diffrence, bug or expected?

[Tom Lane]

I wrote:
> After thinking a bit more about the existing special case for non-foreign
> Vars, I wonder if what we should do is change these code blocks to look
> like

> collation = r->resultcollid;
> if (collation == InvalidOid)
> state = FDW_COLLATE_NONE;
> else if (inner_cxt.state == FDW_COLLATE_SAFE &&
>  collation == inner_cxt.collation)
> state = FDW_COLLATE_SAFE;
> + else if (collation == DEFAULT_COLLATION_OID)
> + state = FDW_COLLATE_NONE;
> else
> state = FDW_COLLATE_UNSAFE;

On further thought, maybe it's the special case for non-foreign Vars
that is busted.  That is, non-foreign Vars should do

/* Var belongs to some other table */
if (var->varcollid != InvalidOid &&
var->varcollid != DEFAULT_COLLATION_OID)
return false;

-   /* We can consider that it doesn't set collation */
-   collation = InvalidOid;
-   state = FDW_COLLATE_NONE;
+   collation = var->varcollid;
+   state = OidIsValid(collation) ? FDW_COLLATE_SAFE : 
FDW_COLLATE_NONE;

and likewise for Consts, Params, etc.

This would basically mean clarifying the meaning of the state values as:

FDW_COLLATE_NONE: the expression is of a noncollatable type, period.

FDW_COLLATE_SAFE: the expression has default collation, or a nondefault
collation that is traceable to a foreign Var.

FDW_COLLATE_UNSAFE: the expression has a nondefault collation that is not
traceable to a foreign Var.

Hm ... actually, we probably need *both* types of changes if that's
what we believe the state values mean.

An alternative definition would be that FDW_COLLATE_NONE subsumes the
"collation doesn't trace to a foreign Var, but it's default so we don't
really care" case.  I think the problem we've got is that the
non-foreign-Var code thinks that's what the definition is, but the rest
of the code isn't quite consistent with it.

In any case I think we want to end up with a clearer specification of
what the states mean.

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] unclear about row-level security USING vs. CHECK

[Stephen Frost]

* Robert Haas (robertmh...@gmail.com) wrote:
> On Wed, Sep 23, 2015 at 11:05 AM, Stephen Frost  wrote:
> >> Gosh, I think it would have been better to have a cleaner separation
> >> of USING and WITH CHECK.  That sounds far too unnecessarily magical.
> >
> > That the USING policy is used if WITH CHECK isn't defined?  That was
> > simply done to make policy management simple as in quite a few cases
> > only one policy is needed.  If a WITH CHECK was always required then
> > you'd be constantly writing:
> >
> > CREATE POLICY p1 ON t1
> > USING (entered_by = current_user)
> > WITH CHECK (entered_by = current_user);
> >
> > With potentially quite lengthy expressions.
> >
> > I'm not against changing that if people feel strongly about it, but I
> > certainly find it extremely handy.
> >
> > If that wasn't what you were referring to then please clarify as I
> > didn't follow.
> 
> No, that's what I was talking about.  Maybe it is the most useful
> behavior, but it seems to have surprised Peter, and it surprised me,
> too.

I'm working on a documentation patch with Adam to improve the docs
around this (and other parts as well).  I agree it doesn't come off as
naturally intuitive to everyone (it did to me, but I'm clearly biased
as, I think anyway, it was my idea) and so I'm not sure that's enough.

Is there strong feeling that USING and WITH CHECK should both always be
required when specifying ALL and UPDATE policies?  It's not a difficult
change to make if people want it.

I will mention that on another thread there was discussion about having
WITH CHECK for all policy types as a way to let users control if an
error should be thrown rather than skipping over a row due to lack of
visibility.  In all cases, USING controls visibility and WITH CHECK will
throw an error on a violation and that would remain the case with this
approach.  Now that I think about it, it might be a bit cleaner if
USING and WITH CHECK are always kept independent for that case, but I'm
not sure it's really all that much of a difference.  The USING will
always be applied first and then the WITH CHECK applied to any rows
which remain, which comes across, to me at least (which isn't fair, of
course, but it's what I can comment on) as quite clear to understand.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Calculage avg. width when operator = is missing

[Shulgin, Oleksandr]

On Wed, Sep 23, 2015 at 3:21 PM, Tom Lane  wrote:

> "Shulgin, Oleksandr"  writes:
> > On Tue, Sep 22, 2015 at 11:56 PM, Alvaro Herrera <
> alvhe...@2ndquadrant.com>
> > wrote:
> >> It looks like a bug to me, but I think it might destabilize approved
> >> execution plans(*), so it may not be such a great idea to back patch
> >> branches that are already released.  I think HEAD + 9.5 is good.
> >>
> >> (*) I hear there are even applications where queries and their approved
> >> execution plans are kept in a manifest, and plans that deviate from that
> >> raise all kinds of alarms.  I have never seen such a thing ...
>
> > Ugh.  Anyway, do you expect any plans to change only due to avg. width
> > estimation being different?  Why would that be so?
>
> Certainly, eg it could affect a decision about whether to use a hash join
> or hash aggregation through changing the planner's estimate of the
> required hashtable size.  We wouldn't be bothering to track that data if
> it didn't affect plans.
>
> Personally I think Alvaro's position is unduly conservative: to the extent
> that plans change it'd likely be for the better.  But I'm not excited
> enough to fight hard about it.
>

Yeah, I can see now, as I was studying the hash node code today intensively
for an unrelated reason.

I also believe that given that we are going to have more accurate stats,
the plan changes in this case hopefully are a good thing.

--
Alex

Re: [HACKERS] Inconsistency in Output function of MergeJoin

[Robert Haas]

On Wed, Sep 23, 2015 at 5:56 AM, Amit Kapila  wrote:
> While working on read functions for plan nodes (required for
> parallelism), it has been observed [1] by KaiGai and separately
> by me that function _outMergeJoin(), appends boolean in a
> slightly different way as compare to other out functions like
> _outSort().  Is there a reason of doing so which is is not apparent
> from code or comments?
>
> Attached patch makes _outMergeJoin() consistent with other _out
> functions which appends boolean to string.

Seems right to me.  I'll go commit this.

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

[Stephen Frost]

Zhaomo,

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.

* Zhaomo Yang (zmp...@gmail.com) wrote:
> > It'd be great if others who are interested can help define the grammar
> > changes necessary
> > and perhaps even help with the code aspect of it.
> 
> I'd like to help on both. Can you elaborate a little bit more, especially
> on the code aspect?

There's a fair bit of information available regarding how to contribute
code on the wiki; here's a few links:

https://wiki.postgresql.org/wiki/Development_information
https://wiki.postgresql.org/wiki/Developer_FAQ
https://wiki.postgresql.org/wiki/Submitting_a_Patch

Regarding this, specifically, we'd need to first decide on what the
syntax/grammar should be.  Right now we have, at a basic level:

CREATE POLICY  ON  USING ();

To define restrictive policies we'd need a new bit of syntax which says
"this policy should be restrictive instead of permissive."  One approach
to that would be:

CREATE RESTRICTIVE POLICY ...

but my initial hunch is that we'd rather have something like:

CREATE POLICY  ON  RESTRICTIVE USING ( > I don't buy that argument.
> 
> It is agreed that blind updates and deletes with RETURNING clause are
> dangerous. It is quite similar here.

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.

> Instead of using
>BEGIN
>UPDATE-or-DELETE-with-RETURNING
>ROLLBACK
> as a substitute for SELECT, a malicious user can do a binary search with
> some trick like divide-by-zero
> to figure out rows he is not allowed to access. Of course, this is not as
> serious as RETURNING, but it is still quite convenient for attackers.

This is true of the current GRANT system.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] TEXT vs VARCHAR join qual push down diffrence, bug or expected?

[Tom Lane]

I wrote:
> Hm ... actually, we probably need *both* types of changes if that's
> what we believe the state values mean.

After a bit more thinking and experimentation, I propose the attached
patch.

regards, tom lane

diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index 81cb2b4..f64482c 100644
*** a/contrib/postgres_fdw/deparse.c
--- b/contrib/postgres_fdw/deparse.c
***
*** 17,27 
   * We do not consider that it is ever safe to send COLLATE expressions to
   * the remote server: it might not have the same collation names we do.
   * (Later we might consider it safe to send COLLATE "C", but even that would
!  * fail on old remote servers.)  An expression is considered safe to send only
!  * if all collations used in it are traceable to Var(s) of the foreign table.
!  * That implies that if the remote server gets a different answer than we do,
!  * the foreign table's columns are not marked with collations that match the
!  * remote table's columns, which we can consider to be user error.
   *
   * Portions Copyright (c) 2012-2015, PostgreSQL Global Development Group
   *
--- 17,28 
   * We do not consider that it is ever safe to send COLLATE expressions to
   * the remote server: it might not have the same collation names we do.
   * (Later we might consider it safe to send COLLATE "C", but even that would
!  * fail on old remote servers.)  An expression is considered safe to send
!  * only if all operator/function input collations used in it are traceable to
!  * Var(s) of the foreign table.  That implies that if the remote server gets
!  * a different answer than we do, the foreign table's columns are not marked
!  * with collations that match the remote table's columns, which we can
!  * consider to be user error.
   *
   * Portions Copyright (c) 2012-2015, PostgreSQL Global Development Group
   *
*** typedef struct foreign_glob_cxt
*** 69,77 
   */
  typedef enum
  {
! 	FDW_COLLATE_NONE,			/* expression is of a noncollatable type */
  	FDW_COLLATE_SAFE,			/* collation derives from a foreign Var */
! 	FDW_COLLATE_UNSAFE			/* collation derives from something else */
  } FDWCollateState;
  
  typedef struct foreign_loc_cxt
--- 70,81 
   */
  typedef enum
  {
! 	FDW_COLLATE_NONE,			/* expression is of a noncollatable type, or
!  * it has default collation that is not
!  * traceable to a foreign Var */
  	FDW_COLLATE_SAFE,			/* collation derives from a foreign Var */
! 	FDW_COLLATE_UNSAFE			/* collation is non-default and derives from
!  * something other than a foreign Var */
  } FDWCollateState;
  
  typedef struct foreign_loc_cxt
*** foreign_expr_walker(Node *node,
*** 272,283 
  else
  {
  	/* Var belongs to some other table */
! 	if (var->varcollid != InvalidOid &&
! 		var->varcollid != DEFAULT_COLLATION_OID)
  		return false;
! 
! 	/* We can consider that it doesn't set collation */
! 	collation = InvalidOid;
  	state = FDW_COLLATE_NONE;
  }
  			}
--- 276,286 
  else
  {
  	/* Var belongs to some other table */
! 	collation = var->varcollid;
! 	if (collation != InvalidOid &&
! 		collation != DEFAULT_COLLATION_OID)
  		return false;
! 	/* For either allowed collation, the state is NONE */
  	state = FDW_COLLATE_NONE;
  }
  			}
*** foreign_expr_walker(Node *node,
*** 291,302 
   * non-builtin type, or it reflects folding of a CollateExpr;
   * either way, it's unsafe to send to the remote.
   */
! if (c->constcollid != InvalidOid &&
! 	c->constcollid != DEFAULT_COLLATION_OID)
  	return false;
! 
! /* Otherwise, we can consider that it doesn't set collation */
! collation = InvalidOid;
  state = FDW_COLLATE_NONE;
  			}
  			break;
--- 294,304 
   * non-builtin type, or it reflects folding of a CollateExpr;
   * either way, it's unsafe to send to the remote.
   */
! collation = c->constcollid;
! if (collation != InvalidOid &&
! 	collation != DEFAULT_COLLATION_OID)
  	return false;
! /* For either allowed collation, the state is NONE */
  state = FDW_COLLATE_NONE;
  			}
  			break;
*** foreign_expr_walker(Node *node,
*** 307,317 
  /*
   * Collation handling is same as for Consts.
   */
! if (p->paramcollid != InvalidOid &&
! 	p->paramcollid != DEFAULT_COLLATION_OID)
  	return false;
- 
- collation = InvalidOid;
  state = FDW_COLLATE_NONE;
  			}
  			break;
--- 309,318 
  /*
   * Collation handling is same as for Consts.
   */
! collation = p->paramcollid;
! if (collation != InvalidOid &&
! 	collation != DEFAULT_COLLATION_OID)
  	return false;
  state = FDW_COLLATE_NONE;
  			}
  			break;
*** foreign_expr_walker(Node *node,
*** 348,353 
--- 349,356 
  else if (inner_cxt.state == 

Re: [HACKERS] Parallel Seq Scan

[Robert Haas]

On Wed, Sep 23, 2015 at 3:22 AM, Amit Kapila  wrote:
>> Instead of passing the Plan down by casting, how about passing
>> _node->plan?  And similarly for scans and joins.
> Changed as per suggestion.

The point of this change was to make it so that we wouldn't need the
casts any more.  You changed it so we didn't, but then didn't actually
get rid of them.  I did that, tweaked a comment, and committed this.

-- 
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] unclear about row-level security USING vs. CHECK

[Robert Haas]

On Wed, Sep 23, 2015 at 12:01 PM, Stephen Frost  wrote:
> * Robert Haas (robertmh...@gmail.com) wrote:
>> On Wed, Sep 23, 2015 at 11:24 AM, Stephen Frost  wrote:
>> > I'm working on a documentation patch with Adam to improve the docs
>> > around this (and other parts as well).  I agree it doesn't come off as
>> > naturally intuitive to everyone (it did to me, but I'm clearly biased
>> > as, I think anyway, it was my idea) and so I'm not sure that's enough.
>> >
>> > Is there strong feeling that USING and WITH CHECK should both always be
>> > required when specifying ALL and UPDATE policies?  It's not a difficult
>> > change to make if people want it.
>>
>> My expectation would have been:
>>
>> If you specify USING, you can see only those rows, but you can give
>> rows away freely.  If you don't want to allow giving rows away under
>> any circumstances, then specify the same expression for USING and WITH
>> CHECK.
>
> Having an implicit 'true' for WITH CHECK would be very much against what
> I would ever expect.  If anything, I'd think we would have an implicit
> 'false' there or simply not allow it to ever be unspecified.

Huh?  If you had an implicit false, wouldn't that prevent updating or
deleting any rows at all?

>> I don't really get that.  If you could make skipping a row trigger an
>> error, then that would create a bunch of covert channel attacks.
>
> Apparently I didn't explain it correctly.  Skipping a row doesn't
> trigger an error.  An example would perhaps help here to clarify:
>
> CREATE POLICY p1 ON t1 FOR DELETE
> USING (true)
> WITH CHECK (inserted_by = current_user);
>
> What would happen above is that, in a DELETE case, you're allowed to
> *try* and delete any record in the table, but if you try to delete a
> record which isn't yours, we throw an error.  Currently the only option,
> if you want to prevent users from deleteing records which are not
> theirs, is to have:
>
> CREATE POLICY p1 ON t1 FOR DELETE
> USING (inserted_by = current_user)
>
> Which certainly has the effect that you can only delete records you own,
> but I can see use-cases where you'd like to know that someone tried to
> delete a record which isn't their own and that isn't something you can
> get directly today.

Well, you can use a trigger, I think.  But the point is that right
now, if you try to delete a record that you don't own, it just says
DELETE 0.  Maybe there was a record there that you can't see, and
maybe there wasn't.

-- 
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] unclear about row-level security USING vs. CHECK

[Peter Eisentraut]

On 9/23/15 11:05 AM, Stephen Frost wrote:
> That the USING policy is used if WITH CHECK isn't defined?  That was
> simply done to make policy management simple as in quite a few cases
> only one policy is needed.  If a WITH CHECK was always required then
> you'd be constantly writing:
> 
> CREATE POLICY p1 ON t1
> USING (entered_by = current_user)
> WITH CHECK (entered_by = current_user);
> 
> With potentially quite lengthy expressions.

That might be reasonable, but the documentation is completely wrong
about that.

That said, why even have USING and CHECK as separate clauses?  Can't you
just create different policies if you want them different?

Hypothetical example:

CREATE POLICY p1 ON t1 FOR SELECT CHECK (extract(year from entered_on) =
extract(year from current_timestamp));
CREATE POLICY p2 ON t2 FOR INSERT, UPDATE, DELETE CHECK (entered_by =
current_user);


-- 
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] unclear about row-level security USING vs. CHECK

[Stephen Frost]

* Robert Haas (robertmh...@gmail.com) wrote:
> On Wed, Sep 23, 2015 at 11:24 AM, Stephen Frost  wrote:
> > I'm working on a documentation patch with Adam to improve the docs
> > around this (and other parts as well).  I agree it doesn't come off as
> > naturally intuitive to everyone (it did to me, but I'm clearly biased
> > as, I think anyway, it was my idea) and so I'm not sure that's enough.
> >
> > Is there strong feeling that USING and WITH CHECK should both always be
> > required when specifying ALL and UPDATE policies?  It's not a difficult
> > change to make if people want it.
> 
> My expectation would have been:
> 
> If you specify USING, you can see only those rows, but you can give
> rows away freely.  If you don't want to allow giving rows away under
> any circumstances, then specify the same expression for USING and WITH
> CHECK.

Having an implicit 'true' for WITH CHECK would be very much against what
I would ever expect.  If anything, I'd think we would have an implicit
'false' there or simply not allow it to ever be unspecified.

> > I will mention that on another thread there was discussion about having
> > WITH CHECK for all policy types as a way to let users control if an
> > error should be thrown rather than skipping over a row due to lack of
> > visibility.  In all cases, USING controls visibility and WITH CHECK will
> > throw an error on a violation and that would remain the case with this
> > approach.  Now that I think about it, it might be a bit cleaner if
> > USING and WITH CHECK are always kept independent for that case, but I'm
> > not sure it's really all that much of a difference.  The USING will
> > always be applied first and then the WITH CHECK applied to any rows
> > which remain, which comes across, to me at least (which isn't fair, of
> > course, but it's what I can comment on) as quite clear to understand.
> 
> I don't really get that.  If you could make skipping a row trigger an
> error, then that would create a bunch of covert channel attacks.

Apparently I didn't explain it correctly.  Skipping a row doesn't
trigger an error.  An example would perhaps help here to clarify:

CREATE POLICY p1 ON t1 FOR DELETE
USING (true)
WITH CHECK (inserted_by = current_user); 

What would happen above is that, in a DELETE case, you're allowed to
*try* and delete any record in the table, but if you try to delete a
record which isn't yours, we throw an error.  Currently the only option,
if you want to prevent users from deleteing records which are not
theirs, is to have:

CREATE POLICY p1 ON t1 FOR DELETE
USING (inserted_by = current_user)

Which certainly has the effect that you can only delete records you own,
but I can see use-cases where you'd like to know that someone tried to
delete a record which isn't their own and that isn't something you can
get directly today.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] hot_standby_feedback default and docs

[Robert Haas]

On Wed, Sep 23, 2015 at 12:26 PM, Peter Eisentraut  wrote:
> On 9/23/15 10:44 AM, Robert Haas wrote:
>> On Tue, Sep 22, 2015 at 3:35 PM, Peter Eisentraut  wrote:
>>> On 9/16/15 5:52 PM, Simon Riggs wrote:
 IMHO the default is the best one at the current time.
 See recovery_min_apply_delay.
>>>
>>> The applications of recovery_min_apply_delay are likely to be varied and
>>> specific, so there might not be a general answer to this, but wouldn't
>>> you want hot_standby_feedback on with it?  Because the longer you wait
>>> on the standby, the more likely it is that the primary will clean stuff
>>> away.
>>
>> If min_recovery_apply_delay was set to 1 hour, and if the standby had
>> hot standby feedback turned on, wouldn't that mean that the master had
>> to not do any HOT pruning or vacuuming of tuples until they had been
>> dead for at least an hour?  That seems like it would be bad.
>
> I suppose that's what would happen, and it might be bad, but the
> alternative is that the primary might vacuum away everything and you
> won't be able to make much use of the delayed standby.
>
> I'm not clear on the intended usage scenarios for
> recovery_min_apply_delay, but the ramifications don't appear to be well
> explained anywhere.

Well, the alternative to enabling hot standby feedback is that the
query might get cancelled.  But it might also NOT get cancelled.  I
mean, if recovery_min_apply_delay is set to an hour, and the query
runs for a minute, you're only going to get a cancel if some data that
is needed got pruned during the correponding minute an hour earlier on
the master.  And even then you can avoid a cancel by setting
max.*standby_delay to at least 61 seconds, which is more likely to be
acceptable for a standby that intentionally lags the master.  But even
if you don't do that, it's not as if every query you issue is going to
get cancelled.

-- 
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] unclear about row-level security USING vs. CHECK

[Robert Haas]

On Wed, Sep 23, 2015 at 11:24 AM, Stephen Frost  wrote:
> I'm working on a documentation patch with Adam to improve the docs
> around this (and other parts as well).  I agree it doesn't come off as
> naturally intuitive to everyone (it did to me, but I'm clearly biased
> as, I think anyway, it was my idea) and so I'm not sure that's enough.
>
> Is there strong feeling that USING and WITH CHECK should both always be
> required when specifying ALL and UPDATE policies?  It's not a difficult
> change to make if people want it.

My expectation would have been:

If you specify USING, you can see only those rows, but you can give
rows away freely.  If you don't want to allow giving rows away under
any circumstances, then specify the same expression for USING and WITH
CHECK.

> I will mention that on another thread there was discussion about having
> WITH CHECK for all policy types as a way to let users control if an
> error should be thrown rather than skipping over a row due to lack of
> visibility.  In all cases, USING controls visibility and WITH CHECK will
> throw an error on a violation and that would remain the case with this
> approach.  Now that I think about it, it might be a bit cleaner if
> USING and WITH CHECK are always kept independent for that case, but I'm
> not sure it's really all that much of a difference.  The USING will
> always be applied first and then the WITH CHECK applied to any rows
> which remain, which comes across, to me at least (which isn't fair, of
> course, but it's what I can comment on) as quite clear to understand.

I don't really get that.  If you could make skipping a row trigger an
error, then that would create a bunch of covert channel attacks.
Granted we will have some of those anyway, but I see no reason to
manufacture more.  You can set row_security=off if you want an attempt
to query a table with RLS enabled to fail outright, but you're not
entitled to know whether a particular query skipped an invisible row.

-- 
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] [PATCH] Refactoring of LWLock tranches

[Robert Haas]

On Wed, Sep 23, 2015 at 11:22 AM, Alvaro Herrera
 wrote:
> Robert Haas wrote:
>> On Tue, Sep 22, 2015 at 5:16 AM, Ildus Kurbangaliev
>>  wrote:
>> > Yes, probably.
>> > I'm going to change API calls as you suggested earlier.
>> > How you do think the tranches registration after initialization should
>> > look like?
>>
>> I don't see any need to change anything there.  The idea there is that
>> an extension allocates a tranche ID and are responsible for making
>> sure that every backend that uses that tranche finds out about the ID
>> that was chosen and registers a matching tranche definition.  How to
>> do that is the extension's problem.  Maybe eventually we'll provide
>> some tools to make that easier, but that's separate from the work
>> we're trying to do here.
>
> FWIW I had assumed, when you created the tranche stuff, that SLRU users
> would all allocate their lwlocks from a tranche provided by slru.c
> itself, and the locks would be stored in the slru Ctl struct.  Does that
> not work for some reason?

I think that should work and that it's a good idea.  I think it's just
a case of nobody having done the work.

-- 
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] Parallel Seq Scan

[Robert Haas]

On Tue, Sep 22, 2015 at 3:14 AM, Haribabu Kommi
 wrote:
> copy_user_generic_string system call is because of file read operations.
> In my test, I gave the shared_buffers as 12GB with the table size of 18GB.

OK, cool.  So that's actually good: all that work would have to be
done either way, and parallelism lets several CPUs work on it at once.

> The _spin_lock calls are from the signals that are generated by the workers.
> With the increase of tuple queue size, there is a change in kernel system
> calls usage.

And this part is not so good: that's additional work created by
parallelism that wouldn't have to be done if we weren't in parallel
mode.  Of course, it's impossible to eliminate that, but we should try
to reduce it.

> - From the above performance readings, increase of tuple queue size
> gets benefited with lesser
>   number of workers compared to higher number of workers.

That makes sense to me, because there's a separate queue for each
worker.  If we have more workers, then the total amount of queue space
available rises in proportion to the number of workers available.

> Workers are getting started irrespective of the system load. If user
> configures 16 workers, but
> because of a sudden increase in the system load, there are only 2 or 3
> cpu's are only IDLE.
> In this case, if any parallel seq scan eligible query is executed, the
> backend may start 16 workers
> thus it can lead to overall increase of system usage and may decrease
> the performance of the
> other backend sessions?

Yep, that could happen.  It's something we should work on, but the
first version isn't going to try to be that smart.  It's similar to
the problem we already have with work_mem, and I want to work on it,
but we need to get this working first.

> If the query have two parallel seq scan plan nodes and how the workers
> will be distributed across
> the two nodes? Currently parallel_seqscan_degree is used per plan
> node, even if we change that
> to per query, I think we need a worker distribution logic, instead of
> using all workers by a single
> plan node.

Yes, we need that, too.  Again, at some point.

> Select with a limit clause is having a performance drawback with
> parallel seq scan in some scenarios,
> because of very less selectivity compared to seq scan, it should be
> better if we document it. Users
> can take necessary actions based on that for the queries with limit clause.

This is something I want to think further about in the near future.
We don't have a great plan for shutting down workers when no further
tuples are needed because, for example, an upper node has filled a
limit.  That makes using parallel query in contexts like Limit and
InitPlan significantly more costly than you might expect.  Perhaps we
should avoid parallel plans altogether in those contexts, or maybe
there is some other approach that can work.  I haven't figured it out
yet.

-- 
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] hot_standby_feedback default and docs

[Peter Eisentraut]

On 9/23/15 10:44 AM, Robert Haas wrote:
> On Tue, Sep 22, 2015 at 3:35 PM, Peter Eisentraut  wrote:
>> On 9/16/15 5:52 PM, Simon Riggs wrote:
>>> IMHO the default is the best one at the current time.
>>> See recovery_min_apply_delay.
>>
>> The applications of recovery_min_apply_delay are likely to be varied and
>> specific, so there might not be a general answer to this, but wouldn't
>> you want hot_standby_feedback on with it?  Because the longer you wait
>> on the standby, the more likely it is that the primary will clean stuff
>> away.
> 
> If min_recovery_apply_delay was set to 1 hour, and if the standby had
> hot standby feedback turned on, wouldn't that mean that the master had
> to not do any HOT pruning or vacuuming of tuples until they had been
> dead for at least an hour?  That seems like it would be bad.

I suppose that's what would happen, and it might be bad, but the
alternative is that the primary might vacuum away everything and you
won't be able to make much use of the delayed standby.

I'm not clear on the intended usage scenarios for
recovery_min_apply_delay, but the ramifications don't appear to be well
explained anywhere.



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

[HACKERS] Postgres - BDR issue

[Rahul Goel]

Hi

I am facing the below issue in setting up BDR:

I have 2 nodes (For simplicity, I will refer them as node 1 & node 2). BDR
group was created from Node 1. When a new postgres node (i.e. node 2) joins
the group, then the node_status in bdr.bdr_nodes table of new node (i.e.
node 2) show 'r', but node_status remains 'i' on the upstream master (i.e.
node 1). I could see conflict has happened in bdr.bdr_nodes table, and node
1 is unable to update the status of node 2, but couldn't able to find the
solution

*Node 1 (BDR group was created from this node):*
(Masked DB Name, and password)

*psql -U postgres -d xyz -c "select * from bdr.bdr_nodes;"*





*node_sysid  | node_timeline | node_dboid | node_status |   node_name
|  node_local_dsn  |  node_init_from_dsn
 
-+---++-+---+-+--
6197340597374984280
| 1 |  16385 | r   | 10.42.157.193 | port=5432
dbname=xyzdb host=10.42.157.193 user=postgres password=password |
 6197344706786291803 | 1 |  12156 | i   |
10.42.99.96   | port=5432 dbname=xyzdb host=10.42.99.96 user=postgres
password=password   | port=5432 dbname=xyzdb host=10.42.157.193
user=postgres password=password(2 rows)*

Logs


















*< 2015-09-22 14:16:36.244 UTC >STATEMENT:  CREATE SCHEMA public;<
2015-09-22 14:16:42.615 UTC >LOG:  registering background worker "bdr db:
xyzdb"< 2015-09-22 14:16:42.615 UTC >LOG:  starting background worker
process "bdr db: xyzdb"< 2015-09-22 14:23:16.498 UTC >LOG:  logical
decoding found consistent point at 0/879E980< 2015-09-22 14:23:16.498 UTC
>DETAIL:  There are no running transactions.< 2015-09-22 14:23:16.498 UTC
>LOG:  exported logical decoding snapshot: "0511-1" with 0 transaction
IDs< 2015-09-22 14:23:25.284 UTC >LOG:  starting logical decoding for slot
"bdr_16385_6197344706786291803_1_12156__"< 2015-09-22 14:23:25.284 UTC
>DETAIL:  streaming transactions committing after 0/879E9B8, reading WAL
from 0/879E980< 2015-09-22 14:23:25.284 UTC >LOG:  logical decoding found
consistent point at 0/879E980< 2015-09-22 14:23:25.284 UTC >DETAIL:  There
are no running transactions.< 2015-09-22 14:23:25.294 UTC >LOG:  could not
receive data from client: Connection reset by peer< 2015-09-22 14:23:25.294
UTC >LOG:  unexpected EOF on standby connection< 2015-09-22 14:23:26.299
UTC >LOG:  registering background worker "bdr
(6197340597374984280,1,16385,)->bdr (6197344706786291803,1,"< 2015-09-22
14:23:26.299 UTC >LOG:  starting background worker process "bdr
(6197340597374984280,1,16385,)->bdr (6197344706786291803,1,"< 2015-09-22
14:23:26.311 UTC >LOG:  starting logical decoding for slot
"bdr_16385_6197344706786291803_1_12156__"< 2015-09-22 14:23:26.311 UTC
>DETAIL:  streaming transactions committing after 0/87B0998, reading WAL
from 0/879E9B8< 2015-09-22 14:23:26.313 UTC >LOG:  logical decoding found
consistent point at 0/879E9B8< 2015-09-22 14:23:26.313 UTC >DETAIL:
 Logical decoding will begin using saved snapshot.< 2015-09-22 14:23:26.539
UTC >LOG:  CONFLICT: remote UPDATE on relation bdr.bdr_nodes originating at
node 6197344706786291803:1:12156 at ts 2015-09-22 14:23:21.776464+00; row
was previously updated at node 0:0. Resolution:
last_update_wins_keep_local; PKEY: node_sysid[text]:6197344706786291803
node_timeline[oid]:1 node_dboid[oid]:12156 node_status[char]:i
node_name[text]:10.42.99.96 node_local_dsn[text]:port=5432 dbname=xyzdb
host=10.42.99.96 user=postgres password=password
node_init_from_dsn[text]:port=5432 dbname=xyzdb host=10.42.157.193
user=postgres password=password*


*Node 2 (check the status of node here. It's ready but in node 1 it is
initializing)*
(Masked DB Name, and password)

[root@3c8668f9183c /]# psql -U postgres -d hubub -c "select * from
bdr.bdr_nodes;"

 node_sysid  | node_timeline | node_dboid | node_status |
node_name   | node_local_dsn |  node_init_from_dsn

-+---++-+---+-+--


6197340597374984280 | 1 |  16385 | r   |
10.42.157.193 | port=5432 dbname=hubub host=10.42.157.193 user=postgres
password=qsV6hKyW94 |
6197344706786291803 | 1 |  12156 | r   |
10.42.99.96   | port=5432 dbname=hubub host=10.42.99.96 user=postgres
password=qsV6hKyW94   | port=5432 dbname=hubub host=10.42.157.193
user=postgres password=qsV6hKyW94
(2 rows)

Logs
< 2015-09-22 14:23:11.824 UTC >LOG:  registering background worker "bdr db:
xyzdb"
< 2015-09-22 14:23:11.824 UTC >LOG:  starting background worker process
"bdr db: xyzdb"
< 2015-09-22 14:23:11.875 UTC >LOG:  Creating replica with:
/usr/pgsql-9.4/bin/bdr_initial_load --snapshot 0511-1 --source
"port=5432 dbname=xyzdb host=10.42.157.193 user=postgres password=password"

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

[Kam Lasater]

Hello,

Last night I heard that Postgres had no issue/bug tracker. At first I
thought the guy was trolling me. Seriously, how could this be. Certainly a
mature open source project that is the database for a measurable percentage
of the internet would have an issue tracker.

Sadly, I was not being trolled. I'm new around here so I will keep the
preaching to a minimum and cut right to the chase...

___It is time for an issue tracker___

Consider it a sign of success.  Y'all have done a GREAT job! Searching the
archives I see that this has come up before. This project is mature enough
that it has graduated to needing the support of an issue tracker.

At this point not having one is borderline negligent. I'd suggest: Github
Issues, Pivotal Tracker or Redmine (probably in that order). There are tens
to hundreds of other great ones out there, I'm sure one of them would also
work.

Hopefully this feedback from a community member is helpful/useful, that was
my goal in writing in, I trust my intent comes through in this email. And
thanks for an awesome product :)

Cheers.
-Kam Lasater
@seekayel

Re: [HACKERS] Rework the way multixact truncations work

[Andres Freund]

On 2015-09-23 15:03:05 -0300, Alvaro Herrera wrote:
> The comment on top of TrimMultiXact states that "no locks are needed
> here", but then goes on to grab a few locks.

Hm. Yea. Although that was the case before.

> It's a bit odd that SetMultiXactIdLimit has the "finishedStartup" test
> so low.  Why bother setting all those local variables only to bail
> out?

Hm. Doesn't seem to matter much to me, but I can change it.

> In MultiXactAdvanceOldest, the test for sawTruncationinCkptCycle seems
> reversed?
>   if (!MultiXactState->sawTruncationInCkptCycle)
> surely we should be doing truncation if it's set?

No, that's correct. If there was a checkpoint cycle where oldestMulti
advanced without seing a truncation record we need to perform a legacy
truncation.

> Honestly, I wonder whether this message
>   ereport(LOG,
>   (errmsg("performing legacy multixact 
> truncation"),
>errdetail("Legacy truncations are 
> sometimes performed when replaying WAL from an older primary."),
>errhint("Upgrade the primary, it is 
> susceptible to data corruption.")));
> shouldn't rather be a PANIC.  (The main reason not to, I think, is that
> once you see this, there is no way to put the standby in a working state
> without recloning).

Huh? The behaviour in that case is still better than what we have in
9.3+ today (not delayed till the restartpoint). Don't see why that
should be a panic. That'd imo make it pretty much impossible to upgrade
a pair of primary/master where you normally upgrade the standby first?

This is all moot given Robert's objection to backpatching this to
9.3/4.

> If the find_multixact_start(oldestMulti) call in TruncateMultiXact
> fails, what recourse does the user have?  I wonder if the elog() should
> be a FATAL instead of just LOG.  It's not like it would work on a
> subsequent run, is it?

It currently only LOGs, I don't want to change that. The cases where we
currently know it's possible to hit this, it should be fixed by the next
set of emergency autovacuums (which we trigger).

Thanks for the look,

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] Rework the way multixact truncations work

[Alvaro Herrera]

Andres Freund wrote:
> On 2015-09-23 15:03:05 -0300, Alvaro Herrera wrote:

> > Honestly, I wonder whether this message
> > ereport(LOG,
> > (errmsg("performing legacy multixact 
> > truncation"),
> >  errdetail("Legacy truncations are 
> > sometimes performed when replaying WAL from an older primary."),
> >  errhint("Upgrade the primary, it is 
> > susceptible to data corruption.")));
> > shouldn't rather be a PANIC.  (The main reason not to, I think, is that
> > once you see this, there is no way to put the standby in a working state
> > without recloning).
> 
> Huh? The behaviour in that case is still better than what we have in
> 9.3+ today (not delayed till the restartpoint). Don't see why that
> should be a panic. That'd imo make it pretty much impossible to upgrade
> a pair of primary/master where you normally upgrade the standby first?
> 
> This is all moot given Robert's objection to backpatching this to
> 9.3/4.

I think we need to make a decision here.  Is this a terribly serious
bug/misdesign that needs addressing?  If so, we need to backpatch.  If
not, then by all means lets leave it alone.  I don't think it is a good
idea to leave it open if we think it's serious, which is what I think is
happening.

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

[Alvaro Herrera]

Kam Lasater wrote:

> I'd suggest: Github Issues, Pivotal Tracker or Redmine (probably in
> that order). There are tens to hundreds of other great ones out there,
> I'm sure one of them would also work.

If you install debbugs and feed it from our lists, maybe enough of us
would jump into the bandwagon enough to get it off the ground.  I'm
unsure that it would work to maintain something that's too removed from
the mailing lists.

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

[Stephen Frost]

* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> Kam Lasater wrote:
> 
> > I'd suggest: Github Issues, Pivotal Tracker or Redmine (probably in
> > that order). There are tens to hundreds of other great ones out there,
> > I'm sure one of them would also work.
> 
> If you install debbugs and feed it from our lists, maybe enough of us
> would jump into the bandwagon enough to get it off the ground.  I'm
> unsure that it would work to maintain something that's too removed from
> the mailing lists.

The difficulty with this is that someone has to actually offer up to
maintain it.  Bug trackers do not maintain themselves.

Personally, I do like debbugs and it would likely be the least offensive
approch to those who ike the current mailing list.  I don't know offhand
how much effort it would be to set up, perhaps I'll ask around.

Further, the distributions which most of our users use do have their own
bug trackers (Debian, Ubuntu, RedHat) which include tracking bugs in
PostgreSQL.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] unclear about row-level security USING vs. CHECK

[Robert Haas]

On Wed, Sep 23, 2015 at 2:39 PM, Stephen Frost  wrote:
> * Robert Haas (robertmh...@gmail.com) wrote:
>> On Wed, Sep 23, 2015 at 12:01 PM, Stephen Frost  wrote:
>> > * Robert Haas (robertmh...@gmail.com) wrote:
>> >> My expectation would have been:
>> >>
>> >> If you specify USING, you can see only those rows, but you can give
>> >> rows away freely.  If you don't want to allow giving rows away under
>> >> any circumstances, then specify the same expression for USING and WITH
>> >> CHECK.
>> >
>> > Having an implicit 'true' for WITH CHECK would be very much against what
>> > I would ever expect.  If anything, I'd think we would have an implicit
>> > 'false' there or simply not allow it to ever be unspecified.
>>
>> Huh?  If you had an implicit false, wouldn't that prevent updating or
>> deleting any rows at all?
>
> Right, just the same as how, if RLS is enabled and no explicit policies
> are provided, non-owners can't see the rows or insert/update/delete
> anything in the table.  The same is true for the GRANT system, where
> there are no permissions granted by default.  I view the lack of an
> explicit definition of a WITH CHECK clause to be the same, excepting the
> simple case where it's the same as USING.

Hmm, interesting.  I guess that's a defensible position, but I still
think that having them default to be the same thing implicitly is
kinda weird.  I'll defer to whatever the consensus, is, though.

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


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

[HACKERS] clearing opfuncid vs. parallel query

[Robert Haas]

readfuncs.c deliberately ignores any opfuncid read in for an OpExpr,
DistinctExpr, NullIfExpr, or ScalarArrayOpExpr, instead resetting the
value in the newly-created node to InvalidOid.  This is because it
assumes we're reading the node from a pg_node_tree column in some
system catalog, and if in the future we wanted to allow an ALTER
OPERATOR command to change the pg_proc mapping, then the opfuncid
could change.  We'd want to look it up again rather than using the
previous value.

As previously discussed, parallel query will use outfuncs/readfuncs to
copy the Plan to be executed by a parallel worker to that worker.
That plan may contain expressions, and the round-trip through
outfuncs/readfuncs will lose their opfuncids.  In this case, that's
pretty annoying, if not outright wrong.  It's annoying because it
means that, after the worker reads the plan tree, it's got to iterate
over the whole thing and repeat the lookups of all the opfuncid
fields.  This turns out not to be straightforward to code, because we
don't have a generic plan tree walker, and even if we did, you still
need knowledge of which plan nodes have expressions inside which
fields, and we don't have a generic facility for that either: it's
there inside e.g. set_plan_refs, but not in a form other code can use.
Moreover, if we ever did have an ALTER OPERATOR command that could
change the pg_proc mapping, this would go from annoying to outright
broken, because it would be real bad if the worker and the leader came
to different conclusions about what opfuncid to use.  Maybe we could
add heavyweight locking to prevent that, but that would be expensive
and we surely don't have it today.

So I think we should abandon the approach Amit took, namely trying to
reset all of the opfuncids.  Instead, I think we should provide a
method of not throwing them out in the first place.  The attached
patch does by adding an "int flags" field to the relevant read
routines.  stringToNode() becomes a macro which passes
STRINGTONODE_INVALIDATE_OPFUNCID to stringToNodeExtended(), which is
one of the functions that now takes an additional "int flags"
argument.  If a caller doesn't wish to ignore opfuncid, they can call
stringToNodeExtended directly.  This way, existing stringToNode()
callers see no behavior change, but the parallel query code can easily
get the behavior that it wants.

Thoughts?  Better ideas?  Objections?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/src/backend/nodes/read.c b/src/backend/nodes/read.c
index 0dabfa7..87ece914 100644
--- a/src/backend/nodes/read.c
+++ b/src/backend/nodes/read.c
@@ -35,7 +35,7 @@ static char *pg_strtok_ptr = NULL;
  *	  returns a Node with a given legal ASCII representation
  */
 void *
-stringToNode(char *str)
+stringToNodeExtended(char *str, int flags)
 {
 	char	   *save_strtok;
 	void	   *retval;
@@ -50,7 +50,7 @@ stringToNode(char *str)
 
 	pg_strtok_ptr = str;		/* point pg_strtok at the string to read */
 
-	retval = nodeRead(NULL, 0); /* do the reading */
+	retval = nodeRead(NULL, 0, flags); /* do the reading */
 
 	pg_strtok_ptr = save_strtok;
 
@@ -275,7 +275,7 @@ nodeTokenType(char *token, int length)
  * this should only be invoked from within a stringToNode operation).
  */
 void *
-nodeRead(char *token, int tok_len)
+nodeRead(char *token, int tok_len, int flags)
 {
 	Node	   *result;
 	NodeTag		type;
@@ -293,7 +293,7 @@ nodeRead(char *token, int tok_len)
 	switch ((int) type)
 	{
 		case LEFT_BRACE:
-			result = parseNodeString();
+			result = parseNodeString(flags);
 			token = pg_strtok(_len);
 			if (token == NULL || token[0] != '}')
 elog(ERROR, "did not find '}' at end of input node");
@@ -359,7 +359,7 @@ nodeRead(char *token, int tok_len)
 		/* We have already scanned next token... */
 		if (token[0] == ')')
 			break;
-		l = lappend(l, nodeRead(token, tok_len));
+		l = lappend(l, nodeRead(token, tok_len, flags));
 		token = pg_strtok(_len);
 		if (token == NULL)
 			elog(ERROR, "unterminated List structure");
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index ef88209..bf101ae 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -121,7 +121,7 @@
 #define READ_NODE_FIELD(fldname) \
 	token = pg_strtok();		/* skip :fldname */ \
 	(void) token;/* in case not used elsewhere */ \
-	local_node->fldname = nodeRead(NULL, 0)
+	local_node->fldname = nodeRead(NULL, 0, flags)
 
 /* Read a bitmapset field */
 #define READ_BITMAPSET_FIELD(fldname) \
@@ -222,7 +222,7 @@ _readBitmapset(void)
  * _readQuery
  */
 static Query *
-_readQuery(void)
+_readQuery(int flags)
 {
 	READ_LOCALS(Query);
 
@@ -266,7 +266,7 @@ _readQuery(void)
  * _readNotifyStmt
  */
 static NotifyStmt *
-_readNotifyStmt(void)
+_readNotifyStmt(int flags)
 {
 	READ_LOCALS(NotifyStmt);
 
@@ -280,7 +280,7 @@ _readNotifyStmt(void)
  * _readDeclareCursorStmt
  */
 static DeclareCursorStmt *

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

[Joshua D. Drake]

On 09/23/2015 11:33 AM, Stephen Frost wrote:

* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:

Kam Lasater wrote:


I'd suggest: Github Issues, Pivotal Tracker or Redmine (probably in
that order). There are tens to hundreds of other great ones out there,
I'm sure one of them would also work.


If you install debbugs and feed it from our lists, maybe enough of us
would jump into the bandwagon enough to get it off the ground.  I'm
unsure that it would work to maintain something that's too removed from
the mailing lists.


The difficulty with this is that someone has to actually offer up to
maintain it.  Bug trackers do not maintain themselves.


If we integrate it into the process it gets easier. This is especially 
true if we use a bug tracker that can be managed via email as well as a 
web interface.


Sincerely,

JD


--
Command Prompt, Inc. - http://www.commandprompt.com/  503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Announcing "I'm offended" is basically telling the world you can't
control your own emotions, so everyone else should do it for 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] Parallel Seq Scan

[Robert Haas]

On Thu, Sep 3, 2015 at 6:21 AM, Amit Kapila  wrote:
> [ new patches ]

More review comments:

ExecParallelEstimate() and ExecParallelInitializeDSM() should use
planstate_tree_walker to iterate over the whole planstate tree.
That's because the parallel node need not be at the top of the tree.
By using the walker, you'll find plan state nodes that need work of
the relevant type even if they are deeply buried.  The logic should be
something like:

if (node == NULL) return false;
switch (nodeTag(node)) { ... /* parallel aware node enumerated here */ }
return planstate_tree_walker(node, ExecParallelEstimate, context);

The function signature should be changed to bool
ExecParallelEstimate(PlanState *planstate, parallel_estimate_ctx
*context) where parallel_estimate_ctx is a structure containing
ParallelContext *context and Size *psize.  The comment about handling
only a few node types should go away, because by using the
planstate_tree_walker we can iterate over anything.

It looks to me like there would be trouble if an initPlan or subPlan
were kept below a Funnel, or as I guess we're going to call it, a
Gather node.  That's because a SubPlan doesn't actually have a pointer
to the node tree for the sub-plan in it.  It just has an index into
PlannedStmt.subplans.  But create_parallel_worker_plannedstmt sets the
subplans list to NIL.  So that's not gonna work.  Now maybe there's no
way for an initPlan or a subPlan to creep down under the funnel, but I
don't immediately see what would prevent it.

+* There should be atleast thousand pages to scan for each worker.

"at least a thousand"

+cost_patialseqscan(Path *path, PlannerInfo *root,

patial->partial.

I also don't see where you are checking that a partial seq scan has
nothing parallel-restricted in its quals.

-- 
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] No Issue Tracker - Say it Ain't So!

[Kam Lasater]

> > I'd suggest: Github Issues, Pivotal Tracker or Redmine (probably in
> > that order). There are tens to hundreds of other great ones out there,
> > I'm sure one of them would also work.
>
> If you install debbugs and feed it from our lists, maybe enough of us
> would jump into the bandwagon enough to get it off the ground.  I'm
> unsure that it would work to maintain something that's too removed from
> the mailing lists.

Alvaro,

Thanks for the suggestion. However, an issue tracker is not a
replacement for mailing list(s) and vice versa. They are both
necessary for success.


-- 
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] unclear about row-level security USING vs. CHECK

[Stephen Frost]

* Peter Eisentraut (pete...@gmx.net) wrote:
> On 9/23/15 11:05 AM, Stephen Frost wrote:
> > That the USING policy is used if WITH CHECK isn't defined?  That was
> > simply done to make policy management simple as in quite a few cases
> > only one policy is needed.  If a WITH CHECK was always required then
> > you'd be constantly writing:
> > 
> > CREATE POLICY p1 ON t1
> > USING (entered_by = current_user)
> > WITH CHECK (entered_by = current_user);
> > 
> > With potentially quite lengthy expressions.
> 
> That might be reasonable, but the documentation is completely wrong
> about that.

Really?  I feel pretty confident that it's at least mentioned.  I
agree that it should be made more clear.

> That said, why even have USING and CHECK as separate clauses?  Can't you
> just create different policies if you want them different?
> 
> Hypothetical example:
> 
> CREATE POLICY p1 ON t1 FOR SELECT CHECK (extract(year from entered_on) =
> extract(year from current_timestamp));
> CREATE POLICY p2 ON t2 FOR INSERT, UPDATE, DELETE CHECK (entered_by =
> current_user);

USING is about visibility of existing records, WITH CHECK is in regards
to new rows being added to the relation (either through an INSERT or an
UPDATE).  It would be possible to change WITH CHECK for INSERT to be
USING, but that doesn't work for UPDATE as there are many use-cases
where you want a different policy for the UPDATE visibility vs. the
resulting record.

To say it another way, you may be allowed to update lots of records but
the resulting records have to pass a different policy to be allowed.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] CREATE POLICY and RETURNING

[Stephen Frost]

* 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


signature.asc
Description: Digital signature

Re: [HACKERS] unclear about row-level security USING vs. CHECK

[Stephen Frost]

Peter,

* Peter Eisentraut (pete...@gmx.net) wrote:
> I'm testing the new row-level security feature.  I'm not clear on the
> difference between the USING and CHECK clauses in the CREATE POLICY
> statement.
> 
> The documentation says:
> 
> """
> A policy grants the ability to SELECT, INSERT, UPDATE, or DELETE rows
> which match the relevant policy expression. Existing table rows are
> checked against the expression specified via USING, while new rows that
> would be created via INSERT or UPDATE are checked against the expression
> specified via WITH CHECK. When a USING expression returns true for a
> given row then that row is visible to the user, while if a false or null
> is returned then the row is not visible. When a WITH CHECK expression
> returns true for a row then that row is added, while if a false or null
> is returned then an error occurs.
> """

The CREATE POLICY documentation discusses how lack of a WITH CHECK
policy means the USING expression is used:

"""
Policies can be applied for specific commands or for specific roles. The
default for newly created policies is that they apply for all commands
and roles, unless otherwise specified. If multiple policies apply to a
given query, they will be combined using OR (although ON CONFLICT DO
UPDATE and INSERT policies are not combined in this way, but rather
enforced as noted at each stage of ON CONFLICT execution). Further, for
commands which can have both USING and WITH CHECK policies (ALL and
UPDATE), if no WITH CHECK policy is defined then the USING policy will
be used for both what rows are visible (normal USING case) and which
rows will be allowed to be added (WITH CHECK case).
"""

> So basically, USING filters out what you see, CHECK controls what you
> can write.

Right.

> But then this doesn't work correctly:
> 
> CREATE TABLE test1 (content text, entered_by text);
> ALTER TABLE test1 ENABLE ROW LEVEL SECURITY;
> CREATE POLICY test1_policy ON test1 FOR ALL TO PUBLIC USING (entered_by
> = current_user);
> GRANT ALL ON TABLE test1 TO PUBLIC;
> 
> CREATE USER foo1;
> SET SESSION AUTHORIZATION foo1;
> INSERT INTO test1 VALUES ('blah', 'foo2');  -- fails

You didn't specify a WITH CHECK policy and so the USING policy of
(entered_by = current_user) was used, as described above in the CREATE
POLICY documentation.

> This is a typical you-can-only-see-your-own-rows setup, which works for
> the reading case, but it evidently also controls writes.  So I'm not
> sure what the CHECK clause is supposed to add on top of that.

It could any number of additional checks; in this example perhaps
'content' which is being updated or newly added must have include
'Copyright 2015' or some such.

> (Btw., what's the meaning of a policy for DELETE?)

The DELETE policy controls what records a user is able to delete.

* Peter Eisentraut (pete...@gmx.net) wrote:
> On 9/23/15 2:52 PM, Stephen Frost wrote:
> >> That might be reasonable, but the documentation is completely wrong
> >> about that.
> > 
> > Really?  I feel pretty confident that it's at least mentioned.  I
> > agree that it should be made more clear.
> 
> I quoted the documentation at the beginning of the thread.  That's all I
> could find about it.

Hopefully the above helps.  There's a lot of information in the
individual POLICY commands, especially in CREATE POLICY.  Perhaps some
of that needs to be brought into the overall RLS section, but I'm not
sure we really want to duplicate it all.

> > USING is about visibility of existing records, WITH CHECK is in regards
> > to new rows being added to the relation (either through an INSERT or an
> > UPDATE).
> 
> That makes sense, but then the current behavior that I mentioned at the
> beginning of the thread is wrong.  If you think these clauses are
> clearly separate, then they should be, er, clearly separate.

They're not seperate as implemented and documented.  The current
discussion is about if we wish to change that.

> Maybe the syntax can be tweaked a little, like USING AND CHECK or
> whatever.  Not that USING and CHECK are terribly intuitive in this
> context anyway.

Ah, so that would be a fourth option along the lines of:

CREATE POLICY p1 ON t1
USING AND WITH CHECK ();

That'd certainly be straight-forward to implement.  Would we then
require the user to explicitly state the WITH CHECK piece, where it
applies, then?

Thanks!

Stephen


signature.asc
Description: Digital signature

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

[Joshua D. Drake]

On 09/23/2015 11:23 AM, Alvaro Herrera wrote:

Kam Lasater wrote:


I'd suggest: Github Issues, Pivotal Tracker or Redmine (probably in
that order). There are tens to hundreds of other great ones out there,
I'm sure one of them would also work.


If you install debbugs and feed it from our lists, maybe enough of us
would jump into the bandwagon enough to get it off the ground.  I'm
unsure that it would work to maintain something that's too removed from
the mailing lists.


There needs to be community buy in on this idea else it is just a waste 
of resources. The hackers need say, "Hey... you know, all those other 
projects may be on to something. Perhaps we should learn from them and 
do better by our users." This is more than just an issue/bug tracker 
discussion. It is a ideology shift.


Until that happens asking anyone to put resources into this idea is just 
not worth it.


Sincerely,

JD

--
Command Prompt, Inc. - http://www.commandprompt.com/  503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Announcing "I'm offended" is basically telling the world you can't
control your own emotions, so everyone else should do it for 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] unclear about row-level security USING vs. CHECK

[Stephen Frost]

* Robert Haas (robertmh...@gmail.com) wrote:
> On Wed, Sep 23, 2015 at 12:01 PM, Stephen Frost  wrote:
> > * Robert Haas (robertmh...@gmail.com) wrote:
> >> My expectation would have been:
> >>
> >> If you specify USING, you can see only those rows, but you can give
> >> rows away freely.  If you don't want to allow giving rows away under
> >> any circumstances, then specify the same expression for USING and WITH
> >> CHECK.
> >
> > Having an implicit 'true' for WITH CHECK would be very much against what
> > I would ever expect.  If anything, I'd think we would have an implicit
> > 'false' there or simply not allow it to ever be unspecified.
> 
> Huh?  If you had an implicit false, wouldn't that prevent updating or
> deleting any rows at all?

Right, just the same as how, if RLS is enabled and no explicit policies
are provided, non-owners can't see the rows or insert/update/delete
anything in the table.  The same is true for the GRANT system, where
there are no permissions granted by default.  I view the lack of an
explicit definition of a WITH CHECK clause to be the same, excepting the
simple case where it's the same as USING.

> >> I don't really get that.  If you could make skipping a row trigger an
> >> error, then that would create a bunch of covert channel attacks.
> >
> > Apparently I didn't explain it correctly.  Skipping a row doesn't
> > trigger an error.  An example would perhaps help here to clarify:
> >
> > CREATE POLICY p1 ON t1 FOR DELETE
> > USING (true)
> > WITH CHECK (inserted_by = current_user);
> >
> > What would happen above is that, in a DELETE case, you're allowed to
> > *try* and delete any record in the table, but if you try to delete a
> > record which isn't yours, we throw an error.  Currently the only option,
> > if you want to prevent users from deleteing records which are not
> > theirs, is to have:
> >
> > CREATE POLICY p1 ON t1 FOR DELETE
> > USING (inserted_by = current_user)
> >
> > Which certainly has the effect that you can only delete records you own,
> > but I can see use-cases where you'd like to know that someone tried to
> > delete a record which isn't their own and that isn't something you can
> > get directly today.
> 
> Well, you can use a trigger, I think.  But the point is that right
> now, if you try to delete a record that you don't own, it just says
> DELETE 0.  Maybe there was a record there that you can't see, and
> maybe there wasn't.

Yes, a trigger would also work for this.  I do understand that right now
the way it works is that there isn't an error thrown.  The notion was to
provide the administrator with the option.  The user in this case likely
would already have access to view the row or at least infer that the row
exists through a FK relationship.  These are all post-9.5 considerations
though.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] unclear about row-level security USING vs. CHECK

[Peter Eisentraut]

On 9/23/15 2:52 PM, Stephen Frost wrote:
>> That might be reasonable, but the documentation is completely wrong
>> about that.
> 
> Really?  I feel pretty confident that it's at least mentioned.  I
> agree that it should be made more clear.

I quoted the documentation at the beginning of the thread.  That's all I
could find about it.

>> That said, why even have USING and CHECK as separate clauses?  Can't you
>> just create different policies if you want them different?
>>
>> Hypothetical example:
>>
>> CREATE POLICY p1 ON t1 FOR SELECT CHECK (extract(year from entered_on) =
>> extract(year from current_timestamp));
>> CREATE POLICY p2 ON t2 FOR INSERT, UPDATE, DELETE CHECK (entered_by =
>> current_user);
> 
> USING is about visibility of existing records, WITH CHECK is in regards
> to new rows being added to the relation (either through an INSERT or an
> UPDATE).

That makes sense, but then the current behavior that I mentioned at the
beginning of the thread is wrong.  If you think these clauses are
clearly separate, then they should be, er, clearly separate.

Maybe the syntax can be tweaked a little, like USING AND CHECK or
whatever.  Not that USING and CHECK are terribly intuitive in this
context anyway.



-- 
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] Rework the way multixact truncations work

[Alvaro Herrera]

The comment on top of TrimMultiXact states that "no locks are needed
here", but then goes on to grab a few locks.  I think we should remove
the comment, or rephrase it to state that we still grab them for
consistency or whatever; or perhaps even remove the lock acquisitions.
(I think the comment is still true: by the time TrimMultiXact runs,
we're out of recovery but not yet running, so it's not possible for
anyone to try to do anything multixact-related.)

I wonder if it would be cleaner to move the setting of finishedStartup
down to just before calling SetMultiXactIdLimit, instead of at the top
of the function.

It's a bit odd that SetMultiXactIdLimit has the "finishedStartup" test
so low.  Why bother setting all those local variables only to bail out?
I think it would make more sense to just do it at the top.  The only
thing you lose AFAICS is that elog(DEBUG1) message -- is that worth it?
Also, the fact that finishedStartup itself is read without a lock at
least merits a comment.

In MultiXactAdvanceOldest, the test for sawTruncationinCkptCycle seems
reversed?
if (!MultiXactState->sawTruncationInCkptCycle)
surely we should be doing truncation if it's set?

Honestly, I wonder whether this message
ereport(LOG,
(errmsg("performing legacy multixact 
truncation"),
 errdetail("Legacy truncations are 
sometimes performed when replaying WAL from an older primary."),
 errhint("Upgrade the primary, it is 
susceptible to data corruption.")));
shouldn't rather be a PANIC.  (The main reason not to, I think, is that
once you see this, there is no way to put the standby in a working state
without recloning).

I think the prevOldestOffsetKnown test in line 2667 ("if we failed to
get ...") is better expressed as an else-if of the previous "if" block.

I think the two "there are NO MultiXacts" cases in TruncateMultiXact
would benefit in readability from adding braces around the lone
statement (and moving the comment to the line prior).

If the find_multixact_start(oldestMulti) call in TruncateMultiXact
fails, what recourse does the user have?  I wonder if the elog() should
be a FATAL instead of just LOG.  It's not like it would work on a
subsequent run, is it?

-- 
Á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] Rework the way multixact truncations work

[Andres Freund]

On 2015-09-23 15:57:02 -0300, Alvaro Herrera wrote:
> I think we need to make a decision here.  Is this a terribly serious
> bug/misdesign that needs addressing?

Imo yes. Not sure about terribly, but definitely serious. It's several
data loss bugs in one package.

> If so, we need to backpatch.  If not, then by all means lets leave it
> alone.  I don't think it is a good idea to leave it open if we think
> it's serious, which is what I think is happening.

Right, but I don't want to backpatch this over an objection, and it
doesn't seem like I have a chance to convince Robert that it'd be a good
idea. So it'll be 9.5+master for now.

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] unclear about row-level security USING vs. CHECK

[Alvaro Herrera]

Robert Haas wrote:
> On Wed, Sep 23, 2015 at 2:39 PM, Stephen Frost  wrote:
> > * Robert Haas (robertmh...@gmail.com) wrote:
> >> On Wed, Sep 23, 2015 at 12:01 PM, Stephen Frost  wrote:
> >> > * Robert Haas (robertmh...@gmail.com) wrote:
> >> >> My expectation would have been:
> >> >>
> >> >> If you specify USING, you can see only those rows, but you can give
> >> >> rows away freely.  If you don't want to allow giving rows away under
> >> >> any circumstances, then specify the same expression for USING and WITH
> >> >> CHECK.
> >> >
> >> > Having an implicit 'true' for WITH CHECK would be very much against what
> >> > I would ever expect.  If anything, I'd think we would have an implicit
> >> > 'false' there or simply not allow it to ever be unspecified.
> >>
> >> Huh?  If you had an implicit false, wouldn't that prevent updating or
> >> deleting any rows at all?
> >
> > Right, just the same as how, if RLS is enabled and no explicit policies
> > are provided, non-owners can't see the rows or insert/update/delete
> > anything in the table.  The same is true for the GRANT system, where
> > there are no permissions granted by default.  I view the lack of an
> > explicit definition of a WITH CHECK clause to be the same, excepting the
> > simple case where it's the same as USING.
> 
> Hmm, interesting.  I guess that's a defensible position, but I still
> think that having them default to be the same thing implicitly is
> kinda weird.  I'll defer to whatever the consensus, is, though.

I think an explicit statement of a "true" as WITH CHECK makes more sense
-- I think Stephen suggested it upthread as making the WITH CHECK be
mandatory.  If you really want to allow rows to be "given away" (which
could be a security issue), a "WITH CHECK (true)" is easy enough to
specify.

-- 
Á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] Calculage avg. width when operator = is missing

[Shulgin, Oleksandr]

On Tue, Sep 22, 2015 at 11:17 PM, Alvaro Herrera 
wrote:

> Shulgin, Oleksandr wrote:
> > On Sep 22, 2015 8:58 PM, "Andrew Dunstan"  wrote:
>
> > > Yes, "/revenons/ à /nos moutons/." You can set up text based comparison
> > > ops fairly easily for json - you just need to be aware of the
> limitations.
> > > See https://gist.github.com/adunstan/32ad224d7499d2603708
> >
> > Yes, I've already tried this approach and have found that analyze
> > performance degrades an order of magnitude due to sql-level function
> > overhead and casts to text.  In my tests, from 200ms to 2000ms with btree
> > ops on a default sample of 30,000 rows.
>
> You should be able to create a C function json_cmp() that simply calls
> bttextcmp() internally, and C functions for each operator using that
> one, in the same way.
>

Yes, but I didn't try this because of the requirement to
compile/install/maintain the externally loadable module.  If I could just
use CREATE FUNCTION on a postgres' internal function such as texteq or
bttextcmp (with obj_file of NULL, for example) I would definitely do that.
:-)

--
Alex

Re: [HACKERS] Calculage avg. width when operator = is missing

[Shulgin, Oleksandr]

On Tue, Sep 22, 2015 at 11:56 PM, Alvaro Herrera 
wrote:

> Tom Lane wrote:
>
> > Should we consider this HEAD-only, or a back-patchable bug fix?
> > Or perhaps compromise on HEAD + 9.5?
>
> It looks like a bug to me, but I think it might destabilize approved
> execution plans(*), so it may not be such a great idea to back patch
> branches that are already released.  I think HEAD + 9.5 is good.
>
> (*) I hear there are even applications where queries and their approved
> execution plans are kept in a manifest, and plans that deviate from that
> raise all kinds of alarms.  I have never seen such a thing ...
>

Ugh.  Anyway, do you expect any plans to change only due to avg. width
estimation being different?  Why would that be so?

--
Alex

Re: [HACKERS] CREATE POLICY and RETURNING

[Zhaomo Yang]

Stephen,

It'd be great if others who are interested can help define the grammar
> changes necessary
> and perhaps even help with the code aspect of it.

I'd like to help on both. Can you elaborate a little bit more, especially
on the code aspect?

I don't buy that argument.

It is agreed that blind updates and deletes with RETURNING clause are
dangerous. It is quite similar here.
Instead of using
   BEGIN
   UPDATE-or-DELETE-with-RETURNING
   ROLLBACK
as a substitute for SELECT, a malicious user can do a binary search with
some trick like divide-by-zero
to figure out rows he is not allowed to access. Of course, this is not as
serious as RETURNING, but it is still quite convenient for attackers.

Thanks,
Zhaomo

Re: [HACKERS] Parallel Seq Scan

[Amit Kapila]

On Wed, Sep 23, 2015 at 5:42 AM, Robert Haas  wrote:
>
> On Tue, Sep 22, 2015 at 3:21 PM, Amit Kapila 
wrote:
> > Attached patch (read_funcs_v1.patch) contains support for all the plan
> > and other nodes (like SubPlan which could be required for worker) except
> > CustomScan node.
>
> It looks like you need to update the top-of-file comment for outfuncs.c.
>

Updated.

> Doesn't _readCommonPlan() leak?

I have tried that way to keep the code simple with a view that this will
not be read in long-lived memory context, however your suggestion
below makes sense and can avoid it.

>  I think we should avoid that.
> _readCommonScan() and _readJoin() are worse: they leak multiple
> objects.  It should be simple enough to avoid this: just have your
> helper function take a Plan * as argument and then use
> READ_TEMP_LOCALS() rather than READ_LOCALS().  Then the caller can use
> READ_LOCALS, call the helper to fill in all the Plan fields, and then
> read the other stuff itself.
>

Changed as per suggestion.

> Instead of passing the Plan down by casting, how about passing
> _node->plan?  And similarly for scans and joins.
>

Changed as per suggestion.

> readAttrNumberCols uses sizeof(Oid) instead of sizeof(AttrNumber).
>

Fixed.

> I still don't understand why we need to handle PlanInvalItem.
> Actually, come to think of it, I'm not sure we need PlannedStmt
> either.  Let's leave those out; they seem like trouble.
>

As discussed below this is required and I haven't changed it.

> I think it would be worth doing something like this:
>
> #define READ_ATTRNUMBER_ARRAY(fldname, len) \
> token = pg_strtok(); \
> local_node->fldname = readAttrNumberCols(len);
>
> And similarly for READ_OID_ARRAY, READ_BOOL_ARRAY, READ_INT_ARRAY.
>

Changed as per suggestion.

> In general these routines are in the same order as plannodes.h, which
> is good. But _readNestLoopParam is out of place.  Can we move it just
> after _readNestLoop?
>

I have kept them in order they appear in nodes.h (that way it seems easier
to keep track if anything is missed), however if you want I can reorder them
as per your suggestion.


Note - Test is changed to verify just the output of readfuncs with changes
in
planner.  I have removed elog related changes in readfuncs, as during last
test, I have verified that make check-world covers all types of nodes that
are added by patch.

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


read_funcs_v2.patch
Description: Binary data


read_funcs_test_v2.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] No Issue Tracker - Say it Ain't So!

[Alvaro Herrera]

Jeff Janes wrote:

> For whatever it is worth, one of the frustrations I've had with projects
> (other than PostgreSQL) of which I am a casual users is that reporting a
> single bug meant signing up for yet another account on yet another site and
> learning yet another bug tracking system.

Right.

> I think the email based system is more friendly for the casual user.  You
> don't even have to sign up for the bugs mail list as long as people keep
> you CC'ed.  I don't think that having a tracker just for the sake of having
> one is going to attract new contributors.

Yay, another vote for debbugs!

> I'd rather, say, put some more work into cleaning the kruft out of the
> To-Do list, then put that effort into migrating the kruft to a fancier
> filing cabinet.

Casual users would need a community account in order to file bugs in the
Todo wiki page.  I don't think a wiki page qualifies (by a long mile) as
a bug tracker, anyway.

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

[David G. Johnston]

On Wed, Sep 23, 2015 at 5:00 PM, Stephen Frost  wrote:

> Kam,
>
> * Kam Lasater (c...@seekayel.com) wrote:
> > > ... The above-referenced individuals
> > > would be the bug tracking system curators, of course.  Unless it's got
> > > serious technical issues, the infrastructure team will do our best to
> > > support the choice.  On the other hand, some of us would likely be
> > > involved in bug curation also.
> >
> > Stephen,
> >
> > In digging around more I found this wiki page that seems to be the
> > closest thing to a BT: https://wiki.postgresql.org/wiki/Todo
> >
> > Is the curation already being done? If the contents of that wiki page
> > were injected into a BT, would that be enough of a start?
>
> If you look at what happens on the -bugs mailing list, you'll see very
> quickly that a bug tracker and the Todo wiki page are very different.
> Perhaps the Todo could be squeezed into a bug tracker, but many of the
> items on there might well end up as 'wontfix' (to use the debbugs
> lingo).
>
> That is certainly not the same curation as what a BT would require.
>

​To put it a bit differently what kind of mechanism is going to exist to
ensure that the BT doesn't simply become a "wish list" of new features?  It
will be much easier for people to simply post to it instead of adding an
entry on the Todo wiki page.

Instead of curation I'd be curious if others have tried to institute
gate-keeping such that end-users still have to post to -bugs and only
authorized persons can convert (forward?) those into actual bug reports.

David J.

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

[Jim Nasby]

On 9/23/15 3:12 PM, Thomas Kellerer wrote:

They also support Postgres as their backend (and you do find hints here and
there
that it is the recommended open source DBMS for them - but they don't
explicitly state it like that). We are using Jira at the company I work for
and
all Jira installations run on Postgres there.


I'll second Jira as well. It's the only issue tracker I've seen that you 
can actually use for multiple different things without it becoming a 
mess. IE: it could track Postgres bugs, infrastructure issues, and the 
TODO list if we wanted, allow issues to reference each other 
intelligently, yet still keep them as 3 separate bodies.


They're also based here in Austin so we've got community folks that can 
interface with them directly if that's ever needed.

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.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] No Issue Tracker - Say it Ain't So!

[Adam Brightwell]

>> Personally I'd also change sending patches in emails to github pull
>> requests :).
>
> That won't happen, at least not this decade.

FWIW, a year ago I might have agreed that a github pull-request would
be preferable.  However, since, I have grown to really like the patch
via email approach.  I can see a lot of value in keeping patch
submission decoupled from a specific service/technology/workflow in
this way.

>> ... or maybe the difference is more in the data structure, the email
>> discussion is a tree (with a horrible interface to the archive) while in a
>> bug tracker, the discussion is linear, and easier to follow.
>
> FWIW in my opinion our mailing list archives interface is the best there
> is --- and I disagree that the linear discussion is easy to follow,
> except for trivial discussions.

In my experience, following other mailing lists, I really appreciate
our interface.  I'm not sure that I'd call it the best, but I've
certainly seen far worse and I have no real complaints about it.  What
I think I like best about it is that it has an community "official"
status, meaning we don't depend on some other mirror/archive site to
support it, like gmane or spinics.  This is just my opinion though.

-Adam

-- 
Adam Brightwell - adam.brightw...@crunchydatasolutions.com
Database Engineer - www.crunchydatasolutions.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] No Issue Tracker - Say it Ain't So!

[Jim Nasby]

On 9/23/15 3:29 PM, Alvaro Herrera wrote:

Joshua D. Drake wrote:


Until that happens asking anyone to put resources into this idea is just not
worth it.


I wonder if you still have this conversation archived:

Date: Thu, 10 May 2007 11:30:55 -0400
From: Andrew Dunstan 
To: "Joshua D. Drake", Magnus Hagander, Alvaro Herrera, Robert Treat, Lukas 
Smith, Andrew Sullivan, David Fetter
Subject: tracker

There was some very good input there -- it would be a shame to have to
repeat that all over again.  Hey, it's only been 8 years ...


Ha! Good to know our scheduling is consistent at least!
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.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] CREATE POLICY and RETURNING

[Zhaomo Yang]

Stephen,

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

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

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

Thanks,
Zhaomo


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

[Robert Haas]

On Wed, Sep 23, 2015 at 2:30 PM, Kam Lasater  wrote:
> Thanks for the suggestion. However, an issue tracker is not a
> replacement for mailing list(s) and vice versa. They are both
> necessary for success.

I venture to say that we are succeeding as it is, although of course
we might have more success if we did some things better, including
this.  However, as Stephen says, the problem with an issue tracker is
that, unless some person or persons committed to keep it up to date,
it would just fill up with crap. We have an issue tracker for database
server issues here at EnterpriseDB, and keeping it up to date is a ton
of work.  If nobody's volunteering to do that work in the PostgreSQL
community, an issue tracker is going to end up not being useful,
because it will just be wrong all the time.

If somebody does do the work, then we get to the next question: if we
had an accurate list of open bugs, would anybody who currently doesn't
work on fixing those bugs step up to help fix them?  I hope so, but I
don't know.  If not, we might not feel that the effort of maintaining
the bug tracker paid much of a dividend.

-- 
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] unclear about row-level security USING vs. CHECK

[Stephen Frost]

* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> Robert Haas wrote:
> > On Wed, Sep 23, 2015 at 2:39 PM, Stephen Frost  wrote:
> > > * Robert Haas (robertmh...@gmail.com) wrote:
> > >> On Wed, Sep 23, 2015 at 12:01 PM, Stephen Frost  
> > >> wrote:
> > >> > * Robert Haas (robertmh...@gmail.com) wrote:
> > >> >> My expectation would have been:
> > >> >>
> > >> >> If you specify USING, you can see only those rows, but you can give
> > >> >> rows away freely.  If you don't want to allow giving rows away under
> > >> >> any circumstances, then specify the same expression for USING and WITH
> > >> >> CHECK.
> > >> >
> > >> > Having an implicit 'true' for WITH CHECK would be very much against 
> > >> > what
> > >> > I would ever expect.  If anything, I'd think we would have an implicit
> > >> > 'false' there or simply not allow it to ever be unspecified.
> > >>
> > >> Huh?  If you had an implicit false, wouldn't that prevent updating or
> > >> deleting any rows at all?
> > >
> > > Right, just the same as how, if RLS is enabled and no explicit policies
> > > are provided, non-owners can't see the rows or insert/update/delete
> > > anything in the table.  The same is true for the GRANT system, where
> > > there are no permissions granted by default.  I view the lack of an
> > > explicit definition of a WITH CHECK clause to be the same, excepting the
> > > simple case where it's the same as USING.
> > 
> > Hmm, interesting.  I guess that's a defensible position, but I still
> > think that having them default to be the same thing implicitly is
> > kinda weird.  I'll defer to whatever the consensus, is, though.
> 
> I think an explicit statement of a "true" as WITH CHECK makes more sense
> -- I think Stephen suggested it upthread as making the WITH CHECK be
> mandatory.  If you really want to allow rows to be "given away" (which
> could be a security issue), a "WITH CHECK (true)" is easy enough to
> specify.

Right, the options, in my view at least, are:

1) keep it as-is
2) make WITH CHECK mandatory
3) keep WITH CHECK optional, but default it to 'false' instead

If an administrator really wants WITH CHECK to be 'true', then they can
always add that clause in explicitly, but that really shouldn't be the
default.

For my part at least, I'm still preferring #1, but if there's a
consensus around #2 or #3 among the others interested then I'm happy to
make the actual code changes required.

Thanks!

Stephen


signature.asc
Description: Digital signature

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

[Alvaro Herrera]

Szymon Lipiński wrote:

> Then I need to read through the emails. This is not user friendly too, as I
> need to click through the email tree, and if an email has multiple replies,
> it is usually hard not to omit some of them, as after going into a reply, I
> need to click to get to the parent mail again.

Evidently, the "flat" link is easy to miss.  Give it a try.

The bug tracker is not intended as a feature-request tracker, anyway.
Those two things are very different, even if many projects just conflate
the two things.

> Personally I'd also change sending patches in emails to github pull
> requests :).

That won't happen, at least not this decade.

> ... or maybe the difference is more in the data structure, the email
> discussion is a tree (with a horrible interface to the archive) while in a
> bug tracker, the discussion is linear, and easier to follow.

FWIW in my opinion our mailing list archives interface is the best there
is --- and I disagree that the linear discussion is easy to follow,
except for trivial discussions.

-- 
Á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] DBT-3 with SF=20 got failed

[Tom Lane]

Tomas Vondra  writes:
> On 09/11/2015 07:16 PM, Robert Haas wrote:
>> On Fri, Sep 11, 2015 at 1:12 PM, Tomas Vondra
>>  wrote:
>>> I'm arguing for fixing the existing bug, and then addressing the case of
>>> over-estimation separately, with proper analysis.

>> Well, this is part of how we're looking it differently.  I think the
>> bug is "we're passing a value to palloc that is too large, so
>> sometimes it fails" and the way to fix that is to properly limit the
>> value.  You are clearly defining the bug a bit differently.

> Yes, I see it differently.

> I don't quite understand why limiting the value is more "proper" than 
> using a function that can handle the actual value.

> The proposed bugfix addresses the issue in the most straightforward way, 
> without introducing additional considerations about possible 
> over-estimations (which the current code completely ignores, so this is 
> a new thing). I think bugfixes should not introduce such changes to 
> behavior (albeit internal), especially not without any numbers.

This thread seems to have stalled out...

After re-reading it, I'm going to agree with Robert that we should clamp
the initial pointer-array size to work with palloc (ie, 512MB worth of
pointers, which please recall is going to represent at least 10X that much
in hashtable entries, probably more).  The argument that allowing it to be
larger would be a performance win seems entirely unbased on any evidence,
while the risks of allowing arbitrarily large allocations based on planner
estimates seem pretty obvious to me.  And the argument that such
overestimates are a bug that we can easily fix is based on even less
evidence; in fact, I would dismiss that out of hand as hubris.

Now there is a separate issue about whether we should allow hashtable
resizes to exceed that limit.  There I would vote yes, as long as the
resize is based on arrival of actual data and not estimates (following
Robert's point that the existing uses of repalloc_huge are driven by
actual need).

So I don't like any of the proposed patches exactly as-is.

BTW, just looking at the code in question, it seems to be desperately
in need of editorial review.  A couple of examples:

* Some of the code goes to the trouble of maintaining a
log2_nbuckets_optimal value, but ExecHashIncreaseNumBuckets doesn't
know about that and recomputes the value.

* ExecHashIncreaseNumBuckets starts out with a run-time test on something
that its sole caller has just Assert()'d to not be the case, and which
really ought to be impossible with or without that Assert.

* ExecHashTableInsert believes it can only increase nbuckets_optimal
if we're still in the first batch, but MultiExecHash() will call
ExecHashIncreaseNumBuckets at the end of input-acquisition whether we've
created more than one batch or not.  Meanwhile, ExecHashIncreaseNumBatches
thinks it can change the number of buckets in any case.  Maybe that's all
okay, but at the very least those tests ought to look like they'd heard of
each other.  And of those three places, having the safety check where it
is seems like the least reasonable place.  Tracking an "optimal" number
of buckets seems like an activity that should not be constrained by
whether we have any hope of being able to apply the number.

So I'm not having a lot of faith that there aren't other bugs in the
immediate area.

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] clearing opfuncid vs. parallel query

[Robert Haas]

On Wed, Sep 23, 2015 at 4:31 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> readfuncs.c deliberately ignores any opfuncid read in for an OpExpr,
>> DistinctExpr, NullIfExpr, or ScalarArrayOpExpr, instead resetting the
>> value in the newly-created node to InvalidOid.  This is because it
>> assumes we're reading the node from a pg_node_tree column in some
>> system catalog, and if in the future we wanted to allow an ALTER
>> OPERATOR command to change the pg_proc mapping, then the opfuncid
>> could change.  We'd want to look it up again rather than using the
>> previous value.
>
> Right, but considering that nobody has even thought about implementing
> such a command in the past twenty years, maybe we should just change
> the behavior of those read routines?

Well, I can't vouch for what any human being on earth has thought
about over a twenty-year period.  It's not intrinsically unreasonable
in my mind to want to alter an operator to point at a different
procedure.

But if we're sure we don't want to support that, changing the behavior
of the read routines would be fine with me, too.  It would even save a
few cycles.  Would you also want to rip out the stuff that fixes up
opfuncid as dead code?  I assume yes, but sometimes I assume things
that are false.

-- 
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] clearing opfuncid vs. parallel query

[Tom Lane]

Robert Haas  writes:
> But if we're sure we don't want to support that, changing the behavior
> of the read routines would be fine with me, too.  It would even save a
> few cycles.  Would you also want to rip out the stuff that fixes up
> opfuncid as dead code?  I assume yes, but sometimes I assume things
> that are false.

Yeah, though I think of that as a longer-term issue, ie we could clean it
up sometime later.  I'm not sure right now that everyplace that initially
creates OpExpr etc. nodes is on board with having to supply opfuncid.
I do know that the main path through the parser provides 'em.  (So another
reason I don't like the current approach is that I doubt all code that
should theoretically be doing set_opfuncid() is actually doing so; it
would be too easy to miss the need for it in simple testing.)

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] No Issue Tracker - Say it Ain't So!

[David G. Johnston]

On Wed, Sep 23, 2015 at 5:10 PM, Szymon Lipiński  wrote:

>
>
> On 23 September 2015 at 22:07, Stephen Frost  wrote:
>
>> * Josh Berkus (j...@agliodbs.com) wrote:
>> > On 09/23/2015 11:18 AM, Kam Lasater wrote:
>> > > At this point not having one is borderline negligent. I'd suggest:
>> > > Github Issues, Pivotal Tracker or Redmine (probably in that order).
>> > > There are tens to hundreds of other great ones out there, I'm sure one
>> > > of them would also work.
>> >
>> > First, understand that the Postgres project was created before bug
>> > trackers existed. And people are very slow to change their habits,
>> > especially since not having a bug tracker was actually a benefit up
>> > until around 2005.  It's not anymore, but I'm sure people will argue
>> > with my statement on that.
>> >
>> > We have to use something OSS; open source projects depending on
>> > closed-source infra is bad news.  Out of what's available, I'd actually
>> > choose Bugzilla; as much as BZ frustrates the heck out of me at times,
>> > it's the only OSS tracker that's at all sophisticated.
>> >
>> > The alternative would be someone building a sophisticated system on top
>> > of RequestTracker, which would also let us have tight mailing list
>> > integration given RT's email-driven model.  However, that would require
>> > someone with the time to build a custom workflow system and web UI on
>> > top of RT.  It's quite possible that Best Practical would be willing to
>> > help here.
>>
>> Yeah, even if we got past the "do we want a bug tracker?" question, any
>> project would probably end up stalling indefinitely on "well then, which
>> one?"
>>
>> In the end, we should probably just throw something up based on whatever
>> the folks who are going to be actually maintaining it want and call it a
>> 'beta' and see what happens with it.  The above-referenced individuals
>> would be the bug tracking system curators, of course.  Unless it's got
>> serious technical issues, the infrastructure team will do our best to
>> support the choice.  On the other hand, some of us would likely be
>> involved in bug curation also.
>>
>> Thanks!
>>
>> Stephen
>>
>
>
> Hi,
> a couple of days ago I was reading through the tickets in the Django bug
> tracker. It was much easier to find any information about the things to
> work on than currently for Postgres.
>

​TBH, if you want to work on PostgreSQL and are not sure where to best
contribute you should actually two-way communicate​ with the people
actively involved.  If you know what you want to work on you likewise
should propose something reasonably concrete for discussion.  The other
resources are solid and do reflect past ideas and desires and while they do
make a good starting point unless you have a personal interest in the topic
putting the question out to the lists will gauge how necessary the
community deems the feature at this moment in time.

>From my point of view, for Postgres, there is just a not updated too often
> list of things to implement on the wiki. If I need to find some additional
> information, then I can find there just some links to mails from the mail
> groups.
>
> Then I need to read through the emails. This is not user friendly too, as
> I need to click through the email tree, and if an email has multiple
> replies, it is usually hard not to omit some of them, as after going into a
> reply, I need to click to get to the parent mail again.
>
>
​Yes, people are not particularly inclined to put a lot of effort into
organizing pure ideas.  The emails that are out there are probably of more
use to the people that wrote and read them originally than to someone
coming in fresh.  In many cases they were never written to be primary
sources.​



> What's more, there are some things on the wiki, and when I asked about
> that, it turned out that "oh, there was some discussion long time ago, that
> it is not doable".
>
>
​So we should constantly manage the entire Todo list because occasionally
someone shows interest in a couple of items that appear on it that were
already declared "not doable" some time in the past?  This doesn't seem
efficient or likely.  The Todo list is an idea generator, not project
management.
​


> It would be also worth storing the information that someone is working on
> something, so the work won't be doubled.
>
>
The ​Commitfest interface, basically.​

​David J.​

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

[Stephen Frost]

Szymon,

* Szymon Lipiński (mabew...@gmail.com) wrote:
> a couple of days ago I was reading through the tickets in the Django bug
> tracker. It was much easier to find any information about the things to
> work on than currently for Postgres.

I tend to doubt that a bug tracker would change that situation.

> >From my point of view, for Postgres, there is just a not updated too often
> list of things to implement on the wiki. If I need to find some additional
> information, then I can find there just some links to mails from the mail
> groups.

As far as 'new features' go, I don't know that we'd put those on the bug
tracker anyway.  Perhaps some of them should go on the todo list, such
as the discussion around restrictive RLS policies, but the canonical
list is really developer oriented and less project oriented.  That's
probably not a good thing, but I don't think trying to use a bug tracker
to track features is the answer there either.

I will say that something easier than the todo list (aka the wiki) to
work with would be nice for tracking new feature thoughts.

> Then I need to read through the emails. This is not user friendly too, as I
> need to click through the email tree, and if an email has multiple replies,
> it is usually hard not to omit some of them, as after going into a reply, I
> need to click to get to the parent mail again.

This is solved by the 'flat' view, which gives you a single page with
all emails for the thread.  Go to any email in the archives and click on
'flat', at the end of the Message-ID line.

> What's more, there are some things on the wiki, and when I asked about
> that, it turned out that "oh, there was some discussion long time ago, that
> it is not doable".

Right, that's one of the challenges with the current todo list.

> It would be also worth storing the information that someone is working on
> something, so the work won't be doubled.

Thankfully, that doesn't seem to happen too much in this community as we
all communicate a fair bit.  I agree that risk exists, but I don't
believe it's a reason for a bug or feature tracker by itself.

> Personally I'd also change sending patches in emails to github pull
> requests :).

Don't get your hopes up.

> ... or maybe the difference is more in the data structure, the email
> discussion is a tree (with a horrible interface to the archive) while in a
> bug tracker, the discussion is linear, and easier to follow.

The interface is entirely open source and I'm sure Magnus would be
happen to hear specific ideas for improvement, or, even better, pull
requests. :)

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Calculage avg. width when operator = is missing

[Tom Lane]

Alvaro Herrera  writes:
> Tom Lane wrote:
>> Personally I think Alvaro's position is unduly conservative: to the extent
>> that plans change it'd likely be for the better.  But I'm not excited
>> enough to fight hard about it.

> I don't really care enough.  We have received some complaints about
> keeping plans stable, but maybe it's okay.

The other side of the coin is that there haven't been so many requests for
changing this; more than just this one, but not a groundswell.  So 9.5
only seems like a good compromise unless we get more votes for back-patch.

I reviewed the patch and concluded that it would be better to split
compute_minimal_stats into two functions instead of sprinkling it so
liberally with if's.  So I did that and pushed 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] No Issue Tracker - Say it Ain't So!

[Thomas Kellerer]

> We have to use something OSS; open source projects depending on
> closed-source infra is bad news.  Out of what's available, I'd actually
> choose Bugzilla; as much as BZ frustrates the heck out of me at times,
> it's the only OSS tracker that's at all sophisticated.

There are several OSS projects that use closed-source trackers. 
I (personally) don't see a real problem with that.  Atlassian offers free
hosting 
for open source projects (I'm sure Postgres would qualify) and Confluence
Jira 
is one of the best trackers I have worked with. I does have a mail gateway
were 
issues can be created and maintained by sending emails (rather than editing
them in 
the web front end) 

They also support Postgres as their backend (and you do find hints here and
there 
that it is the recommended open source DBMS for them - but they don't 
explicitly state it like that). We are using Jira at the company I work for
and 
all Jira installations run on Postgres there. 

https://www.atlassian.com/software/views/open-source-license-request

Thomas




--
View this message in context: 
http://postgresql.nabble.com/No-Issue-Tracker-Say-it-Ain-t-So-tp5867020p5867046.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.


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

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

[Jim Nasby]

On 9/22/15 6:27 PM, Jim Nasby wrote:

+ ereport(LOG,
+  (errcode(ERRCODE_OUT_OF_MEMORY),
+   errmsg("out of memory attempting to pg_stat_statement
file"),
+   errdetail("file \"%s\": size %lld", PGSS_TEXT_FILE,
stat.st_size)));

Uh, what?


Oops. I'll fix that and address David's concern tomorrow.


New patch attached. I stripped the size reporting out and simplified the 
conditionals a bit as well.

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c 
b/contrib/pg_stat_statements/pg_stat_statements.c
index 59b8a2e..c9dcd89 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -1892,15 +1892,24 @@ qtext_load_file(Size *buffer_size)
}
 
/* Allocate buffer; beware that off_t might be wider than size_t */
-   if (stat.st_size <= MaxAllocSize)
-   buf = (char *) malloc(stat.st_size);
-   else
-   buf = NULL;
+   if (stat.st_size > MaxAllocSize)
+   {
+   ereport(LOG,
+   /* Is there a better code to use? IE: SQLSTATE 
53000, 53400 or 54000 */
+   (errcode(ERRCODE_OUT_OF_MEMORY),
+errmsg("pg_stat_statement file is too large to 
process"),
+errdetail("file \"%s\"", 
PGSS_TEXT_FILE.st_size)));
+   CloseTransientFile(fd);
+   return NULL;
+   }
+
+   buf = (char *) malloc(stat.st_size);
+
if (buf == NULL)
{
ereport(LOG,
(errcode(ERRCODE_OUT_OF_MEMORY),
-errmsg("out of memory")));
+errmsg("out of memory attempting to read 
pg_stat_statement file")));
CloseTransientFile(fd);
return NULL;
}

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

[Josh Berkus]

On 09/23/2015 11:43 AM, Robert Haas wrote:
> If somebody does do the work, then we get to the next question: if we
> had an accurate list of open bugs, would anybody who currently doesn't
> work on fixing those bugs step up to help fix them?  I hope so, but I
> don't know.  If not, we might not feel that the effort of maintaining
> the bug tracker paid much of a dividend.

I don't anticipate that getting additional bug fixers would be a benefit
of having a bug tracker, at least not in the first year.  In fact, I
would say that we don't need a bug tracker to fix most significant bugs
at all.  We're pretty good at that.

What we need a bug tracker for is:

1. so users and downstream projects know where to report bugs (and no,
our idiosyncratic bug form doesn't fit into anyone's workflow).

2. so that users know when a bug is fixed, and what release it's fixed
in, rather than depending on "ask someone on IRC".

3. so that we don't completely lose track of low-importance, hard-to-fix
bugs and trivial bugs, which we currently certainly do.

4. so that we can have a clearer idea more immediately that we've fixed
all known bugs in upcoming postgresql releases, instead of depending on
Bruce catching up on his email.

5. so that we have a place to track bugs which require hard, multi-step
fixes and don't lose track of some of the steps like we did with Multixact.

Those are the main reasons to have a BT.  Offering a place for new
hackers to get started with trivial code fixes might be a side benefit,
but isn't a good enough reason to have one.

Obviously, everything said about "who's going to maintain this" is
completely valid.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


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

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

[Kam Lasater]

> > We have to use something OSS; open source projects depending on
> > closed-source infra is bad news.  Out of what's available, I'd actually
> > choose Bugzilla; as much as BZ frustrates the heck out of me at times,
> > it's the only OSS tracker that's at all sophisticated.

Josh,

I'm not sure I agree here on the BT needing to be OSS. That said not
sure its my call :)

> ... The above-referenced individuals
> would be the bug tracking system curators, of course.  Unless it's got
> serious technical issues, the infrastructure team will do our best to
> support the choice.  On the other hand, some of us would likely be
> involved in bug curation also.

Stephen,

In digging around more I found this wiki page that seems to be the
closest thing to a BT: https://wiki.postgresql.org/wiki/Todo

Is the curation already being done? If the contents of that wiki page
were injected into a BT, would that be enough of a start?


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

[Szymon Lipiński]

On 23 September 2015 at 22:07, Stephen Frost  wrote:

> * Josh Berkus (j...@agliodbs.com) wrote:
> > On 09/23/2015 11:18 AM, Kam Lasater wrote:
> > > At this point not having one is borderline negligent. I'd suggest:
> > > Github Issues, Pivotal Tracker or Redmine (probably in that order).
> > > There are tens to hundreds of other great ones out there, I'm sure one
> > > of them would also work.
> >
> > First, understand that the Postgres project was created before bug
> > trackers existed. And people are very slow to change their habits,
> > especially since not having a bug tracker was actually a benefit up
> > until around 2005.  It's not anymore, but I'm sure people will argue
> > with my statement on that.
> >
> > We have to use something OSS; open source projects depending on
> > closed-source infra is bad news.  Out of what's available, I'd actually
> > choose Bugzilla; as much as BZ frustrates the heck out of me at times,
> > it's the only OSS tracker that's at all sophisticated.
> >
> > The alternative would be someone building a sophisticated system on top
> > of RequestTracker, which would also let us have tight mailing list
> > integration given RT's email-driven model.  However, that would require
> > someone with the time to build a custom workflow system and web UI on
> > top of RT.  It's quite possible that Best Practical would be willing to
> > help here.
>
> Yeah, even if we got past the "do we want a bug tracker?" question, any
> project would probably end up stalling indefinitely on "well then, which
> one?"
>
> In the end, we should probably just throw something up based on whatever
> the folks who are going to be actually maintaining it want and call it a
> 'beta' and see what happens with it.  The above-referenced individuals
> would be the bug tracking system curators, of course.  Unless it's got
> serious technical issues, the infrastructure team will do our best to
> support the choice.  On the other hand, some of us would likely be
> involved in bug curation also.
>
> Thanks!
>
> Stephen
>


Hi,
a couple of days ago I was reading through the tickets in the Django bug
tracker. It was much easier to find any information about the things to
work on than currently for Postgres.

>From my point of view, for Postgres, there is just a not updated too often
list of things to implement on the wiki. If I need to find some additional
information, then I can find there just some links to mails from the mail
groups.

Then I need to read through the emails. This is not user friendly too, as I
need to click through the email tree, and if an email has multiple replies,
it is usually hard not to omit some of them, as after going into a reply, I
need to click to get to the parent mail again.

What's more, there are some things on the wiki, and when I asked about
that, it turned out that "oh, there was some discussion long time ago, that
it is not doable".

It would be also worth storing the information that someone is working on
something, so the work won't be doubled.

Personally I'd also change sending patches in emails to github pull
requests :).


... or maybe the difference is more in the data structure, the email
discussion is a tree (with a horrible interface to the archive) while in a
bug tracker, the discussion is linear, and easier to follow.


-- 
regards Szymon Lipiński

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

[Josh Berkus]

On 09/23/2015 11:18 AM, Kam Lasater wrote:
> 
> At this point not having one is borderline negligent. I'd suggest:
> Github Issues, Pivotal Tracker or Redmine (probably in that order).
> There are tens to hundreds of other great ones out there, I'm sure one
> of them would also work.

First, understand that the Postgres project was created before bug
trackers existed. And people are very slow to change their habits,
especially since not having a bug tracker was actually a benefit up
until around 2005.  It's not anymore, but I'm sure people will argue
with my statement on that.

We have to use something OSS; open source projects depending on
closed-source infra is bad news.  Out of what's available, I'd actually
choose Bugzilla; as much as BZ frustrates the heck out of me at times,
it's the only OSS tracker that's at all sophisticated.

The alternative would be someone building a sophisticated system on top
of RequestTracker, which would also let us have tight mailing list
integration given RT's email-driven model.  However, that would require
someone with the time to build a custom workflow system and web UI on
top of RT.  It's quite possible that Best Practical would be willing to
help here.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


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

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

[Jim Nasby]

On 9/22/15 8:01 PM, Peter Geoghegan wrote:

I'm doubtful that this had anything to do with MaxAllocSize. You'd
certainly need a lot of bloat to be affected by that in any way. I
wonder how high pg_stat_statements.max was set to on this system, and
how long each query text was on average.


max was set to 1. I don't know about average query text size, but 
the command that was causing the error was a very large number of 
individual INSERT ... VALUES statements all in one command.


The machine had plenty of free memory and no ulimit, so I don't see how 
this could have been anything but MaxAllocSize, unless there's some 
other failure mode in malloc I don't know about.

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.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] No Issue Tracker - Say it Ain't So!

[Stephen Frost]

Kam,

* Kam Lasater (c...@seekayel.com) wrote:
> > ... The above-referenced individuals
> > would be the bug tracking system curators, of course.  Unless it's got
> > serious technical issues, the infrastructure team will do our best to
> > support the choice.  On the other hand, some of us would likely be
> > involved in bug curation also.
> 
> Stephen,
> 
> In digging around more I found this wiki page that seems to be the
> closest thing to a BT: https://wiki.postgresql.org/wiki/Todo
> 
> Is the curation already being done? If the contents of that wiki page
> were injected into a BT, would that be enough of a start?

If you look at what happens on the -bugs mailing list, you'll see very
quickly that a bug tracker and the Todo wiki page are very different.
Perhaps the Todo could be squeezed into a bug tracker, but many of the
items on there might well end up as 'wontfix' (to use the debbugs
lingo).

That is certainly not the same curation as what a BT would require.

Thanks!

Stephen


signature.asc
Description: Digital signature

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

[Stephen Frost]

* Josh Berkus (j...@agliodbs.com) wrote:
> On 09/23/2015 11:18 AM, Kam Lasater wrote:
> > At this point not having one is borderline negligent. I'd suggest:
> > Github Issues, Pivotal Tracker or Redmine (probably in that order).
> > There are tens to hundreds of other great ones out there, I'm sure one
> > of them would also work.
> 
> First, understand that the Postgres project was created before bug
> trackers existed. And people are very slow to change their habits,
> especially since not having a bug tracker was actually a benefit up
> until around 2005.  It's not anymore, but I'm sure people will argue
> with my statement on that.
> 
> We have to use something OSS; open source projects depending on
> closed-source infra is bad news.  Out of what's available, I'd actually
> choose Bugzilla; as much as BZ frustrates the heck out of me at times,
> it's the only OSS tracker that's at all sophisticated.
> 
> The alternative would be someone building a sophisticated system on top
> of RequestTracker, which would also let us have tight mailing list
> integration given RT's email-driven model.  However, that would require
> someone with the time to build a custom workflow system and web UI on
> top of RT.  It's quite possible that Best Practical would be willing to
> help here.

Yeah, even if we got past the "do we want a bug tracker?" question, any
project would probably end up stalling indefinitely on "well then, which
one?"

In the end, we should probably just throw something up based on whatever
the folks who are going to be actually maintaining it want and call it a
'beta' and see what happens with it.  The above-referenced individuals
would be the bug tracking system curators, of course.  Unless it's got
serious technical issues, the infrastructure team will do our best to
support the choice.  On the other hand, some of us would likely be
involved in bug curation also.

Thanks!

Stephen


signature.asc
Description: Digital signature

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

[Alvaro Herrera]

Joshua D. Drake wrote:

> Until that happens asking anyone to put resources into this idea is just not
> worth it.

I wonder if you still have this conversation archived:

Date: Thu, 10 May 2007 11:30:55 -0400
From: Andrew Dunstan 
To: "Joshua D. Drake", Magnus Hagander, Alvaro Herrera, Robert Treat, Lukas 
Smith, Andrew Sullivan, David Fetter
Subject: tracker

There was some very good input there -- it would be a shame to have to
repeat that all over again.  Hey, it's only been 8 years ...

-- 
Á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] clearing opfuncid vs. parallel query

[Tom Lane]

Robert Haas  writes:
> readfuncs.c deliberately ignores any opfuncid read in for an OpExpr,
> DistinctExpr, NullIfExpr, or ScalarArrayOpExpr, instead resetting the
> value in the newly-created node to InvalidOid.  This is because it
> assumes we're reading the node from a pg_node_tree column in some
> system catalog, and if in the future we wanted to allow an ALTER
> OPERATOR command to change the pg_proc mapping, then the opfuncid
> could change.  We'd want to look it up again rather than using the
> previous value.

Right, but considering that nobody has even thought about implementing
such a command in the past twenty years, maybe we should just change
the behavior of those read routines?  I've wondered for some time why
we don't just insist on the parser filling those nodes fully to begin
with, and get rid of the notion that assorted random places should
be expected to fix them up later.  This is one of the behaviors that
would have to change to support such a simplification.

> ... The attached
> patch does by adding an "int flags" field to the relevant read
> routines.

Ick.  TBH, this is just taking a bad design and putting another one
on top.

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] No Issue Tracker - Say it Ain't So!

[Jeff Janes]

On Wed, Sep 23, 2015 at 11:43 AM, Robert Haas  wrote:

> On Wed, Sep 23, 2015 at 2:30 PM, Kam Lasater  wrote:
> > Thanks for the suggestion. However, an issue tracker is not a
> > replacement for mailing list(s) and vice versa. They are both
> > necessary for success.
>
> I venture to say that we are succeeding as it is, although of course
> we might have more success if we did some things better, including
> this.


For whatever it is worth, one of the frustrations I've had with projects
(other than PostgreSQL) of which I am a casual users is that reporting a
single bug meant signing up for yet another account on yet another site and
learning yet another bug tracking system.

I think the email based system is more friendly for the casual user.  You
don't even have to sign up for the bugs mail list as long as people keep
you CC'ed.  I don't think that having a tracker just for the sake of having
one is going to attract new contributors.

I'd rather, say, put some more work into cleaning the kruft out of the
To-Do list, then put that effort into migrating the kruft to a fancier
filing cabinet.

Cheers,

Jeff

[HACKERS] Inconsistency in Output function of MergeJoin

[Amit Kapila]

While working on read functions for plan nodes (required for
parallelism), it has been observed [1] by KaiGai and separately
by me that function _outMergeJoin(), appends boolean in a
slightly different way as compare to other out functions like
_outSort().  Is there a reason of doing so which is is not apparent
from code or comments?


Attached patch makes _outMergeJoin() consistent with other _out
functions which appends boolean to string.



[1] -
http://www.postgresql.org/message-id/9a28c8860f777e439aa12e8aea7694f801149...@bpxm15gp.gisp.nec.co.jp

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


fix_outMergeJoin_inconsistency_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] Support for N synchronous standby servers - take 2

[Amir Rohan]

> Sent: Thursday, September 24, 2015 at 3:11 AM
> 
> From: "Tom Lane" 
> Robert Haas  writes:
> > Well, I think that if we create our own mini-language, it may well be
> > possible to make the configuration for this compact enough to fit on
> > one line. If we use JSON, I think there's zap chance of that. But...
> > that's just what *I* think.
>> 

I've implemented a parser that reads you mini-language and dumps a JSON
equivalent. Once you start naming groups the line fills up quite quickly,
and on the other hands the JSON is verbose and fiddely.
But implementing a mechanism that can be used by other features in
the future seems the deciding factor here, rather then the brevity of a 
bespoke mini-language.

> 
> <...> we're best off avoiding the challenges of dealing with multi-line 
> postgresql.conf entries.
> 
> And I'm really not much in favor of a separate file; if we go that way
> then we're going to have to reinvent a huge amount of infrastructure
> that already exists for GUCs.
> 
> regards, tom lane

Adding support for JSON objects (or some other kind of composite data type) 
to the .conf parser would negate the need for one, and would also solve the
problem being discussed for future cases.
I don't know whether that would break some tooling you care about, 
but if there's interest, I can probably do some of that work.


-- 
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] Parallel Seq Scan

[Robert Haas]

On Thu, Sep 3, 2015 at 6:21 AM, Amit Kapila  wrote:
> [ new patches ]

Still more review comments:

+   /* Allow space for terminating zero-byte */
+   size = add_size(size, 1);

This is pointless.  The length is already stored separately, and if it
weren't, this wouldn't be adequate anyway because a varlena can
contain NUL bytes.  It won't if it's text, but it could be bytea or
numeric or whatever.

RestoreBoundParams is broken, because it can do unaligned reads, which
will core dump on some architectures (and merely be slow on others).
If there are two or more parameters, and the first one is a varlena
with a length that is not a multiple of MAXIMUM_ALIGNOF, the second
SerializedParamExternData will be misaligned.

Also, it's pretty lame that we send the useless pointer even for a
pass-by-reference data type and then overwrite the bad pointer with a
good one a few lines later.  It would be better to design the
serialization format so that we don't send the bogus pointer over the
wire in the first place.

It's also problematic in my view that there is so much duplicated code
here. SerializedParamExternData and SerializedParamExecData are very
similar and there are large swaths of very similar code to handle each
case.  Both structures contain Datum value, Size length, bool isnull,
and Oid ptype, albeit not in the same order for some reason.  The only
difference is that SerializedParamExternData contains uint16 pflags
and SerializedParamExecData contains int paramid.  I think we need to
refactor this code to get rid of all this duplication.  I suggest that
we decide to represent a datum here in a uniform fashion, perhaps like
this:

First, store a 4-byte header word.  If this is -2, the value is NULL
and no data follows.  If it's -1, the value is pass-by-value and
sizeof(Datum) bytes follow.  If it's >0, the value is
pass-by-reference and the value gives the number of following bytes
that should be copied into a brand-new palloc'd chunk.

Using a format like this, we can serialize and restore datums in
various contexts - including bind and exec params - without having to
rewrite the code each time.  For example, for param extern data, you
can dump an array of all the ptypes and paramids and then follow it
with all of the params one after another.  For param exec data, you
can dump an array of all the ptypes and paramids and then follow it
with the values one after another.  The code that reads and writes the
datums in both cases can be the same.  If we need to send datums in
other contexts, we can use the same code for it.

The attached patch - which I even tested! - shows what I have in mind.
It can save and restore the ParamListInfo (bind parameters).  I
haven't tried to adapt it to the exec parameters because I don't quite
understand what you are doing there yet, but you can see that the
datum-serialization logic is separated from the stuff that knows about
the details of ParamListInfo, so datumSerialize() should be reusable
for other purposes.  This also doesn't have the other problems
mentioned above.

Thoughts?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/src/backend/nodes/params.c b/src/backend/nodes/params.c
index fb803f8..d093263 100644
--- a/src/backend/nodes/params.c
+++ b/src/backend/nodes/params.c
@@ -16,6 +16,7 @@
 #include "postgres.h"
 
 #include "nodes/params.h"
+#include "storage/shmem.h"
 #include "utils/datum.h"
 #include "utils/lsyscache.h"
 
@@ -73,3 +74,157 @@ copyParamList(ParamListInfo from)
 
 	return retval;
 }
+
+/*
+ * Estimate the amount of space required to serialize a ParamListInfo.
+ */
+Size
+EstimateParamListSpace(ParamListInfo paramLI)
+{
+	int		i;
+	Size	sz = sizeof(int);
+
+	if (paramLI == NULL || paramLI->numParams <= 0)
+		return sz;
+
+	for (i = 0; i < paramLI->numParams; i++)
+	{
+		ParamExternData *prm = >params[i];
+		int16		typLen;
+		bool		typByVal;
+
+		/* give hook a chance in case parameter is dynamic */
+		if (!OidIsValid(prm->ptype) && paramLI->paramFetch != NULL)
+			(*paramLI->paramFetch) (paramLI, i + 1);
+
+		sz = add_size(sz, sizeof(Oid));			/* space for type OID */
+		sz = add_size(sz, sizeof(uint16));		/* space for pflags */
+
+		/* space for datum/isnull */
+		if (OidIsValid(prm->ptype))
+			get_typlenbyval(prm->ptype, , );
+		else
+		{
+			/* If no type OID, assume by-value, like copyParamList does. */
+			typLen = sizeof(Datum);
+			typByVal = true;
+		}
+		sz = add_size(sz,
+			datumEstimateSpace(prm->value, prm->isnull, typByVal, typLen));
+	}
+
+	return sz;
+}
+
+/*
+ * Serialize a paramListInfo structure into caller-provided storage.
+ *
+ * We write the number of parameters first, as a 4-byte integer, and then
+ * write details for each parameter in turn.  The details for each parameter
+ * consist of a 4-byte type OID, 2 bytes of flags, and then the datum as
+ * serialized by datumSerialize().  The 

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

[Alvaro Herrera]

Josh Berkus wrote:

> When we discussed this 8 years ago, Debian said debbugs wasn't ready for
> anyone else to use.  Has that changed?

Emacs uses debbugs now, so there's at least one non-Debian user.

-- 
Á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] Support for N synchronous standby servers - take 2

[Robert Haas]

On Wed, Sep 23, 2015 at 12:11 AM, Amir Rohan  wrote:
> It seems like:
> 1) There's a need to support structured data in configuration for future
> needs as well, it isn't specific to this feature.
> 2) There should/must be a better way to validate configuration then
> to restarting the server in search of syntax errors.
>
> Creating a whole new configuration file for just one feature *and* in a
> different
> format seems suboptimal.  What happens when the next 20 features need
> structured
> config data, where does that go? will there be additional JSON config files
> *and* perhaps
> new mini-language values in .conf as development continues?  How many
> dedicated
> configuration files is too many?

Well, I think that if we create our own mini-language, it may well be
possible to make the configuration for this compact enough to fit on
one line.  If we use JSON, I think there's zap chance of that.  But...
that's just what *I* think.

-- 
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] clearing opfuncid vs. parallel query

[Robert Haas]

On Wed, Sep 23, 2015 at 5:39 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> But if we're sure we don't want to support that, changing the behavior
>> of the read routines would be fine with me, too.  It would even save a
>> few cycles.  Would you also want to rip out the stuff that fixes up
>> opfuncid as dead code?  I assume yes, but sometimes I assume things
>> that are false.
>
> Yeah, though I think of that as a longer-term issue, ie we could clean it
> up sometime later.

So, you're thinking of something as simple as the attached?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index ef88209..08519ed 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -665,17 +665,6 @@ _readOpExpr(void)
 
 	READ_OID_FIELD(opno);
 	READ_OID_FIELD(opfuncid);
-
-	/*
-	 * The opfuncid is stored in the textual format primarily for debugging
-	 * and documentation reasons.  We want to always read it as zero to force
-	 * it to be re-looked-up in the pg_operator entry.  This ensures that
-	 * stored rules don't have hidden dependencies on operators' functions.
-	 * (We don't currently support an ALTER OPERATOR command, but might
-	 * someday.)
-	 */
-	local_node->opfuncid = InvalidOid;
-
 	READ_OID_FIELD(opresulttype);
 	READ_BOOL_FIELD(opretset);
 	READ_OID_FIELD(opcollid);
@@ -696,17 +685,6 @@ _readDistinctExpr(void)
 
 	READ_OID_FIELD(opno);
 	READ_OID_FIELD(opfuncid);
-
-	/*
-	 * The opfuncid is stored in the textual format primarily for debugging
-	 * and documentation reasons.  We want to always read it as zero to force
-	 * it to be re-looked-up in the pg_operator entry.  This ensures that
-	 * stored rules don't have hidden dependencies on operators' functions.
-	 * (We don't currently support an ALTER OPERATOR command, but might
-	 * someday.)
-	 */
-	local_node->opfuncid = InvalidOid;
-
 	READ_OID_FIELD(opresulttype);
 	READ_BOOL_FIELD(opretset);
 	READ_OID_FIELD(opcollid);
@@ -727,17 +705,6 @@ _readNullIfExpr(void)
 
 	READ_OID_FIELD(opno);
 	READ_OID_FIELD(opfuncid);
-
-	/*
-	 * The opfuncid is stored in the textual format primarily for debugging
-	 * and documentation reasons.  We want to always read it as zero to force
-	 * it to be re-looked-up in the pg_operator entry.  This ensures that
-	 * stored rules don't have hidden dependencies on operators' functions.
-	 * (We don't currently support an ALTER OPERATOR command, but might
-	 * someday.)
-	 */
-	local_node->opfuncid = InvalidOid;
-
 	READ_OID_FIELD(opresulttype);
 	READ_BOOL_FIELD(opretset);
 	READ_OID_FIELD(opcollid);
@@ -758,17 +725,6 @@ _readScalarArrayOpExpr(void)
 
 	READ_OID_FIELD(opno);
 	READ_OID_FIELD(opfuncid);
-
-	/*
-	 * The opfuncid is stored in the textual format primarily for debugging
-	 * and documentation reasons.  We want to always read it as zero to force
-	 * it to be re-looked-up in the pg_operator entry.  This ensures that
-	 * stored rules don't have hidden dependencies on operators' functions.
-	 * (We don't currently support an ALTER OPERATOR command, but might
-	 * someday.)
-	 */
-	local_node->opfuncid = InvalidOid;
-
 	READ_BOOL_FIELD(useOr);
 	READ_OID_FIELD(inputcollid);
 	READ_NODE_FIELD(args);

-- 
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] clearing opfuncid vs. parallel query

[Tom Lane]

Robert Haas  writes:
> On Wed, Sep 23, 2015 at 5:39 PM, Tom Lane  wrote:
>> Yeah, though I think of that as a longer-term issue, ie we could clean it
>> up sometime later.

> So, you're thinking of something as simple as the attached?

Right.  I'll make a mental to-do to see about getting rid of set_opfuncid
later.

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] Bad row estimation with indexed func returning bool

[Tom Lane]

I wrote:
> However, in the case at hand, the complaint basically is why aren't we
> treating the boolean function expression like a boolean variable, and
> looking to see if there are stats available for it, like this other
> bit in clause_selectivity:

> /*
>  * A Var at the top of a clause must be a bool Var. This is
>  * equivalent to the clause reln.attribute = 't', so we compute
>  * the selectivity as if that is what we have.
>  */
> s1 = restriction_selectivity(root,
>  BooleanEqualOperator,
>  list_make2(var,
> makeBoolConst(true,
>   false)),
>  InvalidOid,
>  varRelid);

> Indeed you could argue that this ought to be the fallback behavior for
> *any* unhandled case, not just function expressions.  Not sure if we'd
> need to restrict it to single-relation expressions or not.

> The implication of doing it like this would be that the default estimate
> in the absence of any matching stats would be 0.5 (since eqsel defaults
> to 1/ndistinct, and get_variable_numdistinct will report 2.0 for any
> boolean-type expression it has no stats for).  That's not a huge change
> from the existing 0.333 estimate, which seems pretty unprincipled
> anyway ... but it would probably be enough to annoy people if we did it in
> stable branches.  So I'd be inclined to propose changing this in HEAD and
> maybe 9.5, but not further back.  (For non-function expressions, 0.5 is
> the default already, so those would not change behavior.)

I experimented with the attached patch.  The change in the default
estimate for a function results in just one change in the standard
regression test results, so far as I can find.

Comments, objections?

regards, tom lane

diff --git a/src/backend/optimizer/path/clausesel.c b/src/backend/optimizer/path/clausesel.c
index dcac1c1..c862b70 100644
*** a/src/backend/optimizer/path/clausesel.c
--- b/src/backend/optimizer/path/clausesel.c
***
*** 14,20 
   */
  #include "postgres.h"
  
- #include "catalog/pg_operator.h"
  #include "nodes/makefuncs.h"
  #include "optimizer/clauses.h"
  #include "optimizer/cost.h"
--- 14,19 
*** clause_selectivity(PlannerInfo *root,
*** 568,585 
  		if (var->varlevelsup == 0 &&
  			(varRelid == 0 || varRelid == (int) var->varno))
  		{
! 			/*
! 			 * A Var at the top of a clause must be a bool Var. This is
! 			 * equivalent to the clause reln.attribute = 't', so we compute
! 			 * the selectivity as if that is what we have.
! 			 */
! 			s1 = restriction_selectivity(root,
! 		 BooleanEqualOperator,
! 		 list_make2(var,
! 	makeBoolConst(true,
!   false)),
! 		 InvalidOid,
! 		 varRelid);
  		}
  	}
  	else if (IsA(clause, Const))
--- 567,574 
  		if (var->varlevelsup == 0 &&
  			(varRelid == 0 || varRelid == (int) var->varno))
  		{
! 			/* Use the restriction selectivity function for a bool Var */
! 			s1 = boolvarsel(root, (Node *) var, varRelid);
  		}
  	}
  	else if (IsA(clause, Const))
*** clause_selectivity(PlannerInfo *root,
*** 680,704 
  		if (IsA(clause, DistinctExpr))
  			s1 = 1.0 - s1;
  	}
- 	else if (is_funcclause(clause))
- 	{
- 		/*
- 		 * This is not an operator, so we guess at the selectivity. THIS IS A
- 		 * HACK TO GET V4 OUT THE DOOR.  FUNCS SHOULD BE ABLE TO HAVE
- 		 * SELECTIVITIES THEMSELVES.   -- JMH 7/9/92
- 		 */
- 		s1 = (Selectivity) 0.333;
- 	}
- #ifdef NOT_USED
- 	else if (IsA(clause, SubPlan) ||
- 			 IsA(clause, AlternativeSubPlan))
- 	{
- 		/*
- 		 * Just for the moment! FIX ME! - vadim 02/04/98
- 		 */
- 		s1 = (Selectivity) 0.5;
- 	}
- #endif
  	else if (IsA(clause, ScalarArrayOpExpr))
  	{
  		/* Use node specific selectivity calculation function */
--- 669,674 
*** clause_selectivity(PlannerInfo *root,
*** 766,771 
--- 736,752 
  jointype,
  sjinfo);
  	}
+ 	else
+ 	{
+ 		/*
+ 		 * For anything else, see if we can consider it as a boolean variable.
+ 		 * This only works if it's an immutable expression in Vars of a single
+ 		 * relation; but there's no point in us checking that here because
+ 		 * boolvarsel() will do it internally, and return a suitable default
+ 		 * selectivity (0.5) if not.
+ 		 */
+ 		s1 = boolvarsel(root, clause, varRelid);
+ 	}
  
  	/* Cache the result if possible */
  	if (cacheable)
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 72bc502..a643c6f 100644
*** a/src/backend/utils/adt/selfuncs.c
--- b/src/backend/utils/adt/selfuncs.c
***
*** 105,110 
--- 105,111 
  #include "access/sysattr.h"
  #include 

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

[Josh Berkus]

On 09/23/2015 03:05 PM, Jim Nasby wrote:
> On 9/23/15 3:12 PM, Thomas Kellerer wrote:
>> They also support Postgres as their backend (and you do find hints
>> here and
>> there
>> that it is the recommended open source DBMS for them - but they don't
>> explicitly state it like that). We are using Jira at the company I
>> work for
>> and
>> all Jira installations run on Postgres there.
> 
> I'll second Jira as well. It's the only issue tracker I've seen that you
> can actually use for multiple different things without it becoming a
> mess. IE: it could track Postgres bugs, infrastructure issues, and the
> TODO list if we wanted, allow issues to reference each other
> intelligently, yet still keep them as 3 separate bodies.

Speaking as someone who uses Jira for commericial work, I'm -1 on them.
 I simply don't find Jira to be superior to OSS BT systems, and inferior
in several ways (like that you can't have more than one person assigned
to a bug).  And email integration for Jira is nonexistant.

When we discussed this 8 years ago, Debian said debbugs wasn't ready for
anyone else to use.  Has that changed?

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


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

Re: [HACKERS] Support for N synchronous standby servers - take 2

[Tom Lane]

Robert Haas  writes:
> Well, I think that if we create our own mini-language, it may well be
> possible to make the configuration for this compact enough to fit on
> one line.  If we use JSON, I think there's zap chance of that.  But...
> that's just what *I* think.

Well, that depends on what you think the typical-case complexity is
and on how long a line will fit in your editor window ;-).

I think that we can't make much progress on this argument without a pretty
concrete idea of what typical and worst-case configurations would look
like.  Would someone like to put forward examples?  Then we could try them
in any specific syntax that's suggested and see how verbose it gets.

FWIW, I tend to agree that if we think common cases can be held to,
say, a hundred or two hundred characters, that we're best off avoiding
the challenges of dealing with multi-line postgresql.conf entries.
And I'm really not much in favor of a separate file; if we go that way
then we're going to have to reinvent a huge amount of infrastructure
that already exists for GUCs.

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] No Issue Tracker - Say it Ain't So!

[Thomas Munro]

On Thu, Sep 24, 2015 at 11:33 AM, Josh Berkus  wrote:
> On 09/23/2015 03:05 PM, Jim Nasby wrote:
>> On 9/23/15 3:12 PM, Thomas Kellerer wrote:
>>> They also support Postgres as their backend (and you do find hints
>>> here and
>>> there
>>> that it is the recommended open source DBMS for them - but they don't
>>> explicitly state it like that). We are using Jira at the company I
>>> work for
>>> and
>>> all Jira installations run on Postgres there.
>>
>> I'll second Jira as well. It's the only issue tracker I've seen that you
>> can actually use for multiple different things without it becoming a
>> mess. IE: it could track Postgres bugs, infrastructure issues, and the
>> TODO list if we wanted, allow issues to reference each other
>> intelligently, yet still keep them as 3 separate bodies.
>
> Speaking as someone who uses Jira for commericial work, I'm -1 on them.
>  I simply don't find Jira to be superior to OSS BT systems, and inferior
> in several ways (like that you can't have more than one person assigned
> to a bug).  And email integration for Jira is nonexistant.
>
> When we discussed this 8 years ago, Debian said debbugs wasn't ready for
> anyone else to use.  Has that changed?

Do you think it would make any sense to consider evolving what we have
already?  At the moment, we have a bug form, and when you submit it it
does this (if I'm looking at the right thing, please correct me if I'm
not):

http://git.postgresql.org/gitweb/?p=pgweb.git;a=blob;f=pgweb/misc/views.py

That is, the database interaction is limited to using a SEQUENCE to
generate a new bug ID, and then an email is sent to pgsql-bugs.  What
if we also created a bug record for that ID to track its status, and
allowed anyone with a community account to edit the bug record and
change the status?  There could be a simple page that lets you see and
search for bugs, with a link to the flat mail archive thread where
discussion is held.  All actual discussion would continue on mailing
lists.  That would be similar to the commitfest.

I suppose some forms of cross-reference would also be useful: when
viewing the bug's page, you might want to see any commitfest items or
pgsql-committers messages that relate to that bug.  Perhaps we could
automatically create those links when bug IDs are recognised in those
messages, so that no extra workflow/maintenance is required in
straightforward cases.  To continue that line of thinking it would
also be possible for bug statuses to be changed when certain words are
spotted in either commit messages (which doesn't have to be a commit
hook, it could be taken from pgsql-committers messages) or pgsql-bugs
messages.

Cf github commit message parsing:
https://help.github.com/articles/closing-issues-via-commit-messages/

-- 
Thomas Munro
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] No Issue Tracker - Say it Ain't So!

[Joe Conway]

On 09/23/2015 05:21 PM, Thomas Munro wrote:
> Do you think it would make any sense to consider evolving what we have
> already?  At the moment, we have a bug form, and when you submit it it
> does this (if I'm looking at the right thing, please correct me if I'm
> not):
> 
> http://git.postgresql.org/gitweb/?p=pgweb.git;a=blob;f=pgweb/misc/views.py
> 
> That is, the database interaction is limited to using a SEQUENCE to
> generate a new bug ID, and then an email is sent to pgsql-bugs.  What
> if we also created a bug record for that ID to track its status, and
> allowed anyone with a community account to edit the bug record and
> change the status?  There could be a simple page that lets you see and
> search for bugs, with a link to the flat mail archive thread where
> discussion is held.  All actual discussion would continue on mailing
> lists.  That would be similar to the commitfest.
> 
> I suppose some forms of cross-reference would also be useful: when
> viewing the bug's page, you might want to see any commitfest items or
> pgsql-committers messages that relate to that bug.  Perhaps we could
> automatically create those links when bug IDs are recognised in those
> messages, so that no extra workflow/maintenance is required in


It would be nice if you could essentially promote a bug into a
commitfest item, maybe through a status change.


> straightforward cases.  To continue that line of thinking it would
> also be possible for bug statuses to be changed when certain words are
> spotted in either commit messages (which doesn't have to be a commit
> hook, it could be taken from pgsql-committers messages) or pgsql-bugs
> messages.
> 
> Cf github commit message parsing:
> https://help.github.com/articles/closing-issues-via-commit-messages/

I was thinking along the same lines. If we could paste a bug reference
number into the commit message and have that change the bug status it
would go a long way to making this workable I think.

Joe

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



signature.asc
Description: OpenPGP digital signature

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

[Thomas Munro]

On Thu, Sep 24, 2015 at 1:31 PM, Joe Conway  wrote:
> On 09/23/2015 05:21 PM, Thomas Munro wrote:
>> Do you think it would make any sense to consider evolving what we have
>> already?  At the moment, we have a bug form, and when you submit it it
>> does this (if I'm looking at the right thing, please correct me if I'm
>> not):
>>
>> http://git.postgresql.org/gitweb/?p=pgweb.git;a=blob;f=pgweb/misc/views.py
>>
>> That is, the database interaction is limited to using a SEQUENCE to
>> generate a new bug ID, and then an email is sent to pgsql-bugs.  What
>> if we also created a bug record for that ID to track its status, and
>> allowed anyone with a community account to edit the bug record and
>> change the status?  There could be a simple page that lets you see and
>> search for bugs, with a link to the flat mail archive thread where
>> discussion is held.  All actual discussion would continue on mailing
>> lists.  That would be similar to the commitfest.
>>
>> I suppose some forms of cross-reference would also be useful: when
>> viewing the bug's page, you might want to see any commitfest items or
>> pgsql-committers messages that relate to that bug.  Perhaps we could
>> automatically create those links when bug IDs are recognised in those
>> messages, so that no extra workflow/maintenance is required in
>
>
> It would be nice if you could essentially promote a bug into a
> commitfest item, maybe through a status change.
>
>
>> straightforward cases.  To continue that line of thinking it would
>> also be possible for bug statuses to be changed when certain words are
>> spotted in either commit messages (which doesn't have to be a commit
>> hook, it could be taken from pgsql-committers messages) or pgsql-bugs
>> messages.
>>
>> Cf github commit message parsing:
>> https://help.github.com/articles/closing-issues-via-commit-messages/
>
> I was thinking along the same lines. If we could paste a bug reference
> number into the commit message and have that change the bug status it
> would go a long way to making this workable I think.

The two most common interactions could go something like this:

1.  User enters bug report via form, creating an issue in NEW state
and creating a pgsql-bugs thread.  Someone responds by email that this
is expected behaviour, not a bug, not worth fixing or not a Postgres
issue etc using special trigger words.  The state is automatically
switched to WORKS_AS_DESIGNED or WONT_FIX.  No need to touch the web
interface: the only change from today's workflow is awareness of the
right wording to trigger the state change.

2.  User enters bug report via form, creating issue #1234 in NEW
state.   Someone responds by email to acknowledge that that may indeed
be an issue, and any response to an issue in NEW state that doesn't
reject it switches it to UNDER_DISCUSSION.  Maybe if a commitfest item
references the same thread (or somehow references the issue number?)
its state is changed to IN_COMMITFEST, or maybe as you say there could
be a way to generate the commitfest item from the issue, not sure
about that.  Eventually a commit log message says "Fixes bug #1234"
and the state automatically goes to FIXED.

Other interactions (curation of unresolved issues, reopening disputed
issues, resolving issues where the committer or responder forgot to
use the right magic words, etc etc) could be done via the web
interface which would initially have only a couple of pages: one for
paging through issues in different states and one for viewing/editing
them.  (Obviously you could go further and deal with assigning issues
to people, and adding more states etc etc).

I don't know much about pgweb and friends but it seems like we already
have a bunch of Python machinery processing all mailing list traffic,
a database and a webapp doing something pretty closely related.  How
hard would it be to teach it to track issues this way, building on the
existing infrastructure, compared to rolling out a new separate
product, and could the result be better integrated?

-- 
Thomas Munro
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] Postgres - BDR issue

[Craig Ringer]

On 22 September 2015 at 23:52, Rahul Goel  wrote:
> Hi
>
> I am facing the below issue in setting up BDR:

Hi.

Please direct questions about using and setting up BDR to
pgsql-general, not pgsql-hackers.

Thanks.

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


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

Re: [HACKERS] PATCH: use foreign keys to improve join estimates v1

[David Rowley]

On 23 September 2015 at 17:11, David Rowley 
wrote:

> find_foreign_key_clauses() should look for the longest match and return a
> Bitmap set of the list indexes to the caller.
> It might be possible to fool the longest match logic by duplicating
> clauses, e.g. a1 = b1 AND a1 = b1 and a1 = b1 AND a2 = b2 AND a3 = b3, but
> I can't imagine that matters, but if it did, we could code it to be smart
> enough to see through that.
>

I took a bash at implementing what I described, and I've ended up with the
attached.

git diff -stat gives me:

 src/backend/optimizer/path/costsize.c | 717
--
 src/backend/optimizer/plan/analyzejoins.c |   1 +
 src/backend/optimizer/util/plancat.c  | 112 +++--
 3 files changed, 228 insertions(+), 602 deletions(-)

So it's removed quite a bit of code. I also discovered that: 1.0 /
Max(rel->tuples,1.0) is no good for SEMI and ANTI joins. I've coded around
this in the attached, but I'm not certain it's the best way of doing things.

I thought that it might be possible to add some regression test around
this, if we simply just find a plan the uses a nested loop due to
underestimation of matching rows, and then make sure that it no longer uses
a nested loop when the foreign key is added. I've not yet done this in the
attached.

Patched attached in delta form and complete form.

I still need to perform more analysis on the plancat.c changes.

Have I made any changes that you disagree with?

Regards

David Rowley

--
 David Rowley   http://www.2ndQuadrant.com/

 PostgreSQL Development, 24x7 Support, Training & Services


estimation-with-fkeys-v2_david.patch
Description: Binary data


estimation-with-fkeys-v2_david_delta.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