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

Revert fixes involving issue #99 (which makes Arel unusable in large datasets) #174

Merged
merged 2 commits into from Mar 15, 2013
Merged

Conversation

eduardordm
Copy link

There are two commits involving issue #99 which should be reverted.

First, #99 is not an issue in Arel at all. Second, the fix provided pretty much destroys the purpose of pagination by causing full table scans.

The original code (it seems I can't simply revert the commits) is 900 times slower than this one.

/cc @tenderlove @rsim

With love,
Eduardo

Explain plans (txt)

SELECT * FROM (
   SELECT raw_sql_.*, rownum raw_rnum_
   FROM (SELECT "LANCAMENTOS".* FROM "LANCAMENTOS" ) raw_sql_
 )
 WHERE raw_rnum_ between 1 and 30


----------------------------------------------------------------------------------
| Id  | Operation           | Name        | Rows  | Bytes | Cost (%CPU)| Time     |
-----------------------------------------------------------------------------------
|   0 | SELECT STATEMENT    |             |  4636K|  2701M| 23442   (2)| 00:04:42 |
|*  1 |  VIEW               |             |  4636K|  2701M| 23442   (2)| 00:04:42 |
|   2 |   COUNT             |             |       |       |            |          |
|   3 |    TABLE ACCESS FULL| LANCAMENTOS |  4636K|   738M| 23442   (2)| 00:04:42 |
-----------------------------------------------------------------------------------

Predicate Information (identified by operation id):
---------------------------------------------------

   1 - filter("RAW_RNUM_"<=30 AND "RAW_RNUM_">=1)

   Statistics
-----------------------------------------------------------
               4  user calls
              13  physical read total multi block requests
       202588160  physical read total bytes
       202588160  cell physical IO interconnect bytes
               0  commit cleanout failures: block lost
               0  IMU commits
               0  IMU Flushes
               0  IMU contention
               0  IMU bind flushes
               0  IMU mbu flush






SELECT * FROM (
   SELECT raw_sql_.*, rownum raw_rnum_
   FROM (SELECT "LANCAMENTOS".* FROM "LANCAMENTOS" ) raw_sql_
   WHERE rownum <= 30
 )
 WHERE raw_rnum_ >= 0


-----------------------------------------------------------------------------------
| Id  | Operation           | Name        | Rows  | Bytes | Cost (%CPU)| Time     |
-----------------------------------------------------------------------------------
|   0 | SELECT STATEMENT    |             |    30 | 18330 |     2   (0)| 00:00:01 |
|*  1 |  VIEW               |             |    30 | 18330 |     2   (0)| 00:00:01 |
|*  2 |   COUNT STOPKEY     |             |       |       |            |          |
|   3 |    TABLE ACCESS FULL| LANCAMENTOS |    30 |  5010 |     2   (0)| 00:00:01 |
-----------------------------------------------------------------------------------

Predicate Information (identified by operation id):
---------------------------------------------------

   1 - filter("RAW_RNUM_">=0)
   2 - filter(ROWNUM<=30)

   Statistics
-----------------------------------------------------------
               4  user calls
               0  physical read total multi block requests
               0  physical read total bytes
               0  cell physical IO interconnect bytes
               0  commit cleanout failures: block lost
               0  IMU commits
               0  IMU Flushes
               0  IMU contention
               0  IMU bind flushes
               0  IMU mbu flush

@eduardordm
Copy link
Author

I'll fix the test (it expects the old sql statement)

@rafaelfranca
Copy link
Member

Interesting. Which arel version are you using? I think we will need to backport to the proper version.

@rafaelfranca
Copy link
Member

Also can I ask to add the content of the gist in the commit message? Just in case where you need to delete the gist or github choose to change the gist url again

@eduardordm
Copy link
Author

Ok 😄 (I fixed on master but the 3-0-stable branch is broken too)

@rafaelfranca
Copy link
Member

The commit message is still pointing to the gist. You can change it using rebase -i

There are two commits involving issue #99 which should be reverted.

First of all, #99 is not an issue in Arel at all. Second, the fix provides pretty much destroys the purpose of pagination by cause full table scans.

The original code (it seems I can't simply revert the commits) is 900 times slower than this one.

```
SELECT * FROM (
   SELECT raw_sql_.*, rownum raw_rnum_
   FROM (SELECT "LANCAMENTOS".* FROM "LANCAMENTOS" ) raw_sql_
 )
 WHERE raw_rnum_ between 1 and 30

----------------------------------------------------------------------------------
| Id  | Operation           | Name        | Rows  | Bytes | Cost (%CPU)| Time     |
-----------------------------------------------------------------------------------
|   0 | SELECT STATEMENT    |             |  4636K|  2701M| 23442   (2)| 00:04:42 |
|*  1 |  VIEW               |             |  4636K|  2701M| 23442   (2)| 00:04:42 |
|   2 |   COUNT             |             |       |       |            |          |
|   3 |    TABLE ACCESS FULL| LANCAMENTOS |  4636K|   738M| 23442   (2)| 00:04:42 |
-----------------------------------------------------------------------------------

Predicate Information (identified by operation id):
---------------------------------------------------

   1 - filter("RAW_RNUM_"<=30 AND "RAW_RNUM_">=1)

   Statistics
-----------------------------------------------------------
               4  user calls
              13  physical read total multi block requests
       202588160  physical read total bytes
       202588160  cell physical IO interconnect bytes
               0  commit cleanout failures: block lost
               0  IMU commits
               0  IMU Flushes
               0  IMU contention
               0  IMU bind flushes
               0  IMU mbu flush

SELECT * FROM (
   SELECT raw_sql_.*, rownum raw_rnum_
   FROM (SELECT "LANCAMENTOS".* FROM "LANCAMENTOS" ) raw_sql_
   WHERE rownum <= 30
 )
 WHERE raw_rnum_ >= 0

-----------------------------------------------------------------------------------
| Id  | Operation           | Name        | Rows  | Bytes | Cost (%CPU)| Time     |
-----------------------------------------------------------------------------------
|   0 | SELECT STATEMENT    |             |    30 | 18330 |     2   (0)| 00:00:01 |
|*  1 |  VIEW               |             |    30 | 18330 |     2   (0)| 00:00:01 |
|*  2 |   COUNT STOPKEY     |             |       |       |            |          |
|   3 |    TABLE ACCESS FULL| LANCAMENTOS |    30 |  5010 |     2   (0)| 00:00:01 |
-----------------------------------------------------------------------------------

Predicate Information (identified by operation id):
---------------------------------------------------

   1 - filter("RAW_RNUM_">=0)
   2 - filter(ROWNUM<=30)

   Statistics
-----------------------------------------------------------
               4  user calls
               0  physical read total multi block requests
               0  physical read total bytes
               0  cell physical IO interconnect bytes
               0  commit cleanout failures: block lost
               0  IMU commits
               0  IMU Flushes
               0  IMU contention
               0  IMU bind flushes
               0  IMU mbu flush
```
@eduardordm
Copy link
Author

Sorry, should be fixed now. I used the web editor and messed my local repo. 🐴

rafaelfranca added a commit that referenced this pull request Mar 15, 2013
Revert fixes involving issue #99 (which makes Arel unusable in large datasets)
@rafaelfranca rafaelfranca merged commit 933ea21 into rails:master Mar 15, 2013
@rafaelfranca
Copy link
Member

Thank you

rafaelfranca added a commit that referenced this pull request Mar 15, 2013
Revert fixes involving issue #99 (which makes Arel unusable in large datasets)
Conflicts:
	lib/arel/visitors/oracle.rb
	test/visitors/test_oracle.rb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants