diff --git a/nom-sql/src/analysis/visit.rs b/nom-sql/src/analysis/visit.rs index 8abd170715..7530d36fcc 100644 --- a/nom-sql/src/analysis/visit.rs +++ b/nom-sql/src/analysis/visit.rs @@ -17,14 +17,14 @@ use crate::set::Variable; use crate::transaction::{CommitStatement, RollbackStatement, StartTransactionStatement}; use crate::{ AlterColumnOperation, AlterTableDefinition, AlterTableStatement, CacheInner, CaseWhenBranch, - Column, ColumnConstraint, ColumnSpecification, CommonTableExpr, CompoundSelectStatement, - CreateCacheStatement, CreateTableStatement, CreateViewStatement, DeleteStatement, - DropAllCachesStatement, DropCacheStatement, DropTableStatement, DropViewStatement, - ExplainStatement, Expr, FieldDefinitionExpr, FieldReference, FunctionExpr, GroupByClause, - InValue, InsertStatement, JoinClause, JoinConstraint, JoinRightSide, Literal, OrderClause, - Relation, SelectSpecification, SelectStatement, SetNames, SetPostgresParameter, SetStatement, - SetVariables, ShowStatement, SqlIdentifier, SqlQuery, SqlType, TableExpr, TableExprInner, - TableKey, UpdateStatement, UseStatement, + Column, ColumnConstraint, ColumnSpecification, CommentStatement, CommonTableExpr, + CompoundSelectStatement, CreateCacheStatement, CreateTableStatement, CreateViewStatement, + DeleteStatement, DropAllCachesStatement, DropCacheStatement, DropTableStatement, + DropViewStatement, ExplainStatement, Expr, FieldDefinitionExpr, FieldReference, FunctionExpr, + GroupByClause, InValue, InsertStatement, JoinClause, JoinConstraint, JoinRightSide, Literal, + OrderClause, Relation, SelectSpecification, SelectStatement, SetNames, SetPostgresParameter, + SetStatement, SetVariables, ShowStatement, SqlIdentifier, SqlQuery, SqlType, TableExpr, + TableExprInner, TableKey, UpdateStatement, UseStatement, }; /// Each method of the `Visitor` trait is a hook to be potentially overridden when recursively @@ -396,6 +396,13 @@ pub trait Visitor<'ast>: Sized { Ok(()) } + fn visit_comment_statement( + &mut self, + comment_statement: &'ast CommentStatement, + ) -> Result<(), Self::Error> { + walk_comment_statement(self, comment_statement) + } + fn visit_sql_query(&mut self, sql_query: &'ast SqlQuery) -> Result<(), Self::Error> { walk_sql_query(self, sql_query) } @@ -1111,7 +1118,29 @@ pub fn walk_sql_query<'a, V: Visitor<'a>>( SqlQuery::Use(statement) => visitor.visit_use_statement(statement), SqlQuery::Show(statement) => visitor.visit_show_statement(statement), SqlQuery::Explain(statement) => visitor.visit_explain_statement(statement), + SqlQuery::Comment(statement) => visitor.visit_comment_statement(statement), + } +} + +pub fn walk_comment_statement<'a, V: Visitor<'a>>( + visitor: &mut V, + comment_statement: &'a CommentStatement, +) -> Result<(), V::Error> { + match comment_statement { + CommentStatement::Column { + column_name, + table_name, + .. + } => { + visitor.visit_sql_identifier(column_name)?; + visitor.visit_sql_identifier(table_name)?; + } + CommentStatement::Table { table_name, .. } => { + visitor.visit_sql_identifier(table_name)?; + } } + + Ok(()) } #[cfg(test)] diff --git a/nom-sql/src/analysis/visit_mut.rs b/nom-sql/src/analysis/visit_mut.rs index 3cea0e0c07..85f4483464 100644 --- a/nom-sql/src/analysis/visit_mut.rs +++ b/nom-sql/src/analysis/visit_mut.rs @@ -17,14 +17,14 @@ use crate::set::Variable; use crate::transaction::{CommitStatement, RollbackStatement, StartTransactionStatement}; use crate::{ AlterColumnOperation, AlterTableDefinition, AlterTableStatement, CacheInner, CaseWhenBranch, - Column, ColumnConstraint, ColumnSpecification, CommonTableExpr, CompoundSelectStatement, - CreateCacheStatement, CreateTableStatement, CreateViewStatement, DeleteStatement, - DropAllCachesStatement, DropCacheStatement, DropTableStatement, DropViewStatement, - ExplainStatement, Expr, FieldDefinitionExpr, FieldReference, FunctionExpr, GroupByClause, - InValue, InsertStatement, JoinClause, JoinConstraint, JoinRightSide, Literal, OrderClause, - Relation, SelectSpecification, SelectStatement, SetNames, SetPostgresParameter, SetStatement, - SetVariables, ShowStatement, SqlIdentifier, SqlQuery, SqlType, TableExpr, TableExprInner, - TableKey, UpdateStatement, UseStatement, + Column, ColumnConstraint, ColumnSpecification, CommentStatement, CommonTableExpr, + CompoundSelectStatement, CreateCacheStatement, CreateTableStatement, CreateViewStatement, + DeleteStatement, DropAllCachesStatement, DropCacheStatement, DropTableStatement, + DropViewStatement, ExplainStatement, Expr, FieldDefinitionExpr, FieldReference, FunctionExpr, + GroupByClause, InValue, InsertStatement, JoinClause, JoinConstraint, JoinRightSide, Literal, + OrderClause, Relation, SelectSpecification, SelectStatement, SetNames, SetPostgresParameter, + SetStatement, SetVariables, ShowStatement, SqlIdentifier, SqlQuery, SqlType, TableExpr, + TableExprInner, TableKey, UpdateStatement, UseStatement, }; /// Each method of the `VisitorMut` trait is a hook to be potentially overridden when recursively @@ -411,6 +411,13 @@ pub trait VisitorMut<'ast>: Sized { Ok(()) } + fn visit_comment_statement( + &mut self, + comment_statement: &'ast mut CommentStatement, + ) -> Result<(), Self::Error> { + walk_comment_statement(self, comment_statement) + } + fn visit_sql_query(&mut self, sql_query: &'ast mut SqlQuery) -> Result<(), Self::Error> { walk_sql_query(self, sql_query) } @@ -1128,7 +1135,29 @@ pub fn walk_sql_query<'a, V: VisitorMut<'a>>( SqlQuery::Use(statement) => visitor.visit_use_statement(statement), SqlQuery::Show(statement) => visitor.visit_show_statement(statement), SqlQuery::Explain(statement) => visitor.visit_explain_statement(statement), + SqlQuery::Comment(statement) => visitor.visit_comment_statement(statement), + } +} + +pub fn walk_comment_statement<'a, V: VisitorMut<'a>>( + visitor: &mut V, + comment_statement: &'a mut CommentStatement, +) -> Result<(), V::Error> { + match comment_statement { + CommentStatement::Column { + column_name, + table_name, + .. + } => { + visitor.visit_sql_identifier(column_name)?; + visitor.visit_sql_identifier(table_name)?; + } + CommentStatement::Table { table_name, .. } => { + visitor.visit_sql_identifier(table_name)?; + } } + + Ok(()) } #[cfg(test)] diff --git a/nom-sql/src/comment.rs b/nom-sql/src/comment.rs new file mode 100644 index 0000000000..6af0e224b9 --- /dev/null +++ b/nom-sql/src/comment.rs @@ -0,0 +1,187 @@ +use std::{fmt, str}; + +use nom::branch::alt; +use nom::bytes::complete::{tag, tag_no_case}; +use nom::combinator::map_res; +use nom::sequence::{delimited, terminated}; +use nom_locate::LocatedSpan; +use readyset_util::fmt::fmt_with; +use serde::{Deserialize, Serialize}; + +use crate::common::statement_terminator; +use crate::whitespace::{whitespace0, whitespace1}; +use crate::{literal, Dialect, NomSqlResult, SqlIdentifier}; + +#[derive(Clone, Debug, Eq, Hash, PartialEq, Serialize, Deserialize)] +pub enum CommentStatement { + Column { + column_name: SqlIdentifier, + table_name: SqlIdentifier, + comment: String, + }, + Table { + table_name: SqlIdentifier, + comment: String, + }, +} + +impl CommentStatement { + pub fn display(&self, dialect: Dialect) -> impl fmt::Display + Copy + '_ { + fmt_with(move |f| match self { + Self::Column { + column_name, + table_name, + comment, + } => { + write!( + f, + "COMMENT ON COLUMN {}.{} IS ", + dialect.quote_identifier(&table_name), + dialect.quote_identifier(&column_name), + )?; + literal::display_string_literal(f, comment) + } + Self::Table { + table_name, + comment, + } => { + write!( + f, + "COMMENT ON TABLE {} IS ", + dialect.quote_identifier(&table_name), + )?; + literal::display_string_literal(f, comment) + } + }) + } +} + +pub fn comment( + dialect: Dialect, +) -> impl Fn(LocatedSpan<&[u8]>) -> NomSqlResult<&[u8], CommentStatement> { + move |i| alt((table_comment(dialect), column_comment(dialect)))(i) +} + +fn table_comment( + dialect: Dialect, +) -> impl Fn(LocatedSpan<&[u8]>) -> NomSqlResult<&[u8], CommentStatement> { + move |i| { + let (i, _) = tag_no_case("comment")(i)?; + let (i, _) = whitespace1(i)?; + let (i, _) = tag_no_case("on")(i)?; + let (i, _) = whitespace1(i)?; + let (i, _) = tag_no_case("table")(i)?; + let (i, _) = whitespace1(i)?; + let (i, table_name) = dialect.identifier()(i)?; + let (i, _) = whitespace1(i)?; + let (i, _) = tag_no_case("is")(i)?; + let (i, _) = whitespace1(i)?; + let (i, comment) = map_res(dialect.string_literal(), String::from_utf8)(i)?; + let (i, _) = statement_terminator(i)?; + + Ok(( + i, + CommentStatement::Table { + table_name, + comment, + }, + )) + } +} + +fn column_comment( + dialect: Dialect, +) -> impl Fn(LocatedSpan<&[u8]>) -> NomSqlResult<&[u8], CommentStatement> { + move |i| { + let (i, _) = tag_no_case("comment")(i)?; + let (i, _) = whitespace1(i)?; + let (i, _) = tag_no_case("on")(i)?; + let (i, _) = whitespace1(i)?; + let (i, _) = tag_no_case("column")(i)?; + let (i, _) = whitespace1(i)?; + let (i, table_name) = terminated( + dialect.identifier(), + delimited(whitespace0, tag("."), whitespace0), + )(i)?; + let (i, column_name) = dialect.identifier()(i)?; + let (i, _) = whitespace1(i)?; + let (i, _) = tag_no_case("is")(i)?; + let (i, _) = whitespace1(i)?; + let (i, comment) = map_res(dialect.string_literal(), String::from_utf8)(i)?; + let (i, _) = statement_terminator(i)?; + + Ok(( + i, + CommentStatement::Column { + table_name, + column_name, + comment, + }, + )) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn table_comment() { + let res = test_parse!( + comment(Dialect::MySQL), + b"COMMENT ON TABLE test IS 'this is a comment'" + ); + + assert_eq!( + res, + CommentStatement::Table { + table_name: "test".into(), + comment: "this is a comment".into(), + } + ) + } + + #[test] + fn column_comment() { + let res = test_parse!( + comment(Dialect::MySQL), + b"COMMENT ON COLUMN test_table.test_column IS 'this is a comment'" + ); + + assert_eq!( + res, + CommentStatement::Column { + table_name: "test_table".into(), + column_name: "test_column".into(), + comment: "this is a comment".into(), + } + ) + } + + #[test] + fn display_column_comment() { + let comment = CommentStatement::Column { + table_name: "test_table".into(), + column_name: "test_column".into(), + comment: "this is a comment".into(), + }; + + assert_eq!( + comment.display(Dialect::PostgreSQL).to_string(), + "COMMENT ON COLUMN \"test_table\".\"test_column\" IS 'this is a comment'" + ); + } + + #[test] + fn display_table_comment() { + let comment = CommentStatement::Table { + table_name: "test_table".into(), + comment: "this is a comment".into(), + }; + + assert_eq!( + comment.display(Dialect::PostgreSQL).to_string(), + "COMMENT ON TABLE \"test_table\" IS 'this is a comment'" + ); + } +} diff --git a/nom-sql/src/lib.rs b/nom-sql/src/lib.rs index 6a359d0ae2..050603230f 100644 --- a/nom-sql/src/lib.rs +++ b/nom-sql/src/lib.rs @@ -16,6 +16,7 @@ pub use self::alter::{ AlterColumnOperation, AlterTableDefinition, AlterTableStatement, ReplicaIdentity, }; pub use self::column::{Column, ColumnConstraint, ColumnSpecification}; +pub use self::comment::CommentStatement; pub use self::common::{FieldDefinitionExpr, FieldReference, IndexType, TableKey}; pub use self::compound_select::{CompoundSelectOperator, CompoundSelectStatement}; pub use self::create::{ @@ -63,6 +64,7 @@ mod macros; mod alter; pub mod analysis; mod column; +mod comment; mod common; mod compound_select; mod create; diff --git a/nom-sql/src/literal.rs b/nom-sql/src/literal.rs index 90b704b667..aa2c2cc1f7 100644 --- a/nom-sql/src/literal.rs +++ b/nom-sql/src/literal.rs @@ -195,9 +195,7 @@ impl Display for Literal { Literal::Numeric(val, scale) => { write!(f, "{}", Decimal::from_i128_with_scale(*val, *scale)) } - Literal::String(ref s) => { - write!(f, "'{}'", s.replace('\'', "''").replace('\\', "\\\\")) - } + Literal::String(ref s) => display_string_literal(f, s), Literal::Blob(ref bv) => write!( f, "{}", @@ -224,6 +222,10 @@ impl Display for Literal { } } +pub(crate) fn display_string_literal(f: &mut fmt::Formatter<'_>, s: &str) -> fmt::Result { + write!(f, "'{}'", s.replace('\'', "''").replace('\\', "\\\\")) +} + impl Literal { pub fn arbitrary_with_type(sql_type: &SqlType) -> impl Strategy + 'static { use proptest::prelude::*; diff --git a/nom-sql/src/parser.rs b/nom-sql/src/parser.rs index 87236c2164..4d67652385 100644 --- a/nom-sql/src/parser.rs +++ b/nom-sql/src/parser.rs @@ -8,6 +8,7 @@ use readyset_util::redacted::Sensitive; use serde::{Deserialize, Serialize}; use crate::alter::{alter_table_statement, AlterTableStatement}; +use crate::comment::{comment, CommentStatement}; use crate::compound_select::{compound_selection, CompoundSelectStatement}; use crate::create::{ create_cached_query, create_table, key_specification, view_creation, CreateCacheStatement, @@ -59,6 +60,7 @@ pub enum SqlQuery { Use(UseStatement), Show(ShowStatement), Explain(ExplainStatement), + Comment(CommentStatement), } impl SqlQuery { @@ -85,6 +87,7 @@ impl SqlQuery { Self::Use(use_db) => write!(f, "{}", use_db), Self::Show(show) => write!(f, "{}", show.display(dialect)), Self::Explain(explain) => write!(f, "{}", explain), + Self::Comment(c) => write!(f, "{}", c.display(dialect)), }) } } @@ -122,6 +125,7 @@ impl SqlQuery { Self::Use(_) => "USE", Self::Show(_) => "SHOW", Self::Explain(_) => "EXPLAIN", + Self::Comment(_) => "COMMENT", } } @@ -135,6 +139,16 @@ pub fn sql_query(dialect: Dialect) -> impl Fn(LocatedSpan<&[u8]>) -> NomSqlResul move |i| { // Ignore preceding whitespace or comments let (i, _) = whitespace0(i)?; + + // `alt` supports a maximum of 21 parsers, so we split the parser up to handle more + alt((sql_query_part1(dialect), sql_query_part2(dialect)))(i) + } +} + +fn sql_query_part1( + dialect: Dialect, +) -> impl Fn(LocatedSpan<&[u8]>) -> NomSqlResult<&[u8], SqlQuery> { + move |i| { alt(( map(create_table(dialect), SqlQuery::CreateTable), map(insertion(dialect), SqlQuery::Insert), @@ -160,6 +174,11 @@ pub fn sql_query(dialect: Dialect) -> impl Fn(LocatedSpan<&[u8]>) -> NomSqlResul ))(i) } } +fn sql_query_part2( + dialect: Dialect, +) -> impl Fn(LocatedSpan<&[u8]>) -> NomSqlResult<&[u8], SqlQuery> { + move |i| map(comment(dialect), SqlQuery::Comment)(i) +} macro_rules! export_parser { ($parser: ident -> $ret:ty, $parse_bytes: ident, $parse: ident) => { diff --git a/readyset-adapter/src/backend.rs b/readyset-adapter/src/backend.rs index 486580259e..c8751dc8ec 100644 --- a/readyset-adapter/src/backend.rs +++ b/readyset-adapter/src/backend.rs @@ -2171,7 +2171,10 @@ where SqlQuery::RenameTable(_) => { unsupported!("{} not yet supported", query.query_type()); } - SqlQuery::Set(_) | SqlQuery::CompoundSelect(_) | SqlQuery::Show(_) => { + SqlQuery::Set(_) + | SqlQuery::CompoundSelect(_) + | SqlQuery::Show(_) + | SqlQuery::Comment(_) => { event.sql_type = SqlQueryType::Other; upstream.query(raw_query).await.map(QueryResult::Upstream) } diff --git a/readyset-logictest/src/from_query_log.rs b/readyset-logictest/src/from_query_log.rs index e628332d04..821594e1ab 100644 --- a/readyset-logictest/src/from_query_log.rs +++ b/readyset-logictest/src/from_query_log.rs @@ -99,7 +99,8 @@ fn is_ddl(query: &SqlQuery) -> bool { | SqlQuery::Commit(_) | SqlQuery::Rollback(_) | SqlQuery::Show(_) - | SqlQuery::Explain(_) => false, + | SqlQuery::Explain(_) + | SqlQuery::Comment(_) => false, SqlQuery::CreateTable(_) | SqlQuery::CreateView(_) | SqlQuery::DropTable(_)