From 537dd462c52ee1f3570a09c01bc56458d9f0dd3c Mon Sep 17 00:00:00 2001 From: Kaitmazian Maksim <m.kaitmazian@picodata.io> Date: Thu, 27 Jun 2024 19:24:13 +0300 Subject: [PATCH] feat: generate metadata from the plan --- sbroad | 2 +- src/sql.rs | 12 +++++- test/int/test_sql.py | 87 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 3 deletions(-) diff --git a/sbroad b/sbroad index 6ec6d8836f..b1b021bff1 160000 --- a/sbroad +++ b/sbroad @@ -1 +1 @@ -Subproject commit 6ec6d8836fff0b590312fa0dd34d83610092cb18 +Subproject commit b1b021bff1ea23aa8d0fb74a0ed0fd409043a6bb diff --git a/src/sql.rs b/src/sql.rs index a2b7eacd4f..85b1ab56a7 100644 --- a/src/sql.rs +++ b/src/sql.rs @@ -23,7 +23,7 @@ use sbroad::errors::{Action, Entity, SbroadError}; use sbroad::executor::engine::helpers::{ build_delete_args, build_insert_args, build_update_args, decode_msgpack, init_delete_tuple_builder, init_insert_tuple_builder, init_local_update_tuple_builder, - normalize_name_for_space_api, + normalize_name_for_space_api, replace_metadata_in_dql_result, try_get_metadata_from_plan, }; use sbroad::executor::protocol::{EncodedRequiredData, RequiredData}; use sbroad::executor::result::ConsumerResult; @@ -263,6 +263,8 @@ pub fn dispatch(mut query: Query<RouterRuntime>) -> traft::Result<Tuple> { let plan = query.get_exec_plan().get_ir_plan(); check_table_privileges(plan)?; + let metadata = try_get_metadata_from_plan(query.get_exec_plan())?; + if query.is_explain() { return Ok(*query .produce_explain()? @@ -275,7 +277,7 @@ pub fn dispatch(mut query: Query<RouterRuntime>) -> traft::Result<Tuple> { return Ok(Tuple::new(&(res,))?); } - match query.dispatch() { + let tuple = match query.dispatch() { Ok(mut any_tuple) => { if let Some(tuple) = any_tuple.downcast_mut::<Tuple>() { debug!( @@ -293,7 +295,13 @@ pub fn dispatch(mut query: Query<RouterRuntime>) -> traft::Result<Tuple> { } } Err(e) => Err(Error::from(e)), + }?; + + // replace tarantool's metadata with the metadata from the plan + if let Some(metadata) = metadata { + return Ok(replace_metadata_in_dql_result(&tuple, &metadata)?); } + Ok(tuple) } } diff --git a/test/int/test_sql.py b/test/int/test_sql.py index e574cd8c81..0343133c77 100644 --- a/test/int/test_sql.py +++ b/test/int/test_sql.py @@ -4476,3 +4476,90 @@ cluster: select "id" from "_pico_table" """ ) + + +def test_metadata(instance: Instance): + ddl = instance.sql( + """ + create table t (a int not null, primary key (a)) + distributed by (a) + option (timeout = 3) + """ + ) + assert ddl["row_count"] == 1 + + data = instance.sql(""" select * from t """, strip_metadata=False) + assert data["metadata"] == [{"name": "A", "type": "integer"}] + + # Previously, we returned metadata from tarantool, not from the plan. + # Sometimes it led to disinformation, for example (pay attention to the types): + # + # picodata> select 1 from g + # --- + # - metadata: + # - {'name': 'COL_1', 'type': 'integer'} + # rows: [] + # ... + # + # picodata> explain select 1 from g + # --- + # - - projection (1::unsigned -> "COL_1") + # - ' scan "G"' + # - 'execution options:' + # - sql_vdbe_max_steps = 45000 + # - vtable_max_rows = 5000 + # ... + data = instance.sql(""" select 1 from t """, strip_metadata=False) + assert data["metadata"] == [{"name": "COL_1", "type": "unsigned"}] + + data = instance.sql(""" select -2 + 1 from t """, strip_metadata=False) + assert data["metadata"] == [{"name": "COL_1", "type": "integer"}] + + # Test that we can infer the actual type of min/max functions, which depends on the argument. + data = instance.sql(""" select min(a) from t """, strip_metadata=False) + assert data["metadata"] == [{"name": "COL_1", "type": "integer"}] + + data = instance.sql(""" select min(a) + max(a) from t """, strip_metadata=False) + assert data["metadata"] == [{"name": "COL_1", "type": "integer"}] + + # verify that we've fixed the problem from + # https://git.picodata.io/picodata/picodata/sbroad/-/issues/632 + ddl = instance.sql( + """ + CREATE TABLE "testing_space" ( "id" INTEGER NOT NULL, PRIMARY KEY ("id") ) + DISTRIBUTED BY ("id") + """ + ) + assert ddl["row_count"] == 1 + + ddl = instance.sql( + """ + CREATE TABLE "space_simple_shard_key" ( "id" INTEGER NOT NULL, PRIMARY KEY ("id") ) + DISTRIBUTED BY ("id") + """ + ) + assert ddl["row_count"] == 1 + + ddl = instance.sql( + """ + CREATE TABLE "space_simple_shard_key_hist" ( "id" INTEGER NOT NULL, PRIMARY KEY ("id") ) + DISTRIBUTED BY ("id") + """ + ) + assert ddl["row_count"] == 1 + + data = instance.sql( + """ + SELECT t1."id" as "id" FROM "testing_space" as t1 + JOIN "space_simple_shard_key" as t2 + ON t1."id" = t2."id" + JOIN "space_simple_shard_key_hist" as t3 + ON t2."id" = t3."id" + WHERE t1."id" = 1 + """, + strip_metadata=False, + ) + + # It used to return "T1.id" column name in metadata, + # though it should return "id" (because of an alias). + assert data["metadata"] == [{"name": "id", "type": "integer"}] -- GitLab