Code review comment for lp:~pbeaman/akiban-server/direct-param-handling

Revision history for this message
Mike McMahon (mmcm) wrote :

It seems that sanitize will add an extra underscore when the first character isn't isJavaIdentifierPart. Perhaps isJavaIdentifierStart should only be checked when it passes Part, too.

I believe that ScriptInvoker will need further refactoring to support ordinary procedures externally defined in a SCRIPT_LIBRARY. Probably the object in the cache just has the new invokeNamedFunction, since any function can be called, and a wrapper remembers which to call for the routine.

The new Direct routines and ordinary SQL routines are in the same URL namespace for /call. That's may even be appropriate, but something then needs to check for duplication between the two registries. This new code does not check for it even among its own entries.

Trying to call the named procedure defined with PARAMETER STYLE LIBRARY via SQL CALL results in a NPE. A pretty specific error is called for.

/procedure does not seem like the best name any more, since what gets posted is a whole script.

The division of labor between RestDMLService and DirectResource is odd, with RestFunctionInvoker doing most of the work.

review: Needs Fixing

« Back to merge proposal