reductionista commented on a change in pull request #356: Keras model arch
table helper functions for keras_fit()
URL: https://github.com/apache/madlib/pull/356#discussion_r267127902
##########
File path: src/ports/postgres/modules/convex/test/keras_model_arch_table.sql_in
##########
@@ -83,35 +83,20 @@ SELECT assert(COUNT(model_id) = 0, 'model id 3 should have
been deleted!')
/* Delete a second time, to make sure nothing weird happens.
* It should archrt to the user that the model_id wasn't found but not
* raise an exception or change anything. */
-SELECT delete_keras_model('test_keras_model_arch_table', 3);
-SELECT assert(COUNT(model_id) = 0, 'model id 3 should have been deleted!')
- FROM test_keras_model_arch_table WHERE model_id = 3;
SELECT delete_keras_model('test_keras_model_arch_table', 1);
SELECT assert(COUNT(relname) = 0, 'Table test_keras_model_arch_table should
have been deleted.')
FROM pg_class where relname = 'test_keras_model_arch_table';
SELECT load_keras_model('test_keras_model_arch_table', '{"config" : [1,2,3]}');
DELETE FROM test_keras_model_arch_table;
-/* Test deletion where empty table exists */
-SELECT delete_keras_model('test_keras_model_arch_table', 3);
-SELECT assert(COUNT(relname) = 0, 'Table test_keras_model_arch_table should
have been deleted.') from pg_class where relname =
'test_keras_model_arch_table';
-
/* Test deletion where invalid table exists */
-
SELECT load_keras_model('test_keras_model_arch_table', '{"config" : [1,2,3]}');
ALTER TABLE test_keras_model_arch_table DROP COLUMN model_id;
-CREATE FUNCTION trap_error(stmt TEXT) RETURNS INTEGER AS $$
-BEGIN
- BEGIN
- EXECUTE stmt;
- EXCEPTION
- WHEN OTHERS THEN
- RETURN 1;
- END;
- RETURN 0;
-END;
-$$ LANGUAGE plpgsql;
+
+/* Test deletion where empty table exists */
+select assert(trap_error($$SELECT
delete_keras_model('test_keras_model_arch_table', 3)$$) = 1,
+ 'Deleting a model in an empty table should generate an exception.');
Review comment:
Making this change in behavior means that there is no way to remove an empty
table unless they do it by hand. I think a better behavior is to delete the
table rather than throw an exception; an empty table is an invalid state. In
principle, we shouldn't be able to get into that state. But if that does
happen, I don't see any advantage in keeping it around and forcing the user to
delete it by hand--we can save them the effort by just deleting it, possibly
with a warning message so they know what's happening.
----------------------------------------------------------------
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
