test: Forge::modifyColumn() and null - #7302

Merged
kenjis merged 1 commit into
codeigniter4:4.4from
kenjis:test-Forge-modifyColumn-null
May 1, 2023
Merged

test: Forge::modifyColumn() and null#7302
kenjis merged 1 commit into
codeigniter4:4.4from
kenjis:test-Forge-modifyColumn-null

Conversation

@kenjis

@kenjiskenjis commented Feb 24, 2023•
edited
Loading

Copy link
Copy Markdown
Member

Needs #7301

Description
From https://zenn.dev/naente/articles/50935eb55c14ea (CodeIgniter4 PostgreSQL always adds NOT NULL constraints when running modifyColumn())

See the current behavior: #7302 (comment)

  • add test for Forge::modifyColumn() and null

This bug was fixed in v4.3.4:

Related #7235

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

@kenjiskenjis added testing Pull requests that changes tests only 4.4 database Issues or pull requests that affect the database layer labels Feb 24, 2023
@kenjis
kenjis marked this pull request as draft February 24, 2023 07:16
@kenjis
kenjisforce-pushed the test-Forge-modifyColumn-null branch 2 times, most recently from 5d49dff to f9c953cCompareFebruary 24, 2023 07:33
@kenjis

kenjis commented Feb 24, 2023

Copy link
Copy Markdown
MemberAuthor

@codeigniter4/database-team
It seems Postgre, SQLSRV, SQLite3 add NOT NULL constraints when running modifyColumn().
See the test: https://github.com/codeigniter4/CodeIgniter4/pull/7302/files#diff-489cc58f8341d1fa6ca26b7bb857562d4274924d2dc691ca64e9dc609731b8eaR1285
Are these bugs or not?

@sclubricants

sclubricants commented Feb 24, 2023

Copy link
Copy Markdown
Member

Looks like nullable is the default. What is the matter with this?

@kenjis

kenjis commented Feb 24, 2023•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

I did not look into all drivers, but Postgre changes to NOT NULL in all cases.
There is no way to keep nuallble. So it is clearly a bug.

Another issue is these behaviors are not consistent. As you see all tests pass
on MySQLi and OCI8.

If a dev changes DB driver, e.g. MySQLi to Postgres,
the definitiion of the table will change when running migrations.

@kenjis
kenjisforce-pushed the test-Forge-modifyColumn-null branch from f9c953c to a5fc127CompareFebruary 25, 2023 03:30
@kenjis

kenjis commented Feb 25, 2023•
edited
Loading

Copy link
Copy Markdown
MemberAuthor
$this->forge->addField([
'col1' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => true],
'col2' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => true],
'col3' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => true],
]);
$this->forge->createTable('forge_test_modify');
$this->forge->modifyColumn('forge_test_modify', [
'col1' => ['type' => 'VARCHAR', 'constraint' => 1],
'col2' => ['type' => 'VARCHAR', 'constraint' => 1, 'null' => true],
'col3' => ['type' => 'VARCHAR', 'constraint' => 1, 'null' => false],
]);
$this->db->resetDataCache();
$fields = $this->db->getFieldData('forge_test_modify');
dd($fields);

Postgres:

array (3) [
0 => stdClass#249 (5) (
public 'name' -> string (4) "col1"
public 'type' -> string (17) "character varying"
public 'nullable' -> boolean false // MySQLi true
public 'default' -> null
public 'max_length' -> string (1) "1"
)
1 => stdClass#74 (5) (
public 'name' -> string (4) "col2"
public 'type' -> string (17) "character varying"
public 'nullable' -> boolean false // should be true
public 'default' -> null
public 'max_length' -> string (1) "1"
)
2 => stdClass#75 (5) (
public 'name' -> string (4) "col3"
public 'type' -> string (17) "character varying"
public 'nullable' -> boolean false
public 'default' -> null
public 'max_length' -> string (1) "1"
)
]

SQLSRV:

array (3) [
0 => stdClass#249 (5) (
public 'name' -> string (4) "col1"
public 'type' -> string (7) "varchar"
public 'default' -> null
public 'max_length' -> integer 1
public 'nullable' -> boolean false // MySQLi true
)
1 => stdClass#74 (5) (
public 'name' -> string (4) "col2"
public 'type' -> string (7) "varchar"
public 'default' -> null
public 'max_length' -> integer 1
public 'nullable' -> boolean false // should be true
)
2 => stdClass#75 (5) (
public 'name' -> string (4) "col3"
public 'type' -> string (7) "varchar"
public 'default' -> null
public 'max_length' -> integer 1
public 'nullable' -> boolean false
)
]

SQLite3:

array (3) [
0 => stdClass#77 (6) (
public 'name' -> string (4) "col1"
public 'type' -> string (7) "VARCHAR"
public 'max_length' -> null
public 'default' -> string (2) "''"
public 'primary_key' -> boolean false
public 'nullable' -> boolean false // MySQLi true
)
1 => stdClass#67 (6) (
public 'name' -> string (4) "col2"
public 'type' -> string (7) "VARCHAR"
public 'max_length' -> null
public 'default' -> string (4) "NULL"
public 'primary_key' -> boolean false
public 'nullable' -> boolean true
)
2 => stdClass#602 (6) (
public 'name' -> string (4) "col3"
public 'type' -> string (7) "VARCHAR"
public 'max_length' -> null
public 'default' -> string (4) "NULL"
public 'primary_key' -> boolean false
public 'nullable' -> boolean true // should be false
)
]

Not directly related, but the all max_length values are incorrect on SQLite3.

@kenjiskenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Feb 25, 2023
@kenjis
kenjisforce-pushed the test-Forge-modifyColumn-null branch from a5fc127 to cedb583CompareMarch 1, 2023 07:00
@michalsn

michalsn commented Mar 3, 2023

Copy link
Copy Markdown
Member

Seems like a bug to me.

@kenjis
kenjisforce-pushed the test-Forge-modifyColumn-null branch from cedb583 to 140ad48CompareMarch 4, 2023 00:42
@kenjis
kenjisforce-pushed the test-Forge-modifyColumn-null branch from 140ad48 to 5232285CompareMarch 23, 2023 01:56
@kenjiskenjis mentioned this pull request Mar 23, 2023
Merged
5 tasks
@kenjis
kenjisforce-pushed the test-Forge-modifyColumn-null branch from 5232285 to 0e86d4cCompareApril 30, 2023 02:49
@kenjiskenjis removed the bug Verified issues on the current code behavior or pull requests that will fix them label Apr 30, 2023
@kenjis
kenjis marked this pull request as ready for review April 30, 2023 02:51
@kenjis

kenjis commented Apr 30, 2023

Copy link
Copy Markdown
MemberAuthor

Rebased and now this PR removes only unneeded test code.

@MGatner

MGatner commented May 1, 2023

Copy link
Copy Markdown
Member

Looks good, thanks!

@kenjis
kenjis merged commit d9fd9d5 into codeigniter4:4.4May 1, 2023
@kenjis
kenjis deleted the test-Forge-modifyColumn-null branch May 1, 2023 12:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@michalsnmichalsnmichalsn approved these changes

Assignees

No one assigned

Labels

databaseIssues or pull requests that affect the database layertestingPull requests that changes tests only

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kenjis@sclubricants@michalsn@MGatner
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
test: Forge::modifyColumn() and null - #7302
Merged
kenjis merged 1 commit into
codeigniter4:4.4from 
kenjis:test-Forge-modifyColumn-null
May 1, 2023
Merged
test: Forge::modifyColumn() and null#7302
kenjis merged 1 commit into
codeigniter4:4.4from 
kenjis:test-Forge-modifyColumn-null
Conversation
@kenjis
@kenjiskenjis
commented
Feb 24, 2023•

edited

Loading

Copy link


Copy Markdown

Member
Needs #7301
Description

From https://zenn.dev/naente/articles/50935eb55c14ea (CodeIgniter4 PostgreSQL always adds NOT NULL constraints when running modifyColumn())
See the current behavior: #7302 (comment)
  • add test for Forge::modifyColumn() and null
This bug was fixed in v4.3.4:
Related #7235
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
@kenjiskenjis
added

testing
Pull requests that changes tests only
4.4

database
Issues or pull requests that affect the database layer
labels
Feb 24, 2023
@kenjis
kenjis
marked this pull request as draft
February 24, 2023 07:16
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
2 times, most recently
from
5d49dff to
f9c953cCompareFebruary 24, 2023 07:33
@kenjis
kenjis
commented
Feb 24, 2023

Copy link


Copy Markdown

MemberAuthor
@codeigniter4/database-team

It seems Postgre, SQLSRV, SQLite3 add NOT NULL constraints when running modifyColumn().

See the test: https://github.com/codeigniter4/CodeIgniter4/pull/7302/files#diff-489cc58f8341d1fa6ca26b7bb857562d4274924d2dc691ca64e9dc609731b8eaR1285

Are these bugs or not?
@sclubricants
sclubricants
commented
Feb 24, 2023

Copy link


Copy Markdown

Member
Looks like nullable is the default. What is the matter with this?
@kenjis
kenjis
commented
Feb 24, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
I did not look into all drivers, but Postgre changes to NOT NULL in all cases.

There is no way to keep nuallble. So it is clearly a bug.
Another issue is these behaviors are not consistent. As you see all tests pass

on MySQLi and OCI8.
If a dev changes DB driver, e.g. MySQLi to Postgres,

the definitiion of the table will change when running migrations.
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
f9c953c to
a5fc127CompareFebruary 25, 2023 03:30
@kenjis
kenjis
commented
Feb 25, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
$this->forge->addField([
'col1' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => true],
'col2' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => true],
'col3' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => true],
]);
$this->forge->createTable('forge_test_modify');
$this->forge->modifyColumn('forge_test_modify', [
'col1' => ['type' => 'VARCHAR', 'constraint' => 1],
'col2' => ['type' => 'VARCHAR', 'constraint' => 1, 'null' => true],
'col3' => ['type' => 'VARCHAR', 'constraint' => 1, 'null' => false],
]);
$this->db->resetDataCache();
$fields = $this->db->getFieldData('forge_test_modify');
dd($fields);
Postgres:
array (3) [
0 => stdClass#249 (5) (
public 'name' -> string (4) "col1"
public 'type' -> string (17) "character varying"
public 'nullable' -> boolean false // MySQLi true
public 'default' -> null
public 'max_length' -> string (1) "1"
)
1 => stdClass#74 (5) (
public 'name' -> string (4) "col2"
public 'type' -> string (17) "character varying"
public 'nullable' -> boolean false // should be true
public 'default' -> null
public 'max_length' -> string (1) "1"
)
2 => stdClass#75 (5) (
public 'name' -> string (4) "col3"
public 'type' -> string (17) "character varying"
public 'nullable' -> boolean false
public 'default' -> null
public 'max_length' -> string (1) "1"
)
]

SQLSRV:
array (3) [
0 => stdClass#249 (5) (
public 'name' -> string (4) "col1"
public 'type' -> string (7) "varchar"
public 'default' -> null
public 'max_length' -> integer 1
public 'nullable' -> boolean false // MySQLi true
)
1 => stdClass#74 (5) (
public 'name' -> string (4) "col2"
public 'type' -> string (7) "varchar"
public 'default' -> null
public 'max_length' -> integer 1
public 'nullable' -> boolean false // should be true
)
2 => stdClass#75 (5) (
public 'name' -> string (4) "col3"
public 'type' -> string (7) "varchar"
public 'default' -> null
public 'max_length' -> integer 1
public 'nullable' -> boolean false
)
]

SQLite3:
array (3) [
0 => stdClass#77 (6) (
public 'name' -> string (4) "col1"
public 'type' -> string (7) "VARCHAR"
public 'max_length' -> null
public 'default' -> string (2) "''"
public 'primary_key' -> boolean false
public 'nullable' -> boolean false // MySQLi true
)
1 => stdClass#67 (6) (
public 'name' -> string (4) "col2"
public 'type' -> string (7) "VARCHAR"
public 'max_length' -> null
public 'default' -> string (4) "NULL"
public 'primary_key' -> boolean false
public 'nullable' -> boolean true
)
2 => stdClass#602 (6) (
public 'name' -> string (4) "col3"
public 'type' -> string (7) "VARCHAR"
public 'max_length' -> null
public 'default' -> string (4) "NULL"
public 'primary_key' -> boolean false
public 'nullable' -> boolean true // should be false
)
]

Not directly related, but the all max_length values are incorrect on SQLite3.
@kenjiskenjis
added
the 
bug
Verified issues on the current code behavior or pull requests that will fix them
label
Feb 25, 2023
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
a5fc127 to
cedb583CompareMarch 1, 2023 07:00
@michalsn
michalsn
commented
Mar 3, 2023

Copy link


Copy Markdown

Member
Seems like a bug to me.
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
cedb583 to
140ad48CompareMarch 4, 2023 00:42
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
140ad48 to
5232285CompareMarch 23, 2023 01:56
@kenjiskenjis
mentioned this pull request
Mar 23, 2023
 Merged

5 tasks
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
5232285 to
0e86d4cCompareApril 30, 2023 02:49
@kenjiskenjis
removed
the 
bug
Verified issues on the current code behavior or pull requests that will fix them
label
Apr 30, 2023
@kenjis
kenjis
marked this pull request as ready for review
April 30, 2023 02:51
@kenjis
kenjis
commented
Apr 30, 2023

Copy link


Copy Markdown

MemberAuthor
Rebased and now this PR removes only unneeded test code.
@MGatner
MGatner
commented
May 1, 2023

Copy link


Copy Markdown

Member
Looks good, thanks!
@kenjis
kenjis
merged commit d9fd9d5
into
codeigniter4:4.4May 1, 2023
@kenjis
kenjis
deleted the

test-Forge-modifyColumn-null

branch
May 1, 2023 12:20
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@michalsnmichalsnmichalsn approved these changes

Assignees


No one assigned


Labels

databaseIssues or pull requests that affect the database layertestingPull requests that changes tests only

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@kenjis@sclubricants@michalsn@MGatner

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
test: Forge::modifyColumn() and null - #7302
Merged
kenjis merged 1 commit into
codeigniter4:4.4from 
kenjis:test-Forge-modifyColumn-null
May 1, 2023
Merged
test: Forge::modifyColumn() and null#7302
kenjis merged 1 commit into
codeigniter4:4.4from 
kenjis:test-Forge-modifyColumn-null
Conversation
@kenjis
@kenjiskenjis
commented
Feb 24, 2023•

edited

Loading

Copy link


Copy Markdown

Member
Needs #7301
Description

From https://zenn.dev/naente/articles/50935eb55c14ea (CodeIgniter4 PostgreSQL always adds NOT NULL constraints when running modifyColumn())
See the current behavior: #7302 (comment)
  • add test for Forge::modifyColumn() and null
This bug was fixed in v4.3.4:
Related #7235
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
@kenjiskenjis
added

testing
Pull requests that changes tests only
4.4

database
Issues or pull requests that affect the database layer
labels
Feb 24, 2023
@kenjis
kenjis
marked this pull request as draft
February 24, 2023 07:16
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
2 times, most recently
from
5d49dff to
f9c953cCompareFebruary 24, 2023 07:33
@kenjis
kenjis
commented
Feb 24, 2023

Copy link


Copy Markdown

MemberAuthor
@codeigniter4/database-team

It seems Postgre, SQLSRV, SQLite3 add NOT NULL constraints when running modifyColumn().

See the test: https://github.com/codeigniter4/CodeIgniter4/pull/7302/files#diff-489cc58f8341d1fa6ca26b7bb857562d4274924d2dc691ca64e9dc609731b8eaR1285

Are these bugs or not?
@sclubricants
sclubricants
commented
Feb 24, 2023

Copy link


Copy Markdown

Member
Looks like nullable is the default. What is the matter with this?
@kenjis
kenjis
commented
Feb 24, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
I did not look into all drivers, but Postgre changes to NOT NULL in all cases.

There is no way to keep nuallble. So it is clearly a bug.
Another issue is these behaviors are not consistent. As you see all tests pass

on MySQLi and OCI8.
If a dev changes DB driver, e.g. MySQLi to Postgres,

the definitiion of the table will change when running migrations.
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
f9c953c to
a5fc127CompareFebruary 25, 2023 03:30
@kenjis
kenjis
commented
Feb 25, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
$this->forge->addField([
'col1' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => true],
'col2' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => true],
'col3' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => true],
]);
$this->forge->createTable('forge_test_modify');
$this->forge->modifyColumn('forge_test_modify', [
'col1' => ['type' => 'VARCHAR', 'constraint' => 1],
'col2' => ['type' => 'VARCHAR', 'constraint' => 1, 'null' => true],
'col3' => ['type' => 'VARCHAR', 'constraint' => 1, 'null' => false],
]);
$this->db->resetDataCache();
$fields = $this->db->getFieldData('forge_test_modify');
dd($fields);
Postgres:
array (3) [
0 => stdClass#249 (5) (
public 'name' -> string (4) "col1"
public 'type' -> string (17) "character varying"
public 'nullable' -> boolean false // MySQLi true
public 'default' -> null
public 'max_length' -> string (1) "1"
)
1 => stdClass#74 (5) (
public 'name' -> string (4) "col2"
public 'type' -> string (17) "character varying"
public 'nullable' -> boolean false // should be true
public 'default' -> null
public 'max_length' -> string (1) "1"
)
2 => stdClass#75 (5) (
public 'name' -> string (4) "col3"
public 'type' -> string (17) "character varying"
public 'nullable' -> boolean false
public 'default' -> null
public 'max_length' -> string (1) "1"
)
]

SQLSRV:
array (3) [
0 => stdClass#249 (5) (
public 'name' -> string (4) "col1"
public 'type' -> string (7) "varchar"
public 'default' -> null
public 'max_length' -> integer 1
public 'nullable' -> boolean false // MySQLi true
)
1 => stdClass#74 (5) (
public 'name' -> string (4) "col2"
public 'type' -> string (7) "varchar"
public 'default' -> null
public 'max_length' -> integer 1
public 'nullable' -> boolean false // should be true
)
2 => stdClass#75 (5) (
public 'name' -> string (4) "col3"
public 'type' -> string (7) "varchar"
public 'default' -> null
public 'max_length' -> integer 1
public 'nullable' -> boolean false
)
]

SQLite3:
array (3) [
0 => stdClass#77 (6) (
public 'name' -> string (4) "col1"
public 'type' -> string (7) "VARCHAR"
public 'max_length' -> null
public 'default' -> string (2) "''"
public 'primary_key' -> boolean false
public 'nullable' -> boolean false // MySQLi true
)
1 => stdClass#67 (6) (
public 'name' -> string (4) "col2"
public 'type' -> string (7) "VARCHAR"
public 'max_length' -> null
public 'default' -> string (4) "NULL"
public 'primary_key' -> boolean false
public 'nullable' -> boolean true
)
2 => stdClass#602 (6) (
public 'name' -> string (4) "col3"
public 'type' -> string (7) "VARCHAR"
public 'max_length' -> null
public 'default' -> string (4) "NULL"
public 'primary_key' -> boolean false
public 'nullable' -> boolean true // should be false
)
]

Not directly related, but the all max_length values are incorrect on SQLite3.
@kenjiskenjis
added
the 
bug
Verified issues on the current code behavior or pull requests that will fix them
label
Feb 25, 2023
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
a5fc127 to
cedb583CompareMarch 1, 2023 07:00
@michalsn
michalsn
commented
Mar 3, 2023

Copy link


Copy Markdown

Member
Seems like a bug to me.
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
cedb583 to
140ad48CompareMarch 4, 2023 00:42
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
140ad48 to
5232285CompareMarch 23, 2023 01:56
@kenjiskenjis
mentioned this pull request
Mar 23, 2023
 Merged

5 tasks
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
5232285 to
0e86d4cCompareApril 30, 2023 02:49
@kenjiskenjis
removed
the 
bug
Verified issues on the current code behavior or pull requests that will fix them
label
Apr 30, 2023
@kenjis
kenjis
marked this pull request as ready for review
April 30, 2023 02:51
@kenjis
kenjis
commented
Apr 30, 2023

Copy link


Copy Markdown

MemberAuthor
Rebased and now this PR removes only unneeded test code.
@MGatner
MGatner
commented
May 1, 2023

Copy link


Copy Markdown

Member
Looks good, thanks!
@kenjis
kenjis
merged commit d9fd9d5
into
codeigniter4:4.4May 1, 2023
@kenjis
kenjis
deleted the

test-Forge-modifyColumn-null

branch
May 1, 2023 12:20
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@michalsnmichalsnmichalsn approved these changes

Assignees


No one assigned


Labels

databaseIssues or pull requests that affect the database layertestingPull requests that changes tests only

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@kenjis@sclubricants@michalsn@MGatner

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
test: Forge::modifyColumn() and null - #7302
Merged
kenjis merged 1 commit into
codeigniter4:4.4from 
kenjis:test-Forge-modifyColumn-null
May 1, 2023
Merged
test: Forge::modifyColumn() and null#7302
kenjis merged 1 commit into
codeigniter4:4.4from 
kenjis:test-Forge-modifyColumn-null
Conversation
@kenjis
@kenjiskenjis
commented
Feb 24, 2023•

edited

Loading

Copy link


Copy Markdown

Member
Needs #7301
Description

From https://zenn.dev/naente/articles/50935eb55c14ea (CodeIgniter4 PostgreSQL always adds NOT NULL constraints when running modifyColumn())
See the current behavior: #7302 (comment)
  • add test for Forge::modifyColumn() and null
This bug was fixed in v4.3.4:
Related #7235
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
@kenjiskenjis
added

testing
Pull requests that changes tests only
4.4

database
Issues or pull requests that affect the database layer
labels
Feb 24, 2023
@kenjis
kenjis
marked this pull request as draft
February 24, 2023 07:16
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
2 times, most recently
from
5d49dff to
f9c953cCompareFebruary 24, 2023 07:33
@kenjis
kenjis
commented
Feb 24, 2023

Copy link


Copy Markdown

MemberAuthor
@codeigniter4/database-team

It seems Postgre, SQLSRV, SQLite3 add NOT NULL constraints when running modifyColumn().

See the test: https://github.com/codeigniter4/CodeIgniter4/pull/7302/files#diff-489cc58f8341d1fa6ca26b7bb857562d4274924d2dc691ca64e9dc609731b8eaR1285

Are these bugs or not?
@sclubricants
sclubricants
commented
Feb 24, 2023

Copy link


Copy Markdown

Member
Looks like nullable is the default. What is the matter with this?
@kenjis
kenjis
commented
Feb 24, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
I did not look into all drivers, but Postgre changes to NOT NULL in all cases.

There is no way to keep nuallble. So it is clearly a bug.
Another issue is these behaviors are not consistent. As you see all tests pass

on MySQLi and OCI8.
If a dev changes DB driver, e.g. MySQLi to Postgres,

the definitiion of the table will change when running migrations.
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
f9c953c to
a5fc127CompareFebruary 25, 2023 03:30
@kenjis
kenjis
commented
Feb 25, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
$this->forge->addField([
'col1' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => true],
'col2' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => true],
'col3' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => true],
]);
$this->forge->createTable('forge_test_modify');
$this->forge->modifyColumn('forge_test_modify', [
'col1' => ['type' => 'VARCHAR', 'constraint' => 1],
'col2' => ['type' => 'VARCHAR', 'constraint' => 1, 'null' => true],
'col3' => ['type' => 'VARCHAR', 'constraint' => 1, 'null' => false],
]);
$this->db->resetDataCache();
$fields = $this->db->getFieldData('forge_test_modify');
dd($fields);
Postgres:
array (3) [
0 => stdClass#249 (5) (
public 'name' -> string (4) "col1"
public 'type' -> string (17) "character varying"
public 'nullable' -> boolean false // MySQLi true
public 'default' -> null
public 'max_length' -> string (1) "1"
)
1 => stdClass#74 (5) (
public 'name' -> string (4) "col2"
public 'type' -> string (17) "character varying"
public 'nullable' -> boolean false // should be true
public 'default' -> null
public 'max_length' -> string (1) "1"
)
2 => stdClass#75 (5) (
public 'name' -> string (4) "col3"
public 'type' -> string (17) "character varying"
public 'nullable' -> boolean false
public 'default' -> null
public 'max_length' -> string (1) "1"
)
]

SQLSRV:
array (3) [
0 => stdClass#249 (5) (
public 'name' -> string (4) "col1"
public 'type' -> string (7) "varchar"
public 'default' -> null
public 'max_length' -> integer 1
public 'nullable' -> boolean false // MySQLi true
)
1 => stdClass#74 (5) (
public 'name' -> string (4) "col2"
public 'type' -> string (7) "varchar"
public 'default' -> null
public 'max_length' -> integer 1
public 'nullable' -> boolean false // should be true
)
2 => stdClass#75 (5) (
public 'name' -> string (4) "col3"
public 'type' -> string (7) "varchar"
public 'default' -> null
public 'max_length' -> integer 1
public 'nullable' -> boolean false
)
]

SQLite3:
array (3) [
0 => stdClass#77 (6) (
public 'name' -> string (4) "col1"
public 'type' -> string (7) "VARCHAR"
public 'max_length' -> null
public 'default' -> string (2) "''"
public 'primary_key' -> boolean false
public 'nullable' -> boolean false // MySQLi true
)
1 => stdClass#67 (6) (
public 'name' -> string (4) "col2"
public 'type' -> string (7) "VARCHAR"
public 'max_length' -> null
public 'default' -> string (4) "NULL"
public 'primary_key' -> boolean false
public 'nullable' -> boolean true
)
2 => stdClass#602 (6) (
public 'name' -> string (4) "col3"
public 'type' -> string (7) "VARCHAR"
public 'max_length' -> null
public 'default' -> string (4) "NULL"
public 'primary_key' -> boolean false
public 'nullable' -> boolean true // should be false
)
]

Not directly related, but the all max_length values are incorrect on SQLite3.
@kenjiskenjis
added
the 
bug
Verified issues on the current code behavior or pull requests that will fix them
label
Feb 25, 2023
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
a5fc127 to
cedb583CompareMarch 1, 2023 07:00
@michalsn
michalsn
commented
Mar 3, 2023

Copy link


Copy Markdown

Member
Seems like a bug to me.
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
cedb583 to
140ad48CompareMarch 4, 2023 00:42
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
140ad48 to
5232285CompareMarch 23, 2023 01:56
@kenjiskenjis
mentioned this pull request
Mar 23, 2023
 Merged

5 tasks
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
5232285 to
0e86d4cCompareApril 30, 2023 02:49
@kenjiskenjis
removed
the 
bug
Verified issues on the current code behavior or pull requests that will fix them
label
Apr 30, 2023
@kenjis
kenjis
marked this pull request as ready for review
April 30, 2023 02:51
@kenjis
kenjis
commented
Apr 30, 2023

Copy link


Copy Markdown

MemberAuthor
Rebased and now this PR removes only unneeded test code.
@MGatner
MGatner
commented
May 1, 2023

Copy link


Copy Markdown

Member
Looks good, thanks!
@kenjis
kenjis
merged commit d9fd9d5
into
codeigniter4:4.4May 1, 2023
@kenjis
kenjis
deleted the

test-Forge-modifyColumn-null

branch
May 1, 2023 12:20
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@michalsnmichalsnmichalsn approved these changes

Assignees


No one assigned


Labels

databaseIssues or pull requests that affect the database layertestingPull requests that changes tests only

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@kenjis@sclubricants@michalsn@MGatner

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
test: Forge::modifyColumn() and null - #7302
Merged
kenjis merged 1 commit into
codeigniter4:4.4from 
kenjis:test-Forge-modifyColumn-null
May 1, 2023
Merged
test: Forge::modifyColumn() and null#7302
kenjis merged 1 commit into
codeigniter4:4.4from 
kenjis:test-Forge-modifyColumn-null
Conversation
@kenjis
@kenjiskenjis
commented
Feb 24, 2023•

edited

Loading

Copy link


Copy Markdown

Member
Needs #7301
Description

From https://zenn.dev/naente/articles/50935eb55c14ea (CodeIgniter4 PostgreSQL always adds NOT NULL constraints when running modifyColumn())
See the current behavior: #7302 (comment)
  • add test for Forge::modifyColumn() and null
This bug was fixed in v4.3.4:
Related #7235
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
@kenjiskenjis
added

testing
Pull requests that changes tests only
4.4

database
Issues or pull requests that affect the database layer
labels
Feb 24, 2023
@kenjis
kenjis
marked this pull request as draft
February 24, 2023 07:16
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
2 times, most recently
from
5d49dff to
f9c953cCompareFebruary 24, 2023 07:33
@kenjis
kenjis
commented
Feb 24, 2023

Copy link


Copy Markdown

MemberAuthor
@codeigniter4/database-team

It seems Postgre, SQLSRV, SQLite3 add NOT NULL constraints when running modifyColumn().

See the test: https://github.com/codeigniter4/CodeIgniter4/pull/7302/files#diff-489cc58f8341d1fa6ca26b7bb857562d4274924d2dc691ca64e9dc609731b8eaR1285

Are these bugs or not?
@sclubricants
sclubricants
commented
Feb 24, 2023

Copy link


Copy Markdown

Member
Looks like nullable is the default. What is the matter with this?
@kenjis
kenjis
commented
Feb 24, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
I did not look into all drivers, but Postgre changes to NOT NULL in all cases.

There is no way to keep nuallble. So it is clearly a bug.
Another issue is these behaviors are not consistent. As you see all tests pass

on MySQLi and OCI8.
If a dev changes DB driver, e.g. MySQLi to Postgres,

the definitiion of the table will change when running migrations.
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
f9c953c to
a5fc127CompareFebruary 25, 2023 03:30
@kenjis
kenjis
commented
Feb 25, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
$this->forge->addField([
'col1' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => true],
'col2' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => true],
'col3' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => true],
]);
$this->forge->createTable('forge_test_modify');
$this->forge->modifyColumn('forge_test_modify', [
'col1' => ['type' => 'VARCHAR', 'constraint' => 1],
'col2' => ['type' => 'VARCHAR', 'constraint' => 1, 'null' => true],
'col3' => ['type' => 'VARCHAR', 'constraint' => 1, 'null' => false],
]);
$this->db->resetDataCache();
$fields = $this->db->getFieldData('forge_test_modify');
dd($fields);
Postgres:
array (3) [
0 => stdClass#249 (5) (
public 'name' -> string (4) "col1"
public 'type' -> string (17) "character varying"
public 'nullable' -> boolean false // MySQLi true
public 'default' -> null
public 'max_length' -> string (1) "1"
)
1 => stdClass#74 (5) (
public 'name' -> string (4) "col2"
public 'type' -> string (17) "character varying"
public 'nullable' -> boolean false // should be true
public 'default' -> null
public 'max_length' -> string (1) "1"
)
2 => stdClass#75 (5) (
public 'name' -> string (4) "col3"
public 'type' -> string (17) "character varying"
public 'nullable' -> boolean false
public 'default' -> null
public 'max_length' -> string (1) "1"
)
]

SQLSRV:
array (3) [
0 => stdClass#249 (5) (
public 'name' -> string (4) "col1"
public 'type' -> string (7) "varchar"
public 'default' -> null
public 'max_length' -> integer 1
public 'nullable' -> boolean false // MySQLi true
)
1 => stdClass#74 (5) (
public 'name' -> string (4) "col2"
public 'type' -> string (7) "varchar"
public 'default' -> null
public 'max_length' -> integer 1
public 'nullable' -> boolean false // should be true
)
2 => stdClass#75 (5) (
public 'name' -> string (4) "col3"
public 'type' -> string (7) "varchar"
public 'default' -> null
public 'max_length' -> integer 1
public 'nullable' -> boolean false
)
]

SQLite3:
array (3) [
0 => stdClass#77 (6) (
public 'name' -> string (4) "col1"
public 'type' -> string (7) "VARCHAR"
public 'max_length' -> null
public 'default' -> string (2) "''"
public 'primary_key' -> boolean false
public 'nullable' -> boolean false // MySQLi true
)
1 => stdClass#67 (6) (
public 'name' -> string (4) "col2"
public 'type' -> string (7) "VARCHAR"
public 'max_length' -> null
public 'default' -> string (4) "NULL"
public 'primary_key' -> boolean false
public 'nullable' -> boolean true
)
2 => stdClass#602 (6) (
public 'name' -> string (4) "col3"
public 'type' -> string (7) "VARCHAR"
public 'max_length' -> null
public 'default' -> string (4) "NULL"
public 'primary_key' -> boolean false
public 'nullable' -> boolean true // should be false
)
]

Not directly related, but the all max_length values are incorrect on SQLite3.
@kenjiskenjis
added
the 
bug
Verified issues on the current code behavior or pull requests that will fix them
label
Feb 25, 2023
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
a5fc127 to
cedb583CompareMarch 1, 2023 07:00
@michalsn
michalsn
commented
Mar 3, 2023

Copy link


Copy Markdown

Member
Seems like a bug to me.
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
cedb583 to
140ad48CompareMarch 4, 2023 00:42
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
140ad48 to
5232285CompareMarch 23, 2023 01:56
@kenjiskenjis
mentioned this pull request
Mar 23, 2023
 Merged

5 tasks
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
5232285 to
0e86d4cCompareApril 30, 2023 02:49
@kenjiskenjis
removed
the 
bug
Verified issues on the current code behavior or pull requests that will fix them
label
Apr 30, 2023
@kenjis
kenjis
marked this pull request as ready for review
April 30, 2023 02:51
@kenjis
kenjis
commented
Apr 30, 2023

Copy link


Copy Markdown

MemberAuthor
Rebased and now this PR removes only unneeded test code.
@MGatner
MGatner
commented
May 1, 2023

Copy link


Copy Markdown

Member
Looks good, thanks!
@kenjis
kenjis
merged commit d9fd9d5
into
codeigniter4:4.4May 1, 2023
@kenjis
kenjis
deleted the

test-Forge-modifyColumn-null

branch
May 1, 2023 12:20
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@michalsnmichalsnmichalsn approved these changes

Assignees


No one assigned


Labels

databaseIssues or pull requests that affect the database layertestingPull requests that changes tests only

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@kenjis@sclubricants@michalsn@MGatner

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
test: Forge::modifyColumn() and null - #7302
Merged
kenjis merged 1 commit into
codeigniter4:4.4from 
kenjis:test-Forge-modifyColumn-null
May 1, 2023
Merged
test: Forge::modifyColumn() and null#7302
kenjis merged 1 commit into
codeigniter4:4.4from 
kenjis:test-Forge-modifyColumn-null
Conversation
@kenjis
@kenjiskenjis
commented
Feb 24, 2023•

edited

Loading

Copy link


Copy Markdown

Member
Needs #7301
Description

From https://zenn.dev/naente/articles/50935eb55c14ea (CodeIgniter4 PostgreSQL always adds NOT NULL constraints when running modifyColumn())
See the current behavior: #7302 (comment)
  • add test for Forge::modifyColumn() and null
This bug was fixed in v4.3.4:
Related #7235
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
@kenjiskenjis
added

testing
Pull requests that changes tests only
4.4

database
Issues or pull requests that affect the database layer
labels
Feb 24, 2023
@kenjis
kenjis
marked this pull request as draft
February 24, 2023 07:16
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
2 times, most recently
from
5d49dff to
f9c953cCompareFebruary 24, 2023 07:33
@kenjis
kenjis
commented
Feb 24, 2023

Copy link


Copy Markdown

MemberAuthor
@codeigniter4/database-team

It seems Postgre, SQLSRV, SQLite3 add NOT NULL constraints when running modifyColumn().

See the test: https://github.com/codeigniter4/CodeIgniter4/pull/7302/files#diff-489cc58f8341d1fa6ca26b7bb857562d4274924d2dc691ca64e9dc609731b8eaR1285

Are these bugs or not?
@sclubricants
sclubricants
commented
Feb 24, 2023

Copy link


Copy Markdown

Member
Looks like nullable is the default. What is the matter with this?
@kenjis
kenjis
commented
Feb 24, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
I did not look into all drivers, but Postgre changes to NOT NULL in all cases.

There is no way to keep nuallble. So it is clearly a bug.
Another issue is these behaviors are not consistent. As you see all tests pass

on MySQLi and OCI8.
If a dev changes DB driver, e.g. MySQLi to Postgres,

the definitiion of the table will change when running migrations.
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
f9c953c to
a5fc127CompareFebruary 25, 2023 03:30
@kenjis
kenjis
commented
Feb 25, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
$this->forge->addField([
'col1' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => true],
'col2' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => true],
'col3' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => true],
]);
$this->forge->createTable('forge_test_modify');
$this->forge->modifyColumn('forge_test_modify', [
'col1' => ['type' => 'VARCHAR', 'constraint' => 1],
'col2' => ['type' => 'VARCHAR', 'constraint' => 1, 'null' => true],
'col3' => ['type' => 'VARCHAR', 'constraint' => 1, 'null' => false],
]);
$this->db->resetDataCache();
$fields = $this->db->getFieldData('forge_test_modify');
dd($fields);
Postgres:
array (3) [
0 => stdClass#249 (5) (
public 'name' -> string (4) "col1"
public 'type' -> string (17) "character varying"
public 'nullable' -> boolean false // MySQLi true
public 'default' -> null
public 'max_length' -> string (1) "1"
)
1 => stdClass#74 (5) (
public 'name' -> string (4) "col2"
public 'type' -> string (17) "character varying"
public 'nullable' -> boolean false // should be true
public 'default' -> null
public 'max_length' -> string (1) "1"
)
2 => stdClass#75 (5) (
public 'name' -> string (4) "col3"
public 'type' -> string (17) "character varying"
public 'nullable' -> boolean false
public 'default' -> null
public 'max_length' -> string (1) "1"
)
]

SQLSRV:
array (3) [
0 => stdClass#249 (5) (
public 'name' -> string (4) "col1"
public 'type' -> string (7) "varchar"
public 'default' -> null
public 'max_length' -> integer 1
public 'nullable' -> boolean false // MySQLi true
)
1 => stdClass#74 (5) (
public 'name' -> string (4) "col2"
public 'type' -> string (7) "varchar"
public 'default' -> null
public 'max_length' -> integer 1
public 'nullable' -> boolean false // should be true
)
2 => stdClass#75 (5) (
public 'name' -> string (4) "col3"
public 'type' -> string (7) "varchar"
public 'default' -> null
public 'max_length' -> integer 1
public 'nullable' -> boolean false
)
]

SQLite3:
array (3) [
0 => stdClass#77 (6) (
public 'name' -> string (4) "col1"
public 'type' -> string (7) "VARCHAR"
public 'max_length' -> null
public 'default' -> string (2) "''"
public 'primary_key' -> boolean false
public 'nullable' -> boolean false // MySQLi true
)
1 => stdClass#67 (6) (
public 'name' -> string (4) "col2"
public 'type' -> string (7) "VARCHAR"
public 'max_length' -> null
public 'default' -> string (4) "NULL"
public 'primary_key' -> boolean false
public 'nullable' -> boolean true
)
2 => stdClass#602 (6) (
public 'name' -> string (4) "col3"
public 'type' -> string (7) "VARCHAR"
public 'max_length' -> null
public 'default' -> string (4) "NULL"
public 'primary_key' -> boolean false
public 'nullable' -> boolean true // should be false
)
]

Not directly related, but the all max_length values are incorrect on SQLite3.
@kenjiskenjis
added
the 
bug
Verified issues on the current code behavior or pull requests that will fix them
label
Feb 25, 2023
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
a5fc127 to
cedb583CompareMarch 1, 2023 07:00
@michalsn
michalsn
commented
Mar 3, 2023

Copy link


Copy Markdown

Member
Seems like a bug to me.
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
cedb583 to
140ad48CompareMarch 4, 2023 00:42
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
140ad48 to
5232285CompareMarch 23, 2023 01:56
@kenjiskenjis
mentioned this pull request
Mar 23, 2023
 Merged

5 tasks
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
5232285 to
0e86d4cCompareApril 30, 2023 02:49
@kenjiskenjis
removed
the 
bug
Verified issues on the current code behavior or pull requests that will fix them
label
Apr 30, 2023
@kenjis
kenjis
marked this pull request as ready for review
April 30, 2023 02:51
@kenjis
kenjis
commented
Apr 30, 2023

Copy link


Copy Markdown

MemberAuthor
Rebased and now this PR removes only unneeded test code.
@MGatner
MGatner
commented
May 1, 2023

Copy link


Copy Markdown

Member
Looks good, thanks!
@kenjis
kenjis
merged commit d9fd9d5
into
codeigniter4:4.4May 1, 2023
@kenjis
kenjis
deleted the

test-Forge-modifyColumn-null

branch
May 1, 2023 12:20
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@michalsnmichalsnmichalsn approved these changes

Assignees


No one assigned


Labels

databaseIssues or pull requests that affect the database layertestingPull requests that changes tests only

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@kenjis@sclubricants@michalsn@MGatner

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
test: Forge::modifyColumn() and null - #7302
Merged
kenjis merged 1 commit into
codeigniter4:4.4from 
kenjis:test-Forge-modifyColumn-null
May 1, 2023
Merged
test: Forge::modifyColumn() and null#7302
kenjis merged 1 commit into
codeigniter4:4.4from 
kenjis:test-Forge-modifyColumn-null
Conversation
@kenjis
@kenjiskenjis
commented
Feb 24, 2023•

edited

Loading

Copy link


Copy Markdown

Member
Needs #7301
Description

From https://zenn.dev/naente/articles/50935eb55c14ea (CodeIgniter4 PostgreSQL always adds NOT NULL constraints when running modifyColumn())
See the current behavior: #7302 (comment)
  • add test for Forge::modifyColumn() and null
This bug was fixed in v4.3.4:
Related #7235
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
@kenjiskenjis
added

testing
Pull requests that changes tests only
4.4

database
Issues or pull requests that affect the database layer
labels
Feb 24, 2023
@kenjis
kenjis
marked this pull request as draft
February 24, 2023 07:16
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
2 times, most recently
from
5d49dff to
f9c953cCompareFebruary 24, 2023 07:33
@kenjis
kenjis
commented
Feb 24, 2023

Copy link


Copy Markdown

MemberAuthor
@codeigniter4/database-team

It seems Postgre, SQLSRV, SQLite3 add NOT NULL constraints when running modifyColumn().

See the test: https://github.com/codeigniter4/CodeIgniter4/pull/7302/files#diff-489cc58f8341d1fa6ca26b7bb857562d4274924d2dc691ca64e9dc609731b8eaR1285

Are these bugs or not?
@sclubricants
sclubricants
commented
Feb 24, 2023

Copy link


Copy Markdown

Member
Looks like nullable is the default. What is the matter with this?
@kenjis
kenjis
commented
Feb 24, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
I did not look into all drivers, but Postgre changes to NOT NULL in all cases.

There is no way to keep nuallble. So it is clearly a bug.
Another issue is these behaviors are not consistent. As you see all tests pass

on MySQLi and OCI8.
If a dev changes DB driver, e.g. MySQLi to Postgres,

the definitiion of the table will change when running migrations.
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
f9c953c to
a5fc127CompareFebruary 25, 2023 03:30
@kenjis
kenjis
commented
Feb 25, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
$this->forge->addField([
'col1' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => true],
'col2' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => true],
'col3' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => true],
]);
$this->forge->createTable('forge_test_modify');
$this->forge->modifyColumn('forge_test_modify', [
'col1' => ['type' => 'VARCHAR', 'constraint' => 1],
'col2' => ['type' => 'VARCHAR', 'constraint' => 1, 'null' => true],
'col3' => ['type' => 'VARCHAR', 'constraint' => 1, 'null' => false],
]);
$this->db->resetDataCache();
$fields = $this->db->getFieldData('forge_test_modify');
dd($fields);
Postgres:
array (3) [
0 => stdClass#249 (5) (
public 'name' -> string (4) "col1"
public 'type' -> string (17) "character varying"
public 'nullable' -> boolean false // MySQLi true
public 'default' -> null
public 'max_length' -> string (1) "1"
)
1 => stdClass#74 (5) (
public 'name' -> string (4) "col2"
public 'type' -> string (17) "character varying"
public 'nullable' -> boolean false // should be true
public 'default' -> null
public 'max_length' -> string (1) "1"
)
2 => stdClass#75 (5) (
public 'name' -> string (4) "col3"
public 'type' -> string (17) "character varying"
public 'nullable' -> boolean false
public 'default' -> null
public 'max_length' -> string (1) "1"
)
]

SQLSRV:
array (3) [
0 => stdClass#249 (5) (
public 'name' -> string (4) "col1"
public 'type' -> string (7) "varchar"
public 'default' -> null
public 'max_length' -> integer 1
public 'nullable' -> boolean false // MySQLi true
)
1 => stdClass#74 (5) (
public 'name' -> string (4) "col2"
public 'type' -> string (7) "varchar"
public 'default' -> null
public 'max_length' -> integer 1
public 'nullable' -> boolean false // should be true
)
2 => stdClass#75 (5) (
public 'name' -> string (4) "col3"
public 'type' -> string (7) "varchar"
public 'default' -> null
public 'max_length' -> integer 1
public 'nullable' -> boolean false
)
]

SQLite3:
array (3) [
0 => stdClass#77 (6) (
public 'name' -> string (4) "col1"
public 'type' -> string (7) "VARCHAR"
public 'max_length' -> null
public 'default' -> string (2) "''"
public 'primary_key' -> boolean false
public 'nullable' -> boolean false // MySQLi true
)
1 => stdClass#67 (6) (
public 'name' -> string (4) "col2"
public 'type' -> string (7) "VARCHAR"
public 'max_length' -> null
public 'default' -> string (4) "NULL"
public 'primary_key' -> boolean false
public 'nullable' -> boolean true
)
2 => stdClass#602 (6) (
public 'name' -> string (4) "col3"
public 'type' -> string (7) "VARCHAR"
public 'max_length' -> null
public 'default' -> string (4) "NULL"
public 'primary_key' -> boolean false
public 'nullable' -> boolean true // should be false
)
]

Not directly related, but the all max_length values are incorrect on SQLite3.
@kenjiskenjis
added
the 
bug
Verified issues on the current code behavior or pull requests that will fix them
label
Feb 25, 2023
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
a5fc127 to
cedb583CompareMarch 1, 2023 07:00
@michalsn
michalsn
commented
Mar 3, 2023

Copy link


Copy Markdown

Member
Seems like a bug to me.
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
cedb583 to
140ad48CompareMarch 4, 2023 00:42
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
140ad48 to
5232285CompareMarch 23, 2023 01:56
@kenjiskenjis
mentioned this pull request
Mar 23, 2023
 Merged

5 tasks
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
5232285 to
0e86d4cCompareApril 30, 2023 02:49
@kenjiskenjis
removed
the 
bug
Verified issues on the current code behavior or pull requests that will fix them
label
Apr 30, 2023
@kenjis
kenjis
marked this pull request as ready for review
April 30, 2023 02:51
@kenjis
kenjis
commented
Apr 30, 2023

Copy link


Copy Markdown

MemberAuthor
Rebased and now this PR removes only unneeded test code.
@MGatner
MGatner
commented
May 1, 2023

Copy link


Copy Markdown

Member
Looks good, thanks!
@kenjis
kenjis
merged commit d9fd9d5
into
codeigniter4:4.4May 1, 2023
@kenjis
kenjis
deleted the

test-Forge-modifyColumn-null

branch
May 1, 2023 12:20
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@michalsnmichalsnmichalsn approved these changes

Assignees


No one assigned


Labels

databaseIssues or pull requests that affect the database layertestingPull requests that changes tests only

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@kenjis@sclubricants@michalsn@MGatner

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
test: Forge::modifyColumn() and null - #7302
Merged
kenjis merged 1 commit into
codeigniter4:4.4from 
kenjis:test-Forge-modifyColumn-null
May 1, 2023
Merged
test: Forge::modifyColumn() and null#7302
kenjis merged 1 commit into
codeigniter4:4.4from 
kenjis:test-Forge-modifyColumn-null
Conversation
@kenjis
@kenjiskenjis
commented
Feb 24, 2023•

edited

Loading

Copy link


Copy Markdown

Member
Needs #7301
Description

From https://zenn.dev/naente/articles/50935eb55c14ea (CodeIgniter4 PostgreSQL always adds NOT NULL constraints when running modifyColumn())
See the current behavior: #7302 (comment)
  • add test for Forge::modifyColumn() and null
This bug was fixed in v4.3.4:
Related #7235
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
@kenjiskenjis
added

testing
Pull requests that changes tests only
4.4

database
Issues or pull requests that affect the database layer
labels
Feb 24, 2023
@kenjis
kenjis
marked this pull request as draft
February 24, 2023 07:16
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
2 times, most recently
from
5d49dff to
f9c953cCompareFebruary 24, 2023 07:33
@kenjis
kenjis
commented
Feb 24, 2023

Copy link


Copy Markdown

MemberAuthor
@codeigniter4/database-team

It seems Postgre, SQLSRV, SQLite3 add NOT NULL constraints when running modifyColumn().

See the test: https://github.com/codeigniter4/CodeIgniter4/pull/7302/files#diff-489cc58f8341d1fa6ca26b7bb857562d4274924d2dc691ca64e9dc609731b8eaR1285

Are these bugs or not?
@sclubricants
sclubricants
commented
Feb 24, 2023

Copy link


Copy Markdown

Member
Looks like nullable is the default. What is the matter with this?
@kenjis
kenjis
commented
Feb 24, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
I did not look into all drivers, but Postgre changes to NOT NULL in all cases.

There is no way to keep nuallble. So it is clearly a bug.
Another issue is these behaviors are not consistent. As you see all tests pass

on MySQLi and OCI8.
If a dev changes DB driver, e.g. MySQLi to Postgres,

the definitiion of the table will change when running migrations.
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
f9c953c to
a5fc127CompareFebruary 25, 2023 03:30
@kenjis
kenjis
commented
Feb 25, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
$this->forge->addField([
'col1' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => true],
'col2' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => true],
'col3' => ['type' => 'VARCHAR', 'constraint' => 255, 'null' => true],
]);
$this->forge->createTable('forge_test_modify');
$this->forge->modifyColumn('forge_test_modify', [
'col1' => ['type' => 'VARCHAR', 'constraint' => 1],
'col2' => ['type' => 'VARCHAR', 'constraint' => 1, 'null' => true],
'col3' => ['type' => 'VARCHAR', 'constraint' => 1, 'null' => false],
]);
$this->db->resetDataCache();
$fields = $this->db->getFieldData('forge_test_modify');
dd($fields);
Postgres:
array (3) [
0 => stdClass#249 (5) (
public 'name' -> string (4) "col1"
public 'type' -> string (17) "character varying"
public 'nullable' -> boolean false // MySQLi true
public 'default' -> null
public 'max_length' -> string (1) "1"
)
1 => stdClass#74 (5) (
public 'name' -> string (4) "col2"
public 'type' -> string (17) "character varying"
public 'nullable' -> boolean false // should be true
public 'default' -> null
public 'max_length' -> string (1) "1"
)
2 => stdClass#75 (5) (
public 'name' -> string (4) "col3"
public 'type' -> string (17) "character varying"
public 'nullable' -> boolean false
public 'default' -> null
public 'max_length' -> string (1) "1"
)
]

SQLSRV:
array (3) [
0 => stdClass#249 (5) (
public 'name' -> string (4) "col1"
public 'type' -> string (7) "varchar"
public 'default' -> null
public 'max_length' -> integer 1
public 'nullable' -> boolean false // MySQLi true
)
1 => stdClass#74 (5) (
public 'name' -> string (4) "col2"
public 'type' -> string (7) "varchar"
public 'default' -> null
public 'max_length' -> integer 1
public 'nullable' -> boolean false // should be true
)
2 => stdClass#75 (5) (
public 'name' -> string (4) "col3"
public 'type' -> string (7) "varchar"
public 'default' -> null
public 'max_length' -> integer 1
public 'nullable' -> boolean false
)
]

SQLite3:
array (3) [
0 => stdClass#77 (6) (
public 'name' -> string (4) "col1"
public 'type' -> string (7) "VARCHAR"
public 'max_length' -> null
public 'default' -> string (2) "''"
public 'primary_key' -> boolean false
public 'nullable' -> boolean false // MySQLi true
)
1 => stdClass#67 (6) (
public 'name' -> string (4) "col2"
public 'type' -> string (7) "VARCHAR"
public 'max_length' -> null
public 'default' -> string (4) "NULL"
public 'primary_key' -> boolean false
public 'nullable' -> boolean true
)
2 => stdClass#602 (6) (
public 'name' -> string (4) "col3"
public 'type' -> string (7) "VARCHAR"
public 'max_length' -> null
public 'default' -> string (4) "NULL"
public 'primary_key' -> boolean false
public 'nullable' -> boolean true // should be false
)
]

Not directly related, but the all max_length values are incorrect on SQLite3.
@kenjiskenjis
added
the 
bug
Verified issues on the current code behavior or pull requests that will fix them
label
Feb 25, 2023
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
a5fc127 to
cedb583CompareMarch 1, 2023 07:00
@michalsn
michalsn
commented
Mar 3, 2023

Copy link


Copy Markdown

Member
Seems like a bug to me.
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
cedb583 to
140ad48CompareMarch 4, 2023 00:42
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
140ad48 to
5232285CompareMarch 23, 2023 01:56
@kenjiskenjis
mentioned this pull request
Mar 23, 2023
 Merged

5 tasks
@kenjis
kenjisforce-pushed
the
test-Forge-modifyColumn-null
branch
from
5232285 to
0e86d4cCompareApril 30, 2023 02:49
@kenjiskenjis
removed
the 
bug
Verified issues on the current code behavior or pull requests that will fix them
label
Apr 30, 2023
@kenjis
kenjis
marked this pull request as ready for review
April 30, 2023 02:51
@kenjis
kenjis
commented
Apr 30, 2023

Copy link


Copy Markdown

MemberAuthor
Rebased and now this PR removes only unneeded test code.
@MGatner
MGatner
commented
May 1, 2023

Copy link


Copy Markdown

Member
Looks good, thanks!
@kenjis
kenjis
merged commit d9fd9d5
into
codeigniter4:4.4May 1, 2023
@kenjis
kenjis
deleted the

test-Forge-modifyColumn-null

branch
May 1, 2023 12:20
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@michalsnmichalsnmichalsn approved these changes

Assignees


No one assigned


Labels

databaseIssues or pull requests that affect the database layertestingPull requests that changes tests only

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@kenjis@sclubricants@michalsn@MGatner