walterddr commented on code in PR #11732:
URL: https://github.com/apache/pinot/pull/11732#discussion_r1346145816
##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/queries/QueryRunnerTest.java:
##########
@@ -122,8 +122,9 @@ public void setUp()
_mailboxService.start();
QueryServerEnclosure server1 = new QueryServerEnclosure(factory1);
- QueryServerEnclosure server2 = new QueryServerEnclosure(factory2);
server1.start();
+ // Start server1 to ensure the next server will have a different port.
Review Comment:
what's this change for?
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/serde/ProtoSerializationUtils.java:
##########
@@ -148,7 +148,7 @@ private static Plan.MemberVariableField
serializeMemberVariable(Object fieldObje
} else if (fieldObject instanceof String) {
builder.setLiteralField(stringField((String) fieldObject));
} else if (fieldObject instanceof byte[]) {
- builder.setLiteralField(bytesField(ByteString.copyFrom((byte[])
fieldObject)));
+ builder.setLiteralField(bytesField((byte[]) fieldObject));
Review Comment:
this is still asymmetric. we should either
1. do not allow `instanceof byte[]` here
2. convert back to byte[] intead of ByteArray
i knew that some classes are all converted into bytes and cannot be
converted back to their original format, but we should explicitly structure
those --> if this will occur many special fields in the future, i propose:
encode the bytesField, with type enum and then the blob. for example
```
enum ProtoEncodedByteType {
byte[] --> 0
ByteArray --> 1
ByteString --> 2
BigDecimal --> 3
GeogrianCalendar --> 4
...
}
```
and have a ser/de util specific for bytes field
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/operands/LiteralOperand.java:
##########
@@ -28,7 +29,22 @@ public class LiteralOperand implements TransformOperand {
public LiteralOperand(RexExpression.Literal rexExpression) {
_resultType = rexExpression.getDataType();
- _value = rexExpression.getValue();
+ _value = convert(rexExpression.getValue());
+ }
+
+ /**
+ * Convert the value passed from the wire protocol to the expected type used
by the literal operator.
+ * E.g. for BYTES, convert byte[] to ByteArray.
+ */
+ private Object convert(Object value) {
+ if (_resultType == ColumnDataType.BYTES) {
+ if (value instanceof ByteArray) {
+ return value;
+ } else {
Review Comment:
check or assert this is actually `byte[]` ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
