Skip to content
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.

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

[regression] migration changing column started to fail #647

Open
dkuku opened this issue Nov 4, 2024 · 3 comments
Open

[regression] migration changing column started to fail #647

dkuku opened this issue Nov 4, 2024 · 3 comments
Labels

Comments

@dkuku
Copy link

dkuku commented Nov 4, 2024

Elixir version

Elixir 1.16.2 (compiled with Erlang/OTP 26)

Database and Version

any

Ecto Versions

master

Database Adapter and Versions (postgrex, myxql, etc)

postgrex 0.19.2

Current behavior

this migrations started to fail recently:

  def change do
    alter table(:employees) do
      modify(:email, :string, size: 65_535, from: {:string, size: 255})
    end
  end

with error

21:00:26.999 [info] alter table employees
** (Postgrex.Error) ERROR 42601 (syntax_error) syntax error at end of input

    query: ALTER TABLE "employees" 
    (ecto_sql 3.12.1) lib/ecto/adapters/sql.ex:1096: Ecto.Adapters.SQL.raise_sql_call_error/1
    (elixir 1.16.2) lib/enum.ex:1700: Enum."-map/2-lists^map/1-1-"/2
    (ecto_sql 3.12.1) lib/ecto/adapters/sql.ex:1203: Ecto.Adapters.SQL.execute_ddl/4
    (ecto_sql 3.12.1) lib/ecto/migration/runner.ex:348: Ecto.Migration.Runner.log_and_execute_ddl/3
    (elixir 1.16.2) lib/enum.ex:1700: Enum."-map/2-lists^map/1-1-"/2
    (ecto_sql 3.12.1) lib/ecto/migration/runner.ex:311: Ecto.Migration.Runner.perform_operation/3
    (stdlib 5.2.3) timer.erl:270: :timer.tc/2
    (ecto_sql 3.12.1) lib/ecto/migration/runner.ex:25: Ecto.Migration.Runner.run/8

It fails into [this branch] of the if (https://github.com/elixir-ecto/ecto_sql/blame/75ddf591fbe56977b041836079c01be298384024/lib/ecto/adapters/postgres/connection.ex#L1588) but returns [[], [], []] I think it suppose to be be executed the else branch.
I'm assuming it was introduced in this pr because my master version from before 2 weeks was working fine

Expected behavior

the migration works on 3.12.1

@dkuku dkuku added the Kind:Bug label Nov 4, 2024
@josevalim
Copy link
Member

Hrm, we may need to revert the PR after all and introduce a command explicitly to change everything but the type.

@dkuku
Copy link
Author

dkuku commented Nov 4, 2024

the type returned from column_type(type, opts) is identical on 167 and 173
Thats the opts to this function:
[size: 65535, from: {:string, [size: 255]}]

  167 +      column_type = column_type(type, opts)                                                                                            
  168 +      from_column_type = extract_column_type(opts[:from])                                                                              
  169 +                                                                                                                                       
  170 +      drop_reference_expr = drop_reference_expr(opts[:from], table, name)                                                              
  171 +      any_drop_ref? = drop_reference_expr != []                                                                                        
  172 +                                                                                                                                       
  173 +      if column_type == column_type(from_column_type, opts) do   

@josevalim
Copy link
Member

Oh, so it would work if we extracted it properly? 🤔 Can you please send a PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants