[Fix](variables) fix session variable does not take effect immediately when set global variable in follower FE#18609
Merged
yiguolei merged 2 commits intoapache:masterfrom Apr 14, 2023
Merged
Conversation
Contributor
|
run buildall |
Contributor
|
PR approved by at least one committer and no changes requested. |
github-actions
bot
added
approved
Contributor
|
PR approved by anyone and no changes requested. |
yiguolei
added
kind/behavior-changed
dev/1.2.4
and removed
kind/behavior-changed
dev/1.2.4
labels
morningman
reviewed
Apr 13, 2023
| if (parsedStmt instanceof SetStmt) { | ||
| SetExecutor executor = new SetExecutor(ctx, (SetStmt) parsedStmt); | ||
| try { | ||
| executor.setSessionVars(); |
Contributor
There was a problem hiding this comment.
I think we can set remove the global property in SetStmt and than call executor.execute() method. No need to create a new method setSessionVars().
Contributor
Author
There was a problem hiding this comment.
I think we can set remove the
globalproperty inSetStmtand than callexecutor.execute()method. No need to create a new methodsetSessionVars().
Ok, I see what you mean.
| } | ||
|
|
||
| private boolean isSessionVar(SetVar var) { | ||
| return !(var instanceof SetPassVar |
Contributor
There was a problem hiding this comment.
This is error-prone, if other people at a new subclass of SetVar, this method may be missed.
How about add a SetVarType or something to specify the stmt's type directly?
And write like return var.setType() == Type.SET_SESSION_VAR;
Contributor
Author
There was a problem hiding this comment.
This is error-prone, if other people at a new subclass of
SetVar, this method may be missed. How about add aSetVarTypeor something to specify the stmt's type directly? And write likereturn var.setType() == Type.SET_SESSION_VAR;
Yes, I will modify it.
github-actions
bot
added
area/planner
Contributor
|
run buildall |
github-actions
bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
gnehil
pushed a commit
to gnehil/doris
that referenced
this pull request
Apr 21, 2023
…y when set global variable in follower FE (apache#18609)
5 tasks
morningman
added a commit
that referenced
this pull request
Apr 25, 2023
…on master FE (#18949) Introduced from #18609. When setting global variables from Non Master FE, there will be error like: `Variable 'password_history' is a GLOBAL variable and should be set with SET GLOBAL` Because when setting global variables from Non Master FE, Doris will do following step: 1. forward this SetStmt to Master FE to execute. 2. Change this SetStmt to "SESSION" level, and execute it again on this Non Master FE. But for "GLOBAL only" variable, such ash "password_history", it doesn't allow to set on SESSION level. So when doing step 2, "set password_history=xxx" without "GLOBAL" keywords will throw exception. So in this case, we should just ignore this exception and return.
gnehil
pushed a commit
to gnehil/doris
that referenced
this pull request
Apr 25, 2023
…on master FE (apache#18949) Introduced from apache#18609. When setting global variables from Non Master FE, there will be error like: `Variable 'password_history' is a GLOBAL variable and should be set with SET GLOBAL` Because when setting global variables from Non Master FE, Doris will do following step: 1. forward this SetStmt to Master FE to execute. 2. Change this SetStmt to "SESSION" level, and execute it again on this Non Master FE. But for "GLOBAL only" variable, such ash "password_history", it doesn't allow to set on SESSION level. So when doing step 2, "set password_history=xxx" without "GLOBAL" keywords will throw exception. So in this case, we should just ignore this exception and return.
xiaokang
pushed a commit
to xiaokang/doris
that referenced
this pull request
Apr 28, 2023
…on master FE (apache#18949) Introduced from apache#18609. When setting global variables from Non Master FE, there will be error like: `Variable 'password_history' is a GLOBAL variable and should be set with SET GLOBAL` Because when setting global variables from Non Master FE, Doris will do following step: 1. forward this SetStmt to Master FE to execute. 2. Change this SetStmt to "SESSION" level, and execute it again on this Non Master FE. But for "GLOBAL only" variable, such ash "password_history", it doesn't allow to set on SESSION level. So when doing step 2, "set password_history=xxx" without "GLOBAL" keywords will throw exception. So in this case, we should just ignore this exception and return.
xiaokang
pushed a commit
to xiaokang/doris
that referenced
this pull request
Apr 28, 2023
…on master FE (apache#18949) Introduced from apache#18609. When setting global variables from Non Master FE, there will be error like: `Variable 'password_history' is a GLOBAL variable and should be set with SET GLOBAL` Because when setting global variables from Non Master FE, Doris will do following step: 1. forward this SetStmt to Master FE to execute. 2. Change this SetStmt to "SESSION" level, and execute it again on this Non Master FE. But for "GLOBAL only" variable, such ash "password_history", it doesn't allow to set on SESSION level. So when doing step 2, "set password_history=xxx" without "GLOBAL" keywords will throw exception. So in this case, we should just ignore this exception and return.
Reminiscent
pushed a commit
to Reminiscent/doris
that referenced
this pull request
May 15, 2023
…y when set global variable in follower FE (apache#18609)
Reminiscent
pushed a commit
to Reminiscent/doris
that referenced
this pull request
May 15, 2023
…on master FE (apache#18949) Introduced from apache#18609. When setting global variables from Non Master FE, there will be error like: `Variable 'password_history' is a GLOBAL variable and should be set with SET GLOBAL` Because when setting global variables from Non Master FE, Doris will do following step: 1. forward this SetStmt to Master FE to execute. 2. Change this SetStmt to "SESSION" level, and execute it again on this Non Master FE. But for "GLOBAL only" variable, such ash "password_history", it doesn't allow to set on SESSION level. So when doing step 2, "set password_history=xxx" without "GLOBAL" keywords will throw exception. So in this case, we should just ignore this exception and return.
gnehil
pushed a commit
to gnehil/doris
that referenced
this pull request
Jul 13, 2023
…on master FE (apache#18949) Introduced from apache#18609. When setting global variables from Non Master FE, there will be error like: `Variable 'password_history' is a GLOBAL variable and should be set with SET GLOBAL` Because when setting global variables from Non Master FE, Doris will do following step: 1. forward this SetStmt to Master FE to execute. 2. Change this SetStmt to "SESSION" level, and execute it again on this Non Master FE. But for "GLOBAL only" variable, such ash "password_history", it doesn't allow to set on SESSION level. So when doing step 2, "set password_history=xxx" without "GLOBAL" keywords will throw exception. So in this case, we should just ignore this exception and return.
gnehil
pushed a commit
to gnehil/doris
that referenced
this pull request
Jul 25, 2023
…on master FE (apache#18949) Introduced from apache#18609. When setting global variables from Non Master FE, there will be error like: `Variable 'password_history' is a GLOBAL variable and should be set with SET GLOBAL` Because when setting global variables from Non Master FE, Doris will do following step: 1. forward this SetStmt to Master FE to execute. 2. Change this SetStmt to "SESSION" level, and execute it again on this Non Master FE. But for "GLOBAL only" variable, such ash "password_history", it doesn't allow to set on SESSION level. So when doing step 2, "set password_history=xxx" without "GLOBAL" keywords will throw exception. So in this case, we should just ignore this exception and return.
gnehil
pushed a commit
to gnehil/doris
that referenced
this pull request
Aug 15, 2023
…on master FE (apache#18949) Introduced from apache#18609. When setting global variables from Non Master FE, there will be error like: `Variable 'password_history' is a GLOBAL variable and should be set with SET GLOBAL` Because when setting global variables from Non Master FE, Doris will do following step: 1. forward this SetStmt to Master FE to execute. 2. Change this SetStmt to "SESSION" level, and execute it again on this Non Master FE. But for "GLOBAL only" variable, such ash "password_history", it doesn't allow to set on SESSION level. So when doing step 2, "set password_history=xxx" without "GLOBAL" keywords will throw exception. So in this case, we should just ignore this exception and return.
gnehil
pushed a commit
to gnehil/doris
that referenced
this pull request
Aug 16, 2023
…on master FE (apache#18949) Introduced from apache#18609. When setting global variables from Non Master FE, there will be error like: `Variable 'password_history' is a GLOBAL variable and should be set with SET GLOBAL` Because when setting global variables from Non Master FE, Doris will do following step: 1. forward this SetStmt to Master FE to execute. 2. Change this SetStmt to "SESSION" level, and execute it again on this Non Master FE. But for "GLOBAL only" variable, such ash "password_history", it doesn't allow to set on SESSION level. So when doing step 2, "set password_history=xxx" without "GLOBAL" keywords will throw exception. So in this case, we should just ignore this exception and return.
gnehil
pushed a commit
to gnehil/doris
that referenced
this pull request
Aug 29, 2023
…on master FE (apache#18949) Introduced from apache#18609. When setting global variables from Non Master FE, there will be error like: `Variable 'password_history' is a GLOBAL variable and should be set with SET GLOBAL` Because when setting global variables from Non Master FE, Doris will do following step: 1. forward this SetStmt to Master FE to execute. 2. Change this SetStmt to "SESSION" level, and execute it again on this Non Master FE. But for "GLOBAL only" variable, such ash "password_history", it doesn't allow to set on SESSION level. So when doing step 2, "set password_history=xxx" without "GLOBAL" keywords will throw exception. So in this case, we should just ignore this exception and return.
gnehil
pushed a commit
to gnehil/doris
that referenced
this pull request
Sep 1, 2023
…on master FE (apache#18949) Introduced from apache#18609. When setting global variables from Non Master FE, there will be error like: `Variable 'password_history' is a GLOBAL variable and should be set with SET GLOBAL` Because when setting global variables from Non Master FE, Doris will do following step: 1. forward this SetStmt to Master FE to execute. 2. Change this SetStmt to "SESSION" level, and execute it again on this Non Master FE. But for "GLOBAL only" variable, such ash "password_history", it doesn't allow to set on SESSION level. So when doing step 2, "set password_history=xxx" without "GLOBAL" keywords will throw exception. So in this case, we should just ignore this exception and return.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
Issue Number: close #xxx
Problem summary
For the non-master node, the
set global xxx = xxxwill not change the variables of the current session, because offorward()to master node execute.So the
set global xxx = xxxneeds to be executed again to change the current session afterforward().This PR will change behavior, but I think the old behavior is a bug.
Checklist(Required)
Further comments
If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...