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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MySQL integration test fail in section testNamedParameters #214

Open
ZVanoZ opened this issue Sep 22, 2021 · 8 comments
Open

MySQL integration test fail in section testNamedParameters #214

ZVanoZ opened this issue Sep 22, 2021 · 8 comments
Labels
Bug Something isn't working
Milestone

Comments

@ZVanoZ
Copy link
Contributor

ZVanoZ commented Sep 22, 2021

Bug Report

Q A
Version(s) 2.14.x

Note, PHP 7.4 in custom Docker image.

Summary

Bound parameters by index is invalid.

Current behavior

When we run tests, laminas-db generate SQL

UPDATE `test` SET `name` = :c_0, `value` = :c_1 WHERE `id` = :where1

with params

Array
(
    "c_0" => 1,
    "c_1" => foo,
    "where1" => bar
)

and we have an error

1) LaminasIntegrationTest\Db\Adapter\Driver\Pdo\Mysql\QueryTest::testNamedParameters
Laminas\Db\Adapter\Exception\InvalidQueryException: Statement could not be executed (22007 - 1292 - Truncated incorrect DOUBLE value: 'bar')

I think, it happend becouse value ["where1" => bar] bonded with id = :where1
id has type int.

How to reproduce

Edit "phpunit.xml" and set TESTS_LAMINAS_DB_ADAPTER_DRIVER_MYSQL in state true
Other adapters set state false.

# Set current directory "laminas-db" sources
$ cd laminas-db

# Run tests
$ docker run --rm -it --volume $(pwd):/app zvanoz/laminas-db-test-by-docker:74 composer -vvv test-integration
Reading ./composer.json (/app/composer.json)
Loading config file ./composer.json (/app/composer.json)
Checked CA file /etc/ssl/certs/ca-certificates.crt: valid
Executing command (/app): git branch -a --no-color --no-abbrev -v
Executing command (/app): git describe --exact-match --tags
Executing command (CWD): git --version
Executing command (/app): git log --pretty="%H" -n1 HEAD
Executing command (/app): hg branch
Executing command (/app): fossil branch list
Executing command (/app): fossil tag list
Executing command (/app): svn info --xml
Failed to initialize global composer: Composer could not find the config file: /root/.config/composer/composer.json

Reading /app/vendor/composer/installed.json
Loading plugin Dealerdirect\Composer\Plugin\Installers\PHPCodeSniffer\Plugin (from dealerdirect/phpcodesniffer-composer-installer)
Running 2.1.8 (2021-09-15 13:55:14) with PHP 7.4.3 on Linux / 4.19.0-17-amd64
> test-integration: phpunit --colors=always --testsuite "integration test"
Executing command (CWD): phpunit --colors=always --testsuite "integration test"
PHPUnit 9.5.9 by Sebastian Bergmann and contributors.


Integration test started.
............E.........SSS..SSSS                                   31 / 31 (100%)

Time: 00:03.203, Memory: 10.00 MB

There was 1 error:

1) LaminasIntegrationTest\Db\Adapter\Driver\Pdo\Mysql\QueryTest::testNamedParameters
Laminas\Db\Adapter\Exception\InvalidQueryException: Statement could not be executed (22007 - 1292 - Truncated incorrect DOUBLE value: 'bar')

/app/src/Adapter/Driver/Pdo/Statement.php:223
/app/test/integration/Adapter/Driver/Pdo/Mysql/QueryTest.php:87

Caused by
PDOException: SQLSTATE[22007]: Invalid datetime format: 1292 Truncated incorrect DOUBLE value: 'bar'

/app/src/Adapter/Driver/Pdo/Statement.php:212
/app/test/integration/Adapter/Driver/Pdo/Mysql/QueryTest.php:87

ERRORS!
Tests: 31, Assertions: 72, Errors: 1, Skipped: 7.
Script phpunit --colors=always --testsuite "integration test" handling the test-integration event returned with error code 2

Expected behavior

All tests is success.

@ZVanoZ ZVanoZ added the Bug Something isn't working label Sep 22, 2021
@Ocramius Ocramius added this to the 2.13.5 milestone Sep 22, 2021
@Ocramius
Copy link
Member

Could you check if:

  • 2.13.x is affected
  • PHP 8.0 and 8.1 are affected

Truncated incorrect DOUBLE value: 'bar'

This seems like MySQL telling us that bar is not a valid bound parameter for that column: could it be that the MySQL instance is running with stricter checks, and is trying to cast 'bar' to double?

@ZVanoZ
Copy link
Contributor Author

ZVanoZ commented Sep 23, 2021

Could you check if:
* 2.13.x is affected

I did it for PHP 7.4 CLI and have the same result.

  1. Prepare to tests
# Went to clone of "laminas-db" and check "is all ok?"

$ git remote -v
# origin	https://github.com/laminas/laminas-db.git (fetch)
# origin	https://github.com/laminas/laminas-db.git (push)

$ git branch -a
#   2.12.x
# * 2.13.x
#   2.14.x
#  remotes/origin/2.11.x
#  remotes/origin/2.12.x
#  remotes/origin/2.13.x
#  remotes/origin/2.13.x-merge-up-into-2.14.x_3Y5r1t4x
#  remotes/origin/2.13.x-merge-up-into-2.14.x_LxxfGKFv
#  remotes/origin/2.14.x
#  remotes/origin/HEAD -> origin/2.13.x
#  remotes/origin/dev-3.0.0
#  remotes/origin/dev-3.0.0-old
#  remotes/origin/gh-pages

# Update vendor libraies
$ docker run --rm -it --volume $(pwd):/app zvanoz/laminas-db-test-by-docker:74 composer update
# Loading composer repositories with package information
# Updating dependencies
# Lock file operations: 0 installs, 4 updates, 1 removal
#  - Removing laminas/laminas-zendframework-bridge (1.4.0)
#  - Upgrading laminas/laminas-servicemanager (3.7.0 => 3.10.0)
#  - Upgrading nikic/php-parser (v4.12.0 => v4.13.0)
#  - Upgrading phpdocumentor/type-resolver (1.4.0 => 1.5.0)
#  - Upgrading phpunit/php-code-coverage (9.2.6 => 9.2.7)
# Writing lock file
# Installing dependencies from lock file (including require-dev)
# Package operations: 46 installs, 0 updates, 0 removals
# ...
# Package container-interop/container-interop is abandoned, you should avoid using it. Use psr/container instead.
# Generating autoload files
# 33 packages you are using are looking for funding.
# Use the `composer fund` command to find out more!
# PHP CodeSniffer Config installed_paths set to ../../laminas/laminas-coding-standard/src,../../slevomat/coding-standard,../../webimpress,../../webimpress/coding-standard/src

$ git status
# На ветке 2.13.x
# Ваша ветка обновлена в соответствии с «origin/2.13.x».
#
# Изменения, которые не в индексе для коммита:
#   (используйте «git add <файл>…», чтобы добавить файл в индекс)
#  (используйте «git checkout -- <файл>…», чтобы отменить изменения
#   в рабочем каталоге)
#
#	изменено:      composer.lock
  1. Run tests
$ docker run --rm -it --volume $(pwd):/app zvanoz/laminas-db-test-by-docker:74 composer -vvv test-integration

Result

Reading ./composer.json (/app/composer.json)
Loading config file ./composer.json (/app/composer.json)
Checked CA file /etc/ssl/certs/ca-certificates.crt: valid
Executing command (/app): git branch -a --no-color --no-abbrev -v
Executing command (/app): git describe --exact-match --tags
Executing command (CWD): git --version
Executing command (/app): git log --pretty="%H" -n1 HEAD
Executing command (/app): hg branch
Executing command (/app): fossil branch list
Executing command (/app): fossil tag list
Executing command (/app): svn info --xml
Failed to initialize global composer: Composer could not find the config file: /root/.config/composer/composer.json

Reading /app/vendor/composer/installed.json
Loading plugin Dealerdirect\Composer\Plugin\Installers\PHPCodeSniffer\Plugin (from dealerdirect/phpcodesniffer-composer-installer)
Running 2.1.8 (2021-09-15 13:55:14) with PHP 7.4.3 on Linux / 4.19.0-17-amd64
> test-integration: phpunit --colors=always --testsuite "integration test"
Executing command (CWD): phpunit --colors=always --testsuite "integration test"
PHPUnit 9.5.9 by Sebastian Bergmann and contributors.


Integration test started.
............E.........SSS..SSSS                                   31 / 31 (100%)

Time: 00:06.887, Memory: 8.00 MB

There was 1 error:

1) LaminasIntegrationTest\Db\Adapter\Driver\Pdo\Mysql\QueryTest::testNamedParameters
Laminas\Db\Adapter\Exception\InvalidQueryException: Statement could not be executed (22007 - 1292 - Truncated incorrect DOUBLE value: 'bar')

/app/src/Adapter/Driver/Pdo/Statement.php:223
/app/test/integration/Adapter/Driver/Pdo/Mysql/QueryTest.php:87

Caused by
PDOException: SQLSTATE[22007]: Invalid datetime format: 1292 Truncated incorrect DOUBLE value: 'bar'

/app/src/Adapter/Driver/Pdo/Statement.php:212
/app/test/integration/Adapter/Driver/Pdo/Mysql/QueryTest.php:87

ERRORS!
Tests: 31, Assertions: 72, Errors: 1, Skipped: 7.
Script phpunit --colors=always --testsuite "integration test" handling the test-integration event returned with error code 2

* PHP 8.0 and 8.1 are affected

I will do it after make test docker for PHP 8.0

@Ocramius
Copy link
Member

To be clear, if we pass in 123 instead of 'bar' it works?

Just making sure it is just a mistake in QueryTest

@ZVanoZ
Copy link
Contributor Author

ZVanoZ commented Sep 23, 2021

To be clear, if we pass in 123 instead of 'bar' it works?

I did it and the error disappeared

PHP 7.4
"laminas-db" 2.13.x + local changes
Note: i changed parameters for two executes ("positional parameters" and "named parameters"). Because both give the error.

// test/integration/Adapter/Driver/Pdo/Mysql/QueryTest.php
 public function testNamedParameters()
    {
        $sql = new Sql($this->adapter);

        $insert = $sql->update('test');
        $insert->set([
            'name'  => ':name',
            'value' => ':value',
        ])->where(['id' => ':id']);
        $stmt = $sql->prepareStatementForSqlObject($insert);

        //positional parameters
        $stmt->execute([
            1,
            'foo',
            123 // prev value is: 'bar',
        ]);

        //"mapped" named parameters
        $stmt->execute([
            'c_0'    => 1,
            'c_1'    => 'foo',
            'where1' => 123//prev value is: 'bar',
        ]);
$ docker run --rm -it --volume $(pwd):/app zvanoz/laminas-db-test-by-docker:74 composer -vvv test-integration
Reading ./composer.json (/app/composer.json)
Loading config file ./composer.json (/app/composer.json)
Checked CA file /etc/ssl/certs/ca-certificates.crt: valid
Executing command (/app): git branch -a --no-color --no-abbrev -v
Executing command (/app): git describe --exact-match --tags
Executing command (CWD): git --version
Executing command (/app): git log --pretty="%H" -n1 HEAD
Executing command (/app): hg branch
Executing command (/app): fossil branch list
Executing command (/app): fossil tag list
Executing command (/app): svn info --xml
Failed to initialize global composer: Composer could not find the config file: /root/.config/composer/composer.json

Reading /app/vendor/composer/installed.json
Loading plugin Dealerdirect\Composer\Plugin\Installers\PHPCodeSniffer\Plugin (from dealerdirect/phpcodesniffer-composer-installer)
Running 2.1.8 (2021-09-15 13:55:14) with PHP 7.4.3 on Linux / 4.19.0-17-amd64
> test-integration: phpunit --colors=always --testsuite "integration test"
Executing command (CWD): phpunit --colors=always --testsuite "integration test"
PHPUnit 9.5.9 by Sebastian Bergmann and contributors.


Integration test started.
............R.........SSS..SSSS                                   31 / 31 (100%)

Time: 00:02.826, Memory: 8.00 MB

There was 1 risky test:

1) LaminasIntegrationTest\Db\Adapter\Driver\Pdo\Mysql\QueryTest::testNamedParameters
This test did not perform any assertions

/app/test/integration/Adapter/Driver/Pdo/Mysql/QueryTest.php:74

OK, but incomplete, skipped, or risky tests!
Tests: 31, Assertions: 87, Skipped: 7, Risky: 1.

@Ocramius
Copy link
Member

Ok, so it's just a faulty test then :-)

1) LaminasIntegrationTest\Db\Adapter\Driver\Pdo\Mysql\QueryTest::testNamedParameters
This test did not perform any assertions

I guess this test is totally foobar'd anyway :-\

@ZVanoZ
Copy link
Contributor Author

ZVanoZ commented Sep 23, 2021

I'm glad I helped figure it out.
Can you fix the test?
I'm new to unit testing and I'm afraid of messing up anything.

@Ocramius
Copy link
Member

Not really understanding what this test is supposed to verify: #160 and @trasher may know more 🤔

I think that replacing 'foo' with a string would be OK, but unsure about what the assertion could be 🤔

@ZVanoZ
Copy link
Contributor Author

ZVanoZ commented Sep 23, 2021

I think I figured out the reason for the error and how to fix it.

Let's say we have the following request

UPDATE `test` SET `name` = :c_0, `value` = :c_1 WHERE ` id` = :where1
-- binding order
-- Index     Bind Name     Field name   Field type
--   0       ":c_0"        "name"       varchar(255)   
--   1       ":c_1"        "value"      varchar(255)
--   2       ":where1"     "id"         int

We can call it in two ways.

  1. Binding substitution variables by index
    Someone need to swap the values ​​of the elements with index 0 and 2.
// positional parameters
 $ stmt-> execute ([
            1,      // Error --   0       ":c_0"        "name"       varchar(255)   
            'foo',  // Ok.   --   1       ":c_1"        "value"      varchar(255)
            'bar',  // Error --   2       ":where1"     "id"         int
        ]);
  1. Binding by substitution variable names
    Someone need to swap the values ​​of the elements with index "c_0" and "where1".
        //"mapped" named parameters
        $ stmt-> execute ([
            'c_0' => 1,            //Error. --   0       ":c_0"        "name"       varchar(255)   
            'c_1' => 'foo',        //Ok.    --   1       ":c_1"        "value"      varchar(255)
            'where1' => 'bar',     // Error --   2       ":where1"     "id"         int
        ]);

PS: 2021-09-24
It turns out that in "laminas-db" there is a third way to bind parameters - by real field names (see the test below).

ZVanoZ added a commit to ZVanoZ/laminas-db that referenced this issue Sep 24, 2021
ZVanoZ added a commit to ZVanoZ/laminas-db that referenced this issue Sep 24, 2021
…rameters"/change failure test to any exception (because github-ci messages different of local messages)
ZVanoZ added a commit to ZVanoZ/laminas-db that referenced this issue Sep 24, 2021
…rameters"/change code style by phpcs and "laminas/laminas-coding-standard"
ZVanoZ added a commit to ZVanoZ/laminas-db that referenced this issue Sep 24, 2021
…rameters"/delete commited "phpcs.xml" (i'm sorry)
ZVanoZ added a commit to ZVanoZ/laminas-db that referenced this issue Sep 27, 2021
…rameters"/fix test "testBindParamByFieldNameIsFail" - wrong name of a fieldName
ZVanoZ added a commit to ZVanoZ/laminas-db that referenced this issue Sep 27, 2021
…tNamedParameters"/fix test "testBindParamByFieldNameIsFail" - wrong name of a fieldName"

This reverts commit 808c64a
ZVanoZ added a commit to ZVanoZ/laminas-db that referenced this issue Sep 27, 2021
…rameters"/fix test "testBindParamByFieldNameIsFail" - wrong name of a fieldName
ZVanoZ added a commit to ZVanoZ/laminas-db that referenced this issue Sep 28, 2021
…rameters"/All in root of the project is ignore

Signed-off-by: ZVanoZ <edu.ZVanoZ@gmail.com>
ZVanoZ added a commit to ZVanoZ/laminas-db that referenced this issue Sep 29, 2021
…tNamedParameters"/fix test "testBindParamByFieldNameIsFail" - wrong name of a fieldName"

This reverts commit 808c64a
ZVanoZ added a commit to ZVanoZ/laminas-db that referenced this issue Sep 29, 2021
ZVanoZ added a commit to ZVanoZ/laminas-db that referenced this issue Sep 29, 2021
…rameters

Signed-off-by: ZVanoZ <edu.ZVanoZ@gmail.com>
Xerkus pushed a commit to ZVanoZ/laminas-db that referenced this issue Feb 1, 2024
…rameters

Signed-off-by: ZVanoZ <edu.ZVanoZ@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants