bsloane1650 commented on a change in pull request #273: WIP: Add User Defined Functions Capability URL: https://github.com/apache/incubator-daffodil/pull/273#discussion_r336723102
########## File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/dpath/UserDefinedFunctionBase.scala ########## @@ -0,0 +1,35 @@ +/* + * 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.daffodil.dpath + +import org.apache.daffodil.exceptions.Assert +import java.lang.reflect.Method +import org.apache.daffodil.udf.UserDefinedFunction + +/** + * Both the evaluate method and the User Defined Function instance are passed in, as both are needed by the Method.invoke + * function. + */ +case class UserDefinedFunctionCall(recipes: List[CompiledDPath], userDefinedFunction: UserDefinedFunction, evaluateFxn: Method) + extends FNArgsList(recipes) { + override def computeValue(values: List[Any], dstate: DState) = { + val jValues = values.map { _.asInstanceOf[Object] } + val res = evaluateFxn.invoke(userDefinedFunction, jValues: _*) Review comment: My proposal was that a UDFException be interperated as a processing error, and all other errors be interperated as some form of nonrecoverable error (either an RSDE, or some new class of error). The idea being that if a UDF throws a UDFException, it is indicating that it considers the input data to be invalid. That is not nessaserilly a problem with the schema, just that the particular data being input is not valid. In such a case, it makes sense for Daffodil to backtrack and try a different branch. If the UDF throws any other kind of exception, it means that Daffodil encountered a bug in the UDF. Since we know this situation can only arise as a result of an error, there is no safe way to proceed from a correctness perspective (eg. the UDF did not signal that it was supposed to fail, so we do not know that backtracking is correct). For the backtrace, I think we do want a backtrace showing all of the UDF portion of the stack. These functions could be highly non-trivial; possible being wrappers around a complex user implemented library. This situation will only arise when there is a programming error somewhere in the UDF and its dependency, so we should give the normal programming debugging output of a stacktrace. Restricting the stacktrace to the UDF portion of the stack is more of a nice-to-have than a critical feature. Daffodil stack traces can get pretty long, and won't have any useful information for debugging the UDF. Plus, restricting the stacktrace will let me pretend that we are not just dynamically splicing user code into our running program. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
