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

Oracle visitor gets undefined method `to_i' for #<Arel::Nodes::BindParam:0x00000002c92910> #438

Closed
yahonda opened this issue Jun 1, 2016 · 6 comments · Fixed by #450
Closed

Comments

@yahonda
Copy link
Member

yahonda commented Jun 1, 2016

The original issue has been reported rsim/oracle-enhanced#848

I need help from Arel developers then opened another issue here.

  • Summary

This Rails application code should generates the sql statement below, but actually it gets undefined method `to_i' for #Arel::Nodes::BindParam:0x00000002c92910

  • Rails code
@employee.order(:sort_order).offset(0).first.first_name
  • Expected sql statement
SELECT * FROM (
  SELECT raw_sql_.*, rownum raw_rnum_
  FROM (SELECT  "TEST_EMPLOYEES".* FROM "TEST_EMPLOYEES"  ORDER BY "TEST_EMPLOYEES"."SORT_ORDER" ASC ) raw_sql_
  WHERE rownum <= 1
)
WHERE raw_rnum_ > 0

Taking a look at Arel code and existing test case which works fine, but somehow offset.expr.to_i does not work anymore.

  • lib/arel/visitors/oracle.rb
          collector << ") raw_sql_
                WHERE rownum <= #{offset.expr.to_i + limit}
              )
              WHERE "
          return visit(offset, collector)
        end
  • Existing test case
          it 'creates a different subquery when there is an offset' do
            stmt = Nodes::SelectStatement.new
            stmt.limit = Nodes::Limit.new(10)
            stmt.offset = Nodes::Offset.new(10)
            sql = compile stmt
            sql.must_be_like %{
              SELECT * FROM (
                SELECT raw_sql_.*, rownum raw_rnum_
                FROM (SELECT ) raw_sql_
                 WHERE rownum <= 20
              )
              WHERE raw_rnum_ > 10
            }
          end
  • Environment
* Rails 5.0.0.rc1
* Arel 7.0.0
* Oracle enhanced adapter `rails5` branch
* Oracle Database 11g

This issue does not reproduce with Oracle 12c database since it uses another visitor named Oracle12.

@yahonda
Copy link
Member Author

yahonda commented Jul 23, 2016

Here is a test case to reproduce. https://gist.github.com/yahonda/f4bcb0fd1701c4620972eb5b38bd4498

It needs Oracle database , here is a step to create rails-dev-box running Oracle database.

git clone git@github.com:yahonda/rails-dev-box.git
cd rails-dev-box
git checkout runs_oracle
cp /path/to/oracle-xe-11.2.0-1.0.x86_64.rpm.zip puppet/modules/oracle/files/.
vagrant up
vagrant ssh
  • At Vagrant guest
$ ruby rep.rb

@elgreco
Copy link

elgreco commented Sep 21, 2016

+1 please fix

@rootatdarkstar
Copy link

rootatdarkstar commented Sep 29, 2016

It's seems it's happen because of this commit:
rails/rails@574f255#diff-bf6dd6226db3aab589916f09236881c7R959

Since commit above Arel have no access to limit and offset values, now it's just BindParam without any value, so oracle visitor can't calculate rawnum value here - https://github.com/rails/arel/blob/master/lib/arel/visitors/oracle.rb#L30

When i tried to solve this problem i doesn't found a way to calculate rawnum in SQL with liimit binding.
I can generate this statement with this hackish code:

          SELECT * FROM (
            SELECT raw_sql_.*, rownum raw_rnum_
            FROM (
                  .....
            ) raw_sql_ WHERE rownum <= (:a1 + :a2)
          )
          WHERE raw_rnum_ > :a2

But i can't bind one variable to two same placeholder, because oracle_enhanced uses OCI bind_params, that binds variables by number, not by name and last :a2 placeholder is empty in SQL query.

@roooodcastro
Copy link

roooodcastro commented Oct 6, 2016

@rootatdarkstar did you get your fix to work? I'm running into the exact same problem here, with this setup:

  • Rails 5.0.0.1
  • Arel 7.1.2
  • Oracle enhanced adapter 1.7.3
  • Oracle Database 10g
  • Using pagination with Kaminari

Edit:

I tried your "solution", and got this query generated:

...start_of_query_using_:a1_and_:a2) raw_sql_
                     WHERE rownum <= (:a3 + :a4)
                 )
                 WHERE raw_rnum_ > :a4

And the values bound to these parameters seem to add up, considering this is SQL for the first page of a query limited to 10 records each.

[["ano", "2016"], ["idcurso", 48], ["OFFSET", 0], ["LIMIT", 10]].

However, that didn't return any results, and it's because your placeholder is in the wrong parameter. It should be using the offset instead of limit for the raw_rnum_, so the row num is between offset and offset + limit. I switched this and my query worked as intended.

This is my hacked oracle.rb that seems to be working, (at least for me).

@roooodcastro
Copy link

I feel like my solution fixes this issue, but I still need to run the test suite and actually test it. I could create a PR for this, but I feel this code isn't very good and that there's a better solution somewhere. What do you guys think?

@yahonda
Copy link
Member Author

yahonda commented Oct 6, 2016

Thanks for finding a fix. It would be appreciated if you to open a pull request even if it is not "best" solution. Then arel committers can review it and it can be merged.

roooodcastro added a commit to roooodcastro/arel that referenced this issue Oct 7, 2016
yahonda added a commit to yahonda/oracle-enhanced that referenced this issue Oct 11, 2016
rails/arel@8813ad8

Also Refer
rails/arel#450
rails/arel#438
rsim#848

This fix itself has been available since Arel 7.1.3
but the latest version of Arel is 7.1.4.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants