Bug: [Forge] addColumn() does not set NOT NULL by default

[kenjis]

PHP Version

8.1

CodeIgniter4 Version

develop (5ffe732) after 4.3.1

CodeIgniter4 Installation Method

Git

Which operating systems have you tested for this bug?

macOS

Which server did you use?

cli-server (PHP built-in webserver)

Database

MySQL 5.7

What happened?

addColumn() does not set NOT NULL by default.

Steps to Reproduce

<?php

namespace App\Database\Migrations;

use CodeIgniter\Database\Migration;

class CreateTableTestTable extends Migration
{
 public function up()
 {
 $this->forge->addField(
 [
 'col1' => ['type' => 'VARCHAR', 'constraint' => 255],
 'col2' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => true],
 'col3' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => false],
 ]
 );
 $this->forge->createTable('test_table');
 }

 public function down()
 {
 }
}

<?php

namespace App\Database\Migrations;

use CodeIgniter\Database\Migration;

class AddColumnToTestTable extends Migration
{
 public function up()
 {
 $this->forge->addColumn('test_table', [
 'col4' => ['type' => 'VARCHAR', 'constraint' => 255],
 'col5' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => true],
 'col6' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => false],
 ]);
 }

 public function down()
 {
 }
}

$ php spark migrate

create table test_table
(
 col1 varchar(255) not null, -- NOT NULL by default
 col2 varchar(255) null,
 col3 varchar(255) not null,
 col4 varchar(255) null, -- NULL by default
 col5 varchar(255) null,
 col6 varchar(255) not null
)
 charset = utf8;

Expected Output

 col4 varchar(255) not null,

Anything else?

The addColumn() method is used to modify an existing table. It accepts the same field array as above, and can be used for an unlimited number of additional fields.
https://codeigniter4.github.io/userguide/dbmgmt/forge.html#forge-addcolumn

And in the above:

null/true : to generate “null” in the field definition. Without this, the field will default to “NOT null”.
https://codeigniter4.github.io/userguide/dbmgmt/forge.html#adding-fields

[kenjis]

I thought this was bug. But it seems intended behavior.

bcit-ci/CodeIgniter@a287a34c#diff-12747b30fae12d4fc5c2f14744e1120a1a32719ebfd3b3d10392752d33b7c73bR734-R744
system/database/DB_forge.php

			if (isset($attributes['NULL']))
			{
				if ($attributes['NULL'] === TRUE)
				{
					$field['null'] = empty($this->_null) ? '' : ' '.$this->_null;
				}
				elseif ($create_table === TRUE)
				{
					$field['null'] = ' NOT NULL';
				}
			}

[kenjis]

The behavior of all drivers is the same.
See the test: #7319

[kenjis]

Since a default value is required to add a NOT NULL column,
there is a certain rationale for defaulting to NULL only when adding a column.

Also, changing this behavior now would be a breaking change.
I don't see the benefit of making the change.

So I think it is better to fix the documentation.

[michalsn]

Fair enough, I think I would vote for fixing it, but it's not a big deal for documenting it instead.

There is a big chance that we will forget about this one in v5... do we have any list of things we're leaving for the next major version?

[kenjis]

This label: https://github.com/codeigniter4/CodeIgniter4/issues?q=label%3A%22next+major+version%3F%22+
and https://github.com/codeigniter4/CodeIgniter4/projects/6

[kenjis]

I sent a PR to fix the docs: #7342