crate / crate-dbal

Doctrine Database Access Layer for Crate.IO
Apache License 2.0
16 stars 10 forks source link

Fix compatibility to dbal 2.13 by defining the statement class #113

Closed alexander-schranz closed 3 years ago

alexander-schranz commented 3 years ago

Summary of the changes / Why this is an improvement

Fixes #112

Checklist

alexander-schranz commented 3 years ago

Looks like the CI is failing with:

Crate\PDO\Exception\PDOException: Unsupported driver attribute

Open for any input here what the best would be to create the correct statement here.

amotl commented 3 years ago

Hi Alexander,

thank you for this patch. Currently, CI is still installing doctrine/dbal (2.10.4).

Can I ask you to accompany your patch by a respective bump to the Doctrine DBAL version this improvement was intended for? Then, CI will pick it up and run the corresponding tests on it [1].

With kind regards, Andreas.

[1] Please note that composer update --lock needs to be invoked with PHP 7.2, as this currently is the minimum supported version before moving on to PHP 8.0, which will implicitly drop support for PHP 7.2 again. Please let me know if your system/sandbox is not able to do that, so I will be able to chime in.

alexander-schranz commented 3 years ago

@amotl did update the lock file.

alexander-schranz commented 3 years ago

@amotl looks like the statementClass attribute was never implemented I did update the code that it is now used. https://github.com/crate/crate-pdo/pull/123

Locally with the PR I still get 3 failing tests but not sure if they only fail for me:

Errors: ``` PHPUnit 8.5.15 by Sebastian Bergmann and contributors. Runtime: PHP 7.4.16 with Xdebug 2.9.8 Configuration: /private/tmp/crate-dbal/phpunit.xml.dist ............F..........F.F.S.S.S.SS............................ 63 / 217 ( 29%) ..............SSSSSSSSSSSSSSSS................................. 126 / 217 ( 58%) .................................SS........SSS.........S....... 189 / 217 ( 87%) .S....S.....SSS...........FF 217 / 217 (100%) Time: 6.68 seconds, Memory: 12.00 MB There were 5 failures: 1) Crate\Test\DBAL\Functional\DataAccessTestCase::testPrepareWithFetchAll Failed asserting that two arrays are equal. --- Expected +++ Actual @@ @@ Array ( 'test_int' => 1 'test_string' => 'foo' - 'test_datetime' => 1262340610000 + 'test_datetime' => 1262337010000 'test_array' => Array (...) 'test_object' => Array (...) ) /private/tmp/crate-dbal/test/Crate/Test/DBAL/Functional/DataAccessTest.php:139 2) Crate\Test\DBAL\Functional\DataAccessTestCase::testExecuteQueryBindDateTimeType Failed asserting that 0 matches expected 1. /private/tmp/crate-dbal/test/Crate/Test/DBAL/Functional/DataAccessTest.php:313 3) Crate\Test\DBAL\Functional\DataAccessTestCase::testPrepareQueryBindValueDateTimeType Failed asserting that 0 matches expected 1. /private/tmp/crate-dbal/test/Crate/Test/DBAL/Functional/DataAccessTest.php:348 4) Crate\Test\DBAL\Platforms\CratePlatformTest::testAsciiSQLDeclaration with data set #0 ('VARCHAR(12)', array(12)) Failed asserting that two strings are equal. --- Expected +++ Actual @@ @@ -'VARCHAR(12)' +'TEXT' /private/tmp/crate-dbal/vendor/doctrine/dbal/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php:1472 5) Crate\Test\DBAL\Platforms\CratePlatformTest::testAsciiSQLDeclaration with data set #1 ('CHAR(12)', array(12, true)) Failed asserting that two strings are equal. --- Expected +++ Actual @@ @@ -'CHAR(12)' +'TEXT' /private/tmp/crate-dbal/vendor/doctrine/dbal/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php:1472 -- There were 32 skipped tests: 1) Crate\Test\DBAL\Functional\DataAccessTestCase::testDateArithmetics Data add day expression not supported by crate platform /private/tmp/crate-dbal/test/Crate/Test/DBAL/Functional/DataAccessTest.php:381 2) Crate\Test\DBAL\Functional\DataAccessTestCase::testBitComparisonExpressionSupport Bit comparison expression not supported by crate /private/tmp/crate-dbal/test/Crate/Test/DBAL/Functional/DataAccessTest.php:416 3) Crate\Test\DBAL\Functional\DataAccessTestCase::testFetchAllSupportFetchClass PDO::FETCH_CLASS not supported by crate PDO /private/tmp/crate-dbal/test/Crate/Test/DBAL/Functional/DataAccessTest.php:479 4) Crate\Test\DBAL\Functional\DataAccessTestCase::testSetFetchModeClassFetchAll PDO::FETCH_CLASS not supported crate PDO /private/tmp/crate-dbal/test/Crate/Test/DBAL/Functional/DataAccessTest.php:523 5) Crate\Test\DBAL\Functional\DataAccessTestCase::testSetFetchModeClassFetch PDO::FETCH_CLASS not supported by crate PDO /private/tmp/crate-dbal/test/Crate/Test/DBAL/Functional/DataAccessTest.php:545 6) Crate\Test\DBAL\Platforms\CratePlatformTest::testGeneratesForeignKeyCreationSql Platform does not support FOREIGN KEY constraints. /private/tmp/crate-dbal/test/Crate/Test/DBAL/Platforms/CratePlatformTest.php:79 7) Crate\Test\DBAL\Platforms\CratePlatformTest::testGeneratesDecimalTypeDeclarationSQL with data set #0 (array(), 'NUMERIC(10, 0)') Platform does not support any decleration of datatype DECIMAL. /private/tmp/crate-dbal/test/Crate/Test/DBAL/Platforms/CratePlatformTest.php:95 8) Crate\Test\DBAL\Platforms\CratePlatformTest::testGeneratesDecimalTypeDeclarationSQL with data set #1 (array(true), 'NUMERIC(10, 0)') Platform does not support any decleration of datatype DECIMAL. /private/tmp/crate-dbal/test/Crate/Test/DBAL/Platforms/CratePlatformTest.php:95 9) Crate\Test\DBAL\Platforms\CratePlatformTest::testGeneratesDecimalTypeDeclarationSQL with data set #2 (array(false), 'NUMERIC(10, 0)') Platform does not support any decleration of datatype DECIMAL. /private/tmp/crate-dbal/test/Crate/Test/DBAL/Platforms/CratePlatformTest.php:95 10) Crate\Test\DBAL\Platforms\CratePlatformTest::testGeneratesDecimalTypeDeclarationSQL with data set #3 (array(5), 'NUMERIC(5, 0)') Platform does not support any decleration of datatype DECIMAL. /private/tmp/crate-dbal/test/Crate/Test/DBAL/Platforms/CratePlatformTest.php:95 11) Crate\Test\DBAL\Platforms\CratePlatformTest::testGeneratesDecimalTypeDeclarationSQL with data set #4 (array(5), 'NUMERIC(10, 5)') Platform does not support any decleration of datatype DECIMAL. /private/tmp/crate-dbal/test/Crate/Test/DBAL/Platforms/CratePlatformTest.php:95 12) Crate\Test\DBAL\Platforms\CratePlatformTest::testGeneratesDecimalTypeDeclarationSQL with data set #5 (array(8, 2), 'NUMERIC(8, 2)') Platform does not support any decleration of datatype DECIMAL. /private/tmp/crate-dbal/test/Crate/Test/DBAL/Platforms/CratePlatformTest.php:95 13) Crate\Test\DBAL\Platforms\CratePlatformTest::testAlterTableChangeQuotedColumn Platform does not support ALTER TABLE. /private/tmp/crate-dbal/test/Crate/Test/DBAL/Platforms/CratePlatformTest.php:107 14) Crate\Test\DBAL\Platforms\CratePlatformTest::testQuotedColumnInForeignKeyPropagation Platform does not support ADD FOREIGN KEY. /private/tmp/crate-dbal/test/Crate/Test/DBAL/Platforms/CratePlatformTest.php:138 15) Crate\Test\DBAL\Platforms\CratePlatformTest::testQuotesAlterTableRenameColumn Platform does not support ALTER TABLE. /private/tmp/crate-dbal/test/Crate/Test/DBAL/Platforms/CratePlatformTest.php:158 16) Crate\Test\DBAL\Platforms\CratePlatformTest::testQuotesAlterTableChangeColumnLength Platform does not support ALTER TABLE. /private/tmp/crate-dbal/test/Crate/Test/DBAL/Platforms/CratePlatformTest.php:168 17) Crate\Test\DBAL\Platforms\CratePlatformTest::testQuotesAlterTableRenameIndexInSchema Platform does not support ALTER TABLE. /private/tmp/crate-dbal/test/Crate/Test/DBAL/Platforms/CratePlatformTest.php:178 18) Crate\Test\DBAL\Platforms\CratePlatformTest::testGeneratesAlterTableRenameColumnSQL Platform does not support ALTER TABLE. /private/tmp/crate-dbal/test/Crate/Test/DBAL/Platforms/CratePlatformTest.php:195 19) Crate\Test\DBAL\Platforms\CratePlatformTest::testQuotesTableIdentifiersInAlterTableSQL Platform does not support ALTER TABLE. /private/tmp/crate-dbal/test/Crate/Test/DBAL/Platforms/CratePlatformTest.php:205 20) Crate\Test\DBAL\Platforms\CratePlatformTest::testGeneratesAlterTableRenameIndexUsedByForeignKeySQL Platform does not support ALTER TABLE. /private/tmp/crate-dbal/test/Crate/Test/DBAL/Platforms/CratePlatformTest.php:215 21) Crate\Test\DBAL\Platforms\CratePlatformTest::testAlterStringToFixedString Platform does not support ALTER TABLE. /private/tmp/crate-dbal/test/Crate/Test/DBAL/Platforms/CratePlatformTest.php:225 22) Crate\Test\DBAL\Platforms\CratePlatformTest::testGeneratesIndexCreationSql Platform does not support CREATE INDEX. /private/tmp/crate-dbal/test/Crate/Test/DBAL/Platforms/CratePlatformTest.php:69 /private/tmp/crate-dbal/vendor/doctrine/dbal/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php:199 23) Crate\Test\DBAL\Platforms\CratePlatformTest::testGeneratesUniqueIndexCreationSql Platform does not support CREATE UNIQUE INDEX. /private/tmp/crate-dbal/test/Crate/Test/DBAL/Platforms/CratePlatformTest.php:74 /private/tmp/crate-dbal/vendor/doctrine/dbal/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php:211 24) Crate\Test\DBAL\Platforms\CratePlatformTest::testCreateTableColumnComments Platform does not support Column comments. /private/tmp/crate-dbal/vendor/doctrine/dbal/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php:517 /private/tmp/crate-dbal/vendor/doctrine/dbal/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php:474 25) Crate\Test\DBAL\Platforms\CratePlatformTest::testAlterTableColumnComments Platform does not support Column comments. /private/tmp/crate-dbal/vendor/doctrine/dbal/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php:525 /private/tmp/crate-dbal/vendor/doctrine/dbal/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php:499 26) Crate\Test\DBAL\Platforms\CratePlatformTest::testCreateTableColumnTypeComments Platform does not support Column comments. /private/tmp/crate-dbal/vendor/doctrine/dbal/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php:533 /private/tmp/crate-dbal/vendor/doctrine/dbal/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php:509 27) Crate\Test\DBAL\Platforms\CratePlatformTest::testQuotesReservedKeywordInTruncateTableSQL Platform does not support TRUNCATE TABLE. /private/tmp/crate-dbal/test/Crate/Test/DBAL/Platforms/CratePlatformTest.php:496 /private/tmp/crate-dbal/vendor/doctrine/dbal/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php:730 28) Crate\Test\DBAL\Platforms\CratePlatformTest::testReturnsBinaryTypeLongerThanMaxDeclarationSQL Not applicable to the platform /private/tmp/crate-dbal/vendor/doctrine/dbal/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php:835 29) Crate\Test\DBAL\Platforms\CratePlatformTest::testQuotesDropForeignKeySQL Crate\DBAL\Platforms\CratePlatform4 does not support foreign key constraints. /private/tmp/crate-dbal/vendor/doctrine/dbal/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php:1074 30) Crate\Test\DBAL\Platforms\CratePlatformTest::testGeneratesInlineColumnCommentSQL with data set "regular comment" ('Regular comment', 'COMMENT 'Regular comment'') Crate\DBAL\Platforms\CratePlatform4 does not support inline column comments. /private/tmp/crate-dbal/vendor/doctrine/dbal/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php:1173 31) Crate\Test\DBAL\Platforms\CratePlatformTest::testGeneratesInlineColumnCommentSQL with data set "comment requiring escaping" ('Using inline comment delimiter ' works', 'COMMENT 'Using inline comment...works'') Crate\DBAL\Platforms\CratePlatform4 does not support inline column comments. /private/tmp/crate-dbal/vendor/doctrine/dbal/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php:1173 32) Crate\Test\DBAL\Platforms\CratePlatformTest::testGeneratesInlineColumnCommentSQL with data set "empty comment" ('', 'COMMENT ''') Crate\DBAL\Platforms\CratePlatform4 does not support inline column comments. /private/tmp/crate-dbal/vendor/doctrine/dbal/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php:1173 ```
amotl commented 3 years ago

Looks like the statementClass attribute was never implemented I did update the code that it is now used: https://github.com/crate/crate-pdo/pull/123.

Thank you. I turned that into another release, see https://github.com/crate/crate-pdo/pull/124.

amotl commented 3 years ago

Locally with the PR I still get 3 failing tests but not sure if they only fail for me:

Down to two, see [1].

There were 2 failures:

1) Crate\Test\DBAL\Platforms\CratePlatformTest::testAsciiSQLDeclaration with data set #0 ('VARCHAR(12)', array(12))
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'VARCHAR(12)'
+'TEXT'

/home/runner/work/crate-dbal/crate-dbal/vendor/doctrine/dbal/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php:1472

2) Crate\Test\DBAL\Platforms\CratePlatformTest::testAsciiSQLDeclaration with data set #1 ('CHAR(12)', array(12, true))
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'CHAR(12)'
+'TEXT'

/home/runner/work/crate-dbal/crate-dbal/vendor/doctrine/dbal/tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php:1472

[1] https://github.com/crate/crate-dbal/runs/2453539267#step:7:17

alexander-schranz commented 3 years ago

@amotl Now only 2 tests are failing:

I did have a look at previous runs and seems like this are new tests (first line this run, second line old CI run on github):

.S....S.....SSS...........FF
.S....S.....SSS...........

I will adjust the test case as I think there is in crate no difference between VARCHAR(12) or TEXT type.

amotl commented 3 years ago

I can confirm all tests work fine on my machine with your most recent b11696a.

alexander-schranz commented 3 years ago

@amotl looks good now from my side :)

alexander-schranz commented 3 years ago

@amotl Thx for your support! The PHP 8 PR #111 is now also rebased.

amotl commented 3 years ago

Thank you!