dawidwys commented on a change in pull request #10068:
[FLINK-13702][FLINK-13740] Fixed issues with generic types and materialization
of lazy binary formats in blink planner
URL: https://github.com/apache/flink/pull/10068#discussion_r342004922
##########
File path:
flink-table/flink-table-runtime-blink/src/main/java/org/apache/flink/table/dataformat/LazyBinaryFormat.java
##########
@@ -68,28 +72,43 @@ public void setJavaObject(T javaObject) {
@Override
public MemorySegment[] getSegments() {
- ensureMaterialized();
+ if (segments == null) {
+ throw new IllegalStateException("Lazy Binary Format was
not materialized");
+ }
return segments;
}
@Override
public int getOffset() {
- ensureMaterialized();
+ if (segments == null) {
+ throw new IllegalStateException("Lazy Binary Format was
not materialized");
+ }
return offset;
}
@Override
public int getSizeInBytes() {
- ensureMaterialized();
+ if (segments == null) {
+ throw new IllegalStateException("Lazy Binary Format was
not materialized");
+ }
return sizeInBytes;
}
/**
* Ensure we have materialized binary format.
*/
- public void ensureMaterialized() {
+ public final void ensureMaterialized(TypeSerializer<T> serializer) {
if (segments == null) {
- materialize();
+ try {
+ Binary binary = materialize(serializer);
+ this.offset = binary.offset;
+ this.sizeInBytes = binary.sizeInBytes;
+
+ // segments must be set last as this variable
tells if the object was materialized
Review comment:
Initially I didn't want to change it this way, because of how the class
hierarchy was organized. The problem is that the subclasses access the fields
of `BinaryFormat` directly. I tried to clean that up a little bit. Please take
a look at the updated PR.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
