COPY FREEZE has no warning

On Wed, Jan 23, 2013 at 2:02 PM, Bruce Momjian <bruce@momjian.us> wrote:

As a reminder, COPY FREEZE still does not issue any warning/notice if
the freezing does not happen:

Requests copying the data with rows already frozen, just as they
would be after running the <command>VACUUM FREEZE</> command.
This is intended as a performance option for initial data loading.
Rows will be frozen only if the table being loaded has been created
in the current subtransaction, there are no cursors open and there
are no older snapshots held by this transaction. If those conditions
are not met the command will continue without error though will not
freeze rows. It is also possible in rare cases that the request
cannot be honoured for internal reasons, hence <literal>FREEZE</literal>
is more of a guideline than a hard rule.

Note that all other sessions will immediately be able to see the data
once it has been successfully loaded. This violates the normal rules
of MVCC visibility and by specifying this option the user acknowledges
explicitly that this is understood.

Didn't we want to issue the user some kind of feedback?

I believe that is what was agreed.

--
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 Wed, Jan 23, 2013 at 02:02:46PM -0500, Bruce Momjian wrote:

As a reminder, COPY FREEZE still does not issue any warning/notice if
the freezing does not happen:

Requests copying the data with rows already frozen, just as they
would be after running the <command>VACUUM FREEZE</> command.
This is intended as a performance option for initial data loading.
Rows will be frozen only if the table being loaded has been created
in the current subtransaction, there are no cursors open and there
are no older snapshots held by this transaction. If those conditions
are not met the command will continue without error though will not
freeze rows. It is also possible in rare cases that the request
cannot be honoured for internal reasons, hence <literal>FREEZE</literal>
is more of a guideline than a hard rule.

Note that all other sessions will immediately be able to see the data
once it has been successfully loaded. This violates the normal rules
of MVCC visibility and by specifying this option the user acknowledges
explicitly that this is understood.

Didn't we want to issue the user some kind of feedback?

As no one wanted to write this patch, I have developed the attached
version.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

On 1/24/13 5:09 PM, Bruce Momjian wrote:

On Wed, Jan 23, 2013 at 02:02:46PM -0500, Bruce Momjian wrote:

As a reminder, COPY FREEZE still does not issue any warning/notice if
the freezing does not happen:

As no one wanted to write this patch, I have developed the attached
version.

I think it would be useful to add why it was unable to honor the option.
Otherwise this might just end up spamming the logs without any chance
for improvement.

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

On Thu, Jan 24, 2013 at 05:30:22PM -0500, Peter Eisentraut wrote:

On 1/24/13 5:09 PM, Bruce Momjian wrote:

On Wed, Jan 23, 2013 at 02:02:46PM -0500, Bruce Momjian wrote:

As a reminder, COPY FREEZE still does not issue any warning/notice if
the freezing does not happen:

As no one wanted to write this patch, I have developed the attached
version.

I think it would be useful to add why it was unable to honor the option.
Otherwise this might just end up spamming the logs without any chance
for improvement.

Well, I would need to repeat what is already in the COPY docs. Do you
have any suggested text?

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

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

Bruce Momjian <bruce@momjian.us> writes:

Didn't we want to issue the user some kind of feedback?

As no one wanted to write this patch, I have developed the attached
version.

Please note the comment directly above where you patched.

The proposed message doesn't seem to me to be following the message
style guide, either.

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

On Thu, Jan 24, 2013 at 06:55:17PM -0500, Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

Didn't we want to issue the user some kind of feedback?

As no one wanted to write this patch, I have developed the attached
version.

Please note the comment directly above where you patched.

The proposed message doesn't seem to me to be following the message
style guide, either.

OK, updated patch attached.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

On 2013-01-23 14:02:46 -0500, Bruce Momjian wrote:

As a reminder, COPY FREEZE still does not issue any warning/notice if
the freezing does not happen:

FWIW, and I won't annoy anyone further after this email, now that its
deterministic, I still think that this should be an ERROR not a WARNING.

Greetings,

Andres Freund

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

On Fri, Jan 25, 2013 at 02:48:37AM +0100, Andres Freund wrote:

On 2013-01-23 14:02:46 -0500, Bruce Momjian wrote:

As a reminder, COPY FREEZE still does not issue any warning/notice if
the freezing does not happen:

FWIW, and I won't annoy anyone further after this email, now that its
deterministic, I still think that this should be an ERROR not a WARNING.

As the FREEZE is just an optimization, I thought NOTICE, vs WARNING or
ERROR was fine. If others want this changed, please reply.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

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

* Bruce Momjian (bruce@momjian.us) wrote:

On Fri, Jan 25, 2013 at 02:48:37AM +0100, Andres Freund wrote:

On 2013-01-23 14:02:46 -0500, Bruce Momjian wrote:

As a reminder, COPY FREEZE still does not issue any warning/notice if
the freezing does not happen:

FWIW, and I won't annoy anyone further after this email, now that its
deterministic, I still think that this should be an ERROR not a WARNING.

As the FREEZE is just an optimization, I thought NOTICE, vs WARNING or
ERROR was fine. If others want this changed, please reply.

tbh, I tend to agree w/ Andres on this one. COPY FREEZE means "do
this", not "if you can get away with it, then do it". That said, I can
really see a use-case for both which would imply that we'd have a way to
specify, ala DROP TABLE and IF EXISTS. Not sure exactly what that'd
look like though and having one or the other is better than nothing
(presuming everyone is fine with the visibility impacts of this, which I
still contend will cause our users to give us grief over in the
future..).

Thanks,

Stephen

On Fri, Jan 25, 2013 at 10:30:40AM -0500, Stephen Frost wrote:

* Bruce Momjian (bruce@momjian.us) wrote:

On Fri, Jan 25, 2013 at 02:48:37AM +0100, Andres Freund wrote:

On 2013-01-23 14:02:46 -0500, Bruce Momjian wrote:

As a reminder, COPY FREEZE still does not issue any warning/notice if
the freezing does not happen:

FWIW, and I won't annoy anyone further after this email, now that its
deterministic, I still think that this should be an ERROR not a WARNING.

As the FREEZE is just an optimization, I thought NOTICE, vs WARNING or
ERROR was fine. If others want this changed, please reply.

tbh, I tend to agree w/ Andres on this one. COPY FREEZE means "do
this", not "if you can get away with it, then do it". That said, I can
really see a use-case for both which would imply that we'd have a way to
specify, ala DROP TABLE and IF EXISTS. Not sure exactly what that'd
look like though and having one or the other is better than nothing
(presuming everyone is fine with the visibility impacts of this, which I
still contend will cause our users to give us grief over in the
future..).

Interesting. I can see the visibility as making this more than an
optimization, because it has external visibility. However, the
visibility problem is when it is silent (no NOTICE). Do we need
a message that says we did honor FREEZE?

We could get fancy and make FREEZE more than a boolean, e.g. OFF,
PREFER, FORCE.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

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

Bruce Momjian <bruce@momjian.us> writes:

On Fri, Jan 25, 2013 at 02:48:37AM +0100, Andres Freund wrote:

FWIW, and I won't annoy anyone further after this email, now that its
deterministic, I still think that this should be an ERROR not a WARNING.

As the FREEZE is just an optimization, I thought NOTICE, vs WARNING or
ERROR was fine. If others want this changed, please reply.

The previous argument about it was "if you bothered to specify FREEZE,
you probably really want/need that behavior". So I can definitely see
Andres' point. Perhaps WARNING would be a suitable compromise?

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

On Fri, Jan 25, 2013 at 11:59 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Bruce Momjian <bruce@momjian.us> writes:

On Fri, Jan 25, 2013 at 02:48:37AM +0100, Andres Freund wrote:

FWIW, and I won't annoy anyone further after this email, now that its
deterministic, I still think that this should be an ERROR not a WARNING.

As the FREEZE is just an optimization, I thought NOTICE, vs WARNING or
ERROR was fine. If others want this changed, please reply.

The previous argument about it was "if you bothered to specify FREEZE,
you probably really want/need that behavior". So I can definitely see
Andres' point. Perhaps WARNING would be a suitable compromise?

I'll vote for ERROR. I don't see why this sound be a best-effort thing.

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

* Robert Haas (robertmhaas@gmail.com) wrote:

On Fri, Jan 25, 2013 at 11:59 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Bruce Momjian <bruce@momjian.us> writes:

On Fri, Jan 25, 2013 at 02:48:37AM +0100, Andres Freund wrote:

FWIW, and I won't annoy anyone further after this email, now that its
deterministic, I still think that this should be an ERROR not a WARNING.

As the FREEZE is just an optimization, I thought NOTICE, vs WARNING or
ERROR was fine. If others want this changed, please reply.

The previous argument about it was "if you bothered to specify FREEZE,
you probably really want/need that behavior". So I can definitely see
Andres' point. Perhaps WARNING would be a suitable compromise?

I'll vote for ERROR. I don't see why this sound be a best-effort thing.

Yeah, I tend to agree. In part, I think having it error when the
conditions aren't met would actually reduce the chances of having this
'feature' end up as the default in some ORM somewhere...

Thanks,

Stephen

On Fri, Jan 25, 2013 at 9:42 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Fri, Jan 25, 2013 at 11:59 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Bruce Momjian <bruce@momjian.us> writes:

On Fri, Jan 25, 2013 at 02:48:37AM +0100, Andres Freund wrote:

FWIW, and I won't annoy anyone further after this email, now that its
deterministic, I still think that this should be an ERROR not a WARNING.

As the FREEZE is just an optimization, I thought NOTICE, vs WARNING or
ERROR was fine. If others want this changed, please reply.

The previous argument about it was "if you bothered to specify FREEZE,
you probably really want/need that behavior". So I can definitely see
Andres' point. Perhaps WARNING would be a suitable compromise?

I'll vote for ERROR. I don't see why this sound be a best-effort thing.

+1. If I had no objection to my database getting stuffed to the gills
with unfrozen tuples, I wouldn't have invoked the feature in the first
place.

As far as can tell, this ERROR/WARNING must occur immediately, because
once the first tuple is inserted frozen it is too late to change ones
mind. So the problem can be immediately fixed and retried.

Except, is there perhaps some way for the user to decide to promote
WARNINGs to ERRORs on for a given command/transaction?

Cheers,

Jeff

--
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, Jan 25, 2013 at 11:55:12AM -0800, Jeff Janes wrote:

On Fri, Jan 25, 2013 at 9:42 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Fri, Jan 25, 2013 at 11:59 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Bruce Momjian <bruce@momjian.us> writes:

On Fri, Jan 25, 2013 at 02:48:37AM +0100, Andres Freund wrote:

FWIW, and I won't annoy anyone further after this email, now that its
deterministic, I still think that this should be an ERROR not a WARNING.

As the FREEZE is just an optimization, I thought NOTICE, vs WARNING or
ERROR was fine. If others want this changed, please reply.

The previous argument about it was "if you bothered to specify FREEZE,
you probably really want/need that behavior". So I can definitely see
Andres' point. Perhaps WARNING would be a suitable compromise?

I'll vote for ERROR. I don't see why this sound be a best-effort thing.

+1. If I had no objection to my database getting stuffed to the gills
with unfrozen tuples, I wouldn't have invoked the feature in the first
place.

As far as can tell, this ERROR/WARNING must occur immediately, because
once the first tuple is inserted frozen it is too late to change ones
mind. So the problem can be immediately fixed and retried.

Except, is there perhaps some way for the user to decide to promote
WARNINGs to ERRORs on for a given command/transaction?

OK, updated patch attached that throws an error with a more specific
message.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Bruce Momjian <bruce@momjian.us> writes:

OK, updated patch attached that throws an error with a more specific
message.

*
* As noted above rd_newRelfilenodeSubid is not set in all cases
* where we can apply the optimization, so in those rare cases
! * where we cannot honor the request.
*/

This sentence not complete. I kind of think the entire para visible
above could be removed, anyway.

! ereport(ERROR, (errmsg("cannot perform FREEZE operation due to invalid table or transaction state")));

I don't find this terribly specific. It would at least be useful to
have two messages distinguishing whether the cause was invalid table
state (rd_createSubid and rd_newRelfilenodeSubid not set) or invalid
transaction state (the snapshot and portal tests). The former might
usefully be phrased as "because the table was not created or truncated
in the current transaction" and the latter as "because other actions are
in progress within the current transaction".

I'd also suggest "cannot perform COPY FREEZE because <whatever>" rather
than using the unnecessarily vague "operation".

Also, this is missing an errcode, which means it will report itself as
an internal error, which it ain't. It's also randomly unlike the
standard layout for ereport calls.
ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE would do for the table case,
not sure about the other.

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

On Sat, Jan 26, 2013 at 2:42 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Fri, Jan 25, 2013 at 11:59 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Bruce Momjian <bruce@momjian.us> writes:

On Fri, Jan 25, 2013 at 02:48:37AM +0100, Andres Freund wrote:

FWIW, and I won't annoy anyone further after this email, now that its
deterministic, I still think that this should be an ERROR not a

WARNING.

As the FREEZE is just an optimization, I thought NOTICE, vs WARNING or
ERROR was fine. If others want this changed, please reply.

The previous argument about it was "if you bothered to specify FREEZE,
you probably really want/need that behavior". So I can definitely see
Andres' point. Perhaps WARNING would be a suitable compromise?

I'll vote for ERROR. I don't see why this sound be a best-effort thing.

+ 1. I was surprised to see COPY FREEZE failing silently when testing the
feature. An ERROR would be suited.
-- 
Michael Paquier
http://michael.otacoo.com

On Fri, Jan 25, 2013 at 05:30:58PM -0500, Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

OK, updated patch attached that throws an error with a more specific
message.

*
* As noted above rd_newRelfilenodeSubid is not set in all cases
* where we can apply the optimization, so in those rare cases
! * where we cannot honor the request.
*/

This sentence not complete. I kind of think the entire para visible
above could be removed, anyway.

! ereport(ERROR, (errmsg("cannot perform FREEZE operation due to invalid table or transaction state")));

I don't find this terribly specific. It would at least be useful to
have two messages distinguishing whether the cause was invalid table
state (rd_createSubid and rd_newRelfilenodeSubid not set) or invalid
transaction state (the snapshot and portal tests). The former might
usefully be phrased as "because the table was not created or truncated
in the current transaction" and the latter as "because other actions are
in progress within the current transaction".

I'd also suggest "cannot perform COPY FREEZE because <whatever>" rather
than using the unnecessarily vague "operation".

Also, this is missing an errcode, which means it will report itself as
an internal error, which it ain't. It's also randomly unlike the
standard layout for ereport calls.
ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE would do for the table case,
not sure about the other.

OK, that was tricky, but completed with the attached patch.
Surprisingly, truncation wasn't mention in our docs, though it was used
in the regression tests. I have fixed that.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Bruce Momjian <bruce@momjian.us> writes:

! ereport(ERROR,
! (ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE,
! errmsg("cannot perform FREEZE because of previous table activity in the current transaction")));

[ itch... ] What is "table activity"? I always thought of tables as
being rather passive objects. And anyway, isn't this backwards? What
we're complaining of is *lack* of activity. I don't see why this isn't
using the same message as the other code path, namely

+ 		ereport(ERROR,
+ 				(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE,
+ 				 errmsg("cannot perform FREEZE because the table was not created or truncated in the current transaction")));

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