Re: [HACKERS] Remaining 9.5 open items

[Etsuro Fujita]

On 2015/12/11 1:18, Robert Haas wrote:

On Wed, Dec 9, 2015 at 2:52 AM, Etsuro Fujita
 wrote:

Thank you for committing the patch!

Sorry, I overlooked a typo in docs: s/more that one/more than one/ Please
find attached a patch.



Committed, thanks.


Thanks!

Best regards,
Etsuro Fujita




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

Re: [HACKERS] Remaining 9.5 open items

[Stephen Frost]

* Robert Haas (robertmh...@gmail.com) wrote:
> On Fri, Dec 4, 2015 at 3:22 PM, Stephen Frost  wrote:
> > * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> >> Stephen Frost wrote:
> >> > Still, I'll get a patch worked up for it and then we can discuss the
> >> > merits of that patch going in to 9.5 now versus just into HEAD.
> >>
> >> Cool.
> >
> > While working on the DROP OWNED BY patch, and part of what took me a bit
> > longer with it, I came to the realiziation that ALTER POLICY wasn't
> > handling dependencies quite right.  All of the policy's dependencies
> > would be dropped, but then only those objects referred to in the ALTER
> > POLICY command would have dependencies recreated for them.
> >
> > The attached patch fixes that (using the same approach that I used in
> > the DROP OWNED BY patch).
> >
> > Comments welcome, as always.
> >
> > I'll plan to apply these two patches in a couple of days.
> 
> It's been a week?

I've pushed these now.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Remaining 9.5 open items

[Robert Haas]

On Fri, Dec 4, 2015 at 3:22 PM, Stephen Frost  wrote:
> * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
>> Stephen Frost wrote:
>> > Still, I'll get a patch worked up for it and then we can discuss the
>> > merits of that patch going in to 9.5 now versus just into HEAD.
>>
>> Cool.
>
> While working on the DROP OWNED BY patch, and part of what took me a bit
> longer with it, I came to the realiziation that ALTER POLICY wasn't
> handling dependencies quite right.  All of the policy's dependencies
> would be dropped, but then only those objects referred to in the ALTER
> POLICY command would have dependencies recreated for them.
>
> The attached patch fixes that (using the same approach that I used in
> the DROP OWNED BY patch).
>
> Comments welcome, as always.
>
> I'll plan to apply these two patches in a couple of days.

It's been a week?

-- 
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] Remaining 9.5 open items

[Robert Haas]

On Wed, Dec 9, 2015 at 2:52 AM, Etsuro Fujita
 wrote:
> Thank you for committing the patch!
>
> Sorry, I overlooked a typo in docs: s/more that one/more than one/ Please
> find attached a patch.

Committed, thanks.

-- 
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] Remaining 9.5 open items

[Robert Haas]

On Thu, Dec 3, 2015 at 9:51 PM, Noah Misch  wrote:
> On Tue, Dec 01, 2015 at 11:05:47AM -0500, Robert Haas wrote:
>> On Mon, Nov 30, 2015 at 2:43 PM, Tom Lane  wrote:
>> > * Foreign join pushdown vs EvalPlanQual
>> >
>> > Is this fixed by 5fc4c26db?  If not, what remains to do?
>>
>> Unfortunately, no.  That commit allows FDWs to do proper EPQ handling
>> for plain table scans, but it proves to be inadequate for EPQ handling
>> for joins. Solving that problem will require another patch, and,
>> modulo a bunch of cosmetic issues, I'm reasonably happy with KaiGai
>> Kohei's latest submission.  I'll respond in more detail on that
>> thread, but the question I want to raise here is: do we want to
>> back-patch those changes to 9.5 at this late date?
>
> Yes.  If 9.5 added a bad interface, better to fix the interface even now than
> to live with the bad one.

OK, I've pushed the latest patch for that issue to master and 9.5.
I'm not completely positive we've killed this problem dead, but I hope
so.

-- 
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] Remaining 9.5 open items

[Etsuro Fujita]

On 2015/12/09 2:56, Robert Haas wrote:

On Thu, Dec 3, 2015 at 9:51 PM, Noah Misch  wrote:

On Tue, Dec 01, 2015 at 11:05:47AM -0500, Robert Haas wrote:

On Mon, Nov 30, 2015 at 2:43 PM, Tom Lane  wrote:

* Foreign join pushdown vs EvalPlanQual

Is this fixed by 5fc4c26db?  If not, what remains to do?



Unfortunately, no.  That commit allows FDWs to do proper EPQ handling
for plain table scans, but it proves to be inadequate for EPQ handling
for joins. Solving that problem will require another patch, and,
modulo a bunch of cosmetic issues, I'm reasonably happy with KaiGai
Kohei's latest submission.  I'll respond in more detail on that
thread, but the question I want to raise here is: do we want to
back-patch those changes to 9.5 at this late date?



Yes.  If 9.5 added a bad interface, better to fix the interface even now than
to live with the bad one.



OK, I've pushed the latest patch for that issue to master and 9.5.
I'm not completely positive we've killed this problem dead, but I hope
so.


Thank you for committing the patch!

Sorry, I overlooked a typo in docs: s/more that one/more than one/ 
Please find attached a patch.


Best regards,
Etsuro Fujita
diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml
index 0090e24..dc2d890 100644
--- a/doc/src/sgml/fdwhandler.sgml
+++ b/doc/src/sgml/fdwhandler.sgml
@@ -793,7 +793,7 @@ RecheckForeignScan (ForeignScanState *node, TupleTableSlot *slot);
  ForeignScan.  When a recheck is required, this subplan
  can be executed and the resulting tuple can be stored in the slot.
  This plan need not be efficient since no base table will return more
- that one row; for example, it may implement all joins as nested loops.
+ than one row; for example, it may implement all joins as nested loops.
 

 

-- 
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] Remaining 9.5 open items

[Simon Riggs]

On 1 December 2015 at 17:05, Robert Haas  wrote:


> do we want to
> back-patch those changes to 9.5 at this late date?
>

Surely the whole point of a release process is to fix issues in the
release. If we don't ever dare put something in the release, we may as well
have released it earlier.

I'm thinking about a two stage release process...

Stage 1 - released, but with caveats and some parts marked
experimental/beta whatever

Stage 2 - released, all caveats resolved

Not sure what to call that.

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

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

Re: [HACKERS] Remaining 9.5 open items

[Tom Lane]

Simon Riggs  writes:
> On 1 December 2015 at 17:05, Robert Haas  wrote:
>> do we want to
>> back-patch those changes to 9.5 at this late date?

> Surely the whole point of a release process is to fix issues in the
> release. If we don't ever dare put something in the release, we may as well
> have released it earlier.

> I'm thinking about a two stage release process...

> Stage 1 - released, but with caveats and some parts marked
> experimental/beta whatever

> Stage 2 - released, all caveats resolved

> Not sure what to call that.

9.5beta3.  If you're saying we are not ready for an RC, then it's a beta.

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] Remaining 9.5 open items

[Simon Riggs]

On 4 December 2015 at 16:29, Tom Lane  wrote:

> Simon Riggs  writes:
> > On 1 December 2015 at 17:05, Robert Haas  wrote:
> >> do we want to
> >> back-patch those changes to 9.5 at this late date?
>
> > Surely the whole point of a release process is to fix issues in the
> > release. If we don't ever dare put something in the release, we may as
> well
> > have released it earlier.
>
> > I'm thinking about a two stage release process...
>
> > Stage 1 - released, but with caveats and some parts marked
> > experimental/beta whatever
>
> > Stage 2 - released, all caveats resolved
>
> > Not sure what to call that.
>
> 9.5beta3.  If you're saying we are not ready for an RC, then it's a beta.
>

I guess I really meant "in the future" or "next time", but perhaps that
could apply now.

The main issue is that most of these things are pretty trivial and hardly
worth delaying the release process of The World's Most Advanced Open Source
Database for; its not like the Saturn V will fail to fly.

We don't seem to put any negative weighting on delay; even the smallest
issues are enough to delay us. That's daft because we know we'll hit a raft
of bugs soon after release, we just don't know where they are yet - so
fooling ourselves that it needs to be perfect before release of 9.5.0
doesn't help anybody.

Do we think they ever launched a Saturn V that didn't have some marginal
flashing lights somewhere?

I accept there are open items. I'd like a way to indicate to people they
can start using it with a safety, apart from the listed caveats.

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

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

Re: [HACKERS] Remaining 9.5 open items

[Stephen Frost]

* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> Stephen Frost wrote:
> > * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> > > Stephen Frost wrote:
> > > 
> > > > The non-documentation question is around DROP OWNED.  We need to either
> > > > have policies dropped by DROP OWNED (well, roles removed, unless it's
> > > > the last one, in which case the policy should be dropped), or update the
> > > > documentation to reflect that they don't.  I had been thinking we'd
> > > > fix DROP OWNED to deal with the policies, but if folks feel it's too
> > > > late for that kind of a change, then we can simply document it.  I don't
> > > > believe that's unreasonable for a new feature and we can work to get it
> > > > addressed in 9.6.
> > > 
> > > DROP OWNED is documented as a mechanism to help you drop the role, so
> > > it should do whatever is needed for that.  I don't think documenting the
> > > fact that it leaves the user as part of policies is good enough.
> > 
> > We already can't take care of everything with DROP OWNED though, since
> > we can't do cross-database queries, and the overall process almost
> > certainly requires additional effort (to reassign objects, etc...), so
> > while I'd be happier if policies were handled by it, I don't think it's
> > as serious of an issue.
> 
> Yes, the documentation says to apply a combination of REASSIGN OWNED
> plus DROP OWNED to each database.  Sure, it's not a single command, but
> if you additionally put the burden that the policies must be taken care
> of separately, then the whole process is made a little worse.
> 
> > Still, I'll get a patch worked up for it and then we can discuss the
> > merits of that patch going in to 9.5 now versus just into HEAD.
> 
> Cool.
> 
> In the past, we've made a bunch of changes to DROP OWNED in order to
> deal with object types that caused errors, even in minor releases.  I
> think this is just another case of the same problem.

Patch attached for review/comment.

I noticed in passing that the role removal documentation should really
also discuss shared objects (as the DROP OWNED BY reference page does).

Thanks!

Stephen
From e3f59f57add2afbb27b68486865274db44e38ab6 Mon Sep 17 00:00:00 2001
From: Stephen Frost 
Date: Fri, 4 Dec 2015 11:00:52 -0500
Subject: [PATCH] Handle policies during DROP OWNED BY

DROP OWNED BY handled GRANT-based ACLs but was not removing roles from
policies.  Fix that by having DROP OWNED BY remove the role specified
from the list of roles the policy (or policies) apply to, or the entire
policy (or policies) if it only applied to the role specified.

As with ACLs, the DROP OWNED BY caller must have permission to modify
the policy or a WARNING is thrown and no change is made to the policy.
---
 src/backend/catalog/pg_shdepend.c |  13 ++
 src/backend/commands/policy.c | 246 ++
 src/include/commands/policy.h |   2 +
 src/test/regress/expected/rowsecurity.out |  14 ++
 src/test/regress/sql/rowsecurity.sql  |  19 +++
 5 files changed, 294 insertions(+)

diff --git a/src/backend/catalog/pg_shdepend.c b/src/backend/catalog/pg_shdepend.c
index 43076c9..eeb231b 100644
--- a/src/backend/catalog/pg_shdepend.c
+++ b/src/backend/catalog/pg_shdepend.c
@@ -50,6 +50,7 @@
 #include "commands/defrem.h"
 #include "commands/event_trigger.h"
 #include "commands/extension.h"
+#include "commands/policy.h"
 #include "commands/proclang.h"
 #include "commands/schemacmds.h"
 #include "commands/tablecmds.h"
@@ -1245,6 +1246,18 @@ shdepDropOwned(List *roleids, DropBehavior behavior)
 			sdepForm->classid,
 			sdepForm->objid);
 	break;
+case SHARED_DEPENDENCY_POLICY:
+	/* If unable to remove role from policy, remove policy. */
+	if (!RemoveRoleFromObjectPolicy(roleid,
+	sdepForm->classid,
+	sdepForm->objid))
+	{
+		obj.classId = sdepForm->classid;
+		obj.objectId = sdepForm->objid;
+		obj.objectSubId = sdepForm->objsubid;
+		add_exact_object_address(, deleteobjs);
+	}
+	break;
 case SHARED_DEPENDENCY_OWNER:
 	/* If a local object, save it for deletion below */
 	if (sdepForm->dbid == MyDatabaseId)
diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c
index 8851fe7..7bdc1a4 100644
--- a/src/backend/commands/policy.c
+++ b/src/backend/commands/policy.c
@@ -408,6 +408,252 @@ RemovePolicyById(Oid policy_id)
 }
 
 /*
+ * RemoveRoleFromObjectPolicy -
+ *	 remove a role from a policy by its OID.  If the role is not a member of
+ *	 the policy then an error is raised.  False is returned to indicate that
+ *	 the role could not be removed due to being the only role on the policy
+ *	 and therefore the entire policy should be removed.
+ *
+ * Note that a warning will be thrown and true will be returned on a
+ * permission error, as the policy should not be removed in that case.
+ *
+ * roleid - the oid of the role to remove
+ * classid - 

Re: [HACKERS] Remaining 9.5 open items

[Geoff Winkless]

On 4 December 2015 at 15:50, Simon Riggs  wrote:

> Do we think they ever launched a Saturn V that didn't have some marginal
> flashing lights somewhere?
>

​Almost certainly. They had triple-redundant systems that were certified
for correctness. You don't knowingly send rockets into space with dubious
control systems.

I accept there are open items. I'd like a way to indicate to people they
> can start using it with a safety, apart from the listed caveats.
>

Just to add my .2c worth...

​T​
hat's what betas are for.

There's an implied open-source contract​

​that anyone who wants to use a feature in the next version will invest a
little of their time ​making sure that the feature works for them in the
beta before it gets released.

The developer side of that contract is that you fix the bugs people found
in the beta before release, because otherwise next time they won't bother.

And if you start pushing out full releases that you *know *introduce bugs
that weren't in the previous release, "with caveats" or not, you end up in
a situation where people won't upgrade until the second or third point
release.

Geoff

Re: [HACKERS] Remaining 9.5 open items

[Stephen Frost]

* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> Stephen Frost wrote:
> > Still, I'll get a patch worked up for it and then we can discuss the
> > merits of that patch going in to 9.5 now versus just into HEAD.
> 
> Cool.

While working on the DROP OWNED BY patch, and part of what took me a bit
longer with it, I came to the realiziation that ALTER POLICY wasn't
handling dependencies quite right.  All of the policy's dependencies
would be dropped, but then only those objects referred to in the ALTER
POLICY command would have dependencies recreated for them.

The attached patch fixes that (using the same approach that I used in
the DROP OWNED BY patch).

Comments welcome, as always.

I'll plan to apply these two patches in a couple of days.

Thanks!

Stephen
From c3d396f0ca6b5caedb96cd3d00f15e271fda08a3 Mon Sep 17 00:00:00 2001
From: Stephen Frost 
Date: Fri, 4 Dec 2015 14:01:23 -0500
Subject: [PATCH] Handle dependencies properly in ALTER POLICY

ALTER POLICY hadn't fully considered partial policy alternation
(eg: change just the roles on the policy, or just change one of
the expressions) when rebuilding the dependencies.  Instead, it
would happily remove all dependencies which existed for the
policy and then only recreate the dependencies for the objects
referred to in the specific ALTER POLICY command.

Correct that by extracting and building the dependencies for all
objects referenced by the policy, regardless of if they were
provided as part of the ALTER POLICY command or were already in
place as part of the pre-existing policy.
---
 src/backend/commands/policy.c | 97 +++
 src/test/regress/expected/rowsecurity.out | 43 ++
 src/test/regress/sql/rowsecurity.sql  | 31 ++
 3 files changed, 171 insertions(+)

diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c
index 8851fe7..2db8125 100644
--- a/src/backend/commands/policy.c
+++ b/src/backend/commands/policy.c
@@ -766,6 +766,35 @@ AlterPolicy(AlterPolicyStmt *stmt)
 		replaces[Anum_pg_policy_polroles - 1] = true;
 		values[Anum_pg_policy_polroles - 1] = PointerGetDatum(role_ids);
 	}
+	else
+	{
+		Oid*roles;
+		Datum		roles_datum;
+		bool		attr_isnull;
+		ArrayType  *policy_roles;
+
+		/*
+		 * We need to pull the set of roles this policy applies to from
+		 * what's in the catalog, so that we can recreate the dependencies
+		 * correctly for the policy.
+		 */
+
+		roles_datum = heap_getattr(policy_tuple, Anum_pg_policy_polroles,
+   RelationGetDescr(pg_policy_rel),
+   _isnull);
+		Assert(!attr_isnull);
+
+		policy_roles = DatumGetArrayTypePCopy(roles_datum);
+
+		roles = (Oid *) ARR_DATA_PTR(policy_roles);
+
+		nitems = ARR_DIMS(policy_roles)[0];
+
+		role_oids = (Datum *) palloc(nitems * sizeof(Datum));
+
+		for (i = 0; i < nitems; i++)
+			role_oids[i] = ObjectIdGetDatum(roles[i]);
+	}
 
 	if (qual != NULL)
 	{
@@ -773,6 +802,40 @@ AlterPolicy(AlterPolicyStmt *stmt)
 		values[Anum_pg_policy_polqual - 1]
 			= CStringGetTextDatum(nodeToString(qual));
 	}
+	else
+	{
+		Datum	value_datum;
+		bool	attr_isnull;
+
+		/*
+		 * We need to pull the USING expression and build the range table for
+		 * the policy from what's in the catalog, so that we can recreate
+		 * the dependencies correctly for the policy.
+		 */
+
+		/* Check if the policy has a USING expr */
+		value_datum = heap_getattr(policy_tuple, Anum_pg_policy_polqual,
+   RelationGetDescr(pg_policy_rel),
+   _isnull);
+		if (!attr_isnull)
+		{
+			char	   *qual_value;
+			ParseState *qual_pstate = make_parsestate(NULL);
+
+			/* parsestate is built just to build the range table */
+			qual_pstate = make_parsestate(NULL);
+
+			qual_value = TextDatumGetCString(value_datum);
+			qual = stringToNode(qual_value);
+
+			/* Add this rel to the parsestate's rangetable, for dependencies */
+			addRangeTableEntryForRelation(qual_pstate, target_table, NULL,
+		  false, false);
+
+			qual_parse_rtable = qual_pstate->p_rtable;
+			free_parsestate(qual_pstate);
+		}
+	}
 
 	if (with_check_qual != NULL)
 	{
@@ -780,6 +843,40 @@ AlterPolicy(AlterPolicyStmt *stmt)
 		values[Anum_pg_policy_polwithcheck - 1]
 			= CStringGetTextDatum(nodeToString(with_check_qual));
 	}
+	else
+	{
+		Datum	value_datum;
+		bool	attr_isnull;
+
+		/*
+		 * We need to pull the WITH CHECK expression and build the range table
+		 * for the policy from what's in the catalog, so that we can recreate
+		 * the dependencies correctly for the policy.
+		 */
+
+		/* Check if the policy has a WITH CHECK expr */
+		value_datum = heap_getattr(policy_tuple, Anum_pg_policy_polwithcheck,
+   RelationGetDescr(pg_policy_rel),
+   _isnull);
+		if (!attr_isnull)
+		{
+			char	   *with_check_value;
+			ParseState *with_check_pstate = make_parsestate(NULL);
+
+			/* parsestate is built just to build the range table */
+			with_check_pstate = make_parsestate(NULL);
+
+			with_check_value 

Re: [HACKERS] Remaining 9.5 open items

[Tom Lane]

Stephen Frost  writes:
> I noticed in passing that the role removal documentation should really
> also discuss shared objects (as the DROP OWNED BY reference page does).

If you're speaking of section 20.4, that text is all my fault ... but
I'm not clear on what you think needs to be added?  The first DROP OWNED
BY will take care of any privileges on shared objects, so I didn't really
think we need to burden the recipe with that detail.

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] Remaining 9.5 open items

[Stephen Frost]

* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Stephen Frost  writes:
> > I noticed in passing that the role removal documentation should really
> > also discuss shared objects (as the DROP OWNED BY reference page does).
> 
> If you're speaking of section 20.4, that text is all my fault ... but
> I'm not clear on what you think needs to be added?  The first DROP OWNED
> BY will take care of any privileges on shared objects, so I didn't really
> think we need to burden the recipe with that detail.

Specifically this:


Once any valuable objects have been transferred to new owners, any
remaining objects owned by the role-to-be-dropped can be dropped with
the DROP OWNED command.  Again, this command can only access objects in
the current database, so it is necessary to run it in each database that
contains objects owned by the role.


Isn't quite right, as databases which are owned by the role you're
trying to get rid of won't be dropped.  The "Again," does pay it some
back-handed service but it felt to me like it'd be better if it was more
explicit about shared objects, which won't be dropped even if you do go
through and connect to each database and issue the command.  Perhaps
that's a bit excessive as, really, the only kinds of 'owned, shared'
objects currently are databases and maybe it's clear enough that you
have to manually drop databases owned by the role you are trying to
drop, if you don't reassign the ownership.  That's what I was referring
to, anyway.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Remaining 9.5 open items

[Tom Lane]

Stephen Frost  writes:
> * Tom Lane (t...@sss.pgh.pa.us) wrote:
>> If you're speaking of section 20.4, that text is all my fault ... but
>> I'm not clear on what you think needs to be added?  The first DROP OWNED
>> BY will take care of any privileges on shared objects, so I didn't really
>> think we need to burden the recipe with that detail.

> Specifically this:
> ...
> Isn't quite right, as databases which are owned by the role you're
> trying to get rid of won't be dropped.

Ah, good point.  I'll add something about that.  I'm not sure that we
should talk about shared objects in general, since as you say databases
are the only instance.  It would feel like handwaving I think.  The point
of that section IMO is to be as concrete as we can be about how to drop
a role.

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] Remaining 9.5 open items

[Etsuro Fujita]

On 2015/12/04 11:51, Noah Misch wrote:

On Tue, Dec 01, 2015 at 11:05:47AM -0500, Robert Haas wrote:

On Mon, Nov 30, 2015 at 2:43 PM, Tom Lane  wrote:

* Foreign join pushdown vs EvalPlanQual

Is this fixed by 5fc4c26db?  If not, what remains to do?



Unfortunately, no.  That commit allows FDWs to do proper EPQ handling
for plain table scans, but it proves to be inadequate for EPQ handling
for joins. Solving that problem will require another patch, and,
modulo a bunch of cosmetic issues, I'm reasonably happy with KaiGai
Kohei's latest submission.  I'll respond in more detail on that
thread, but the question I want to raise here is: do we want to
back-patch those changes to 9.5 at this late date?



Yes.  If 9.5 added a bad interface, better to fix the interface even now than
to live with the bad one.


I'd vote for fixing this.

I think the latest version of the patch for this is in good shape, but 
that would need some changes as proposed on that thread.  So, if there 
are no objections, I'll update the patch.


Best regards,
Etsuro Fujita




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

Re: [HACKERS] Remaining 9.5 open items

[Noah Misch]

On Tue, Dec 01, 2015 at 11:05:47AM -0500, Robert Haas wrote:
> On Mon, Nov 30, 2015 at 2:43 PM, Tom Lane  wrote:
> > * Foreign join pushdown vs EvalPlanQual
> >
> > Is this fixed by 5fc4c26db?  If not, what remains to do?
> 
> Unfortunately, no.  That commit allows FDWs to do proper EPQ handling
> for plain table scans, but it proves to be inadequate for EPQ handling
> for joins. Solving that problem will require another patch, and,
> modulo a bunch of cosmetic issues, I'm reasonably happy with KaiGai
> Kohei's latest submission.  I'll respond in more detail on that
> thread, but the question I want to raise here is: do we want to
> back-patch those changes to 9.5 at this late date?

Yes.  If 9.5 added a bad interface, better to fix the interface even now than
to live with the bad one.


-- 
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] Remaining 9.5 open items

[Peter Geoghegan]

On Wed, Dec 2, 2015 at 5:27 AM, Robert Haas  wrote:
>> These are mainly just documentation improvements which I'm working on,
>> though the docs were recently updated and I need to incorporate Peter's
>> changes which I wasn't exactly anticipating.
>
> So, when do you foresee this documentation stuff getting taken care
> of?  We kinda need to do a release here.  Is this really a blocker?

If it's a blocker, I can write the documentation myself. Let's just
fix it, rather than discussing whether or not it needs to be a blocker
-- it will take less time.

Stephen?

-- 
Peter Geoghegan


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

Re: [HACKERS] Remaining 9.5 open items

[Andres Freund]

On 2015-12-02 08:25:13 -0800, Joshua D. Drake wrote:
> A feature does not exist without documentation.

Uh, you do realize there's actually documentation about RLS? The issues
mentioned here are some small adjustments, not entirely new docs.


-- 
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] Remaining 9.5 open items

[Joshua D. Drake]

On 12/02/2015 05:27 AM, Robert Haas wrote:

On Mon, Nov 30, 2015 at 4:55 PM, Stephen Frost  wrote:

* Tom Lane (t...@sss.pgh.pa.us) wrote:

Well, it's December nearly, and we don't seem to be making much progress
towards pushing out 9.5.0.  I see the following items on
https://wiki.postgresql.org/wiki/PostgreSQL_9.5_Open_Items

* Open Row-Level Security Issues

Seems like what's left here is only documentation fixes, but they still
need to get done.


These are mainly just documentation improvements which I'm working on,
though the docs were recently updated and I need to incorporate Peter's
changes which I wasn't exactly anticipating.


So, when do you foresee this documentation stuff getting taken care
of?  We kinda need to do a release here.  Is this really a blocker?



A feature does not exist without documentation.

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] Remaining 9.5 open items

[Joshua D. Drake]

On 12/02/2015 08:39 AM, Andres Freund wrote:

On 2015-12-02 08:25:13 -0800, Joshua D. Drake wrote:

A feature does not exist without documentation.


Uh, you do realize there's actually documentation about RLS? The issues
mentioned here are some small adjustments, not entirely new docs.


No I didn't. I apologize for the noise.

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] Remaining 9.5 open items

[Andres Freund]

On 2015-12-02 12:25:37 +0100, Joel Jacobson wrote:
> On Wed, Dec 2, 2015 at 12:19 PM, Andres Freund  wrote:
> > The significant changes are in 9.5.
>
> Will multixact truncations be WAL logged in 9.5?

Yes.

C.f. the release notes:

* Rework truncation of the multixact commit log to be properly WAL-logged
  (Andres Freund)

  This makes things substantially simpler and more robust.

The relevant commits are

commit aa29c1ccd9f785f9365809f5133e5491acc7ae53
Author: Andres Freund 
Date:   2015-09-26 19:04:25 +0200

Remove legacy multixact truncation support.
...

Backpatch: 9.5

and
commit 4f627f897367f15702d59973f75f6391d5d3e06f
Author: Andres Freund 
Date:   2015-09-26 19:04:25 +0200

Rework the way multixact truncations work.
...

Backpatch: 9.5


-- 
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] Remaining 9.5 open items

[Joel Jacobson]

On Mon, Nov 30, 2015 at 8:43 PM, Tom Lane  wrote:
> * Finish multixact truncation rework
>
> We're not seriously going to push something this large into 9.5 at this
> point, are we?

I don't know all the details here, so my apologies if any of this is
incorrect/stupid/misinformed.

Given all the quite serious data corruption issues related to
multixact, wouldn't it be motivated to wait releasing 9.5.0 until this
much needed multixct truncation rework has been finished?

This is of course not an issue for users already on >=9.3, since the
problems started in 9.3 and those already on 9.3 and 9.4 already face
the risk.

But users who will consider upgrading from previous versions, will
have to ask themselves if all the new features in 9.5 are worth the
extra risk of data corruption due to the multixact issues.

We at Trustly are still running 9.1 and have been waiting for the
multixact problems to be fixed, which is why we didn't upgrade to 9.4,
and now when I read this I feel really sad we probably have to wait
for 9.6, unless we can accept some increase risk of data corruption.


-- 
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] Remaining 9.5 open items

[Andres Freund]

On 2015-12-02 12:14:42 +0100, Joel Jacobson wrote:
> On Mon, Nov 30, 2015 at 8:43 PM, Tom Lane  wrote:
> > * Finish multixact truncation rework
> >
> > We're not seriously going to push something this large into 9.5 at this
> > point, are we?
> 
> I don't know all the details here, so my apologies if any of this is
> incorrect/stupid/misinformed.
> 
> Given all the quite serious data corruption issues related to
> multixact, wouldn't it be motivated to wait releasing 9.5.0 until this
> much needed multixct truncation rework has been finished?

The significant changes are in 9.5.


-- 
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] Remaining 9.5 open items

[Joel Jacobson]

On Wed, Dec 2, 2015 at 12:19 PM, Andres Freund  wrote:
> The significant changes are in 9.5.

Will multixact truncations be WAL logged in 9.5?


-- 
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] Remaining 9.5 open items

[Joel Jacobson]

On Wed, Dec 2, 2015 at 12:36 PM, Andres Freund  wrote:
>
> On 2015-12-02 12:25:37 +0100, Joel Jacobson wrote:
> > On Wed, Dec 2, 2015 at 12:19 PM, Andres Freund  wrote:
> > > The significant changes are in 9.5.
> >
> > Will multixact truncations be WAL logged in 9.5?
>
> Yes.
>
> C.f. the release notes

Excellent! :-) Many many thanks for your efforts! By far the most
important change in 9.5. So much looking forward to upgrade.


-- 
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] Remaining 9.5 open items

[Robert Haas]

On Mon, Nov 30, 2015 at 4:55 PM, Stephen Frost  wrote:
> * Tom Lane (t...@sss.pgh.pa.us) wrote:
>> Well, it's December nearly, and we don't seem to be making much progress
>> towards pushing out 9.5.0.  I see the following items on
>> https://wiki.postgresql.org/wiki/PostgreSQL_9.5_Open_Items
>>
>> * Open Row-Level Security Issues
>>
>> Seems like what's left here is only documentation fixes, but they still
>> need to get done.
>
> These are mainly just documentation improvements which I'm working on,
> though the docs were recently updated and I need to incorporate Peter's
> changes which I wasn't exactly anticipating.

So, when do you foresee this documentation stuff getting taken care
of?  We kinda need to do a release here.  Is this really a blocker?

-- 
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] Remaining 9.5 open items

[Robert Haas]

On Mon, Nov 30, 2015 at 2:43 PM, Tom Lane  wrote:
> * Foreign join pushdown vs EvalPlanQual
>
> Is this fixed by 5fc4c26db?  If not, what remains to do?

Unfortunately, no.  That commit allows FDWs to do proper EPQ handling
for plain table scans, but it proves to be inadequate for EPQ handling
for joins. Solving that problem will require another patch, and,
modulo a bunch of cosmetic issues, I'm reasonably happy with KaiGai
Kohei's latest submission.  I'll respond in more detail on that
thread, but the question I want to raise here is: do we want to
back-patch those changes to 9.5 at this late date?

If we don't, then join pushdown won't be usable in 9.5 for queries
that have locking clauses; it will crash.  However, it may be that
nobody's going to try to do that anyway.  And if they do, they have
the PlannerInfo available when generating paths, so they can just not
push down any joins when there are row marks, which doesn't sound like
the worst thing that ever happened to anybody.  It's also possible
that the fix isn't really correct and we won't find that out until
after release, at which point it'll be too late to tinker with the API
(if it isn't already).  On the other hand, what do we get out of
shipping an API that we know to be a few bricks short of a load?  I
think the risk of collateral damage is low.  If there's a problem, I
expect it to be that the join-pushdown-vs-EPQ problem is still not
solved, not that anything else stops working.

>From the point of view of existing FDWs, the most noticeable effect of
the patch is that you'll have to pass one more NULL argument to
functions make_foreignscan().  This will break compiles, but it should
be a more trivial repair than what 5fc4c26db demanded.  So maybe it's
OK.

I could go either way on this.  What do others 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] Remaining 9.5 open items

[Stephen Frost]

* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> Stephen Frost wrote:
> 
> > The non-documentation question is around DROP OWNED.  We need to either
> > have policies dropped by DROP OWNED (well, roles removed, unless it's
> > the last one, in which case the policy should be dropped), or update the
> > documentation to reflect that they don't.  I had been thinking we'd
> > fix DROP OWNED to deal with the policies, but if folks feel it's too
> > late for that kind of a change, then we can simply document it.  I don't
> > believe that's unreasonable for a new feature and we can work to get it
> > addressed in 9.6.
> 
> DROP OWNED is documented as a mechanism to help you drop the role, so
> it should do whatever is needed for that.  I don't think documenting the
> fact that it leaves the user as part of policies is good enough.

We already can't take care of everything with DROP OWNED though, since
we can't do cross-database queries, and the overall process almost
certainly requires additional effort (to reassign objects, etc...), so
while I'd be happier if policies were handled by it, I don't think it's
as serious of an issue.

Still, I'll get a patch worked up for it and then we can discuss the
merits of that patch going in to 9.5 now versus just into HEAD.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Remaining 9.5 open items

[Stephen Frost]

* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Well, it's December nearly, and we don't seem to be making much progress
> towards pushing out 9.5.0.  I see the following items on
> https://wiki.postgresql.org/wiki/PostgreSQL_9.5_Open_Items
> 
> * Open Row-Level Security Issues
> 
> Seems like what's left here is only documentation fixes, but they still
> need to get done.

These are mainly just documentation improvements which I'm working on,
though the docs were recently updated and I need to incorporate Peter's
changes which I wasn't exactly anticipating.

The non-documentation question is around DROP OWNED.  We need to either
have policies dropped by DROP OWNED (well, roles removed, unless it's
the last one, in which case the policy should be dropped), or update the
documentation to reflect that they don't.  I had been thinking we'd
fix DROP OWNED to deal with the policies, but if folks feel it's too
late for that kind of a change, then we can simply document it.  I don't
believe that's unreasonable for a new feature and we can work to get it
addressed in 9.6.

I'm starting to think that just documenting it makes sense for 9.5.  I 
doubt it's going to be a serious issue during 9.5's lifetime.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Remaining 9.5 open items

[Alvaro Herrera]

Stephen Frost wrote:

> The non-documentation question is around DROP OWNED.  We need to either
> have policies dropped by DROP OWNED (well, roles removed, unless it's
> the last one, in which case the policy should be dropped), or update the
> documentation to reflect that they don't.  I had been thinking we'd
> fix DROP OWNED to deal with the policies, but if folks feel it's too
> late for that kind of a change, then we can simply document it.  I don't
> believe that's unreasonable for a new feature and we can work to get it
> addressed in 9.6.

DROP OWNED is documented as a mechanism to help you drop the role, so
it should do whatever is needed for that.  I don't think documenting the
fact that it leaves the user as part of policies is good enough.

-- 
Á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] Remaining 9.5 open items

[Andres Freund]

On 2015-11-30 14:43:59 -0500, Tom Lane wrote:
> * pg_rewind exiting with error code 1 when source and target are on the same 
> timeline
> 
> Is this a new-in-9.5 bug, or a pre-existing problem?  If the latter,
> I'm not sure it's a release blocker.

pg_rewind was only introduced in 9.5, no?


> * Finish multixact truncation rework
> 
> We're not seriously going to push something this large into 9.5 at this
> point, are we?

To my knowledge this mostly comment changes. And some, actually
independent, improvements. More blocked on procedual disagreements -
with Noah wanting to revert the existing commits, fixup some stuff, then
reapply them - and me just wanting to do the improvements independently.



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] Remaining 9.5 open items

[Alvaro Herrera]

Tom Lane wrote:

> * DDL deparsing testing module should have detected that transforms were not 
> supported, but it failed to notice that
> 
> Is this really a release blocker?  As a testing matter, it seems like any
> fix would go into HEAD only.

Not a blocker as far as I'm concerned.

> * another strange behavior with track_commit_timestamp
> 
> Where are we on this?

I will test the patch I proposed more thoroughly, then push.  I intend
to add a few test cases on top of the recovery testing patch Michael
submitted (but that's branch master only), to make sure I don't
re-introduce bugs already fixed.

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


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

[HACKERS] Remaining 9.5 open items

[Tom Lane]

Well, it's December nearly, and we don't seem to be making much progress
towards pushing out 9.5.0.  I see the following items on
https://wiki.postgresql.org/wiki/PostgreSQL_9.5_Open_Items

* Open Row-Level Security Issues

Seems like what's left here is only documentation fixes, but they still
need to get done.

* DDL deparsing testing module should have detected that transforms were not 
supported, but it failed to notice that

Is this really a release blocker?  As a testing matter, it seems like any
fix would go into HEAD only.

* Foreign join pushdown vs EvalPlanQual

Is this fixed by 5fc4c26db?  If not, what remains to do?

* pg_rewind exiting with error code 1 when source and target are on the same 
timeline

Is this a new-in-9.5 bug, or a pre-existing problem?  If the latter,
I'm not sure it's a release blocker.

* psql extended wrapped format off by one error in line wrapping

There's a submitted patch, so I'll take a look at whether it's pushable.

* Finish multixact truncation rework

We're not seriously going to push something this large into 9.5 at this
point, are we?

* another strange behavior with track_commit_timestamp

Where are we on this?

* Relation files of unlogged relation for btree and spgist indexes not 
initialized after promotion

Again, is this a release blocker?  It's evidently a very old bug.

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] Remaining 9.5 open items

[Michael Paquier]

On Tue, Dec 1, 2015 at 4:43 AM, Tom Lane  wrote:
> * pg_rewind exiting with error code 1 when source and target are on the same 
> timeline
>
> Is this a new-in-9.5 bug, or a pre-existing problem?  If the latter,
> I'm not sure it's a release blocker.

pg_rewind has been introduced in 9.5. It would be good to get
something consistent before GA.

> * Relation files of unlogged relation for btree and spgist indexes not 
> initialized after promotion
>
> Again, is this a release blocker?  It's evidently a very old bug.

Actually for 9.5 it could be said so. The approach proposed by Andres
and that I have patched requires a bump of the WAL format. It would be
nice to get that into 9.5 tree. Or 9.5 will have to use a solution
similar to the back branches, which would be something like syncing
unconditionally INIT_FORKNUM at replay after replaying its FPW.
Thoughts welcome on the dedicated thread.
-- 
Michael


-- 
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] Remaining 9.5 open items

[Alvaro Herrera]

Stephen Frost wrote:
> * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> > Stephen Frost wrote:
> > 
> > > The non-documentation question is around DROP OWNED.  We need to either
> > > have policies dropped by DROP OWNED (well, roles removed, unless it's
> > > the last one, in which case the policy should be dropped), or update the
> > > documentation to reflect that they don't.  I had been thinking we'd
> > > fix DROP OWNED to deal with the policies, but if folks feel it's too
> > > late for that kind of a change, then we can simply document it.  I don't
> > > believe that's unreasonable for a new feature and we can work to get it
> > > addressed in 9.6.
> > 
> > DROP OWNED is documented as a mechanism to help you drop the role, so
> > it should do whatever is needed for that.  I don't think documenting the
> > fact that it leaves the user as part of policies is good enough.
> 
> We already can't take care of everything with DROP OWNED though, since
> we can't do cross-database queries, and the overall process almost
> certainly requires additional effort (to reassign objects, etc...), so
> while I'd be happier if policies were handled by it, I don't think it's
> as serious of an issue.

Yes, the documentation says to apply a combination of REASSIGN OWNED
plus DROP OWNED to each database.  Sure, it's not a single command, but
if you additionally put the burden that the policies must be taken care
of separately, then the whole process is made a little worse.

> Still, I'll get a patch worked up for it and then we can discuss the
> merits of that patch going in to 9.5 now versus just into HEAD.

Cool.

In the past, we've made a bunch of changes to DROP OWNED in order to
deal with object types that caused errors, even in minor releases.  I
think this is just another case of the same problem.

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