Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[YSQL] Table is in inconsistent state when adding column that is DEFAULT NOT NULL #9970

Closed
lukaseder opened this issue Sep 10, 2021 · 6 comments · May be fixed by ryan-ally/yugabyte-db#213
Closed
Assignees
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug priority/medium Medium priority issue

Comments

@lukaseder
Copy link

lukaseder commented Sep 10, 2021

Jira Link: DB-2570
I understand that adding columns with a new default doesn't work yet:

create table t (id int, a varchar(10), primary key (id));
insert into t (id, a) values (1, 'A');
alter table t add b int default 1;
select * from t;

Producing:

|id |a  |b  |
|---|---|---|
|1  |A  |   |

Relevant issues are:

But when adding a not null constraint to the column, the statement should be rejected, alas it works:

create table t (id int, a varchar(10), primary key (id));
insert into t (id, a) values (1, 'A');
alter table t add b int default 1 not null;
select column_name, is_nullable from information_schema.columns where table_name = 't';

Producing:

|column_name|is_nullable|
|-----------|-----------|
|a          |YES        |
|b          |NO         |
|id         |NO         |

I think this is a bug, independent of the above missing features. The following statement sequence where the DEFAULT clause and the NOT NULL constraint are separated doesn't work:

create table t (id int, a varchar(10), primary key (id));
insert into t (id, a) values (1, 'A');
alter table t add b int default 1;
alter table t alter b set not null;

This produces an error:

SQL Error [23502]: ERROR: column "b" contains null values

I think that this statement should produce the same error:

alter table t add b int default 1 not null;
@tedyu
Copy link
Contributor

tedyu commented Sep 11, 2021

I looked at related code for the 'default 1 not null' case.
In AddRelationNewConstraints():

        /* If the DEFAULT is volatile we cannot use a missing value */
        if (colDef->missingMode && contain_volatile_functions((Node *) expr))
            colDef->missingMode = false;

In the caller, ATExecAddColumn():

		rawEnt->missingMode = true;
...
		AddRelationNewConstraints(rel, list_make1(rawEnt), NIL,
...
		if (!rawEnt->missingMode)
			tab->rewrite |= AT_REWRITE_DEFAULT_VAL;

Since the default is non-volatile, missingMode field carries value of true.
Later in ATExecAddColumn(), there is check for identity column, DomainHasConstraints and Oid column. None of them is satisfied, leading to AT_REWRITE_DEFAULT_VAL not being added to rewrite field.
The result is that, phase 3 of 'alter table', the rewrite, is not performed. Hence there is no error.

This logic is the same in PG (apart from Oid check being gone since Oid is not special column in upstream PG).

Here is same commands executed on PG 13:
https://dbfiddle.uk/?rdbms=postgres_13&fiddle=6159b7ab7d654077e201ccfc56b41681

We can see that, PG handles default value correctly but YB doesn't (for 'default 1 not null' case)

@tedyu
Copy link
Contributor

tedyu commented Sep 11, 2021

If I temporarily drop the contain_volatile_functions() check, I see another error:

yugabyte=# alter table t add b int default 1 not null;
ERROR:  Operation not allowed in YugaByte mode heap_insert

@tedyu
Copy link
Contributor

tedyu commented Sep 11, 2021

I was looking at the PG commit:

commit 16828d5c0273b4fe5f10f42588005f16b415b2d8
Author: Andrew Dunstan <andrew@dunslane.net>
Date:   Wed Mar 28 10:43:52 2018 +1030

    Fast ALTER TABLE ADD COLUMN with a non-NULL default

I added some debug logs in getmissingattr() and slot_getmissingattrs().
None of which shows up when performing

yugabyte=# select * from t;
 id | a | b
----+---+---
  1 | A |
(1 row)

Still investigating ...

@tedyu
Copy link
Contributor

tedyu commented Sep 12, 2021

Query on table created with column having default value returns the default value (correct behavior).
After adding column with default (without 'not null'), the query doesn't return the default value either.

yugabyte=# alter table t add b int default 1;
ALTER TABLE
yugabyte=# select * from t;
 id | a | b
----+---+---
  1 | A |
(1 row)

If 'default z not null' errors out for 'alter table x add y', 'default z' should error out as well.

@ddorian ddorian added the area/ysql Yugabyte SQL (YSQL) label Sep 23, 2021
@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug priority/medium Medium priority issue labels Jun 9, 2022
@gherciu
Copy link

gherciu commented Jul 24, 2023

Yep, this behaviour is not fixed, and that is kinda a big problem being honest
Because locally we develop using pg in doker and that works so we expect yugabyte to behave the same

@fizaaluthra fizaaluthra self-assigned this Jul 26, 2023
@fizaaluthra
Copy link
Member

Fixed by commit 0aca19f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug priority/medium Medium priority issue
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

7 participants