OpenMLDB icon indicating copy to clipboard operation
OpenMLDB copied to clipboard

batch: bug in handle key if is null

Open vagetablechicken opened this issue 3 years ago • 2 comments

https://github.com/4paradigm/OpenMLDB/blob/908e9edbe05a86dccdbf8d1579bad88454480d59/java/openmldb-batch/src/main/scala/com/_4paradigm/openmldb/batch/nodes/WindowAggPlan.scala#L424-L425

We get key by extractKey, and then check it by isValidOrder. After #1408 fixed, isValidOrder arg is java Long, it may be null or a long.

But extractKey returns scala Long, which can't be null, ref https://github.com/4paradigm/OpenMLDB/blob/8a20b4849c52dfee7dacd02f04a6adede05673c2/java/openmldb-batch/src/main/scala/com/_4paradigm/openmldb/batch/window/WindowComputer.scala#L225-L227 The function getLongFromIndex called by extractKey, returns java Long. https://github.com/4paradigm/OpenMLDB/blob/2e7fb76aca6a41a96c43b46b1be2194281c2166f/java/openmldb-batch/src/main/scala/com/_4paradigm/openmldb/batch/utils/SparkRowUtil.scala#L66

We miss null in extractKey. And after I temporarily make extractKey return java Long, some batch tests failed. e.g. https://github.com/4paradigm/OpenMLDB/blob/836b34119249ff9b667130b125c88395ef18ee4f/java/openmldb-batch/src/test/scala/com/_4paradigm/openmldb/batch/nulldata/TestWindowWithNullData.scala#L28

reproduce:

  1. make extractKey return java Long
  2. run TestWindowWithNullData

vagetablechicken avatar May 30 '22 09:05 vagetablechicken

@vagetablechicken this is fixed already?

lumianph avatar Jul 02 '22 03:07 lumianph

@vagetablechicken this is fixed already?

nope. isValidOrder has been fixed, but extractKey is not. And the tests will failed if we fix the extractKey, needs @tobegit3hub to check it.

vagetablechicken avatar Jul 03 '22 15:07 vagetablechicken