[multistage] Fixing the bytes literal usage in leaf stage#11732
[multistage] Fixing the bytes literal usage in leaf stage#11732xiangfu0 wants to merge 1 commit intoapache:masterfrom
Conversation
d145798 to
de1fe30
Compare
Codecov Report
@@ Coverage Diff @@
## master #11732 +/- ##
=============================================
- Coverage 63.11% 14.48% -48.64%
+ Complexity 1115 201 -914
=============================================
Files 2342 2342
Lines 125899 125907 +8
Branches 19364 19367 +3
=============================================
- Hits 79464 18237 -61227
- Misses 40780 106120 +65340
+ Partials 5655 1550 -4105
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1514 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
d89d213 to
1f1b572
Compare
...uery-planner/src/main/java/org/apache/pinot/query/planner/serde/ProtoSerializationUtils.java
Outdated
Show resolved
Hide resolved
4424df9 to
e7290eb
Compare
e7290eb to
4db5ef6
Compare
...y-runtime/src/main/java/org/apache/pinot/query/runtime/operator/operands/LiteralOperand.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
I saw port AlreadyBind issue from here in the tests.
...uery-planner/src/main/java/org/apache/pinot/query/planner/serde/ProtoSerializationUtils.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
We want to convert the value to the internal type. I think TIMESTAMP might not be handled correctly right now, where the raw value is not really the external value format Timestamp, so we may add a special case for that and add a TODO to fix that in the future
| _value = convert(rexExpression.getValue()); | |
| _value = _resultType.toInternal(rexExpression.getValue()); |
There was a problem hiding this comment.
+1. this and the rest of the places, ProtoSer/De only provide an identical mapping from ser/de perspective. for this particular situation, we need to leverage the RexExpression.Literal structure, which not only encode the value but also the type
we also need to make sure that proto ser/de only accepts particular type -->
if the data is ByteArray, byte[], or ByteString which can all ser/de into ByteString we should not allow that --> e.g. need to make sure the Ser/De is symmetrics
particularly in this case we should not allow the object to be GeogeorianCalendar --> it shouldl explicitly converted into long before setting it into the Literal
There was a problem hiding this comment.
Changed the proto serde to generate internal data type directly
8b2ddfe to
5923197
Compare
5923197 to
a52c914
Compare
The literal bytes passed from planner is a wrapped proto ByteString. This should be converted to Pinot internal ByteArray at inside
LiteralOperandto ensure the bytes RexExpression returns same internal type.