Skip to content

FIX: errors running rake db:migrate while creating a table #10

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

FIX: errors running rake db:migrate while creating a table #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1aae382
FIX: ArgumentError: wrong number of arguments (4 for 3)
nunommc Aug 20, 2014
734d633
ERROR: Column "table_name.id" has unsupported type "serial".
nunommc Aug 20, 2014
eeb83b0
FIX: ArgumentError: wrong number of arguments (3 for 1..2)
nunommc Aug 20, 2014
15620fb
FIX: ERROR: relation "FactSale" does not exist
nunommc Aug 27, 2014
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
2 changes: 1 addition & 1 deletion lib/active_record/connection_adapters/redshift/database_statements.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def pp(result)

# Executes a SELECT query and returns an array of rows. Each row is an
# array of field values.
def select_rows(sql, name = nil)
def select_rows(sql, name = nil, binds = [])
select_raw(sql, name).last
end

Expand Down
13 changes: 7 additions & 6 deletions lib/active_record/connection_adapters/redshift_adapter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ class Table < ActiveRecord::ConnectionAdapters::Table
ADAPTER_NAME = 'Redshift'

NATIVE_DATABASE_TYPES = {
primary_key: "serial primary key",
primary_key: "integer identity",
Copy link
Owner

Choose a reason for hiding this comment

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

"primary key" and "identity" are not identical, this should be "integer primary key".

Copy link

Choose a reason for hiding this comment

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

To chime in with some of my initial experiences here:

  • The default serial primary key seems to cause ActiveRecord::StatementInvalid: PG::FeatureNotSupported: ERROR: Column "sequences.id" has unsupported type "serial". Why others don't get this, I don't know.
  • By using integer primary key as suggested by @aamine , I get: ActiveRecord::StatementInvalid: PG::InternalError: ERROR: Cannot insert a NULL value into column id - indicating that RDS is not auto-incrementing, and that perhaps I should be doing something for that application-side. Not sure what is actually recommended in this case.
  • With using @nunommc 's integer identity, I get ActiveRecord::UnknownPrimaryKey: Unknown primary key for table lists in model List. This is solved by adding to my models: self.primary_key = "id".

string: { name: "character varying", limit: 255 },
text: { name: "text" },
integer: { name: "integer" },
Expand Down Expand Up @@ -867,15 +867,16 @@ def select_raw(sql, name = nil)
# - format_type includes the column size constraint, e.g. varchar(50)
# - ::regclass is a function that gives the id for a table name
def column_definitions(table_name) #:nodoc:
exec_query(<<-end_sql, 'SCHEMA').rows

exec_query(<<-SQL, 'SCHEMA').rows
SELECT a.attname, format_type(a.atttypid, a.atttypmod),
pg_get_expr(d.adbin, d.adrelid), a.attnotnull, a.atttypid, a.atttypmod
FROM pg_attribute a LEFT JOIN pg_attrdef d
ON a.attrelid = d.adrelid AND a.attnum = d.adnum
WHERE a.attrelid = '#{quote_table_name(table_name)}'::regclass
WHERE a.attrelid = '#{table_name}'::regclass
Copy link
Owner

Choose a reason for hiding this comment

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

Is this change really correct? PostgreSQL adapter (bundled with ActiveRecord 4.1.5) is applying quote_table_name here.

Copy link
Author

Choose a reason for hiding this comment

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

If table_name was for example "DimTime" the condition created was something like

WHERE a.attrelid = '"DimTime."'::regclass -- quotes followed by double-quotes

If you run the following Query

SELECT '"DimTime"'::regclass -- Note: I removed the dot also

it returns ERROR: relation "DimTime" does not exist

The other issue I had in this function was about the dot that follows the table_name (that is being saved also like that in the db/schema.rb), so I ended up doing the following:

table_name_with_no_dots, _, _ = table_name.partition('.')
exec_query(<<-SQL, 'SCHEMA').rows
              SELECT -- (...)
               WHERE a.attrelid = '#{table_name_with_no_dots}'::regclass
               -- (...)
          SQL

Copy link
Owner

Choose a reason for hiding this comment

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

As an another problem, the table name such like "aaa'xxx" (including single quote) is not correctly handled. Also it might cause SQL injection.

I think quote_table_name method should be rewritten for Redshift --- current (PostgreSQL) version of quote_table_name simply does not work.

AND a.attnum > 0 AND NOT a.attisdropped
ORDER BY a.attnum
end_sql
SQL
end

def extract_pg_identifier_from_name(name)
Expand All @@ -893,8 +894,8 @@ def extract_table_ref_from_insert_sql(sql)
$1.strip if $1
end

def create_table_definition(name, temporary, options)
TableDefinition.new native_database_types, name, temporary, options
def create_table_definition(name, temporary, options, as = nil)
TableDefinition.new native_database_types, name, temporary, options, as
end

def update_table_definition(table_name, base)
Expand Down