Add SAP Sybase SQL Anywhere database vendor #293

deeky666 · 2013-03-28T13:45:33Z

This PR adds the driver and DBAL for SAP Sybase SQL Anywhere databases. It distinguishes between versions 10 and below (base SQLAnywherePlatform), 11 (SQLAnywhere11Platform) and 12 (SQLAnywherer12Platform), similar to Microsoft SQL Server.
Most of the functional tests pass but there are some exceptions:

DataAccessTest::testFetchAllWithTypes
```
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'2010-01-01 10:10:10'
+'2010-01-01 10:10:10.000'
```
The test is not aware of the datetime format string of the platform, which differs in SQL Anywhere. Therefore it fails. In SQL Anywhere a datetime is stored with second fractions by default.
IMO I have two options here. Changing the default timestamp format for data output in the driver on every connection so that it fits Y-m-d H:i:s and changing the date format strings in the platforms or modifying the test to be aware of the platform specific datetime format.
MasterSlaveConnectionTest::testKeepSlaveBeginTransactionStaysOnMaster
The test simply hangs up here and I don't exactly know why. When I change the value for insert in line 95 to some other value than 30 it perfectly works. Any help would be appreciated here.
ModifyLimitQueryTest::testModifyLimitQueryGroupBy
```
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
Array (
--    0 => 1
--    1 => 2
++    0 => 2
++    1 => 1
)
```
The test does not tell the SELECT statement to order the result, thus the result is not deterministic and returns the results in wrong order.
IMO the test should be fixed so that the result is ordered correctly. Then the test will pass for SQL Anywhere.
TemporaryTableTest
```
Exception: [Doctrine\DBAL\DBALException] An exception occurred while executing 'CREATE GLOBAL TEMPORARY TABLE temporary (id INT NOT NULL)':
SQLSTATE [42W04] [-131] Syntaxfehler bei 'temporary' in Zeile 1
With queries:
SQL: 'DROP TABLE temporary' Params: 
SQL: 'CREATE GLOBAL TEMPORARY TABLE temporary (id INT NOT NULL)'     Params: 
SQL: 'DROP TABLE nontemporary' Params:
```
The tests use the unquoted keyword "temporary" reserved by SQL Anywhere for the temporary table name to create.
I cannot quote the name of the temporary table in the platform as the method getTemporaryTableName expects a string and not an instance of Doctrine\DBAL\Schema\Table as parameter.
Either the tests should use a non reserved keyword as temporary table name or the temporary table name has to be quoted in the platform (but I don't know exactly how).
TemporaryTableTest::testDropTemporaryTableNotAutoCommitTransaction
```
In an event of an error this result has one row, because of an implicit commit.
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
Array (
++    0 => Array (...)
)
```
The test fails because of an implicit commit during the transaction (drop temporary table) which inserts the first value. I couldn't find a solution for this problem. Seems to be the same problem as reported in DDC-1337.
WriteTest::testLastInsertIdNoSequenceGiven
```
Failed asserting that 14 is false.
```
I don't exactly know why the test expects the lastInsertId to be false here. Using the same connection for 14 inserts into a newly created autoincrement table should result in lastInsertId = 14 IMO. Is this a bug in the test? How do other vendors behave here?

I hope this addition is appreciated and any feedback/help on the above issues would be welcomed.

doctrinebot · 2013-03-28T13:45:42Z

Hello,

thank you for positing this Pull Request. I have automatically opened an issue on our Jira Bug Tracker for you with the details of this Pull-Request. See the Link:

http://doctrine-project.org/jira/browse/DBAL-475

stof · 2013-03-28T13:58:13Z

lib/Doctrine/DBAL/Schema/SQLAnywhereSchemaManager.php

+use Doctrine\DBAL\Schema\Column;
+use Doctrine\DBAL\Schema\ForeignKeyConstraint;
+use Doctrine\DBAL\Schema\Sequence;
+use Doctrine\DBAL\Schema\View;


All these use statements are useless as they are already in the same namespace

mvrhov · 2013-03-28T14:15:20Z

ModifyLimitQueryTest::testModifyLimitQueryGroupBy
IMO you should extend the pager the same way that's done for postgresql. On the second hand the way it's done for postgresql is the proper way. So that should be default.

deeky666 · 2013-03-28T14:21:45Z

I don't see that the PostgreSQLPlatform makes any modifications to the limit query? It is inherited from AbstractPlatform...

mvrhov · 2013-03-28T14:37:43Z

ouch this is DBAL... The exception for PostgreSQL is made in ORM.
Nonetheless the PostgreSQL is also respecting SQL standard and without the ORDER BY the order of returned values is non-deterministic. It's just a pure luck that they are returned in the same order as by default PostgreSQL returns them in the same order they are found on the disk.

deeky666 · 2013-03-28T14:50:40Z

Yep but if the result is non-deterministic you can't make any expectations about the order of the results. This is the problem here. Maybe other vendors order it correctly without an ORDER BY clause but then this is due to the internal SQL query optimizer rather than by the intention of the query itself. The result is simply unpredictable and SQL Anywhere sometimes seems to order them different without an ORDER BY clause.

deeky666 · 2013-03-28T14:54:25Z

IMO this is more an issue in the test rather than the platform. If you don't explicitly order a result you cannot make any expectations about the order afterwards.

mvrhov · 2013-03-28T14:57:15Z

Yeah, I agree that the test should be fixed by adding an order by clause.

deeky666 · 2013-03-30T16:37:07Z

ModifyLimitQueryTest::testModifyLimitQueryGroupBy problem fixed in PR #295.

beberlei · 2013-03-30T21:25:05Z

@deeky666 how could i verify the tests run? What are the steps to install SQL Anywhere 10-12? Would you be willing to be the champion for this platform? We cannot support this ourselves, because nobody in the team uses SAP SQL Anywhere.

Otherwise this PR is impressive work, I am very amazed by it.

deeky666 · 2013-03-31T14:49:40Z

@beberlei You can download the database versions 10, 11 and 12 here. You have to fill in the registration form to get the download link and product key needed for installation. During the installation you need to make sure that you install the SQL Anywhere database client and server and optionally the Sybase Central administration tool for easier testing purposes. Other components like MobiLink, QAnywhere and UltraLite are not required for testing the platform.
You can download the PHP sqlanywhere extension (native driver) here. You can either download a precompiled binary for your specific PHP version or compile it yourself using the source code and phpize.
After the installation of the database server and PHP extension you need to initially setup the doctrine test databases for functional testing. You can use the Sybase Central tool to create and start them.
I would suggest that if you need more detailed information on how to setup the databases just contact me again as it might be a little too much for this topic.
In general I would definitely like to actively maintain this platform and I am glad that you like my work!

deeky666 · 2013-10-09T19:25:26Z

@beberlei rebased PR and adjusted to current abstract platform / schema. Also added version 16 of the platform. Most of the test pass but there are some problems left I'd like to discuss. Maybe we should talk soon to get this baby going :D Awaiting your feedback.

Slashbunny · 2013-10-22T15:49:33Z

lib/Doctrine/DBAL/Driver/SQLAnywhere/Driver.php

+
+        return
+            'LINKS=tcpip(HOST=' . $host . ';PORT=' . $port . ';DoBroadcast=Direct)' .
+            ';SERVER=' . $server .


The "SERVER" connection parameter is only valid in SQLAnywhere 12 databases and newer[1]. Prior to version 12, the connection parameter name was "ENG" [2]. On SQLA 11, I get the following error because "SERVER" isn't recognized as valid:

SQLSTATE [08W49] [-95] Parse error: Invalid or missing keyword near 'SERVER''

However, ENG and SERVER are just the "short" versions of the parameter. The "long" version of the parameter before and after SQLA12 is "ServerName". Perhaps an easy solution is to just change this to "ServerName" so it supports all versions?

Also, I'd like to thank you for your work on this driver! :)

[1] http://infocenter.sybase.com/help/index.jsp?topic=/com.sybase.help.sqlanywhere.12.0.1/sachanges/sa12-deprecated-features.html
[2] http://infocenter.sybase.com/help/index.jsp?topic=/com.sybase.help.sqlanywhere.11.0.1/dbadmin_en11/servernameparameter.html

Thanks for pointing this out! Seems I have missed that compatibility issue. Fixed it. :)

deeky666 · 2013-11-11T23:13:41Z

@doctrine/team-doctrine2 okay, what else needs to be done here from your perspectives? Adjusted some more tests in PR #402 and PR #399 for this platform to work with. The only tests left failing are those where assumptions on DDL statements during transactions are done (which only works on some vendors) and @beberlei suggested that they can be ignored for now. ;)

Ocramius · 2013-11-11T23:44:04Z

lib/Doctrine/DBAL/Platforms/SQLAnywherePlatform.php

+        if ($constraint instanceof Index) {
+            if ($constraint->isPrimary()) {
+                $query .= $this->getPrimaryKeyDeclarationSQL($constraint, $constraint->getQuotedName($this));
+            } elseif ($constraint->isUnique()) {


Returning early avoids these elseifs

Add SAP Sybase SQL Anywhere database vendor

beberlei · 2013-11-13T17:39:34Z

@deeky666 thank you very much for this tremendous work!

deeky666 · 2013-11-13T17:55:43Z

@beberlei You're welcome! :) Thanks for merging this one.

stof reviewed Mar 28, 2013
View reviewed changes

Slashbunny reviewed Oct 22, 2013
View reviewed changes

deeky666 mentioned this pull request Oct 22, 2013

Fix reserved keyword conflicts in TemporaryTableTest #391

Merged

deeky666 added 12 commits November 2, 2013 18:49

add SAP SQL Anywhere database vendor

d0c9ef8

remove useless use statements

bfbc1e2

remove useless platform class property

2d1c970

add comma to the end of the array of keyword lists

0779db9

add missing platform test

9f6a405

rebase platform and tests

9c042ee

rebase platform

edf42f1

rebase platform tests

7d408fc

add more SQL Anywhere 12 platform tests

acb3603

add SQL Anywhere 16 platform

56e8e09

fix server connection parameter for older driver versions

11bbb16

add missing date expressions

0083774

Ocramius reviewed Nov 11, 2013
View reviewed changes

deeky666 added 2 commits November 12, 2013 01:45

fix CS

87049d6

refactored platform

617b824

beberlei added a commit that referenced this pull request Nov 13, 2013

Merge pull request #293 from deeky666/add-sql-anywhere-database

556b624

Add SAP Sybase SQL Anywhere database vendor

beberlei merged commit 556b624 into doctrine:master Nov 13, 2013

This was referenced Dec 6, 2015

DDC-1672: [GH-293] by FabioBatSilva: [DDC-1663]Native SQL Query Result Set Mappings doctrine/orm#2317

Closed

DBAL-475: [GH-293] Add SAP SQL Anywhere database vendor #1675

Closed

DBAL-639: [GH-391] Fix reserved keyword conflicts in TemporaryTableTest #1856

Closed

github-actions bot locked as resolved and limited conversation to collaborators Sep 2, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add SAP Sybase SQL Anywhere database vendor #293

Add SAP Sybase SQL Anywhere database vendor #293

deeky666 commented Mar 28, 2013

doctrinebot commented Mar 28, 2013

stof Mar 28, 2013

mvrhov commented Mar 28, 2013

deeky666 commented Mar 28, 2013

mvrhov commented Mar 28, 2013

deeky666 commented Mar 28, 2013

deeky666 commented Mar 28, 2013

mvrhov commented Mar 28, 2013

deeky666 commented Mar 30, 2013

beberlei commented Mar 30, 2013

deeky666 commented Mar 31, 2013

deeky666 commented Oct 9, 2013

Slashbunny Oct 22, 2013

deeky666 Oct 22, 2013

deeky666 commented Nov 11, 2013

Ocramius Nov 11, 2013

beberlei commented Nov 13, 2013

deeky666 commented Nov 13, 2013

Add SAP Sybase SQL Anywhere database vendor #293

Add SAP Sybase SQL Anywhere database vendor #293

Conversation

deeky666 commented Mar 28, 2013

doctrinebot commented Mar 28, 2013

stof Mar 28, 2013

Choose a reason for hiding this comment

mvrhov commented Mar 28, 2013

deeky666 commented Mar 28, 2013

mvrhov commented Mar 28, 2013

deeky666 commented Mar 28, 2013

deeky666 commented Mar 28, 2013

mvrhov commented Mar 28, 2013

deeky666 commented Mar 30, 2013

beberlei commented Mar 30, 2013

deeky666 commented Mar 31, 2013

deeky666 commented Oct 9, 2013

Slashbunny Oct 22, 2013

Choose a reason for hiding this comment

deeky666 Oct 22, 2013

Choose a reason for hiding this comment

deeky666 commented Nov 11, 2013

Ocramius Nov 11, 2013

Choose a reason for hiding this comment

beberlei commented Nov 13, 2013

deeky666 commented Nov 13, 2013