[GitHub] [madlib] fmcquillan99 commented on pull request #498: DL: Add utility function for custom functions

[GitBox <gi...@apache.org>]

fmcquillan99 commented on pull request #498:
URL: https://github.com/apache/madlib/pull/498#issuecomment-631510579


   Preliminary comments:
   
   (1)
   in addition to 
   ```
   delete_custom_function(
       object_table,
       id
   )
   ```
   I suggest we also support
   ```
   delete_custom_function(
       object_table,
       name
   )
   ```
   since `name` is unique and is the way that users will pass object to Keras, so I suggest we allow to delete by `name` in addition to `id`
   
   
   (2)
   duplicate object name gives an error as it should, but it is not a user friendly error: 
   ```
   InternalError_: AttributeError: 'module' object has no attribute 'UniqueViolation' (plpython.c:5038)
   CONTEXT:  Traceback (most recent call last):
     PL/Python function "load_custom_function", line 21, in <module>
       madlib_keras_custom_function.load_custom_function(**globals())
     PL/Python function "load_custom_function", line 42, in wrapper
     PL/Python function "load_custom_function", line 93, in load_custom_function
   PL/Python function "load_custom_function"
   ```
   Could we trap this error and give a friendlier error message?
   
   
   (3)
   selection on id not contiguous:
   ```
   SELECT id, name FROM test_custom_function_table ORDER BY id;
    id |  name   
   ----+---------
     1 | sum_fn
     3 | sum_fn1
     5 | sum_fn2
     ```
     Is there a reason why this is not 1,2,3?  It is contiguous with model ids for model architecture table so I would expect the same thing here, plus probably what the user would expect.
   
   
   (4)
   minor spelling error
   
   ```
   select madlib.delete_custom_function( 'test_custom_function_table', 1);
   
   ERROR:  plpy.Error: Keras Custom Funtion: Object id 1 not found (plpython.c:5038)
   CONTEXT:  Traceback (most recent call last):
     PL/Python function "delete_custom_function", line 21, in <module>
       madlib_keras_custom_function.delete_custom_function(**globals())
     PL/Python function "delete_custom_function", line 42, in wrapper
     PL/Python function "delete_custom_function", line 121, in delete_custom_function
   PL/Python function "delete_custom_function"
   ```
   miss spelled "Funtion"
   
   (5)
   When we delete the last id in the table, the table gets deleted:
   ```
   select madlib.delete_custom_function( 'test_custom_function_table', 1);
   
   ERROR:  plpy.Error: Keras Custom Funtion error: Input table 'test_custom_function_table' does not exist. (plpython.c:5038)
   CONTEXT:  Traceback (most recent call last):
     PL/Python function "delete_custom_function", line 21, in <module>
       madlib_keras_custom_function.delete_custom_function(**globals())
     PL/Python function "delete_custom_function", line 42, in wrapper
     PL/Python function "delete_custom_function", line 104, in delete_custom_function
     PL/Python function "delete_custom_function", line 619, in input_tbl_valid
   PL/Python function "delete_custom_function"
   ```
   Is this expected behavior?  i.e., do we do this with model arch table?  Maybe it's OK, not sure.  The other option is to not delete the table, but leave it empty.


----------------------------------------------------------------
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:
users@infra.apache.org