From d66559f693c92aecc9dfbba36c4c30948acfcaba Mon Sep 17 00:00:00 2001 From: Guangdong Liu Date: Sun, 18 Feb 2024 14:43:54 +0800 Subject: [PATCH] [bugfix](sessionVariable) Fix sessionVariable has sessionOriginValue property, and execute cloneSessionVariable method will throw java.io.NotSerializableException: java.lang.reflect.Field (#31029) --- .../org/apache/doris/qe/SessionVariable.java | 6 +- .../apache/doris/qe/SessionVariableField.java | 85 +++++++++++++++++++ .../java/org/apache/doris/qe/VariableMgr.java | 41 +++++---- .../apache/doris/qe/SessionVariablesTest.java | 12 +++ 4 files changed, 124 insertions(+), 20 deletions(-) create mode 100644 fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariableField.java diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java b/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java index acf5bbf8a9..3e99b22c39 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java +++ b/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java @@ -531,7 +531,7 @@ public class SessionVariable implements Serializable, Writable { public boolean enableStats = true; // session origin value - public Map sessionOriginValue = new HashMap(); + public Map sessionOriginValue = new HashMap<>(); // check stmt is or not [select /*+ SET_VAR(...)*/ ...] // if it is setStmt, we needn't collect session origin value public boolean isSingleSetVar = false; @@ -2464,11 +2464,11 @@ public class SessionVariable implements Serializable, Writable { this.isSingleSetVar = issinglesetvar; } - public Map getSessionOriginValue() { + public Map getSessionOriginValue() { return sessionOriginValue; } - public void addSessionOriginValue(Field key, String value) { + public void addSessionOriginValue(SessionVariableField key, String value) { if (sessionOriginValue.containsKey(key)) { // If we already set the origin value, just skip the reset. return; diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariableField.java b/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariableField.java new file mode 100644 index 0000000000..55d2d69b4f --- /dev/null +++ b/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariableField.java @@ -0,0 +1,85 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.qe; + +import java.io.IOException; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; +import java.io.Serializable; +import java.lang.reflect.Field; +import java.util.Objects; + +public class SessionVariableField implements Serializable { + private transient Field field; + + public SessionVariableField(Field field) { + this.field = field; + } + + public Field getField() { + return field; + } + + private void writeObject(ObjectOutputStream out) throws IOException { + out.defaultWriteObject(); + out.writeObject(field.getName()); + out.writeObject(field.getType()); + } + + private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException { + in.defaultReadObject(); + String fieldName = (String) in.readObject(); + Class fieldType = (Class) in.readObject(); + try { + field = getField(fieldName, fieldType); + } catch (NoSuchFieldException e) { + e.printStackTrace(); + } + } + + private Field getField(String fieldName, Class fieldType) throws NoSuchFieldException { + try { + return SessionVariable.class.getDeclaredField(fieldName); + } catch (NoSuchFieldException e) { + Class superclass = SessionVariable.class.getSuperclass(); + if (superclass != null) { + return getField(fieldName, fieldType); + } + throw e; + } + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null || getClass() != obj.getClass()) { + return false; + } + SessionVariableField other = (SessionVariableField) obj; + // 忽略 transient 字段的比较 + return Objects.equals(this.getField(), other.getField()); + } + + @Override + public int hashCode() { + return Objects.hashCode(this.getField()); + } + +} diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/VariableMgr.java b/fe/fe-core/src/main/java/org/apache/doris/qe/VariableMgr.java index aa509fd0fd..84dfcef543 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/qe/VariableMgr.java +++ b/fe/fe-core/src/main/java/org/apache/doris/qe/VariableMgr.java @@ -152,7 +152,11 @@ public class VariableMgr { } // Set value to a variable - private static boolean setValue(Object obj, Field field, String value) throws DdlException { + private static boolean setValue(Object obj, SessionVariableField sessionVariableField, String value) + throws DdlException { + + Field field = sessionVariableField.getField(); + field.setAccessible(true); VarAttr attr = field.getAnnotation(VarAttr.class); if (VariableVarConverters.hasConverter(attr.name())) { @@ -196,22 +200,22 @@ public class VariableMgr { } break; case "byte": - field.setByte(obj, Byte.valueOf(value)); + field.setByte(obj, Byte.parseByte(value)); break; case "short": - field.setShort(obj, Short.valueOf(value)); + field.setShort(obj, Short.parseShort(value)); break; case "int": - field.setInt(obj, Integer.valueOf(value)); + field.setInt(obj, Integer.parseInt(value)); break; case "long": - field.setLong(obj, Long.valueOf(value)); + field.setLong(obj, Long.parseLong(value)); break; case "float": - field.setFloat(obj, Float.valueOf(value)); + field.setFloat(obj, Float.parseFloat(value)); break; case "double": - field.setDouble(obj, Double.valueOf(value)); + field.setDouble(obj, Double.parseDouble(value)); break; case "String": field.set(obj, value); @@ -236,9 +240,9 @@ public class VariableMgr { // revert the operator[set_var] on select/*+ SET_VAR()*/ sql; public static void revertSessionValue(SessionVariable obj) throws DdlException { - Map sessionOriginValue = obj.getSessionOriginValue(); + Map sessionOriginValue = obj.getSessionOriginValue(); if (!sessionOriginValue.isEmpty()) { - for (Field field : sessionOriginValue.keySet()) { + for (SessionVariableField field : sessionOriginValue.keySet()) { // revert session value setValue(obj, field, sessionOriginValue.get(field)); } @@ -351,22 +355,24 @@ public class VariableMgr { // No matter this is a global setting or not, always set session variable. Field field = ctx.getField(); + SessionVariableField sessionVariableField = new SessionVariableField(field); // if stmt is "Select /*+ SET_VAR(...)*/" if (sessionVariable.getIsSingleSetVar()) { try { - sessionVariable.addSessionOriginValue(field, field.get(sessionVariable).toString()); + sessionVariable.addSessionOriginValue(sessionVariableField, field.get(sessionVariable).toString()); } catch (Exception e) { LOG.warn("failed to collect origin session value ", e); } } - setValue(sessionVariable, field, value); + setValue(sessionVariable, sessionVariableField, value); } private static void setGlobalVarAndWriteEditLog(VarContext ctx, String name, String value) throws DdlException { // global variable will make effect when is set immediately. wlock.lock(); try { - setValue(ctx.getObj(), ctx.getField(), value); + + setValue(ctx.getObj(), new SessionVariableField(ctx.getField()), value); // write edit log GlobalVarPersistInfo info = new GlobalVarPersistInfo(defaultSessionVariable, Lists.newArrayList(name)); Env.getCurrentEnv().getEditLog().logGlobalVariableV2(info); @@ -380,7 +386,7 @@ public class VariableMgr { try { VarContext ctx = ctxByVarName.get(SessionVariable.PARALLEL_PIPELINE_TASK_NUM); try { - setValue(ctx.getObj(), ctx.getField(), String.valueOf(instance)); + setValue(ctx.getObj(), new SessionVariableField(ctx.getField()), String.valueOf(instance)); } catch (DdlException e) { LOG.warn("failed to set global variable: {}", SessionVariable.PARALLEL_PIPELINE_TASK_NUM, e); return; @@ -400,7 +406,7 @@ public class VariableMgr { try { VarContext ctx = ctxByVarName.get(SessionVariable.BROADCAST_RIGHT_TABLE_SCALE_FACTOR); try { - setValue(ctx.getObj(), ctx.getField(), String.valueOf(factor)); + setValue(ctx.getObj(), new SessionVariableField(ctx.getField()), String.valueOf(factor)); } catch (DdlException e) { LOG.warn("failed to set global variable: {}", SessionVariable.BROADCAST_RIGHT_TABLE_SCALE_FACTOR, e); return; @@ -420,7 +426,7 @@ public class VariableMgr { try { VarContext ctx = ctxByVarName.get(SessionVariable.ENABLE_NEREIDS_PLANNER); try { - setValue(ctx.getObj(), ctx.getField(), String.valueOf(true)); + setValue(ctx.getObj(), new SessionVariableField(ctx.getField()), String.valueOf(true)); } catch (DdlException e) { LOG.warn("failed to set global variable: {}", SessionVariable.ENABLE_NEREIDS_PLANNER, e); return; @@ -440,7 +446,7 @@ public class VariableMgr { try { VarContext ctx = ctxByVarName.get(SessionVariable.ENABLE_NEREIDS_DML); try { - setValue(ctx.getObj(), ctx.getField(), String.valueOf(true)); + setValue(ctx.getObj(), new SessionVariableField(ctx.getField()), String.valueOf(true)); } catch (DdlException e) { LOG.warn("failed to set global variable: {}", SessionVariable.ENABLE_NEREIDS_DML, e); return; @@ -511,7 +517,8 @@ public class VariableMgr { continue; } try { - setValue(varContext.getObj(), varContext.getField(), root.get((String) varName).toString()); + setValue(varContext.getObj(), new SessionVariableField(varContext.getField()), + root.get(varName).toString()); } catch (Exception exception) { LOG.warn("Exception during replay global variabl {} oplog, {}, THIS EXCEPTION WILL BE IGNORED.", (String) varName, exception.getMessage()); diff --git a/fe/fe-core/src/test/java/org/apache/doris/qe/SessionVariablesTest.java b/fe/fe-core/src/test/java/org/apache/doris/qe/SessionVariablesTest.java index 273c89d80e..bad7842e01 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/qe/SessionVariablesTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/qe/SessionVariablesTest.java @@ -156,4 +156,16 @@ public class SessionVariablesTest extends TestWithFeService { Assertions.assertEquals(123, sessionVariable.getQueryTimeoutS()); Assertions.assertEquals(123, sessionVariable.getInsertTimeoutS()); } + + @Test + public void testCloneSessionVariablesWithSessionOriginValueNotEmpty() throws NoSuchFieldException { + Field txIsolation = SessionVariable.class.getField("txIsolation"); + SessionVariableField txIsolationSessionVariableField = new SessionVariableField(txIsolation); + sessionVariable.addSessionOriginValue(txIsolationSessionVariableField, "test"); + + SessionVariable sessionVariableClone = VariableMgr.cloneSessionVariable(sessionVariable); + + Assertions.assertEquals("test", + sessionVariableClone.getSessionOriginValue().get(txIsolationSessionVariableField)); + } }