Skip to content

Bind only non-NULL BLOB values as BLOB on oci8 and ibm_db2 #5466

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

Merged
morozov merged 1 commit into from
Jun 26, 2022
Merged

Bind only non-NULL BLOB values as BLOB on oci8 and ibm_db2 #5466

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 0 additions & 29 deletions src/Driver/IBMDB2/Exception/CannotWriteToTemporaryFile.php
View file
Open in desktop

This file was deleted.

65 changes: 29 additions & 36 deletions src/Driver/IBMDB2/Statement.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
use Doctrine\DBAL\Driver\Exception;
use Doctrine\DBAL\Driver\IBMDB2\Exception\CannotCopyStreamToStream;
use Doctrine\DBAL\Driver\IBMDB2\Exception\CannotCreateTemporaryFile;
use Doctrine\DBAL\Driver\IBMDB2\Exception\CannotWriteToTemporaryFile;
use Doctrine\DBAL\Driver\IBMDB2\Exception\StatementError;
use Doctrine\DBAL\Driver\Result as ResultInterface;
use Doctrine\DBAL\Driver\Statement as StatementInterface;
Expand All @@ -16,7 +15,6 @@
use function db2_execute;
use function error_get_last;
use function fclose;
use function fwrite;
use function is_int;
use function is_resource;
use function stream_copy_to_stream;
Expand All @@ -41,7 +39,7 @@ final class Statement implements StatementInterface
* 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
*
* @var mixed[][]
* @var array<int,string|resource|null>
*/
private $lobs = [];

Expand Down Expand Up @@ -78,17 +76,7 @@ public function bindParam($param, &$variable, $type = ParameterType::STRING, $le
break;

case ParameterType::LARGE_OBJECT:
if (isset($this->lobs[$param])) {
[, $handle] = $this->lobs[$param];
fclose($handle);
}

$handle = $this->createTemporaryFile();
$path = stream_get_meta_data($handle)['uri'];

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

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

default:
Expand Down Expand Up @@ -119,19 +107,11 @@ private function bind($position, &$variable, int $parameterType, int $dataType):
*/
public function execute($params = null): ResultInterface
{
foreach ($this->lobs as [$source, $target]) {
if (is_resource($source)) {
$this->copyStreamToStream($source, $target);

continue;
}

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

$result = @db2_execute($this->stmt, $params ?? $this->parameters);

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

Expand All @@ -144,6 +124,31 @@ public function execute($params = null): ResultInterface
return new Result($this->stmt);
}

/**
* @return list<resource>
*
* @throws Exception
*/
private function bindLobs(): array
{
$handles = [];

foreach ($this->lobs as $param => $value) {
if (is_resource($value)) {
$handle = $handles[] = $this->createTemporaryFile();
$path = stream_get_meta_data($handle)['uri'];

$this->copyStreamToStream($value, $handle);

$this->bind($param, $path, DB2_PARAM_FILE, DB2_BINARY);
} else {
$this->bind($param, $value, DB2_PARAM_IN, DB2_CHAR);
}
}

return $handles;
}

/**
* @return resource
*
Expand Down Expand Up @@ -172,16 +177,4 @@ private function copyStreamToStream($source, $target): void
throw CannotCopyStreamToStream::new(error_get_last());
}
}

/**
* @param resource $target
*
* @throws Exception
*/
private function writeStringToStream(string $string, $target): void
{
if (@fwrite($target, $string) === false) {
throw CannotWriteToTemporaryFile::new(error_get_last());
}
}
}
10 changes: 7 additions & 3 deletions src/Driver/OCI8/Statement.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,14 @@ public function bindParam($param, &$variable, $type = ParameterType::STRING, $le
}

if ($type === ParameterType::LARGE_OBJECT) {
$lob = oci_new_descriptor($this->connection, OCI_D_LOB);
$lob->writeTemporary($variable, OCI_TEMP_BLOB);
if ($variable !== null) {
$lob = oci_new_descriptor($this->connection, OCI_D_LOB);
$lob->writeTemporary($variable, OCI_TEMP_BLOB);

$variable =& $lob;
$variable =& $lob;
} else {
$type = ParameterType::STRING;
Copy link

@fabacino fabacino Jun 27, 2022

Choose a reason for hiding this comment

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

Just out of curiosity, why did you choose ParameterType::STRING here over ParameterType::NULL?

Copy link
Member Author

@morozov morozov Jun 27, 2022

Choose a reason for hiding this comment

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

There’s no special handling of the null parameters in oci8.

Copy link

@fabacino fabacino Jun 27, 2022

Choose a reason for hiding this comment

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

I see, thanks for explaining.

}
}

return oci_bind_by_name(
Expand Down
43 changes: 36 additions & 7 deletions tests/Functional/BlobTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@ protected function setUp(): void
if (TestUtil::isDriverOneOf('pdo_oci')) {
// inserting BLOBs as streams on Oracle requires Oracle-specific SQL syntax which is currently not supported
// see http://php.net/manual/en/pdo.lobs.php#example-1035
self::markTestSkipped('DBAL doesn\'t support storing LOBs represented as streams using PDO_OCI');
self::markTestSkipped("DBAL doesn't support storing LOBs represented as streams using PDO_OCI");
}

$table = new Table('blob_table');
$table->addColumn('id', 'integer');
$table->addColumn('clobcolumn', 'text');
$table->addColumn('blobcolumn', 'blob');
$table->addColumn('clobcolumn', 'text', ['notnull' => false]);
$table->addColumn('blobcolumn', 'blob', ['notnull' => false]);
$table->setPrimaryKey(['id']);

$this->dropAndCreateTable($table);
Expand All @@ -46,6 +46,25 @@ public function testInsert(): void
self::assertEquals(1, $ret);
}

public function testInsertNull(): void
{
$ret = $this->connection->insert('blob_table', [
'id' => 1,
'clobcolumn' => null,
'blobcolumn' => null,
], [
ParameterType::INTEGER,
ParameterType::STRING,
ParameterType::LARGE_OBJECT,
]);

self::assertEquals(1, $ret);

[$clobValue, $blobValue] = $this->fetchRow();
self::assertNull($clobValue);
self::assertNull($blobValue);
}

public function testInsertProcessesStream(): void
{
// https://github.com/doctrine/dbal/issues/3290
Expand Down Expand Up @@ -152,13 +171,23 @@ public function testBindParamProcessesStream(): void

private function assertBlobContains(string $text): void
{
$rows = $this->connection->fetchFirstColumn('SELECT blobcolumn FROM blob_table');
[, $blobValue] = $this->fetchRow();

self::assertCount(1, $rows);

$blobValue = Type::getType('blob')->convertToPHPValue($rows[0], $this->connection->getDatabasePlatform());
$blobValue = Type::getType('blob')->convertToPHPValue($blobValue, $this->connection->getDatabasePlatform());

self::assertIsResource($blobValue);
self::assertEquals($text, stream_get_contents($blobValue));
}

/**
* @return list<mixed>
*/
private function fetchRow(): array
{
$rows = $this->connection->fetchAllNumeric('SELECT clobcolumn, blobcolumn FROM blob_table');

self::assertCount(1, $rows);

return $rows[0];
}
}