Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Convert int to string when execute insert query if the type of mysql column is 'bigint unsigned' #14663

Closed
bittenApple opened this issue Aug 18, 2017 · 17 comments

Comments

@bittenApple
Copy link

bittenApple commented Aug 18, 2017

What steps will reproduce the problem?

  1. Create a MySQL table and mapped ActiveRecord.
create table foo(id bigint unsigned);

Save a model

    $f = new Foo;
    $f->id = 1;
    $f->save();

What is the expected result?

The query should be

INSERT INTO `foo` (`id`) VALUES (1)

What do you get instead?

The executed query is

INSERT INTO `foo` (`id`) VALUES ('1')

The range of MySQL unsigned bigint is larger than PHP_INT_SIZE, but it's not reasonable to convert int attribute manually set to string when execute sql, which may slow the query performance.

Additional info

Q A
Yii version 2.0.12
PHP version 7.1.3
Operating system CentOS 7
MySQL 5.7.16
@bittenApple bittenApple changed the title Convert int to string when execute sql query if the type of mysql column is 'bigint unsigned' Convert int to string when execute insert query if the type of mysql column is 'bigint unsigned' Aug 18, 2017
@fcaldarelli
Copy link
Member

Have you checked that it is not related to PDO ?

@yyxx9988
Copy link
Contributor

This is not a bug.

The maximum int value of php depends on the system. 32 bit systems have a maximum signed integer range of -2147483648 to 2147483647. So for example on such a system, intval('1000000000000') will return 2147483647.

see http://php.net/manual/en/language.types.integer.php for more details

@samdark samdark closed this as completed Aug 19, 2017
@bittenApple
Copy link
Author

bittenApple commented Aug 21, 2017

This is not a bug.

The maximum int value of php depends on the system. 32 bit systems have a maximum signed integer range of -2147483648 to 2147483647. So for example on such a system, intval('1000000000000') will return 2147483647.

see http://php.net/manual/en/language.types.integer.php for more details

I think it's the programmer's responsibility to take care of the risk of int overflow.
If the framework user set the int value of an attribute explicitly, I don't think it's appropriate to cast int to string implicitly by framework

@cebe
Copy link
Member

cebe commented Aug 21, 2017

The executed query is

how did you verify the type of the value bound on query execution time? These are bound as parameter values and are converted inside of PDO, Yii has no logic to convert int to string by default in that case. I can not reproduce the issue. Please provide the CREATE TABLE statement of your table.

@bittenApple
Copy link
Author

bittenApple commented Aug 23, 2017

how did you verify the type of the value bound on query execution time

I find some slow query in my production MySQL slow log which the type of value is not as expected. Maybe the performance of type casting is not the key reason for mysql, but I still think it's better not to cast the type of value if user set it explicitly.

These are bound as parameter values and are converted inside of PDO, Yii has no logic to convert int to string by default in that case

I think the following code do the casting.

$params[$phName] = !is_array($value) && isset($columnSchemas[$name]) ? $columnSchemas[$name]->dbTypecast($value) : $value;

Please provide the CREATE TABLE statement of your table.

create table foo(id bigint unsigned);

Insert any table which has bigint unsigned type would case the casting.

@cebe
Copy link
Member

cebe commented Aug 23, 2017

yeah, the reason for this is that bigint unsigned is bigger than integer in PHP so the string representation is safe to use, while integer may overflow.

@cebe
Copy link
Member

cebe commented Aug 23, 2017

I find some slow query in my production MySQL slow log which the type of value is not as expected.

can you provide more information about this? What exactly is logged in MySQL?

@bittenApple
Copy link
Author

bittenApple commented Aug 23, 2017

can you provide more information about this? What exactly is logged in MySQL?

The table has a union index with two unsigned bigint columns and has ten million of rows. There are some inserted sql logged at MySQL slow query log. Although I think it's not a heavy work doing type casting for mysql, I still migrated the unsigned bigint to signed.

yeah, the reason for this is that bigint unsigned is bigger than integer in PHP so the string representation is safe to use, while integer may overflow.

The overflow only happened when string casted to int, but if programmer set the type as int explicitly, my point is that maybe it's not necessary to cast int back to string.

@cebe
Copy link
Member

cebe commented Aug 23, 2017

The overflow only happened when string casted to int, but if programmer set the type as int explicitly, my point is that maybe it's not necessary to cast int back to string.

it is probably not necessary, but we need to find the condition to find that case.

@cebe cebe reopened this Aug 23, 2017
@samdark samdark added this to the 2.0.15 milestone Aug 25, 2017
@samdark samdark modified the milestones: 2.0.15, 2.0.14 Feb 3, 2018
@samdark samdark modified the milestones: 2.0.14, 2.0.15 Feb 11, 2018
@machour machour modified the milestones: 2.0.16, 2.0.17 Jan 14, 2019
@samdark samdark removed this from the 2.0.17 milestone Mar 9, 2019
@samdark samdark closed this as completed Feb 8, 2021
samdark pushed a commit that referenced this issue Feb 8, 2021
@samdark samdark reopened this Feb 9, 2021
@darkdef
Copy link
Contributor

darkdef commented Feb 9, 2021

I'm check query for this code

        $model = new Order();
        $model->customer_id = 1000000000000;
        $model->save(false);

Result query is
``sql
INSERT INTO order (`customer_id`, `created_at`) VALUES (1000000000000, 1612863588)

@darkdef
Copy link
Contributor

darkdef commented Feb 9, 2021

for the

$model->customer_id = 10000000000000000000000;

sql

INSERT INTO `order` (`customer_id`, `created_at`) VALUES (1864712049423024128, 1612863706)

@bizley
Copy link
Member

bizley commented Feb 9, 2021

PHP_INT_MAX check required?

@darkdef
Copy link
Contributor

darkdef commented Feb 9, 2021

PHP_INT_MAX check required?

You may add rules to model. But generated sql query is correct.

@samdark samdark closed this as completed Feb 9, 2021
@rob006
Copy link
Contributor

rob006 commented Feb 9, 2021

@darkdef What is the type of customer_id column? Try convert it to unsigned bigint. The inserted value is irrelevant here, incorrect typecast comes from column schema, you will get the same result even for $model->customer_id = 1 (as explained in first comment).

@darkdef
Copy link
Contributor

darkdef commented Feb 9, 2021

You right. Reproduced the problem

@samdark samdark reopened this Feb 9, 2021
@egorrishe
Copy link
Contributor

egorrishe commented Jun 30, 2021

phpunit --filter testInsertInteger tests/framework/db/mysql/QueryBuilderTest.php
Failed on PHP 5.6:

Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'INSERT INTO customer (profile_id) VALUES (1000000000000)'
+'INSERT INTO customer (profile_id) VALUES (2147483647)'

The value 2147483647 is correct - because type of customer.profile_id is INT (mysql.sql). And it's maximum in MySql is 2147483647.
So behaviour is ok. Test should be changed.

egorrishe added a commit to egorrishe/yii2 that referenced this issue Jul 2, 2021
egorrishe added a commit to egorrishe/yii2 that referenced this issue Jul 2, 2021
egorrishe added a commit to egorrishe/yii2 that referenced this issue Jul 2, 2021
egorrishe added a commit to egorrishe/yii2 that referenced this issue Jul 3, 2021
egorrishe added a commit to egorrishe/yii2 that referenced this issue Jul 6, 2021
egorrishe added a commit to egorrishe/yii2 that referenced this issue Jul 6, 2021
egorrishe added a commit to egorrishe/yii2 that referenced this issue Jul 6, 2021
egorrishe added a commit to egorrishe/yii2 that referenced this issue Jul 6, 2021
bizley pushed a commit that referenced this issue Jul 6, 2021
* write adequate test for issue #14663

* don't convert int to string if db type of column is numeric (#14663)

* fix bigint schema test for MySql >8.0.17 (#14663)

* Update CHANGELOG.md

* Update CHANGELOG.md

* update phpdoc [ci skip] (#14663)

* refactoring test case to make it clearer [ci skip] (#14663)

* check `int unsigned` in `QueryBuilderTest::testInsertInteger()` (#14663)

* Update Upgrade.md (#14663)

* fix `int unsigned` schema test for MySql >8.0.17 (#14663)

* fix `int unsigned` schema test for MySql <5.7 (#14663)

Co-authored-by: Alexander Makarov <sam@rmcreative.ru>
Co-authored-by: Bizley <pawel@positive.codes>
@bizley
Copy link
Member

bizley commented Jul 6, 2021

Closed via 17742cb

@bizley bizley closed this as completed Jul 6, 2021
@bizley bizley added this to the 2.0.43 milestone Jul 6, 2021
markhuot pushed a commit to markhuot/yii2 that referenced this issue Jul 27, 2021
…18741)

* write adequate test for issue yiisoft#14663

* don't convert int to string if db type of column is numeric (yiisoft#14663)

* fix bigint schema test for MySql >8.0.17 (yiisoft#14663)

* Update CHANGELOG.md

* Update CHANGELOG.md

* update phpdoc [ci skip] (yiisoft#14663)

* refactoring test case to make it clearer [ci skip] (yiisoft#14663)

* check `int unsigned` in `QueryBuilderTest::testInsertInteger()` (yiisoft#14663)

* Update Upgrade.md (yiisoft#14663)

* fix `int unsigned` schema test for MySql >8.0.17 (yiisoft#14663)

* fix `int unsigned` schema test for MySql <5.7 (yiisoft#14663)

Co-authored-by: Alexander Makarov <sam@rmcreative.ru>
Co-authored-by: Bizley <pawel@positive.codes>
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

10 participants