From 851b6bae8a8515913589de5155cc7b3eb620e199 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 13 Nov 2019 19:16:45 +0100 Subject: [PATCH] Add import_as parameter to ODBC connectors See #15213 --- .../connector/providers/fdw/odbc/bigquery.rb | 2 +- .../connector/providers/fdw/odbc/odbc.rb | 4 +- services/importer/spec/unit/connector_spec.rb | 137 +++++++++++++++++- 3 files changed, 138 insertions(+), 5 deletions(-) diff --git a/lib/carto/connector/providers/fdw/odbc/bigquery.rb b/lib/carto/connector/providers/fdw/odbc/bigquery.rb index 7594d9fae8..304fca5e6f 100644 --- a/lib/carto/connector/providers/fdw/odbc/bigquery.rb +++ b/lib/carto/connector/providers/fdw/odbc/bigquery.rb @@ -102,7 +102,7 @@ module Carto end required_parameters %I(project table) - optional_parameters %I(dataset sql_query) + optional_parameters %I(import_as dataset sql_query) def remote_schema_name # Note that DefaultDataset may not be defined and not needed when using IMPORT FOREIGN SCHEMA diff --git a/lib/carto/connector/providers/fdw/odbc/odbc.rb b/lib/carto/connector/providers/fdw/odbc/odbc.rb index 21e38a30ef..1b6d1d5279 100644 --- a/lib/carto/connector/providers/fdw/odbc/odbc.rb +++ b/lib/carto/connector/providers/fdw/odbc/odbc.rb @@ -48,7 +48,7 @@ module Carto end required_parameters %I(table connection) - optional_parameters %I(schema sql_query sql_count encoding columns) + optional_parameters %I(import_as schema sql_query sql_count encoding columns) # ODBC attributes for (non-connection) parameters: { name: :internal_name }# # The :internal_name is what is passed to the driver (through odbc_fdw 'odbc_' options) @@ -81,7 +81,7 @@ module Carto end def table_name - @params[:table] + @params[:import_as] || @params[:table] end def foreign_table_name_for(server_name, name = nil) diff --git a/services/importer/spec/unit/connector_spec.rb b/services/importer/spec/unit/connector_spec.rb index b65ff97b58..7e64c72eff 100644 --- a/services/importer/spec/unit/connector_spec.rb +++ b/services/importer/spec/unit/connector_spec.rb @@ -276,7 +276,135 @@ describe Carto::Connector do ) end - it 'Should quote ODBC paremeters that require it' do + it 'can import an external table under a different name' do + parameters = { + provider: 'mysql', + connection: { + server: 'theserver', + username: 'theuser', + password: 'thepassword', + database: 'thedatabase' + }, + table: 'thetable', + import_as: 'theimportedtable', + encoding: 'theencoding' + } + options = { + logger: @fake_log, + user: @user + } + context = TestConnectorContext.new(@executed_commands = [], options) + connector = Carto::Connector.new(parameters, context) + connector.copy_table schema_name: 'xyz', table_name: 'abc' + + @executed_commands.size.should eq 9 + server_name = match_sql_command(@executed_commands[0][1])[:server_name] + foreign_table_name = %{"cdb_importer"."#{server_name}_theimportedtable"} + user_name = @user.username + user_role = @user.database_username + connector.remote_table_name.should == 'theimportedtable' + + + expect_executed_commands( + @executed_commands, + { + # CREATE SERVER + mode: :superuser, + sql: [{ + command: :create_server, + fdw_name: 'odbc_fdw', + options: { + 'odbc_Driver' => 'MySQL', + 'odbc_server' => 'theserver', + 'odbc_database' => 'thedatabase', + 'odbc_port' => '3306', + "odbc_option" => '0', + "odbc_prefetch" => '0', + "odbc_no_ssps" => '0', + "odbc_can_handle_exp_pwd" => '0' + } + }] + }, { + # CREATE USER MAPPING + mode: :superuser, + sql: [{ + command: :create_user_mapping, + server_name: server_name, + user_name: user_role, + options: { 'odbc_uid' => 'theuser', 'odbc_pwd' => 'thepassword' } + }] + }, { + # CREATE USER MAPPING + mode: :superuser, + sql: [{ + command: :create_user_mapping, + server_name: server_name, + user_name: 'postgres', + options: { 'odbc_uid' => 'theuser', 'odbc_pwd' => 'thepassword' } + }] + }, { + # IMPORT FOREIGH SCHEMA; GRANT SELECT + mode: :superuser, + sql: [{ + command: :import_foreign_schema, + server_name: server_name, + schema_name: 'cdb_importer', + options: { + "schema" => 'thedatabase', + "table" => 'thetable', + "import_as" => 'theimportedtable', + "encoding" => 'theencoding', + "prefix" => "#{server_name}_" + } + }, { + command: :grant_select, + table_name: foreign_table_name, + user_name: user_role + }] + }, { + # CREATE TABLE AS SELECT + mode: :user, + user: user_name, + sql: [{ + command: :create_table_as_select, + table_name: %{"xyz"."abc"}, + select: /\s*\*\s+FROM\s+#{Regexp.escape foreign_table_name}/ + }] + }, { + # DROP FOREIGN TABLE + mode: :superuser, + sql: [{ + command: :drop_foreign_table_if_exists, + table_name: foreign_table_name + }] + }, { + # DROP USER MAPPING + mode: :superuser, + sql: [{ + command: :drop_usermapping_if_exists, + server_name: server_name, + user_name: 'postgres' + }] + }, { + # DROP USER MAPPING + mode: :superuser, + sql: [{ + command: :drop_usermapping_if_exists, + server_name: server_name, + user_name: user_role + }] + }, { + # DROP SERVER + mode: :superuser, + sql: [{ + command: :drop_server_if_exists, + server_name: server_name + }] + } + ) + end + + it 'Should quote ODBC parameters that require it' do parameters = { provider: 'mysql', connection: { @@ -896,6 +1024,7 @@ describe Carto::Connector do 'database' => { required: false } }, 'table' => { required: true }, + 'import_as' => { required: false }, 'schema' => { required: false }, 'sql_query' => { required: false }, 'sql_count' => { required: false }, @@ -1050,6 +1179,7 @@ describe Carto::Connector do 'sslmode' => { required: false } }, 'table' => { required: true }, + 'import_as' => { required: false }, 'schema' => { required: false }, 'sql_query' => { required: false }, 'sql_count' => { required: false }, @@ -1200,6 +1330,7 @@ describe Carto::Connector do 'database' => { required: true } }, 'table' => { required: true }, + 'import_as' => { required: false }, 'schema' => { required: false }, 'sql_query' => { required: false }, 'sql_count' => { required: false }, @@ -1348,6 +1479,7 @@ describe Carto::Connector do 'database' => { required: false } }, 'table' => { required: true }, + 'import_as' => { required: false }, 'schema' => { required: false }, 'sql_query' => { required: false }, 'sql_count' => { required: false }, @@ -1504,6 +1636,7 @@ describe Carto::Connector do 'database' => { required: false } }, 'table' => { required: true }, + 'import_as' => { required: false }, 'schema' => { required: false }, 'sql_query' => { required: false }, 'sql_count' => { required: false }, @@ -2072,7 +2205,7 @@ describe Carto::Connector do 'table' => { required: true }, 'schema' => { required: false }, 'username' => { required: true }, - 'password' => { required: true }, + 'password' => { required: true }, 'server' => { required: true }, 'port' => { required: false }, 'database' => { required: true }