Issue Details (XML | Word | Printable)

Key: CORE-2696
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Adriano dos Santos Fernandes
Reporter: Jacob Lee
Votes: 0
Watchers: 1
Operations

If you were logged in you would be able to see more operations.
Firebird Core

"alter table" command can add a field which has "not null" definition

Created: 19/Oct/09 12:58 PM   Updated: 05/Sep/16 04:25 AM
Component/s: Engine
Affects Version/s: None
Fix Version/s: 3.0 Alpha 1, 3.0.0

Issue Links:
Duplicate
 
Relate
 

QA Status: Covered by another test(s)
Test Details:
See tests that manipulates with NULL fields/domains and check results:
* CORE-1518 Adding a non-null restricted column to a populated table renders the table inconsistent
* CORE-4453 (Regression: NOT NULL constraint, declared in domain, does not work)
* CORE-4725 (Inconsistencies with ALTER DOMAIN and ALTER TABLE with DROP NOT NULL and PRIMARY KEYs)
* CORE-4733 (Command "Alter table <T> alter TYPE <C> <DOMAIN_WITH_NOT_NULL" does not verifies data in column <C> and makes incorrect assignments in <C> to ZERO / JULIAN_DATE / ASCII(0) for types INT, TIMESTAMP and VARCHAR)


 Description  « Hide
"alter table" command can add a field which has "not null" definition but does not have "default" definition.

example:

SQL> select * from equip_tbl;

EQUIPNAME EQUIPID
==================== =======
carrier a
svc b
charger
feeding roll c

SQL> alter table equip_tbl add equip_id char(2) not null;
SQL> select * from equip_tbl;

EQUIPNAME EQUIPID EQUIP_ID
==================== ======= ========
carrier a <null>
svc b <null>
charger <null>
feeding roll c <null>


In my knowledge, a table should add a field which has "not null" and "default" both like:
alter table equip_tbl add equip_id char(2) default '' not null. in this case the result is like:

SQL> alter table equip_tbl add equip_id char(2) default '' not null;
SQL> select * from equip_tbl;

EQUIPNAME EQUIPID EQUIP_ID
==================== ======= ========
carrier a
svc b
charger
feeding roll c

 All   Comments   Change History   Subversion Commits      Sort Order: Ascending order - Click to sort in descending order
Dmitry Yemanov added a comment - 19/Oct/09 03:42 PM
FWIW, this is an originally intended behavior, so it actually belongs to a "design pitfall" category. But I do agree that this legacy behavior is very questionable, to say at least. I hope to have it changed in future versions, hopefully in v3.0.

Sean Leyne added a comment - 20/Oct/09 12:40 AM
Dmitry,

I agree with your "design pitfall" characterization for this issue. I do not, however, believe that any fix is required or desirable.


As I see it, there is no way of knowing what value should be assigned to the existing table rows, the default value can not be infered to applicable.

It is for the developer to realize that they need to "fix" the data for these types of schema changes, not for the engine to "guess".

Dmitry Yemanov added a comment - 20/Oct/09 03:13 AM
I strongly believe that *any* execution of the SQL statement *must not* make the data inconsistent (in this case -- violating the just created constraint). If this rule would be followed from the beginning, we'd virtually never hear about "unrestorable backups".

So I see only two ways to proceed with the issue: prohibit such a schema change at all or populate the new column with a default value (if specified, otherwise also prohibit). The third one might be to introduce some special syntax for the desired ability, although I don't like it much.

As for clever developers knowing what they're doing, they're free to add the new column as nullable, then populate it with data and finally change the nullability flag (and the engine must validate the last change against the stored data).

Sean Leyne added a comment - 20/Oct/09 03:38 AM - edited
Dmitry,

You are trying to solve the "unrecoverable backup" by adding schema requirements which are not part of the SQL standard and not imposed by other engines.

The solution to the backup problem seems straight-forward to me:

Have the backup treat the application of the column constraints ("NOT NULL" flag, constraints) as a separate step in the backup/restore cycle to the effect that when performing a restore the order of operation would be:
   1. Restore basic column definition
   2. Restore table data
   3. Restore column constraints (i.e. full column definition)

By following this order of operation the existing data can be kept untouched while the database is fully restorable.


As for the assignment of the default value to existing rows, MS SQL addresses the assignment of values through the use of a "WITH VALUES" syntax to provide values for each existing row in the table. If "WITH VALUES" is not used, each row has the value NULL in the new column.

In the end, the inconsistency in historical data should be supported without any additional requirement. It is up to the developer to resolve the inconsistency, not the engine.

Dmitry Yemanov added a comment - 20/Oct/09 02:45 PM
Sean, the "unrestorable backup" is just a consequence of the issue and yes, it can be addressed separately. But anyway, I'll be considering this ticket a bug requiring fixing as long as you'll be able to read NULLs from the NOT NULL column, period.

Now regarding the standards and other engines. From the SQL specification:

    The column defined by the <column definition> is added to T. Let C be the column added to T.
    Case:
        a) If C is a generated column, then <...>
        b) Otherwise, C is a base column.
       Case:
           i) If C is an identity column, then <...>
          ii) Otherwise, every value in C is the default value for C.
          NOTE 292 — The default value of a column is defined in Subclause 11.5, "<default clause>".

From the Oracle documentation:

    You cannot add a column with a NOT NULL constraint if table has any rows unless you also specify the DEFAULT clause.

Regarding MS SQL and WITH VALUES clause:

    If the new column does not allow nulls, the default value is stored in new rows regardless of whether WITH VALUES is specified.

From the PostgreSQL documentation:

    The new column is initially filled with whatever default value is given.

From the DB2 documentation:

    ADD column-definition
        NOT NULL
            Prevents the column from containing null values. The default-clause must also be specified.

Philippe Makowski added a comment - 20/Oct/09 03:09 PM
Agree Dmitry
and it would solved also CORE-1355