From c9ce9fd321572d63b361ff28049d701009c454c5 Mon Sep 17 00:00:00 2001 From: Aaron Piotrowski Date: Fri, 15 Nov 2024 17:04:50 -0600 Subject: [PATCH 1/5] Do not attempt to parse int2vector and oidvector Fixes #68 --- src/Internal/PgSqlHandle.php | 18 +++++++++---- src/Internal/PgSqlResultIterator.php | 38 +++++++++++++++++----------- src/Internal/PgSqlType.php | 5 ++-- 3 files changed, 39 insertions(+), 22 deletions(-) diff --git a/src/Internal/PgSqlHandle.php b/src/Internal/PgSqlHandle.php index e37bd55..b0b5fb8 100644 --- a/src/Internal/PgSqlHandle.php +++ b/src/Internal/PgSqlHandle.php @@ -175,7 +175,7 @@ public function __construct( */ private static function fetchTypes(\PgSql\Connection $handle): array { - $result = \pg_query($handle, "SELECT t.oid, t.typcategory, t.typdelim, t.typelem + $result = \pg_query($handle, "SELECT t.oid, t.typcategory, t.typname, t.typdelim, t.typelem FROM pg_catalog.pg_type t JOIN pg_catalog.pg_namespace n ON t.typnamespace=n.oid WHERE t.typisdefined AND n.nspname IN ('pg_catalog', 'public') ORDER BY t.oid"); @@ -185,10 +185,18 @@ private static function fetchTypes(\PgSql\Connection $handle): array $types = []; while ($row = \pg_fetch_array($result, mode: \PGSQL_NUM)) { - [$oid, $type, $delimiter, $element] = $row; - \assert(\is_numeric($oid) && \is_numeric($element), "OID and element type expected to be integers"); - \assert(\is_string($type) && \is_string($delimiter), "Unexpected types in type catalog query results"); - $types[(int) $oid] = new PgSqlType($type, $delimiter, (int) $element); + [$oid, $typeCategory, $typeName, $delimiter, $element] = $row; + + \assert( + \is_numeric($oid) && \is_numeric($element), + "OID and element type expected to be integers", + ); + \assert( + \is_string($typeCategory) && \is_string($typeName) && \is_string($delimiter), + "Unexpected types in type catalog query results", + ); + + $types[(int) $oid] = new PgSqlType($typeCategory, $typeName, $delimiter, (int) $element); } return $types; diff --git a/src/Internal/PgSqlResultIterator.php b/src/Internal/PgSqlResultIterator.php index 856a04f..d970ffc 100644 --- a/src/Internal/PgSqlResultIterator.php +++ b/src/Internal/PgSqlResultIterator.php @@ -81,23 +81,31 @@ private function cast(int $oid, ?string $value): array|bool|int|float|string|nul $type = $this->types[$oid] ?? PgSqlType::getDefaultType(); - return match ($type->type) { - 'A' => ArrayParser::parse( // Array - $value, - fn (string $data) => $this->cast($type->element, $data), - $type->delimiter, - ), - 'B' => $value === 't', // Boolean - 'N' => match ($oid) { // Numeric - 700, 701 => (float) $value, // "float4" and "float8" to float - 1700 => $value, // Return "numeric" as string to retain precision - 790 => $value, // "money" includes currency symbol as string - default => (int) $value, // All other numeric types cast to an integer + return match ($type->category) { + 'A' => match ($type->name) { // Array + 'int2vector', 'oidvector' => $value, // Deprecated array types + default => ArrayParser::parse( + $value, + fn (string $data) => $this->cast($type->element, $data), + $type->delimiter, + ), }, - default => match ($oid) { // String - 17 => \pg_unescape_bytea($value), - default => $value, // Return a string for all other types + 'B' => match ($value) { + 't' => true, + 'f' => false, + default => throw new PostgresParseException('Unexpected value for boolean field: ' . $value), + }, // Boolean + 'N' => match ($type->name) { // Numeric + 'float4', 'float8' => (float) $value, + 'int2', 'int4', 'oid' => (int) $value, + 'int8' => \PHP_INT_SIZE >= 8 ? (int) $value : $value, // String on 32-bit systems + default => $value, // Return a string for all other numeric types }, + 'U' => match ($type->name) { + 'bytea' => \pg_unescape_bytea($value), + default => $value, + }, + default => $value, // Return a string for all other types }; } } diff --git a/src/Internal/PgSqlType.php b/src/Internal/PgSqlType.php index f158ff7..6cd3b10 100644 --- a/src/Internal/PgSqlType.php +++ b/src/Internal/PgSqlType.php @@ -8,7 +8,8 @@ final class PgSqlType private static ?self $default = null; public function __construct( - public readonly string $type, + public readonly string $category, + public readonly string $name, public readonly string $delimiter, public readonly int $element, ) { @@ -16,6 +17,6 @@ public function __construct( public static function getDefaultType(): self { - return self::$default ??= new self('S', ',', 0); + return self::$default ??= new self('S', 'text', ',', 0); } } From 9693c1de1739c657c71b83a7c8533825effe9d82 Mon Sep 17 00:00:00 2001 From: Aaron Piotrowski Date: Fri, 15 Nov 2024 18:45:57 -0600 Subject: [PATCH 2/5] Async querying of types --- src/Internal/PgSqlHandle.php | 99 +++++++++++++++++++++++++----------- 1 file changed, 70 insertions(+), 29 deletions(-) diff --git a/src/Internal/PgSqlHandle.php b/src/Internal/PgSqlHandle.php index b0b5fb8..f9cb262 100644 --- a/src/Internal/PgSqlHandle.php +++ b/src/Internal/PgSqlHandle.php @@ -17,9 +17,19 @@ use Revolt\EventLoop; use function Amp\async; -/** @internal */ +/** + * @internal + * + * @psalm-type PgSqlTypeMap = array Map of OID to corresponding PgSqlType. + */ final class PgSqlHandle extends AbstractHandle { + private const TYPE_QUERY = << "severity", \PGSQL_DIAG_SQLSTATE => "sqlstate", @@ -35,7 +45,7 @@ final class PgSqlHandle extends AbstractHandle \PGSQL_DIAG_SOURCE_FUNCTION => "source_function", ]; - /** @var array> */ + /** @var array> */ private static array $typeCache; private static ?\Closure $errorHandler = null; @@ -43,8 +53,8 @@ final class PgSqlHandle extends AbstractHandle /** @var \PgSql\Connection PostgreSQL connection handle. */ private ?\PgSql\Connection $handle; - /** @var array */ - private readonly array $types; + /** @var PgSqlTypeMap|null */ + private ?array $types = null; /** @var array> */ private array $statements = []; @@ -57,13 +67,11 @@ final class PgSqlHandle extends AbstractHandle public function __construct( \PgSql\Connection $handle, $socket, - string $id, + private readonly string $id, PostgresConfig $config, ) { $this->handle = $handle; - $this->types = (self::$typeCache[$id] ??= self::fetchTypes($handle)); - $handle = &$this->handle; $lastUsedAt = &$this->lastUsedAt; $deferred = &$this->pendingOperation; @@ -171,35 +179,66 @@ public function __construct( } /** - * @return array + * @return Future */ - private static function fetchTypes(\PgSql\Connection $handle): array + private function fetchTypes(): Future { - $result = \pg_query($handle, "SELECT t.oid, t.typcategory, t.typname, t.typdelim, t.typelem - FROM pg_catalog.pg_type t JOIN pg_catalog.pg_namespace n ON t.typnamespace=n.oid - WHERE t.typisdefined AND n.nspname IN ('pg_catalog', 'public') ORDER BY t.oid"); + if (isset(self::$typeCache[$this->id])) { + return self::$typeCache[$this->id]; + } - if ($result === false) { - throw new SqlException(\pg_last_error($handle)); + \assert($this->pendingOperation === null, 'Operation pending when fetching types!'); + + if ($this->handle === null) { + throw new \Error("The connection to the database has been closed"); } - $types = []; - while ($row = \pg_fetch_array($result, mode: \PGSQL_NUM)) { - [$oid, $typeCategory, $typeName, $delimiter, $element] = $row; + $result = \pg_send_query($this->handle, self::TYPE_QUERY); + if ($result === false) { + $this->close(); + throw new SqlException(\pg_last_error($this->handle)); + } - \assert( - \is_numeric($oid) && \is_numeric($element), - "OID and element type expected to be integers", - ); - \assert( - \is_string($typeCategory) && \is_string($typeName) && \is_string($delimiter), - "Unexpected types in type catalog query results", - ); + $this->pendingOperation = $queryDeferred = new DeferredFuture(); + $typesDeferred = new DeferredFuture(); - $types[(int) $oid] = new PgSqlType($typeCategory, $typeName, $delimiter, (int) $element); + EventLoop::reference($this->poll); + if ($result === 0) { + EventLoop::enable($this->await); } - return $types; + EventLoop::queue(function () use ($queryDeferred, $typesDeferred): void { + try { + $result = $queryDeferred->getFuture()->await(); + if (\pg_result_status($result) !== \PGSQL_TUPLES_OK) { + throw new SqlException(\pg_result_error($result)); + } + + $types = []; + while ($row = \pg_fetch_array($result, mode: \PGSQL_NUM)) { + [$oid, $category, $name, $delimiter, $element] = $row; + + \assert( + \is_numeric($oid) && \is_numeric($element), + "OID and element type expected to be integers", + ); + \assert( // For Psalm + \is_string($category) && \is_string($name) && \is_string($delimiter), + "Unexpected nulls in type catalog query results", + ); + + $types[(int) $oid] = new PgSqlType($category, $name, $delimiter, (int) $element); + } + + $typesDeferred->complete($types); + } catch (\Throwable $exception) { + $this->close(); + $typesDeferred->error($exception); + unset(self::$typeCache[$this->id]); + } + }); + + return self::$typeCache[$this->id] = $typesDeferred->getFuture(); } private static function getErrorHandler(): \Closure @@ -224,12 +263,12 @@ public function isClosed(): bool * @param \Closure $function Function to execute. * @param mixed ...$args Arguments to pass to function. * - * @return \PgSql\Result - * * @throws SqlException */ private function send(\Closure $function, mixed ...$args): mixed { + $this->types ??= $this->fetchTypes()->await(); + while ($this->pendingOperation) { try { $this->pendingOperation->getFuture()->await(); @@ -275,6 +314,8 @@ private function createResult(\PgSql\Result $result, string $sql): PostgresResul throw new \Error("The connection to the database has been closed"); } + \assert($this->types !== null, 'Expected type array to be populated before creating a result'); + switch (\pg_result_status($result)) { case \PGSQL_EMPTY_QUERY: throw new SqlQueryError("Empty query string"); From e793c96a8734dcb158d9ba2b5c10bfb40a018686 Mon Sep 17 00:00:00 2001 From: Aaron Piotrowski Date: Sun, 17 Nov 2024 09:32:49 -0600 Subject: [PATCH 3/5] Improve array parser performance --- src/Internal/ArrayParser.php | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src/Internal/ArrayParser.php b/src/Internal/ArrayParser.php index 35183e6..61c8b64 100644 --- a/src/Internal/ArrayParser.php +++ b/src/Internal/ArrayParser.php @@ -27,14 +27,14 @@ public static function parse(string $data, \Closure $cast, string $delimiter = ' { $data = \trim($data); - $parser = (new self($data, $cast, $delimiter))->parser(); - $data = \iterator_to_array($parser, false); + $parser = new self($data, $cast, $delimiter); + $result = $parser->parseToArray(); - if ($parser->getReturn() !== '') { + if ($parser->data !== '') { throw new PostgresParseException("Data left in buffer after parsing"); } - return $data; + return $result; } /** @@ -50,12 +50,14 @@ private function __construct( } /** - * Recursive generator parser yielding array values. + * @return list Parsed column data. * * @throws PostgresParseException */ - private function parser(): \Generator + private function parseToArray(): array { + $result = []; + if ($this->data === '') { throw new PostgresParseException("Unexpected end of data"); } @@ -72,13 +74,14 @@ private function parser(): \Generator } if ($this->data[0] === '}') { // Empty array - return \ltrim(\substr($this->data, 1)); + $this->data = \ltrim(\substr($this->data, 1)); + break; } if ($this->data[0] === '{') { // Array - $parser = (new self($this->data, $this->cast, $this->delimiter))->parser(); - yield \iterator_to_array($parser, false); - $this->data = $parser->getReturn(); + $parser = new self($this->data, $this->cast, $this->delimiter); + $result[] = $parser->parseToArray(); + $this->data = $parser->data; $end = $this->trim(0); continue; } @@ -113,15 +116,15 @@ private function parser(): \Generator $end = $this->trim($position); if (\strcasecmp($yield, "NULL") === 0) { // Literal NULL is always unquoted. - yield null; + $result[] = null; continue; } } - yield ($this->cast)($yield); + $result[] = ($this->cast)($yield); } while ($end !== '}'); - return $this->data; + return $result; } /** From 660c00fbeee036fbaf923e6bfb76844f06e59cfb Mon Sep 17 00:00:00 2001 From: Aaron Piotrowski Date: Tue, 19 Nov 2024 22:50:21 -0600 Subject: [PATCH 4/5] Reduce string manipulation --- src/Internal/ArrayParser.php | 82 +++++++++++++++++++++--------------- 1 file changed, 49 insertions(+), 33 deletions(-) diff --git a/src/Internal/ArrayParser.php b/src/Internal/ArrayParser.php index 61c8b64..a8e7cbd 100644 --- a/src/Internal/ArrayParser.php +++ b/src/Internal/ArrayParser.php @@ -14,6 +14,8 @@ final class ArrayParser use ForbidCloning; use ForbidSerialization; + private const WHITESPACE_CHARS = " \n\r\t\v\0"; + /** * @param string $data String representation of PostgresSQL array. * @param \Closure(string):mixed $cast Callback to cast parsed values. @@ -25,12 +27,10 @@ final class ArrayParser */ public static function parse(string $data, \Closure $cast, string $delimiter = ','): array { - $data = \trim($data); - $parser = new self($data, $cast, $delimiter); $result = $parser->parseToArray(); - if ($parser->data !== '') { + if (isset($parser->data[$parser->position])) { throw new PostgresParseException("Data left in buffer after parsing"); } @@ -43,9 +43,10 @@ public static function parse(string $data, \Closure $cast, string $delimiter = ' * @param string $delimiter Delimiter used to separate values. */ private function __construct( - private string $data, + private readonly string $data, private readonly \Closure $cast, - private readonly string $delimiter = ',', + private readonly string $delimiter, + private int $position = 0, ) { } @@ -58,36 +59,39 @@ private function parseToArray(): array { $result = []; - if ($this->data === '') { + $this->position = $this->skipWhitespace($this->position); + + if (!isset($this->data[$this->position])) { throw new PostgresParseException("Unexpected end of data"); } - if ($this->data[0] !== '{') { + if ($this->data[$this->position] !== '{') { throw new PostgresParseException("Missing opening bracket"); } - $this->data = \ltrim(\substr($this->data, 1)); + $this->position = $this->skipWhitespace($this->position + 1); do { - if ($this->data === '') { + if (!isset($this->data[$this->position])) { throw new PostgresParseException("Unexpected end of data"); } - if ($this->data[0] === '}') { // Empty array - $this->data = \ltrim(\substr($this->data, 1)); + if ($this->data[$this->position] === '}') { // Empty array + $this->position = $this->skipWhitespace($this->position + 1); break; } - if ($this->data[0] === '{') { // Array - $parser = new self($this->data, $this->cast, $this->delimiter); + if ($this->data[$this->position] === '{') { // Array + $parser = new self($this->data, $this->cast, $this->delimiter, $this->position); $result[] = $parser->parseToArray(); - $this->data = $parser->data; - $end = $this->trim(0); + $this->position = $parser->position; + $delimiter = $this->moveToNextDelimiter($this->position); continue; } - if ($this->data[0] === '"') { // Quoted value - for ($position = 1; isset($this->data[$position]); ++$position) { + if ($this->data[$this->position] === '"') { // Quoted value + ++$this->position; + for ($position = $this->position; isset($this->data[$position]); ++$position) { if ($this->data[$position] === '\\') { ++$position; // Skip next character continue; @@ -102,27 +106,30 @@ private function parseToArray(): array throw new PostgresParseException("Could not find matching quote in quoted value"); } - $yield = \stripslashes(\substr($this->data, 1, $position - 1)); + $entry = \stripslashes(\substr($this->data, $this->position, $position - $this->position)); - $end = $this->trim($position + 1); + $delimiter = $this->moveToNextDelimiter($position + 1); } else { // Unquoted value - $position = 0; - while (isset($this->data[$position]) && $this->data[$position] !== $this->delimiter && $this->data[$position] !== '}') { + $position = $this->position; + while (isset($this->data[$position]) + && $this->data[$position] !== $this->delimiter + && $this->data[$position] !== '}' + ) { ++$position; } - $yield = \trim(\substr($this->data, 0, $position)); + $entry = \trim(\substr($this->data, $this->position, $position - $this->position)); - $end = $this->trim($position); + $delimiter = $this->moveToNextDelimiter($position); - if (\strcasecmp($yield, "NULL") === 0) { // Literal NULL is always unquoted. + if (\strcasecmp($entry, "NULL") === 0) { // Literal NULL is always unquoted. $result[] = null; continue; } } - $result[] = ($this->cast)($yield); - } while ($end !== '}'); + $result[] = ($this->cast)($entry); + } while ($delimiter !== '}'); return $result; } @@ -134,22 +141,31 @@ private function parseToArray(): array * * @throws PostgresParseException */ - private function trim(int $position): string + private function moveToNextDelimiter(int $position): string { - $this->data = \ltrim(\substr($this->data, $position)); + $position = $this->skipWhitespace($position); - if ($this->data === '') { + if (!isset($this->data[$position])) { throw new PostgresParseException("Unexpected end of data"); } - $end = $this->data[0]; + $delimiter = $this->data[$position]; - if ($end !== $this->delimiter && $end !== '}') { + if ($delimiter !== $this->delimiter && $delimiter !== '}') { throw new PostgresParseException("Invalid delimiter"); } - $this->data = \ltrim(\substr($this->data, 1)); + $this->position = $this->skipWhitespace($position + 1); + + return $delimiter; + } + + private function skipWhitespace(int $position): int + { + while (isset($this->data[$position]) && \str_contains(self::WHITESPACE_CHARS, $this->data[$position])) { + ++$position; + } - return $end; + return $position; } } From c62555c6497782399a166f15c8da8e64be456141 Mon Sep 17 00:00:00 2001 From: Aaron Piotrowski Date: Wed, 20 Nov 2024 22:45:08 -0600 Subject: [PATCH 5/5] Remove unneeded arg --- src/Internal/PgSqlHandle.php | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/Internal/PgSqlHandle.php b/src/Internal/PgSqlHandle.php index f9cb262..b3077d2 100644 --- a/src/Internal/PgSqlHandle.php +++ b/src/Internal/PgSqlHandle.php @@ -316,7 +316,7 @@ private function createResult(\PgSql\Result $result, string $sql): PostgresResul \assert($this->types !== null, 'Expected type array to be populated before creating a result'); - switch (\pg_result_status($result)) { + switch ($status = \pg_result_status($result)) { case \PGSQL_EMPTY_QUERY: throw new SqlQueryError("Empty query string"); @@ -353,7 +353,11 @@ private function createResult(\PgSql\Result $result, string $sql): PostgresResul default: // @codeCoverageIgnoreStart $this->close(); - throw new SqlException("Unknown result status"); + throw new SqlException(\sprintf( + "Unknown result status: %d; error: %s", + $status, + \pg_result_error($result) ?: 'none', + )); // @codeCoverageIgnoreEnd } } @@ -471,7 +475,7 @@ public function prepare(string $sql): PostgresStatement $future = async(function () use ($name, $modifiedSql, $sql): string { $result = $this->send(\pg_send_prepare(...), $name, $modifiedSql); - switch (\pg_result_status($result, \PGSQL_STATUS_LONG)) { + switch ($status = \pg_result_status($result)) { case \PGSQL_COMMAND_OK: return $name; // Statement created successfully. @@ -488,7 +492,11 @@ public function prepare(string $sql): PostgresStatement default: // @codeCoverageIgnoreStart - throw new SqlException("Unknown result status"); + throw new SqlException(\sprintf( + "Unknown result status: %d; error: %s", + $status, + \pg_result_error($result) ?: 'none', + )); // @codeCoverageIgnoreEnd } });