Skip to content

Commit

Permalink
[SPARK-46987][CONNECT] ProtoUtils.abbreviate avoid unnecessary `set…
Browse files Browse the repository at this point in the history
…Field` operation

### What changes were proposed in this pull request?
`ProtoUtils.abbreviate` avoid unnecessary `setField` operation

### Why are the changes needed?
according to the [API reference](https://protobuf.dev/reference/java/api-docs/com/google/protobuf/Message.html#toBuilder--):

> Message.Builder toBuilder()
Constructs a builder initialized with the current message. Use this to derive a new message from the current one.

the builder we used already has all the fields, so we only need to update the truncated fields.

### Does this PR introduce _any_ user-facing change?
no

### How was this patch tested?
ci

### Was this patch authored or co-authored using generative AI tooling?
no

Closes apache#45045 from zhengruifeng/connect_redaction_nit.

Authored-by: Ruifeng Zheng <ruifengz@apache.org>
Signed-off-by: yangjie01 <yangjie01@baidu.com>
zhengruifeng authored and LuciferYang committed Feb 7, 2024
1 parent 0bed573 commit a95aa7a
Showing 1 changed file with 1 addition and 7 deletions.
Original file line number Diff line number Diff line change
@@ -43,8 +43,6 @@ private[connect] object ProtoUtils {
val threshold = thresholds.getOrElse(STRING, MAX_STRING_SIZE)
if (size > threshold) {
builder.setField(field, createString(string.take(threshold), size))
} else {
builder.setField(field, string)
}

case (field: FieldDescriptor, byteString: ByteString)
@@ -57,8 +55,6 @@ private[connect] object ProtoUtils {
byteString
.substring(0, threshold)
.concat(createTruncatedByteString(size)))
} else {
builder.setField(field, byteString)
}

case (field: FieldDescriptor, byteArray: Array[Byte])
@@ -71,16 +67,14 @@ private[connect] object ProtoUtils {
ByteString
.copyFrom(byteArray, 0, threshold)
.concat(createTruncatedByteString(size)))
} else {
builder.setField(field, byteArray)
}

// TODO(SPARK-43117): should also support 1, repeated msg; 2, map<xxx, msg>
case (field: FieldDescriptor, msg: Message)
if field.getJavaType == FieldDescriptor.JavaType.MESSAGE && msg != null =>
builder.setField(field, abbreviate(msg, thresholds))

case (field: FieldDescriptor, value: Any) => builder.setField(field, value)
case _ =>
}

builder.build()

0 comments on commit a95aa7a

Please sign in to comment.