JackieTien97 commented on code in PR #14135:
URL: https://github.com/apache/iotdb/pull/14135#discussion_r1856138653
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/write/function/DropFunctionPlan.java:
##########
@@ -31,29 +32,37 @@
public class DropFunctionPlan extends ConfigPhysicalPlan {
+ private Model model;
private String functionName;
public DropFunctionPlan() {
super(ConfigPhysicalPlanType.DropFunction);
}
- public DropFunctionPlan(String functionName) {
+ public DropFunctionPlan(Model model, String functionName) {
super(ConfigPhysicalPlanType.DropFunction);
+ this.model = model;
this.functionName = functionName;
}
+ public Model getModel() {
+ return model;
+ }
+
public String getFunctionName() {
return functionName;
}
@Override
protected void serializeImpl(DataOutputStream stream) throws IOException {
stream.writeShort(getType().getPlanType());
+ ReadWriteIOUtils.write(model.getValue(), stream);
ReadWriteIOUtils.write(functionName, stream);
}
@Override
protected void deserializeImpl(ByteBuffer buffer) throws IOException {
+ model = Model.findByValue(ReadWriteIOUtils.readInt(buffer));
Review Comment:
won't be compatible to previous version
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/udf/service/UDFExecutableManager.java:
##########
@@ -53,6 +58,47 @@ public static synchronized UDFExecutableManager
setupAndGetInstance(
return INSTANCE;
}
+ /** check whether local jar is correct according to md5 */
+ public boolean isLocalJarConflicted(UDFInformation udfInformation) throws
UDFManagementException {
+ String functionName = udfInformation.getFunctionName();
+ // A jar with the same name exists, we need to check md5
+ String existedMd5 = "";
+ String md5FilePath = functionName + ".txt";
Review Comment:
`.txt` already used previously? if not, I think `.md5` suffix may be better
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/conf/IoTDBConstant.java:
##########
Review Comment:
change the function type to:
built-in scalar
built-in aggregate
built-in table
user-defined scalar
user-defined aggregate
user-defined table
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/execution/config/metadata/ShowFunctionsTask.java:
##########
@@ -55,9 +61,29 @@
public class ShowFunctionsTask implements IConfigTask {
Review Comment:
will this class be used for table model? it seems that you don't deal with
that
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/udf/UDFTable.java:
##########
@@ -19,99 +19,101 @@
package org.apache.iotdb.commons.udf;
-import
org.apache.iotdb.commons.udf.builtin.BuiltinTimeSeriesGeneratingFunction;
+import org.apache.iotdb.common.rpc.thrift.Model;
import org.apache.iotdb.commons.udf.service.UDFClassLoader;
import org.apache.iotdb.commons.utils.TestOnly;
+import org.apache.tsfile.utils.Pair;
import org.apache.tsfile.utils.ReadWriteIOUtils;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
+import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.stream.Collectors;
+/**
+ * UDFTable is a table that stores UDF information. On DataNode, it stores all
UDF information. On
+ * ConfigNode, it does not store built-in UDF information.
+ */
public class UDFTable {
- private final Map<String, UDFInformation> udfInformationMap;
- /** maintain a map for creating instance */
- private final Map<String, Class<?>> functionToClassMap;
+ /** functionName -> information * */
+ private final Map<Pair<Model, String>, UDFInformation> udfInformationMap;
+
+ /** maintain a map for creating instance, functionName -> class */
+ private final Map<Pair<Model, String>, Class<?>> functionToClassMap;
Review Comment:
Why not using Map<Model, Map<String, UDFInformation>>? In this way, you
won't need to iterate all map entry while trying to all functions of one kind
of model.
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/write/function/DropFunctionPlan.java:
##########
@@ -31,29 +32,37 @@
public class DropFunctionPlan extends ConfigPhysicalPlan {
+ private Model model;
private String functionName;
public DropFunctionPlan() {
super(ConfigPhysicalPlanType.DropFunction);
}
- public DropFunctionPlan(String functionName) {
+ public DropFunctionPlan(Model model, String functionName) {
super(ConfigPhysicalPlanType.DropFunction);
+ this.model = model;
this.functionName = functionName;
}
+ public Model getModel() {
+ return model;
+ }
+
public String getFunctionName() {
return functionName;
}
@Override
protected void serializeImpl(DataOutputStream stream) throws IOException {
stream.writeShort(getType().getPlanType());
+ ReadWriteIOUtils.write(model.getValue(), stream);
ReadWriteIOUtils.write(functionName, stream);
}
@Override
protected void deserializeImpl(ByteBuffer buffer) throws IOException {
+ model = Model.findByValue(ReadWriteIOUtils.readInt(buffer));
Review Comment:
may need to add a new PlanType
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/udf/UDFType.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * 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.iotdb.commons.udf;
+
+import org.apache.tsfile.utils.ReadWriteIOUtils;
+
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+
+public enum UDFType {
+ TREE_EXTERNAL((byte) 0),
+ TREE_BUILT_IN((byte) 1),
+ TREE_UNAVAILABLE((byte) 2),
+ TABLE_EXTERNAL((byte) 3),
Review Comment:
```suggestion
TABLE_AVAILABLE((byte) 3),
```
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/udf/UDFType.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * 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.iotdb.commons.udf;
+
+import org.apache.tsfile.utils.ReadWriteIOUtils;
+
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+
+public enum UDFType {
+ TREE_EXTERNAL((byte) 0),
+ TREE_BUILT_IN((byte) 1),
Review Comment:
add some comments for this enum to explain that this won't appear in
previous snapshot file or raft log, just a place holder for some unforeseen
circumstances.
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/udf/UDFType.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * 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.iotdb.commons.udf;
+
+import org.apache.tsfile.utils.ReadWriteIOUtils;
+
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+
+public enum UDFType {
+ TREE_EXTERNAL((byte) 0),
Review Comment:
```suggestion
TREE_AVAILABLE((byte) 0),
```
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/UDFManager.java:
##########
@@ -83,35 +86,43 @@ public TSStatus createFunction(TCreateFunctionReq req) {
final String jarMD5 = req.getJarMD5();
final String jarName = req.getJarName();
final byte[] jarFile = req.getJarFile();
- udfInfo.validate(udfName, jarName, jarMD5);
+ final Model model = req.getModel();
+ udfInfo.validate(model, udfName, jarName, jarMD5);
- final UDFInformation udfInformation =
- new UDFInformation(udfName, req.getClassName(), false, isUsingURI,
jarName, jarMD5);
- final boolean needToSaveJar = isUsingURI &&
udfInfo.needToSaveJar(jarName);
+ UDFInformation udfInformation =
+ new UDFInformation(
+ udfName, req.getClassName(), model, false, isUsingURI, jarName,
jarMD5);
- LOGGER.info(
- "Start to create UDF [{}] on Data Nodes, needToSaveJar[{}]",
udfName, needToSaveJar);
-
- final TSStatus dataNodesStatus =
- RpcUtils.squashResponseStatusList(
- createFunctionOnDataNodes(udfInformation, needToSaveJar ?
jarFile : null));
- if (dataNodesStatus.getCode() !=
TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
- return dataNodesStatus;
- }
+ final boolean needToSaveJar = isUsingURI &&
udfInfo.needToSaveJar(jarName);
+ LOGGER.info("Start to add UDF [{}] in UDF_Table on Config Nodes",
udfName);
CreateFunctionPlan createFunctionPlan =
new CreateFunctionPlan(udfInformation, needToSaveJar ? new
Binary(jarFile) : null);
+
+ udfInformation.setAvailable(true);
Review Comment:
true or false?
--
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]
