Fix incorrect loading/reload of schema [GH #16]

SQLite keeps the temporary and main meta information tables separate. The main
tables are in 'sqlite_master' and the temporary tables are in
'sqlite_temp_master'. Amalgalite was not finding the temp tables when looking up
the tables by name, and it was not reloading the schema correctly if there was a
change to the temporary table.

lib/amalgalite/schema.rb

@@ -15,55 +15,85 @@ module Amalgalite
   #
   class Schema
 
+    # The internal database that this schema is for. Most of the time this will
+    # be 'main' for the main database. For the temp tables, this will be 'temp'
+    # and for any attached databsae, this is the name of attached database.
     attr_reader :catalog
-    attr_reader :schema 
+
+    # The schema_version at the time this schema was taken.
     attr_reader :schema_version
-    attr_writer :dirty
+
+    # The Amalagalite::Database this schema is associated with.
     attr_reader :db
 
     #
     # Create a new instance of Schema
     #
-    def initialize( db, catalog = 'main', schema = 'sqlite')
-      @db = db
-      @catalog = catalog
-      @schema = schema
+    def initialize( db, catalog = 'main', master_table = 'sqlite_master' )
+      @db             = db
+      @catalog        = catalog
       @schema_version = nil
-      @tables = {}
-      @views  = {}
+      @tables         = {}
+      @views          = {}
+      @master_table   = master_table
+
+      if @master_table == 'sqlite_master' then
+        @temp_schema = ::Amalgalite::Schema.new( db, 'temp', 'sqlite_temp_master')
+      else
+        @temp_schema = nil
+      end
       load_schema!
     end
 
-    def dirty?() 
-      return (@schema_version != self.current_version)
+    def catalog_master_table
+      "#{catalog}.#{@master_table}"
+    end
+
+    def temporary?
+      catalog == "temp"
+    end
+
+    def dirty?()
+      return true  if (@schema_version != self.current_version)
+      return false unless @temp_schema
+      return @temp_schema.dirty?
     end
 
     def current_version
-      @db.first_value_from("PRAGMA schema_version")
+      @db.first_value_from("PRAGMA #{catalog}.schema_version")
     end
 
     #
     # load the schema from the database
     def load_schema!
       load_tables
       load_views
+      if @temp_schema then
+        @temp_schema.load_schema!
+      end
       @schema_version = self.current_version
       nil
     end
 
     ##
-    # return the tables, reloading if dirty
+    # return the tables, reloading if dirty.
+    # If there is a temp table and a normal table with the same name, then the
+    # temp table is the one that is returned in the hash.
     def tables
       load_schema! if dirty?
-      return @tables
+      t = @tables
+      if @temp_schema then
+        t = @tables.merge( @temp_schema.tables )
+      end
+      return t
     end
 
     ##
     # load all the tables
     #
     def load_tables
       @tables = {}
-      @db.execute("SELECT tbl_name FROM sqlite_master WHERE type = 'table' AND name != 'sqlite_sequence'") do |table_info|
+      @db.execute("SELECT tbl_name FROM #{catalog_master_table} WHERE type = 'table' AND name != 'sqlite_sequence'") do |table_info|
         table = load_table( table_info['tbl_name'] )
         table.indexes = load_indexes( table )
         @tables[table.name] = table
@@ -74,36 +104,26 @@ def load_tables
     ##
     # Load a single table
     def load_table( table_name )
-      rows = @db.execute("SELECT tbl_name, sql FROM sqlite_master WHERE type = 'table' AND tbl_name = ?", table_name)
+      rows = @db.execute("SELECT tbl_name, sql FROM #{catalog_master_table} WHERE type = 'table' AND tbl_name = ?", table_name)
       table_info = rows.first
       table = nil
-      if table_info then 
+      if table_info then
         table = Amalgalite::Table.new( table_info['tbl_name'], table_info['sql'] )
-        table.columns = load_columns( table )
         table.schema = self
+        table.columns = load_columns( table )
         table.indexes = load_indexes( table )
-        @tables[table.name] = table 
-      else
-        # might be a temporary table
-        table = Amalgalite::Table.new( table_name, nil )
-        cols = load_columns( table )
-        if cols.size > 0 then
-          table.columns = cols
-          table.schema = self
-          table.indexes = load_indexes( table )
-          @tables[table.name] = table
-        end
+        @tables[table.name] = table
       end
       return table
     end
 
-    ## 
+    ##
     # load all the indexes for a particular table
     #
     def load_indexes( table )
       indexes = {}
 
-      @db.prepare("SELECT name, sql FROM sqlite_master WHERE type ='index' and tbl_name = $name") do |idx_stmt|
+      @db.prepare("SELECT name, sql FROM #{catalog_master_table} WHERE type ='index' and tbl_name = $name") do |idx_stmt|
         idx_stmt.execute( "$name" => table.name) do |idx_info|
           indexes[idx_info['name']] = Amalgalite::Index.new( idx_info['name'], idx_info['sql'], table )
         end
@@ -134,8 +154,8 @@ def load_indexes( table )
     def load_columns( table )
       cols = {}
       idx = 0
-      @db.execute("PRAGMA table_info(#{@db.quote(table.name)})") do |row|
-        col = Amalgalite::Column.new( "main", table.name, row['name'], row['cid'])
+      @db.execute("PRAGMA #{catalog}.table_info(#{@db.quote(table.name)})") do |row|
+        col = Amalgalite::Column.new( catalog,  table.name, row['name'], row['cid'])
 
         col.default_value       = row['dflt_value']
 
@@ -154,7 +174,7 @@ def load_columns( table )
 
         unless table.temporary? then
           # get more exact information
-          @db.api.table_column_metadata( "main", table.name, col.name ).each_pair do |key, value|
+          @db.api.table_column_metadata( catalog, table.name, col.name ).each_pair do |key, value|
             col.send("#{key}=", value)
           end
         end
@@ -168,16 +188,23 @@ def load_columns( table )
     ##
     # return the views, reloading if dirty
     #
+    # If there is a temp view, and a regular view of the same name, then the
+    # temporary view is the one that is returned in the hash.
+    #
     def views
       reload_schema! if dirty?
-      return @views
+      v = @views
+      if @temp_schema then
+        v = @views.merge( @temp_schema.views )
+      end
+      return v
     end
 
     ##
     # load a single view
     #
     def load_view( name )
-      rows = @db.execute("SELECT name, sql FROM sqlite_master WHERE type = 'view' AND name = ?", name )
+      rows = @db.execute("SELECT name, sql FROM #{catalog_master_table} WHERE type = 'view' AND name = ?", name )
       view_info = rows.first
       view = Amalgalite::View.new( view_info['name'], view_info['sql'] )
       view.schema = self
@@ -188,7 +215,7 @@ def load_view( name )
     # load all the views for the database
     #
     def load_views
-      @db.execute("SELECT name, sql FROM sqlite_master WHERE type = 'view'") do |view_info|
+      @db.execute("SELECT name, sql FROM #{catalog_master_table} WHERE type = 'view'") do |view_info|
         view = load_view( view_info['name'] )
         @views[view.name] = view
       end

lib/amalgalite/statement.rb

@@ -343,9 +343,10 @@ def result_meta
           column_meta.schema = ::Amalgalite::Column.new( db_name, tbl_name, col_name, idx )
           column_meta.schema.declared_data_type = @stmt_api.column_declared_type( idx )
 
-          # only check for rowid if we have a table name and it is not the
-          # sqlite_master table.  We could get recursion in those cases.
-          if not using_rowid_column? and tbl_name and tbl_name != 'sqlite_master' and is_column_rowid?( tbl_name, col_name ) then
+          # only check for rowid if we have a table name and it is not one of the
+          # sqlite_master tables.  We could get recursion in those cases.
+          if not using_rowid_column? and tbl_name and
+             not %w[ sqlite_master sqlite_temp_master].include?( tbl_name ) and is_column_rowid?( tbl_name, col_name ) then
             @rowid_index = idx
           end
 

lib/amalgalite/table.rb

@@ -5,7 +5,7 @@
 require 'set'
 module Amalgalite
   #
-  # a class representing the meta information about an SQLite table 
+  # a class representing the meta information about an SQLite table
   #
   class Table
     # the schema object the table is associated with
@@ -25,19 +25,19 @@ class Table
     # in this table.  keys are the column names
     attr_accessor :columns
 
-    def initialize( name, sql = nil ) 
+    def initialize( name, sql = nil )
       @name    = name
       @sql     = sql
       @indexes = {}
       @columns = {}
+      @schema  = nil
     end
 
     # Is the table a temporary table or not
     def temporary?
-      !sql
+      schema.temporary?
     end
 
-
     # the Columns in original definition order
     def columns_in_order
       @columns.values.sort_by { |c| c.order }

spec/schema_spec.rb

@@ -63,9 +63,8 @@
 
   it "knows the primary key of a temporary table" do
     @iso_db.execute "CREATE TEMPORARY TABLE tt( a, b INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, c )"
-    tt = @iso_db.schema.load_table( 'tt' )
+    tt = @iso_db.schema.tables[ 'tt' ]
     tt.primary_key.should == [ tt.columns['b'] ]
-
   end
 
   it "knows what the primary key of a table is when it is a multiple column primary key" do
@@ -113,11 +112,20 @@
     s.dirty?.should == true
   end
 
+  it "knows if a temporary table exists" do
+    @iso_db.execute "CREATE TEMPORARY TABLE tt(a,b,c)"
+    @iso_db.schema.tables.keys.include?('tt').should == true
+    @iso_db.schema.tables['tt'].temporary?.should == true
+  end
 
-  it "can load the schema of a temporary table" do
-    @iso_db.execute "CREATE TEMPORARY TABLE tt( a, b, c )"
-    @iso_db.schema.tables['tt'].should be_nil
-    @iso_db.schema.load_table('tt').should_not be_nil
-    @iso_db.schema.tables['tt'].should be_temporary
+  it "sees that temporary tables shadow real tables" do
+    @iso_db.execute "CREATE TABLE tt(x)"
+    @iso_db.schema.tables['tt'].temporary?.should == false
+    @iso_db.execute "CREATE TEMP TABLE tt(a,b,c)"
+    @iso_db.schema.tables['tt'].temporary?.should == true
+    @iso_db.execute "DROP TABLE tt"
+    @iso_db.schema.tables['tt'].temporary?.should == false
+    @iso_db.schema.tables['tt'].columns.size.should == 1
   end
+
 end