github-actions[bot] commented on code in PR #66604:
URL: https://github.com/apache/doris/pull/66604#discussion_r3747692633
##########
fe/fe-core/src/main/java/org/apache/doris/qe/VariableMgr.java:
##########
@@ -1112,6 +1112,9 @@ public static void forceUpdateVariables() {
VariableMgr.refreshDefaultSessionVariables(updateInfo,
SessionVariable.ENABLE_SQL_CACHE,
String.valueOf(true));
+ VariableMgr.refreshDefaultSessionVariables(updateInfo,
Review Comment:
This extends a migration fence that released clusters have already consumed.
A 3.1 image can contain planner=false/version=300; released 4.0 migrates that
image to version 400 without changing the flag. On a later upgrade, replay
restores false/400 before `forceUpdateVariables()`, so `400 < 400` is false and
this assignment never runs. Please add a fresh one-shot version (for example,
`VARIABLE_VERSION_401`, made current, with this assignment under `< 401`) and
make the regression start from version 400 plus planner=false. Otherwise
already-upgraded clusters remain on the legacy planner.
##########
fe/fe-core/src/test/java/org/apache/doris/qe/VariableMgrTest.java:
##########
@@ -105,6 +105,31 @@ public void testSetGlobalDefault() throws Exception {
Assertions.assertEquals(defaultSessionVar.enableProfile(),
VariableMgr.newSessionVariable().enableProfile());
}
+ @Test
+ public void testEnableNereidsDistributePlannerAfterUpgrade() {
+ SessionVariable defaultSessionVariable =
VariableMgr.getDefaultSessionVariable();
+ boolean originalValue =
defaultSessionVariable.isEnableNereidsDistributePlanner();
+ boolean originalEnableSqlCache =
defaultSessionVariable.isEnableSqlCache();
+ boolean originalAnsiBehavior =
GlobalVariable.enable_ansi_query_organization_behavior;
+ boolean originalTypeCoercionBehavior =
GlobalVariable.enableNewTypeCoercionBehavior;
+ int originalVersion = GlobalVariable.variableVersion;
+ try {
+ defaultSessionVariable.setEnableNereidsDistributePlanner(false);
+ GlobalVariable.variableVersion =
GlobalVariable.VARIABLE_VERSION_300;
+
+ VariableMgr.forceUpdateVariables();
Review Comment:
This test journals the real version-400 migration, including unrelated
ANSI/type-coercion changes, but `finally` restores them only in memory. If it
precedes `testGlobalVariablePersist`, that test's `edit_log_roll_num=1`
finalizes these records and its manual checkpoint replays them. Checkpoint
`SessionVariable` objects are isolated, but `GlobalVariable` contexts still
target shared statics (`obj=null`), so ANSI/type-coercion become false again
after this cleanup and leak into later methods. Please use a scoped edit-log
spy/stub restored in `finally`, or otherwise make cleanup durable.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
