Problem with dumping bloom extension

On Fri, Jun 3, 2016 at 8:57 PM, Thom Brown <thom@linux.com> wrote:

If a database with the bloom extension installed is dumped and restored,
there's an error with the access method creation:

createdb bloomtest
psql -c 'CREATE EXTENSION bloom;' bloomtest
pg_dump -d bloomtest > ~/tmp/bloom.sql
createdb bloomtest2
psql -d bloomtest2 -f ~/tmp/bloom.sql

The output of the last command produces:

"psql:/home/thom/tmp/bloom.sql:48: ERROR: access method "bloom" already
exists"

So pg_dump shouldn't be dumping this access method as it's part of the
extension.

Stephen, something is smelling wrong in checkExtensionMembership()
since 5d58999, an access method does not have ACLs and I would have
expected that when this routine is invoked in
selectDumpableAccessMethod() the object is not selected as dumpable.
--
Michael

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

Michael,

* Michael Paquier (michael.paquier@gmail.com) wrote:

On Fri, Jun 3, 2016 at 8:57 PM, Thom Brown <thom@linux.com> wrote:

If a database with the bloom extension installed is dumped and restored,
there's an error with the access method creation:

createdb bloomtest
psql -c 'CREATE EXTENSION bloom;' bloomtest
pg_dump -d bloomtest > ~/tmp/bloom.sql
createdb bloomtest2
psql -d bloomtest2 -f ~/tmp/bloom.sql

The output of the last command produces:

"psql:/home/thom/tmp/bloom.sql:48: ERROR: access method "bloom" already
exists"

So pg_dump shouldn't be dumping this access method as it's part of the
extension.

Stephen, something is smelling wrong in checkExtensionMembership()
since 5d58999, an access method does not have ACLs and I would have
expected that when this routine is invoked in
selectDumpableAccessMethod() the object is not selected as dumpable.

Yeah, I saw this also and was going to look into it.

I suspect the issue may actually be that dumpAccessMethod() wasn't ever
updated to have the appropriate conditionals for each of the components
of the object.

Specifically, there should be if statements along the lines of:

if (aminfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
ArchiveEntry ...

if (aminfo->dobj.dump & DUMP_COMPONENT_COMMENT)
dumpComment()

towards the end of dumpAccessMethod().

I'm not 100% sure that addresses this, but it definitely needs to be
fixed also. I'll take care of it in the next few days.

I'll also look more directly into what's going on here this weekend when
I've got a bit more time to do so.

Thanks!

Stephen

On Fri, Jun 3, 2016 at 12:31 PM, Stephen Frost <sfrost@snowman.net> wrote:

Stephen, something is smelling wrong in checkExtensionMembership()
since 5d58999, an access method does not have ACLs and I would have
expected that when this routine is invoked in
selectDumpableAccessMethod() the object is not selected as dumpable.

Yeah, I saw this also and was going to look into it.

I suspect the issue may actually be that dumpAccessMethod() wasn't ever
updated to have the appropriate conditionals for each of the components
of the object.

Specifically, there should be if statements along the lines of:

if (aminfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
ArchiveEntry ...

if (aminfo->dobj.dump & DUMP_COMPONENT_COMMENT)
dumpComment()

towards the end of dumpAccessMethod().

I'm not 100% sure that addresses this, but it definitely needs to be
fixed also. I'll take care of it in the next few days.

I'll also look more directly into what's going on here this weekend when
I've got a bit more time to do so.

It seems important to get this fixed. I added it to the open items list.

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

On Tue, Jun 7, 2016 at 12:01 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Fri, Jun 3, 2016 at 12:31 PM, Stephen Frost <sfrost@snowman.net> wrote:

Stephen, something is smelling wrong in checkExtensionMembership()
since 5d58999, an access method does not have ACLs and I would have
expected that when this routine is invoked in
selectDumpableAccessMethod() the object is not selected as dumpable.

Yeah, I saw this also and was going to look into it.

I suspect the issue may actually be that dumpAccessMethod() wasn't ever
updated to have the appropriate conditionals for each of the components
of the object.

Specifically, there should be if statements along the lines of:

if (aminfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
ArchiveEntry ...

if (aminfo->dobj.dump & DUMP_COMPONENT_COMMENT)
dumpComment()

towards the end of dumpAccessMethod().

I'm not 100% sure that addresses this, but it definitely needs to be
fixed also. I'll take care of it in the next few days.

I'll also look more directly into what's going on here this weekend when
I've got a bit more time to do so.

It seems important to get this fixed. I added it to the open items list.

I added already it as " Access methods created with extensions are
dumped individually ". That's not specific to bloom.
--
Michael

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

On Tue, Jun 7, 2016 at 6:55 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Tue, Jun 7, 2016 at 12:01 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Fri, Jun 3, 2016 at 12:31 PM, Stephen Frost <sfrost@snowman.net> wrote:

Stephen, something is smelling wrong in checkExtensionMembership()
since 5d58999, an access method does not have ACLs and I would have
expected that when this routine is invoked in
selectDumpableAccessMethod() the object is not selected as dumpable.

Yeah, I saw this also and was going to look into it.

I suspect the issue may actually be that dumpAccessMethod() wasn't ever
updated to have the appropriate conditionals for each of the components
of the object.

Specifically, there should be if statements along the lines of:

if (aminfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
ArchiveEntry ...

if (aminfo->dobj.dump & DUMP_COMPONENT_COMMENT)
dumpComment()

towards the end of dumpAccessMethod().

I'm not 100% sure that addresses this, but it definitely needs to be
fixed also. I'll take care of it in the next few days.

I'll also look more directly into what's going on here this weekend when
I've got a bit more time to do so.

It seems important to get this fixed. I added it to the open items list.

Stephen, are you working on a patch or should I get one out of my
pocket? That's something we should get fixed quickly. We need as well
to be careful with the support for COMMENT with access methods, the
current consensus on the matter is that it will be soon committed.
--
Michael

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

On Fri, Jun 03, 2016 at 12:31:44PM -0400, Stephen Frost wrote:

* Michael Paquier (michael.paquier@gmail.com) wrote:

On Fri, Jun 3, 2016 at 8:57 PM, Thom Brown <thom@linux.com> wrote:

If a database with the bloom extension installed is dumped and restored,
there's an error with the access method creation:

createdb bloomtest
psql -c 'CREATE EXTENSION bloom;' bloomtest
pg_dump -d bloomtest > ~/tmp/bloom.sql
createdb bloomtest2
psql -d bloomtest2 -f ~/tmp/bloom.sql

The output of the last command produces:

"psql:/home/thom/tmp/bloom.sql:48: ERROR: access method "bloom" already
exists"

So pg_dump shouldn't be dumping this access method as it's part of the
extension.

Stephen, something is smelling wrong in checkExtensionMembership()
since 5d58999, an access method does not have ACLs and I would have
expected that when this routine is invoked in
selectDumpableAccessMethod() the object is not selected as dumpable.

Yeah, I saw this also and was going to look into it.

[Action required within 72 hours. This is a generic notification.]

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

[1]: /messages/by-id/20160527025039.GA447393@tornado.leadboat.com

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

On Mon, Jun 6, 2016 at 5:55 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

It seems important to get this fixed. I added it to the open items list.

I added already it as " Access methods created with extensions are
dumped individually ". That's not specific to bloom.

Oh, sorry, I didn't spot that.

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

On Tue, Jun 7, 2016 at 8:10 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On Mon, Jun 6, 2016 at 5:55 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

It seems important to get this fixed. I added it to the open items list.

I added already it as " Access methods created with extensions are
dumped individually ". That's not specific to bloom.

Oh, sorry, I didn't spot that.

Never mind. I refreshed the wiki again after that, and kept your entry
which uses directly $subject.
--
Michael

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

Michael,

* Michael Paquier (michael.paquier@gmail.com) wrote:

Stephen, are you working on a patch or should I get one out of my
pocket? That's something we should get fixed quickly. We need as well
to be careful with the support for COMMENT with access methods, the
current consensus on the matter is that it will be soon committed.

I'll fix it today.

Thanks!

Stephen

* Noah Misch (noah@leadboat.com) wrote:

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

Addressed with 562f06f.

Moved to CLOSE_WAIT.

Thanks!

Stephen

On 6/7/16 11:16 AM, Stephen Frost wrote:

Moved to CLOSE_WAIT.

Could you add an explanation on the wiki page about what this section means?

--
Peter Eisentraut http://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

On Tue, Jun 7, 2016 at 2:40 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

On 6/7/16 11:16 AM, Stephen Frost wrote:

Moved to CLOSE_WAIT.

Could you add an explanation on the wiki page about what this section means?

Noah created that section. My interpretation is that it's supposed to
be for things we think are fixed, but maybe there's a chance they
aren't - like a performance problem that we've patched but not
received confirmation from the original reporter that the patch fixes
it for them. I'm inclined to think that most issues should just
become "resolved" right away.

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

* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:

On 6/7/16 11:16 AM, Stephen Frost wrote:

Moved to CLOSE_WAIT.

Could you add an explanation on the wiki page about what this section means?

I understood it to simply be a step on the way to being resolved- that
is, everything goes through CLOSE_WAIT for some period of time and then
is moved to resolved when it's clear that the consensus is that it's
closed.

That doesn't appear to be the consensus on the meaning though, and I
didn't add it, so I'm not the one to update the wiki page to explain it.

Thanks!

Stephen

On Tue, Jun 07, 2016 at 03:23:46PM -0400, Robert Haas wrote:

On Tue, Jun 7, 2016 at 2:40 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

On 6/7/16 11:16 AM, Stephen Frost wrote:

Moved to CLOSE_WAIT.

Could you add an explanation on the wiki page about what this section means?

Noah created that section. My interpretation is that it's supposed to
be for things we think are fixed, but maybe there's a chance they
aren't - like a performance problem that we've patched but not
received confirmation from the original reporter that the patch fixes
it for them. I'm inclined to think that most issues should just
become "resolved" right away.

Yep, pretty much that. CLOSE_WAIT is for performance defects, race
conditions, and other defects where a successful fix is difficult to verify
beyond reasonable doubt. Other things can move directly to "resolved". I
don't mind if practice diverges from that intent, and I don't really process
the two sections differently.

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

* Noah Misch (noah@leadboat.com) wrote:

Yep, pretty much that. CLOSE_WAIT is for performance defects, race
conditions, and other defects where a successful fix is difficult to verify
beyond reasonable doubt. Other things can move directly to "resolved". I
don't mind if practice diverges from that intent, and I don't really process
the two sections differently.

Ok, thanks for the clarification. I've updated the entries which I had
accordingly.

Thanks again!

Stephen