Repository: risingwavelabs/risingwave (language: python)

Issue:
Sqlsmith: Sql feature generation
As we add more features, these should be tested via sqlsmith as well.

SQL Features:
- [ ] Generate other table functions: `generate_series`, `unnest`. See: https://github.com/singularity-data/risingwave/issues/4540
- [ ] Generate array_agg, jsonb_agg, jsonb_object_agg
- [ ] Generate select from System Tables
- [ ] Generate `NATURAL JOIN`
- [ ] Generate `JOIN` with `USING`
- [ ] Generate query w/o `GROUP BY`
- [ ] Randomly toggle session variables
- [ ] Generate `pow`/`exp`.
- [ ] Generate TopN by group: https://www.risingwave.dev/docs/current/sql-pattern-topn/
- [ ] Generate jsonb exprs, see: https://github.com/risingwavelabs/risingwave/issues/7714
- [ ] #7132
- [x] Generate order-by-clauses with `NULLS { FIRST | LAST }` (will be done in https://github.com/risingwavelabs/risingwave/pull/10864/commits/000cbc11cbc04f20abfd127b1bf4b3fd24e75c44)
- [ ] Generate `max`,`min`,`count` for `timestamptz`
- [ ] Generate `bit_and`, `bit_or` and `big_xor` in batch queries, and `bit_xor` in streaming queries.
- [ ] Generate array range access.
- [ ] https://github.com/risingwavelabs/risingwave/issues/10241
- [ ] Generate window functions other than `row_number`, `rank`.
- [ ] Generate `ALTER` DDL statements.\
- [ ] https://github.com/risingwavelabs/risingwave/issues/10516
- [ ] Generate temporal filter in sqlsmith


Discussion / hints:

Fix patch:
diff --git a/src/frontend/src/utils/with_options.rs b/src/frontend/src/utils/with_options.rs
index 62d0bd2170e6..b2c564098a45 100644
--- a/src/frontend/src/utils/with_options.rs
+++ b/src/frontend/src/utils/with_options.rs
@@ -30,7 +30,8 @@ use risingwave_pb::secret::secret_ref::PbRefAsType;
 use risingwave_pb::secret::PbSecretRef;
 use risingwave_sqlparser::ast::{
     ConnectionRefValue, CreateConnectionStatement, CreateSinkStatement, CreateSourceStatement,
-    CreateSubscriptionStatement, SecretRefAsType, SecretRefValue, SqlOption, Statement, Value,
+    CreateSubscriptionStatement, SecretRefAsType, SecretRefValue, SqlOption, SqlOptionValue,
+    Statement, Value,
 };
 
 use super::OverwriteOptions;
@@ -346,7 +347,7 @@ impl TryFrom<&[SqlOption]> for WithOptions {
         for option in options {
             let key = option.name.real_value();
             match &option.value {
-                Value::SecretRef(r) => {
+                SqlOptionValue::SecretRef(r) => {
                     if secret_ref.insert(key.clone(), r.clone()).is_some()
                         || inner.contains_key(&key)
                     {
@@ -357,7 +358,7 @@ impl TryFrom<&[SqlOption]> for WithOptions {
                     }
                     continue;
                 }
-                Value::ConnectionRef(r) => {
+                SqlOptionValue::ConnectionRef(r) => {
                     if key != CONNECTION_REF_KEY {
                         return Err(RwError::from(ErrorCode::InvalidParameterValue(format!(
                             "expect 'profile' as the key for connection ref, but got: {}",
@@ -377,10 +378,10 @@ impl TryFrom<&[SqlOption]> for WithOptions {
                 _ => {}
             }
             let value: String = match option.value.clone() {
-                Value::CstyleEscapedString(s) => s.value,
-                Value::SingleQuotedString(s) => s,
-                Value::Number(n) => n,
-                Value::Boolean(b) => b.to_string(),
+                SqlOptionValue::Value(Value::CstyleEscapedString(s)) => s.value,
+                SqlOptionValue::Value(Value::SingleQuotedString(s)) => s,
+                SqlOptionValue::Value(Value::Number(n)) => n,
+                SqlOptionValue::Value(Value::Boolean(b)) => b.to_string(),
                 _ => {
                     return Err(RwError::from(ErrorCode::InvalidParameterValue(
                         "`with options` or `with properties` only support single quoted string value and C style escaped string"
diff --git a/src/meta/src/manager/diagnose.rs b/src/meta/src/manager/diagnose.rs
index fd4d67c25668..33a97537ab46 100644
--- a/src/meta/src/manager/diagnose.rs
+++ b/src/meta/src/manager/diagnose.rs
@@ -759,12 +759,12 @@ fn redact_all_sql_options(sql: &str) -> Option<String> {
         };
         if let Some(options) = options.0 {
             for option in options {
-                option.value = Value::SingleQuotedString("[REDACTED]".into());
+                option.value = Value::SingleQuotedString("[REDACTED]".into()).into();
             }
         }
         if let Some(options) = options.1 {
             for option in options {
-                option.value = Value::SingleQuotedString("[REDACTED]".into());
+                option.value = Value::SingleQuotedString("[REDACTED]".into()).into();
             }
         }
         writeln!(&mut redacted, "{statement}").unwrap();
diff --git a/src/sqlparser/src/ast/legacy_source.rs b/src/sqlparser/src/ast/legacy_source.rs
index 7a5abf35a2df..592cbf79c581 100644
--- a/src/sqlparser/src/ast/legacy_source.rs
+++ b/src/sqlparser/src/ast/legacy_source.rs
@@ -163,7 +163,7 @@ impl LegacyRowFormat {
                         value: "message".into(),
                         quote_style: None,
                     }]),
-                    value: Value::SingleQuotedString(schema.message_name.0),
+                    value: Value::SingleQuotedString(schema.message_name.0).into(),
                 }];
                 if schema.use_schema_registry {
                     options.push(SqlOption {
@@ -171,7 +171,7 @@ impl LegacyRowFormat {
                             value: "schema.registry".into(),
                             quote_style: None,
                         }]),
-                        value: Value::SingleQuotedString(schema.row_schema_location.0),
+                        value: Value::SingleQuotedString(schema.row_schema_location.0).into(),
                     });
                 } else {
                     options.push(SqlOption {
@@ -179,7 +179,7 @@ impl LegacyRowFormat {
                             value: "schema.location".into(),
                             quote_style: None,
                         }]),
-                        value: Value::SingleQuotedString(schema.row_schema_location.0),
+                        value: Value::SingleQuotedString(schema.row_schema_location.0).into(),
                     })
                 }
                 options
@@ -191,7 +191,7 @@ impl LegacyRowFormat {
                             value: "schema.registry".into(),
                             quote_style: None,
                         }]),
-                        value: Value::SingleQuotedString(schema.row_schema_location.0),
+                        value: Value::SingleQuotedString(schema.row_schema_location.0).into(),
                     }]
                 } else {
                     vec![SqlOption {
@@ -199,7 +199,7 @@ impl LegacyRowFormat {
                             value: "schema.location".into(),
                             quote_style: None,
                         }]),
-                        value: Value::SingleQuotedString(schema.row_schema_location.0),
+                        value: Value::SingleQuotedString(schema.row_schema_location.0).into(),
                     }]
                 }
             }
@@ -209,7 +209,7 @@ impl LegacyRowFormat {
                         value: "schema.registry".into(),
                         quote_style: None,
                     }]),
-                    value: Value::SingleQuotedString(schema.row_schema_location.0),
+                    value: Value::SingleQuotedString(schema.row_schema_location.0).into(),
                 }]
             }
             LegacyRowFormat::Csv(schema) => {
@@ -221,7 +221,8 @@ impl LegacyRowFormat {
                         }]),
                         value: Value::SingleQuotedString(
                             String::from_utf8_lossy(&[schema.delimiter]).into(),
-                        ),
+                        )
+                        .into(),
                     },
                     SqlOption {
                         name: ObjectName(vec![Ident {
@@ -232,7 +233,8 @@ impl LegacyRowFormat {
                             "false".into()
                         } else {
                             "true".into()
-                        }),
+                        })
+                        .into(),
                     },
                 ]
             }
diff --git a/src/sqlparser/src/ast/mod.rs b/src/sqlparser/src/ast/mod.rs
index bae38996066e..9c48f1d03ce1 100644
--- a/src/sqlparser/src/ast/mod.rs
+++ b/src/sqlparser/src/ast/mod.rs
@@ -2736,7 +2736,7 @@ impl ParseTo for ObjectType {
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 pub struct SqlOption {
     pub name: ObjectName,
-    pub value: Value,
+    pub value: SqlOptionValue,
 }
 
 impl fmt::Display for SqlOption {
@@ -2755,6 +2755,44 @@ impl fmt::Display for SqlOption {
     }
 }
 
+#[derive(Clone, PartialEq, Eq, Hash)]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+pub enum SqlOptionValue {
+    Value(Value),
+    SecretRef(SecretRefValue),
+    ConnectionRef(ConnectionRefValue),
+}
+
+impl fmt::Debug for SqlOptionValue {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            SqlOptionValue::Value(value) => write!(f, "{:?}", value),
+            SqlOptionValue::SecretRef(secret_ref) => write!(f, "secret {:?}", secret_ref),
+            SqlOptionValue::ConnectionRef(connection_ref) => {
+                write!(f, "connection {:?}", connection_ref)
+            }
+        }
+    }
+}
+
+impl fmt::Display for SqlOptionValue {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            SqlOptionValue::Value(value) => write!(f, "{}", value),
+            SqlOptionValue::SecretRef(secret_ref) => write!(f, "secret {}", secret_ref),
+            SqlOptionValue::ConnectionRef(connection_ref) => {
+                write!(f, "connection {}", connection_ref)
+            }
+        }
+    }
+}
+
+impl From<Value> for SqlOptionValue {
+    fn from(value: Value) -> Self {
+        SqlOptionValue::Value(value)
+    }
+}
+
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 pub enum EmitMode {
@@ -3148,7 +3186,10 @@ impl TryFrom<Vec<SqlOption>> for CreateFunctionWithOptions {
         let mut always_retry_on_network_error = None;
         for option in with_options {
             if option.name.to_string().to_lowercase() == "always_retry_on_network_error" {
-                always_retry_on_network_error = Some(option.value == Value::Boolean(true));
+                always_retry_on_network_error = Some(matches!(
+                    option.value,
+                    SqlOptionValue::Value(Value::Boolean(true))
+                ));
             } else {
                 return Err(StrError(format!("Unsupported option: {}", option.name)));
             }
diff --git a/src/sqlparser/src/ast/statement.rs b/src/sqlparser/src/ast/statement.rs
index 72680161defe..4d4b62f8f97e 100644
--- a/src/sqlparser/src/ast/statement.rs
+++ b/src/sqlparser/src/ast/statement.rs
@@ -845,7 +845,7 @@ impl ParseTo for CreateSecretStatement {
         impl_parse_to!(with_properties: WithProperties, parser);
         let mut credential = Value::Null;
         if parser.parse_keyword(Keyword::AS) {
-            credential = parser.parse_value()?;
+            credential = parser.ensure_parse_value()?;
         }
         Ok(Self {
             if_not_exists,
diff --git a/src/sqlparser/src/ast/value.rs b/src/sqlparser/src/ast/value.rs
index 051e0814a3d9..7f6cdb048b4e 100644
--- a/src/sqlparser/src/ast/value.rs
+++ b/src/sqlparser/src/ast/value.rs
@@ -59,10 +59,6 @@ pub enum Value {
     },
     /// `NULL` value
     Null,
-    /// name of the reference to secret
-    SecretRef(SecretRefValue),
-    /// name of the reference to connection
-    ConnectionRef(ConnectionRefValue),
 }
 
 impl fmt::Display for Value {
@@ -117,8 +113,6 @@ impl fmt::Display for Value {
                 Ok(())
             }
             Value::Null => write!(f, "NULL"),
-            Value::SecretRef(v) => write!(f, "secret {}", v),
-            Value::ConnectionRef(v) => write!(f, "connection {}", v),
         }
     }
 }
diff --git a/src/sqlparser/src/parser.rs b/src/sqlparser/src/parser.rs
index 9d7f308b7833..bae54c21124a 100644
--- a/src/sqlparser/src/parser.rs
+++ b/src/sqlparser/src/parser.rs
@@ -583,7 +583,7 @@ impl Parser<'_> {
             Token::Word(w) => match w.keyword {
                 Keyword::TRUE | Keyword::FALSE | Keyword::NULL => {
                     *self = checkpoint;
-                    Ok(Expr::Value(self.parse_value()?))
+                    Ok(Expr::Value(self.ensure_parse_value()?))
                 }
                 Keyword::CASE => self.parse_case_expr(),
                 Keyword::CAST => self.parse_cast_expr(),
@@ -706,7 +706,7 @@ impl Parser<'_> {
             | Token::HexStringLiteral(_)
             | Token::CstyleEscapesString(_) => {
                 *self = checkpoint;
-                Ok(Expr::Value(self.parse_value()?))
+                Ok(Expr::Value(self.ensure_parse_value()?))
             }
             Token::Parameter(number) => self.parse_param(number),
             Token::Pipe => {
@@ -2941,7 +2941,7 @@ impl Parser<'_> {
     pub fn parse_sql_option(&mut self) -> PResult<SqlOption> {
         let name = self.parse_object_name()?;
         self.expect_token(&Token::Eq)?;
-        let value = self.parse_value()?;
+        let value = self.parse_value_and_obj_ref::<false>()?;
         Ok(SqlOption { name, value })
     }
 
@@ -3592,44 +3592,69 @@ impl Parser<'_> {
         values
     }
 
+    pub fn ensure_parse_value(&mut self) -> PResult<Value> {
+        match self.parse_value_and_obj_ref::<true>()? {
+            SqlOptionValue::Value(value) => Ok(value),
+            SqlOptionValue::SecretRef(_) | SqlOptionValue::ConnectionRef(_) => unreachable!(),
+        }
+    }
+
     /// Parse a literal value (numbers, strings, date/time, booleans)
-    pub fn parse_value(&mut self) -> PResult<Value> {
+    pub fn parse_value_and_obj_ref<const FORBID_OBJ_REF: bool>(
+        &mut self,
+    ) -> PResult<SqlOptionValue> {
         let checkpoint = *self;
         let token = self.next_token();
         match token.token {
             Token::Word(w) => match w.keyword {
-                Keyword::TRUE => Ok(Value::Boolean(true)),
-                Keyword::FALSE => Ok(Value::Boolean(false)),
-                Keyword::NULL => Ok(Value::Null),
+                Keyword::TRUE => Ok(Value::Boolean(true).into()),
+                Keyword::FALSE => Ok(Value::Boolean(false).into()),
+                Keyword::NULL => Ok(Value::Null.into()),
                 Keyword::NoKeyword if w.quote_style.is_some() => match w.quote_style {
-                    Some('"') => Ok(Value::DoubleQuotedString(w.value)),
-                    Some('\'') => Ok(Value::SingleQuotedString(w.value)),
+                    Some('"') => Ok(Value::DoubleQuotedString(w.value).into()),
+                    Some('\'') => Ok(Value::SingleQuotedString(w.value).into()),
                     _ => self.expected_at(checkpoint, "A value")?,
                 },
                 Keyword::SECRET => {
+                    if FORBID_OBJ_REF {
+                        return self.expected_at(
+                            checkpoint,
+                            "a concrete value rather than a secret reference",
+                        );
+                    }
                     let secret_name = self.parse_object_name()?;
                     let ref_as = if self.parse_keywords(&[Keyword::AS, Keyword::FILE]) {
                         SecretRefAsType::File
                     } else {
                         SecretRefAsType::Text
                     };
-                    Ok(Value::SecretRef(SecretRefValue {
+                    Ok(SqlOptionValue::SecretRef(SecretRefValue {
                         secret_name,
                         ref_as,
                     }))
                 }
                 Keyword::CONNECTION => {
+                    if FORBID_OBJ_REF {
+                        return self.expected_at(
+                            checkpoint,
+                            "a concrete value rather than a connection reference",
+                        );
+                    }
                     let connection_name = self.parse_object_name()?;
-                    Ok(Value::ConnectionRef(ConnectionRefValue { connection_name }))
+                    Ok(SqlOptionValue::ConnectionRef(ConnectionRefValue {
+                        connection_name,
+                    }))
                 }
                 _ => self.expected_at(checkpoint, "a concrete value"),
             },
-            Token::Number(ref n) => Ok(Value::Number(n.clone())),
-            Token::SingleQuotedString(ref s) => Ok(Value::SingleQuotedString(s.to_string())),
-            Token::DollarQuotedString(ref s) => Ok(Value::DollarQuotedString(s.clone())),
-            Token::CstyleEscapesString(ref s) => Ok(Value::CstyleEscapedString(s.clone())),
-            Token::NationalStringLiteral(ref s) => Ok(Value::NationalStringLiteral(s.to_string())),
-            Token::HexStringLiteral(ref s) => Ok(Value::HexStringLiteral(s.to_string())),
+            Token::Number(ref n) => Ok(Value::Number(n.clone()).into()),
+            Token::SingleQuotedString(ref s) => Ok(Value::SingleQuotedString(s.to_string()).into()),
+            Token::DollarQuotedString(ref s) => Ok(Value::DollarQuotedString(s.clone()).into()),
+            Token::CstyleEscapesString(ref s) => Ok(Value::CstyleEscapedString(s.clone()).into()),
+            Token::NationalStringLiteral(ref s) => {
+                Ok(Value::NationalStringLiteral(s.to_string()).into())
+            }
+            Token::HexStringLiteral(ref s) => Ok(Value::HexStringLiteral(s.to_string()).into()),
             _ => self.expected_at(checkpoint, "a value"),
         }
     }
@@ -3640,7 +3665,7 @@ impl Parser<'_> {
             separated(
                 1..,
                 alt((
-                    Self::parse_value.map(SetVariableValueSingle::Literal),
+                    Self::ensure_parse_value.map(SetVariableValueSingle::Literal),
                     |parser: &mut Self| {
                         let checkpoint = *parser;
                         let ident = parser.parse_identifier()?;
@@ -3667,7 +3692,7 @@ impl Parser<'_> {
 
     pub fn parse_number_value(&mut self) -> PResult<String> {
         let checkpoint = *self;
-        match self.parse_value()? {
+        match self.ensure_parse_value()? {
             Value::Number(v) => Ok(v),
             _ => self.expected_at(checkpoint, "literal number"),
         }
@@ -4352,7 +4377,7 @@ impl Parser<'_> {
                     })),
                 ),
                 Self::parse_identifier.map(SetTimeZoneValue::Ident),
-                Self::parse_value.map(SetTimeZoneValue::Literal),
+                Self::ensure_parse_value.map(SetTimeZoneValue::Literal),
             ))
             .expect("variable")
             .parse_next(self)?;
@@ -4371,7 +4396,7 @@ impl Parser<'_> {
             })
         } else if self.parse_keyword(Keyword::TRANSACTION) && modifier.is_none() {
             if self.parse_keyword(Keyword::SNAPSHOT) {
-                let snapshot_id = self.parse_value()?;
+                let snapshot_id = self.ensure_parse_value()?;
                 return Ok(Statement::SetTransaction {
                     modes: vec![],
                     snapshot: Some(snapshot_id),


Test patch:
diff --git a/src/sqlparser/tests/sqlparser_common.rs b/src/sqlparser/tests/sqlparser_common.rs
index c8f6fb41d32a..71af4bba2987 100644
--- a/src/sqlparser/tests/sqlparser_common.rs
+++ b/src/sqlparser/tests/sqlparser_common.rs
@@ -1543,11 +1543,11 @@ fn parse_create_table_with_options() {
                 vec![
                     SqlOption {
                         name: vec!["foo".into()].into(),
-                        value: Value::SingleQuotedString("bar".into())
+                        value: Value::SingleQuotedString("bar".into()).into(),
                     },
                     SqlOption {
                         name: vec!["a".into()].into(),
-                        value: number("123")
+                        value: number("123").into(),
                     },
                 ],
                 with_options
@@ -3145,11 +3145,11 @@ fn parse_create_view_with_options() {
                 vec![
                     SqlOption {
                         name: vec!["foo".into()].into(),
-                        value: Value::SingleQuotedString("bar".into())
+                        value: Value::SingleQuotedString("bar".into()).into(),
                     },
                     SqlOption {
                         name: vec!["a".into()].into(),
-                        value: number("123")
+                        value: number("123").into(),
                     },
                 ],
                 with_options
diff --git a/src/sqlparser/tests/sqlparser_postgres.rs b/src/sqlparser/tests/sqlparser_postgres.rs
index 549920d1c758..a2edb08edda4 100644
--- a/src/sqlparser/tests/sqlparser_postgres.rs
+++ b/src/sqlparser/tests/sqlparser_postgres.rs
@@ -159,15 +159,15 @@ fn parse_create_table_with_defaults() {
                 vec![
                     SqlOption {
                         name: vec!["fillfactor".into()].into(),
-                        value: number("20")
+                        value: number("20").into(),
                     },
                     SqlOption {
                         name: vec!["user_catalog_table".into()].into(),
-                        value: Value::Boolean(true)
+                        value: Value::Boolean(true).into(),
                     },
                     SqlOption {
                         name: vec!["autovacuum_vacuum_threshold".into()].into(),
-                        value: number("100")
+                        value: number("100").into(),
                     },
                 ]
             );