Horton

Thread

Commits

  1. Simplify restriction handling of two-phase commit for temporary objects

  1. Simplify set of flags used by MyXactFlags

    Michael Paquier <michael@paquier.xyz> — 2019-01-18T05:31:26Z

    Hi all,

c5660e0 has introduced a new flag for MyXactFlags to restrict
two-phase commit from working with temporary objects, and as a matter
of fact XACT_FLAGS_ACCESSEDTEMPREL has been kept around to keep the
error handling message compatible with past versions, still it is
weird to keep both ACCESSEDTEMPNAMESPACE and ACCESSEDTEMPREL as the
former implies the latter, so attached is a cleanup patch for HEAD.

Keeping both messages makes the error handling at PREPARE time perhaps
a bit cleaner to make the difference about schema-level access or
table-level access, still I'd rather simplify the code and just only
keep the schema-level change as something we complain about.  Another
thing is that ACCESSEDTEMPREL is used in PreCommit_on_commit_actions()
to avoid the truncates of ON COMMIT DELETE ROWS if no temporary tables
have been accessed, still it would just mean that truncation would be
tried on empty tables for nothing even if say a function is created in
pg_temp.

Thoughts?
--
Michael

  2. Re: Simplify set of flags used by MyXactFlags

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-01-21T17:58:39Z

    On 2019-Jan-18, Michael Paquier wrote:

> Keeping both messages makes the error handling at PREPARE time perhaps
> a bit cleaner to make the difference about schema-level access or
> table-level access, still I'd rather simplify the code and just only
> keep the schema-level change as something we complain about.  Another
> thing is that ACCESSEDTEMPREL is used in PreCommit_on_commit_actions()
> to avoid the truncates of ON COMMIT DELETE ROWS if no temporary tables
> have been accessed, still it would just mean that truncation would be
> tried on empty tables for nothing even if say a function is created in
> pg_temp.

"... operated on temp namespace" doesn't look good; seems to me to be
missing an article, for one thing, but really I'm not sure that
'namespace' is the term to be using here.  I'd say "... operated on
temporary objects" instead (the plural avoids the need for the article;
and the user doesn't really care about the namespace itself but rather
about the objects it contains.)

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

  3. Re: Simplify set of flags used by MyXactFlags

    Michael Paquier <michael@paquier.xyz> — 2019-01-22T00:31:56Z

    On Mon, Jan 21, 2019 at 02:58:39PM -0300, Alvaro Herrera wrote:
> "... operated on temp namespace" doesn't look good; seems to me to be
> missing an article, for one thing, but really I'm not sure that
> 'namespace' is the term to be using here.  I'd say "... operated on
> temporary objects" instead (the plural avoids the need for the article;
> and the user doesn't really care about the namespace itself but rather
> about the objects it contains.)

Thanks for the input.  Would you prefer something like the attached
then?  I have switched the error message to "temporary objects", and
renamed the flag to XACT_FLAGS_ACCESSEDTEMPOBJECTS.
--
Michael

  4. Re: Simplify set of flags used by MyXactFlags

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-01-24T11:56:05Z

    On 2019-Jan-22, Michael Paquier wrote:

> On Mon, Jan 21, 2019 at 02:58:39PM -0300, Alvaro Herrera wrote:
> > "... operated on temp namespace" doesn't look good; seems to me to be
> > missing an article, for one thing, but really I'm not sure that
> > 'namespace' is the term to be using here.  I'd say "... operated on
> > temporary objects" instead (the plural avoids the need for the article;
> > and the user doesn't really care about the namespace itself but rather
> > about the objects it contains.)
> 
> Thanks for the input.  Would you prefer something like the attached
> then?  I have switched the error message to "temporary objects", and
> renamed the flag to XACT_FLAGS_ACCESSEDTEMPOBJECTS.

Uh, I didn't think it was necessary nor desirable to rename the flag,
only the user-visible message.

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

  5. Re: Simplify set of flags used by MyXactFlags

    Michael Paquier <michael@paquier.xyz> — 2019-01-25T10:19:32Z

    On Thu, Jan 24, 2019 at 08:56:05AM -0300, Alvaro Herrera wrote:
> Uh, I didn't think it was necessary nor desirable to rename the flag,
> only the user-visible message.

Oh, OK.  I have overstated your comment then.  Are you fine with the
attached instead?  The flag name remains the same, and the comment is
updated.
--
Michael

  6. Re: Simplify set of flags used by MyXactFlags

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-01-25T15:30:39Z

    On 2019-Jan-25, Michael Paquier wrote:

> On Thu, Jan 24, 2019 at 08:56:05AM -0300, Alvaro Herrera wrote:
> > Uh, I didn't think it was necessary nor desirable to rename the flag,
> > only the user-visible message.
> 
> Oh, OK.  I have overstated your comment then.  Are you fine with the
> attached instead?  The flag name remains the same, and the comment is
> updated.

I am, except that the change of "table" to "object" in xact.c line 2262
makes the new paragraph read a bit weird -- it's now saying "if we've
added a temp object ...   Other objects have the same problem".  Doesn't
make sense.  If you keep that word as "table", it works fine.

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

  7. Re: Simplify set of flags used by MyXactFlags

    Michael Paquier <michael@paquier.xyz> — 2019-01-26T03:05:42Z

    On Fri, Jan 25, 2019 at 12:30:39PM -0300, Alvaro Herrera wrote:
> I am, except that the change of "table" to "object" in xact.c line 2262
> makes the new paragraph read a bit weird -- it's now saying "if we've
> added a temp object ...   Other objects have the same problem".  Doesn't
> make sense.  If you keep that word as "table", it works fine.

Indeed.  Committed with this suggestion.  Thanks for the review.
--
Michael