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

Merge Arel #32097

Merged
merged 1,749 commits into from Apr 24, 2018

Conversation

Projects
None yet
@matthewd
Member

matthewd commented Feb 24, 2018

We've been bumping Arel in lockstep with AR for some time now: every Rails release requires a new Arel release, and Arel releases occur specifically for Rails releases.

We're not gaining any maintenance freedom by keeping it as a separate gem, while we are paying the costs of coordination.

Scrolling through https://rubygems.org/gems/arel/reverse_dependencies shows few notable downstreams that aren't also depending on Active Record, so this change doesn't force new dependencies onto the ecosystem at large. We'll release a new version of the arel gem that depends on the new version of activerecord, so people will still be able to smoothly upgrade: this just means they'll need AR installed (but not loaded/required).

We have no plans to rename the top level Arel constant.


Housekeeping:

Anything else?

sgrif and others added some commits Oct 25, 2014

Deprecate passing ranges to `#in` and `#not_in`
The goal of these methods should be to generate in nodes, not handle
every possible permutation of more than one value. The `#between` and
`#not_between` methods have been extracted, which better represent the
semantics of handling ranges in SQL.
Merge pull request #333 from sgrif/sg-separate-range-method
Deprecate passing ranges to `#in` and `#not_in`
Dry up building quoted nodes in predication
Given that we are going to remove casting from Arel in the near future,
having a single place nodes in predications will help.
Revert "Merge pull request #300 from jpcody/master"
This reverts commit 36836fa, reversing
changes made to 53bc842.
Revert "Revert "Merge pull request #300 from jpcody/master""
This reverts commit 9b92af7.

beta2 is out, and we've fixed the issue that this caused in Rails
Look for `Enumerable` rather than `Array` in predicates
It's not quite duck typed, but it will allow us to pass in our own
objects with additional logic (like type casting).
cache the dispatch table on the depth first visitor
We know the API for the depth first visitor in advance, so it's OK to
calcuate this cache in advance
Add order to BindParams in the ToSql collector
This removes the need for us to do the re-ordering by walking the AST in
ActiveRecord. We're using a block to communicate with the collector,
since the collector needs to be the thing which knows about the index,
while the visitor is the thing that needs to know the syntax. The
BindParam needs to know about neither of these things, so it's been
changed to stop being a subclass of SqlLiteral

I could also see an alternative implementation using format strings if
for some reason blocks cause a problem.
Use class objects rather than strings for the dispatch cache
The only reason we're using strings is to pre-populate the cache, but
`Class#name` returns a new string instance on every call. This is a
pretty major source of memory usage. We don't technically need to
pre-populate the cache, and not doing so allows us to go back to using
cache objects
Merge pull request #177 from NARKOZ/patch-0
remove extra space from select statement
Remove deprecated method "Table#primary_key"
The only place this method was still used is on the MSSQL visitor. The
visitor has all of the objects required to inline this lookup there.
Since the `primary_key` method on the connection adapter will perform a
query when called, we can cache the result on the visitor.
Remove engine from the constructor arguments `Arel::Table`
It is never used outside of convenience methods which are only used in
tests. In practice, it just made constructing tables more complicated on
the rails side. This is the minimum possible change to remove the
constructor argument, but continue to have the tests passing.

I'm not sure if we have a reason to keep `project` and friends, and the
solution might actually just be to remove the engine from
`SelectManager` and friends. As such I've held off on deleting those
methods.

We need to figure out what to do with `Table#from`. It's old invocation,
which read `table.from(table)` was certainly nonsensical.
Remove `engine` from `TreeManager` and subclasses
This constructor parameter was unused for everything except the
convenience methods `to_sql` and `where_sql`. We can pass the engine
into those methods directly.
Don't re-quote nodes which are already quoted
We're going to start working on removing type casting from arel. To
avoid doing one gigantic commit which moves everything over to eager
casting, we need a way to tell Arel that we've already cast it. The
easiest path to that is to give it a quoted node, and then we remove
this case once we're never returning a Casted node
Quoted nodes respond to the same method as Casted nodes
We need to be able to not care which we've gotten in ActiveRecord
💣
`nil?` not `nil`
{Matches,DoesNotMatch} support the ESCAPE clause with PostgreSQL
to_SQL already has supported the ESCAPE clause in #318.
PostgreSQL can use the ESCAPE clause too.

itsNikolay and others added some commits Dec 27, 2017

Reduce `Reduce`
Remove `Arel::Visitors::Reduce` because it almost completely duplicates
`Arel::Visitors::Visitor`
Remove Unused `require`
It looks like they are left from old design
Merge pull request #481 from lautis/lateral
Lateral expressions for PostgreSQL

@matthewd matthewd self-assigned this Feb 24, 2018

@composerinteralia

This comment has been minimized.

Member

composerinteralia commented Feb 24, 2018

Is 1,749 commits correct?

@matthewd

This comment has been minimized.

Member

matthewd commented Feb 24, 2018

@composerinteralia

This comment has been minimized.

Member

composerinteralia commented Feb 24, 2018

Got it. Thanks

@matthewd matthewd referenced this pull request Feb 24, 2018

Merged

Merge to Active Record #523

@pixeltrix

This comment has been minimized.

Member

pixeltrix commented Feb 24, 2018

Glad to see you haven't made the same mistake I did when integrating Journey and have changed the test file names from test_*.rb to *_test.rb. 👍

@sgrif

sgrif approved these changes Feb 24, 2018

@yahonda

This comment has been minimized.

Contributor

yahonda commented Feb 27, 2018

This commit will help me a lot because when some git bisect was necessary it was very difficult to bisect rails and gem repositories at the same time.

As a 3rd party database adapter maintainer, I'd like to know if if there is a plan to separate visitors for non bundled database adapters, such as oracle, oracle12, ibm_db, mssql and informix .

Actually, I'd like Arel will have these visitors at least for Rails 6, Just wanted to prepare if there is a plan.

Thank you.

@matthewd

This comment has been minimized.

Member

matthewd commented Feb 27, 2018

@yahonda in the short term, we'll keep the visitors in place, despite the inconsistency.

Looking forward, I'd like to rebalance some responsibilities between the Arel and AR parts, and at that time I expect we'll try to move them closer together. But no concrete plans for now.

@yahonda

This comment has been minimized.

Contributor

yahonda commented Feb 27, 2018

@matthewd Thanks for the answer and I am happy to hear there will be no change in the short term.
Of course, I'm looking forward to getting any updates for future plans.

@matthewd matthewd merged commit 989b1cb into rails:master Apr 24, 2018

2 checks passed

codeclimate All good!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

yahonda added a commit to yahonda/oracle-enhanced that referenced this pull request Apr 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment