Improved existing tables handling and some fixes related to #1103

app/models/data_import.rb

@@ -297,7 +297,7 @@ class DataImport < Sequel::Model
 
     candidates =  current_user.tables.select_map(:name)
     table_name = ::Table.get_valid_table_name(name, {
-        name_candidates: candidates,
+        connection: current_user.in_database,
         database_schema: current_user.database_schema
     })
     current_user.in_database.run(%Q{CREATE TABLE #{table_name} AS #{query}})

app/models/table.rb

@@ -489,7 +489,7 @@ class Table < Sequel::Model(:user_tables)
         @data_import  = DataImport.find(:id=>self.data_import_id)
       end
 
-      importer_result_name = import_to_cartodb
+      importer_result_name = import_to_cartodb(name)
 
       @data_import.reload
       @data_import.table_name = importer_result_name
@@ -1498,6 +1498,7 @@ class Table < Sequel::Model(:user_tables)
     name_candidates = self.owner.tables.select_map(:name) if owner
 
     options.merge!(name_candidates: name_candidates)
+    options.merge!(connection: self.owner.connection) unless self.owner.nil?
     unless options[:database_schema].present? || self.owner.nil?
       options.merge!(database_schema: self.owner.database_schema)
     end
@@ -1526,8 +1527,9 @@ class Table < Sequel::Model(:user_tables)
     database_schema = options[:database_schema].present? ? options[:database_schema] : 'public'
 
     # We don't want to use an existing table name
-    existing_names = options[:name_candidates] || \
-      options[:connection]["select relname from pg_stat_user_tables WHERE schemaname='#{database_schema}'"].map(:relname)
+    # 
+    existing_names = []
+    existing_names = options[:name_candidates] || options[:connection]["select relname from pg_stat_user_tables WHERE schemaname='#{database_schema}'"].map(:relname) if options[:connection]
     existing_names = existing_names + User::SYSTEM_TABLE_NAMES
     rx = /_(\d+)$/
     count = name[rx][1].to_i rescue 0

app/models/table_registrar.rb

@@ -10,7 +10,8 @@ module CartoDB
     def register(table_name, data_import_id)
       self.table                    = table_klass.new
       table.user_id                 = user.id
-      table.name                    = table_name
+      # INFO: we're not creating but registering an existent table, so we want fixed, known name
+      table.instance_eval { self[:name] = table_name }
       table.migrate_existing_table  = table_name
       table.data_import_id          = data_import_id
       table.save
@@ -24,7 +25,7 @@ module CartoDB
 
     def get_valid_table_name(table_name)
       table_klass.get_valid_table_name(table_name, {
-          name_candidates: user.reload.tables.map(&:name),
+          connection: user.in_database,
           database_schema: user.database_schema
         })
     end

app/models/user.rb

@@ -399,9 +399,7 @@ class User < Sequel::Model
     configuration = get_db_configuration_for(options[:as])
 
     connection = $pool.fetch(configuration) do
-      db = ::Sequel.connect(configuration.merge(:after_connect=>(proc do |conn|
-        conn.execute(%Q{ SET search_path TO "#{self.database_schema}", cartodb, public }) unless options[:as] == :cluster_admin
-      end)))
+      db = get_database(options, configuration)
       db.extension(:connection_validator)
       db.pool.connection_validation_timeout = configuration.fetch('conn_validator_timeout', -1)
       db
@@ -414,6 +412,20 @@ class User < Sequel::Model
     end
   end
 
+  def connection(options = {})
+    configuration = get_db_configuration_for(options[:as])
+
+    $pool.fetch(configuration) do
+      get_database(options, configuration)
+    end
+  end
+
+  def get_database(options, configuration)
+      ::Sequel.connect(configuration.merge(:after_connect=>(proc do |conn|
+        conn.execute(%Q{ SET search_path TO "#{self.database_schema}", cartodb, public }) unless options[:as] == :cluster_admin
+      end)))
+  end
+
   def get_db_configuration_for(user = nil)
     logger = (Rails.env.development? || Rails.env.test? ? ::Rails.logger : nil)
     if user == :superuser

lib/importer/lib/cartodb-migrator/migrator.rb

@@ -170,9 +170,12 @@ module CartoDB
         }
       ]
 
+      sanitization_count = 0
+
       sanitization_map = sanitization_map.inject({}) { |memo, pair|
         if memo.values.include?(pair.last) || correct_columns.include?(pair.last)
-          memo.merge(pair.first => "#{pair.last}_1")
+          sanitization_count += 1 
+          memo.merge(pair.first => "#{pair.last}_#{sanitization_count}")
         else
           memo.merge(pair.first => pair.last)
         end

services/importer/spec/unit/georeferencer_spec.rb

@@ -181,9 +181,7 @@ describe Importer2::Georeferencer do
   # Attempts to create a new database schema
   # Does not raise exception if the schema already exists
   def create_schema(db, schema)
-    db.run(%Q{CREATE SCHEMA #{schema}})
-  rescue Sequel::DatabaseError => e
-    raise unless e.message =~ /schema .* already exists/
+    db.run(%Q{CREATE SCHEMA IF NOT EXISTS #{schema}})
   end #create_schema
 
   def create_table(db, options={})

spec/support/factories/tables.rb

@@ -5,16 +5,17 @@ module CartoDB
       table = ::Table.new(attributes)
       table.user_id = if attributes[:user_id].nil?
         UUIDTools::UUID.timestamp_create.to_s
-        #create_user.id
       else
         attributes.delete(:user_id)
       end
+
       table.name = if attributes.keys.include?(:name) && attributes[:name] == nil
         attributes.delete(:name)
         nil
       else
         attributes[:name] || String.random(10)
       end
+      
       table
     end