twalthr commented on a change in pull request #12346:
URL: https://github.com/apache/flink/pull/12346#discussion_r434346939
##########
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/gateway/local/result/MaterializedCollectStreamResult.java
##########
@@ -224,7 +224,7 @@ private void processDelete(Row row) {
}
for (int i = startSearchPos; i >= validRowPosition; i--) {
- if (materializedTable.get(i).equals(row)) {
+ if (materializedTable.get(i).fieldsEquals(row)) {
materializedTable.remove(i);
rowPositionCache.remove(row);
Review comment:
@zjffdu I looked into this topic again in depth. I think your fix is not
enough. Because `rowPositionCache.remove(row);` needs to ignore the row kind. I
wonder if we can fix the problem in a different layer. @godfreyhe how about we
erase the row kind for `Tuple2<Boolean, Row>`? In the future, we should have a
`tableEnv.toDataStream(ChangelogMode): Row` so the `Tuple2` case will not be
needed. Until then we should not encode the deletion twice in the tuple and in
the row. What do you think?
----------------------------------------------------------------
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]
