Skip to content

Add GLOBAL context/modifier to SET statements #1767

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.

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

Merged
merged 4 commits into from
Mar 20, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
33 changes: 26 additions & 7 deletions src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ use serde::{Deserialize, Serialize};
#[cfg(feature = "visitor")]
use sqlparser_derive::{Visit, VisitMut};

use crate::tokenizer::Span;
use crate::{keywords::Keyword, tokenizer::Span};

pub use self::data_type::{
ArrayElemTypeDef, BinaryLength, CharLengthUnits, CharacterLength, DataType, EnumMember,
Expand Down Expand Up @@ -2578,7 +2578,7 @@ pub enum Set {
/// SQL Standard-style
/// SET a = 1;
SingleAssignment {
local: bool,
scope: ContextModifier,
hivevar: bool,
variable: ObjectName,
values: Vec<Expr>,
Expand Down Expand Up @@ -2660,7 +2660,7 @@ impl Display for Set {
role_name,
} => {
let role_name = role_name.clone().unwrap_or_else(|| Ident::new("NONE"));
write!(f, "SET{context_modifier} ROLE {role_name}")
write!(f, "SET {context_modifier}ROLE {role_name}")
}
Self::SetSessionParam(kind) => write!(f, "SET {kind}"),
Self::SetTransaction {
Expand Down Expand Up @@ -2707,15 +2707,15 @@ impl Display for Set {
Ok(())
}
Set::SingleAssignment {
local,
scope,
hivevar,
variable,
values,
} => {
write!(
f,
"SET {}{}{} = {}",
if *local { "LOCAL " } else { "" },
scope,
if *hivevar { "HIVEVAR:" } else { "" },
variable,
display_comma_separated(values)
Expand Down Expand Up @@ -7910,6 +7910,8 @@ pub enum ContextModifier {
Local,
/// `SESSION` identifier
Session,
/// `GLOBAL` identifier
Global,
}

impl fmt::Display for ContextModifier {
Expand All @@ -7919,11 +7921,28 @@ impl fmt::Display for ContextModifier {
write!(f, "")
}
Self::Local => {
write!(f, " LOCAL")
write!(f, "LOCAL ")
}
Self::Session => {
write!(f, " SESSION")
write!(f, "SESSION ")
}
Self::Global => {
write!(f, "GLOBAL ")
}
}
}
}

impl From<Option<Keyword>> for ContextModifier {
fn from(kw: Option<Keyword>) -> Self {
match kw {
Some(kw) => match kw {
Keyword::LOCAL => Self::Local,
Keyword::SESSION => Self::Session,
Keyword::GLOBAL => Self::Global,
_ => Self::None,
},
None => Self::None,
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can instead turn this into a regular function if the goal is to reuse it, since its not expected to be able to turn an arbitrary keyword into a ContextModifier.

Copy link
Contributor Author

@MohamedAbdeen21 MohamedAbdeen21 Mar 18, 2025

Choose a reason for hiding this comment

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

I'm not following. Instead of an impl From<Option<Keyword>> for ContextModifier you'd prefer a fn into_modifier(k: Option<Keyword>) -> ContextModifier?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah exactly, essentially to put the logic in a function instead, maybe something more like?

fn context_modifier_from_keyword(kw: Keyword) -> Result<ContextModifier> {
}
// on the caller side if it has an option
modifier.map(context_modifier_from_keyword)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You'd still need to account for when the modifier var is a None. Unless you extract the None variant from ContextModifier and replace all ContextModifiers with options, which is a breaking change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would fn context_modifier_from_keyword(kw: Option<Keyword>) -> Result<ContextModifier> be fine to account for that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess, but at that point, what's the difference? Visibility, maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

}
Expand Down
20 changes: 10 additions & 10 deletions src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11100,11 +11100,7 @@ impl<'a> Parser<'a> {
/// Parse a `SET ROLE` statement. Expects SET to be consumed already.
fn parse_set_role(&mut self, modifier: Option<Keyword>) -> Result<Statement, ParserError> {
self.expect_keyword_is(Keyword::ROLE)?;
let context_modifier = match modifier {
Some(Keyword::LOCAL) => ContextModifier::Local,
Some(Keyword::SESSION) => ContextModifier::Session,
_ => ContextModifier::None,
};
let context_modifier = ContextModifier::from(modifier);

let role_name = if self.parse_keyword(Keyword::NONE) {
None
Expand Down Expand Up @@ -11176,8 +11172,12 @@ impl<'a> Parser<'a> {
}

fn parse_set(&mut self) -> Result<Statement, ParserError> {
let modifier =
self.parse_one_of_keywords(&[Keyword::SESSION, Keyword::LOCAL, Keyword::HIVEVAR]);
let modifier = self.parse_one_of_keywords(&[
Keyword::SESSION,
Keyword::LOCAL,
Keyword::HIVEVAR,
Keyword::GLOBAL,
]);

if let Some(Keyword::HIVEVAR) = modifier {
self.expect_token(&Token::Colon)?;
Expand All @@ -11193,7 +11193,7 @@ impl<'a> Parser<'a> {
{
if self.consume_token(&Token::Eq) || self.parse_keyword(Keyword::TO) {
return Ok(Set::SingleAssignment {
local: modifier == Some(Keyword::LOCAL),
scope: ContextModifier::from(modifier),
hivevar: modifier == Some(Keyword::HIVEVAR),
variable: ObjectName::from(vec!["TIMEZONE".into()]),
values: self.parse_set_values(false)?,
Expand Down Expand Up @@ -11283,7 +11283,7 @@ impl<'a> Parser<'a> {
}?;

Ok(Set::SingleAssignment {
local: modifier == Some(Keyword::LOCAL),
scope: ContextModifier::from(modifier),
hivevar: modifier == Some(Keyword::HIVEVAR),
variable,
values,
Expand Down Expand Up @@ -11311,7 +11311,7 @@ impl<'a> Parser<'a> {
if self.consume_token(&Token::Eq) || self.parse_keyword(Keyword::TO) {
let stmt = match variables {
OneOrManyWithParens::One(var) => Set::SingleAssignment {
local: modifier == Some(Keyword::LOCAL),
scope: ContextModifier::from(modifier),
hivevar: modifier == Some(Keyword::HIVEVAR),
variable: var,
values: self.parse_set_values(false)?,
Expand Down
32 changes: 26 additions & 6 deletions tests/sqlparser_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8627,12 +8627,12 @@ fn parse_set_transaction() {
fn parse_set_variable() {
match verified_stmt("SET SOMETHING = '1'") {
Statement::Set(Set::SingleAssignment {
local,
scope,
hivevar,
variable,
values,
}) => {
assert!(!local);
assert_eq!(scope, ContextModifier::None);
assert!(!hivevar);
assert_eq!(variable, ObjectName::from(vec!["SOMETHING".into()]));
assert_eq!(
Expand All @@ -8645,6 +8645,26 @@ fn parse_set_variable() {
_ => unreachable!(),
}

match verified_stmt("SET GLOBAL VARIABLE = 'Value'") {
Statement::Set(Set::SingleAssignment {
scope,
hivevar,
variable,
values,
}) => {
assert_eq!(scope, ContextModifier::Global);
assert!(!hivevar);
assert_eq!(variable, ObjectName::from(vec!["VARIABLE".into()]));
assert_eq!(
values,
vec![Expr::Value(
(Value::SingleQuotedString("Value".into())).with_empty_span()
)]
);
}
_ => unreachable!(),
}

let multi_variable_dialects = all_dialects_where(|d| d.supports_parenthesized_set_variables());
let sql = r#"SET (a, b, c) = (1, 2, 3)"#;
match multi_variable_dialects.verified_stmt(sql) {
Expand Down Expand Up @@ -8719,12 +8739,12 @@ fn parse_set_variable() {
fn parse_set_role_as_variable() {
match verified_stmt("SET role = 'foobar'") {
Statement::Set(Set::SingleAssignment {
local,
scope,
hivevar,
variable,
values,
}) => {
assert!(!local);
assert_eq!(scope, ContextModifier::None);
assert!(!hivevar);
assert_eq!(variable, ObjectName::from(vec!["role".into()]));
assert_eq!(
Expand Down Expand Up @@ -8766,12 +8786,12 @@ fn parse_double_colon_cast_at_timezone() {
fn parse_set_time_zone() {
match verified_stmt("SET TIMEZONE = 'UTC'") {
Statement::Set(Set::SingleAssignment {
local,
scope,
hivevar,
variable,
values,
}) => {
assert!(!local);
assert_eq!(scope, ContextModifier::None);
assert!(!hivevar);
assert_eq!(variable, ObjectName::from(vec!["TIMEZONE".into()]));
assert_eq!(
Expand Down
9 changes: 5 additions & 4 deletions tests/sqlparser_hive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@
//! is also tested (on the inputs it can handle).

use sqlparser::ast::{
ClusteredBy, CommentDef, CreateFunction, CreateFunctionBody, CreateFunctionUsing, CreateTable,
Expr, Function, FunctionArgumentList, FunctionArguments, Ident, ObjectName, OrderByExpr,
OrderByOptions, SelectItem, Set, Statement, TableFactor, UnaryOperator, Use, Value,
ClusteredBy, CommentDef, ContextModifier, CreateFunction, CreateFunctionBody,
CreateFunctionUsing, CreateTable, Expr, Function, FunctionArgumentList, FunctionArguments,
Ident, ObjectName, OrderByExpr, OrderByOptions, SelectItem, Set, Statement, TableFactor,
UnaryOperator, Use, Value,
};
use sqlparser::dialect::{GenericDialect, HiveDialect, MsSqlDialect};
use sqlparser::parser::ParserError;
Expand Down Expand Up @@ -369,7 +370,7 @@ fn set_statement_with_minus() {
assert_eq!(
hive().verified_stmt("SET hive.tez.java.opts = -Xmx4g"),
Statement::Set(Set::SingleAssignment {
local: false,
scope: ContextModifier::None,
hivevar: false,
variable: ObjectName::from(vec![
Ident::new("hive"),
Expand Down
2 changes: 1 addition & 1 deletion tests/sqlparser_mssql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1251,7 +1251,7 @@ fn parse_mssql_declare() {
}]
},
Statement::Set(Set::SingleAssignment {
local: false,
scope: ContextModifier::None,
hivevar: false,
variable: ObjectName::from(vec![Ident::new("@bar")]),
values: vec![Expr::Value(
Expand Down
2 changes: 1 addition & 1 deletion tests/sqlparser_mysql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,7 @@ fn parse_set_variables() {
assert_eq!(
mysql_and_generic().verified_stmt("SET LOCAL autocommit = 1"),
Statement::Set(Set::SingleAssignment {
local: true,
scope: ContextModifier::Local,
hivevar: false,
variable: ObjectName::from(vec!["autocommit".into()]),
values: vec![Expr::value(number("1"))],
Expand Down
18 changes: 8 additions & 10 deletions tests/sqlparser_postgres.rs
Original file line number Diff line number Diff line change
Expand Up @@ -988,8 +988,7 @@ fn parse_create_schema_if_not_exists() {
Statement::CreateSchema {
if_not_exists: true,
schema_name,
options: _,
default_collate_spec: _,
..
} => assert_eq!("schema_name", schema_name.to_string()),
_ => unreachable!(),
}
Expand Down Expand Up @@ -1433,7 +1432,7 @@ fn parse_set() {
assert_eq!(
stmt,
Statement::Set(Set::SingleAssignment {
local: false,
scope: ContextModifier::None,
hivevar: false,
variable: ObjectName::from(vec![Ident::new("a")]),
values: vec![Expr::Identifier(Ident {
Expand All @@ -1448,7 +1447,7 @@ fn parse_set() {
assert_eq!(
stmt,
Statement::Set(Set::SingleAssignment {
local: false,
scope: ContextModifier::None,
hivevar: false,
variable: ObjectName::from(vec![Ident::new("a")]),
values: vec![Expr::Value(
Expand All @@ -1461,7 +1460,7 @@ fn parse_set() {
assert_eq!(
stmt,
Statement::Set(Set::SingleAssignment {
local: false,
scope: ContextModifier::None,
hivevar: false,
variable: ObjectName::from(vec![Ident::new("a")]),
values: vec![Expr::value(number("0"))],
Expand All @@ -1472,7 +1471,7 @@ fn parse_set() {
assert_eq!(
stmt,
Statement::Set(Set::SingleAssignment {
local: false,
scope: ContextModifier::None,
hivevar: false,
variable: ObjectName::from(vec![Ident::new("a")]),
values: vec![Expr::Identifier(Ident::new("DEFAULT"))],
Expand All @@ -1483,7 +1482,7 @@ fn parse_set() {
assert_eq!(
stmt,
Statement::Set(Set::SingleAssignment {
local: true,
scope: ContextModifier::Local,
hivevar: false,
variable: ObjectName::from(vec![Ident::new("a")]),
values: vec![Expr::Identifier("b".into())],
Expand All @@ -1494,7 +1493,7 @@ fn parse_set() {
assert_eq!(
stmt,
Statement::Set(Set::SingleAssignment {
local: false,
scope: ContextModifier::None,
hivevar: false,
variable: ObjectName::from(vec![Ident::new("a"), Ident::new("b"), Ident::new("c")]),
values: vec![Expr::Identifier(Ident {
Expand All @@ -1512,7 +1511,7 @@ fn parse_set() {
assert_eq!(
stmt,
Statement::Set(Set::SingleAssignment {
local: false,
scope: ContextModifier::None,
hivevar: false,
variable: ObjectName::from(vec![
Ident::new("hive"),
Expand All @@ -1526,7 +1525,6 @@ fn parse_set() {
);

pg_and_generic().one_statement_parses_to("SET a TO b", "SET a = b");
pg_and_generic().one_statement_parses_to("SET SESSION a = b", "SET a = b");

assert_eq!(
pg_and_generic().parse_sql_statements("SET"),
Expand Down