[DBAL-335] Is MasterSlaveConnection implemented correctly - seems to overwrite master connection on transaction methods? Created: 31/Aug/12  Updated: 16/Oct/14  Resolved: 17/Sep/12

Status: Resolved
Project: Doctrine DBAL
Component/s: None
Affects Version/s: 2.3
Fix Version/s: 2.3

Type: Bug Priority: Major
Reporter: Jonathan Ingram Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 0
Labels: None

Issue Links:
Dependency
is required for DBAL-1006 [GH-690] Backport [DBAL-717] Fix bug ... Resolved

 Description   

Forgive me to doubt, but I think there may be a bug in MasterSlaveConnection.

It's easier to understand what I'm saying by debugging and tracing the flow, but I'll illustrate it with gists.

First, here is a simple service method to create a user. It opens up a transaction, persists the user, commits and returns. On error, if there is an active transaction, rollback. Here is the gist:

https://gist.github.com/3547674

The "$conn->beginTransaction();" line is where we trace through (the remainder of the service method is now irrelevant). Looking into MasterSlaveConnection.php, we see the method tries to connect to the master connection (call this point ###):

https://gist.github.com/3547720

Now looking in the next gist, we see what happens when "$this->connect('master');" is called. At this point it's not that interesting, the internal "$this->_conn" property is set to "master".

https://gist.github.com/3547750

Now here lies the bug I believe. "parent::beginTransaction();" is called. When looking into this method, we see that another call is made to connect but this time without "master" as the argument (i.e. connect to slave). This call to connect is made before incrementing the transaction nesting level.

https://gist.github.com/3547808

Now, I won't do another gist for "MasterSlaveConnection::connect", but if you refer to the file at line 13 https://gist.github.com/3547750#file_master_slave_connection.php, you will see that it checks the transaction nesting level and if it is there, forces master. However, we don't increment the level until after the method returns, so the slave is used. Ultimately, this results in the internal "$this->_conn" property set to the "slave" connection which violates our original action at ### above where we said we want to connect to "master".

Am I missing something here? Here is a gist the is a basic attempt at fixing this one method. It simply copies the code from the parent method except does not connect twice. I believe the same would have to occur for all the other methods unless it can be fixed once at the "MasterSlaveConnection::connect" level.

https://gist.github.com/3547880

I've just fleshed out "beginTransaction", "commit" and "rollBack" in "MasterSlaveConnection" by basically copying and pasting the code from the parent class and for my failing use case, this fixes the issue. However, it did require updating "Connection" slightly so that I had access to some private variables.



 Comments   
Comment by Lars Strojny [ 31/Aug/12 ]

This looks indeed like a bug. From a first glimpse the fix would be to use master, if master is already connected.

Comment by Benjamin Eberlei [ 05/Sep/12 ]

This only happens when "keepSlave" = true, because then the master is not written into the slave property aswell:

   } else {
     $this->connections['slave'] = $this->_conn = $this->connectTo($connectionName);
   } 

Are you using keepSlave = true?

Comment by Jonathan Ingram [ 05/Sep/12 ]

Yes I am. Does that render this moot or still a bug?

Comment by Benjamin Eberlei [ 06/Sep/12 ]

Its still a bug, but it helps to know why this happens.

Comment by Benjamin Eberlei [ 17/Sep/12 ]

Fixed in master and 2.3, can you test it?

Comment by Jonathan Ingram [ 19/Sep/12 ]

Thanks for doing this. I will test it shortly.

Comment by Ivan Andric [ 22/Sep/12 ]

Hi,

not sure if you managed to test this but now test on mysql database fails with results below.
Probably some typo.

There was 1 error:

1) Doctrine\Tests\DBAL\Functional\MasterSlaveConnectionTest::testKeepSlaveBeginTransactionStaysOnMaster
Exception: [Doctrine\DBAL\DBALException] An exception occurred while executing 'INSERT INTO master_slave_table (test_int) VALUES ' with params

{"1":30}

:

SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '30' for key 'PRIMARY'

With queries:
2. SQL: 'CREATE TABLE master_slave_table (test_int INT NOT NULL, PRIMARY KEY(test_int)) DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci ENGINE = InnoDB' Params:

Trace:
/home/ivan/git/dbal2/dbal/lib/Doctrine/DBAL/Connection.php:793
/home/ivan/git/dbal2/dbal/lib/Doctrine/DBAL/Connections/MasterSlaveConnection.php:231
/home/ivan/git/dbal2/dbal/lib/Doctrine/DBAL/Connection.php:539
/home/ivan/git/dbal2/dbal/lib/Doctrine/DBAL/Connections/MasterSlaveConnection.php:285
/home/ivan/git/dbal2/dbal/tests/Doctrine/Tests/DBAL/Functional/MasterSlaveConnectionTest.php:92

/home/ivan/git/dbal2/dbal/tests/Doctrine/Tests/DbalFunctionalTestCase.php:73
/home/ivan/git/dbal2/dbal/lib/Doctrine/DBAL/Connection.php:793
/home/ivan/git/dbal2/dbal/lib/Doctrine/DBAL/Connections/MasterSlaveConnection.php:231
/home/ivan/git/dbal2/dbal/lib/Doctrine/DBAL/Connection.php:539
/home/ivan/git/dbal2/dbal/lib/Doctrine/DBAL/Connections/MasterSlaveConnection.php:285
/home/ivan/git/dbal2/dbal/tests/Doctrine/Tests/DBAL/Functional/MasterSlaveConnectionTest.php:92

Caused by
Doctrine\DBAL\DBALException: An exception occurred while executing 'INSERT INTO master_slave_table (test_int) VALUES ' with params

{"1":30}

:

SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '30' for key 'PRIMARY'

/home/ivan/git/dbal2/dbal/lib/Doctrine/DBAL/DBALException.php:47
/home/ivan/git/dbal2/dbal/lib/Doctrine/DBAL/Connection.php:786
/home/ivan/git/dbal2/dbal/lib/Doctrine/DBAL/Connections/MasterSlaveConnection.php:231
/home/ivan/git/dbal2/dbal/lib/Doctrine/DBAL/Connection.php:539
/home/ivan/git/dbal2/dbal/lib/Doctrine/DBAL/Connections/MasterSlaveConnection.php:285
/home/ivan/git/dbal2/dbal/tests/Doctrine/Tests/DBAL/Functional/MasterSlaveConnectionTest.php:92

Caused by
PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '30' for key 'PRIMARY'

/home/ivan/git/dbal2/dbal/lib/Doctrine/DBAL/Connection.php:786
/home/ivan/git/dbal2/dbal/lib/Doctrine/DBAL/Connections/MasterSlaveConnection.php:231
/home/ivan/git/dbal2/dbal/lib/Doctrine/DBAL/Connection.php:539
/home/ivan/git/dbal2/dbal/lib/Doctrine/DBAL/Connections/MasterSlaveConnection.php:285
/home/ivan/git/dbal2/dbal/tests/Doctrine/Tests/DBAL/Functional/MasterSlaveConnectionTest.php:92





[DBAL-278] add support for lastInsertId method on OCI8 Driver Created: 16/May/12  Updated: 22/May/12  Resolved: 22/May/12

Status: Resolved
Project: Doctrine DBAL
Component/s: Drivers
Affects Version/s: 2.2.2
Fix Version/s: 2.3

Type: Improvement Priority: Major
Reporter: Franek Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 0
Labels: None
Environment:

oci8



 Description   

The method lastInsertId() is not defined for OCI8 Driver in Doctrine\DBAL\Driver\OCI8\OCI8Connection.php :

OCI8Connection.php
public function lastInsertId($name = null)
{
    //TODO: throw exception or support sequences?
}

I propose this method to handle lastInsertId for sequence :

OCI8Connection.php
public function lastInsertId($name = null)
 {
        // For sequence
        if (is_string($name)) {
            // We can check eventually check the presence of the sequence in the table
            // USER_SEQUENCES
            $sql = 'SELECT ' . $name . '.CURRVAL FROM DUAL';
            // will throw an exception if this sequence does not exist
            $stmt = $this->query($sql);
            $result = $stmt->fetch(\PDO::FETCH_ASSOC);
            if ($result !== false && isset($result['CURRVAL'])) {
                return (int) $result['CURRVAL'];
            }
    }
    // OCI8 driver does not provide support of lastInsertId
    return null;
}

Thanks,






[DBAL-214] Unable to use PDO::FETCH_CLASS with a call to fetch() Created: 30/Jan/12  Updated: 17/Apr/14  Resolved: 05/May/12

Status: Resolved
Project: Doctrine DBAL
Component/s: Drivers
Affects Version/s: 2.0.0-BETA2, 2.0.0-BETA3, 2.0.0-BETA4, 2.0.0-RC1-RC3, 2.0-RC4, 2.0-RC5, 2.0, 2.0.1, 2.0.2, 2.0.3, 2.0.4, 2.0.5, 2.0.6, 2.0.7, 2.0.8, 2.1, 2.1.1, 2.1.2, 2.1.3, 2.1.5, 2.1.6, 2.2-BETA1, 2.2-BETA2, 2.2-RC1/RC2, 2.2.0-RC3, 2.2, 2.2.1, 2.2.2, 2.3, 2.5
Fix Version/s: 2.3

Type: Improvement Priority: Minor
Reporter: Andy Leon Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 1
Labels: None


 Description   

EDITED: 2nd attempt to describe this - first one was confusing.

I don't understand why the setFetchMode() method of Doctrine\DBAL\Driver\PDOStatement drops any arguments passed to it. It means that PDO::FETCH_CLASS cannot be used with calls to fetch() and no warning is given until the point when the underlying \PDOStatement complains that no class has been specified.



 Comments   
Comment by Antoine Froger [ 03/Feb/12 ]

In Doctrine/DBAL/Statement.php the 2nd and 3rd arguments of setFetchMode are dropped too.

Error message example when PDO::FETCH_CLASS is used as the first argument of setFetchMode:
$stmt->setFetchMode(PDO::FETCH_CLASS | PDO::FETCH_PROPS_LATE, 'ClassName');
display the error: PDOException: SQLSTATE[HY000]: General error: fetch mode requires the classname argument

Comment by Fabien Potencier [ 05/May/12 ]

This regression was introduced here: https://github.com/doctrine/dbal/commit/f4acc79a3e91059a932d7a2d43309f6f8f65fa59

It breaks some of my websites when upgrading DBAL. So, this is not an improvement but a regression bug.

Comment by Benjamin Eberlei [ 05/May/12 ]

Yes, i have to change this again.

The problem is its complex to support the 2nd/3rd arguments in the statement caching layer, i will just throw an exception for now and add an improvement ticket.

Comment by Benjamin Eberlei [ 05/May/12 ]

Fixed

Comment by Benjamin Eberlei [ 05/May/12 ]

https://github.com/doctrine/dbal/commit/d3930dcdb89cc818798c8f13e4126f76cf82ef8b





[DBAL-178] Unknown column type requested Created: 02/Nov/11  Updated: 05/Aug/13  Resolved: 26/Jun/12

Status: Resolved
Project: Doctrine DBAL
Component/s: None
Affects Version/s: None
Fix Version/s: 2.3

Type: Improvement Priority: Minor
Reporter: Francois Mazerolle Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 1
Labels: None
Environment:

OSX ( Using Symfony2 )



 Description   

When I run doctrine:schema:create, doctrine throw the following exception:
[Doctrine\DBAL\DBALException]
Unknown column type requested.

( Note: their is 2 spaces between type and requested )

The problem with this error message is how much it's unspecific. Event with -v, I still have no clue about that type is wrong, and what file is concerned.
So basically, I have to manually look at all my mapping files, one by one.

Also note that doctrine:mapping:info return all OK.



 Comments   
Comment by Denny Swindle [ 21/Nov/11 ]

Ironically, this same issue has recently started happening for me as well. It just started randomly over the weekend. Same exact issue (with 2 spaces between type and requested). For me, it happens when using Doctrine\ORM\EntityManager->find() on a valid entity.

#0 /doctrine-2.1.2/Doctrine/DBAL/DBALException.php(81): Doctrine\DBAL\DBALException::unknownColumnType()
#1 /doctrine-2.1.2/Doctrine/DBAL/Types/Type.php(140): Doctrine\DBAL\DBALException::unknownColumnType()
#2 /doctrine-2.1.2/Doctrine/ORM/Internal/Hydration/SimpleObjectHydrator.php(84): Doctrine\DBAL\Types\Type::getType()
#3 /doctrine-2.1.2/Doctrine/ORM/Internal/Hydration/SimpleObjectHydrator.php(43): Doctrine\ORM\Internal\Hydration\SimpleObjectHydrator->_hydrateRow()
#4 /doctrine-2.1.2/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php(99): Doctrine\ORM\Internal\Hydration\SimpleObjectHydrator->_hydrateAll()
#5 /doctrine-2.1.2/Doctrine/ORM/Persisters/BasicEntityPersister.php(581): Doctrine\ORM\Internal\Hydration\AbstractHydrator->hydrateAll()
#6 /doctrine-2.1.2/Doctrine/ORM/EntityRepository.php(130): Doctrine\ORM\Persisters\BasicEntityPersister->load()
#7 /doctrine-2.1.2/Doctrine/ORM/EntityManager.php(350): Doctrine\ORM\EntityRepository->find()

Comment by Gediminas Morkevicius [ 21/Jan/12 ]

hi, seems like doctrine does not clean cache fully.

in my case I'm using APC cache
run: in php *apc_clear_cache('user');*

it should work fine now

PS.:
running cache clearing commands, does not help.

doctrine:cache:clear-metadata Clears all metadata cache for a entity manager
doctrine:cache:clear-query Clears all query cache for a entity manager
doctrine:cache:clear-result Clears result cache for a entity manager

Comment by Francois Mazerolle [ 23/Jan/12 ]

Hi,

@gediminas : I don't have the error anymore so I can't test, but it's nice if there's a solution to fix it.

However, this error is problematic as it's anti-verbose.
There's is no indication of a cache problem in the error message. It try to point out to a field, and that field is not even displayed in the error.
I hope someone will be able to fix the error so that's it become a useful error, like by following some sort of guideline:

http://www.useit.com/alertbox/20010624.html

I don't want to troll or anything, but as I'Ve started using doctrine2 with symfony2, the MAJOR problem I've encountered what bad error message that was leaving me lost. Often I had -v, find the file where the error occurred, look at the code, understand by myself what's wrong and attempt to fix it. This is counter-productive and really show that the exception message doesn't follow most error message guide line. For sure, we're not end-users, but we're "programmer-user" that use the system, and this "title" come with a knowledge that is not the same as we could expect of a core-developer, for example.

I hope this message was constructive.

Comment by Benjamin Eberlei [ 26/Jun/12 ]

Improved this error message alot and also changed some regarding reverse engineering of custom types (where this error often occured):

You can now implement Type#getMappedDatabaseTypes(AbstractPlatform $platform); on your custom type and return a list of database types that this doctrine type maps to.

Comment by Almog Baku [ 05/Aug/13 ]

this bug is still happening to me.. but its works as excepted within `schema:create` and throw error on `schema:update` or `schema:drop`

I using doctrine with symfony





[DBAL-161] Character Set of Database not UTF-8 Created: 07/Sep/11  Updated: 20/Nov/12  Resolved: 05/May/12

Status: Resolved
Project: Doctrine DBAL
Component/s: None
Affects Version/s: None
Fix Version/s: 2.3

Type: Bug Priority: Major
Reporter: Hari K T Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 0
Labels: None
Environment:

Ubuntu



 Description   

Hi Guys,

I was working with symfony command line and created the database with the app/console doctrine:create:database

Though the characterset I specified was UTF-8 , it created Latin character set .

@elliot was right too, the create database is not using any character set https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Platforms/MySqlPlatform.php#L328

You can see the issue created at https://github.com/symfony/symfony/issues/2044 and later Fabien confirmed the bug is related to Doctrine .

I am not sure its a Bug or a feature I am asking .

Thanks



 Comments   
Comment by Benjamin Eberlei [ 05/May/12 ]

Yes these are not connected, however we changed the default collation to UTF-8 for DBAL 2.3

Comment by Ivan Borzenkov [ 20/Nov/12 ]

not fixed or broken now
doctrine:create:database still create database whis latin1 charset

https://github.com/doctrine/DoctrineBundle/issues/49





[DBAL-153] Github-PR-48 by phekmat: Added a regression test case for recently fixed PostgreSQLSchemaManager bug Created: 21/Aug/11  Updated: 20/Nov/13  Resolved: 20/Nov/13

Status: Resolved
Project: Doctrine DBAL
Component/s: None
Affects Version/s: None
Fix Version/s: 2.3

Type: Improvement Priority: Major
Reporter: Benjamin Eberlei Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 0
Labels: None


 Description   

This issue is created automatically through a Github pull request on behalf of

{username}

:

Url: https://github.com/doctrine/dbal/pull/48

Message:

Regression test for the following change: https://github.com/doctrine/dbal/commit/2434d95aab231273eea8fb555155e9e9c195bcc9



 Comments   
Comment by Benjamin Eberlei [ 24/Mar/12 ]

A related Github Pull-Request [GH-48] was
https://github.com/doctrine/dbal/pull/48

Comment by Benjamin Eberlei [ 24/Mar/12 ]

A related Github Pull-Request [GH-48] was
https://github.com/doctrine/dbal/pull/48

Comment by Benjamin Eberlei [ 24/Mar/12 ]

A related Github Pull-Request [GH-48] was
https://github.com/doctrine/dbal/pull/48





[DBAL-100] Add Drizzle Support Created: 16/Mar/11  Updated: 21/Nov/13  Resolved: 21/Nov/13

Status: Resolved
Project: Doctrine DBAL
Component/s: None
Affects Version/s: None
Fix Version/s: 2.3

Type: New Feature Priority: Major
Reporter: Benjamin Eberlei Assignee: Benjamin Eberlei
Resolution: Fixed Votes: 1
Labels: None


 Description   

Drizzle is out, we should add support for the Dialect.

http://docs.drizzle.org/mysql_differences.html



 Comments   
Comment by Andreas Streichardt [ 22/Dec/11 ]

i have created some hackish fork and the whole testsuite is working already:

https://github.com/m0ppers/dbal

Still WIP but may be a start. I think the C extension is not really ready yet either. When i find time i will most likely have a look at it.

Comment by Benjamin Eberlei [ 22/Dec/11 ]

Can you branch it into something, like git checkout -bDrizzle then push it to your repo and open a Pull Request? Thats way easier to review and discuss.

Comment by Steve Müller [ 24/Jun/13 ]

Isn't this already implemented?

Comment by Kim Hemsø [ 02/Aug/13 ]

Yes it is, long time ago. Well.. difference is that Andreas here is using the now (dead?) native drizzle ext. Where dbal is using pdo for mysql.





Generated at Fri Sep 04 10:31:40 EDT 2015 using JIRA 6.4.10#64025-sha1:5b8b74079161cd76a20ab66dda52747ee6701bd6.