-
Notifications
You must be signed in to change notification settings - Fork 11
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
Ecto 3 support #3
base: master
Are you sure you want to change the base?
Conversation
@@ -95,7 +95,7 @@ defmodule RedshiftEcto do | |||
""" | |||
|
|||
# Inherit all behaviour from Ecto.Adapters.SQL | |||
use Ecto.Adapters.SQL, :postgrex | |||
use Ecto.Adapters.SQL, driver: :postgrex, migration_lock: nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
migration_lock is intended to create a read lock on the schema migrations table so that migrations can be run concurrently, but only one runner proceeds. For postgres and mysql, it uses FOR UPDATE
as in SELECT * FROM schema_migrations FOR UPDATE
. I didn't check if redshift has something equivalent.
@@ -515,7 +515,7 @@ if Code.ensure_loaded?(Postgrex) do | |||
defp sources_unaliased(prefix, sources, pos, limit) when pos < limit do | |||
current = | |||
case elem(sources, pos) do | |||
{table, schema} -> | |||
{table, schema, _alias} -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ecto 3 allows naming for bindings. I believe that's what the third argument is, but it'll need to be double checked.
@@ -795,7 +795,7 @@ if Code.ensure_loaded?(Postgrex) do | |||
do: [" DEFAULT ", to_string(literal)] | |||
|
|||
defp default_expr({:ok, %{} = map}, :map) do | |||
default = Ecto.Adapter.json_library().encode!(map) | |||
default = json_library().encode!(map) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ecto 3 got rid of this method and instead tells you to configure postgrex
with the json library. I went with just using the postgrex configured one for now.
{:ecto, "~> 2.2"}, | ||
{:postgrex, "~> 0.13"}, | ||
{:ecto_replay_sandbox, "~> 1.0.0"}, | ||
{:ecto_sql, "~> 3.2"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ecto 3 split into ecto
and ecto_sql
, ecto_sql
pulls in both dependencies and contains the adapter code.
@@ -405,8 +405,8 @@ defmodule RedshiftEctoTest do | |||
query = | |||
"schema" | |||
|> select([m], {m.id, ^true}) | |||
|> join(:inner, [], Schema2, fragment("?", ^true)) | |||
|> join(:inner, [], Schema2, fragment("?", ^false)) | |||
|> join(:inner, [], Schema2, on: fragment("?", ^true)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
join now requires the on clause to be specified under the on:
key
@@ -1126,7 +1163,7 @@ defmodule RedshiftEctoTest do | |||
]} | |||
|
|||
assert execute_ddl(create) == [ | |||
~s|CREATE TABLE "posts" ("a" varchar(max) DEFAULT '{"foo":"bar","baz":"boom"}')| | |||
~s|CREATE TABLE "posts" ("a" varchar(max) DEFAULT '{"baz":"boom","foo":"bar"}')| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JSON library produced a different order
Started trying to upgrade this to Ecto 3. I've got 4 tests failing with these changes. Haven't attempted to run the integration tests yet.