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

PdoDriver: fix pdo connection in exception mode (target >=4.1.0) #293

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

PdoDriver: fix pdo connection in exception mode (target >=4.1.0) #293

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
411ce51
PdoDriver: fix pdo connection in exception mode
jkuchar Jun 11, 2018
1472f06
code style
jkuchar Jun 11, 2018
7d87a70
removed previous exception
jkuchar Jul 6, 2018
b9d0ba1
PdoDriver: simplified query flow
jkuchar Jul 6, 2018
98e10a4
PdoDriver: Experimental support for warning error mode?
jkuchar Jul 6, 2018
1e1d8b5
comment
jkuchar Jul 6, 2018
39f03e9
PdoDriver: provided connection test
jkuchar Jul 30, 2018
f55f22f
coding standard
jkuchar Jul 31, 2018
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
10 changes: 5 additions & 5 deletions src/Dibi/Drivers/MySqliDriver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,19 +150,19 @@ public function query(string $sql): ?Dibi\ResultDriver
}


public static function createException(string $message, $code, string $sql): Dibi\DriverException
public static function createException(string $message, $code, string $sql, \Throwable $previous = null): Dibi\DriverException
{
if (in_array($code, [1216, 1217, 1451, 1452, 1701], true)) {
return new Dibi\ForeignKeyConstraintViolationException($message, $code, $sql);
return new Dibi\ForeignKeyConstraintViolationException($message, $code, $sql, $previous);

} elseif (in_array($code, [1062, 1557, 1569, 1586], true)) {
return new Dibi\UniqueConstraintViolationException($message, $code, $sql);
return new Dibi\UniqueConstraintViolationException($message, $code, $sql, $previous);

} elseif (in_array($code, [1048, 1121, 1138, 1171, 1252, 1263, 1566], true)) {
return new Dibi\NotNullConstraintViolationException($message, $code, $sql);
return new Dibi\NotNullConstraintViolationException($message, $code, $sql, $previous);

} else {
return new Dibi\DriverException($message, $code, $sql);
return new Dibi\DriverException($message, $code, $sql, $previous);
}
}

Expand Down
10 changes: 5 additions & 5 deletions src/Dibi/Drivers/OracleDriver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,19 +112,19 @@ public function query(string $sql): ?Dibi\ResultDriver
}


public static function createException(string $message, $code, string $sql): Dibi\DriverException
public static function createException(string $message, $code, string $sql, \Throwable $previous = null): Dibi\DriverException
{
if (in_array($code, [1, 2299, 38911], true)) {
return new Dibi\UniqueConstraintViolationException($message, $code, $sql);
return new Dibi\UniqueConstraintViolationException($message, $code, $sql, $previous);

} elseif (in_array($code, [1400], true)) {
return new Dibi\NotNullConstraintViolationException($message, $code, $sql);
return new Dibi\NotNullConstraintViolationException($message, $code, $sql, $previous);

} elseif (in_array($code, [2266, 2291, 2292], true)) {
return new Dibi\ForeignKeyConstraintViolationException($message, $code, $sql);
return new Dibi\ForeignKeyConstraintViolationException($message, $code, $sql, $previous);

} else {
return new Dibi\DriverException($message, $code, $sql);
return new Dibi\DriverException($message, $code, $sql, $previous);
}
}

Expand Down
33 changes: 23 additions & 10 deletions src/Dibi/Drivers/PdoDriver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,31 +89,44 @@ public function disconnect(): void
*/
public function query(string $sql): ?Dibi\ResultDriver
{
$res = $this->connection->query($sql);
if ($res) {
$this->affectedRows = $res->rowCount();
return $res->columnCount() ? $this->createResultDriver($res) : null;
try {
$res = $this->connection->query($sql);

if ($res) {
$this->affectedRows = $res->rowCount();
return $res->columnCount() ? $this->createResultDriver($res) : null;
}

} catch (\PDOException $pdoException) {
$this->affectedRows = null;
throw $this->createException($pdoException->errorInfo, $sql, $pdoException);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not like that I need to pass $previous all around. Any ideas how to do better?

Copy link
Owner

Choose a reason for hiding this comment

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

I think the PDOException does not contain any additional information, so there is no need to pass on the previous one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, as it is implementation detail, no code ever should depend on that. Gonna remove that

}

$this->affectedRows = null;
throw $this->createException($this->connection->errorInfo(), $sql, null);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not like this handling of error cases. Is is done twice, any ideas?

Copy link
Owner

Choose a reason for hiding this comment

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

Is your intention to make it work regardless of ATTR_ERRMODE, ie. with ERRMODE_WARNING and ERRMODE_EXCEPTION together?

Maybe you can use finally:

try {
    $res = $this->connection->query($sql);
    ...

} catch (\PDOException $e) {
} finally {
	throw $this->createException(isset($e) ?  $e->errorInfo : $this->connection->errorInfo(), $sql);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Finally will be executed also on successful case, I have extracted this in main body of the function.


}


private function createException(array $errorInfo, string $sql, ?\Throwable $previous): Dibi\DriverException {
[$sqlState, $code, $message] = $errorInfo;

[$sqlState, $code, $message] = $this->connection->errorInfo();
$message = "SQLSTATE[$sqlState]: $message";
switch ($this->driverName) {
case 'mysql':
throw MySqliDriver::createException($message, $code, $sql);
return MySqliDriver::createException($message, $code, $sql, $previous);

case 'oci':
throw OracleDriver::createException($message, $code, $sql);
return OracleDriver::createException($message, $code, $sql, $previous);

case 'pgsql':
throw PostgreDriver::createException($message, $sqlState, $sql);
return PostgreDriver::createException($message, $sqlState, $sql, $previous);

case 'sqlite':
throw SqliteDriver::createException($message, $code, $sql);
return SqliteDriver::createException($message, $code, $sql, $previous);

default:
throw new Dibi\DriverException($message, $code, $sql);
return new Dibi\DriverException($message, $code, $sql, $previous);
}
}

Expand Down
12 changes: 6 additions & 6 deletions src/Dibi/Drivers/PostgreDriver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,27 +132,27 @@ public function query(string $sql): ?Dibi\ResultDriver
}


public static function createException(string $message, $code = null, string $sql = null): Dibi\DriverException
public static function createException(string $message, $code = null, string $sql = null, \Throwable $previous = null): Dibi\DriverException
{
if ($code === null && preg_match('#^ERROR:\s+(\S+):\s*#', $message, $m)) {
$code = $m[1];
$message = substr($message, strlen($m[0]));
}

if ($code === '0A000' && strpos($message, 'truncate') !== false) {
return new Dibi\ForeignKeyConstraintViolationException($message, $code, $sql);
return new Dibi\ForeignKeyConstraintViolationException($message, $code, $sql, $previous);

} elseif ($code === '23502') {
return new Dibi\NotNullConstraintViolationException($message, $code, $sql);
return new Dibi\NotNullConstraintViolationException($message, $code, $sql, $previous);

} elseif ($code === '23503') {
return new Dibi\ForeignKeyConstraintViolationException($message, $code, $sql);
return new Dibi\ForeignKeyConstraintViolationException($message, $code, $sql, $previous);

} elseif ($code === '23505') {
return new Dibi\UniqueConstraintViolationException($message, $code, $sql);
return new Dibi\UniqueConstraintViolationException($message, $code, $sql, $previous);

} else {
return new Dibi\DriverException($message, $code, $sql);
return new Dibi\DriverException($message, $code, $sql, $previous);
}
}

Expand Down
10 changes: 5 additions & 5 deletions src/Dibi/Drivers/SqliteDriver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public function query(string $sql): ?Dibi\ResultDriver
}


public static function createException(string $message, $code, string $sql): Dibi\DriverException
public static function createException(string $message, $code, string $sql, \Throwable $throwable = null): Dibi\DriverException
{
if ($code !== 19) {
return new Dibi\DriverException($message, $code, $sql);
Expand All @@ -107,20 +107,20 @@ public static function createException(string $message, $code, string $sql): Dib
|| strpos($message, 'is not unique') !== false
|| strpos($message, 'UNIQUE constraint failed') !== false
) {
return new Dibi\UniqueConstraintViolationException($message, $code, $sql);
return new Dibi\UniqueConstraintViolationException($message, $code, $sql, $throwable);

} elseif (strpos($message, 'may not be null') !== false
|| strpos($message, 'NOT NULL constraint failed') !== false
) {
return new Dibi\NotNullConstraintViolationException($message, $code, $sql);
return new Dibi\NotNullConstraintViolationException($message, $code, $sql, $throwable);

} elseif (strpos($message, 'foreign key constraint failed') !== false
|| strpos($message, 'FOREIGN KEY constraint failed') !== false
) {
return new Dibi\ForeignKeyConstraintViolationException($message, $code, $sql);
return new Dibi\ForeignKeyConstraintViolationException($message, $code, $sql, $throwable);

} else {
return new Dibi\ConstraintViolationException($message, $code, $sql);
return new Dibi\ConstraintViolationException($message, $code, $sql, $throwable);
}
}

Expand Down