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.

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

Added new file full of session variables, plus new types #373

Merged
merged 1 commit into from Apr 20, 2021

Conversation

Hydrocharged
Copy link
Contributor

I'm sure there are mistakes in here. This was like 85% manually done, and I had lots of mental TODOs while working through it, so I very well may have forgotten to edit a variable, add a note, or change the type. Either way, the majority of variables are here. I did exclude some as I couldn't find any GLOBAL/SESSION or Dynamic information for them, but this is like 98% of the variables.

Now to hook them up and replace our current variable handling.

Copy link
Member

@zachmu zachmu left a comment

Choose a reason for hiding this comment

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

This is fine for what it is, but I don't know what you have in mind for the new types, esp. things like encode / decode. Listing the full set of supported system vars is definitely important, so it's good to get that in.

There are a bunch of other outstanding issues with variable assignments, like that we don't differentiate between @@timestamp and @timestamp for example, either in the analysis logic or the session storage. We really need to do that, since it's totally fine for a single var to have 3 different values: a global system value, a session system value, and a local var value.

Anyway, LGTM, looking forward to the next part

sql/system_variables.go Show resolved Hide resolved
sql/system_variables.go Show resolved Hide resolved
sql/system_variables.go Show resolved Hide resolved
sql/system_booltype.go Show resolved Hide resolved
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.

None yet

2 participants