[#11720] DocDB: Use arena for PgConstant

Summary: This diff adds arena to PgStatement and switches PgConstant to LWQLValuePB to avoid superfluous conversions.

Test Plan: Jenkins

Reviewers: lnguyen

Reviewed By: lnguyen

Subscribers: ybase, bogdan

Differential Revision: https://phabricator.dev.yugabyte.com/D16220

src/yb/common/CMakeLists.txt

@@ -44,7 +44,7 @@ YRPC_GENERATE(
       value.proto wire_protocol.proto)
 ADD_YB_LIBRARY(yb_common_proto
   SRCS ${COMMON_PROTO_SRCS}
-  DEPS protobuf yrpc_base
+  DEPS protobuf yrpc_base rpc_base_proto
   NONLINK_DEPS ${COMMON_PROTO_TGTS})
 
 set(COMMON_BASE_SRCS

src/yb/common/partition.cc

@@ -1301,12 +1301,12 @@ YBHashSchema PartitionSchema::hash_schema() const {
   return *hash_schema_;
 }
 
-void PartitionSchema::ProcessHashKeyEntry(const QLValuePB& value_pb, std::string* out) {
+void PartitionSchema::ProcessHashKeyEntry(const LWQLValuePB& value_pb, std::string* out) {
   AppendToKey(value_pb, out);
 }
 
 void PartitionSchema::ProcessHashKeyEntry(const LWPgsqlExpressionPB& expr, std::string* out) {
-  AppendToKey(expr.value().ToGoogleProtobuf(), out);
+  AppendToKey(expr.value(), out);
 }
 
 void PartitionSchema::ProcessHashKeyEntry(const PgsqlExpressionPB& expr, std::string* out) {

src/yb/common/partition.h

@@ -254,7 +254,14 @@ class PartitionSchema {
 
   static bool IsValidHashPartitionKeyBound(const std::string& partition_key);
 
-  static void ProcessHashKeyEntry(const QLValuePB& value_pb, std::string* out);
+  template <class T>
+  static void ProcessHashKeyEntry(const T* value_pb, std::string* out) {
+    if (value_pb) {
+      ProcessHashKeyEntry(*value_pb, out);
+    }
+  }
+
+  static void ProcessHashKeyEntry(const LWQLValuePB& value_pb, std::string* out);
   static void ProcessHashKeyEntry(const LWPgsqlExpressionPB& expr, std::string* out);
   static void ProcessHashKeyEntry(const PgsqlExpressionPB& expr, std::string* out);
 

src/yb/common/pgsql_protocol.proto

@@ -20,6 +20,7 @@ import "yb/common/common.proto";
 import "yb/common/common_types.proto";
 import "yb/common/transaction.proto";
 import "yb/common/value.proto";
+import "yb/rpc/lightweight_message.proto";
 
 //--------------------------------------------------------------------------------------------------
 // Expressions.
@@ -344,13 +345,13 @@ message PgsqlReadRequestPB {
   // - In general, hash_code should be set by PgGate, but for some reasons, when the field
   // "ybctid_column_value" is used, hash_code is set by "common" lib.
   optional uint32 hash_code = 4;
-  optional PgsqlExpressionPB ybctid_column_value = 20;
+  optional PgsqlExpressionPB ybctid_column_value = 20 [(rpc.lightweight_field).pointer = true];
 
   // Primary key.
   // - Partition columns are used to compute the hash code,
   //   e.g. for h1 = 1 AND h2 = 2 partition_column_values would be [1, 2].
   // - Range columns combining with partition columns are used for indexing.
-  repeated PgsqlExpressionPB partition_column_values = 5;
+  repeated PgsqlExpressionPB partition_column_values = 5 [(rpc.lightweight_field).pointer = true];
   repeated PgsqlExpressionPB range_column_values = 18;
 
   // For select using local secondary index: this request selects the ybbasectids to fetch the rows
@@ -405,7 +406,7 @@ message PgsqlReadRequestPB {
 
   //------------------------------------------------------------------------------------------------
   // Paging state retrieved from the last response.
-  optional PgsqlPagingStatePB paging_state = 14;
+  optional PgsqlPagingStatePB paging_state = 14 [(rpc.lightweight_field).pointer = true];
 
   // Return paging state when "limit" number of rows are returned? In case when "limit" is the
   // page size, this is set for PgsqlResponsePB to return the paging state for the next fetch.
@@ -450,7 +451,7 @@ message PgsqlReadRequestPB {
   optional bool is_for_backfill = 29 [default = false];
 
   // If present, the request is to collect random sample
-  optional PgsqlSamplingStatePB sampling_state = 30;
+  optional PgsqlSamplingStatePB sampling_state = 30 [(rpc.lightweight_field).pointer = true];
 
   // Instruction for BACKFILL operation from the tablet server.
   // This is the serialized-string of "message PgsqlBackfillSpecPB" which will be used by
@@ -503,7 +504,7 @@ message PgsqlResponsePB {
   optional int32 rows_data_sidecar = 4;
 
   // Paging state for continuing the read in the next QLReadRequestPB fetch.
-  optional PgsqlPagingStatePB paging_state = 5;
+  optional PgsqlPagingStatePB paging_state = 5 [(rpc.lightweight_field).pointer = true];
 
   // When client sends a request that has a batch of many arguments, server might process only a
   // subset of the arguments. Attribute "batch_arg_count" is to indicate how many arguments have
@@ -533,7 +534,7 @@ message PgsqlResponsePB {
   optional uint32 txn_error_code = 9;
 
   // Random sample state to hand over to the next block
-  optional PgsqlSamplingStatePB sampling_state = 11;
+  optional PgsqlSamplingStatePB sampling_state = 11 [(rpc.lightweight_field).pointer = true];
 
   // Instruction for BACKFILL operation from master/table server.
   // This is the serialized-string of "message PgsqlBackfillSpecPB".

src/yb/common/placement_info-test.cc

@@ -46,11 +46,10 @@ TEST(PlacementInfoTest, TestTablespaceJsonProcessing) {
       "[{\"cloud\":\"c1\",\"region\":\"r1\",\"zone\":\"z1\",\"min_num_replicas\":2},"
       "{\"cloud\":\"c2\",\"region\":\"r2\",\"zone\":\"z2\",\"min_num_replicas\":1}]}";
 
-  QLValuePB option, invalid_option;
-  option.set_string_value("replica_placement=" + valid_json);
-  invalid_option.set_string_value("read_replica_placement=" + valid_json);
+  auto option = "replica_placement=" + valid_json;
+  auto invalid_option = "read_replica_placement=" + valid_json;
 
-  vector<QLValuePB> options;
+  vector<std::string> options;
 
   // Negative tests.
   // 1. Empty input.
@@ -68,53 +67,51 @@ TEST(PlacementInfoTest, TestTablespaceJsonProcessing) {
 
   // 4. Empty json.
   options.clear();
-  QLValuePB opt_empty_value;
-  opt_empty_value.set_string_value("replica_placement=[{}]");
+  auto opt_empty_value = "replica_placement=[{}]";
   options.emplace_back(opt_empty_value);
   ASSERT_NOK(PlacementInfoConverter::FromQLValue(options));
 
   // 5. Missing num_replicas field.
   options.clear();
-  QLValuePB invalid_json_option;
-  invalid_json_option.set_string_value("replica_placement={\"placement_blocks\":"
-      "[{\"cloud\":\"c1\",\"region\":\"r1\",\"zone\":\"z1\",\"min_num_replicas\":3}]}");
+  auto invalid_json_option = "replica_placement={\"placement_blocks\":"
+      "[{\"cloud\":\"c1\",\"region\":\"r1\",\"zone\":\"z1\",\"min_num_replicas\":3}]}";
   options.emplace_back(invalid_json_option);
   ASSERT_NOK(PlacementInfoConverter::FromQLValue(options));
 
   // 6. Invalid value for num_replicas field.
   options.clear();
-  invalid_json_option.set_string_value(
+  invalid_json_option =
       "replica_placement={\"num_replicas\":\"abc\",\"placement_blocks\":"
-      "[{\"cloud\":\"c1\",\"region\":\"r1\",\"zone\":\"z1\",\"min_num_replicas\":3}]}");
+      "[{\"cloud\":\"c1\",\"region\":\"r1\",\"zone\":\"z1\",\"min_num_replicas\":3}]}";
   options.emplace_back(invalid_json_option);
   ASSERT_NOK(PlacementInfoConverter::FromQLValue(options));
 
   // 7. Missing placement blocks field.
   options.clear();
-  invalid_json_option.set_string_value("replica_placement={\"num_replicas\":3}");
+  invalid_json_option = "replica_placement={\"num_replicas\":3}";
   options.emplace_back(invalid_json_option);
   ASSERT_NOK(PlacementInfoConverter::FromQLValue(options));
 
   // 8. Missing keys in placement blocks.
   options.clear();
-  invalid_json_option.set_string_value(
+  invalid_json_option =
       "replica_placement={\"num_replicas\":\"abc\",\"placement_blocks\":"
-      "[{\"cloud\":\"c1\",\"region\":\"r1\",\"zone\":\"z1\"}]}");
+      "[{\"cloud\":\"c1\",\"region\":\"r1\",\"zone\":\"z1\"}]}";
   options.emplace_back(invalid_json_option);
   ASSERT_NOK(PlacementInfoConverter::FromQLValue(options));
 
   // 9. Invalid format for "min_num_replicas".
   options.clear();
-  invalid_json_option.set_string_value(
-        "replica_placement={\"num_replicas\":3,\"placement_blocks\":"
-        "[{\"cloud\":\"c1\",\"region\":\"r1\",\"zone\":\"z1\",\"min_num_replicas\":\"abc\"}]}");
+  invalid_json_option =
+      "replica_placement={\"num_replicas\":3,\"placement_blocks\":"
+      "[{\"cloud\":\"c1\",\"region\":\"r1\",\"zone\":\"z1\",\"min_num_replicas\":\"abc\"}]}";
   options.emplace_back(invalid_json_option);
   ASSERT_NOK(PlacementInfoConverter::FromQLValue(options));
 
   // 10. Invalid json.
   options.clear();
-  invalid_json_option.set_string_value("replica_placement=["
-      "{\"cloud\":\"c1\",\"region\":\"r1\",\"zone\":\"z1\",\"min_number_of_replica");
+  invalid_json_option = "replica_placement=["
+      "{\"cloud\":\"c1\",\"region\":\"r1\",\"zone\":\"z1\",\"min_number_of_replica";
   options.emplace_back(invalid_json_option);
   ASSERT_NOK(PlacementInfoConverter::FromQLValue(options));
 

src/yb/common/placement_info.cc

@@ -124,15 +124,14 @@ Result<PlacementInfoConverter::Placement> PlacementInfoConverter::FromString(
 }
 
 Result<PlacementInfoConverter::Placement> PlacementInfoConverter::FromQLValue(
-    const vector<QLValuePB>& placement) {
+    const std::vector<std::string>& placement) {
   // Today only one option is supported, so this array should have only one option.
   if (placement.size() != 1) {
     return STATUS_FORMAT(Corruption,
                          "Unexpected number of options: $0", placement.size());
   }
 
-  const string& option = placement[0].string_value();
-  return FromString(option);
+  return FromString(placement.front());
 }
 
 } // namespace yb

src/yb/common/placement_info.h

@@ -42,7 +42,7 @@ class PlacementInfoConverter {
 
   static Result<Placement> FromString(const std::string& placement);
 
-  static Result<Placement> FromQLValue(const vector<QLValuePB>& placement);
+  static Result<Placement> FromQLValue(const std::vector<std::string>& placement);
 
  private:
   static Result<Placement> FromJson(const std::string& placement_str,

src/yb/common/ql_value.cc

@@ -137,10 +137,21 @@ int QLValue::CompareTo(const QLValue& other) const {
   return 0;
 }
 
+namespace {
+
+void AppendBytesToKey(const std::string& str, string* bytes) {
+  YBPartition::AppendBytesToKey(str.c_str(), str.length(), bytes);
+}
+
+void AppendBytesToKey(const Slice& str, string* bytes) {
+  YBPartition::AppendBytesToKey(str.cdata(), str.size(), bytes);
+}
+
 // TODO(mihnea) After the hash changes, this method does not do the key encoding anymore
 // (not needed for hash computation), so AppendToBytes() is better describes what this method does.
 // The internal methods such as AppendIntToKey should be renamed accordingly.
-void AppendToKey(const QLValuePB &value_pb, string *bytes) {
+template <class PB>
+void DoAppendToKey(const PB& value_pb, string* bytes) {
   switch (value_pb.value_case()) {
     case InternalType::kBoolValue: {
       YBPartition::AppendIntToKey<bool, uint8>(value_pb.bool_value() ? 1 : 0, bytes);
@@ -183,38 +194,31 @@ void AppendToKey(const QLValuePB &value_pb, string *bytes) {
       break;
     }
     case InternalType::kStringValue: {
-      const string& str = value_pb.string_value();
-      YBPartition::AppendBytesToKey(str.c_str(), str.length(), bytes);
+      AppendBytesToKey(value_pb.string_value(), bytes);
       break;
     }
     case InternalType::kUuidValue: {
-      const string& str = value_pb.uuid_value();
-      YBPartition::AppendBytesToKey(str.c_str(), str.length(), bytes);
+      AppendBytesToKey(value_pb.uuid_value(), bytes);
       break;
     }
     case InternalType::kTimeuuidValue: {
-      const string& str = value_pb.timeuuid_value();
-      YBPartition::AppendBytesToKey(str.c_str(), str.length(), bytes);
+      AppendBytesToKey(value_pb.timeuuid_value(), bytes);
       break;
     }
     case InternalType::kInetaddressValue: {
-      const string& str = value_pb.inetaddress_value();
-      YBPartition::AppendBytesToKey(str.c_str(), str.length(), bytes);
+      AppendBytesToKey(value_pb.inetaddress_value(), bytes);
       break;
     }
     case InternalType::kDecimalValue: {
-      const string& str = value_pb.decimal_value();
-      YBPartition::AppendBytesToKey(str.c_str(), str.length(), bytes);
+      AppendBytesToKey(value_pb.decimal_value(), bytes);
       break;
     }
     case InternalType::kVarintValue: {
-      const string& str = value_pb.varint_value();
-      YBPartition::AppendBytesToKey(str.c_str(), str.length(), bytes);
+      AppendBytesToKey(value_pb.varint_value(), bytes);
       break;
     }
     case InternalType::kBinaryValue: {
-      const string& str = value_pb.binary_value();
-      YBPartition::AppendBytesToKey(str.c_str(), str.length(), bytes);
+      AppendBytesToKey(value_pb.binary_value(), bytes);
       break;
     }
     case InternalType::kFloatValue: {
@@ -252,6 +256,16 @@ void AppendToKey(const QLValuePB &value_pb, string *bytes) {
   }
 }
 
+} // namespace
+
+void AppendToKey(const QLValuePB &value_pb, std::string *bytes) {
+  DoAppendToKey(value_pb, bytes);
+}
+
+void AppendToKey(const LWQLValuePB &value_pb, std::string *bytes) {
+  DoAppendToKey(value_pb, bytes);
+}
+
 void QLValue::Serialize(
     const std::shared_ptr<QLType>& ql_type, const QLClient& client, const QLValuePB& pb,
     faststring* buffer) {
@@ -263,10 +277,10 @@ void QLValue::Serialize(
 
   switch (ql_type->main()) {
     case INT8:
-      CQLEncodeNum(Store8, int8_value(pb), buffer);
+      CQLEncodeNum(Store8, static_cast<uint8_t>(int8_value(pb)), buffer);
       return;
     case INT16:
-      CQLEncodeNum(NetworkByteOrder::Store16, int16_value(pb), buffer);
+      CQLEncodeNum(NetworkByteOrder::Store16, static_cast<uint16_t>(int16_value(pb)), buffer);
       return;
     case INT32:
       CQLEncodeNum(NetworkByteOrder::Store32, int32_value(pb), buffer);
@@ -894,7 +908,13 @@ string QLValue::ToString() const {
 
 InetAddress QLValue::inetaddress_value(const QLValuePB& pb) {
   InetAddress addr;
-  CHECK_OK(addr.FromBytes(inetaddress_value_pb(pb)));
+  CHECK_OK(addr.FromSlice(inetaddress_value_pb(pb)));
+  return addr;
+}
+
+InetAddress QLValue::inetaddress_value(const LWQLValuePB& pb) {
+  InetAddress addr;
+  CHECK_OK(addr.FromSlice(inetaddress_value_pb(pb)));
   return addr;
 }
 
@@ -905,12 +925,25 @@ Uuid QLValue::timeuuid_value(const QLValuePB& pb) {
   return timeuuid;
 }
 
+Uuid QLValue::timeuuid_value(const LWQLValuePB& pb) {
+  Uuid timeuuid;
+  CHECK_OK(timeuuid.FromSlice(timeuuid_value_pb(pb)));
+  CHECK_OK(timeuuid.IsTimeUuid());
+  return timeuuid;
+}
+
 Uuid QLValue::uuid_value(const QLValuePB& pb) {
   Uuid uuid;
   CHECK_OK(uuid.FromBytes(uuid_value_pb(pb)));
   return uuid;
 }
 
+Uuid QLValue::uuid_value(const LWQLValuePB& pb) {
+  Uuid uuid;
+  CHECK_OK(uuid.FromSlice(uuid_value_pb(pb)));
+  return uuid;
+}
+
 util::VarInt QLValue::varint_value(const QLValuePB& pb) {
   CHECK(pb.has_varint_value()) << "Value: " << pb.ShortDebugString();
   util::VarInt varint;
@@ -973,6 +1006,18 @@ QLValuePB QLValue::PrimitiveInt64(int64_t value) {
   return result;
 }
 
+Timestamp QLValue::timestamp_value(const QLValuePB& pb) {
+  return Timestamp(timestamp_value_pb(pb));
+}
+
+Timestamp QLValue::timestamp_value(const LWQLValuePB& pb) {
+  return Timestamp(timestamp_value_pb(pb));
+}
+
+Timestamp QLValue::timestamp_value(const QLValue& value) {
+  return timestamp_value(value.value());
+}
+
 //----------------------------------- QLValuePB operators --------------------------------
 
 InternalType type(const QLValuePB& v) {
@@ -986,7 +1031,6 @@ bool IsNull(const LWQLValuePB& v) {
   return v.value_case() == QLValuePB::VALUE_NOT_SET;
 }
 
-
 void SetNull(QLValuePB* v) {
   v->Clear();
 }
@@ -1109,9 +1153,9 @@ int Compare(const QLValuePB& lhs, const QLValue& rhs) {
   CHECK(BothNotNull(lhs, rhs));
   switch (type(lhs)) {
     case QLValuePB::kInt8Value:
-      return GenericCompare(static_cast<int8_t>(lhs.int8_value()), rhs.int8_value());
+      return GenericCompare<int8_t>(lhs.int8_value(), rhs.int8_value());
     case QLValuePB::kInt16Value:
-      return GenericCompare(static_cast<int16_t>(lhs.int16_value()), rhs.int16_value());
+      return GenericCompare<int16_t>(lhs.int16_value(), rhs.int16_value());
     case QLValuePB::kInt32Value:  return GenericCompare(lhs.int32_value(), rhs.int32_value());
     case QLValuePB::kInt64Value:  return GenericCompare(lhs.int64_value(), rhs.int64_value());
     case QLValuePB::kUint32Value:  return GenericCompare(lhs.uint32_value(), rhs.uint32_value());
@@ -1168,7 +1212,7 @@ int Compare(const QLValuePB& lhs, const QLValue& rhs) {
       }
       break;
     case QLValuePB::kGinNullValue:
-      return GenericCompare(static_cast<uint8_t>(lhs.gin_null_value()), rhs.gin_null_value());
+      return GenericCompare<uint8_t>(lhs.gin_null_value(), rhs.gin_null_value());
 
     // default: fall through
   }