You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "yoavcloud (via GitHub)" <gi...@apache.org> on 2025/09/17 10:32:50 UTC
[PR] Snowflake: ALTER USER and KeyValueOptions Refactoring [datafusion-sqlparser-rs]
yoavcloud opened a new pull request, #2035:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2035
This PR adds support for the `ALTER USER` statement with specific support for the Snowflake options. In addition, the `KeyValueOptions` struct was refactored:
1. Uses the `Value` type instead of its custom types for option values
2. Added support for multi-value options, i.e. KEY=(1,2,3)
3. Added support for nested key-value options, i.e. KEY=(K1=1, K2=2)
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Snowflake: ALTER USER and KeyValueOptions Refactoring [datafusion-sqlparser-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #2035:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2035#issuecomment-3304022156
Perhaps this would be a good PR for @yoavcloud to try and merge to ensure the credentials are setup correctly
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Snowflake: ALTER USER and KeyValueOptions Refactoring [datafusion-sqlparser-rs]
Posted by "yoavcloud (via GitHub)" <gi...@apache.org>.
yoavcloud commented on PR #2035:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2035#issuecomment-3335785158
That is soooo cool! Thank you @alamb and @iffyio!
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Snowflake: ALTER USER and KeyValueOptions Refactoring [datafusion-sqlparser-rs]
Posted by "yoavcloud (via GitHub)" <gi...@apache.org>.
yoavcloud commented on PR #2035:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2035#issuecomment-3317826531
@iffyio please take a look, and if OK I'll try to merge using my creds
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Snowflake: ALTER USER and KeyValueOptions Refactoring [datafusion-sqlparser-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #2035:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2035#issuecomment-3328328294
> @alamb I don't see the merge option, even though the PR has been approved. Who should I check this with?
@yoavcloud -- perhaps you need to link your github and apache account. To do so, follow the instructions at [gitbox.apache.org](https://gitbox.apache.org/)
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Snowflake: ALTER USER and KeyValueOptions Refactoring [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on code in PR #2035:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2035#discussion_r2362158376
##########
src/parser/mod.rs:
##########
@@ -17463,40 +17471,67 @@ impl<'a> Parser<'a> {
_ => return self.expected("another option, EOF, Comma or ')'", self.peek_token()),
};
}
- Ok(options)
+
+ Ok(KeyValueOptions { delimiter, options })
}
/// Parses a `KEY = VALUE` construct based on the specified key
pub(crate) fn parse_key_value_option(
&mut self,
- key: Word,
+ key: &Word,
) -> Result<KeyValueOption, ParserError> {
self.expect_token(&Token::Eq)?;
- match self.next_token().token {
- Token::SingleQuotedString(value) => Ok(KeyValueOption {
- option_name: key.value,
- option_type: KeyValueOptionType::STRING,
- value,
+ match self.peek_token().token {
+ Token::SingleQuotedString(_) => Ok(KeyValueOption {
+ option_name: key.value.clone(),
+ option_value: KeyValueOptionKind::Single(self.parse_value()?.into()),
}),
Token::Word(word)
if word.keyword == Keyword::TRUE || word.keyword == Keyword::FALSE =>
{
Ok(KeyValueOption {
- option_name: key.value,
- option_type: KeyValueOptionType::BOOLEAN,
- value: word.value.to_uppercase(),
+ option_name: key.value.clone(),
+ option_value: KeyValueOptionKind::Single(self.parse_value()?.into()),
})
}
- Token::Word(word) => Ok(KeyValueOption {
- option_name: key.value,
- option_type: KeyValueOptionType::ENUM,
- value: word.value,
- }),
- Token::Number(n, _) => Ok(KeyValueOption {
- option_name: key.value,
- option_type: KeyValueOptionType::NUMBER,
- value: n,
+ Token::Number(..) => Ok(KeyValueOption {
+ option_name: key.value.clone(),
+ option_value: KeyValueOptionKind::Single(self.parse_value()?.into()),
}),
+ Token::Word(word) => {
+ self.next_token();
+ Ok(KeyValueOption {
+ option_name: key.value.clone(),
+ option_value: KeyValueOptionKind::Single(Value::Placeholder(
+ word.value.clone(),
+ )),
+ })
+ }
+ Token::LParen => {
+ // Can be a list of values or a list of key value properties.
+ // Try to parse a list of values and if that fails, try to parse
+ // a list of key-value properties.
+ match self.try_parse(|parser| {
Review Comment:
should this use `maybe_parse` instead of `try_parse` (noticed we're ignoring the error from which on first glance looks unintentional)?
##########
src/ast/helpers/key_value_options.rs:
##########
@@ -41,6 +39,13 @@ pub struct KeyValueOptions {
pub delimiter: KeyValueOptionsDelimiter,
}
+impl KeyValueOptions {
+ /// Returns true iff the options list is empty
+ pub fn is_empty(&self) -> bool {
+ self.options.is_empty()
+ }
+}
+
Review Comment:
I'm thinking we could skip the added API and have the caller call `value.options.is_empty()` instead? since this is only a thin wrapper might not be worth it
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Snowflake: ALTER USER and KeyValueOptions Refactoring [datafusion-sqlparser-rs]
Posted by "yoavcloud (via GitHub)" <gi...@apache.org>.
yoavcloud commented on code in PR #2035:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2035#discussion_r2374526777
##########
src/ast/mod.rs:
##########
@@ -10558,6 +10564,199 @@ impl fmt::Display for CreateUser {
}
}
+/// Modifies the properties of a user
+///
+/// Syntax:
+/// ```sql
+/// ALTER USER [ IF EXISTS ] [ <name> ] [ OPTIONS ]
+/// ```
+///
+/// [Snowflake](https://docs.snowflake.com/en/sql-reference/sql/alter-user)
+#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
+pub struct AlterUser {
+ pub if_exists: bool,
+ pub name: Ident,
+ pub rename_to: Option<Ident>,
+ pub reset_password: bool,
+ pub abort_all_queries: bool,
+ pub add_role_delegation: Option<AlterUserAddRoleDelegation>,
+ pub remove_role_delegation: Option<AlterUserRemoveRoleDelegation>,
+ pub enroll_mfa: bool,
+ pub set_default_mfa_method: Option<MfaMethodKind>,
+ pub remove_mfa_method: Option<MfaMethodKind>,
+ pub modify_mfa_method: Option<AlterUserModifyMfaMethod>,
+ pub set_policy: Option<AlterUserSetPolicy>,
+ pub unset_policy: Option<UserPolicyKind>,
+ pub set_tag: KeyValueOptions,
+ pub unset_tag: Vec<String>,
+ pub set_props: KeyValueOptions,
+ pub unset_props: Vec<String>,
Review Comment:
Unfortunately Snowflake docs doesn't have a link to each, but we can group them all together. I added a comment about that.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Snowflake: ALTER USER and KeyValueOptions Refactoring [datafusion-sqlparser-rs]
Posted by "yoavcloud (via GitHub)" <gi...@apache.org>.
yoavcloud merged PR #2035:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2035
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Snowflake: ALTER USER and KeyValueOptions Refactoring [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on code in PR #2035:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2035#discussion_r2372043660
##########
src/ast/mod.rs:
##########
@@ -10558,6 +10564,199 @@ impl fmt::Display for CreateUser {
}
}
+/// Modifies the properties of a user
+///
+/// Syntax:
+/// ```sql
+/// ALTER USER [ IF EXISTS ] [ <name> ] [ OPTIONS ]
+/// ```
+///
+/// [Snowflake](https://docs.snowflake.com/en/sql-reference/sql/alter-user)
+#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
+pub struct AlterUser {
+ pub if_exists: bool,
+ pub name: Ident,
+ pub rename_to: Option<Ident>,
+ pub reset_password: bool,
+ pub abort_all_queries: bool,
+ pub add_role_delegation: Option<AlterUserAddRoleDelegation>,
+ pub remove_role_delegation: Option<AlterUserRemoveRoleDelegation>,
+ pub enroll_mfa: bool,
+ pub set_default_mfa_method: Option<MfaMethodKind>,
+ pub remove_mfa_method: Option<MfaMethodKind>,
+ pub modify_mfa_method: Option<AlterUserModifyMfaMethod>,
+ pub set_policy: Option<AlterUserSetPolicy>,
+ pub unset_policy: Option<UserPolicyKind>,
+ pub set_tag: KeyValueOptions,
+ pub unset_tag: Vec<String>,
+ pub set_props: KeyValueOptions,
+ pub unset_props: Vec<String>,
Review Comment:
@yoavcloud one thing I realised now with these props, could we add a link tho the snowflake docs individually for them? Thinking so that when new options are inevitably added for other dialects it would be clear where each propery comes from
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Snowflake: ALTER USER and KeyValueOptions Refactoring [datafusion-sqlparser-rs]
Posted by "yoavcloud (via GitHub)" <gi...@apache.org>.
yoavcloud commented on PR #2035:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2035#issuecomment-3326692755
@alamb I don't see the merge option, even though the PR has been approved. Who should I check this with?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Snowflake: ALTER USER and KeyValueOptions Refactoring [datafusion-sqlparser-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #2035:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2035#issuecomment-3335747274
<img width="824" height="70" alt="Screenshot 2025-09-25 at 4 00 27 PM" src="https://github.com/user-attachments/assets/3cc35b98-a06a-46ee-9aae-9af934f4584e" />
It's working!
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Snowflake: ALTER USER and KeyValueOptions Refactoring [datafusion-sqlparser-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #2035:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2035#issuecomment-3340455452
> That is soooo cool! Thank you @alamb and @iffyio!
Welcome aboard!
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org