From 068d23c97d48007c00a5ad2962d67b68aa6f1e20 Mon Sep 17 00:00:00 2001 From: Chojan Shang Date: Thu, 25 Jan 2024 23:48:10 +0800 Subject: [PATCH] refactor: remove udf from system functions table (#14471) * refactor: remove udf from system functions table Signed-off-by: Chojan Shang * fix: try to make tests happy Signed-off-by: Chojan Shang * fix: try to make tests happy Signed-off-by: Chojan Shang --------- Signed-off-by: Chojan Shang --- src/query/service/tests/it/storages/system.rs | 2 +- .../it/storages/testdata/columns_table.txt | 3 - src/query/sql/src/planner/binder/show.rs | 2 +- .../storages/system/src/functions_table.rs | 83 ++----------------- .../base/06_show/06_0005_show_functions.test | 20 ++--- .../mode/standalone/explain/prune_column.test | 4 +- .../explain_native/prune_column.test | 4 +- 7 files changed, 23 insertions(+), 95 deletions(-) diff --git a/src/query/service/tests/it/storages/system.rs b/src/query/service/tests/it/storages/system.rs index 59528eabfe47..338fb6fae6dc 100644 --- a/src/query/service/tests/it/storages/system.rs +++ b/src/query/service/tests/it/storages/system.rs @@ -285,7 +285,7 @@ async fn test_functions_table() -> Result<()> { let stream = table.read_data_block_stream(ctx, &source_plan).await?; let result = stream.try_collect::>().await?; let block = &result[0]; - assert_eq!(block.num_columns(), 8); + assert_eq!(block.num_columns(), 5); Ok(()) } diff --git a/src/query/service/tests/it/storages/testdata/columns_table.txt b/src/query/service/tests/it/storages/testdata/columns_table.txt index e180ef687556..b792170f3634 100644 --- a/src/query/service/tests/it/storages/testdata/columns_table.txt +++ b/src/query/service/tests/it/storages/testdata/columns_table.txt @@ -27,7 +27,6 @@ DB.Table: 'system'.'columns', Table: columns-table_id:1, ver:0, Engine: SystemCo | 'catalog' | 'system' | 'tables' | 'String' | 'VARCHAR' | '' | '' | 'NO' | '' | | 'catalog' | 'system' | 'tables_with_history' | 'String' | 'VARCHAR' | '' | '' | 'NO' | '' | | 'catalog_name' | 'information_schema' | 'schemata' | 'String' | 'VARCHAR' | '' | '' | 'NO' | '' | -| 'category' | 'system' | 'functions' | 'String' | 'VARCHAR' | '' | '' | 'NO' | '' | | 'character_maximum_length' | 'information_schema' | 'columns' | 'NULL' | 'NULL' | '' | '' | 'NO' | '' | | 'character_octet_length' | 'information_schema' | 'columns' | 'NULL' | 'NULL' | '' | '' | 'NO' | '' | | 'character_set_catalog' | 'information_schema' | 'columns' | 'NULL' | 'NULL' | '' | '' | 'NO' | '' | @@ -114,7 +113,6 @@ DB.Table: 'system'.'columns', Table: columns-table_id:1, ver:0, Engine: SystemCo | 'default_expression' | 'system' | 'columns' | 'String' | 'VARCHAR' | '' | '' | 'NO' | '' | | 'default_kind' | 'system' | 'columns' | 'String' | 'VARCHAR' | '' | '' | 'NO' | '' | | 'default_role' | 'system' | 'users' | 'String' | 'VARCHAR' | '' | '' | 'NO' | '' | -| 'definition' | 'system' | 'functions' | 'String' | 'VARCHAR' | '' | '' | 'NO' | '' | | 'definition' | 'system' | 'indexes' | 'String' | 'VARCHAR' | '' | '' | 'NO' | '' | | 'definition' | 'system' | 'task_history' | 'String' | 'VARCHAR' | '' | '' | 'NO' | '' | | 'definition' | 'system' | 'tasks' | 'String' | 'VARCHAR' | '' | '' | 'NO' | '' | @@ -176,7 +174,6 @@ DB.Table: 'system'.'columns', Table: columns-table_id:1, ver:0, Engine: SystemCo | 'invalid_reason' | 'system' | 'streams' | 'String' | 'VARCHAR' | '' | '' | 'NO' | '' | | 'is_aggregate' | 'system' | 'functions' | 'Boolean' | 'BOOLEAN' | '' | '' | 'NO' | '' | | 'is_aggregate' | 'system' | 'user_functions' | 'Nullable(Boolean)' | 'BOOLEAN' | '' | '' | 'YES' | '' | -| 'is_builtin' | 'system' | 'functions' | 'Boolean' | 'BOOLEAN' | '' | '' | 'NO' | '' | | 'is_configured' | 'system' | 'users' | 'String' | 'VARCHAR' | '' | '' | 'NO' | '' | | 'is_insertable_into' | 'information_schema' | 'views' | 'Boolean' | 'BOOLEAN' | '' | '' | 'NO' | '' | | 'is_nullable' | 'information_schema' | 'columns' | 'String' | 'VARCHAR' | '' | '' | 'NO' | '' | diff --git a/src/query/sql/src/planner/binder/show.rs b/src/query/sql/src/planner/binder/show.rs index 8faff5116f80..5d1c46e09b7b 100644 --- a/src/query/sql/src/planner/binder/show.rs +++ b/src/query/sql/src/planner/binder/show.rs @@ -34,7 +34,7 @@ impl Binder { let (show_limit, limit_str) = get_show_options(show_options, None); // rewrite show functions to select * from system.functions ... let query = format!( - "SELECT name, is_builtin, is_aggregate, definition, description FROM system.functions {} ORDER BY name {}", + "SELECT name, is_aggregate, description FROM system.functions {} ORDER BY name {}", show_limit, limit_str, ); self.bind_rewrite_to_query(bind_context, &query, RewriteKind::ShowFunctions) diff --git a/src/query/storages/system/src/functions_table.rs b/src/query/storages/system/src/functions_table.rs index 90cc15bf93f1..9c9417346c71 100644 --- a/src/query/storages/system/src/functions_table.rs +++ b/src/query/storages/system/src/functions_table.rs @@ -27,12 +27,10 @@ use databend_common_expression::TableField; use databend_common_expression::TableSchemaRefExt; use databend_common_functions::aggregates::AggregateFunctionFactory; use databend_common_functions::BUILTIN_FUNCTIONS; -use databend_common_meta_app::principal::UserDefinedFunction; use databend_common_meta_app::schema::TableIdent; use databend_common_meta_app::schema::TableInfo; use databend_common_meta_app::schema::TableMeta; use databend_common_sql::TypeChecker; -use databend_common_users::UserApiProvider; use crate::table::AsyncOneBlockSystemTable; use crate::table::AsyncSystemTable; @@ -52,7 +50,7 @@ impl AsyncSystemTable for FunctionsTable { #[async_backtrace::framed] async fn get_full_data( &self, - ctx: Arc, + _: Arc, _push_downs: Option, ) -> Result { let mut scalar_func_names: Vec = BUILTIN_FUNCTIONS.registered_names(); @@ -69,86 +67,28 @@ impl AsyncSystemTable for FunctionsTable { scalar_func_names.sort(); let aggregate_function_factory = AggregateFunctionFactory::instance(); let aggr_func_names = aggregate_function_factory.registered_names(); - let enable_experimental_rbac_check = - ctx.get_settings().get_enable_experimental_rbac_check()?; - let udfs = if enable_experimental_rbac_check { - let visibility_checker = ctx.get_visibility_checker().await?; - let udfs = FunctionsTable::get_udfs(ctx).await?; - udfs.into_iter() - .filter(|udf| visibility_checker.check_udf_visibility(&udf.name)) - .collect::>() - } else { - FunctionsTable::get_udfs(ctx).await? - }; let names: Vec<&str> = scalar_func_names .iter() .chain(&aggr_func_names) - .chain(udfs.iter().map(|udf| &udf.name)) .map(|x| x.as_str()) .collect(); - let builtin_func_len = scalar_func_names.len() + aggr_func_names.len(); - - let is_builtin = (0..names.len()) - .map(|i| i < builtin_func_len) - .collect::>(); - let is_aggregate = (0..names.len()) - .map(|i| i >= scalar_func_names.len() && i < builtin_func_len) + .map(|i| i >= scalar_func_names.len()) .collect::>(); - let definitions = (0..names.len()) - .map(|i| { - if i < builtin_func_len { - "".to_string() - } else { - udfs.get(i - builtin_func_len) - .map_or("".to_string(), |udf| udf.definition.to_string()) - } - }) - .collect::>(); - - let categories = (0..names.len()) - .map(|i| if i < builtin_func_len { "" } else { "UDF" }) - .collect::>(); - - let descriptions = (0..names.len()) - .map(|i| { - if i < builtin_func_len { - "" - } else { - udfs.get(i - builtin_func_len) - .map_or("", |udf| udf.description.as_str()) - } - }) - .collect::>(); - - let syntaxes = (0..names.len()) - .map(|i| { - if i < builtin_func_len { - "".to_string() - } else { - udfs.get(i - builtin_func_len) - .map_or("".to_string(), |udf| udf.definition.to_string()) - } - }) - .collect::>(); + let descriptions = (0..names.len()).map(|_| "").collect::>(); + + let syntaxes = (0..names.len()).map(|_| "").collect::>(); let examples = (0..names.len()).map(|_| "").collect::>(); + Ok(DataBlock::new_from_columns(vec![ StringType::from_data(names), - BooleanType::from_data(is_builtin), BooleanType::from_data(is_aggregate), - StringType::from_data( - definitions - .iter() - .map(String::as_str) - .collect::>(), - ), - StringType::from_data(categories), StringType::from_data(descriptions), - StringType::from_data(syntaxes.iter().map(String::as_str).collect::>()), + StringType::from_data(syntaxes), StringType::from_data(examples), ])) } @@ -158,10 +98,7 @@ impl FunctionsTable { pub fn create(table_id: u64) -> Arc { let schema = TableSchemaRefExt::create(vec![ TableField::new("name", TableDataType::String), - TableField::new("is_builtin", TableDataType::Boolean), TableField::new("is_aggregate", TableDataType::Boolean), - TableField::new("definition", TableDataType::String), - TableField::new("category", TableDataType::String), TableField::new("description", TableDataType::String), TableField::new("syntax", TableDataType::String), TableField::new("example", TableDataType::String), @@ -182,10 +119,4 @@ impl FunctionsTable { AsyncOneBlockSystemTable::create(FunctionsTable { table_info }) } - - #[async_backtrace::framed] - async fn get_udfs(ctx: Arc) -> Result> { - let tenant = ctx.get_tenant(); - UserApiProvider::instance().get_udfs(&tenant).await - } } diff --git a/tests/sqllogictests/suites/base/06_show/06_0005_show_functions.test b/tests/sqllogictests/suites/base/06_show/06_0005_show_functions.test index 8a03593cc1ae..0bcc0f9b023f 100644 --- a/tests/sqllogictests/suites/base/06_show/06_0005_show_functions.test +++ b/tests/sqllogictests/suites/base/06_show/06_0005_show_functions.test @@ -1,25 +1,25 @@ -query TBBTT +query TBT SHOW FUNCTIONS LIKE 'today%' ---- -today 1 0 (empty) (empty) +today 0 (empty) -query TBBTT +query TBT SHOW FUNCTIONS LIKE 'to_day%' ---- -to_day_of_month 1 0 (empty) (empty) -to_day_of_week 1 0 (empty) (empty) -to_day_of_year 1 0 (empty) (empty) +to_day_of_month 0 (empty) +to_day_of_week 0 (empty) +to_day_of_year 0 (empty) -query TBBTT +query TBT SHOW FUNCTIONS LIKE 'to_day%' LIMIT 1 ---- -to_day_of_month 1 0 (empty) (empty) +to_day_of_month 0 (empty) -query TBBTT +query TBT SHOW FUNCTIONS WHERE name='to_day_of_year' LIMIT 1 ---- -to_day_of_year 1 0 (empty) (empty) +to_day_of_year 0 (empty) statement error SHOW FUNCTIONS WHERE mu='err' LIMIT 1 diff --git a/tests/sqllogictests/suites/mode/standalone/explain/prune_column.test b/tests/sqllogictests/suites/mode/standalone/explain/prune_column.test index a8e16b1a1603..9e694563c7e0 100644 --- a/tests/sqllogictests/suites/mode/standalone/explain/prune_column.test +++ b/tests/sqllogictests/suites/mode/standalone/explain/prune_column.test @@ -206,12 +206,12 @@ query T explain select name from system.functions order by example ---- Sort -├── output columns: [functions.name (#0), functions.example (#7)] +├── output columns: [functions.name (#0), functions.example (#4)] ├── sort keys: [example ASC NULLS LAST] ├── estimated rows: 0.00 └── TableScan ├── table: default.system.functions - ├── output columns: [name (#0), example (#7)] + ├── output columns: [name (#0), example (#4)] ├── read rows: 0 ├── read bytes: 0 ├── partitions total: 0 diff --git a/tests/sqllogictests/suites/mode/standalone/explain_native/prune_column.test b/tests/sqllogictests/suites/mode/standalone/explain_native/prune_column.test index a8571ac69ad6..5636413619d4 100644 --- a/tests/sqllogictests/suites/mode/standalone/explain_native/prune_column.test +++ b/tests/sqllogictests/suites/mode/standalone/explain_native/prune_column.test @@ -206,12 +206,12 @@ query T explain select name from system.functions order by example ---- Sort -├── output columns: [functions.name (#0), functions.example (#7)] +├── output columns: [functions.name (#0), functions.example (#4)] ├── sort keys: [example ASC NULLS LAST] ├── estimated rows: 0.00 └── TableScan ├── table: default.system.functions - ├── output columns: [name (#0), example (#7)] + ├── output columns: [name (#0), example (#4)] ├── read rows: 0 ├── read bytes: 0 ├── partitions total: 0