Skip to content

SQLite Update to get it closer to Postgres with ecto. #101

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

Merged
merged 18 commits into from
Apr 12, 2023

Conversation

jeregrine
Copy link
Contributor

@jeregrine jeregrine commented Jan 23, 2023

I'm bringing the discussion here, changes so far and work todo

  • DISTINCT is supported, just not column wise, need to check that distinct is called like MYSQL with distinct: true only
    • remove order by code I added
  • Converted JSON types to TEXT
  • Added Proper casing to REALS
  • Adding parens around sub commands for consistency with Postgres. (NOT union/except/intersect)
  • STARTED adding actual errors when users try to use some unsupported features. This is incomplete.
    • DROP Index
    • Unsupported column type
    • Joins not supported with CTE
    • LOCKs not supported in general, CTE's as well
    • UPDATE with joins
    • ALTER table stuff has lots of unsupported behaviors
  • Finish the "in" support tests
  • Check I didn't miss any tests from the OLD connection test.
  • Check the MYSQL tests to see if I have any others there that are relevant.

@warmwaffles
Copy link
Member

Exciting.

@jeregrine jeregrine marked this pull request as ready for review January 25, 2023 16:14
@warmwaffles
Copy link
Member

@jeregrine how's this coming along?

query: query,
message: "join hints are not supported by SQLite3"
end

Copy link
Contributor

Choose a reason for hiding this comment

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

Should Enum.map(hints, &[?\s | &1]), be removed from the iolist below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch!

@jeregrine jeregrine force-pushed the js/pg-connection-test branch from 96abca9 to f2a61ed Compare March 1, 2023 16:46
@jeregrine
Copy link
Contributor Author

@warmwaffles I fixed the merge conflicts but integration tests will continue to fail until a new ecto version is cut with this in it elixir-ecto/ecto#4083

@warmwaffles
Copy link
Member

Ah no problem then. Let's wait for them to cut a new version. There shouldn't be too many merge conflicts until then.

@fredwu fredwu mentioned this pull request Apr 12, 2023
@jeregrine
Copy link
Contributor Author

Are we ready to go? Or should we merge #107 instead?

@warmwaffles
Copy link
Member

I want to give you all the credit for the work you did here. When I squash merge #107, it will consolidate under @fredwu's name.

What I can do is just bump to ecto 3.10, then you rebase this on top. Then @fredwu rebase his work after we merge your work @jeregrine.

How does that sound?

@warmwaffles
Copy link
Member

Actually @jeregrine let me just squash merge this and @fredwu can just rebase on top.

@warmwaffles warmwaffles merged commit 6f1ca67 into elixir-sqlite:main Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants