refactor: remove udf from system functions table (databendlabs#14471)

* refactor: remove udf from system functions table Signed-off-by: Chojan Shang <[email protected]> * fix: try to make tests happy Signed-off-by: Chojan Shang <[email protected]> * fix: try to make tests happy Signed-off-by: Chojan Shang <[email protected]> --------- Signed-off-by: Chojan Shang <[email protected]>
guojidan · Jan 25, 2024 · 068d23c · 068d23c
1 parent a696f34
commit 068d23c
Show file tree

Hide file tree

Showing 7 changed files with 23 additions and 95 deletions.
diff --git 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::<Vec<_>>().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
@@ -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
@@ -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
@@ -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<dyn TableContext>,
+        _: Arc<dyn TableContext>,
         _push_downs: Option<PushDownInfo>,
     ) -> Result<DataBlock> {
         let mut scalar_func_names: Vec<String> = 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::<Vec<_>>()
-        } 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::<Vec<bool>>();
-
         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::<Vec<bool>>();
 
-        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::<Vec<String>>();
-
-        let categories = (0..names.len())
-            .map(|i| if i < builtin_func_len { "" } else { "UDF" })
-            .collect::<Vec<&str>>();
-
-        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::<Vec<&str>>();
-
-        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::<Vec<String>>();
+        let descriptions = (0..names.len()).map(|_| "").collect::<Vec<&str>>();
+
+        let syntaxes = (0..names.len()).map(|_| "").collect::<Vec<&str>>();
 
         let examples = (0..names.len()).map(|_| "").collect::<Vec<&str>>();
+
         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::<Vec<&str>>(),
-            ),
-            StringType::from_data(categories),
             StringType::from_data(descriptions),
-            StringType::from_data(syntaxes.iter().map(String::as_str).collect::<Vec<&str>>()),
+            StringType::from_data(syntaxes),
             StringType::from_data(examples),
         ]))
     }
@@ -158,10 +98,7 @@ impl FunctionsTable {
     pub fn create(table_id: u64) -> Arc<dyn Table> {
         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<dyn TableContext>) -> Result<Vec<UserDefinedFunction>> {
-        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
@@ -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
@@ -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
@@ -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