zendframework#186 zendframework#165 truncate based on length. If long…

…er than 63 bytes for postgresql, query the actual name.
alextech · Dec 25, 2016 · ce3b28c · ce3b28c
1 parent 99cfd74
commit ce3b28c
Show file tree

Hide file tree

Showing 4 changed files with 98 additions and 56 deletions.
diff --git a/doc/book/table-gateway.md b/doc/book/table-gateway.md
@@ -286,19 +286,14 @@ There are a number of features built-in and shipped with zend-db:
   
   With second parameter left null, `TableGateway` will generate sequence name based on same rule
   PostgreSQL uses (*tablename_columnname_seq* but if the resultant name is determined to be greater than 
-  63 characters, an additional query will be made to database schema to find what PostgreSQL has created 
+  63 bytes, an additional query will be made to database schema to find what PostgreSQL has created 
   instead, since transaction rules are more complex.
   
   This is important to know if you have long table and column names, and do not want
   to run an extra metadata query on every `TableGateway` object construction. If that is the case,
   take note of what PostgreSQL created using
   ```sql
-    SELECT 'column_default'
-    FROM information_schema.columns 
-    WHERE table_schema = 'public'
-      AND table_name = 'artist'
-      AND column_name = 'id'
-      AND column_default LIKE 'nextval%';
+    pg_get_serial_sequence('artist', 'id');
   ```
   
   take note of what `nextval` is reading from, and add it to `SequenceFeature` constructor.

diff --git a/src/TableGateway/Feature/SequenceFeature.php b/src/TableGateway/Feature/SequenceFeature.php
@@ -9,6 +9,7 @@
 
 namespace Zend\Db\TableGateway\Feature;
 
+use Zend\Db\Adapter\Adapter;
 use Zend\Db\Sql\Insert;
 use Zend\Db\Adapter\Driver\ResultInterface;
 use Zend\Db\Adapter\Driver\StatementInterface;
@@ -50,22 +51,50 @@ public function getSequenceName() {
             return $this->sequenceName;
         }
 
-        $platform = $this->tableGateway->getAdapter()->getPlatform();
-        $table = $this->tableGateway->getTable();
+        //@TODO move to PostgreSQL specific class (possibly decorator)
+        /** @var Adapter $adapter */
+        $adapter = $this->tableGateway->getAdapter();
+        $platform = $adapter->getPlatform();
+        $tableIdentifier = $this->tableGateway->getTable();
+        // need to preserve table name in case have to query postgres metadata
+        // (case for large resultant identifier names)
+        $tableName = '';
 
         $sequenceSuffix = '_' . $this->primaryKeyField . '_seq';
+        // To find whether exceed identifier length, need to keep track of combination of
+        // table name ane suffix but not including schema name.
+        // Since schema has to be appended in the end,
+        $sequenceObjectName = '';
 
-        if(is_string($table)) {
-            $this->sequenceName = $table . $sequenceSuffix;
-        } elseif(is_array($table)) {
+        if(is_string($tableIdentifier)) {
+            $tableName = $tableIdentifier;
+
+            $sequenceObjectName = $this->sequenceName = $tableIdentifier . $sequenceSuffix;
+        } elseif(is_array($tableIdentifier)) {
             // assuming key 0 is schema name
-            $table[1] .= $sequenceSuffix;
-            $this->sequenceName = $table;
-        } elseif($table instanceof TableIdentifier) {
-            $this->sequenceName = $table->hasSchema() ? [$table->getSchema(), $table->getTable().$sequenceSuffix] : $table->getTable().$sequenceSuffix;
+            $tableName = $tableIdentifier[1];
+
+            $this->sequenceName = $tableIdentifier;
+            $this->sequenceName[1] = $tableName.$sequenceSuffix;
+            $sequenceObjectName = $this->sequenceName[1];
+        } elseif($tableIdentifier instanceof TableIdentifier) {
+            $tableName = $tableIdentifier->getTable();
+            $sequenceObjectName = $tableName.$sequenceSuffix;
+            $this->sequenceName = $tableIdentifier->hasSchema() ? [$tableIdentifier->getSchema(), $sequenceObjectName] : $sequenceObjectName;
+        }
+
+        if(strlen($sequenceObjectName) < 64 ) {
+            $this->sequenceName = $platform->quoteIdentifierChain($this->sequenceName);
+            return  $this->sequenceName;
         }
 
-        $this->sequenceName = $platform->quoteIdentifierChain($this->sequenceName);
+        $statement = $adapter->createStatement();
+        $statement->prepare('SELECT pg_get_serial_sequence(:table, :column)');
+        $result = $statement->execute(['table' => $tableIdentifier, 'column' => $this->primaryKeyField]);
+        $this->sequenceName = $result->current()['pg_get_serial_sequence'];
+
+        // there could be a benefit porting this algorithm here instead of extra query call
+        // https://github.com/postgres/postgres/blob/f0e44751d7175fa3394da2c8f85e3ceb3cdbfe63/src/backend/commands/indexcmds.c#L1485
 
         return $this->sequenceName;
     }
@@ -116,23 +145,27 @@ public function nextSequenceId($columnName = null)
             return;
         }
 
-        $platform = $this->tableGateway->adapter->getPlatform();
+        /** @var Adapter $adapter */
+        $adapter = $this->tableGateway->adapter;
+        $platform = $adapter->getPlatform();
         $platformName = $platform->getName();
 
         switch ($platformName) {
             case 'Oracle':
                 $sql = 'SELECT ' . $platform->quoteIdentifier($this->sequenceName) . '.NEXTVAL as "nextval" FROM dual';
+                $param = [];
                 break;
             case 'PostgreSQL':
-                $sql = 'SELECT NEXTVAL(\'"' . $this->sequenceName . '"\')';
+                $sql = 'SELECT NEXTVAL( :sequence_name )';
+                $param = ['sequence_name' => $this->getSequenceName()];
                 break;
             default :
                 return;
         }
 
-        $statement = $this->tableGateway->adapter->createStatement();
+        $statement = $adapter->createStatement();
         $statement->prepare($sql);
-        $result = $statement->execute();
+        $result = $statement->execute($param);
         $sequence = $result->current();
         unset($statement, $result);
         return $sequence['nextval'];
@@ -150,23 +183,28 @@ public function lastSequenceId($columnName = null)
             return;
         }
 
-        $platform = $this->tableGateway->adapter->getPlatform();
+        /** @var Adapter $adapter */
+        $adapter = $this->tableGateway->adapter;
+        $platform = $adapter->getPlatform();
         $platformName = $platform->getName();
 
         switch ($platformName) {
             case 'Oracle':
                 $sql = 'SELECT ' . $platform->quoteIdentifier($this->sequenceName) . '.CURRVAL as "currval" FROM dual';
+                $param = [];
                 break;
             case 'PostgreSQL':
-                $sql = 'SELECT CURRVAL(\'' . $this->sequenceName . '\')';
+                $sql = 'SELECT CURRVAL( :sequence_name )';
+                $param = ['sequence_name' => $this->getSequenceName()];
                 break;
+            //@TODO add SQLServer2016
             default :
                 return;
         }
 
-        $statement = $this->tableGateway->adapter->createStatement();
+        $statement = $adapter->createStatement();
         $statement->prepare($sql);
-        $result = $statement->execute();
+        $result = $statement->execute($param);
         $sequence = $result->current();
         unset($statement, $result);
         return $sequence['currval'];

diff --git a/test/TableGateway/Feature/FeatureSetTest.php b/test/TableGateway/Feature/FeatureSetTest.php
@@ -180,7 +180,7 @@ public function testCanCallMagicCallReturnsFalseWhenNoFeaturesHaveBeenAdded()
     public function testCallMagicCallSucceedsForValidMethodOfAddedFeature()
     {
         $featureSet = new FeatureSet;
-        $featureSet->addFeature($this->getMockSequence('table_sequence', 1));
+        $featureSet->addFeature($this->getMockSequence('table_name', 'sequence_name', 1));
         $this->assertEquals(1, $featureSet->callMagicCall('lastSequenceId', null));
     }
 
@@ -191,17 +191,17 @@ public function testCallMagicMethodAllSimilarFeaturesUntilNotNull()
     {
         $featureSet = new FeatureSet();
 
-        $featureSet->addFeature($this->getMockSequence('seq_1', 1));
-        $featureSet->addFeature($this->getMockSequence('seq_2', 2));
-        $featureSet->addFeature($this->getMockSequence('seq_3', 3));
+        $featureSet->addFeature($this->getMockSequence('col_1', 'seq_1', 1));
+        $featureSet->addFeature($this->getMockSequence('col_2', 'seq_2', 2));
+        $featureSet->addFeature($this->getMockSequence('col_3', 'seq_3', 3));
 
-        $result = $featureSet->callMagicCall('lastSequenceId', 'seq_2');
+        $result = $featureSet->callMagicCall('lastSequenceId', 'col_2');
 
         $this->assertEquals(2, $result);
     }
 
     // FeatureSet uses method_exists which does not work on mock objects. Therefore, need a real object.
-    private function getMockSequence($sequenceName, $expectedCurrVal)
+    private function getMockSequence($columnName, $sequenceName, $expectedCurrVal)
     {
         $platformMock = $this->getMock('Zend\Db\Adapter\Platform\Postgresql');
         $platformMock->expects($this->any())
@@ -214,8 +214,7 @@ private function getMockSequence($sequenceName, $expectedCurrVal)
 
         $statementMock = $this->getMock('Zend\Db\Adapter\Driver\StatementInterface');
         $statementMock->expects($this->any())
-            ->method('prepare')
-            ->with('SELECT CURRVAL(\'' . $sequenceName . '\')');
+            ->method('prepare');
         $statementMock->expects($this->any())
             ->method('execute')
             ->will($this->returnValue($resultMock));
@@ -237,7 +236,7 @@ private function getMockSequence($sequenceName, $expectedCurrVal)
         $reflectionProperty->setAccessible(true);
         $reflectionProperty->setValue($tableGatewayMock, $adapterMock);
 
-        $feature = new SequenceFeature('id', $sequenceName);
+        $feature = new SequenceFeature($columnName, $sequenceName);
         $feature->setTableGateway($tableGatewayMock);
 
         return $feature;

diff --git a/test/TableGateway/Feature/SequenceFeatureTest.php b/test/TableGateway/Feature/SequenceFeatureTest.php
@@ -10,9 +10,9 @@
 namespace ZendTest\Db\TableGateway\Feature;
 
 use PHPUnit_Framework_TestCase;
-use Zend\Db\Adapter\Platform\Postgresql;
 use Zend\Db\Sql\TableIdentifier;
 use Zend\Db\TableGateway\Feature\SequenceFeature;
+use ZendTest\Db\TestAsset\TrustingOraclePlatform;
 use ZendTest\Db\TestAsset\TrustingPostgresqlPlatform;
 
 class SequenceFeatureTest extends PHPUnit_Framework_TestCase
@@ -24,17 +24,7 @@ class SequenceFeatureTest extends PHPUnit_Framework_TestCase
     protected $primaryKeyField = 'id';
 
     /** @var string  sequence name */
-    protected $sequenceName = 'table_sequence';
-
-    /**
-     * @dataProvider nextSequenceIdProvider
-     */
-    public function testNextSequenceIdForNamedSequence($platformName, $statementSql)
-    {
-        $feature = new SequenceFeature($this->primaryKeyField, $this->sequenceName);
-        $feature->setTableGateway($this->tableGateway);
-        $feature->nextSequenceId();
-    }
+    protected $sequenceName = 'sequence_name';
 
     /**
      * @dataProvider tableIdentifierProvider
@@ -55,7 +45,31 @@ public function testSequenceNameGenerated($tableIdentifier, $sequenceName)
 
     public function testSequenceNameQueriedWhenTooLong()
     {
+        $adapter = $this->getMock('Zend\Db\Adapter\Adapter', ['getPlatform', 'createStatement'], [], '', false);
+        $adapter->expects($this->any())
+            ->method('getPlatform')
+            ->will($this->returnValue(new TrustingPostgresqlPlatform()));
+        $result = $this->getMockForAbstractClass('Zend\Db\Adapter\Driver\ResultInterface', [], '', false, true, true, ['current']);
+        $result->expects($this->any())
+            ->method('current')
+            ->will($this->returnValue(['pg_get_serial_sequence' => 'table_name_column_very_long_name_causing_postgresql_to_trun_seq']));
+        $statement = $this->getMockForAbstractClass('Zend\Db\Adapter\Driver\StatementInterface', [], '', false, true, true, ['prepare', 'execute']);
+        $statement->expects($this->any())
+            ->method('execute')
+            ->with(['table' => 'table_name', 'column' => 'column_very_long_name_causing_postgresql_to_truncate'])
+            ->will($this->returnValue($result));
+        $statement->expects($this->any())
+            ->method('prepare')
+            ->with('SELECT pg_get_serial_sequence(:table, :column)');
+        $adapter->expects($this->once())
+            ->method('createStatement')
+            ->will($this->returnValue($statement));
+        $this->tableGateway = $this->getMockForAbstractClass('Zend\Db\TableGateway\TableGateway', ['table_name', $adapter], '', true);
 
+        $sequence = new SequenceFeature('column_very_long_name_causing_postgresql_to_truncate');
+        $sequence->setTableGateway($this->tableGateway);
+
+        $this->assertEquals('table_name_column_very_long_name_causing_postgresql_to_trun_seq', $sequence->getSequenceName());
     }
 
     /**
@@ -73,15 +87,8 @@ public function testCacheSequenceName()
     /**
      * @dataProvider nextSequenceIdProvider
      */
-    public function testNextSequenceIdForSerialColumn($platformName, $statementSql)
+    public function testNextSequenceIdByPlatform($platform, $statementSql, $statementParameter)
     {
-        $platform = $this->getMockForAbstractClass('Zend\Db\Adapter\Platform\PlatformInterface', ['getName']);
-        $platform->expects($this->any())
-            ->method('getName')
-            ->will($this->returnValue($platformName));
-        $platform->expects($this->any())
-            ->method('quoteIdentifier')
-            ->will($this->returnValue($this->sequenceName));
         $adapter = $this->getMock('Zend\Db\Adapter\Adapter', ['getPlatform', 'createStatement'], [], '', false);
         $adapter->expects($this->any())
             ->method('getPlatform')
@@ -93,6 +100,7 @@ public function testNextSequenceIdForSerialColumn($platformName, $statementSql)
         $statement = $this->getMockForAbstractClass('Zend\Db\Adapter\Driver\StatementInterface', [], '', false, true, true, ['prepare', 'execute']);
         $statement->expects($this->any())
             ->method('execute')
+            ->with($statementParameter)
             ->will($this->returnValue($result));
         $statement->expects($this->any())
             ->method('prepare')
@@ -115,8 +123,10 @@ public function testDoNotReactToDifferentColumnName() {
 
     public function nextSequenceIdProvider()
     {
-        return [['PostgreSQL', 'SELECT NEXTVAL(\'"' . $this->sequenceName . '"\')'],
-            ['Oracle', 'SELECT ' . $this->sequenceName . '.NEXTVAL as "nextval" FROM dual']];
+        return [
+            [new TrustingPostgresqlPlatform(), 'SELECT NEXTVAL( :sequence_name )', ['sequence_name' => $this->sequenceName]],
+            [new TrustingOraclePlatform(),     'SELECT "' . $this->sequenceName . '".NEXTVAL as "nextval" FROM dual', []]
+        ];
     }
 
     public function tableIdentifierProvider()