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

refactor: make object ref outside ast::Value #19559

Merged
merged 2 commits into from
Nov 25, 2024
Merged

refactor: make object ref outside ast::Value #19559

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
92a5e6e
fix
Nov 25, 2024
316536b
rename
Nov 25, 2024
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
13 changes: 7 additions & 6 deletions src/frontend/src/utils/with_options.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ use risingwave_pb::secret::PbSecretRef;
use risingwave_sqlparser::ast::{
ConnectionRefValue, CreateConnectionStatement, CreateSinkStatement, CreateSourceStatement,
CreateSubscriptionStatement, SecretRefAsType, SecretRefValue, SqlOption, Statement, Value,
ValueAndObjRef,
};

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) => {
ValueAndObjRef::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) => {
ValueAndObjRef::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(),
ValueAndObjRef::Value(Value::CstyleEscapedString(s)) => s.value,
ValueAndObjRef::Value(Value::SingleQuotedString(s)) => s,
ValueAndObjRef::Value(Value::Number(n)) => n,
ValueAndObjRef::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
Open in desktop
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
Open in desktop
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
Open in desktop
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: ValueAndObjRef,
tabVersion marked this conversation as resolved.
Show resolved Hide resolved
}

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 ValueAndObjRef {
Value(Value),
SecretRef(SecretRefValue),
ConnectionRef(ConnectionRefValue),
}

impl fmt::Debug for ValueAndObjRef {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
ValueAndObjRef::Value(value) => write!(f, "{:?}", value),
ValueAndObjRef::SecretRef(secret_ref) => write!(f, "secret {:?}", secret_ref),
ValueAndObjRef::ConnectionRef(connection_ref) => {
write!(f, "connection {:?}", connection_ref)
}
}
}
}

impl fmt::Display for ValueAndObjRef {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
ValueAndObjRef::Value(value) => write!(f, "{}", value),
ValueAndObjRef::SecretRef(secret_ref) => write!(f, "secret {}", secret_ref),
ValueAndObjRef::ConnectionRef(connection_ref) => {
write!(f, "connection {}", connection_ref)
}
}
}
}

impl From<Value> for ValueAndObjRef {
fn from(value: Value) -> Self {
ValueAndObjRef::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,
ValueAndObjRef::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
Open in desktop
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
Open in desktop
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
Open in desktop
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>()? {
ValueAndObjRef::Value(value) => Ok(value),
ValueAndObjRef::SecretRef(_) | ValueAndObjRef::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<ValueAndObjRef> {
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(ValueAndObjRef::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(ValueAndObjRef::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
8 changes: 4 additions & 4 deletions src/sqlparser/tests/sqlparser_common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading