paul-rogers opened a new pull request #1948: DRILL-7506: Simplify code gen error handling URL: https://github.com/apache/drill/pull/1948 Revises error handling in code generation to push exceptions closer to the source of the error to both provide clearer errors and simplify code. ## Background Drill evolved to have 2 1/2 ways to report errors. This PR is a step towards simplifying the execution engine to use just one form of error reporting. The original design of the "Volcano iterator" envisioned a `STOP` status that indicated that an error occurred. When an operator encountered and error, it would: * Log the error. * Pass an exception describing the error to the fragment context, telling the fragment context that the fragment has failed. * Pass `STOP` up the iterator call stack to stop execution. Different operators implemented this in different ways. Some performed `close()` operations at the time the error was found. Others put themselves into a special "stop" state. Some call `kill()` on incoming batches others do not. Even from the beginning, the fragment executor had to handle runtime exceptions such as NPE, OOM and so on. As a result, some operators adopted a second way to report errors: simply throw a runtime (unchecked) exception. At the time that the managed sort was created, we put in effort to ensure that the fragment executor properly closes all operators in a fragment when a runtime exception was thrown. The years since have shown that this solution works. We also resolved many cases in which it was not clear when `close()` should be called. When the exception is thrown (or `STOP`) is returned? In the `kill()` call? Only by the fragment executor? It turned out that the only reliable solution was for the fragment executor to call `close()`, from the leaf-most to the root-most operators. The "2 1/2th" solution is that code *should* throw a `UserException` that contains information useful to the user to describe the error. Some operators use `UserException`, others throw other unchecked exceptions. This PR moves toward using `UserException` as the one reliable way to report errors. This PR focuses on code generation. It pushes code gen error handling close to the code gen itself to allow clearer error messages. Doing so avoids the need to bubble code gen exceptions up the call stack, resulting in cleaner operator code. ## Tests All unit tests were rerun to ensure there is no behavior change from this PR. A couple of tests were modified to account for the use of a standardized `UserException` instead of the various other exceptions thrown previously.
---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services
