From ba190b9faeed8336f7652e057012be436ef36690 Mon Sep 17 00:00:00 2001 From: saranshdhingra Date: Wed, 24 Apr 2024 18:44:29 +0530 Subject: [PATCH 1/8] Improved latency for executeStreaminSql calls --- Spanner/src/Connection/Grpc.php | 87 ++++++++++++++++++++++ Spanner/tests/Unit/Connection/GrpcTest.php | 35 +++++++++ 2 files changed, 122 insertions(+) diff --git a/Spanner/src/Connection/Grpc.php b/Spanner/src/Connection/Grpc.php index ef357964b1b3..a3563b882e3a 100644 --- a/Spanner/src/Connection/Grpc.php +++ b/Spanner/src/Connection/Grpc.php @@ -55,6 +55,7 @@ use Google\Cloud\Spanner\V1\Mutation; use Google\Cloud\Spanner\V1\Mutation\Delete; use Google\Cloud\Spanner\V1\Mutation\Write; +use Google\Cloud\Spanner\V1\PartialResultSet; use Google\Cloud\Spanner\V1\PartitionOptions; use Google\Cloud\Spanner\V1\RequestOptions; use Google\Cloud\Spanner\V1\Session; @@ -68,6 +69,7 @@ use Google\Protobuf; use Google\Protobuf\FieldMask; use Google\Protobuf\GPBEmpty; +use Google\Protobuf\Internal\RepeatedField; use Google\Protobuf\ListValue; use Google\Protobuf\Struct; use Google\Protobuf\Value; @@ -235,6 +237,24 @@ public function __construct(array $config = []) 'google.protobuf.Timestamp' => function ($v) { return $this->formatTimestampFromApi($v); } + ], [], [], [ + // A custom encoder that short-circuits the encodeMessage in Serializer class, + // but only if the argument is of the type PartialResultSet. + PartialResultSet::class => function ($msg) { + $data = json_decode($msg->serializeToJsonString(), true); + + // The transaction id is serialized as a base64 encoded string in $data. So, we + // add a step to get the transaction id using a getter instead of the serialized value. + // The null-safe operator is used to handle edge cases where the relevant fields are not present. + $data['metadata']['transaction']['id'] = (string) $msg?->getMetadata()?->getTransaction()?->getId(); + + // Helps convert metadata enum values from string types to their respective code/annotation + // pairs. Ex: INT64 is converted to {code: 2, typeAnnotation: 0}. + $fields = $msg->getMetadata()?->getRowType()?->getFields(); + $data['metadata']['rowType']['fields'] = $this->getFieldDataFromRepeatedFields($fields); + + return $data; + } ]); //@codeCoverageIgnoreEnd @@ -1620,4 +1640,71 @@ private function setDirectedReadOptions(array &$args) ); } } + + /** + * Utiltiy method to take in a Google\Cloud\Spanner\V1\Type value and return + * the data as an array. The method takes care of array and struct elements. + * + * @param Type $type The "type" object + * + * @return array The formatted data. + */ + private function getTypeData(Type $type): array + { + $data = [ + 'code' => $type->getCode(), + 'typeAnnotation' => $type->getTypeAnnotation(), + 'protoTypeFqn' => $type->getProtoTypeFqn() + ]; + + // If this is a struct field, then recursisevly call getTypeData + if ($type->hasStructType()) { + $nestedType = $type->getStructType(); + $fields = $nestedType->getFields(); + $data['structType'] = [ + 'fields' => $this->getFieldDataFromRepeatedFields($fields) + ]; + } + // If this is an array field, then recursisevly call getTypeData + if ($type->hasArrayElementType()) { + $nestedType = $type->getArrayElementType(); + $data['arrayElementType'] = $this->getTypeData($nestedType); + } + + return $data; + } + + /** + * Utility method to return "fields data" in the format: + * [ + * "name" => "" + * "type" => [] + * ]. + * + * The type is converted from a string like INT64 to ["code" => 2, "typeAnnotation" => 0] + * conforming with the Google\Cloud\Spanner\V1\TypeCode class. + * + * @param ?RepeatedField $fields The array contain list of fields. + * + * @return array The formatted fields data. + */ + private function getFieldDataFromRepeatedFields(?RepeatedField $fields): array + { + if (is_null($fields)) { + return []; + } + + $fieldsData = []; + foreach ($fields as $key => $field) { + $type = $field->getType(); + $typeData = $this->getTypeData($type); + + $fieldsData[$key] = [ + 'name' => $field->getName(), + 'type' => $typeData + ]; + } + + return $fieldsData; + } } diff --git a/Spanner/tests/Unit/Connection/GrpcTest.php b/Spanner/tests/Unit/Connection/GrpcTest.php index b776ef4d42cc..badebcf27d81 100644 --- a/Spanner/tests/Unit/Connection/GrpcTest.php +++ b/Spanner/tests/Unit/Connection/GrpcTest.php @@ -21,6 +21,7 @@ use Google\ApiCore\CredentialsWrapper; use Google\ApiCore\OperationResponse; use Google\ApiCore\Serializer; +use Google\ApiCore\Testing\MockResponse; use Google\ApiCore\Transport\TransportInterface; use Google\Cloud\Core\GrpcRequestWrapper; use Google\Cloud\Core\GrpcTrait; @@ -1418,6 +1419,30 @@ public function larOptions() ]; } + public function testSerializerCustomEncoder() + { + $grpc = new GrpcStub(); + $msg = new MockResponse(['name' => 'foo']); + $serializer = new Serializer([], [], [], [], [ + MockResponse::class => function ($msg) { + return ['name' => 'bar']; + } + ]); + $grpc->setSerializer($serializer); + $arr = $grpc->callEncodeMessage($msg); + $this->assertEquals('bar', $arr['name']); + } + + public function testSerializerWithoutCustomEncoder() + { + $grpc = new GrpcStub(); + $msg = new MockResponse(['name' => 'foo']); + $serializer = new Serializer(); + $grpc->setSerializer($serializer); + $arr = $grpc->callEncodeMessage($msg); + $this->assertEquals('foo', $arr['name']); + } + private function assertCallCorrect( $method, array $args, @@ -1578,6 +1603,7 @@ private function transactionSelector() class GrpcStub extends Grpc { public $config; + private $serializer; protected function constructGapic($gapicName, array $config) { @@ -1585,5 +1611,14 @@ protected function constructGapic($gapicName, array $config) return parent::constructGapic($gapicName, $config); } + + public function callEncodeMessage($msg) { + return $this->serializer->encodeMessage($msg); + } + + public function setSerializer(Serializer $serializer) + { + $this->serializer = $serializer; + } } //@codingStandardsIgnoreEnd From d9322ca05100711ed7d0f805d3ecfba4285d50d2 Mon Sep 17 00:00:00 2001 From: saranshdhingra Date: Wed, 24 Apr 2024 19:41:20 +0530 Subject: [PATCH 2/8] Modified GAX version for tests --- Spanner/composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Spanner/composer.json b/Spanner/composer.json index 42ab1da8a32f..e8b93dda4ecb 100644 --- a/Spanner/composer.json +++ b/Spanner/composer.json @@ -7,7 +7,7 @@ "php": "^8.0", "ext-grpc": "*", "google/cloud-core": "^1.52.7", - "google/gax": "^1.30" + "google/gax": "dev-add-custom-encoders as 1.31" }, "require-dev": { "phpunit/phpunit": "^9.0", From 0a8c41762187e6ecebf1da0ea342bbb27ec26b61 Mon Sep 17 00:00:00 2001 From: saranshdhingra Date: Wed, 24 Apr 2024 21:27:05 +0530 Subject: [PATCH 3/8] Update GAX dependency to dev-main --- Spanner/composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Spanner/composer.json b/Spanner/composer.json index e8b93dda4ecb..ebe4ad76fe1a 100644 --- a/Spanner/composer.json +++ b/Spanner/composer.json @@ -7,7 +7,7 @@ "php": "^8.0", "ext-grpc": "*", "google/cloud-core": "^1.52.7", - "google/gax": "dev-add-custom-encoders as 1.31" + "google/gax": "dev-main as 1.31" }, "require-dev": { "phpunit/phpunit": "^9.0", From fdda755b76733aa0eab26a930aa7afc951191808 Mon Sep 17 00:00:00 2001 From: saranshdhingra Date: Thu, 25 Apr 2024 11:31:20 +0530 Subject: [PATCH 4/8] Increasing GAX version to run tests --- Spanner/composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Spanner/composer.json b/Spanner/composer.json index ebe4ad76fe1a..9517d536577f 100644 --- a/Spanner/composer.json +++ b/Spanner/composer.json @@ -7,7 +7,7 @@ "php": "^8.0", "ext-grpc": "*", "google/cloud-core": "^1.52.7", - "google/gax": "dev-main as 1.31" + "google/gax": "dev-main as 1.32" }, "require-dev": { "phpunit/phpunit": "^9.0", From babd51aa97901944a51fc03304f448f12ac006a6 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Thu, 25 Apr 2024 08:35:37 -0700 Subject: [PATCH 5/8] Update composer.json --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index ea6d094edf64..c58bd84baa6b 100644 --- a/composer.json +++ b/composer.json @@ -50,7 +50,7 @@ "monolog/monolog": "^2.9||^3.0", "psr/http-message": "^1.0|^2.0", "ramsey/uuid": "^4.0", - "google/gax": "^1.30", + "google/gax": "dev-main as 1.32", "google/common-protos": "^4.4", "google/auth": "^1.34" }, From 03e905dfbd08b06abdcf2e10e5a0a8a281d91fec Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Thu, 25 Apr 2024 08:46:24 -0700 Subject: [PATCH 6/8] update to latest GAX (1.32) --- Spanner/composer.json | 2 +- composer.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Spanner/composer.json b/Spanner/composer.json index 9517d536577f..379d4e2d1492 100644 --- a/Spanner/composer.json +++ b/Spanner/composer.json @@ -7,7 +7,7 @@ "php": "^8.0", "ext-grpc": "*", "google/cloud-core": "^1.52.7", - "google/gax": "dev-main as 1.32" + "google/gax": "^1.32" }, "require-dev": { "phpunit/phpunit": "^9.0", diff --git a/composer.json b/composer.json index 71dcd93b3957..fdeb7418e49c 100644 --- a/composer.json +++ b/composer.json @@ -50,7 +50,7 @@ "monolog/monolog": "^2.9||^3.0", "psr/http-message": "^1.0|^2.0", "ramsey/uuid": "^4.0", - "google/gax": "dev-main as 1.32", + "google/gax": "^1.32", "google/common-protos": "^4.4", "google/auth": "^1.34" }, From d8f4145367015c5a910bf0d9d8d99877e297b60d Mon Sep 17 00:00:00 2001 From: Saransh Dhingra Date: Fri, 26 Apr 2024 13:13:21 +0530 Subject: [PATCH 7/8] Update Spanner/tests/Unit/Connection/GrpcTest.php Update GrpcTest Co-authored-by: Brent Shaffer --- Spanner/tests/Unit/Connection/GrpcTest.php | 30 ++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/Spanner/tests/Unit/Connection/GrpcTest.php b/Spanner/tests/Unit/Connection/GrpcTest.php index badebcf27d81..1e440958fddb 100644 --- a/Spanner/tests/Unit/Connection/GrpcTest.php +++ b/Spanner/tests/Unit/Connection/GrpcTest.php @@ -1442,7 +1442,37 @@ public function testSerializerWithoutCustomEncoder() $arr = $grpc->callEncodeMessage($msg); $this->assertEquals('foo', $arr['name']); } + public function testPartialResultSetCustomEncoder() + { + $partialResultSet = new PartialResultSet(); + $partialResultSet->mergeFromJsonString(json_encode([ + 'metadata' => [ + 'transaction' => [ + 'id' => base64_encode(0b00010100) // bytedata is represented as a base64-encoded string in JSON + ], + 'rowType' => [ + 'fields' => [ + ['type' => ['code' => 'INT64']] // enums are represented as their string equivalents in JSON + ] + ], + ], + ])); + + $this->assertEquals(0b00010100, $partialResultSet->getMetadata()->getTransaction()->getId()); + $this->assertEquals(2, $partialResultSet->getMetadata()->getRowType()->getFields()[0]->getType()->getCode()); + // decode the message and ensure it's decoded as expected + $grpc = new Grpc(); + $serializerProp = new \ReflectionProperty($grpc, 'serializer'); + $serializerProp->setAccessible(true); + $serializer = $serializerProp->getValue($grpc); + $arr = $serializer->encodeMessage($partialResultSet); + + // We expect this to be the binary string + $this->assertEquals(0b00010100, $arr['metadata']['transaction']['id']); + // We expect this to be the integer + $this->assertEquals(2, $arr['metadata']['rowType']['fields'][0]['type']['code']); + } private function assertCallCorrect( $method, array $args, From 79b610d789544bb1469ff87199c22b6f4ccfa3d5 Mon Sep 17 00:00:00 2001 From: saranshdhingra Date: Fri, 26 Apr 2024 13:15:18 +0530 Subject: [PATCH 8/8] Addressed comments about GrpcTest --- Spanner/tests/Unit/Connection/GrpcTest.php | 34 +--------------------- 1 file changed, 1 insertion(+), 33 deletions(-) diff --git a/Spanner/tests/Unit/Connection/GrpcTest.php b/Spanner/tests/Unit/Connection/GrpcTest.php index 1e440958fddb..07822d51cea5 100644 --- a/Spanner/tests/Unit/Connection/GrpcTest.php +++ b/Spanner/tests/Unit/Connection/GrpcTest.php @@ -41,6 +41,7 @@ use Google\Cloud\Spanner\V1\Mutation; use Google\Cloud\Spanner\V1\Mutation\Delete; use Google\Cloud\Spanner\V1\Mutation\Write; +use Google\Cloud\Spanner\V1\PartialResultSet; use Google\Cloud\Spanner\V1\PartitionOptions; use Google\Cloud\Spanner\V1\RequestOptions; use Google\Cloud\Spanner\V1\Session; @@ -1419,29 +1420,6 @@ public function larOptions() ]; } - public function testSerializerCustomEncoder() - { - $grpc = new GrpcStub(); - $msg = new MockResponse(['name' => 'foo']); - $serializer = new Serializer([], [], [], [], [ - MockResponse::class => function ($msg) { - return ['name' => 'bar']; - } - ]); - $grpc->setSerializer($serializer); - $arr = $grpc->callEncodeMessage($msg); - $this->assertEquals('bar', $arr['name']); - } - - public function testSerializerWithoutCustomEncoder() - { - $grpc = new GrpcStub(); - $msg = new MockResponse(['name' => 'foo']); - $serializer = new Serializer(); - $grpc->setSerializer($serializer); - $arr = $grpc->callEncodeMessage($msg); - $this->assertEquals('foo', $arr['name']); - } public function testPartialResultSetCustomEncoder() { $partialResultSet = new PartialResultSet(); @@ -1633,7 +1611,6 @@ private function transactionSelector() class GrpcStub extends Grpc { public $config; - private $serializer; protected function constructGapic($gapicName, array $config) { @@ -1641,14 +1618,5 @@ protected function constructGapic($gapicName, array $config) return parent::constructGapic($gapicName, $config); } - - public function callEncodeMessage($msg) { - return $this->serializer->encodeMessage($msg); - } - - public function setSerializer(Serializer $serializer) - { - $this->serializer = $serializer; - } } //@codingStandardsIgnoreEnd