Обсуждение: BUG #17376: Adding unique column with a function() default results in "could not read block 0 in file" error

The following bug has been logged on the website:

Bug reference:      17376
Logged by:          akg
Email address:      adrien.gilmore+pg@gmail.com
PostgreSQL version: 13.5
Operating system:   Linux
Description:

Hello,

SQL demonstrating the issue on 13.5 is below. 

--
BEGIN;
CREATE TABLE t1 (id SERIAL PRIMARY KEY);
INSERT INTO t1 VALUES (default);

CREATE FUNCTION myfunc() RETURNS TEXT LANGUAGE plpgsql AS $$
  BEGIN
    SELECT r FROM t1;
    RETURN random()::text;
END $$;

ALTER TABLE t1 ADD COLUMN r TEXT NOT NULL UNIQUE DEFAULT myfunc();
--

Results in the error:
ERROR:  could not read block 0 in file "base/84505/84705": read only 0 of
8192 bytes

The error message content returned is what I suspect of being a bug, not so
much that this SQL didn't work.

On Fri, 21 Jan 2022 at 17:22, PG Bug reporting form <noreply@postgresql.org> wrote:
> The following bug has been logged on the website:
>
> Bug reference:      17376
> Logged by:          akg
> Email address:      adrien.gilmore+pg@gmail.com
> PostgreSQL version: 13.5
> Operating system:   Linux
> Description:        
>
> Hello,
>
> SQL demonstrating the issue on 13.5 is below. 
>
> --
> BEGIN;
> CREATE TABLE t1 (id SERIAL PRIMARY KEY);
> INSERT INTO t1 VALUES (default);
>
> CREATE FUNCTION myfunc() RETURNS TEXT LANGUAGE plpgsql AS $$
>   BEGIN
>     SELECT r FROM t1;
>     RETURN random()::text;
> END $$;
>
> ALTER TABLE t1 ADD COLUMN r TEXT NOT NULL UNIQUE DEFAULT myfunc();
> --
> 
> Results in the error:
> ERROR:  could not read block 0 in file "base/84505/84705": read only 0 of
> 8192 bytes
>

I found that the ALTER TABLE ... ADD COLUMN ... UNIQUE will create a sub-command
to create an index for table, however, it does not create the file on disk,
which leads the above error.  The new unique index's oid is 84705 in your
environment. You can try debug it and make a breakpoint at ATExecAddIndex() to
see it.

When calling _SPI_execute_plan() to execute plpgsql code, the SELECT statement
tries to open the index which is create by ALTER command, since it does not
exists, so you get the error like above.

> The error message content returned is what I suspect of being a bug, not so
> much that this SQL didn't work.

+1. The error message makes user confused IMO, maybe we can fix it, but I have
no idea for this.  Any suggestion is welcomed.


OTOH, you can use the following code to replace it:

    ALTER TABLE t1 ADD COLUMN r TEXT NOT NULL DEFAULT myfunc();
    ALTER TABLE t1 ADD UNIQUE (r);

-- 
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.

Japin Li <japinli@hotmail.com> writes:
> On Fri, 21 Jan 2022 at 17:22, PG Bug reporting form <noreply@postgresql.org> wrote:
>> The error message content returned is what I suspect of being a bug, not so
>> much that this SQL didn't work.

> +1. The error message makes user confused IMO, maybe we can fix it, but I have
> no idea for this.  Any suggestion is welcomed.

Yeah.  Ideally we'd throw an error along the lines of "can't access a
table that's in process of being altered".  The SELECT inside the function
is not part of the ALTER TABLE machinery and ought to be locked out.
However, I fear we don't have adequate infrastructure to tell which
table accesses *are* part of the ALTER TABLE machinery and which aren't.
Maybe it'd be sufficient to check for an active ALTER TABLE in the
parser, but I'm not sure.

            regards, tom lane

On Sat, 22 Jan 2022 at 01:36, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Japin Li <japinli@hotmail.com> writes:
>> On Fri, 21 Jan 2022 at 17:22, PG Bug reporting form <noreply@postgresql.org> wrote:
>>> The error message content returned is what I suspect of being a bug, not so
>>> much that this SQL didn't work.
>
>> +1. The error message makes user confused IMO, maybe we can fix it, but I have
>> no idea for this.  Any suggestion is welcomed.
>
> Yeah.  Ideally we'd throw an error along the lines of "can't access a
> table that's in process of being altered".  The SELECT inside the function
> is not part of the ALTER TABLE machinery and ought to be locked out.
> However, I fear we don't have adequate infrastructure to tell which
> table accesses *are* part of the ALTER TABLE machinery and which aren't.

Right.

> Maybe it'd be sufficient to check for an active ALTER TABLE in the
> parser, but I'm not sure.
>

Do you mean check the table is accessed by ALTER TABLE when calling SPI to
execute the function?  How can we get the table that is be accessed by
ALTER TABLE in parser?


-- 
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.

Japin Li <japinli@hotmail.com> writes:
> On Sat, 22 Jan 2022 at 01:36, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> However, I fear we don't have adequate infrastructure to tell which
>> table accesses *are* part of the ALTER TABLE machinery and which aren't.

> Right.

>> Maybe it'd be sufficient to check for an active ALTER TABLE in the
>> parser, but I'm not sure.

> Do you mean check the table is accessed by ALTER TABLE when calling SPI to
> execute the function?  How can we get the table that is be accessed by
> ALTER TABLE in parser?

The reason there's not a problem like this with respect to cross-session
accesses is that the exclusive lock acquired by ALTER TABLE prevents
other sessions from touching the table.  Conversely, the reason we do
have a problem here is that such locks don't prevent accesses within
our own session.  So a rough sketch for fixing this might be "at
the point where we acquire a lock on some table for a new statement,
also check to see if some internal-to-our-session operation has
declared a need for exclusive access".  The problem is that we don't
have a well-defined line as to what's part of the ALTER and what
is not.  For example, I recall that ALTER TABLE handles index rebuilding
by creating textual ALTER ADD INDEX commands, and parsing and executing
those.  How can we distinguish that behavior from a SQL function within
an index or CHECK constraint trying to execute ALTER ADD INDEX?

            regards, tom lane