Re: [HACKERS] md.c is feeling much better now, thank you

> I have just committed changes that address the problem of relcache
> entries not being updated promptly after another backend issues
> a shared-invalidation report. LockRelation() now checks for sinval
> reports just after acquiring a lock, and the relcache entry will be
> updated if necessary --- or, if the relation has actually disappeared
> entirely, an elog(ERROR) will occur.
>
> As a side effect of the relcache update, the underlying md.c/fd.c files
> will be closed, and then reopened if necessary. This should solve our
> concerns about vacuum.c not being able to truncate relations safely.
>
> There is still some potential for misbehavior as a result of the fact
> that the parser looks at relcache entries without bothering to obtain
> any kind of lock on the relation. For example:
>
> -- In backend #1:
> regression=> create table z1 (f1 int4);
> CREATE
> regression=> select * from z1;
> f1
> --
> (0 rows)
>
> regression=> begin;
> BEGIN
>
> -- In backend #2:
> regression=> alter table z1 add column f2 int4;
> ADD
> regression=>
>
> -- In backend #1:
> regression=> select * from z1;
> f1
> --
> (0 rows)
>
> -- parser uses un-updated relcache entry and sees only one column in z1.
> -- However, the relcache *will* get updated as soon as we either lock a
> -- table or do the CommandCounterIncrement() at end of query, so a second
> -- try sees the new info:
> regression=> select * from z1;
> f1|f2
> --+--
> (0 rows)
>
> regression=> end;
> END
>
> The window for problems is pretty small: you have to be within a
> transaction (otherwise the StartTransaction will notice the sinval
> report), and your very first query after the other backend does
> ALTER TABLE has to reference the altered table. So I'm not sure
> this is worth worrying about. But perhaps the parser ought to obtain
> the weakest possible lock on each table referenced in a query before
> it does any looking at the attributes of the table. Comments?

Ok. I will give another example that seems more serious.

test=> begin;
BEGIN
test=> create table t1(i int);
CREATE
-- a table file named "t1" is created.
test=> aaa;
ERROR: parser: parse error at or near "aaa"
-- transaction is aborted and the table file t1 is unlinked.
test=> select * from t1;
-- but parser doesn't know t1 does not exist any more.
-- it tries to open t1 using mdopen(). (see including backtrace)
-- mdopen() tries to open t1 and fails. In this case mdopen()
-- creates t1!
NOTICE: (transaction aborted): queries ignored until END
*ABORT STATE*
test=> end;
END
test=> create table t1(i int);
ERROR: cannot create t1
-- since relation file t1 already exists.
test=>
EOF
[t-ishii(at)ext16 src]$ !!
psql test
Welcome to the POSTGRESQL interactive sql monitor:
Please read the file COPYRIGHT for copyright terms of POSTGRESQL
[PostgreSQL 6.6.0 on powerpc-unknown-linux-gnu, compiled by gcc egcs-2.90.25 980302 (egcs-1.0.2 prerelease)]

type \? for help on slash commands
type \q to quit
type \g or terminate with semicolon to execute query
You are currently connected to the database: test

test=> select * from t1;

ERROR: t1: Table does not exist.
test=> create table t1(i int);

ERROR: cannot create t1
-- again, since relation file t1 already exists.
-- user would never be able to create t1!

I think the long range solution would be let parser obtain certain
locks as Tom said. Until that I propose following solution. It looks
simple, safe and would be neccessary anyway (I don't know why that
check had not been implemented). See included patches.

---------------------------- backtrace -----------------------------
#0 mdopen (reln=0x1a9af18) at md.c:279
#1 0x18cb784 in smgropen (which=425, reln=0xbfffdef0) at smgr.c:185
#2 0x18cb784 in smgropen (which=0, reln=0x1a9af18) at smgr.c:185
#3 0x1904c1c in RelationBuildDesc ()
#4 0x1905360 in RelationNameGetRelation ()
#5 0x18259a4 in heap_openr ()
#6 0x187f59c in addRangeTableEntry ()
#7 0x1879cb0 in transformTableEntry ()
#8 0x1879d40 in parseFromClause ()
#9 0x1879a90 in makeRangeTable ()
#10 0x1871fd8 in transformSelectStmt ()
#11 0x1870d14 in transformStmt ()
#12 0x18709e0 in parse_analyze ()
#13 0x18792d4 in parser ()
#14 0x18cd158 in pg_parse_and_plan ()
#15 0x18cd5c0 in pg_exec_query_dest ()
#16 0x18cd524 in pg_exec_query ()
#17 0x18ce9ac in PostgresMain ()
#18 0x18a5994 in DoBackend ()
#19 0x18a53c8 in BackendStartup ()
#20 0x18a46d0 in ServerLoop ()
#21 0x18a4108 in PostmasterMain ()
#22 0x1870928 in main ()
---------------------------- backtrace -----------------------------

---------------------------- patches -----------------------------
*** md.c~ Sun Sep 5 08:41:28 1999
--- md.c Sun Sep 5 11:01:57 1999
***************
*** 286,296 ****
--- 286,303 ----

/* this should only happen during bootstrap processing */
if (fd < 0)
+ {
+ if (!IsBootstrapProcessingMode())
+ {
+ elog(ERROR,"Couldn't open %s\n", path);
+ return -1;
+ }
#ifndef __CYGWIN32__
fd = FileNameOpenFile(path, O_RDWR | O_CREAT | O_EXCL, 0600);
#else
fd = FileNameOpenFile(path, O_RDWR | O_CREAT | O_EXCL | O_BINARY, 0600);
#endif
+ }

vfd = _fdvec_alloc();
if (vfd < 0)