From 834ae3e989e59dbcae038ee77a8c795256356669 Mon Sep 17 00:00:00 2001 From: yihong0618 Date: Sun, 19 Jan 2025 16:32:02 +0800 Subject: [PATCH 1/3] fix: vector function for PromQL need to ignore the time index also close #5392 Signed-off-by: yihong0618 --- src/query/src/promql/planner.rs | 7 +++++-- .../common/promql/set_operation.result | 17 +++++++++++++++++ .../standalone/common/promql/set_operation.sql | 4 ++++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/query/src/promql/planner.rs b/src/query/src/promql/planner.rs index bfdfb5981ae1..230511eb970b 100644 --- a/src/query/src/promql/planner.rs +++ b/src/query/src/promql/planner.rs @@ -2014,9 +2014,12 @@ impl PromPlanner { .collect::>() }; - // push time index column if it exist + // push time index column if it exists if let Some(time_index_column) = &self.ctx.time_index_column { - tag_columns.push(Column::from_name(time_index_column)); + // issue #5392 if only_join_time_index is true, we don't need to push time_index_column to tag_columns + if !only_join_time_index { + tag_columns.push(Column::from_name(time_index_column)); + } } let right = LogicalPlanBuilder::from(right) diff --git a/tests/cases/standalone/common/promql/set_operation.result b/tests/cases/standalone/common/promql/set_operation.result index 230ee5b566bc..605582e19543 100644 --- a/tests/cases/standalone/common/promql/set_operation.result +++ b/tests/cases/standalone/common/promql/set_operation.result @@ -311,6 +311,23 @@ tql eval (3000, 3000, '1s') http_requests AND IGNORING (g, instance, job) vector | 1970-01-01T00:50:00 | app | 1 | production | 600.0 | +---------------------+-----+----------+------------+----------------+ +-- https://github.com/GreptimeTeam/greptimedb/issues/5392 +-- SQLNESS SORT_RESULT 3 1 +tql eval (3000, 3000, '1s') vector(1) * http_requests; + ++-----+----------+------------+---------------------+------------------------------------------------+ +| job | instance | g | ts | .greptime_value * http_requests.greptime_value | ++-----+----------+------------+---------------------+------------------------------------------------+ +| api | 0 | canary | 1970-01-01T00:50:00 | 300.0 | +| api | 0 | production | 1970-01-01T00:50:00 | 100.0 | +| api | 1 | canary | 1970-01-01T00:50:00 | 400.0 | +| api | 1 | production | 1970-01-01T00:50:00 | 200.0 | +| app | 0 | canary | 1970-01-01T00:50:00 | 700.0 | +| app | 0 | production | 1970-01-01T00:50:00 | 500.0 | +| app | 1 | canary | 1970-01-01T00:50:00 | 800.0 | +| app | 1 | production | 1970-01-01T00:50:00 | 600.0 | ++-----+----------+------------+---------------------+------------------------------------------------+ + drop table http_requests; Affected Rows: 0 diff --git a/tests/cases/standalone/common/promql/set_operation.sql b/tests/cases/standalone/common/promql/set_operation.sql index a386ca96c40a..87189323fdad 100644 --- a/tests/cases/standalone/common/promql/set_operation.sql +++ b/tests/cases/standalone/common/promql/set_operation.sql @@ -165,6 +165,10 @@ tql eval (3000, 3000, '1s') http_requests AND ON (dummy) vector(1); -- SQLNESS SORT_RESULT 3 1 tql eval (3000, 3000, '1s') http_requests AND IGNORING (g, instance, job) vector(1); +-- https://github.com/GreptimeTeam/greptimedb/issues/5392 +-- SQLNESS SORT_RESULT 3 1 +tql eval (3000, 3000, '1s') vector(1) * http_requests; + drop table http_requests; drop table cpu_count; From dd1121ba6837fbe62caabd7a85ceba7c8d63ec65 Mon Sep 17 00:00:00 2001 From: yihong0618 Date: Sun, 19 Jan 2025 18:40:39 +0800 Subject: [PATCH 2/3] fix: do not affect scalar function Signed-off-by: yihong0618 --- src/query/src/promql/planner.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/query/src/promql/planner.rs b/src/query/src/promql/planner.rs index 230511eb970b..b354e4e5d90e 100644 --- a/src/query/src/promql/planner.rs +++ b/src/query/src/promql/planner.rs @@ -409,6 +409,10 @@ impl PromPlanner { } } let mut field_columns = left_field_columns.iter().zip(right_field_columns.iter()); + let is_vector_matching = (left_field_columns.len() == 1 + && left_field_columns[0] == GREPTIME_VALUE) + || (right_field_columns.len() == 1 && right_field_columns[0] == GREPTIME_VALUE); + let join_plan = self.join_on_non_field_columns( left_input, right_input, @@ -417,6 +421,7 @@ impl PromPlanner { // if left plan or right plan tag is empty, means case like `scalar(...) + host` or `host + scalar(...)` // under this case we only join on time index left_context.tag_columns.is_empty() || right_context.tag_columns.is_empty(), + is_vector_matching, )?; let join_plan_schema = join_plan.schema().clone(); @@ -2003,6 +2008,7 @@ impl PromPlanner { left_table_ref: TableReference, right_table_ref: TableReference, only_join_time_index: bool, + is_vector_matching: bool, ) -> Result { let mut tag_columns = if only_join_time_index { vec![] @@ -2016,8 +2022,8 @@ impl PromPlanner { // push time index column if it exists if let Some(time_index_column) = &self.ctx.time_index_column { - // issue #5392 if only_join_time_index is true, we don't need to push time_index_column to tag_columns - if !only_join_time_index { + // issue #5392 if is special vector function + if !is_vector_matching { tag_columns.push(Column::from_name(time_index_column)); } } From 5485b7715c050ebe58663bd338258910a7c90587 Mon Sep 17 00:00:00 2001 From: yihong0618 Date: Mon, 20 Jan 2025 08:40:58 +0800 Subject: [PATCH 3/3] fix: betteer name for it Signed-off-by: yihong0618 --- src/query/src/promql/planner.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/query/src/promql/planner.rs b/src/query/src/promql/planner.rs index b354e4e5d90e..48a88a2cb55f 100644 --- a/src/query/src/promql/planner.rs +++ b/src/query/src/promql/planner.rs @@ -409,7 +409,7 @@ impl PromPlanner { } } let mut field_columns = left_field_columns.iter().zip(right_field_columns.iter()); - let is_vector_matching = (left_field_columns.len() == 1 + let has_special_vector_function = (left_field_columns.len() == 1 && left_field_columns[0] == GREPTIME_VALUE) || (right_field_columns.len() == 1 && right_field_columns[0] == GREPTIME_VALUE); @@ -421,7 +421,7 @@ impl PromPlanner { // if left plan or right plan tag is empty, means case like `scalar(...) + host` or `host + scalar(...)` // under this case we only join on time index left_context.tag_columns.is_empty() || right_context.tag_columns.is_empty(), - is_vector_matching, + has_special_vector_function, )?; let join_plan_schema = join_plan.schema().clone(); @@ -2008,7 +2008,7 @@ impl PromPlanner { left_table_ref: TableReference, right_table_ref: TableReference, only_join_time_index: bool, - is_vector_matching: bool, + has_special_vector_function: bool, ) -> Result { let mut tag_columns = if only_join_time_index { vec![] @@ -2023,7 +2023,7 @@ impl PromPlanner { // push time index column if it exists if let Some(time_index_column) = &self.ctx.time_index_column { // issue #5392 if is special vector function - if !is_vector_matching { + if !has_special_vector_function { tag_columns.push(Column::from_name(time_index_column)); } }