Skip to content

use SQLite3::Exception consistently #490

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 1 addition & 4 deletions ext/sqlite3/aggregator.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
16 changes: 4 additions & 12 deletions ext/sqlite3/database.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand All @@ -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;
}
Expand Down
20 changes: 19 additions & 1 deletion ext/sqlite3/exception.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
2 changes: 2 additions & 0 deletions ext/sqlite3/exception.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 3 additions & 6 deletions lib/sqlite3/pragmas.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
Expand Down Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion test/test_database.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down
14 changes: 14 additions & 0 deletions test/test_pragmas.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down