Merge pull request #490 from sparklemotion/flavorjones-exec-batch-exc…

…eptions

use `SQLite3::Exception` consistently

CHANGELOG.md

@@ -23,11 +23,12 @@ This release drops support for Ruby 2.7. [#453] @flavorjones
 - Improve performance of `ResultSet` hashes. [#154, #484, #468] @tenderlove
 - Fix a GC compaction issue with `busy_handler`. [#466] @byroot
 - Remove unused `ResultSet` instance variable. [#469] @tenderlove
+- Fix encoding for values passed to custom functions. [#218, #488] @tenderlove
 
 
 ### Changed
 
-- Raise `StandardError` in a few places where `Exception` was previously raised. [#467] @flavorjones
+- Consistently use `SQLite3::Exception` or subclasses. Previously some `Pragmas` methods raised `Exception`, and `Database#execute_batch2` and `Database#load_extension` raised `RuntimeError`. [#467] @flavorjones
 - `Database#columns` returns a list of frozen strings. [#474] @tenderlove
 - Freeze results that come from the database. [#480] @tenderlove
 - The encoding of a Database is no longer cached. [#485] @tenderlove

ext/sqlite3/aggregator.c

@@ -251,10 +251,7 @@ rb_sqlite3_define_aggregator2(VALUE self, VALUE aggregator, VALUE ruby_name)
                  rb_sqlite3_aggregator_final
              );
 
-    if (status != SQLITE_OK) {
-        rb_sqlite3_raise(ctx->db, status);
-        return self; // just in case rb_sqlite3_raise returns.
-    }
+    CHECK(ctx->db, status);
 
     rb_ary_push(aggregators, aw);
 

ext/sqlite3/database.c

@@ -668,16 +668,13 @@ load_extension(VALUE self, VALUE file)
     sqlite3RubyPtr ctx;
     int status;
     char *errMsg;
-    VALUE errexp;
+
     TypedData_Get_Struct(self, sqlite3Ruby, &database_type, ctx);
     REQUIRE_OPEN_DB(ctx);
 
     status = sqlite3_load_extension(ctx->db, StringValuePtr(file), 0, &errMsg);
-    if (status != SQLITE_OK) {
-        errexp = rb_exc_new2(rb_eRuntimeError, errMsg);
-        sqlite3_free(errMsg);
-        rb_exc_raise(errexp);
-    }
+
+    CHECK_MSG(ctx->db, status, errMsg);
 
     return self;
 }
@@ -779,7 +776,6 @@ exec_batch(VALUE self, VALUE sql, VALUE results_as_hash)
     int status;
     VALUE callback_ary = rb_ary_new();
     char *errMsg;
-    VALUE errexp;
 
     TypedData_Get_Struct(self, sqlite3Ruby, &database_type, ctx);
     REQUIRE_OPEN_DB(ctx);
@@ -794,11 +790,7 @@ exec_batch(VALUE self, VALUE sql, VALUE results_as_hash)
                               &errMsg);
     }
 
-    if (status != SQLITE_OK) {
-        errexp = rb_exc_new2(rb_eRuntimeError, errMsg);
-        sqlite3_free(errMsg);
-        rb_exc_raise(errexp);
-    }
+    CHECK_MSG(ctx->db, status, errMsg);
 
     return callback_ary;
 }

ext/sqlite3/exception.c

@@ -90,10 +90,28 @@ rb_sqlite3_raise(sqlite3 *db, int status)
             klass = rb_path2class("SQLite3::NotADatabaseException");
             break;
         default:
-            klass = rb_eRuntimeError;
+            klass = rb_path2class("SQLite3::Exception");
     }
 
     klass = rb_exc_new2(klass, sqlite3_errmsg(db));
     rb_iv_set(klass, "@code", INT2FIX(status));
     rb_exc_raise(klass);
 }
+
+/*
+ *  accepts a sqlite3 error message as the final argument, which will be `sqlite3_free`d
+ */
+void
+rb_sqlite3_raise_msg(sqlite3 *db, int status, const char* msg)
+{
+  VALUE exception;
+
+  if (status == SQLITE_OK) {
+    return;
+  }
+
+  exception = rb_exc_new2(rb_path2class("SQLite3::Exception"), msg);
+  sqlite3_free((void*)msg);
+  rb_iv_set(exception, "@code", INT2FIX(status));
+  rb_exc_raise(exception);
+}

ext/sqlite3/exception.h

@@ -2,7 +2,9 @@
 #define SQLITE3_EXCEPTION_RUBY
 
 #define CHECK(_db, _status) rb_sqlite3_raise(_db, _status);
+#define CHECK_MSG(_db, _status, _msg) rb_sqlite3_raise_msg(_db, _status, _msg);
 
 void rb_sqlite3_raise(sqlite3 *db, int status);
+void rb_sqlite3_raise_msg(sqlite3 *db, int status, const char* msg);
 
 #endif

lib/sqlite3/pragmas.rb

@@ -22,16 +22,14 @@ def set_boolean_pragma(name, mode)
         when "on", "yes", "true", "y", "t" then mode = "'ON'"
         when "off", "no", "false", "n", "f" then mode = "'OFF'"
         else
-          raise StandardError,
-            "unrecognized pragma parameter #{mode.inspect}"
+          raise SQLite3::Exception, "unrecognized pragma parameter #{mode.inspect}"
         end
       when true, 1
         mode = "ON"
       when false, 0, nil
         mode = "OFF"
       else
-        raise StandardError,
-          "unrecognized pragma parameter #{mode.inspect}"
+        raise SQLite3::Exception, "unrecognized pragma parameter #{mode.inspect}"
       end
 
       execute("PRAGMA #{name}=#{mode}")
@@ -62,8 +60,7 @@ def get_enum_pragma(name)
     def set_enum_pragma(name, mode, enums)
       match = enums.find { |p| p.find { |i| i.to_s.downcase == mode.to_s.downcase } }
       unless match
-        raise StandardError,
-          "unrecognized #{name} #{mode.inspect}"
+        raise SQLite3::Exception, "unrecognized #{name} #{mode.inspect}"
       end
       execute("PRAGMA #{name}='#{match.first.upcase}'")
     end

test/test_database.rb

@@ -200,7 +200,7 @@ def test_execute_batch2
       end
       assert_equal [[1, 50], [2, 60]], return_value
 
-      assert_raises(RuntimeError) do
+      assert_raises(SQLite3::Exception) do
         # "names" is not a valid column
         @db.execute_batch2 'INSERT INTO items (names) VALUES ("bazz")'
       end
@@ -631,6 +631,11 @@ def test_load_extension_with_nonstring_argument
       assert_raises(TypeError) { db.load_extension(Pathname.new("foo.so")) }
     end
 
+    def test_load_extension_error
+      db = SQLite3::Database.new(":memory:")
+      assert_raises(SQLite3::Exception) { db.load_extension("path/to/foo.so") }
+    end
+
     def test_raw_float_infinity
       # https://github.com/sparklemotion/sqlite3-ruby/issues/396
       skip if SQLite3::SQLITE_LOADED_VERSION >= "3.43.0"

test/test_pragmas.rb

@@ -7,6 +7,20 @@ def setup
       @db = SQLite3::Database.new(":memory:")
     end
 
+    def test_pragma_errors
+      assert_raises(SQLite3::Exception) do
+        @db.set_enum_pragma("foo", "bar", [])
+      end
+
+      assert_raises(SQLite3::Exception) do
+        @db.set_boolean_pragma("read_uncommitted", "foo")
+      end
+
+      assert_raises(SQLite3::Exception) do
+        @db.set_boolean_pragma("read_uncommitted", 42)
+      end
+    end
+
     def test_get_boolean_pragma
       refute(@db.get_boolean_pragma("read_uncommitted"))
     end