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.

Sign up for GitHub

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

Handle CREATE [TEMPORARY|TEMP] VIEW [IF NOT EXISTS] #993

Merged
merged 1 commit into from
Oct 5, 2023
Merged

Handle CREATE [TEMPORARY|TEMP] VIEW [IF NOT EXISTS] #993

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 10 additions & 2 deletions src/ast/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1318,6 +1318,10 @@ pub enum Statement {
cluster_by: Vec<Ident>,
/// if true, has RedShift [`WITH NO SCHEMA BINDING`] clause <https://docs.aws.amazon.com/redshift/latest/dg/r_CREATE_VIEW.html>
with_no_schema_binding: bool,
/// if true, has SQLite `IF NOT EXISTS` clause <https://www.sqlite.org/lang_createview.html>
if_not_exists: bool,
/// if true, has SQLite `TEMP` or `TEMPORARY` clause <https://www.sqlite.org/lang_createview.html>
temporary: bool,
},
/// CREATE TABLE
CreateTable {
Expand Down Expand Up @@ -2274,13 +2278,17 @@ impl fmt::Display for Statement {
with_options,
cluster_by,
with_no_schema_binding,
if_not_exists,
temporary,
} => {
write!(
f,
"CREATE {or_replace}{materialized}VIEW {name}",
"CREATE {or_replace}{materialized}{temporary}VIEW {if_not_exists}{name}",
or_replace = if *or_replace { "OR REPLACE " } else { "" },
materialized = if *materialized { "MATERIALIZED " } else { "" },
name = name
name = name,
temporary = if *temporary { "TEMPORARY " } else { "" },
if_not_exists = if *if_not_exists { "IF NOT EXISTS " } else { "" }
)?;
if !with_options.is_empty() {
write!(f, " WITH ({})", display_comma_separated(with_options))?;
Expand Down
12 changes: 10 additions & 2 deletions src/parser/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2478,7 +2478,7 @@ impl<'a> Parser<'a> {
self.parse_create_table(or_replace, temporary, global, transient)
} else if self.parse_keyword(Keyword::MATERIALIZED) || self.parse_keyword(Keyword::VIEW) {
self.prev_token();
self.parse_create_view(or_replace)
self.parse_create_view(or_replace, temporary)
} else if self.parse_keyword(Keyword::EXTERNAL) {
self.parse_create_external_table(or_replace)
} else if self.parse_keyword(Keyword::FUNCTION) {
Expand Down Expand Up @@ -2955,9 +2955,15 @@ impl<'a> Parser<'a> {
}
}

pub fn parse_create_view(&mut self, or_replace: bool) -> Result<Statement, ParserError> {
pub fn parse_create_view(
&mut self,
or_replace: bool,
temporary: bool,
) -> Result<Statement, ParserError> {
let materialized = self.parse_keyword(Keyword::MATERIALIZED);
self.expect_keyword(Keyword::VIEW)?;
let if_not_exists = dialect_of!(self is SQLiteDialect|GenericDialect)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this on purpose ? Most other constructs do not generate parse errors when parsed with the "wrong" dialect. Is there a rule for how sqlparser handles that, or does every contributor do it their own way ?

I was under the impression that dialects allowed disambiguating syntax that would parse differently in different databases, but did not cause a parse error when parsing unambiguous syntax from one dialect with a different one.

@alamb

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/gabivlj/sqlparser-rs/blob/96c5979481536c6f6fe3193674059b2fff1d81b7/src/parser/mod.rs#L2984-L2990

For context, I took the idea from already existing RedshiftSqlDialect with_no_schema_binding. The intent here was excluding Dialects that do not support CREATE VIEW IF NOT EXISTS, for example the most similar feature for Postgresql is CREATE OR REPLACE VIEW as IF NOT EXISTS on a view is not supported there.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I see that. There does not seem to be a coherent rule over the entire codebase for whether database-specific syntax should cause a syntax error when parsed with a different dialect.

Copy link
Contributor

@alamb alamb Oct 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I see that. There does not seem to be a coherent rule over the entire codebase for whether database-specific syntax should cause a syntax error when parsed with a different dialect.

I agree this is not uniform in the codebase and I think sqlparser-rs is more permissive than most other parsers in the sense that it will parse SQL that would generate a syntax error in the original implementation. This is broadly consistent with the sentiment of https://github.com/sqlparser-rs/sqlparser-rs#syntax-vs-semantics

In general I expect that the GenericDialect will parse the union of all other dialects, as long as they aren't in conflict with each other (e.g. the rules on how to parse strings vs identifiers differ in some dialects)

&& self.parse_keywords(&[Keyword::IF, Keyword::NOT, Keyword::EXISTS]);
// Many dialects support `OR ALTER` right after `CREATE`, but we don't (yet).
// ANSI SQL and Postgres support RECURSIVE here, but we don't support it either.
let name = self.parse_object_name()?;
Expand Down Expand Up @@ -2992,6 +2998,8 @@ impl<'a> Parser<'a> {
with_options,
cluster_by,
with_no_schema_binding,
if_not_exists,
temporary,
})
}

Expand Down
55 changes: 55 additions & 0 deletions tests/sqlparser_common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5321,6 +5321,8 @@ fn parse_create_view() {
with_options,
cluster_by,
with_no_schema_binding: late_binding,
if_not_exists,
temporary,
} => {
assert_eq!("myschema.myview", name.to_string());
assert_eq!(Vec::<Ident>::new(), columns);
Expand All @@ -5330,6 +5332,8 @@ fn parse_create_view() {
assert_eq!(with_options, vec![]);
assert_eq!(cluster_by, vec![]);
assert!(!late_binding);
assert!(!if_not_exists);
assert!(!temporary);
}
_ => unreachable!(),
}
Expand Down Expand Up @@ -5371,6 +5375,8 @@ fn parse_create_view_with_columns() {
materialized,
cluster_by,
with_no_schema_binding: late_binding,
if_not_exists,
temporary,
} => {
assert_eq!("v", name.to_string());
assert_eq!(columns, vec![Ident::new("has"), Ident::new("cols")]);
Expand All @@ -5380,6 +5386,39 @@ fn parse_create_view_with_columns() {
assert!(!or_replace);
assert_eq!(cluster_by, vec![]);
assert!(!late_binding);
assert!(!if_not_exists);
assert!(!temporary);
}
_ => unreachable!(),
}
}

#[test]
fn parse_create_view_temporary() {
let sql = "CREATE TEMPORARY VIEW myschema.myview AS SELECT foo FROM bar";
match verified_stmt(sql) {
Statement::CreateView {
name,
columns,
query,
or_replace,
materialized,
with_options,
cluster_by,
with_no_schema_binding: late_binding,
if_not_exists,
temporary,
} => {
assert_eq!("myschema.myview", name.to_string());
assert_eq!(Vec::<Ident>::new(), columns);
assert_eq!("SELECT foo FROM bar", query.to_string());
assert!(!materialized);
assert!(!or_replace);
assert_eq!(with_options, vec![]);
assert_eq!(cluster_by, vec![]);
assert!(!late_binding);
assert!(!if_not_exists);
assert!(temporary);
}
_ => unreachable!(),
}
Expand All @@ -5398,6 +5437,8 @@ fn parse_create_or_replace_view() {
materialized,
cluster_by,
with_no_schema_binding: late_binding,
if_not_exists,
temporary,
} => {
assert_eq!("v", name.to_string());
assert_eq!(columns, vec![]);
Expand All @@ -5407,6 +5448,8 @@ fn parse_create_or_replace_view() {
assert!(or_replace);
assert_eq!(cluster_by, vec![]);
assert!(!late_binding);
assert!(!if_not_exists);
assert!(!temporary);
}
_ => unreachable!(),
}
Expand All @@ -5429,6 +5472,8 @@ fn parse_create_or_replace_materialized_view() {
materialized,
cluster_by,
with_no_schema_binding: late_binding,
if_not_exists,
temporary,
} => {
assert_eq!("v", name.to_string());
assert_eq!(columns, vec![]);
Expand All @@ -5438,6 +5483,8 @@ fn parse_create_or_replace_materialized_view() {
assert!(or_replace);
assert_eq!(cluster_by, vec![]);
assert!(!late_binding);
assert!(!if_not_exists);
assert!(!temporary);
}
_ => unreachable!(),
}
Expand All @@ -5456,6 +5503,8 @@ fn parse_create_materialized_view() {
with_options,
cluster_by,
with_no_schema_binding: late_binding,
if_not_exists,
temporary,
} => {
assert_eq!("myschema.myview", name.to_string());
assert_eq!(Vec::<Ident>::new(), columns);
Expand All @@ -5465,6 +5514,8 @@ fn parse_create_materialized_view() {
assert!(!or_replace);
assert_eq!(cluster_by, vec![]);
assert!(!late_binding);
assert!(!if_not_exists);
assert!(!temporary);
}
_ => unreachable!(),
}
Expand All @@ -5483,6 +5534,8 @@ fn parse_create_materialized_view_with_cluster_by() {
with_options,
cluster_by,
with_no_schema_binding: late_binding,
if_not_exists,
temporary,
} => {
assert_eq!("myschema.myview", name.to_string());
assert_eq!(Vec::<Ident>::new(), columns);
Expand All @@ -5492,6 +5545,8 @@ fn parse_create_materialized_view_with_cluster_by() {
assert!(!or_replace);
assert_eq!(cluster_by, vec![Ident::new("foo")]);
assert!(!late_binding);
assert!(!if_not_exists);
assert!(!temporary);
}
_ => unreachable!(),
}
Expand Down
31 changes: 31 additions & 0 deletions tests/sqlparser_sqlite.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,37 @@ fn parse_create_virtual_table() {
sqlite_and_generic().verified_stmt(sql);
}

#[test]
fn parse_create_view_temporary_if_not_exists() {
let sql = "CREATE TEMPORARY VIEW IF NOT EXISTS myschema.myview AS SELECT foo FROM bar";
match sqlite_and_generic().verified_stmt(sql) {
Statement::CreateView {
name,
columns,
query,
or_replace,
materialized,
with_options,
cluster_by,
with_no_schema_binding: late_binding,
if_not_exists,
temporary,
} => {
assert_eq!("myschema.myview", name.to_string());
assert_eq!(Vec::<Ident>::new(), columns);
assert_eq!("SELECT foo FROM bar", query.to_string());
assert!(!materialized);
assert!(!or_replace);
assert_eq!(with_options, vec![]);
assert_eq!(cluster_by, vec![]);
assert!(!late_binding);
assert!(if_not_exists);
assert!(temporary);
}
_ => unreachable!(),
}
}

#[test]
fn double_equality_operator() {
// Sqlite supports this operator: https://www.sqlite.org/lang_expr.html#binaryops
Expand Down