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

Fix IBM DB2 implementation / ibm_db2 driver #447

Merged
merged 22 commits into from
Dec 20, 2013
Merged

Conversation

deeky666
Copy link
Member

This PR is a first approach towards officially supporting IBM DB2 platform in DBAL. The current implementation of the platform and schema manager are very outdated, buggy and incomplete. There is no platform test case yet and running the general DBAL test suite shows a lot of errors.
The focus of this PR is to bring back the DB2 platform and schema manager into sync mit the abstract classes so that at least the test suite runs. Furthermore this PR adds a platform test case. Along with achieving this, a lot of bugs needed to be fixed. The following changes have been made:

  • Add support for \PDO::FETCH_CLASS and \PDO::FETCH_OBJ fetch modes in ibm_db2 driver
  • Optimize table indexes introspection (cleaner list SQL statement and codebase in schema manager)
  • Fix foreign key constraints introspection (also cleaned up codebase in schema manager)
  • Introduce BLOB type column support
  • Fix TRUNCATE TABLE statement generation
  • Fix SQL snippets for current date/time/timestamp constants
  • Add date arithmetic expressions
  • Fix bit operator expressions
  • Add support for portability connection
  • Fix default value declarations
  • Fix renaming columns
  • Fix renaming tables
  • Fix autoincrement column introspection
  • Fix required table reorganization after table alteration
  • Fix DROP DATABASE statement generation
  • Remove unnecessary method overrides in the platform
  • Add platform test case

The complete test suite now runs fine on ibm_db2 driver except for TypeConversionTest::testIdempotentDataConversion:

There was 1 failure:

1) Doctrine\Tests\DBAL\Functional\TypeConversionTest::testIdempotentDataConversion with data set #13 ('decimal', 1.55, 'string')
Conversion between values should produce the same out as in value, but doesnt!
Failed asserting that '1,55' matches expected 1.55.

/home/deeky/dev/doctrine/dbal/tests/Doctrine/Tests/DBAL/Functional/TypeConversionTest.php:94

I have tried to find a solution for this issue but apparently this is a driver issue as the decimal separator seems to depend on the system's locale settings. Unfortunately there is no driver option where we could set the desired decimal separator. For now it seems we have to live with it and I guess it works on english locale systems.

The pdo_ibm driver reveals more problems as it is still buggy (for years now). The driver still segfaults on decoding CLOB/BLOB resources. So every test that uses BLOBs in any way does not run. The second issue I encountered was a test where a table is referenced quoted in a SELECT statement:

There was 1 error:

1) Doctrine\Tests\DBAL\Functional\DataAccessTest::testPrepareWithQuoted
Doctrine\DBAL\DBALException: An exception occurred while executing 'SELECT test_int, test_string FROM "fetch_table" WHERE test_int = '1' AND test_string = 'foo'':

SQLSTATE[42S02]: Base table or view not found: -204 [IBM][CLI Driver][DB2/LINUXX8664] SQL0204N  "DB2INST1.fetch_table" is an undefined name.  SQLSTATE=42704
 (SQLNumResultCols[-204] at /home/deeky/db2/PDO_IBM-1.3.3/ibm_driver.c:153)

/home/deeky/dev/doctrine/dbal/lib/Doctrine/DBAL/DBALException.php:110
/home/deeky/dev/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOConnection.php:50
/home/deeky/dev/doctrine/dbal/lib/Doctrine/DBAL/Statement.php:86
/home/deeky/dev/doctrine/dbal/lib/Doctrine/DBAL/Connection.php:702
/home/deeky/dev/doctrine/dbal/tests/Doctrine/Tests/DBAL/Functional/DataAccessTest.php:154

This is weird and I couldn't find out why this does not work on pdo_ibm but works on ibm_db2.
Besides those issues pdo_ibm runs fine.

This PR does not make the IBM DB2 implementation perfect yet but it is a good start which is already usable. If we decide on officially supporting this vendor, I would do further work on this.

@doctrinebot
Copy link

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DBAL-707

We use Jira to track the state of pull requests and the versions they got
included in.

@beberlei
Copy link
Member

@deeky666 I would go and remove pdo_db2 from the driver list then to mark it as explicitly unsupported (people can use driver_class if they want). Segfaulting is not good.

@deeky666
Copy link
Member Author

@beberlei Yeah that makes sense, I was thinking about this, too. Shall I remove it in this PR or shall I do it in another PR? Otherwise are you willing to add support for DB2 in general if I do maintenance?

@beberlei
Copy link
Member

@deeky666 Remove in this PR and yes about support.

@deeky666
Copy link
Member Author

@beberlei done.

beberlei added a commit that referenced this pull request Dec 20, 2013
Fix IBM DB2 implementation / ibm_db2 driver
@beberlei beberlei merged commit 752b569 into doctrine:master Dec 20, 2013
}

$sourceReflection = new \ReflectionObject($sourceObject);
$destinationClassReflection = new \ReflectionObject($destinationClass);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks wrong when $destinationClass was passed as string, as it was then replaced by a ReflectionClass, and you are creating a ReflectionObject for a ReflectionClass instance here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arf no, sorry. It is replaced a second time. The naming of variables is really confusing on this method

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stof Yeah this is far from perfect. I have copied and modified this from my SQL Anywhere driver implementation which I coded long time ago and my knowlegde of reflection was not as good back then ;)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants