Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Freeze results that come from the database #480

Merged
merged 2 commits into from
Jan 24, 2024
Merged

Freeze results that come from the database #480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cb9481e
Freeze results that come from the database
tenderlove Jan 23, 2024
839c288
Only test Ractor sharability if Ractors are defined
tenderlove Jan 24, 2024
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
4 changes: 4 additions & 0 deletions ext/sqlite3/statement.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,15 @@ step(VALUE self)
if (internal_encoding) {
val = rb_str_export_to_enc(val, internal_encoding);
}
rb_obj_freeze(val);
Copy link
Member

@flavorjones flavorjones Apr 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, this seems to break Rails's schema cache tests (which use Marshal):

bin/test test/cases/connection_adapters/schema_cache_test.rb -n/test_gzip_dumps_identical/

Without this line, the test passes. With it, the test fails (in isolation).

I'm looking into it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Context on that test: rails/rails#49166

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See rails/rails#51581 for the exciting conclusion.

}
break;
case SQLITE_BLOB: {
val = rb_str_new(
(const char *)sqlite3_column_blob(stmt, i),
(long)sqlite3_column_bytes(stmt, i)
);
rb_obj_freeze(val);
}
break;
case SQLITE_NULL:
Expand All @@ -192,6 +194,8 @@ step(VALUE self)
CHECK(sqlite3_db_handle(ctx->st), value);
}

rb_obj_freeze(list);

return list;
}

Expand Down
2 changes: 1 addition & 1 deletion lib/sqlite3/database.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ def execute sql, bind_vars = [], *args, &block
yield row
end
else
stmt.to_a
stmt.to_a.freeze
end
end
end
Expand Down
6 changes: 3 additions & 3 deletions test/test_encoding.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def test_blob_is_binary

string = @db.execute("select data from foo").first.first
assert_equal Encoding.find("ASCII-8BIT"), string.encoding
assert_equal str, string.force_encoding("UTF-8")
assert_equal str, string.dup.force_encoding("UTF-8")
end

def test_blob_is_ascii8bit
Expand All @@ -95,7 +95,7 @@ def test_blob_is_ascii8bit

string = @db.execute("select data from foo").first.first
assert_equal Encoding.find("ASCII-8BIT"), string.encoding
assert_equal str, string.force_encoding("UTF-8")
assert_equal str, string.dup.force_encoding("UTF-8")
end

def test_blob_with_eucjp
Expand All @@ -108,7 +108,7 @@ def test_blob_with_eucjp

string = @db.execute("select data from foo").first.first
assert_equal Encoding.find("ASCII-8BIT"), string.encoding
assert_equal str, string.force_encoding("EUC-JP")
assert_equal str, string.dup.force_encoding("EUC-JP")
end

def test_db_with_eucjp
Expand Down
19 changes: 19 additions & 0 deletions test/test_statement.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,25 @@ def teardown
@db.close
end

def test_rows_should_be_frozen
@db.execute 'CREATE TABLE "things" ("float" float, "int" int, "text" blob, "string" string, "nil" string)'
stmt = @db.prepare "INSERT INTO things (float, int, text, string, nil) VALUES (?, ?, ?, ?, ?)"
stmt.execute(1.2, 2, "blob", "string", nil)
stmt.close

rows = @db.execute "SELECT float, int, text, string, nil FROM things"
assert_predicate rows, :frozen?
assert_equal 1, rows.length
row = rows[0]
assert_predicate row, :frozen?
row.each { |item| assert_predicate item, :frozen? }

if defined?(Ractor)
assert Ractor.shareable?(rows)
assert Ractor.shareable?(row)
end
end

def test_double_close_does_not_segv
@db.execute 'CREATE TABLE "things" ("number" float NOT NULL)'

Expand Down