fix: quote ident on rendered SQL (#2248)

* fix: quote ident on rendered SQL

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

* read quote style from query context

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

* update sqlness result

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

---------

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
This commit is contained in:
Ruihang Xia
2023-08-25 02:25:21 -05:00
committed by GitHub
parent de1daec680
commit 8d446ed741
10 changed files with 154 additions and 113 deletions

View File

@@ -141,7 +141,8 @@ impl Instance {
let table_ref = TableReference::full(&catalog, &schema, &table);
let table = self.sql_handler.get_table(&table_ref).await?;
query::sql::show_create_table(table, None).context(ExecuteStatementSnafu)
query::sql::show_create_table(table, None, query_ctx.clone())
.context(ExecuteStatementSnafu)
}
Statement::TruncateTable(truncate_table) => {
let (catalog_name, schema_name, table_name) =

View File

@@ -399,7 +399,8 @@ impl DistInstance {
.context(TableNotFoundSnafu { table_name: &table })?;
let table_name = TableName::new(catalog, schema, table);
self.show_create_table(table_name, table_ref).await
self.show_create_table(table_name, table_ref, query_ctx.clone())
.await
}
Statement::TruncateTable(stmt) => {
let (catalog, schema, table) =
@@ -416,7 +417,12 @@ impl DistInstance {
}
}
async fn show_create_table(&self, table_name: TableName, table: TableRef) -> Result<Output> {
async fn show_create_table(
&self,
table_name: TableName,
table: TableRef,
query_ctx: QueryContextRef,
) -> Result<Output> {
let partitions = self
.catalog_manager
.partition_manager()
@@ -428,7 +434,8 @@ impl DistInstance {
let partitions = create_partitions_stmt(partitions)?;
query::sql::show_create_table(table, partitions).context(error::ExecuteStatementSnafu)
query::sql::show_create_table(table, partitions, query_ctx)
.context(error::ExecuteStatementSnafu)
}
/// Handles distributed database creation

View File

@@ -185,11 +185,28 @@ pub async fn show_tables(
}
}
pub fn show_create_table(table: TableRef, partitions: Option<Partitions>) -> Result<Output> {
pub fn show_create_table(
table: TableRef,
partitions: Option<Partitions>,
query_ctx: QueryContextRef,
) -> Result<Output> {
let table_info = table.table_info();
let table_name = &table_info.name;
let mut stmt = show_create_table::create_table_stmt(&table_info)?;
stmt.partitions = partitions;
// Default to double quote and fallback to back quote
let quote_style = if query_ctx.sql_dialect().is_delimited_identifier_start('"') {
'"'
} else if query_ctx.sql_dialect().is_delimited_identifier_start('\'') {
'\''
} else {
'`'
};
let mut stmt = show_create_table::create_table_stmt(&table_info, quote_style)?;
stmt.partitions = partitions.map(|mut p| {
p.set_quote(quote_style);
p
});
let sql = format!("{}", stmt);
let columns = vec![
Arc::new(StringVector::from(vec![table_name.clone()])) as _,

View File

@@ -20,7 +20,7 @@ use datatypes::schema::{ColumnDefaultConstraint, ColumnSchema, SchemaRef, COMMEN
use humantime::format_duration;
use snafu::ResultExt;
use sql::ast::{
ColumnDef, ColumnOption, ColumnOptionDef, Expr, ObjectName, SqlOption, TableConstraint,
ColumnDef, ColumnOption, ColumnOptionDef, Expr, Ident, ObjectName, SqlOption, TableConstraint,
Value as SqlValue,
};
use sql::dialect::GreptimeDbDialect;
@@ -90,7 +90,7 @@ fn column_option_def(option: ColumnOption) -> ColumnOptionDef {
ColumnOptionDef { name: None, option }
}
fn create_column_def(column_schema: &ColumnSchema) -> Result<ColumnDef> {
fn create_column_def(column_schema: &ColumnSchema, quote_style: char) -> Result<ColumnDef> {
let name = &column_schema.name;
let mut options = Vec::with_capacity(2);
@@ -119,7 +119,7 @@ fn create_column_def(column_schema: &ColumnSchema) -> Result<ColumnDef> {
}
Ok(ColumnDef {
name: name[..].into(),
name: Ident::with_quote(quote_style, name),
data_type: statements::concrete_data_type_to_sql_data_type(&column_schema.data_type)
.with_context(|_| ConvertSqlTypeSnafu {
datatype: column_schema.data_type.clone(),
@@ -129,20 +129,24 @@ fn create_column_def(column_schema: &ColumnSchema) -> Result<ColumnDef> {
})
}
fn create_table_constraints(schema: &SchemaRef, table_meta: &TableMeta) -> Vec<TableConstraint> {
fn create_table_constraints(
schema: &SchemaRef,
table_meta: &TableMeta,
quote_style: char,
) -> Vec<TableConstraint> {
let mut constraints = Vec::with_capacity(2);
if let Some(timestamp_column) = schema.timestamp_column() {
let column_name = &timestamp_column.name;
constraints.push(TableConstraint::Unique {
name: Some(TIME_INDEX.into()),
columns: vec![column_name[..].into()],
columns: vec![Ident::with_quote(quote_style, column_name)],
is_primary: false,
});
}
if !table_meta.primary_key_indices.is_empty() {
let columns = table_meta
.row_key_column_names()
.map(|name| name[..].into())
.map(|name| Ident::with_quote(quote_style, name))
.collect();
constraints.push(TableConstraint::Unique {
name: None,
@@ -155,7 +159,7 @@ fn create_table_constraints(schema: &SchemaRef, table_meta: &TableMeta) -> Vec<T
}
/// Create a CreateTable statement from table info.
pub fn create_table_stmt(table_info: &TableInfoRef) -> Result<CreateTable> {
pub fn create_table_stmt(table_info: &TableInfoRef, quote_style: char) -> Result<CreateTable> {
let table_meta = &table_info.meta;
let table_name = &table_info.name;
let schema = &table_info.meta.schema;
@@ -163,15 +167,15 @@ pub fn create_table_stmt(table_info: &TableInfoRef) -> Result<CreateTable> {
let columns = schema
.column_schemas()
.iter()
.map(create_column_def)
.map(|c| create_column_def(c, quote_style))
.collect::<Result<Vec<_>>>()?;
let constraints = create_table_constraints(schema, table_meta);
let constraints = create_table_constraints(schema, table_meta, quote_style);
Ok(CreateTable {
if_not_exists: true,
table_id: table_info.ident.table_id,
name: ObjectName(vec![table_name[..].into()]),
name: ObjectName(vec![Ident::with_quote(quote_style, table_name)]),
columns,
engine: table_meta.engine.clone(),
constraints,
@@ -246,19 +250,19 @@ mod tests {
.unwrap(),
);
let stmt = create_table_stmt(&info).unwrap();
let stmt = create_table_stmt(&info, '"').unwrap();
let sql = format!("\n{}", stmt);
assert_eq!(
r#"
CREATE TABLE IF NOT EXISTS system_metrics (
id INT UNSIGNED NULL,
host STRING NULL,
cpu DOUBLE NULL,
disk FLOAT NULL,
ts TIMESTAMP(3) NOT NULL DEFAULT current_timestamp(),
TIME INDEX (ts),
PRIMARY KEY (id, host)
CREATE TABLE IF NOT EXISTS "system_metrics" (
"id" INT UNSIGNED NULL,
"host" STRING NULL,
"cpu" DOUBLE NULL,
"disk" FLOAT NULL,
"ts" TIMESTAMP(3) NOT NULL DEFAULT current_timestamp(),
TIME INDEX ("ts"),
PRIMARY KEY ("id", "host")
)
ENGINE=mito
WITH(
@@ -315,14 +319,14 @@ WITH(
.unwrap(),
);
let stmt = create_table_stmt(&info).unwrap();
let stmt = create_table_stmt(&info, '"').unwrap();
let sql = format!("\n{}", stmt);
assert_eq!(
r#"
CREATE EXTERNAL TABLE IF NOT EXISTS system_metrics (
host STRING NULL,
cpu DOUBLE NULL,
CREATE EXTERNAL TABLE IF NOT EXISTS "system_metrics" (
"host" STRING NULL,
"cpu" DOUBLE NULL,
)
ENGINE=file

View File

@@ -65,7 +65,12 @@ pub enum Error {
source: std::io::Error,
},
#[snafu(display("Failed to execute query: {}, source: {}", query, source))]
#[snafu(display(
"Failed to execute query, source: {}, query: {}, location: {}",
source,
query,
location
))]
ExecuteQuery {
query: String,
location: Location,

View File

@@ -57,7 +57,7 @@ pub enum Error {
UnsupportedDefaultValue { column_name: String, expr: Expr },
// Syntax error from sql parser.
#[snafu(display("Syntax error, sql: {}, source: {}", sql, source))]
#[snafu(display("Syntax error, source: {}, sql: {}", source, sql))]
Syntax { sql: String, source: ParserError },
#[snafu(display("Missing time index constraint"))]

View File

@@ -130,6 +130,15 @@ pub struct Partitions {
pub entries: Vec<PartitionEntry>,
}
impl Partitions {
/// set quotes to all [Ident]s from column list
pub fn set_quote(&mut self, quote_style: char) {
self.column_list
.iter_mut()
.for_each(|c| c.quote_style = Some(quote_style));
}
}
#[derive(Debug, PartialEq, Eq, Clone)]
pub struct PartitionEntry {
pub name: Ident,

View File

@@ -109,18 +109,17 @@ PARTITION BY RANGE COLUMNS (ts) (
let output = execute_sql(&frontend, "show create table demo").await;
let expected = if instance.is_distributed_mode() {
"\
+-------+----------------------------------------------------------+
r#"+-------+----------------------------------------------------------+
| Table | Create Table |
+-------+----------------------------------------------------------+
| demo | CREATE TABLE IF NOT EXISTS demo ( |
| | host STRING NULL, |
| | cpu DOUBLE NULL, |
| | memory DOUBLE NULL, |
| | ts BIGINT NOT NULL, |
| | TIME INDEX (ts) |
| demo | CREATE TABLE IF NOT EXISTS "demo" ( |
| | "host" STRING NULL, |
| | "cpu" DOUBLE NULL, |
| | "memory" DOUBLE NULL, |
| | "ts" BIGINT NOT NULL, |
| | TIME INDEX ("ts") |
| | ) |
| | PARTITION BY RANGE COLUMNS (ts) ( |
| | PARTITION BY RANGE COLUMNS ("ts") ( |
| | PARTITION r0 VALUES LESS THAN (1), |
| | PARTITION r1 VALUES LESS THAN (10), |
| | PARTITION r2 VALUES LESS THAN (100), |
@@ -130,24 +129,23 @@ PARTITION BY RANGE COLUMNS (ts) (
| | WITH( |
| | regions = 4 |
| | ) |
+-------+----------------------------------------------------------+"
+-------+----------------------------------------------------------+"#
} else {
"\
+-------+-----------------------------------+
| Table | Create Table |
+-------+-----------------------------------+
| demo | CREATE TABLE IF NOT EXISTS demo ( |
| | host STRING NULL, |
| | cpu DOUBLE NULL, |
| | memory DOUBLE NULL, |
| | ts BIGINT NOT NULL, |
| | TIME INDEX (ts) |
| | ) |
| | ENGINE=mito |
| | WITH( |
| | regions = 1 |
| | ) |
+-------+-----------------------------------+"
r#"+-------+-------------------------------------+
| Table | Create Table |
+-------+-------------------------------------+
| demo | CREATE TABLE IF NOT EXISTS "demo" ( |
| | "host" STRING NULL, |
| | "cpu" DOUBLE NULL, |
| | "memory" DOUBLE NULL, |
| | "ts" BIGINT NOT NULL, |
| | TIME INDEX ("ts") |
| | ) |
| | ENGINE=mito |
| | WITH( |
| | regions = 1 |
| | ) |
+-------+-------------------------------------+"#
};
check_output_stream(output, expected).await;

View File

@@ -19,29 +19,29 @@ Affected Rows: 0
SHOW CREATE TABLE system_metrics;
+----------------+----------------------------------------------------------+
| Table | Create Table |
+----------------+----------------------------------------------------------+
| system_metrics | CREATE TABLE IF NOT EXISTS system_metrics ( |
| | id INT UNSIGNED NULL, |
| | host STRING NULL, |
| | cpu DOUBLE NULL, |
| | disk FLOAT NULL, |
| | n INT NULL, |
| | ts TIMESTAMP(3) NOT NULL DEFAULT current_timestamp(), |
| | TIME INDEX (ts), |
| | PRIMARY KEY (id, host) |
| | ) |
| | PARTITION BY RANGE COLUMNS (n) ( |
| | PARTITION r0 VALUES LESS THAN (5), |
| | PARTITION r1 VALUES LESS THAN (9), |
| | PARTITION r2 VALUES LESS THAN (MAXVALUE) |
| | ) |
| | ENGINE=mito |
| | WITH( |
| | regions = 3 |
| | ) |
+----------------+----------------------------------------------------------+
+----------------+-----------------------------------------------------------+
| Table | Create Table |
+----------------+-----------------------------------------------------------+
| system_metrics | CREATE TABLE IF NOT EXISTS "system_metrics" ( |
| | "id" INT UNSIGNED NULL, |
| | "host" STRING NULL, |
| | "cpu" DOUBLE NULL, |
| | "disk" FLOAT NULL, |
| | "n" INT NULL, |
| | "ts" TIMESTAMP(3) NOT NULL DEFAULT current_timestamp(), |
| | TIME INDEX ("ts"), |
| | PRIMARY KEY ("id", "host") |
| | ) |
| | PARTITION BY RANGE COLUMNS ("n") ( |
| | PARTITION r0 VALUES LESS THAN (5), |
| | PARTITION r1 VALUES LESS THAN (9), |
| | PARTITION r2 VALUES LESS THAN (MAXVALUE) |
| | ) |
| | ENGINE=mito |
| | WITH( |
| | regions = 3 |
| | ) |
+----------------+-----------------------------------------------------------+
DROP TABLE system_metrics;
@@ -55,19 +55,19 @@ Affected Rows: 0
show create table table_without_partition;
+-------------------------+---------------------------------------------------------+
| Table | Create Table |
+-------------------------+---------------------------------------------------------+
| table_without_partition | CREATE TABLE IF NOT EXISTS table_without_partition ( |
| | ts TIMESTAMP(3) NOT NULL DEFAULT current_timestamp(), |
| | TIME INDEX (ts) |
| | ) |
| | |
| | ENGINE=mito |
| | WITH( |
| | regions = 1 |
| | ) |
+-------------------------+---------------------------------------------------------+
+-------------------------+-----------------------------------------------------------+
| Table | Create Table |
+-------------------------+-----------------------------------------------------------+
| table_without_partition | CREATE TABLE IF NOT EXISTS "table_without_partition" ( |
| | "ts" TIMESTAMP(3) NOT NULL DEFAULT current_timestamp(), |
| | TIME INDEX ("ts") |
| | ) |
| | |
| | ENGINE=mito |
| | WITH( |
| | regions = 1 |
| | ) |
+-------------------------+-----------------------------------------------------------+
drop table table_without_partition;

View File

@@ -17,25 +17,25 @@ Affected Rows: 0
SHOW CREATE TABLE system_metrics;
+----------------+---------------------------------------------------------+
| Table | Create Table |
+----------------+---------------------------------------------------------+
| system_metrics | CREATE TABLE IF NOT EXISTS system_metrics ( |
| | id INT UNSIGNED NULL, |
| | host STRING NULL, |
| | cpu DOUBLE NULL COMMENT 'cpu', |
| | disk FLOAT NULL, |
| | ts TIMESTAMP(3) NOT NULL DEFAULT current_timestamp(), |
| | TIME INDEX (ts), |
| | PRIMARY KEY (id, host) |
| | ) |
| | ENGINE=mito |
| | WITH( |
| | regions = 1, |
| | ttl = '7days', |
| | write_buffer_size = '1.0KiB' |
| | ) |
+----------------+---------------------------------------------------------+
+----------------+-----------------------------------------------------------+
| Table | Create Table |
+----------------+-----------------------------------------------------------+
| system_metrics | CREATE TABLE IF NOT EXISTS "system_metrics" ( |
| | "id" INT UNSIGNED NULL, |
| | "host" STRING NULL, |
| | "cpu" DOUBLE NULL COMMENT 'cpu', |
| | "disk" FLOAT NULL, |
| | "ts" TIMESTAMP(3) NOT NULL DEFAULT current_timestamp(), |
| | TIME INDEX ("ts"), |
| | PRIMARY KEY ("id", "host") |
| | ) |
| | ENGINE=mito |
| | WITH( |
| | regions = 1, |
| | ttl = '7days', |
| | write_buffer_size = '1.0KiB' |
| | ) |
+----------------+-----------------------------------------------------------+
DROP TABLE system_metrics;