Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Correctly cache mysql server version #536

Closed
wants to merge 4 commits into from

2 participants

Hannes Georg Jeremy Evans
Hannes Georg

Hi

I came across the following issue: the mysql adapter is constantly requerying the version of the server. I found out that the responsible code does some caching but only for the last step of the computation ( which happens to be the least expensive one, btw. ). I fixed the caching and ran into the next problem: I could not add a spec for it since the server_version is fixed in the mysql_spec after one certain set of tests using meta_def . I added a method which reverts meta_def and added a clean-up code so that later tests get the correct server_version.

Is it okay to leave the two things in one pull request or should I split them up?

Thank you
Hannes

Jeremy Evans
Owner

The fixing of the caching looks good. However, I'm not OK with the addition of meta_remove. The meta_def line in the spec can just be removed, it's unlikely anyone is running the specs on MySQL < 5.0.14. I'll just cherry-pick the cache fix and spec. Thanks for your help!

Jeremy Evans
Owner

Actually, the spec needs to be modified. Sequel's specs need to run cleanly on RSpec 1.3+, and the expect syntax was only added to a recent release of RSpec 2.

Jeremy Evans
Owner

I cherry-picked 2 of these commits and added another to cleanup the specs. Thanks for your help!

Jeremy Evans jeremyevans closed this August 16, 2012
Hannes Georg

Ah, okay, I wasn't aware that compatibility with rspec 1.3+ is required.

Thank you for simply cherry-picking. I would have changed the commit if you would have asked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
6  lib/sequel/adapters/shared/mysql.rb
@@ -111,8 +111,10 @@ def rollback_prepared_transaction(transaction_id)
111 111
 
112 112
       # Get version of MySQL server, used for determined capabilities.
113 113
       def server_version
114  
-        m = /(\d+)\.(\d+)\.(\d+)/.match(get(SQL::Function.new(:version)))
115  
-        @server_version ||= (m[1].to_i * 10000) + (m[2].to_i * 100) + m[3].to_i
  114
+        @server_version ||= begin
  115
+          m = /(\d+)\.(\d+)\.(\d+)/.match(get(SQL::Function.new(:version)))
  116
+          (m[1].to_i * 10000) + (m[2].to_i * 100) + m[3].to_i
  117
+        end
116 118
       end
117 119
       
118 120
       # MySQL supports CREATE TABLE IF NOT EXISTS syntax.
17  lib/sequel/metaprogramming.rb
@@ -9,5 +9,22 @@ module Metaprogramming
9 9
     def meta_def(name, &block)
10 10
       (class << self; self end).send(:define_method, name, &block)
11 11
     end
  12
+
  13
+    # Remove a meta method with the given name.
  14
+    # This is useful in conjunction with meta_def to temporarily change the behavior of an object.
  15
+    #
  16
+    #   object = Object.new
  17
+    #   object.extend(Sequel::Metaprogramming)
  18
+    #   object.to_s #=> "#<Object:0x000000022b6dd8>"
  19
+    #   
  20
+    #   object.meta_def(:to_s){ "my fancy to_s" }
  21
+    #   object.to_s #=> "my fancy to_s"
  22
+    #   object.meta_remove(:to_s)
  23
+    #   
  24
+    #   object.to_s #=> "#<Object:0x000000022b6dd8>"
  25
+    #
  26
+    def meta_remove(name)
  27
+      (class << self; self end).send(:remove_method, name)
  28
+    end
12 29
   end
13 30
 end
12  spec/adapters/mysql_spec.rb
@@ -261,6 +261,10 @@ def logger.method_missing(m, msg)
261 261
     @ds.db.meta_def(:server_version) {50014}
262 262
   end
263 263
 
  264
+  after do
  265
+    @ds.db.meta_remove(:server_version)
  266
+  end
  267
+
264 268
   specify "should raise error for :full_outer join requests." do
265 269
     lambda{@ds.join_table(:full_outer, :nodes)}.should raise_error(Sequel::Error)
266 270
   end
@@ -356,6 +360,14 @@ def logger.method_missing(m, msg)
356 360
     MYSQL_DB.server_version.should >= 40000
357 361
   end
358 362
 
  363
+  specify "should cache the server version" do
  364
+    # warm cache:
  365
+    MYSQL_DB.server_version
  366
+    expect{
  367
+      100.times{ MYSQL_DB.server_version }
  368
+    }.not_to change(MYSQL_DB.sqls, :size)
  369
+  end
  370
+
359 371
   specify "should support for_share" do
360 372
     MYSQL_DB.transaction{MYSQL_DB[:test2].for_share.all.should == []}
361 373
   end
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.