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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test with MySQL 8 #4721

Merged
merged 3 commits into from
May 22, 2021
Merged

Test with MySQL 8 #4721

merged 3 commits into from
May 22, 2021

Conversation

jeromegamez
Copy link
Contributor

@jeromegamez jeromegamez commented May 21, 2021

Greetings 馃憢 ,

I'm running PHP 8 and MySQL 8 on my machine and noticed one failing test (ForgeTest::testAddFields(), it seems that the max_length cannot be retrieved for BIGINT fields).

if ($this->db->DBDriver === 'MySQLi')
{
// Check types
$this->assertEquals($fieldsData[0]->type, 'int');
$this->assertEquals($fieldsData[1]->type, 'varchar');
$this->assertEquals($fieldsData[0]->max_length, 11);

Here's the failing test on my fork: https://github.com/jeromegamez/CodeIgniter4/runs/2643301715?check_suite_focus=true

In order to be able to show the error, I started by adding it to the test matrix (but am not sure how to change the workflow name to reflect it, since all matrix combinations start all types of databases).

The added mysql8 branch in the triggers is only so that I could run the action, this would of course be removed before merging, if you wish to proceed, that is 馃槄

I don't know CodeIgniter at all and am only using this repo, so, if you have an idea why this test might fail, I'm all ears馃憘, otherwise, I will try to dive deeper into it in the hopes to find a good explanation or even a fix 馃槄

:octocat:

@paulbalandan
Copy link
Member

According to the MySQL documentation, this feature is already deprecated as of v8.0.17.

As of MySQL 8.0.17, the display width attribute is deprecated for integer data types; you should expect support for it to be removed in a future version of MySQL.

@jeromegamez
Copy link
Contributor Author

Thanks for the research, @paulbalandan 馃! I added a commit so that the max_length check is skipped when tested with MySQL >= 8.0.17

(The formatting change in the testCreateTableWithArrayFieldConstraints() method was automatically applied by the project's pre-commit hook)

@jeromegamez jeromegamez marked this pull request as ready for review May 22, 2021 16:34
Copy link
Member

@MGatner MGatner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice sleuthing! I can see I need to spend some more time on MySQL 8 馃槸

@MGatner MGatner merged commit f925658 into codeigniter4:develop May 22, 2021
@jeromegamez jeromegamez deleted the mysql8 branch May 22, 2021 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants