#2926 Fix backslashes double escaping

[ostrolucky]

Q	A
Type	bug
BC Break	yes
Fixed issues	#2926

Seems during #2442 these backslash strategies were added to several DB engines blindly, without verifying if they require such backslashes. What several people experience at least with Postgres then is that DBAL generates SQLs with duplicated backslashes and upon retrieving it from database can't match it with definition (where it's single backslashed).

I can't write backlash specific test case for every single DB operation, but I verified several operations manually (schema, foreign keys, closeActiveDatabaseConnectionsSQL...) and from that I see postgresql does not treat backslash as escape character anywhere.

[morozov]

Looks like a duplicate of #3330.

[ostrolucky]

Indeed 😢 What's odd though is that my testcase passes for oracle

[morozov]

What's odd though is that this testcase passes for oracle.

Is it because the test doesn't perform any assertions? It should check that the table was created exactly as specified, not just the absence of an exception.

[ostrolucky]

I don't see a good, platform agnostic way to do that, do you? SchemaManager removes doctrine type from comments, so it's empty.

[morozov]

Does it have to be tested this indirectly (custom type → column comment → escaping) if we're only modifying the way how escaping works?

I can't write backlash specific test case for every single DB operation

And you don't have to unless there's an escaping-specific case apart from just escaping tested in #3330.

[ostrolucky]

Concern of this testcase isn't really about internals of escaping, but about ensuring whatever DBAL writes can read too. If it screws up the write part, at least it should also screw up read part same way, so it matches.

And you don't have to unless there's an escaping-specific case apart from just escaping tested in #3330.

In theory yes, but it turns out indirect test passes for oracle, so there might be other things wrong. For the record - yes, your testcase should be enough, but we need to investigate what the heck is going on with oci8.

[ostrolucky]

It fails locally for me with oci8

There was 1 error:

1) Doctrine\Tests\DBAL\Functional\Schema\OracleSchemaManagerTest::testCanCreateAndRetrieveInfoAboutTypeWithBackslashes
Exception: [Doctrine\DBAL\DBALException] Unknown column type "Foo\\Bar" requested. Any Doctrine type that you use has to be registered with \Doctrine\DBAL\Types\Type::addType(). You can get a list of all the known types with \Doctrine\DBAL\Types\Type::getTypesMap(). If this error occurs during database introspection then you might have forgotten to register all database types for a Doctrine Type. Use AbstractPlatform#registerDoctrineTypeMapping() or have your custom types implement Type#getMappedDatabaseTypes(). If the type name is empty you might have a problem with the cache or forgot some mapping information.

With queries:
9. SQL: 'SELECT * FROM sys.user_tables' Params: 
8. SQL: 'DROP SCHEMA testschema' Params: 
7. SQL: 'DROP TABLE testschema.my_table_in_namespace' Params: 
6. SQL: 'DROP TRIGGER TESTSCHEMA.MY_TABLE_IN_NAMESPACE_AI_PK' Params: 
5. SQL: 'SELECT   c.*,
         (
             SELECT d.comments
             FROM   all_col_comments d
             WHERE  d.TABLE_NAME = c.TABLE_NAME AND d.OWNER = c.OWNER
             AND    d.COLUMN_NAME = c.COLUMN_NAME
         ) AS comments
FROM     all_tab_columns c
WHERE    c.table_name = 'TEST_ESCAPING' AND c.owner = 'SYSTEM'
ORDER BY c.column_id' Params: 
4. SQL: 'COMMENT ON COLUMN test_escaping."column" IS '(DC2Type:Foo\\Bar)'' Params: 
3. SQL: 'CREATE TABLE test_escaping ("column" VARCHAR2(255) NOT NULL)' Params: 
2. SQL: 'DROP TABLE test_escaping' Params: 
1. SQL: 'DROP TRIGGER TEST_ESCAPING_AI_PK' Params: 

Trace:
/project/lib/Doctrine/DBAL/Types/Type.php:165
/project/lib/Doctrine/DBAL/Schema/OracleSchemaManager.php:232
/project/lib/Doctrine/DBAL/Schema/AbstractSchemaManager.php:806
/project/lib/Doctrine/DBAL/Schema/AbstractSchemaManager.php:164
/project/lib/Doctrine/DBAL/Schema/AbstractSchemaManager.php:266
/project/tests/Doctrine/Tests/DBAL/Functional/Schema/SchemaManagerFunctionalTestCase.php:1592
/project/vendor/phpunit/phpunit/src/Framework/TestCase.php:1150
/project/vendor/phpunit/phpunit/src/Framework/TestCase.php:844
/project/vendor/phpunit/phpunit/src/Framework/TestResult.php:675
/project/vendor/phpunit/phpunit/src/Framework/TestCase.php:798
/project/vendor/phpunit/phpunit/src/Framework/TestSuite.php:750
/project/vendor/phpunit/phpunit/src/Framework/TestSuite.php:750
/project/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:613
/project/vendor/phpunit/phpunit/src/TextUI/Command.php:206
/project/vendor/phpunit/phpunit/src/TextUI/Command.php:162
/project/vendor/phpunit/phpunit/phpunit:61

So, now why does it fail as it should locally, but not on CI 🤔

[ostrolucky]

So this is because of shared connection between tests. Comments are init only once 🙄