Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add is_attach column to system.tables and disable index creation for attache tables #16166

Merged
merged 5 commits into from
Aug 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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' | '' |
Expand Down
15 changes: 15 additions & 0 deletions src/query/sql/src/planner/binder/ddl/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion src/query/storages/fuse/src/fuse_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
13 changes: 13 additions & 0 deletions src/query/storages/system/src/tables_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -518,6 +520,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())
Expand Down Expand Up @@ -550,6 +562,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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions tests/suites/5_ee/04_attach_read_only/02_0004_attach_table.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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;"

Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Loading