fix: delete data from associated pre-aggr table

[zhanghaohit]

• What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
  Fix delete key in pre-aggr table when delete occur in base table #1673

#2307 has to be fixed before this PR work correctly.

[codecov]

Codecov Report

Merging #2300 (1930963) into main (180f0e0) will decrease coverage by 0.01%.
The diff coverage is 96.46%.

@@             Coverage Diff              @@
##               main    #2300      +/-   ##
============================================
- Coverage     75.71%   75.70%   -0.02%     
  Complexity      368      368              
============================================
  Files           627      627              
  Lines        118168   118431     +263     
  Branches       1050     1022      -28     
============================================
+ Hits          89470    89657     +187     
- Misses        28482    28558      +76     
  Partials        216      216              

Impacted Files	Coverage Δ
src/storage/aggregator.h	100.00% <ø> (ø)
src/tablet/tablet_impl.cc	61.48% <58.33%> (-0.20%)	⬇️
src/storage/aggregator.cc	75.92% <89.65%> (+0.70%)	⬆️
src/tablet/tablet_impl_test.cc	99.53% <100.00%> (+0.01%)	⬆️
...4paradigm/openmldb/batch/utils/UnsafeRowUtil.scala	44.73% <0.00%> (-39.48%)	⬇️
...s/query_response_time/deploy_query_response_time.h	81.25% <0.00%> (-18.75%)	⬇️
.../com/_4paradigm/openmldb/batch/SparkRowCodec.scala	57.47% <0.00%> (-9.98%)	⬇️
...aradigm/openmldb/batch/window/WindowComputer.scala	75.92% <0.00%> (-5.81%)	⬇️
...atistics/query_response_time/query_response_time.h	78.26% <0.00%> (-4.35%)	⬇️
.../query_response_time/deploy_query_response_time.cc	97.10% <0.00%> (-2.90%)	⬇️
... and 21 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[github-actions]

SDK Test Report

  79 files    79 suites   7m 12s ⏱️
167 tests 165 ✔️ 2 💤 0 ❌
209 runs  207 ✔️ 2 💤 0 ❌

Results for commit 1930963.

♻️ This comment has been updated with latest results.

[github-actions]

Linux Test Report

       55 files       200 suites   51m 52s ⏱️
  9 255 tests   9 251 ✔️ 4 💤 0 ❌
13 616 runs  13 612 ✔️ 4 💤 0 ❌

Results for commit 1930963.

♻️ This comment has been updated with latest results.

[nautaa]

OpenMLDB/src/storage/aggregator.cc

Lines 359 to 363 in 309dc04

	// TODO(nauta): When the base table key is deleted, the pre-aggr table needs to be deleted at the same time.
	if (entry.has_method_type() && entry.method_type() == ::openmldb::api::MethodType::kDelete) {
	PDLOG(WARNING, "unsupport delete method for pre-aggr table");
	continue;
	}

There seems to be nothing to do, you can remove the warning log.

[zhanghaohit]

OpenMLDB/src/storage/aggregator.cc

Lines 359 to 363 in 309dc04

	// TODO(nauta): When the base table key is deleted, the pre-aggr table needs to be deleted at the same time.
	if (entry.has_method_type() && entry.method_type() == ::openmldb::api::MethodType::kDelete) {
	PDLOG(WARNING, "unsupport delete method for pre-aggr table");
	continue;
	}

There seems to be nothing to do, you can remove the warning log.

here if we found a kDelete entry, we shall do Delete(key) again, right? in case during the recover, we recover the key in the aggr_buffer_map_

[nautaa]

OpenMLDB/src/storage/aggregator.cc

Lines 359 to 363 in 309dc04

	// TODO(nauta): When the base table key is deleted, the pre-aggr table needs to be deleted at the same time.
	if (entry.has_method_type() && entry.method_type() == ::openmldb::api::MethodType::kDelete) {
	PDLOG(WARNING, "unsupport delete method for pre-aggr table");
	continue;
	}

There seems to be nothing to do, you can remove the warning log.

here if we found a kDelete entry, we shall do Delete(key) again, right? in case during the recover, we recover the key in the aggr_buffer_map_

yep, you are right. aggregator maybe update in recovery phase.

[zhanghaohit]

merged after 060 release

[wuyou10206]

checked