feat: add is_attach column to system.tables and disable index cre…

…ation for attache tables (#16166) * new field `is_attach` in `system.tables` * prevent index creation on attach table * update ut golden file columns_table.txt * Tweak UT `test_simple_sql` Extract field `is_attache` has been added, thun adjust assertions * tweak test `18_0007_privilege_access.result`
databendlabs · Aug 2, 2024 · 6d664c9 · 6d664c9
1 parent cb33a8e
commit 6d664c9
Show file tree

Hide file tree

Showing 10 changed files with 48 additions and 4 deletions.
diff --git a/src/query/service/tests/it/servers/http/http_query_handlers.rs b/src/query/service/tests/it/servers/http/http_query_handlers.rs
@@ -292,7 +292,7 @@ async fn test_simple_sql() -> Result<()> {
     assert_eq!(result.state, ExecuteStateKind::Succeeded, "{:?}", result);
     assert_eq!(result.next_uri, Some(final_uri.clone()), "{:?}", result);
     assert_eq!(result.data.len(), 10, "{:?}", result);
-    assert_eq!(result.schema.len(), 19, "{:?}", result);
+    assert_eq!(result.schema.len(), 20, "{:?}", result);
 
     // get state
     let uri = result.stats_uri.unwrap();

diff --git a/src/query/service/tests/it/storages/testdata/columns_table.txt b/src/query/service/tests/it/storages/testdata/columns_table.txt
@@ -209,6 +209,8 @@ 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_attach'                       | 'system'             | 'tables'               | 'String'              | 'VARCHAR'           | ''       | ''       | 'NO'     | ''       |
+| 'is_attach'                       | 'system'             | 'tables_with_history'  | 'String'              | 'VARCHAR'           | ''       | ''       | '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/ddl/index.rs b/src/query/sql/src/planner/binder/ddl/index.rs
@@ -246,6 +246,13 @@ impl Binder {
         let table_entry = &tables[0];
         let table = table_entry.table();
 
+        if table.is_read_only() {
+            return Err(ErrorCode::UnsupportedIndex(format!(
+                "Table {} is read-only, creating index not allowed",
+                table.name()
+            )));
+        }
+
         if !table.support_index() {
             return Err(ErrorCode::UnsupportedIndex(format!(
                 "Table engine {} does not support create index",
@@ -416,6 +423,14 @@ impl Binder {
             self.normalize_object_identifier_triple(catalog, database, table);
 
         let table = self.ctx.get_table(&catalog, &database, &table).await?;
+
+        if table.is_read_only() {
+            return Err(ErrorCode::UnsupportedIndex(format!(
+                "Table {} is read-only, creating inverted index not allowed",
+                table.name()
+            )));
+        }
+
         if !table.support_index() {
             return Err(ErrorCode::UnsupportedIndex(format!(
                 "Table engine {} does not support create inverted index",

diff --git a/src/query/storages/fuse/src/fuse_table.rs b/src/query/storages/fuse/src/fuse_table.rs
@@ -449,7 +449,7 @@ impl FuseTable {
     }
 
     // Check if table is attached.
-    fn is_table_attached(table_meta_options: &BTreeMap<String, String>) -> bool {
+    pub fn is_table_attached(table_meta_options: &BTreeMap<String, String>) -> bool {
         table_meta_options
             .get(OPT_KEY_TABLE_ATTACHED_DATA_URI)
             .is_some()

diff --git a/src/query/storages/system/src/tables_table.rs b/src/query/storages/system/src/tables_table.rs
@@ -40,6 +40,7 @@ use databend_common_meta_app::schema::TableIdent;
 use databend_common_meta_app::schema::TableInfo;
 use databend_common_meta_app::schema::TableMeta;
 use databend_common_meta_app::tenant::Tenant;
+use databend_common_storages_fuse::FuseTable;
 use databend_common_storages_view::view_table::QUERY;
 use databend_common_users::GrantObjectVisibilityChecker;
 use databend_common_users::UserApiProvider;
@@ -144,6 +145,7 @@ where TablesTable<T, U>: HistoryAware
                 TableField::new("engine_full", TableDataType::String),
                 TableField::new("cluster_by", TableDataType::String),
                 TableField::new("is_transient", TableDataType::String),
+                TableField::new("is_attach", TableDataType::String),
                 TableField::new("created_on", TableDataType::Timestamp),
                 TableField::new(
                     "dropped_on",
@@ -520,6 +522,16 @@ where TablesTable<T, U>: HistoryAware
                 }
             })
             .collect();
+        let is_attach: Vec<String> = database_tables
+            .iter()
+            .map(|v| {
+                if FuseTable::is_table_attached(&v.get_table_info().meta.options) {
+                    "ATTACH".to_string()
+                } else {
+                    "".to_string()
+                }
+            })
+            .collect();
         let comment: Vec<String> = database_tables
             .iter()
             .map(|v| v.get_table_info().meta.comment.clone())
@@ -552,6 +564,7 @@ where TablesTable<T, U>: HistoryAware
                 StringType::from_data(engines_full),
                 StringType::from_data(cluster_bys),
                 StringType::from_data(is_transient),
+                StringType::from_data(is_attach),
                 TimestampType::from_data(created_on),
                 TimestampType::from_opt_data(dropped_on),
                 TimestampType::from_data(updated_on),

diff --git a/tests/suites/0_stateless/18_rbac/18_0007_privilege_access.result b/tests/suites/0_stateless/18_rbac/18_0007_privilege_access.result
@@ -69,6 +69,7 @@ dropped_on	TIMESTAMP	YES		NULL	NULL
 engine	VARCHAR	NO		NULL	NULL
 engine_full	VARCHAR	NO		NULL	NULL
 index_size	BIGINT UNSIGNED	YES		NULL	NULL
+is_attach	VARCHAR	NO		NULL	NULL
 is_transient	VARCHAR	NO		NULL	NULL
 name	VARCHAR	NO		NULL	NULL
 num_rows	BIGINT UNSIGNED	YES		NULL	NULL

diff --git a/tests/suites/5_ee/04_attach_read_only/02_0004_attach_table.result b/tests/suites/5_ee/04_attach_read_only/02_0004_attach_table.result
@@ -15,6 +15,10 @@
 1
 2
 <<<<
+#### select attach table from system.tables
+>>>> select is_attach from system.tables where name = 'table_to';
+ATTACH
+<<<<
 #### select attach table with self-defined connection
 >>>> select * from table_to2 order by a;
 0

diff --git a/tests/suites/5_ee/04_attach_read_only/02_0004_attach_table.sh b/tests/suites/5_ee/04_attach_read_only/02_0004_attach_table.sh
@@ -37,6 +37,9 @@ echo "attach table table_to2 's3://testbucket/admin/data/$storage_prefix' connec
 comment "select attach table"
 query "select * from table_to order by a;"
 
+comment "select attach table from system.tables"
+query "select is_attach from system.tables where name = 'table_to';"
+
 comment "select attach table with self-defined connection"
 query "select * from table_to2 order by a;"
 

diff --git a/tests/suites/5_ee/04_attach_read_only/04_0001_check_mutations.result b/tests/suites/5_ee/04_attach_read_only/04_0001_check_mutations.result
@@ -1,3 +1,5 @@
+create index should fail
+Error: APIError: ResponseError with 1601: Table test_json_read_only is read-only, creating inverted index not allowed
 create virtual column should fail
 Error: APIError: ResponseError with 3905: Modification not permitted: Table 'test_json_read_only' is READ ONLY, preventing any changes or updates.
 alter virtual column should fail

diff --git a/tests/suites/5_ee/04_attach_read_only/04_0001_check_mutations.sh b/tests/suites/5_ee/04_attach_read_only/04_0001_check_mutations.sh
@@ -3,15 +3,19 @@
 CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
 . "$CURDIR"/../../../shell_env.sh
 
-echo "create database if not exists test_attach_only;" | $BENDSQL_CLIENT_CONNECT
+echo "create or replace database test_attach_only;" | $BENDSQL_CLIENT_CONNECT
 
 # mutation related enterprise features
 
-echo "create table test_attach_only.test_json(id int, val json) 's3://testbucket/admin/data/' connection=(access_key_id ='minioadmin' secret_access_key ='minioadmin' endpoint_url='${STORAGE_S3_ENDPOINT_URL}');" | $BENDSQL_CLIENT_CONNECT
+echo "create or replace table test_attach_only.test_json(id int, val json) 's3://testbucket/admin/data/' connection=(access_key_id ='minioadmin' secret_access_key ='minioadmin' endpoint_url='${STORAGE_S3_ENDPOINT_URL}');" | $BENDSQL_CLIENT_CONNECT
 echo "insert into test_attach_only.test_json values(1, '{\"a\":33,\"b\":44}'),(2, '{\"a\":55,\"b\":66}')" | $BENDSQL_CLIENT_CONNECT
 storage_prefix=$(mysql -uroot -h127.0.0.1 -P3307  -e "set global hide_options_in_show_create_table=0;show create table test_attach_only.test_json" | grep -i snapshot_location | awk -F'SNAPSHOT_LOCATION='"'"'|_ss' '{print $2}')
 echo "attach table test_attach_only.test_json_read_only 's3://testbucket/admin/data/$storage_prefix' connection=(access_key_id ='minioadmin' secret_access_key ='minioadmin' endpoint_url='${STORAGE_S3_ENDPOINT_URL}');" | $BENDSQL_CLIENT_CONNECT
 
+
+echo "create index should fail"
+echo "CREATE INVERTED INDEX IF NOT EXISTS idx1 ON test_attach_only.test_json_read_only(val)" | $BENDSQL_CLIENT_CONNECT
+
 echo "create virtual column should fail"
 echo "CREATE VIRTUAL COLUMN (val['a'], val['b']) FOR test_attach_only.test_json_read_only" | $BENDSQL_CLIENT_CONNECT