From c4db9e8aa752faa7fe817ecac6e41e3b36b3f0e4 Mon Sep 17 00:00:00 2001 From: Yohan Wal <59358312+CookiePieWw@users.noreply.github.com> Date: Wed, 3 Jul 2024 11:09:23 +0800 Subject: [PATCH] fix!: forbid to change information_schema (#4233) * fix: forbid to change tables in information_schema * refactor: use unified read-only check function * test: add more sqlness tests for information_schema * refactor: move is_readonly_schema to common_catalog --- src/common/catalog/src/consts.rs | 4 ++ src/operator/src/error.rs | 13 +++++-- .../src/req_convert/insert/stmt_to_region.rs | 7 +++- src/operator/src/statement/ddl.rs | 39 +++++++++++++++++-- src/query/src/datafusion.rs | 24 ++++++++++-- src/query/src/error.rs | 8 ++++ .../information_schema/region_peers.result | 10 +++-- .../information_schema/region_peers.sql | 10 ++--- .../table_constraints.result | 38 +++++++++++------- .../information_schema/table_constraints.sql | 8 +++- .../common/information_schema/tables.result | 36 +++++++++++++++++ .../common/information_schema/tables.sql | 18 +++++++++ 12 files changed, 179 insertions(+), 36 deletions(-) diff --git a/src/common/catalog/src/consts.rs b/src/common/catalog/src/consts.rs index c039d4dbf62b..876d4bcb67f4 100644 --- a/src/common/catalog/src/consts.rs +++ b/src/common/catalog/src/consts.rs @@ -108,3 +108,7 @@ pub const FILE_ENGINE: &str = "file"; pub const SEMANTIC_TYPE_PRIMARY_KEY: &str = "TAG"; pub const SEMANTIC_TYPE_FIELD: &str = "FIELD"; pub const SEMANTIC_TYPE_TIME_INDEX: &str = "TIMESTAMP"; + +pub fn is_readonly_schema(schema: &str) -> bool { + matches!(schema, INFORMATION_SCHEMA_NAME) +} diff --git a/src/operator/src/error.rs b/src/operator/src/error.rs index ab035f90a84f..a521ae26e0e2 100644 --- a/src/operator/src/error.rs +++ b/src/operator/src/error.rs @@ -269,6 +269,13 @@ pub enum Error { location: Location, }, + #[snafu(display("Schema `{name}` is read-only"))] + SchemaReadOnly { + name: String, + #[snafu(implicit)] + location: Location, + }, + #[snafu(display("Table occurs error"))] Table { #[snafu(implicit)] @@ -741,9 +748,9 @@ impl ErrorExt for Error { StatusCode::TableAlreadyExists } - Error::NotSupported { .. } | Error::ShowCreateTableBaseOnly { .. } => { - StatusCode::Unsupported - } + Error::NotSupported { .. } + | Error::ShowCreateTableBaseOnly { .. } + | Error::SchemaReadOnly { .. } => StatusCode::Unsupported, Error::TableMetadataManager { source, .. } => source.status_code(), diff --git a/src/operator/src/req_convert/insert/stmt_to_region.rs b/src/operator/src/req_convert/insert/stmt_to_region.rs index 4cd9c0856695..5317f92dd7c8 100644 --- a/src/operator/src/req_convert/insert/stmt_to_region.rs +++ b/src/operator/src/req_convert/insert/stmt_to_region.rs @@ -29,7 +29,7 @@ use table::TableRef; use crate::error::{ CatalogSnafu, ColumnDataTypeSnafu, ColumnDefaultValueSnafu, ColumnNoneDefaultValueSnafu, ColumnNotFoundSnafu, InvalidSqlSnafu, MissingInsertBodySnafu, ParseSqlSnafu, Result, - TableNotFoundSnafu, + SchemaReadOnlySnafu, TableNotFoundSnafu, }; use crate::req_convert::common::partitioner::Partitioner; use crate::req_convert::insert::semantic_type; @@ -65,6 +65,11 @@ impl<'a> StatementToRegion<'a> { let table_schema = table.schema(); let table_info = table.table_info(); + ensure!( + !common_catalog::consts::is_readonly_schema(&schema), + SchemaReadOnlySnafu { name: schema } + ); + let column_names = column_names(stmt, &table_schema); let column_count = column_names.len(); diff --git a/src/operator/src/statement/ddl.rs b/src/operator/src/statement/ddl.rs index 42a9fd2fafbf..daef8b74579b 100644 --- a/src/operator/src/statement/ddl.rs +++ b/src/operator/src/statement/ddl.rs @@ -20,7 +20,7 @@ use api::v1::meta::CreateFlowTask as PbCreateFlowTask; use api::v1::{column_def, AlterExpr, CreateFlowExpr, CreateTableExpr, CreateViewExpr}; use catalog::CatalogManagerRef; use chrono::Utc; -use common_catalog::consts::{DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME}; +use common_catalog::consts::{is_readonly_schema, DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME}; use common_catalog::{format_full_flow_name, format_full_table_name}; use common_error::ext::BoxedError; use common_meta::cache_invalidator::Context; @@ -72,8 +72,8 @@ use crate::error::{ ExtractTableNamesSnafu, FlowNotFoundSnafu, InvalidPartitionColumnsSnafu, InvalidPartitionRuleSnafu, InvalidPartitionSnafu, InvalidTableNameSnafu, InvalidViewNameSnafu, InvalidViewStmtSnafu, ParseSqlValueSnafu, Result, SchemaInUseSnafu, SchemaNotFoundSnafu, - SubstraitCodecSnafu, TableAlreadyExistsSnafu, TableMetadataManagerSnafu, TableNotFoundSnafu, - UnrecognizedTableOptionSnafu, ViewAlreadyExistsSnafu, + SchemaReadOnlySnafu, SubstraitCodecSnafu, TableAlreadyExistsSnafu, TableMetadataManagerSnafu, + TableNotFoundSnafu, UnrecognizedTableOptionSnafu, ViewAlreadyExistsSnafu, }; use crate::expr_factory; use crate::statement::show::create_partitions_stmt; @@ -151,6 +151,13 @@ impl StatementExecutor { partitions: Option, query_ctx: QueryContextRef, ) -> Result { + ensure!( + !is_readonly_schema(&create_table.schema_name), + SchemaReadOnlySnafu { + name: create_table.schema_name.clone() + } + ); + // Check if is creating logical table if create_table.engine == METRIC_ENGINE_NAME && create_table @@ -645,6 +652,13 @@ impl StatementExecutor { ) -> Result { let mut tables = Vec::with_capacity(table_names.len()); for table_name in table_names { + ensure!( + !is_readonly_schema(&table_name.schema_name), + SchemaReadOnlySnafu { + name: table_name.schema_name.clone() + } + ); + if let Some(table) = self .catalog_manager .table( @@ -694,6 +708,11 @@ impl StatementExecutor { drop_if_exists: bool, query_context: QueryContextRef, ) -> Result { + ensure!( + !is_readonly_schema(&schema), + SchemaReadOnlySnafu { name: schema } + ); + if self .catalog_manager .schema_exists(&catalog, &schema) @@ -725,6 +744,13 @@ impl StatementExecutor { table_name: TableName, query_context: QueryContextRef, ) -> Result { + ensure!( + !is_readonly_schema(&table_name.schema_name), + SchemaReadOnlySnafu { + name: table_name.schema_name.clone() + } + ); + let table = self .catalog_manager .table( @@ -794,6 +820,13 @@ impl StatementExecutor { expr: AlterExpr, query_context: QueryContextRef, ) -> Result { + ensure!( + !is_readonly_schema(&expr.schema_name), + SchemaReadOnlySnafu { + name: expr.schema_name.clone() + } + ); + let catalog_name = if expr.catalog_name.is_empty() { DEFAULT_CATALOG_NAME.to_string() } else { diff --git a/src/query/src/datafusion.rs b/src/query/src/datafusion.rs index 30fff004d671..14c01b05d389 100644 --- a/src/query/src/datafusion.rs +++ b/src/query/src/datafusion.rs @@ -23,6 +23,7 @@ use std::sync::Arc; use async_trait::async_trait; use common_base::Plugins; +use common_catalog::consts::is_readonly_schema; use common_error::ext::BoxedError; use common_function::function::FunctionRef; use common_function::scalars::aggregate::AggregateFunctionMetaRef; @@ -50,7 +51,7 @@ pub use crate::datafusion::planner::DfContextProviderAdapter; use crate::error::{ CatalogSnafu, CreateRecordBatchSnafu, DataFusionSnafu, MissingTableMutationHandlerSnafu, MissingTimestampColumnSnafu, QueryExecutionSnafu, Result, TableMutationSnafu, - TableNotFoundSnafu, UnsupportedExprSnafu, + TableNotFoundSnafu, TableReadOnlySnafu, UnsupportedExprSnafu, }; use crate::executor::QueryExecutor; use crate::logical_optimizer::LogicalOptimizer; @@ -173,6 +174,12 @@ impl DatafusionQueryEngine { let schema_name = table_name.schema.to_string(); let table_name = table_name.table.to_string(); let table_schema = table.schema(); + + ensure!( + !is_readonly_schema(&schema_name), + TableReadOnlySnafu { table: table_name } + ); + let ts_column = table_schema .timestamp_column() .map(|x| &x.name) @@ -212,10 +219,19 @@ impl DatafusionQueryEngine { column_vectors: HashMap, query_ctx: QueryContextRef, ) -> Result { + let catalog_name = table_name.catalog.to_string(); + let schema_name = table_name.schema.to_string(); + let table_name = table_name.table.to_string(); + + ensure!( + !is_readonly_schema(&schema_name), + TableReadOnlySnafu { table: table_name } + ); + let request = InsertRequest { - catalog_name: table_name.catalog.to_string(), - schema_name: table_name.schema.to_string(), - table_name: table_name.table.to_string(), + catalog_name, + schema_name, + table_name, columns_values: column_vectors, }; diff --git a/src/query/src/error.rs b/src/query/src/error.rs index 6eeb764b8a9f..74316a66a767 100644 --- a/src/query/src/error.rs +++ b/src/query/src/error.rs @@ -307,6 +307,13 @@ pub enum Error { location: Location, source: BoxedError, }, + + #[snafu(display("Cannot change read-only table: {}", table))] + TableReadOnly { + table: String, + #[snafu(implicit)] + location: Location, + }, } impl ErrorExt for Error { @@ -356,6 +363,7 @@ impl ErrorExt for Error { TableMutation { source, .. } => source.status_code(), MissingTableMutationHandler { .. } => StatusCode::Unexpected, GetRegionMetadata { .. } => StatusCode::Internal, + TableReadOnly { .. } => StatusCode::Unsupported, } } diff --git a/tests/cases/standalone/common/information_schema/region_peers.result b/tests/cases/standalone/common/information_schema/region_peers.result index 2ab814dc5c03..0c7919e5dd4d 100644 --- a/tests/cases/standalone/common/information_schema/region_peers.result +++ b/tests/cases/standalone/common/information_schema/region_peers.result @@ -1,5 +1,5 @@ --- test information_schema.region_peers ---- -USE INFORMATION_SCHEMA; +USE public; Affected Rows: 0 @@ -35,6 +35,10 @@ CREATE TABLE region_peers_test ( Affected Rows: 0 +use INFORMATION_SCHEMA; + +Affected Rows: 0 + SELECT COUNT(distinct region_id) FROM region_peers; +----------------------------------------+ @@ -43,11 +47,11 @@ SELECT COUNT(distinct region_id) FROM region_peers; | 6 | +----------------------------------------+ -DROP TABLE region_peers_t1, region_peers_t2, region_peers_phy, region_peers_test; +use public; Affected Rows: 0 -USE public; +DROP TABLE region_peers_t1, region_peers_t2, region_peers_phy, region_peers_test; Affected Rows: 0 diff --git a/tests/cases/standalone/common/information_schema/region_peers.sql b/tests/cases/standalone/common/information_schema/region_peers.sql index bcc7d75c64a8..34fbde6beaca 100644 --- a/tests/cases/standalone/common/information_schema/region_peers.sql +++ b/tests/cases/standalone/common/information_schema/region_peers.sql @@ -1,5 +1,5 @@ --- test information_schema.region_peers ---- -USE INFORMATION_SCHEMA; +USE public; CREATE TABLE region_peers_phy (ts timestamp time index, val double) engine = metric with ("physical_metric_table" = ""); @@ -25,10 +25,10 @@ CREATE TABLE region_peers_test ( a >= 20, ); -SELECT COUNT(distinct region_id) FROM region_peers; - -DROP TABLE region_peers_t1, region_peers_t2, region_peers_phy, region_peers_test; +use INFORMATION_SCHEMA; -USE public; +SELECT COUNT(distinct region_id) FROM region_peers; +use public; +DROP TABLE region_peers_t1, region_peers_t2, region_peers_phy, region_peers_test; diff --git a/tests/cases/standalone/common/information_schema/table_constraints.result b/tests/cases/standalone/common/information_schema/table_constraints.result index d86a12b6b038..573336458771 100644 --- a/tests/cases/standalone/common/information_schema/table_constraints.result +++ b/tests/cases/standalone/common/information_schema/table_constraints.result @@ -25,34 +25,42 @@ SELECT * FROM TABLE_CONSTRAINTS ORDER BY TABLE_NAME, CONSTRAINT_NAME; | def | public | PRIMARY | public | numbers | PRIMARY KEY | YES | +--------------------+-------------------+-----------------+--------------+------------+-----------------+----------+ +use public; + +Affected Rows: 0 + CREATE TABLE test(i double, j string, ts timestamp time index, primary key(j)); Affected Rows: 0 +use INFORMATION_SCHEMA; + +Affected Rows: 0 + SELECT * FROM TABLE_CONSTRAINTS ORDER BY TABLE_NAME, CONSTRAINT_NAME; -+--------------------+--------------------+-----------------+--------------------+------------+-----------------+----------+ -| constraint_catalog | constraint_schema | constraint_name | table_schema | table_name | constraint_type | enforced | -+--------------------+--------------------+-----------------+--------------------+------------+-----------------+----------+ -| def | public | PRIMARY | public | numbers | PRIMARY KEY | YES | -| def | information_schema | PRIMARY | information_schema | test | PRIMARY KEY | YES | -| def | information_schema | TIME INDEX | information_schema | test | TIME INDEX | YES | -+--------------------+--------------------+-----------------+--------------------+------------+-----------------+----------+ ++--------------------+-------------------+-----------------+--------------+------------+-----------------+----------+ +| constraint_catalog | constraint_schema | constraint_name | table_schema | table_name | constraint_type | enforced | ++--------------------+-------------------+-----------------+--------------+------------+-----------------+----------+ +| def | public | PRIMARY | public | numbers | PRIMARY KEY | YES | +| def | public | PRIMARY | public | test | PRIMARY KEY | YES | +| def | public | TIME INDEX | public | test | TIME INDEX | YES | ++--------------------+-------------------+-----------------+--------------+------------+-----------------+----------+ SELECT * FROM TABLE_CONSTRAINTS WHERE TABLE_NAME = 'test' ORDER BY TABLE_NAME, CONSTRAINT_NAME; -+--------------------+--------------------+-----------------+--------------------+------------+-----------------+----------+ -| constraint_catalog | constraint_schema | constraint_name | table_schema | table_name | constraint_type | enforced | -+--------------------+--------------------+-----------------+--------------------+------------+-----------------+----------+ -| def | information_schema | PRIMARY | information_schema | test | PRIMARY KEY | YES | -| def | information_schema | TIME INDEX | information_schema | test | TIME INDEX | YES | -+--------------------+--------------------+-----------------+--------------------+------------+-----------------+----------+ ++--------------------+-------------------+-----------------+--------------+------------+-----------------+----------+ +| constraint_catalog | constraint_schema | constraint_name | table_schema | table_name | constraint_type | enforced | ++--------------------+-------------------+-----------------+--------------+------------+-----------------+----------+ +| def | public | PRIMARY | public | test | PRIMARY KEY | YES | +| def | public | TIME INDEX | public | test | TIME INDEX | YES | ++--------------------+-------------------+-----------------+--------------+------------+-----------------+----------+ -DROP TABLE test; +use public; Affected Rows: 0 -USE public; +DROP TABLE test; Affected Rows: 0 diff --git a/tests/cases/standalone/common/information_schema/table_constraints.sql b/tests/cases/standalone/common/information_schema/table_constraints.sql index f715e8334ce9..f5e0542696d3 100644 --- a/tests/cases/standalone/common/information_schema/table_constraints.sql +++ b/tests/cases/standalone/common/information_schema/table_constraints.sql @@ -6,12 +6,16 @@ DESC TABLE TABLE_CONSTRAINTS; SELECT * FROM TABLE_CONSTRAINTS ORDER BY TABLE_NAME, CONSTRAINT_NAME; +use public; + CREATE TABLE test(i double, j string, ts timestamp time index, primary key(j)); +use INFORMATION_SCHEMA; + SELECT * FROM TABLE_CONSTRAINTS ORDER BY TABLE_NAME, CONSTRAINT_NAME; SELECT * FROM TABLE_CONSTRAINTS WHERE TABLE_NAME = 'test' ORDER BY TABLE_NAME, CONSTRAINT_NAME; -DROP TABLE test; +use public; -USE public; +DROP TABLE test; diff --git a/tests/cases/standalone/common/information_schema/tables.result b/tests/cases/standalone/common/information_schema/tables.result index d5fd7021e817..93a93a9c9805 100644 --- a/tests/cases/standalone/common/information_schema/tables.result +++ b/tests/cases/standalone/common/information_schema/tables.result @@ -44,3 +44,39 @@ drop schema abcde; Affected Rows: 0 +drop schema information_schema; + +Error: 1001(Unsupported), Schema `information_schema` is read-only + +use information_schema; + +Affected Rows: 0 + +create table t (ts timestamp time index); + +Error: 1001(Unsupported), Schema `information_schema` is read-only + +drop table build_info; + +Error: 1001(Unsupported), Schema `information_schema` is read-only + +alter table build_info add q string; + +Error: 1001(Unsupported), Schema `information_schema` is read-only + +truncate table build_info; + +Error: 1001(Unsupported), Schema `information_schema` is read-only + +insert into build_info values ("", "", "", "", ""); + +Error: 1001(Unsupported), Schema `information_schema` is read-only + +delete from build_info; + +Error: 1001(Unsupported), Cannot change read-only table: build_info + +use public; + +Affected Rows: 0 + diff --git a/tests/cases/standalone/common/information_schema/tables.sql b/tests/cases/standalone/common/information_schema/tables.sql index 4a03a4a3b63a..ae2bcf04306a 100644 --- a/tests/cases/standalone/common/information_schema/tables.sql +++ b/tests/cases/standalone/common/information_schema/tables.sql @@ -17,3 +17,21 @@ use public; drop schema abc; drop schema abcde; + +drop schema information_schema; + +use information_schema; + +create table t (ts timestamp time index); + +drop table build_info; + +alter table build_info add q string; + +truncate table build_info; + +insert into build_info values ("", "", "", "", ""); + +delete from build_info; + +use public;