fix: batch: compile warning that check a long for null #1408

[Ivyee17]

close #1408

[github-actions]

SDK Test Report

  75 files    75 suites   6m 9s ⏱️
163 tests 161 ✔️ 2 💤 0 ❌
204 runs  202 ✔️ 2 💤 0 ❌

Results for commit 3bce8ef.

♻️ This comment has been updated with latest results.

[github-actions]

Linux Test Report

       57 files       193 suites   1h 3m 15s ⏱️
  8 673 tests   8 669 ✔️ 4 💤 0 ❌
12 744 runs  12 740 ✔️ 4 💤 0 ❌

Results for commit 3bce8ef.

♻️ This comment has been updated with latest results.

[tobegit3hub]

There are some test failures. Please make sure you can run tests in local @Ivyee17 .

[Ivyee17]

May be I will try it once the issue is assigned to me. I misunderstand that this issue has already assigned to me. 😂

[dl239]

May be I will try it once the issue is assigned to me. I misunderstand that this issue has already assigned to me. 😂

hi @Ivyee17 the issue have been assigned to you.

[codecov]

Codecov Report

Merging #1870 (3bce8ef) into main (908e9ed) will increase coverage by 0.02%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main    #1870      +/-   ##
============================================
+ Coverage     75.69%   75.72%   +0.02%     
  Complexity      347      347              
============================================
  Files           613      613              
  Lines        117059   117059              
  Branches       1024     1008      -16     
============================================
+ Hits          88613    88640      +27     
+ Misses        28237    28210      -27     
  Partials        209      209              

Impacted Files	Coverage Δ
...4paradigm/openmldb/batch/nodes/WindowAggPlan.scala	63.87% <ø> (ø)
src/catalog/tablet_catalog.cc	73.77% <0.00%> (-0.87%)	⬇️
src/catalog/client_manager.cc	42.20% <0.00%> (-0.29%)	⬇️
hybridse/src/vm/runner.cc	67.75% <0.00%> (-0.04%)	⬇️
hybridse/src/codec/fe_row_codec.cc	78.64% <0.00%> (ø)
src/nameserver/name_server_impl.cc	42.86% <0.00%> (+0.01%)	⬆️
src/sdk/sql_cluster_router.cc	54.97% <0.00%> (+0.12%)	⬆️
src/tablet/tablet_impl.cc	59.44% <0.00%> (+0.17%)	⬆️
src/client/tablet_client.cc	55.33% <0.00%> (+0.22%)	⬆️
src/zk/zk_client.cc	84.07% <0.00%> (+0.52%)	⬆️
... and 1 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 908e9ed...3bce8ef. Read the comment docs.

[Ivyee17]

It has been correct now. @dl239

[Ivyee17]

If there is any question about this issue, feel free to contact me. @vagetablechicken @tobegit3hub

[vagetablechicken]

isValidOrder always check the orderKey from val orderKey = computer.extractKey(row).
But extractKey returns scala Long, you should make it return java Long too.

[Ivyee17]

There are some test failures. Please make sure you can run tests in local @Ivyee17 .

I've already made changes in the previous PR, it will cause test failures, as @dl239 said before. @vagetablechicken

[Ivyee17]

The failure is about delete-extract-key, if changing it to java.lang.Lang too, then this test will fail. Searching a key which is deleted before will be true, not false. I wonder if there's mistakes in extractKey function. Besides, I do think it's far beyond what should be done in this PR. I just need to resolve the warning log, not the complete process, right? Another issue and pr should be taken instead of this issue!!

[tobegit3hub]

LGTM

[vagetablechicken]

After check, the pr should only fix isValidOrder argument type. We want it to be java Long.
Actually we only pass scala Long to isValidOrder, the mistake will be tracked by another issue #1903 .