Skip to content

Conversation

mvzink
Copy link
Contributor

@mvzink mvzink commented Jan 31, 2025

This also stops rewriting SESSION away.

Closes #1694

This also stops rewriting `SESSION` away.

Closes apache#1694
Self::Local => write!(f, " LOCAL"),
Self::Session => write!(f, " SESSION"),
Self::Global => write!(f, " GLOBAL"),
Self::None => write!(f, ""),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it work the same to define scope as an optional field (scope: Option<SetVariableScope>)? in order to represent None by convention?

pub fn parse_set(&mut self) -> Result<Statement, ParserError> {
let modifier =
self.parse_one_of_keywords(&[Keyword::SESSION, Keyword::LOCAL, Keyword::HIVEVAR]);
let modifier_keywords = if self.dialect.supports_global_variable_modifier() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking having the lone global keyword as a dialect flag might not scale API wise, to make it dialect specific maybe it might make sense to have delegate to the dialect? like with parse_column_option as an example except with a default implementation that others like hive and mysql can override.
e.g.

let scope = self.dialect.parse_set_variable_scope(self)?;

@mvzink
Copy link
Contributor Author

mvzink commented Feb 4, 2025

I actually think between the point about API scalability and the further/future need for substantially different parsing for MySQL (#1697), this particular approach that just shoehorns in GLOBAL is not worthwhile. So I'm going to close this PR, and if I end up needing this in the future I'll try to do it right & cover the more general MySQL case.

@mvzink mvzink closed this Feb 4, 2025
@mvzink mvzink deleted the set-global branch June 20, 2025 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Parse MySQL SET GLOBAL variables

2 participants