Postgre & SQLSRV - Should Never Have A Field Length For TEXT

[sclubricants]

This PR fixes an issue where a table definition in Forge has a length value for type TEXT. Postgre does not support a length for the TEXT type.

Its probably possible for a developer just to set length null when developing on Postgre. This PR is useful when using the same table definition across different DBMS.

The following query works fine for Mysql, Sqlite but fails on Postgre & SQLSRV:

        $this->forge->addColumn('actions', [
            'summary'     => ['type' => 'varchar', 'constraint' => 255],
            'description' => ['type' => 'text', 'constraint' => 255],
        ]);

https://www.postgresql.org/docs/current/datatype-character.html

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[kenjis]

Thank you for sending PR.

If this PR is a bug fix, it should go to develop branch.

[sclubricants]

Its not a bug per se, You can assign the constraint as null and it will not cause a problem.

This PR just allows using the same table definition to work between databases.

Put another way, Forge allows you to assign an illegal constraint. This PR will just ignore such illegal constraint if exists.

SQLSRV has same issue. I can add here.

[kenjis]

Okay, this PR is an enhancement.

Can you add a test to prove this enhancement?

[sclubricants]

Added test and another fix for SQLSRV.

SQLSRV adds a constraint whenever a column is defined with a default value. Whenever dropColumn is called the current code drops all indexes associated with the column but it doesn't drop the default constraints. These use a slightly different command as well: Instead of DROP INDEX they use DROP CONSTRAINT. These constraint names are generated by the system and do not list in getIndexData(). There has been an attempt to name these in the modifyColumn() method however they are not named in createTable(). These are kind of annoying and I'm not sure why they don't automatically drop with the column name.

[sclubricants]

The test demonstrates that these two simple commands can now execute successfully on all DBMS

        $result = $this->forge->addColumn('user', [
            'text_with_constraint' => ['type' => 'text', 'constraint' => 255, 'default' => ''],
        ]);

        $result = $this->forge->dropColumn('user', 'text_with_constraint');

Without this PR Postgre and SQLSRV would fail.

[MGatner]

Shows what I know, I didn't think MySQL could have a length spec for TEXT 🤦‍♂️

[kenjis]

Can you add a short description about this enhancement in changelog?

[kenjis]

For example, this commit is not good. Because the document change has nothing to do with the code change.

We added the explanation for commits and commit messages:
See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/workflow.md#committing

[kenjis]

It seems you used git merge, but we recommend to use git rebase when updating the PR branch.
See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/workflow.md#updating-your-branch