Обсуждение: [PATCH] PREPARE TRANSACTION unexpected behavior with TEMP TABLE

Поиск
Список
Период
Сортировка

[PATCH] PREPARE TRANSACTION unexpected behavior with TEMP TABLE

От
Himanshu Upadhyaya
Дата:
Hi,

While working on one of the issue, I have noticed below unexpected behavior with "PREPARE TRANSACTION".

We are getting this unexpected behavior with PREPARE TRANSACTION when it is mixed with Temporary Objects. Please consider the below setup and SQL block.

set max_prepared_transactions to 1 (or any non zero value), this is to enable the “prepare transaction”.

Now please try to run the below set of statements.
[BLOCK-1:]
postgres=# create temp table fullname (first text, last text);
CREATE TABLE
postgres=# BEGIN;
BEGIN
postgres=*# create function longname(fullname) returns text language sql
postgres-*# as $$select $1.first || ' ' || $1.last$$;
CREATE FUNCTION
postgres=*# prepare transaction 'mytran';
ERROR:  cannot PREPARE a transaction that has operated on temporary objects

Above error is expected.

The problem is if we again try to create the same function in the “PREPARE TRANSACTION” as below.

[BLOCK-2:]
postgres=# BEGIN;
BEGIN
postgres=*# create function longname(fullname) returns text language sql
as $$select $1.first || ' ' || $1.last$$;
CREATE FUNCTION
postgres=*# PREPARE transaction 'mytran';
PREPARE TRANSACTION

Now, this time we succeed and not getting the above error (”ERROR:  cannot PREPARE a transaction that has operated on temporary objects), like the way we were getting with BLOCK-1

This is happening because we set MyXactFlags in relation_open function call, and here relation_open is getting called from load_typcache_tupdesc, but in the second run of “create function…” in the above #2 block will not call load_typcache_tupdesc because of the below condition(typentry->tupDesc == NULL) in  lookup_type_cache().

        /*
         * If it's a composite type (row type), get tupdesc if requested
         */
        if ((flags & TYPECACHE_TUPDESC) &&
                typentry->tupDesc == NULL &&
                typentry->typtype == TYPTYPE_COMPOSITE)
        {
                load_typcache_tupdesc(typentry);
        }

We set typentry->tupDesc to non NULL(and populates it with proper tuple descriptor in the cache) value during our first call to “create function…” in BLOCK-1.
We have logic in file xact.c::PrepareTransaction() to simply error out if we have accessed any temporary object in the current transaction but because of the above-described issue of not setting XACT_FLAGS_ACCESSEDTEMPNAMESPACE in MyXactFlags second run of “create function..” Works and PREPARE TRANSACTION succeeds(but it should fail).

Please find attached the proposed patch to FIX this issue.

Thoughts?

Thanks,
Himanshu Upadhyaya
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Вложения

Re: [PATCH] PREPARE TRANSACTION unexpected behavior with TEMP TABLE

От
vignesh C
Дата:
On Tue, Apr 6, 2021 at 8:18 PM Himanshu Upadhyaya
<upadhyaya.himanshu@gmail.com> wrote:
>
> Hi,
>
> While working on one of the issue, I have noticed below unexpected behavior with "PREPARE TRANSACTION".
>
> We are getting this unexpected behavior with PREPARE TRANSACTION when it is mixed with Temporary Objects. Please
considerthe below setup and SQL block. 
>
> set max_prepared_transactions to 1 (or any non zero value), this is to enable the “prepare transaction”.
>
> Now please try to run the below set of statements.
> [BLOCK-1:]
> postgres=# create temp table fullname (first text, last text);
> CREATE TABLE
> postgres=# BEGIN;
> BEGIN
> postgres=*# create function longname(fullname) returns text language sql
> postgres-*# as $$select $1.first || ' ' || $1.last$$;
> CREATE FUNCTION
> postgres=*# prepare transaction 'mytran';
> ERROR:  cannot PREPARE a transaction that has operated on temporary objects
>
> Above error is expected.
>
> The problem is if we again try to create the same function in the “PREPARE TRANSACTION” as below.
>
> [BLOCK-2:]
> postgres=# BEGIN;
> BEGIN
> postgres=*# create function longname(fullname) returns text language sql
> as $$select $1.first || ' ' || $1.last$$;
> CREATE FUNCTION
> postgres=*# PREPARE transaction 'mytran';
> PREPARE TRANSACTION
>
> Now, this time we succeed and not getting the above error (”ERROR:  cannot PREPARE a transaction that has operated on
temporaryobjects), like the way we were getting with BLOCK-1 
>
> This is happening because we set MyXactFlags in relation_open function call, and here relation_open is getting called
fromload_typcache_tupdesc, but in the second run of “create function…” in the above #2 block will not call
load_typcache_tupdescbecause of the below condition(typentry->tupDesc == NULL) in  lookup_type_cache(). 
>
>         /*
>          * If it's a composite type (row type), get tupdesc if requested
>          */
>         if ((flags & TYPECACHE_TUPDESC) &&
>                 typentry->tupDesc == NULL &&
>                 typentry->typtype == TYPTYPE_COMPOSITE)
>         {
>                 load_typcache_tupdesc(typentry);
>         }
>
> We set typentry->tupDesc to non NULL(and populates it with proper tuple descriptor in the cache) value during our
firstcall to “create function…” in BLOCK-1. 
> We have logic in file xact.c::PrepareTransaction() to simply error out if we have accessed any temporary object in
thecurrent transaction but because of the above-described issue of not setting XACT_FLAGS_ACCESSEDTEMPNAMESPACE in
MyXactFlagssecond run of “create function..” Works and PREPARE TRANSACTION succeeds(but it should fail). 
>
> Please find attached the proposed patch to FIX this issue.

I was able to reproduce the issue with your patch and your patch fixes
the issue.

Few comments:
1) We can drop the table after this test.
+CREATE TEMP TABLE temp_tbl (first TEXT, last TEXT);
+BEGIN;
+CREATE FUNCTION longname(temp_tbl) RETURNS TEXT LANGUAGE SQL
+AS $$SELECT $1.first || ' ' || $1.last$$;
+PREPARE TRANSACTION 'temp_tbl_access';
+
+BEGIN;
+CREATE FUNCTION longname(temp_tbl) RETURNS TEXT LANGUAGE SQL
+AS $$SELECT $1.first || ' ' || $1.last$$;
+PREPARE TRANSACTION 'temp_tbl_access';

2) +-- Test for accessing Temporary table
+-- in prepare transaction.
can be changed to
-- Test for accessing cached temporary table in a prepared transaction.

3) +-- These cases must fail and generate errors about Temporary objects.
can be changed to
-- These cases should fail with cannot access temporary object error.

Regards,
Vignesh



Re: [PATCH] PREPARE TRANSACTION unexpected behavior with TEMP TABLE

От
Himanshu Upadhyaya
Дата:
Hi Vignesh,
Thanks for sharing the review comments. Please find my response below.
1) We can drop the table after this test.
Done.
2) +-- Test for accessing Temporary table
+-- in prepare transaction.
can be changed to
-- Test for accessing cached temporary table in a prepared transaction.
Comment is now modified as above.
3) +-- These cases must fail and generate errors about Temporary objects.
can be changed to
-- These cases should fail with cannot access temporary object error.
The error is not about accessing the temporary object rather it's about disallowing create transaction as it is referring to the temporary objects.
I have changed it with the exact error we get in those cases.

Please find attached the V2 patch.

Thanks,
Himanshu

On Wed, Apr 7, 2021 at 4:11 PM vignesh C <vignesh21@gmail.com> wrote:
On Tue, Apr 6, 2021 at 8:18 PM Himanshu Upadhyaya
<upadhyaya.himanshu@gmail.com> wrote:
>
> Hi,
>
> While working on one of the issue, I have noticed below unexpected behavior with "PREPARE TRANSACTION".
>
> We are getting this unexpected behavior with PREPARE TRANSACTION when it is mixed with Temporary Objects. Please consider the below setup and SQL block.
>
> set max_prepared_transactions to 1 (or any non zero value), this is to enable the “prepare transaction”.
>
> Now please try to run the below set of statements.
> [BLOCK-1:]
> postgres=# create temp table fullname (first text, last text);
> CREATE TABLE
> postgres=# BEGIN;
> BEGIN
> postgres=*# create function longname(fullname) returns text language sql
> postgres-*# as $$select $1.first || ' ' || $1.last$$;
> CREATE FUNCTION
> postgres=*# prepare transaction 'mytran';
> ERROR:  cannot PREPARE a transaction that has operated on temporary objects
>
> Above error is expected.
>
> The problem is if we again try to create the same function in the “PREPARE TRANSACTION” as below.
>
> [BLOCK-2:]
> postgres=# BEGIN;
> BEGIN
> postgres=*# create function longname(fullname) returns text language sql
> as $$select $1.first || ' ' || $1.last$$;
> CREATE FUNCTION
> postgres=*# PREPARE transaction 'mytran';
> PREPARE TRANSACTION
>
> Now, this time we succeed and not getting the above error (”ERROR:  cannot PREPARE a transaction that has operated on temporary objects), like the way we were getting with BLOCK-1
>
> This is happening because we set MyXactFlags in relation_open function call, and here relation_open is getting called from load_typcache_tupdesc, but in the second run of “create function…” in the above #2 block will not call load_typcache_tupdesc because of the below condition(typentry->tupDesc == NULL) in  lookup_type_cache().
>
>         /*
>          * If it's a composite type (row type), get tupdesc if requested
>          */
>         if ((flags & TYPECACHE_TUPDESC) &&
>                 typentry->tupDesc == NULL &&
>                 typentry->typtype == TYPTYPE_COMPOSITE)
>         {
>                 load_typcache_tupdesc(typentry);
>         }
>
> We set typentry->tupDesc to non NULL(and populates it with proper tuple descriptor in the cache) value during our first call to “create function…” in BLOCK-1.
> We have logic in file xact.c::PrepareTransaction() to simply error out if we have accessed any temporary object in the current transaction but because of the above-described issue of not setting XACT_FLAGS_ACCESSEDTEMPNAMESPACE in MyXactFlags second run of “create function..” Works and PREPARE TRANSACTION succeeds(but it should fail).
>
> Please find attached the proposed patch to FIX this issue.

I was able to reproduce the issue with your patch and your patch fixes
the issue.

Few comments:
1) We can drop the table after this test.
+CREATE TEMP TABLE temp_tbl (first TEXT, last TEXT);
+BEGIN;
+CREATE FUNCTION longname(temp_tbl) RETURNS TEXT LANGUAGE SQL
+AS $$SELECT $1.first || ' ' || $1.last$$;
+PREPARE TRANSACTION 'temp_tbl_access';
+
+BEGIN;
+CREATE FUNCTION longname(temp_tbl) RETURNS TEXT LANGUAGE SQL
+AS $$SELECT $1.first || ' ' || $1.last$$;
+PREPARE TRANSACTION 'temp_tbl_access';

2) +-- Test for accessing Temporary table
+-- in prepare transaction.
can be changed to
-- Test for accessing cached temporary table in a prepared transaction.

3) +-- These cases must fail and generate errors about Temporary objects.
can be changed to
-- These cases should fail with cannot access temporary object error.

Regards,
Vignesh
Вложения

Re: [PATCH] PREPARE TRANSACTION unexpected behavior with TEMP TABLE

От
Robert Haas
Дата:
On Wed, Apr 7, 2021 at 12:19 PM Himanshu Upadhyaya
<upadhyaya.himanshu@gmail.com> wrote:
> Please find attached the V2 patch.

Hi,

This patch is essentially taking the position that calling
load_typcache_tupdesc before using that tupdesc for anything is
required for correctness. I'm not sure whether that's a good
architectural decision: to me, it looks like whoever wrote this code
originally - I think it was Tom - had the idea that it would be OK to
skip calling that function whenever we already have the value.
Changing that has some small performance cost, and it also just looks
kind of weird, because you don't expect a function called
load_typcache_tupdesc() to have the side effect of preventing some
kind of bad thing from happening. You just expect it to be loading
stuff. The comments in this code are not exactly stellar as things
stand, but the patch also doesn't update them in a meaningful way.
Sure, it corrects a few comments that would be flat-out wrong
otherwise, but it doesn't add any kind of explanation that would help
the next person who looks at this code understand why they shouldn't
just put back the exact same performance optimization you're proposing
to rip out.

An alternative design would be to find some new place to set
XACT_FLAGS_ACCESSEDTEMPNAMESPACE. For example, we could set a flag in
the TypeCacheEntry indicating whether this flag ought to be set when
somebody looks up the entry.

But, before we get too deeply into what the design should be, I think
we need to be clear about what problem we're trying to fix. I agree
that the behavior you demonstrate in your example looks inconsistent,
but that doesn't necessarily mean that the code is wrong. What exactly
is the code trying to prohibit here, and does this test case really
show that principle being violated? The comments in
PrepareTransaction() justify this prohibition by saying that "Having
the prepared xact hold locks on another backend's temp table seems a
bad idea --- for instance it would prevent the backend from exiting.
There are other problems too, such as how to clean up the source
backend's local buffers and ON COMMIT state if the prepared xact
includes a DROP of a temp table." But, in this case, none of that
stuff actually happens. If I run your test case without the patch, the
backend has no problem exiting, and the prepared xact holds no lock on
the temp table, and the prepared xact does not include a DROP of a
temp table. That's not to say that everything is great, because after
starting a new session and committing mytran, this happens:

rhaas=# \df longname
ERROR:  cache lookup failed for type 16432

But the patch doesn't actually prevent that from happening, because
even with the patch applied I can still recreate the same failure
using a different sequence of steps:

[ session 1 ]
rhaas=# create temp table fullname (first text, last text);
CREATE TABLE

[ session 2 ]
rhaas=# select oid::regclass from pg_class where relname = 'fullname';
        oid
--------------------
 pg_temp_3.fullname
(1 row)

rhaas=# begin;
BEGIN
rhaas=*# create function longname(pg_temp_3.fullname) returns text
language sql as $$select $1.first || ' ' || $1.last$$;
CREATE FUNCTION

[ session 1 ]
rhaas=# \q

[ session 2 ]
rhaas=*# commit;
COMMIT
rhaas=# \df longname
ERROR:  cache lookup failed for type 16448

To really fix this, you'd need CREATE FUNCTION to take a lock on the
containing namespace, whether permanent or temporary. You'd also need
every other CREATE statement that creates a schema-qualified object to
do the same thing. Maybe that's a good idea, but we've been reluctant
to go that far in the past due to performance consequences, and it's
not clear whether any of those problems are related to the issue that
prompted you to submit the patch. So, I'm kind of left wondering what
exactly you're trying to solve here. Can you clarify?

-- 
Robert Haas
EDB: http://www.enterprisedb.com



Re: [PATCH] PREPARE TRANSACTION unexpected behavior with TEMP TABLE

От
Himanshu Upadhyaya
Дата:
Hi Robert,

Thanks for sharing your thoughts.
The purpose of this FIX is to mainly focus on getting consistent behavior
with PREPARE TRANSACTION. With the case that I had mentioned
previously, my expectation was either both PREPARE TRANSACTION should fail
or both should succeed but here second same "PREPARE TRANSACTION" was
successful however first one was failing with an error, which is kind of weird to me.


I have also tried to reproduce the behavior.

[session:1]
postgres=# create temp table fullname (first text, last text);
CREATE TABLE

[session:2]
postgres=# select oid::regclass from pg_class where relname = 'fullname';
        oid        
--------------------
 pg_temp_3.fullname

postgres=# BEGIN;
create function longname1(pg_temp_3.fullname) returns text language sql
as $$select $1.first || ' ' || $1.last$$;
BEGIN
CREATE FUNCTION
postgres=*# prepare transaction 'mytran2';
ERROR:  cannot PREPARE a transaction that has operated on temporary objects
postgres=# BEGIN;                        
create function longname1(pg_temp_3.fullname) returns text language sql
as $$select $1.first || ' ' || $1.last$$;
BEGIN
CREATE FUNCTION

[session:1]
postgres=# \q                        // no problem in exiting

[session:2]

postgres=*# prepare transaction 'mytran2';
PREPARE TRANSACTION
postgres=# \df
ERROR:  cache lookup failed for type 16429

looking at the comment in the code [session:1] should hang while exiting but
I don't see a problem here, you have already explained that in your reply.
Even then I feel that behavior should be consistent when we mix temporary
objects in PREPARE TRANSACTION.

The comments in
PrepareTransaction() justify this prohibition by saying that "Having
the prepared xact hold locks on another backend's temp table seems
a bad idea --- for instance it would prevent the backend from exiting.
There are other problems too, such as how to clean up the source
backend's local buffers and ON COMMIT state if the prepared xact
includes a DROP of a temp table."
I can see from the above experiment that there is no problem with the
lock in the above case but not sure if there is any issue with "clean up
the source backend's local buffers", if not then we don't even need this
ERROR (ERROR:  cannot PREPARE a transaction that has operated
on temporary objects) in PREPARE TRANSACTION?

To really fix this, you'd need CREATE FUNCTION to take a lock on the
containing namespace, whether permanent or temporary. You'd also need
every other CREATE statement that creates a schema-qualified object to
do the same thing. Maybe that's a good idea, but we've been reluctant
to go that far in the past due to performance consequences, and it's
not clear whether any of those problems are related to the issue that
prompted you to submit the patch.
Yes, the purpose of this patch is to actually have a valid value in
XACT_FLAGS_ACCESSEDTEMPNAMESPACE, having said that it should
always be true if we access temporary object else false.
Even if we do changes to have lock in case of "CREATE FUNCTION", we
also need to have this FIX in place so that "PREPARE TRANSACTION"
mixed with TEMPORARY OBJECT will always be restricted and will not
cause any hang issue(which we will start observing once we implement
these "CREATE STATEMENT" changes) as mentioned in the comment
in the PrepareTransaction().

Just thinking if it's acceptable to FIX this and make it consistent by properly
setting XACT_FLAGS_ACCESSEDTEMPNAMESPACE, so that it should always
fail if we access the temporary object, I also agree here that it will not actually cause
any issue because of xact lock but then from user perspective it seems weird
when the same PREPARE TRANSACTION is working second time onwards, thoughts?

Thanks,
Himanshu


On Thu, Apr 8, 2021 at 7:43 PM Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Apr 7, 2021 at 12:19 PM Himanshu Upadhyaya
<upadhyaya.himanshu@gmail.com> wrote:
> Please find attached the V2 patch.

Hi,

This patch is essentially taking the position that calling
load_typcache_tupdesc before using that tupdesc for anything is
required for correctness. I'm not sure whether that's a good
architectural decision: to me, it looks like whoever wrote this code
originally - I think it was Tom - had the idea that it would be OK to
skip calling that function whenever we already have the value.
Changing that has some small performance cost, and it also just looks
kind of weird, because you don't expect a function called
load_typcache_tupdesc() to have the side effect of preventing some
kind of bad thing from happening. You just expect it to be loading
stuff. The comments in this code are not exactly stellar as things
stand, but the patch also doesn't update them in a meaningful way.
Sure, it corrects a few comments that would be flat-out wrong
otherwise, but it doesn't add any kind of explanation that would help
the next person who looks at this code understand why they shouldn't
just put back the exact same performance optimization you're proposing
to rip out.

An alternative design would be to find some new place to set
XACT_FLAGS_ACCESSEDTEMPNAMESPACE. For example, we could set a flag in
the TypeCacheEntry indicating whether this flag ought to be set when
somebody looks up the entry.

But, before we get too deeply into what the design should be, I think
we need to be clear about what problem we're trying to fix. I agree
that the behavior you demonstrate in your example looks inconsistent,
but that doesn't necessarily mean that the code is wrong. What exactly
is the code trying to prohibit here, and does this test case really
show that principle being violated? The comments in
PrepareTransaction() justify this prohibition by saying that "Having
the prepared xact hold locks on another backend's temp table seems a
bad idea --- for instance it would prevent the backend from exiting.
There are other problems too, such as how to clean up the source
backend's local buffers and ON COMMIT state if the prepared xact
includes a DROP of a temp table." But, in this case, none of that
stuff actually happens. If I run your test case without the patch, the
backend has no problem exiting, and the prepared xact holds no lock on
the temp table, and the prepared xact does not include a DROP of a
temp table. That's not to say that everything is great, because after
starting a new session and committing mytran, this happens:

rhaas=# \df longname
ERROR:  cache lookup failed for type 16432

But the patch doesn't actually prevent that from happening, because
even with the patch applied I can still recreate the same failure
using a different sequence of steps:

[ session 1 ]
rhaas=# create temp table fullname (first text, last text);
CREATE TABLE

[ session 2 ]
rhaas=# select oid::regclass from pg_class where relname = 'fullname';
        oid
--------------------
 pg_temp_3.fullname
(1 row)

rhaas=# begin;
BEGIN
rhaas=*# create function longname(pg_temp_3.fullname) returns text
language sql as $$select $1.first || ' ' || $1.last$$;
CREATE FUNCTION

[ session 1 ]
rhaas=# \q

[ session 2 ]
rhaas=*# commit;
COMMIT
rhaas=# \df longname
ERROR:  cache lookup failed for type 16448

To really fix this, you'd need CREATE FUNCTION to take a lock on the
containing namespace, whether permanent or temporary. You'd also need
every other CREATE statement that creates a schema-qualified object to
do the same thing. Maybe that's a good idea, but we've been reluctant
to go that far in the past due to performance consequences, and it's
not clear whether any of those problems are related to the issue that
prompted you to submit the patch. So, I'm kind of left wondering what
exactly you're trying to solve here. Can you clarify?

--
Robert Haas
EDB: http://www.enterprisedb.com

Re: [PATCH] PREPARE TRANSACTION unexpected behavior with TEMP TABLE

От
Robert Haas
Дата:
On Wed, Apr 14, 2021 at 6:45 AM Himanshu Upadhyaya
<upadhyaya.himanshu@gmail.com> wrote:
> The purpose of this FIX is to mainly focus on getting consistent behavior
> with PREPARE TRANSACTION. With the case that I had mentioned
> previously, my expectation was either both PREPARE TRANSACTION should fail
> or both should succeed but here second same "PREPARE TRANSACTION" was
> successful however first one was failing with an error, which is kind of weird to me.

I agree that it's weird, but that doesn't mean that your patch is an
improvement, and I don't think it is. If we could make this thing more
consistent without incurring any negatives, I'd be in favor of that.
But the patch does have some negatives, which in my opinion are more
substantial than the problem you're trying to fix. Namely, possible
performance consequences, and undocumented and fragile assumptions
that, as it seems to me, may easily get broken in the future. I see
that you've repeatedly capitalized the word FIX in your reply, but
it's just not that simple. If this had really bad consequences like
corrupting data or crashing the server then it would be essential to
do something about it, but so far the worst consequence you've
indicated is that an obscure sequence of SQL commands that no real
user is likely to issue produces a slightly surprising result. That's
not great, but neither is it an emergency.

> I have also tried to reproduce the behavior.

Your test case isn't ideal for reproducing the problem that the
comment is worrying about. Normally, when we take a lock on a table,
we hold it until commit. But, that only applies when we run a query
that mentions the table, like a SELECT or an UPDATE. In your case, we
only end up opening the table to build a relcache entry for it, so
that we can look at the metadata. And, catalog scans used to build
syscache and relcache entries release locks immediately, rather than
waiting until the end of the transaction. So it might be that if we
failed to ever set XACT_FLAGS_ACCESSEDTEMPNAMESPACE in your test case,
everything would be fine.

That doesn't seem to be true in general though. I tried changing
"cannot PREPARE a transaction that has operated on temporary objects"
from an ERROR to a NOTICE and then ran 'make check'. It hung. I think
this test case is the same problem as the regression tests hit; in any
case, it also hangs:

rhaas=# begin;
BEGIN
rhaas=*# create temp table first ();
CREATE TABLE
rhaas=*# prepare transaction 'whatever';
NOTICE:  cannot PREPARE a transaction that has operated on temporary objects
PREPARE TRANSACTION
rhaas=# create temp table second ();
[ hangs ]

I haven't checked, but I think the problem here is that the first
transaction had to create this backend's pg_temp schema and the second
one can't see the results of the first one doing it so it wants to do
the same thing and that results in waiting for a lock the prepared
transaction already holds. I made a quick attempt to reproduce a hang
at backend exit time, but couldn't find a case where that happened.
That doesn't mean there isn't one, though. There's a very good chance
that the person who wrote that comment knew that a real problem
existed, and just didn't describe it well enough for you or I to
immediately know what it is. It is also possible that they were
completely wrong, or that things have changed since the comment was
written, but we can't assume that without considerably more research
and analysis than either of us has done so far.

I think one point to take away here is that question of whether a
temporary relation has been "accessed" is not all black and white. If
I ran a SELECT statement against a relation, I think it's clear that
I've accessed it. But, if I just used the name of that relation as a
type name in some other SQL command, did I really access it? The
current code's answer to that is that if we had to open and lock the
relation to get its metadata, then we accessed it, and if we already
had the details that we needed in cache, then we did not access it.
Now, again, I agree that looks a little weird from a user point of
view, but looking at the implementation, you can kind of see why it
ends up like that. From a certain point of view, it would be more
surprising if we never opened or locked the relation and yet ended up
deciding that we'd accessed it. Now maybe we should further explore
going the other direction and avoiding setting the flag at all in this
case, since I think we're neither retaining a lock nor touching any
relation buffers, but I think that needs more analysis. Even if we
decide that's safe, there's still the problem of finding a better
implementation that's not overly complex for what really feels like a
very minor issue.

--
Robert Haas
EDB: http://www.enterprisedb.com