Allow to do a change_table to change partition key

[louiseGrandjonc]

This PR allows a user to create a migration to do

  change_table :project_categories, partition_key: :account_id do |t|
  end

Which will create the primary key (partittion_key, id).

Things this PR does not do:

• Allow the user to reverse his decision (go back to pkey: id instead of partition_key, id). This should be okay as users rarely undistribute a table, and they could just drop the migration
• Check that the primary key already has the right columns before dropping and creating the pkey. Though it checks the number of column.

[lfittl]

Can we use a bind parameter here, instead of string substitution?

Whilst its unlikely that someone will do SQL injection here, it seems better to avoid problems with table_name called '; DROP TABLE xyz; SELECT '

[lfittl]

Same here regarding SQL injection - I think there are methods to escape identifier names in ActiveRecord::Base that we can use

[louiseGrandjonc]

Actually, I tried looking into it, the method to sanitize raw sql is sanitize_sql_array, except here, we are taking the name of the table in the ALTER TABLE which ends up like this:

query = ActiveRecord::Base::sanitize_sql_array(["ALTER TABLE $1 DROP CONSTRAINT IF EXISTS $2_pkey;", table_name, table_name])
execute(query)

And this triggers errors because SQL doesn't suppose that format

If you have an idea how to do change the queries, let me know

[lfittl]

Of course, to be fair my own code was not up to my comments :-)

[louiseGrandjonc]

haha, noted, I'll work on avoiding SQL injection ;)

[lfittl]

I think we can not pass the block at all, unless that throws an error:

change_table :project_categories, partition_key: :account_id

[lfittl]

For one liners usually in Ruby you'd use the { .. } syntax:

columns_result.values.each { |c| columns.push(c[0]) }

[louiseGrandjonc]

is there anyway to do this without initializing a constant? in python you can do
columns = [e[0] for e in columns_result.values]

[lfittl]

Yes, you could do it like this:

columns = columns_result.values.map { |c| c[0] } 

or without the block, utilizing the fact that there is a first method on arrays:

columns = columns_result.values.map(&:first)

[brunoprietog]

Hi, are you planning to merge this?

[serprex]

No, both Louis & Lukas are no longer with Citus

If someone else wants this to get through they can create a new PR based on this one