From 4fbb64071a91f2672c3105046defb2840b59391f Mon Sep 17 00:00:00 2001 From: "Lei, HUANG" Date: Fri, 30 Aug 2024 09:42:43 +0000 Subject: [PATCH 1/4] fix: avoid caching empty batches in row group --- src/mito2/src/read/last_row.rs | 5 ++ .../common/select/last_value.result | 69 +++++++++++++++++++ .../standalone/common/select/last_value.sql | 38 ++++++++++ 3 files changed, 112 insertions(+) create mode 100644 tests/cases/standalone/common/select/last_value.result create mode 100644 tests/cases/standalone/common/select/last_value.sql diff --git a/src/mito2/src/read/last_row.rs b/src/mito2/src/read/last_row.rs index 40767bc48314..57dfa946458a 100644 --- a/src/mito2/src/read/last_row.rs +++ b/src/mito2/src/read/last_row.rs @@ -17,6 +17,7 @@ use std::sync::Arc; use async_trait::async_trait; +use common_telemetry::info; use store_api::storage::TimeSeriesRowSelector; use crate::cache::{ @@ -213,6 +214,10 @@ impl RowGroupLastRowReader { /// Updates row group's last row cache if cache manager is present. fn maybe_update_cache(&mut self) { if let Some(cache) = &self.cache_manager { + if self.yielded_batches.len() == 0 { + // we always expect that row groups yields batches. + return; + } let value = Arc::new(SelectorResultValue { result: std::mem::take(&mut self.yielded_batches), projection: self diff --git a/tests/cases/standalone/common/select/last_value.result b/tests/cases/standalone/common/select/last_value.result new file mode 100644 index 000000000000..9c6411edb43f --- /dev/null +++ b/tests/cases/standalone/common/select/last_value.result @@ -0,0 +1,69 @@ +create table t ( + ts timestamp time index, + host string primary key, + not_pk string, + val double, +) with (append_mode='true'); + +Affected Rows: 0 + +insert into t values + (0, 'a', '🌕', 1.0), + (1, 'b', '🌖', 2.0), + (2, 'a', '🌗', 3.0), + (3, 'c', '🌘', 4.0), + (4, 'a', '🌑', 5.0), + (5, 'b', '🌒', 6.0), + (6, 'a', '🌓', 7.0), + (7, 'c', '🌔', 8.0), + (8, 'd', '🌕', 9.0); + +Affected Rows: 9 + +select flush_table('t'); + ++------------------------+ +| flush_table(Utf8("t")) | ++------------------------+ +| 0 | ++------------------------+ + +select + last_value(host order by ts), + last_value(not_pk order by ts), + last_value(val order by ts) +from t + group by host + order by host; + ++---------------------------------------------------+-----------------------------------------------------+--------------------------------------------------+ +| last_value(t.host) ORDER BY [t.ts ASC NULLS LAST] | last_value(t.not_pk) ORDER BY [t.ts ASC NULLS LAST] | last_value(t.val) ORDER BY [t.ts ASC NULLS LAST] | ++---------------------------------------------------+-----------------------------------------------------+--------------------------------------------------+ +| a | 🌓 | 7.0 | +| b | 🌒 | 6.0 | +| c | 🌔 | 8.0 | +| d | 🌕 | 9.0 | ++---------------------------------------------------+-----------------------------------------------------+--------------------------------------------------+ + +-- repeat the query again, ref: https://github.com/GreptimeTeam/greptimedb/issues/4650 +select + last_value(host order by ts), + last_value(not_pk order by ts), + last_value(val order by ts) +from t + group by host + order by host; + ++---------------------------------------------------+-----------------------------------------------------+--------------------------------------------------+ +| last_value(t.host) ORDER BY [t.ts ASC NULLS LAST] | last_value(t.not_pk) ORDER BY [t.ts ASC NULLS LAST] | last_value(t.val) ORDER BY [t.ts ASC NULLS LAST] | ++---------------------------------------------------+-----------------------------------------------------+--------------------------------------------------+ +| a | 🌓 | 7.0 | +| b | 🌒 | 6.0 | +| c | 🌔 | 8.0 | +| d | 🌕 | 9.0 | ++---------------------------------------------------+-----------------------------------------------------+--------------------------------------------------+ + +drop table t; + +Affected Rows: 0 + diff --git a/tests/cases/standalone/common/select/last_value.sql b/tests/cases/standalone/common/select/last_value.sql new file mode 100644 index 000000000000..edaf4a97cbea --- /dev/null +++ b/tests/cases/standalone/common/select/last_value.sql @@ -0,0 +1,38 @@ +create table t ( + ts timestamp time index, + host string primary key, + not_pk string, + val double, +) with (append_mode='true'); + +insert into t values + (0, 'a', '🌕', 1.0), + (1, 'b', '🌖', 2.0), + (2, 'a', '🌗', 3.0), + (3, 'c', '🌘', 4.0), + (4, 'a', '🌑', 5.0), + (5, 'b', '🌒', 6.0), + (6, 'a', '🌓', 7.0), + (7, 'c', '🌔', 8.0), + (8, 'd', '🌕', 9.0); + +select flush_table('t'); + +select + last_value(host order by ts), + last_value(not_pk order by ts), + last_value(val order by ts) +from t + group by host + order by host; + +-- repeat the query again, ref: https://github.com/GreptimeTeam/greptimedb/issues/4650 +select + last_value(host order by ts), + last_value(not_pk order by ts), + last_value(val order by ts) +from t + group by host + order by host; + +drop table t; \ No newline at end of file From c5b089095e84675a5baed43063c6a44893ee881f Mon Sep 17 00:00:00 2001 From: "Lei, HUANG" Date: Fri, 30 Aug 2024 09:50:56 +0000 Subject: [PATCH 2/4] fix: clippy --- src/mito2/src/read/last_row.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/mito2/src/read/last_row.rs b/src/mito2/src/read/last_row.rs index 57dfa946458a..f40172c21d3b 100644 --- a/src/mito2/src/read/last_row.rs +++ b/src/mito2/src/read/last_row.rs @@ -17,7 +17,6 @@ use std::sync::Arc; use async_trait::async_trait; -use common_telemetry::info; use store_api::storage::TimeSeriesRowSelector; use crate::cache::{ @@ -214,7 +213,7 @@ impl RowGroupLastRowReader { /// Updates row group's last row cache if cache manager is present. fn maybe_update_cache(&mut self) { if let Some(cache) = &self.cache_manager { - if self.yielded_batches.len() == 0 { + if self.yielded_batches.is_empty() { // we always expect that row groups yields batches. return; } From 99c9ee0f63ba5a87d2539692111308c706771ed3 Mon Sep 17 00:00:00 2001 From: "Lei, HUANG" <6406592+v0y4g3r@users.noreply.github.com> Date: Fri, 30 Aug 2024 17:54:39 +0800 Subject: [PATCH 3/4] Update tests/cases/standalone/common/select/last_value.sql --- tests/cases/standalone/common/select/last_value.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cases/standalone/common/select/last_value.sql b/tests/cases/standalone/common/select/last_value.sql index edaf4a97cbea..6e73276d009f 100644 --- a/tests/cases/standalone/common/select/last_value.sql +++ b/tests/cases/standalone/common/select/last_value.sql @@ -35,4 +35,4 @@ from t group by host order by host; -drop table t; \ No newline at end of file +drop table t; From 9f428978bc7e3b1da5c502c7574c5a78a35971f4 Mon Sep 17 00:00:00 2001 From: "Lei, HUANG" Date: Fri, 30 Aug 2024 10:45:32 +0000 Subject: [PATCH 4/4] fix: sqlness --- tests/cases/standalone/common/select/last_value.result | 4 ++-- tests/cases/standalone/common/select/last_value.sql | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/cases/standalone/common/select/last_value.result b/tests/cases/standalone/common/select/last_value.result index 9c6411edb43f..d078e0aa36b5 100644 --- a/tests/cases/standalone/common/select/last_value.result +++ b/tests/cases/standalone/common/select/last_value.result @@ -20,10 +20,10 @@ insert into t values Affected Rows: 9 -select flush_table('t'); +admin flush_table('t'); +------------------------+ -| flush_table(Utf8("t")) | +| ADMIN flush_table('t') | +------------------------+ | 0 | +------------------------+ diff --git a/tests/cases/standalone/common/select/last_value.sql b/tests/cases/standalone/common/select/last_value.sql index 6e73276d009f..969c2ce1a1fd 100644 --- a/tests/cases/standalone/common/select/last_value.sql +++ b/tests/cases/standalone/common/select/last_value.sql @@ -16,7 +16,7 @@ insert into t values (7, 'c', '🌔', 8.0), (8, 'd', '🌕', 9.0); -select flush_table('t'); +admin flush_table('t'); select last_value(host order by ts),