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.

Sign up for GitHub

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

Jump to bottom

Implemented handling BLOBs represented as stream resources for IBM DB2 #3309

Merged
merged 1 commit into from Oct 13, 2018
Merged

Implemented handling BLOBs represented as stream resources for IBM DB2 #3309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
127 changes: 109 additions & 18 deletions lib/Doctrine/DBAL/Driver/IBMDB2/DB2Statement.php
View file
Open in desktop
Expand Up @@ -13,8 +13,10 @@
use ReflectionProperty;
use stdClass;
use const CASE_LOWER;
use const DB2_BINARY;
use const DB2_CHAR;
use const DB2_LONG;
use const DB2_PARAM_FILE;
use const DB2_PARAM_IN;
use function array_change_key_case;
use function db2_bind_param;
Expand All @@ -28,14 +30,21 @@
use function db2_num_rows;
use function db2_stmt_error;
use function db2_stmt_errormsg;
use function error_get_last;
use function fclose;
use function func_get_args;
use function func_num_args;
use function fwrite;
use function gettype;
use function is_object;
use function is_resource;
use function is_string;
use function ksort;
use function sprintf;
use function stream_copy_to_stream;
use function stream_get_meta_data;
use function strtolower;
use function tmpfile;

class DB2Statement implements IteratorAggregate, Statement
{
Expand All @@ -45,6 +54,14 @@ class DB2Statement implements IteratorAggregate, Statement
/** @var mixed[] */
private $bindParam = [];

/**
* Map of LOB parameter positions to the tuples containing reference to the variable bound to the driver statement
* and the temporary file handle bound to the underlying statement
morozov marked this conversation as resolved.
Show resolved Hide resolved
*
* @var mixed[][]
*/
private $lobs = [];

/** @var string Name of the default class to instantiate when fetching class instances. */
private $defaultFetchClass = '\stdClass';

Expand All @@ -61,16 +78,6 @@ class DB2Statement implements IteratorAggregate, Statement
*/
private $result = false;

/**
* DB2_BINARY, DB2_CHAR, DB2_DOUBLE, or DB2_LONG
*
* @var int[]
*/
static private $typeMap = [
ParameterType::INTEGER => DB2_LONG,
ParameterType::STRING => DB2_CHAR,
];

/**
* @param resource $stmt
*/
Expand All @@ -92,21 +99,48 @@ public function bindValue($param, $value, $type = ParameterType::STRING)
*/
public function bindParam($column, &$variable, $type = ParameterType::STRING, $length = null)
{
$this->bindParam[$column] =& $variable;
switch ($type) {
case ParameterType::INTEGER:
$this->bind($column, $variable, DB2_PARAM_IN, DB2_LONG);
break;

if ($type && isset(self::$typeMap[$type])) {
$type = self::$typeMap[$type];
} else {
$type = DB2_CHAR;
}
case ParameterType::LARGE_OBJECT:
if (isset($this->lobs[$column])) {
[, $handle] = $this->lobs[$column];
fclose($handle);
}

if (! db2_bind_param($this->stmt, $column, 'variable', DB2_PARAM_IN, $type)) {
throw new DB2Exception(db2_stmt_errormsg());
$handle = $this->createTemporaryFile();
$path = stream_get_meta_data($handle)['uri'];

$this->bind($column, $path, DB2_PARAM_FILE, DB2_BINARY);

$this->lobs[$column] = [&$variable, $handle];
break;

default:
$this->bind($column, $variable, DB2_PARAM_IN, DB2_CHAR);
break;
}

return true;
}

/**
* @param int|string $parameter Parameter position or name
* @param mixed $variable
*
* @throws DB2Exception
*/
private function bind($parameter, &$variable, int $parameterType, int $dataType) : void
{
$this->bindParam[$parameter] =& $variable;

if (! db2_bind_param($this->stmt, $parameter, 'variable', $parameterType, $dataType)) {
throw new DB2Exception(db2_stmt_errormsg());
}
}

/**
* {@inheritdoc}
*/
Expand Down Expand Up @@ -177,8 +211,24 @@ public function execute($params = null)
}
}

foreach ($this->lobs as [$source, $target]) {
if (is_resource($source)) {
$this->copyStreamToStream($source, $target);

continue;
}

$this->writeStringToStream($source, $target);
}

$retval = db2_execute($this->stmt, $params);

foreach ($this->lobs as [, $handle]) {
fclose($handle);
}

$this->lobs = [];

if ($retval === false) {
throw new DB2Exception(db2_stmt_errormsg());
}
Expand Down Expand Up @@ -372,4 +422,45 @@ private function castObject(stdClass $sourceObject, $destinationClass, array $ct

return $destinationClass;
}

/**
* @return resource
*
* @throws DB2Exception
*/
private function createTemporaryFile()
{
$handle = @tmpfile();

if ($handle === false) {
throw new DB2Exception('Could not create temporary file: ' . error_get_last()['message']);
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, this may be uncoverable code, I have no idea how to simulate tmpfile() failure. :/

Copy link
Member Author

Choose a reason for hiding this comment

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

The /tmp filesystem suddenly became read-only? Or partition is full. Not sure if we want to bother too much testing it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Mostly no space left on device, that may happen on tmpfs (RAM-based) /tmp. :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but I still don't want to change the API to make it unit-testable.

}

return $handle;
}

/**
* @param resource $source
* @param resource $target
*
* @throws DB2Exception
*/
private function copyStreamToStream($source, $target) : void
Copy link
Member

Choose a reason for hiding this comment

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

Please document these types

{
if (@stream_copy_to_stream($source, $target) === false) {
throw new DB2Exception('Could not copy source stream to temporary file: ' . error_get_last()['message']);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we somehow test this failure?

Copy link
Member Author

Choose a reason for hiding this comment

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

Same as above. We could emulate the failure with a mock if streams were objects. Otherwise, we'd have to define an object wrapper for a temporary file and mock it. Too much of a trouble for an implementation detail IMO.

}
}

/**
* @param resource $target
*
* @throws DB2Exception
*/
private function writeStringToStream(string $string, $target) : void
{
if (@fwrite($target, $string) === false) {
throw new DB2Exception('Could not write string to temporary file: ' . error_get_last()['message']);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we somehow test this failure?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, without unnecessarily complicating the API.

}
}
}
22 changes: 9 additions & 13 deletions tests/Doctrine/Tests/DBAL/Functional/BlobTest.php
View file
Open in desktop
Expand Up @@ -2,6 +2,7 @@

namespace Doctrine\Tests\DBAL\Functional;

use Doctrine\DBAL\Driver\OCI8\Driver as OCI8Driver;
use Doctrine\DBAL\Driver\PDOSqlsrv\Driver as PDOSQLSrvDriver;
use Doctrine\DBAL\FetchMode;
use Doctrine\DBAL\ParameterType;
Expand All @@ -10,7 +11,6 @@
use Doctrine\DBAL\Types\Type;
use Doctrine\Tests\DbalFunctionalTestCase;
use function fopen;
use function in_array;
use function str_repeat;
use function stream_get_contents;

Expand Down Expand Up @@ -55,10 +55,9 @@ public function testInsert()

public function testInsertProcessesStream()
{
if (in_array($this->connection->getDatabasePlatform()->getName(), ['oracle', 'db2'], true)) {
// https://github.com/doctrine/dbal/issues/3288 for DB2
// https://github.com/doctrine/dbal/issues/3290 for Oracle
$this->markTestIncomplete('Platform does not support stream resources as parameters');
// https://github.com/doctrine/dbal/issues/3290
if ($this->connection->getDriver() instanceof OCI8Driver) {
$this->markTestIncomplete('The oci8 driver does not support stream resources as parameters');
}

$longBlob = str_repeat('x', 4 * 8192); // send 4 chunks
Expand Down Expand Up @@ -112,10 +111,9 @@ public function testUpdate()

public function testUpdateProcessesStream()
{
if (in_array($this->connection->getDatabasePlatform()->getName(), ['oracle', 'db2'], true)) {
// https://github.com/doctrine/dbal/issues/3288 for DB2
// https://github.com/doctrine/dbal/issues/3290 for Oracle
$this->markTestIncomplete('Platform does not support stream resources as parameters');
// https://github.com/doctrine/dbal/issues/3290
if ($this->connection->getDriver() instanceof OCI8Driver) {
$this->markTestIncomplete('The oci8 driver does not support stream resources as parameters');
}

$this->connection->insert('blob_table', [
Expand All @@ -141,10 +139,8 @@ public function testUpdateProcessesStream()

public function testBindParamProcessesStream()
{
if (in_array($this->connection->getDatabasePlatform()->getName(), ['oracle', 'db2'], true)) {
// https://github.com/doctrine/dbal/issues/3288 for DB2
// https://github.com/doctrine/dbal/issues/3290 for Oracle
$this->markTestIncomplete('Platform does not support stream resources as parameters');
if ($this->connection->getDriver() instanceof OCI8Driver) {
$this->markTestIncomplete('The oci8 driver does not support stream resources as parameters');
}

$stmt = $this->connection->prepare("INSERT INTO blob_table(id, clobfield, blobfield) VALUES (1, 'ignored', ?)");
Expand Down