iffyio commented on code in PR #2250:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/2250#discussion_r2883929428
##########
src/parser/mod.rs:
##########
@@ -5100,7 +5100,14 @@ impl<'a> Parser<'a> {
let persistent = dialect_of!(self is DuckDbDialect)
&& self.parse_one_of_keywords(&[Keyword::PERSISTENT]).is_some();
let create_view_params = self.parse_create_view_params()?;
- if self.parse_keyword(Keyword::TABLE) {
+ if self.parse_keywords(&[Keyword::TEXT, Keyword::SEARCH]) {
+ if or_replace || or_alter || temporary || global.is_some() ||
transient || persistent {
+ return Err(ParserError::ParserError(
+ "CREATE TEXT SEARCH does not support CREATE
modifiers".to_string(),
+ ));
+ }
Review Comment:
```suggestion
```
I think we can skip this validation and leave up to downstream crates if
desired
##########
src/parser/mod.rs:
##########
@@ -5173,6 +5180,145 @@ impl<'a> Parser<'a> {
}
}
+ fn parse_text_search_object_type(&mut self) ->
Result<TextSearchObjectType, ParserError> {
+ match self.expect_one_of_keywords(&[
+ Keyword::DICTIONARY,
+ Keyword::CONFIGURATION,
+ Keyword::TEMPLATE,
+ Keyword::PARSER,
+ ])? {
+ Keyword::DICTIONARY => Ok(TextSearchObjectType::Dictionary),
+ Keyword::CONFIGURATION => Ok(TextSearchObjectType::Configuration),
+ Keyword::TEMPLATE => Ok(TextSearchObjectType::Template),
+ Keyword::PARSER => Ok(TextSearchObjectType::Parser),
+ // unreachable because expect_one_of_keywords used above
+ unexpected_keyword => Err(ParserError::ParserError(format!(
+ "Internal parser error: expected any of {{DICTIONARY,
CONFIGURATION, TEMPLATE, PARSER}}, got {unexpected_keyword:?}"
+ ))),
+ }
+ }
+
+ fn parse_text_search_option(&mut self) -> Result<SqlOption, ParserError> {
+ let key = self.parse_identifier()?;
+ self.expect_token(&Token::Eq)?;
+ let value = self.parse_expr()?;
+ Ok(SqlOption::KeyValue { key, value })
+ }
+
+ /// Parse a PostgreSQL `CREATE TEXT SEARCH ...` statement.
+ pub fn parse_create_text_search(&mut self) -> Result<CreateTextSearch,
ParserError> {
+ let object_type = self.parse_text_search_object_type()?;
+ let name = self.parse_object_name(false)?;
+ self.expect_token(&Token::LParen)?;
+ let options =
self.parse_comma_separated(Parser::parse_text_search_option)?;
Review Comment:
```suggestion
let options = self.parse_comma_separated(Parser::parse_sql_option)?;
```
I think we can reuse this parser given it matches the representation
##########
tests/sqlparser_postgres.rs:
##########
@@ -7932,6 +7932,73 @@ fn parse_alter_operator_class() {
.is_err());
}
+#[test]
+fn parse_create_and_alter_text_search_failure_cases() {
+ let sql_cases = [
+ "CREATE TEXT SEARCH DICTIONARY alt_ts_dict1 (template=simple)",
+ "CREATE TEXT SEARCH DICTIONARY alt_ts_dict2 (template=simple)",
Review Comment:
could we do a pass through the test cases to remove duplicate/superfluous
cases? (for example the first two highlighted here seem identical and expected
to cover the same codepaths)
##########
src/parser/mod.rs:
##########
@@ -5100,7 +5100,14 @@ impl<'a> Parser<'a> {
let persistent = dialect_of!(self is DuckDbDialect)
&& self.parse_one_of_keywords(&[Keyword::PERSISTENT]).is_some();
let create_view_params = self.parse_create_view_params()?;
- if self.parse_keyword(Keyword::TABLE) {
+ if self.parse_keywords(&[Keyword::TEXT, Keyword::SEARCH]) {
Review Comment:
instead of parse, can we 'peek' `TEXT, SEARCH`? so that
`parse_create_text_search()` can be somewhat standalone
##########
tests/sqlparser_postgres.rs:
##########
@@ -7932,6 +7932,73 @@ fn parse_alter_operator_class() {
.is_err());
}
+#[test]
+fn parse_create_and_alter_text_search_failure_cases() {
+ let sql_cases = [
+ "CREATE TEXT SEARCH DICTIONARY alt_ts_dict1 (template=simple)",
+ "CREATE TEXT SEARCH DICTIONARY alt_ts_dict2 (template=simple)",
+ "ALTER TEXT SEARCH DICTIONARY alt_ts_dict1 RENAME TO alt_ts_dict2",
+ "ALTER TEXT SEARCH DICTIONARY alt_ts_dict1 RENAME TO alt_ts_dict3",
+ "ALTER TEXT SEARCH DICTIONARY alt_ts_dict2 OWNER TO
regress_alter_generic_user2",
+ "ALTER TEXT SEARCH DICTIONARY alt_ts_dict2 OWNER TO
regress_alter_generic_user3",
+ "ALTER TEXT SEARCH DICTIONARY alt_ts_dict2 SET SCHEMA alt_nsp2",
+ "ALTER TEXT SEARCH DICTIONARY alt_ts_dict3 RENAME TO alt_ts_dict4",
+ "ALTER TEXT SEARCH DICTIONARY alt_ts_dict1 RENAME TO alt_ts_dict4",
+ "ALTER TEXT SEARCH DICTIONARY alt_ts_dict3 OWNER TO
regress_alter_generic_user2",
+ "ALTER TEXT SEARCH DICTIONARY alt_ts_dict3 SET SCHEMA alt_nsp2",
+ "CREATE TEXT SEARCH CONFIGURATION alt_ts_conf1 (copy=english)",
+ "CREATE TEXT SEARCH CONFIGURATION alt_ts_conf2 (copy=english)",
+ "ALTER TEXT SEARCH CONFIGURATION alt_ts_conf1 RENAME TO alt_ts_conf2",
+ "ALTER TEXT SEARCH CONFIGURATION alt_ts_conf1 RENAME TO alt_ts_conf3",
+ "ALTER TEXT SEARCH CONFIGURATION alt_ts_conf2 OWNER TO
regress_alter_generic_user2",
+ "ALTER TEXT SEARCH CONFIGURATION alt_ts_conf2 OWNER TO
regress_alter_generic_user3",
+ "ALTER TEXT SEARCH CONFIGURATION alt_ts_conf2 SET SCHEMA alt_nsp2",
+ "ALTER TEXT SEARCH CONFIGURATION alt_ts_conf3 RENAME TO alt_ts_conf4",
+ "ALTER TEXT SEARCH CONFIGURATION alt_ts_conf1 RENAME TO alt_ts_conf4",
+ "ALTER TEXT SEARCH CONFIGURATION alt_ts_conf3 OWNER TO
regress_alter_generic_user2",
+ "ALTER TEXT SEARCH CONFIGURATION alt_ts_conf3 SET SCHEMA alt_nsp2",
+ "CREATE TEXT SEARCH TEMPLATE alt_ts_temp1 (lexize=dsimple_lexize)",
+ "CREATE TEXT SEARCH TEMPLATE alt_ts_temp2 (lexize=dsimple_lexize)",
+ "ALTER TEXT SEARCH TEMPLATE alt_ts_temp1 RENAME TO alt_ts_temp2",
+ "ALTER TEXT SEARCH TEMPLATE alt_ts_temp1 RENAME TO alt_ts_temp3",
+ "ALTER TEXT SEARCH TEMPLATE alt_ts_temp2 SET SCHEMA alt_nsp2",
+ "CREATE TEXT SEARCH TEMPLATE tstemp_case (\"Init\" = init_function)",
+ "CREATE TEXT SEARCH PARSER alt_ts_prs1 (start = prsd_start, gettoken =
prsd_nexttoken, end = prsd_end, lextypes = prsd_lextype)",
+ "CREATE TEXT SEARCH PARSER alt_ts_prs2 (start = prsd_start, gettoken =
prsd_nexttoken, end = prsd_end, lextypes = prsd_lextype)",
+ "ALTER TEXT SEARCH PARSER alt_ts_prs1 RENAME TO alt_ts_prs2",
+ "ALTER TEXT SEARCH PARSER alt_ts_prs1 RENAME TO alt_ts_prs3",
+ "ALTER TEXT SEARCH PARSER alt_ts_prs2 SET SCHEMA alt_nsp2",
+ "CREATE TEXT SEARCH PARSER tspars_case (\"Start\" = start_function)",
+ ];
+
+ for sql in sql_cases {
+ if let Err(err) = pg().parse_sql_statements(sql) {
Review Comment:
can the tests use verified_statement, also `pg_and_generic()` where possible?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
