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?

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

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

Search Discussions

  • Robert Haas at Jan 23, 2013 at 8:04 pm

    On Wed, Jan 23, 2013 at 2:02 PM, 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?
    I believe that is what was agreed.

    --
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company
  • Bruce Momjian at Jan 24, 2013 at 10:09 pm

    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. +
  • Peter Eisentraut at Jan 24, 2013 at 10:30 pm

    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.
  • Bruce Momjian at Jan 24, 2013 at 11:15 pm

    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. +
  • Tom Lane at Jan 24, 2013 at 11:55 pm

    Bruce Momjian 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
  • Bruce Momjian at Jan 25, 2013 at 1:18 am

    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. +
  • Andres Freund at Jan 25, 2013 at 1:48 am

    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
  • Bruce Momjian at Jan 25, 2013 at 3:03 pm

    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. +
  • Stephen Frost at Jan 25, 2013 at 3:30 pm

    * 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
  • Bruce Momjian at Jan 25, 2013 at 3:51 pm

    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. +
  • Tom Lane at Jan 25, 2013 at 4:59 pm

    Bruce Momjian 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
  • Robert Haas at Jan 25, 2013 at 5:42 pm

    On Fri, Jan 25, 2013 at 11:59 AM, Tom Lane 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
  • Stephen Frost at Jan 25, 2013 at 6:06 pm

    * Robert Haas (robertmhaas@gmail.com) wrote:
    On Fri, Jan 25, 2013 at 11:59 AM, Tom Lane 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
  • Jeff Janes at Jan 25, 2013 at 7:55 pm

    On Fri, Jan 25, 2013 at 9:42 AM, Robert Haas wrote:
    On Fri, Jan 25, 2013 at 11:59 AM, Tom Lane 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
  • Bruce Momjian at Jan 25, 2013 at 8:16 pm

    On Fri, Jan 25, 2013 at 11:55:12AM -0800, Jeff Janes wrote:
    On Fri, Jan 25, 2013 at 9:42 AM, Robert Haas wrote:
    On Fri, Jan 25, 2013 at 11:59 AM, Tom Lane 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. +
  • Tom Lane at Jan 25, 2013 at 10:31 pm

    Bruce Momjian 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
  • Bruce Momjian at Jan 26, 2013 at 3:59 am

    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. +
  • Tom Lane at Jan 26, 2013 at 4:09 am

    Bruce Momjian 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
  • Bruce Momjian at Jan 26, 2013 at 4:29 am

    On Fri, Jan 25, 2013 at 11:08:56PM -0500, Tom Lane wrote:
    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
    Well, here is an example of this message:

      BEGIN;
      TRUNCATE vistest;
      SAVEPOINT s1;
      COPY vistest FROM stdin CSV FREEZE;
      ERROR: cannot perform FREEZE because of previous table activity in the current transaction
      COMMIT;

    Clearly it was truncated in the same transaction, but the savepoint
    somehow invalidates the freeze. There is a C comment about it:

          * BEGIN;
          * TRUNCATE t;
          * SAVEPOINT save;
          * TRUNCATE t;
          * ROLLBACK TO save;
          * COPY ...

    I changed it to:

             ERROR: cannot perform FREEZE because of transaction activity after table creation or truncation

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

       + It's impossible for everything to be true. +
  • Bruce Momjian at Jan 26, 2013 at 6:34 pm

    On Fri, Jan 25, 2013 at 11:28:58PM -0500, Bruce Momjian wrote:
    On Fri, Jan 25, 2013 at 11:08:56PM -0500, Tom Lane wrote:
    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
    Well, here is an example of this message:

    BEGIN;
    TRUNCATE vistest;
    SAVEPOINT s1;
    COPY vistest FROM stdin CSV FREEZE;
    ERROR: cannot perform FREEZE because of previous table activity in the current transaction
    COMMIT;

    Clearly it was truncated in the same transaction, but the savepoint
    somehow invalidates the freeze. There is a C comment about it:

    * BEGIN;
    * TRUNCATE t;
    * SAVEPOINT save;
    * TRUNCATE t;
    * ROLLBACK TO save;
    * COPY ...

    I changed it to:

    ERROR: cannot perform FREEZE because of transaction activity after table creation or truncation
    Patch applied.

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

       + It's impossible for everything to be true. +
  • Noah Misch at Jan 30, 2013 at 1:34 am

    On Fri, Jan 25, 2013 at 11:28:58PM -0500, Bruce Momjian wrote:
    On Fri, Jan 25, 2013 at 11:08:56PM -0500, Tom Lane wrote:
    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
    Well, here is an example of this message:

    BEGIN;
    TRUNCATE vistest;
    SAVEPOINT s1;
    COPY vistest FROM stdin CSV FREEZE;
    ERROR: cannot perform FREEZE because of previous table activity in the current transaction
    COMMIT;

    Clearly it was truncated in the same transaction, but the savepoint
    somehow invalidates the freeze. There is a C comment about it:
    The savepoint prevents the COPY FREEZE, because COPY FREEZE needs the table to
    have been created or truncated in the current *sub*transaction. Issuing
    "RELEASE s1" before the COPY makes it work again, for example.
    * BEGIN;
    * TRUNCATE t;
    * SAVEPOINT save;
    * TRUNCATE t;
    * ROLLBACK TO save;
    * COPY ...
    This is different. The table was truncated in the current subtransaction, and
    it's safe in principle to apply the optimization. Due to an implementation
    artifact, we'll reject it anyway.
  • Bruce Momjian at Feb 1, 2013 at 5:57 pm

    On Tue, Jan 29, 2013 at 08:34:24PM -0500, Noah Misch wrote:
    On Fri, Jan 25, 2013 at 11:28:58PM -0500, Bruce Momjian wrote:
    On Fri, Jan 25, 2013 at 11:08:56PM -0500, Tom Lane wrote:
    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
    Well, here is an example of this message:

    BEGIN;
    TRUNCATE vistest;
    SAVEPOINT s1;
    COPY vistest FROM stdin CSV FREEZE;
    ERROR: cannot perform FREEZE because of previous table activity in the current transaction
    COMMIT;

    Clearly it was truncated in the same transaction, but the savepoint
    somehow invalidates the freeze. There is a C comment about it:
    The savepoint prevents the COPY FREEZE, because COPY FREEZE needs the table to
    have been created or truncated in the current *sub*transaction. Issuing
    "RELEASE s1" before the COPY makes it work again, for example.
    * BEGIN;
    * TRUNCATE t;
    * SAVEPOINT save;
    * TRUNCATE t;
    * ROLLBACK TO save;
    * COPY ...
    This is different. The table was truncated in the current subtransaction, and
    it's safe in principle to apply the optimization. Due to an implementation
    artifact, we'll reject it anyway.
    OK, so, should we change the error message:

      cannot perform FREEZE because of transaction activity after table
      creation or truncation

    to

      cannot perform FREEZE because the table was not created or
      truncated in the current subtransaction

    or do we need to keep the "transaction activity" weasel wording because
    of the second case you listed above? I am suspecting the later.

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

       + It's impossible for everything to be true. +
  • Noah Misch at Feb 2, 2013 at 2:51 pm

    On Fri, Feb 01, 2013 at 12:57:18PM -0500, Bruce Momjian wrote:
    On Tue, Jan 29, 2013 at 08:34:24PM -0500, Noah Misch wrote:
    On Fri, Jan 25, 2013 at 11:28:58PM -0500, Bruce Momjian wrote:
    BEGIN;
    TRUNCATE vistest;
    SAVEPOINT s1;
    COPY vistest FROM stdin CSV FREEZE;
    ERROR: cannot perform FREEZE because of previous table activity in the current transaction
    COMMIT;

    Clearly it was truncated in the same transaction, but the savepoint
    somehow invalidates the freeze. There is a C comment about it:
    The savepoint prevents the COPY FREEZE, because COPY FREEZE needs the table to
    have been created or truncated in the current *sub*transaction. Issuing
    "RELEASE s1" before the COPY makes it work again, for example.
    * BEGIN;
    * TRUNCATE t;
    * SAVEPOINT save;
    * TRUNCATE t;
    * ROLLBACK TO save;
    * COPY ...
    This is different. The table was truncated in the current subtransaction, and
    it's safe in principle to apply the optimization. Due to an implementation
    artifact, we'll reject it anyway.
    OK, so, should we change the error message:

    cannot perform FREEZE because of transaction activity after table
    creation or truncation

    to

    cannot perform FREEZE because the table was not created or
    truncated in the current subtransaction

    or do we need to keep the "transaction activity" weasel wording because
    of the second case you listed above? I am suspecting the later.
    Let's touch on the exception in passing by using the phrase "last truncated",
    giving this wording for both the second and the third COPY FREEZE error sites:

      cannot perform FREEZE because the table was not created or last
      truncated in the current subtransaction
  • Bruce Momjian at Feb 2, 2013 at 3:13 pm

    On Sat, Feb 2, 2013 at 09:51:13AM -0500, Noah Misch wrote:
    OK, so, should we change the error message:

    cannot perform FREEZE because of transaction activity after table
    creation or truncation

    to

    cannot perform FREEZE because the table was not created or
    truncated in the current subtransaction

    or do we need to keep the "transaction activity" weasel wording because
    of the second case you listed above? I am suspecting the later.
    Let's touch on the exception in passing by using the phrase "last truncated",
    giving this wording for both the second and the third COPY FREEZE error sites:

    cannot perform FREEZE because the table was not created or last
    truncated in the current subtransaction
    Well, so you are saying that there really isn't any use-visible logic
    for those messages to be different, i.e. that the transaction id can be
    set to invalid even if we created/truncated in the same transaction, but
    not the same subtransaction?

    The comparisons that trigger the two messages are:

      if (cstate->rel->rd_createSubid != InvalidSubTransactionId ||
          cstate->rel->rd_newRelfilenodeSubid != InvalidSubTransactionId)

    and
      if (cstate->rel->rd_createSubid != GetCurrentSubTransactionId() ||
          cstate->rel->rd_newRelfilenodeSubid != GetCurrentSubTransactionId())

    The first comparison is creation, the second, truncation.

    Please confirm and I will make the change, or you can.

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

       + It's impossible for everything to be true. +
  • Tom Lane at Feb 2, 2013 at 5:09 pm

    Bruce Momjian writes:
    Well, so you are saying that there really isn't any use-visible logic
    for those messages to be different,
    No, and in fact the whole block of code is badly written because it
    conflates two unrelated tests. I guess somebody was trying to save
    a couple of nanoseconds by not calling GetCurrentSubTransactionId
    if a previous test had failed, but really why should we care about
    that number of cycles in COPY preliminaries? The code ought to be
    more like this:

         /* comment about skipping FSM or WAL here */
         if (cstate->rel->rd_createSubid != InvalidSubTransactionId ||
             cstate->rel->rd_newRelfilenodeSubid != InvalidSubTransactionId)
         {
             hi_options |= HEAP_INSERT_SKIP_FSM;
             if (!XLogIsNeeded())
                 hi_options |= HEAP_INSERT_SKIP_WAL;
         }
         /* comment about when we can perform FREEZE here */
         if (cstate->freeze)
         {
             if (!ThereAreNoPriorRegisteredSnapshots() || !ThereAreNoReadyPortals())
                 ereport(ERROR,
                         (ERRCODE_INVALID_TRANSACTION_STATE,
                         errmsg("cannot perform FREEZE because of prior transaction activity")));

             if (cstate->rel->rd_createSubid != GetCurrentSubTransactionId() &&
                 cstate->rel->rd_newRelfilenodeSubid != GetCurrentSubTransactionId())
                     ereport(ERROR,
                             (ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE,
                             errmsg("cannot perform FREEZE because the table was not created or truncated in the current subtransaction")));
             hi_options |= HEAP_INSERT_FROZEN;
         }


        regards, tom lane
  • Bruce Momjian at Feb 2, 2013 at 5:56 pm

    On Sat, Feb 2, 2013 at 12:09:05PM -0500, Tom Lane wrote:
    Bruce Momjian <bruce@momjian.us> writes:
    Well, so you are saying that there really isn't any use-visible logic
    for those messages to be different,
    No, and in fact the whole block of code is badly written because it
    conflates two unrelated tests. I guess somebody was trying to save
    a couple of nanoseconds by not calling GetCurrentSubTransactionId
    if a previous test had failed, but really why should we care about
    that number of cycles in COPY preliminaries? The code ought to be
    more like this:

    /* comment about skipping FSM or WAL here */
    if (cstate->rel->rd_createSubid != InvalidSubTransactionId ||
    cstate->rel->rd_newRelfilenodeSubid != InvalidSubTransactionId)
    {
    hi_options |= HEAP_INSERT_SKIP_FSM;
    if (!XLogIsNeeded())
    hi_options |= HEAP_INSERT_SKIP_WAL;
    }
    /* comment about when we can perform FREEZE here */
    if (cstate->freeze)
    {
    if (!ThereAreNoPriorRegisteredSnapshots() || !ThereAreNoReadyPortals())
    ereport(ERROR,
    (ERRCODE_INVALID_TRANSACTION_STATE,
    errmsg("cannot perform FREEZE because of prior transaction activity")));

    if (cstate->rel->rd_createSubid != GetCurrentSubTransactionId() &&
    cstate->rel->rd_newRelfilenodeSubid != GetCurrentSubTransactionId())
    ereport(ERROR,
    (ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE,
    errmsg("cannot perform FREEZE because the table was not created or truncated in the current subtransaction")));
    hi_options |= HEAP_INSERT_FROZEN;
    }
    Yes, I found the blocking odd too --- the test for
    InvalidSubTransactionId is used by hi_options, and for freeze checking.
    I assumed "!= InvalidSubTransactionId" and "!=
    GetCurrentSubTransactionId()" had different meanings for freeze
    checking.

    I compounded the problem because originally there was no FREEZE failure
    so no action was taken if "!= InvalidSubTransactionId".

    Applied patch attached.

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

       + It's impossible for everything to be true. +
  • Noah Misch at Feb 2, 2013 at 7:27 pm

    On Sat, Feb 02, 2013 at 10:12:54AM -0500, Bruce Momjian wrote:
    On Sat, Feb 2, 2013 at 09:51:13AM -0500, Noah Misch wrote:
    Let's touch on the exception in passing by using the phrase "last truncated",
    giving this wording for both the second and the third COPY FREEZE error sites:

    cannot perform FREEZE because the table was not created or last
    truncated in the current subtransaction
    Well, so you are saying that there really isn't any use-visible logic
    for those messages to be different, i.e. that the transaction id can be
    set to invalid even if we created/truncated in the same transaction, but
    not the same subtransaction?
    Right. The latest committed code makes sense to me.
  • Michael Paquier at Jan 26, 2013 at 12:45 am

    On Sat, Jan 26, 2013 at 2:42 AM, Robert Haas wrote:
    On Fri, Jan 25, 2013 at 11:59 AM, Tom Lane 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

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
grouppgsql-hackers @
categoriespostgresql
postedJan 23, '13 at 7:02p
activeFeb 2, '13 at 7:27p
posts29
users9
websitepostgresql.org...
irc#postgresql

People

Translate

site design / logo © 2021 Grokbase