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.

Sign up for GitHub

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

Jump to bottom

Merge Arel visitors #2002

Merged
merged 13 commits into from
Apr 15, 2020
Merged

Merge Arel visitors #2002

merged 13 commits into from
Apr 15, 2020

Commits on Apr 15, 2020

  1. Merge Arel into Active Record

    @matthewd @koic
    matthewd authored and koic committed Apr 15, 2020
    Configuration menu
    Copy the full SHA
    be42e8c View commit details
    Browse the repository at this point in the history

  2. Arel: rubocop -a

    @matthewd @koic
    matthewd authored and koic committed Apr 15, 2020
    Configuration menu
    Copy the full SHA
    1029090 View commit details
    Browse the repository at this point in the history

  3. Arel: :nodoc: all

    @matthewd @koic
    matthewd authored and koic committed Apr 15, 2020
    Configuration menu
    Copy the full SHA
    db3ae64 View commit details
    Browse the repository at this point in the history

  4. Arel: Implemented DB-aware NULL-safe comparison (#34451) 

    * Arel: Implemented DB-aware NULL-safe comparison

* Fixed where clause inversion for NULL-safe comparison

* Renaming "null_safe_eq" to "is_not_distinct_from", "null_safe_not_eq" to "is_distinct_from"

[Dmytro Shteflyuk + Rafael Mendonça França]
    @kpumuk @koic
    kpumuk authored and koic committed Apr 15, 2020
    Configuration menu
    Copy the full SHA
    e692334 View commit details
    Browse the repository at this point in the history

  5. Enable Lint/UselessAssignment cop to avoid unused variable warnings… 

    … (#34904)

* Enable `Lint/UselessAssignment` cop to avoid unused variable warnings

Since we've addressed the warning "assigned but unused variable"
frequently.

370537de05092aeea552146b42042833212a1acc
3040446cece8e7a6d9e29219e636e13f180a1e03
5ed618e192e9788094bd92c51255dda1c4fd0eae
76ebafe594fc23abc3764acc7a3758ca473799e5

And also, I've found the unused args in c1b14ad which raises no warnings
by the cop, it shows the value of the cop.
    @kamipo @koic
    kamipo authored and koic committed Apr 15, 2020
    Configuration menu
    Copy the full SHA
    590972e View commit details
    Browse the repository at this point in the history

  6. Address ORA-01795: maximum number of expressions in a list is 1000 

    * To address this error, this commit splits expressions by slices of 1000 elements.

* "Oracle Database Error Messages 18c"
https://docs.oracle.com/en/database/oracle/oracle-database/18/errmg/

```
ORA-01795: maximum number of expressions in a list is 1000
Cause: Number of expressions in the query exceeded than 1000. Note that unused column/expressions are also counted Maximum number of expressions that are allowed are 1000.
```

* This commit addresses this ORA-01795 error

Note: Actually addressing this error raises another "ORA-00913: too many values"
Number of values Oracle database allows is 65535 regardless bind values or literal values.

```ruby
$ ARCONN=oracle bin/test test/cases/bind_parameter_test.rb -n test_too_many_binds

... snip ...

Error:
ActiveRecord::BindParameterTest#test_too_many_binds:
ActiveRecord::StatementInvalid: OCIError: ORA-01795: maximum number of expressions in a list is 1000
    stmt.c:267:in oci8lib_260.so
    /home/yahonda/.rbenv/versions/2.6.2/lib/ruby/gems/2.6.0/gems/ruby-oci8-2.2.7/lib/oci8/cursor.rb:131:in `exec'
    /home/yahonda/git/oracle-enhanced/lib/active_record/connection_adapters/oracle_enhanced/oci_connection.rb:142:in `exec'
    /home/yahonda/git/oracle-enhanced/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb:41:in `block in exec_query'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb:676:in `block (2 levels) in log'
    /home/yahonda/.rbenv/versions/2.6.2/lib/ruby/2.6.0/monitor.rb:230:in `mon_synchronize'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb:675:in `block in log'
    /home/yahonda/git/rails/activesupport/lib/active_support/notifications/instrumenter.rb:24:in `instrument'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb:666:in `log'
    /home/yahonda/git/oracle-enhanced/lib/active_record/connection_adapters/oracle_enhanced/dbms_output.rb:36:in `log'
    /home/yahonda/git/oracle-enhanced/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb:24:in `exec_query'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:484:in `select'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:70:in `select_all'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb:106:in `select_all'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation/calculations.rb:299:in `block in execute_simple_calculation'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation.rb:755:in `skip_query_cache_if_necessary'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation/calculations.rb:299:in `execute_simple_calculation'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation/calculations.rb:251:in `perform_calculation'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation/calculations.rb:141:in `calculate'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation/calculations.rb:49:in `count'
    /home/yahonda/git/rails/activerecord/test/cases/bind_parameter_test.rb:113:in `test_too_many_binds'

bin/test test/cases/bind_parameter_test.rb:109

.............................F

```
    @yahonda @koic
    yahonda authored and koic committed Apr 15, 2020
    Configuration menu
    Copy the full SHA
    041995a View commit details
    Browse the repository at this point in the history

  7. Fix sliced IN clauses to be grouped 

    Follow up of #35838.

And also this refactors `in_clause_length` handling is entirely
integrated in Arel visitor.
    @kamipo @koic
    kamipo authored and koic committed Apr 15, 2020
    Configuration menu
    Copy the full SHA
    8666547 View commit details
    Browse the repository at this point in the history

  8. Enable Layout/EmptyLinesAroundAccessModifier cop 

    We sometimes say "✂️ newline after `private`" in a code review (e.g.
rails/rails#18546 (comment),
rails/rails#34832 (comment)).

Now `Layout/EmptyLinesAroundAccessModifier` cop have new enforced style
`EnforcedStyle: only_before` (rubocop/rubocop#7059).

That cop and enforced style will reduce the our code review cost.
    @kamipo @koic
    kamipo authored and koic committed Apr 15, 2020
    Configuration menu
    Copy the full SHA
    4a96ba3 View commit details
    Browse the repository at this point in the history

  9. Use match? where we don't need MatchData 

    We're already running Performance/RegexpMatch cop, but it seems like the cop is not always =~ justice
    @amatsuda @koic
    amatsuda authored and koic committed Apr 15, 2020
    Configuration menu
    Copy the full SHA
    053e83a View commit details
    Browse the repository at this point in the history

  10. Address to rubocop offences

    @kamipo @koic
    kamipo authored and koic committed Apr 15, 2020
    Configuration menu
    Copy the full SHA
    8ab91ce View commit details
    Browse the repository at this point in the history

  11. Use Regext#match? where MatchData is not needed

    @amatsuda @koic
    amatsuda authored and koic committed Apr 15, 2020
    Configuration menu
    Copy the full SHA
    61becb6 View commit details
    Browse the repository at this point in the history

  12. Address `no implicit conversion of Arel::Attributes::Attribute into S… 

    …tring`

rails/rails@4a9ef5e triggers this failure,
then restored the original code for `activerecord/lib/arel/visitors/oracle.rb` .

```
$ bundle exec rspec ./spec/active_record/connection_adapters/oracle_enhanced/context_index_spec.rb:92
==> Loading config from ENV or use default
==> Running specs with MRI version 2.6.5
==> Effective ActiveRecord version 6.1.0.alpha
... snip ...
Run options: include {:locations=>{"./spec/active_record/connection_adapters/oracle_enhanced/context_index_spec.rb"=>[92]}}
F

Failures:

  1) OracleEnhancedAdapter context index on single table should create single VARCHAR2 column index
     Failure/Error: expect(Post.contains(:title, word).to_a).to eq([@POST2, @post1])

     TypeError:
       no implicit conversion of Arel::Attributes::Attribute into String
     # /home/yahonda/git/rails/activerecord/lib/arel/visitors/oracle.rb:107:in `match?'
     # /home/yahonda/git/rails/activerecord/lib/arel/visitors/oracle.rb:107:in `block (2 levels) in order_hacks'
     # /home/yahonda/git/rails/activerecord/lib/arel/visitors/oracle.rb:106:in `any?'
     # /home/yahonda/git/rails/activerecord/lib/arel/visitors/oracle.rb:106:in `block in order_hacks'
     # /home/yahonda/git/rails/activerecord/lib/arel/visitors/oracle.rb:105:in `any?'
     # /home/yahonda/git/rails/activerecord/lib/arel/visitors/oracle.rb:105:in `order_hacks'
     # /home/yahonda/git/rails/activerecord/lib/arel/visitors/oracle.rb:8:in `visit_Arel_Nodes_SelectStatement'
     # /home/yahonda/git/rails/activerecord/lib/arel/visitors/visitor.rb:30:in `visit'
     # /home/yahonda/git/rails/activerecord/lib/arel/visitors/visitor.rb:11:in `accept'
     # /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/determine_if_preparable_visitor.rb:10:in `accept'
     # /home/yahonda/git/rails/activerecord/lib/arel/visitors/to_sql.rb:18:in `compile'
     # /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:25:in `to_sql_and_binds'
     # /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:63:in `select_all'
     # /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb:107:in `select_all'
     # /home/yahonda/git/rails/activerecord/lib/active_record/querying.rb:46:in `find_by_sql'
     # /home/yahonda/git/rails/activerecord/lib/active_record/relation.rb:826:in `block in exec_queries'
     # /home/yahonda/git/rails/activerecord/lib/active_record/relation.rb:844:in `skip_query_cache_if_necessary'
     # /home/yahonda/git/rails/activerecord/lib/active_record/relation.rb:811:in `exec_queries'
     # /home/yahonda/git/rails/activerecord/lib/active_record/relation.rb:629:in `load'
     # /home/yahonda/git/rails/activerecord/lib/active_record/relation.rb:250:in `records'
     # /home/yahonda/git/rails/activerecord/lib/active_record/relation.rb:245:in `to_ary'
     # ./spec/active_record/connection_adapters/oracle_enhanced/context_index_spec.rb:95:in `block (4 levels) in <top (required)>'
     ... snip ...
```
    @yahonda @koic
    yahonda authored and koic committed Apr 15, 2020
    Configuration menu
    Copy the full SHA
    74edc91 View commit details
    Browse the repository at this point in the history

  13. Require Arel visitors

    @koic
    koic committed Apr 15, 2020
    Configuration menu
    Copy the full SHA
    af14bb0 View commit details
    Browse the repository at this point in the history