Skip to content

Commit

Permalink
refactor: make object ref outside ast::Value (#19559)
Browse files Browse the repository at this point in the history 
Co-authored-by: tabversion <[email protected]>
  • Loading branch information
@tabVersion
tabVersion and tabversion authored Nov 25, 2024
1 parent 28bb651 commit 9c0e9df
Show file tree
Hide file tree
Showing 9 changed files with 117 additions and 54 deletions.
15 changes: 8 additions & 7 deletions src/frontend/src/utils/with_options.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
{
Expand All @@ -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: {}",
Expand All @@ -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"
Expand Down
4 changes: 2 additions & 2 deletions src/meta/src/manager/diagnose.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
18 changes: 10 additions & 8 deletions src/sqlparser/src/ast/legacy_source.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,23 +163,23 @@ 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 {
name: ObjectName(vec![Ident {
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 {
name: ObjectName(vec![Ident {
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
Expand All @@ -191,15 +191,15 @@ 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 {
name: ObjectName(vec![Ident {
value: "schema.location".into(),
quote_style: None,
}]),
value: Value::SingleQuotedString(schema.row_schema_location.0),
value: Value::SingleQuotedString(schema.row_schema_location.0).into(),
}]
}
}
Expand All @@ -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) => {
Expand All @@ -221,7 +221,8 @@ impl LegacyRowFormat {
}]),
value: Value::SingleQuotedString(
String::from_utf8_lossy(&[schema.delimiter]).into(),
),
)
.into(),
},
SqlOption {
name: ObjectName(vec![Ident {
Expand All @@ -232,7 +233,8 @@ impl LegacyRowFormat {
"false".into()
} else {
"true".into()
}),
})
.into(),
},
]
}
Expand Down
45 changes: 43 additions & 2 deletions src/sqlparser/src/ast/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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)));
}
Expand Down
2 changes: 1 addition & 1 deletion src/sqlparser/src/ast/statement.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 0 additions & 6 deletions src/sqlparser/src/ast/value.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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),
}
}
}
Expand Down
67 changes: 46 additions & 21 deletions src/sqlparser/src/parser.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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 })
}

Expand Down Expand Up @@ -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"),
}
}
Expand All @@ -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()?;
Expand All @@ -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"),
}
Expand Down Expand Up @@ -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)?;
Expand All @@ -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),
Expand Down
Loading

0 comments on commit 9c0e9df

Please sign in to comment.