Permalink
Browse files

Minimal change to query generation of exists? that makes SQLServer an…

…d others happy that do not work without a column alias.

Conflicts:
	activerecord/lib/active_record/relation/finder_methods.rb
  • Loading branch information...
iaddict authored and rafaelfranca committed May 19, 2011
1 parent 4da1af7 commit be4ecdcc87984e9421ff5d5c90d33f475e0fbc01
Showing with 1 addition and 1 deletion.
  1. +1 −1 activerecord/lib/active_record/relation/finder_methods.rb
@@ -176,7 +176,7 @@ def exists?(id = false)
join_dependency = construct_join_dependency_for_association_find
relation = construct_relation_for_association_find(join_dependency)
- relation = relation.except(:select, :order).select("1").limit(1)
+ relation = relation.except(:select, :order).select("1 AS _one").limit(1)
case id
when Array, Hash

9 comments on commit be4ecdc

@yahonda

This comment has been minimized.

Show comment Hide comment
@yahonda

yahonda Jun 11, 2012

Contributor

This commit causes 72 failures with Oracle database. The _ underscore at the beginning of _one causes it.
I'm not familiar with SQL Server though.

One of these errors

  1) Error:
test_dont_add_if_before_callback_raises_exception(AssociationCallbacksTest):
ActiveRecord::StatementInvalid: OCIError: ORA-00911: invalid character: SELECT  1 AS _one FROM "POSTS"  WHERE "POSTS"."AUTHOR_ID" = 1 AND "POSTS"."ID" = 3 AND ROWNUM <= 1
    stmt.c:253:in oci8lib_191.so
Contributor

yahonda replied Jun 11, 2012

This commit causes 72 failures with Oracle database. The _ underscore at the beginning of _one causes it.
I'm not familiar with SQL Server though.

One of these errors

  1) Error:
test_dont_add_if_before_callback_raises_exception(AssociationCallbacksTest):
ActiveRecord::StatementInvalid: OCIError: ORA-00911: invalid character: SELECT  1 AS _one FROM "POSTS"  WHERE "POSTS"."AUTHOR_ID" = 1 AND "POSTS"."ID" = 3 AND ROWNUM <= 1
    stmt.c:253:in oci8lib_191.so
@rafaelfranca

This comment has been minimized.

Show comment Hide comment
@rafaelfranca

rafaelfranca Jun 11, 2012

Owner

💔

@yahonda could you test if changing from _one to one will work? If it works please open a pull request.

Owner

rafaelfranca replied Jun 11, 2012

💔

@yahonda could you test if changing from _one to one will work? If it works please open a pull request.

@yahonda

This comment has been minimized.

Show comment Hide comment
@yahonda

yahonda Jun 11, 2012

Contributor

Replacing _one to one works except for test_exists_does_not_select_columns_without_alias, which is expected.
It also fails with postgresql, mysql, mysql2 and sqlite3 adapters.

$ ARCONN=oracle ruby -Itest test/cases/finder_test.rb -n test_exists_does_not_select_columns_without_aliasUsing oracle with Identity Map off
Run options: -n test_exists_does_not_select_columns_without_alias

# Running tests:

F

Finished tests in 0.963351s, 1.0380 tests/s, 1.0380 assertions/s.

  1) Failure:
test_exists_does_not_select_columns_without_alias(FinderTest) [test/cases/finder_test.rb:49]:
Query pattern(s) /SELECT\W+1 AS _one FROM ["`]topics["`]\W+LIMIT 1/ not found.
Queries:
SELECT  1 AS one FROM "TOPICS"  WHERE ROWNUM <= 1

1 tests, 1 assertions, 1 failures, 0 errors, 0 skips

I'd like to know there should be some reasons putting _ in front of one..

Contributor

yahonda replied Jun 11, 2012

Replacing _one to one works except for test_exists_does_not_select_columns_without_alias, which is expected.
It also fails with postgresql, mysql, mysql2 and sqlite3 adapters.

$ ARCONN=oracle ruby -Itest test/cases/finder_test.rb -n test_exists_does_not_select_columns_without_aliasUsing oracle with Identity Map off
Run options: -n test_exists_does_not_select_columns_without_alias

# Running tests:

F

Finished tests in 0.963351s, 1.0380 tests/s, 1.0380 assertions/s.

  1) Failure:
test_exists_does_not_select_columns_without_alias(FinderTest) [test/cases/finder_test.rb:49]:
Query pattern(s) /SELECT\W+1 AS _one FROM ["`]topics["`]\W+LIMIT 1/ not found.
Queries:
SELECT  1 AS one FROM "TOPICS"  WHERE ROWNUM <= 1

1 tests, 1 assertions, 1 failures, 0 errors, 0 skips

I'd like to know there should be some reasons putting _ in front of one..

@rafaelfranca

This comment has been minimized.

Show comment Hide comment
@rafaelfranca

rafaelfranca Jun 11, 2012

Owner

There is no reason to, we can use any name, since it doesn't brake another adapters. You can change from _one to one in the method and in the test too.

Owner

rafaelfranca replied Jun 11, 2012

There is no reason to, we can use any name, since it doesn't brake another adapters. You can change from _one to one in the method and in the test too.

@rafaelfranca

This comment has been minimized.

Show comment Hide comment
@rafaelfranca

rafaelfranca Jun 11, 2012

Owner

Also you can change the assert_query call to:

assert_query(/SELECT\W+1 AS one FROM ["`]topics["`]/i) do
Owner

rafaelfranca replied Jun 11, 2012

Also you can change the assert_query call to:

assert_query(/SELECT\W+1 AS one FROM ["`]topics["`]/i) do
@yahonda

This comment has been minimized.

Show comment Hide comment
@yahonda

yahonda Jun 11, 2012

Contributor

Thanks. I'm going to open separate pull requests for master and 3-2 stable branches. It would include some test changes because Oracle database does not support LIMIT statement, using ROWNUM instead.

Contributor

yahonda replied Jun 11, 2012

Thanks. I'm going to open separate pull requests for master and 3-2 stable branches. It would include some test changes because Oracle database does not support LIMIT statement, using ROWNUM instead.

@yahonda

This comment has been minimized.

Show comment Hide comment
@yahonda

yahonda Jun 11, 2012

Contributor

Opened a pull request for the master branch. See #6698.

Contributor

yahonda replied Jun 11, 2012

Opened a pull request for the master branch. See #6698.

@rafaelfranca

This comment has been minimized.

Show comment Hide comment
@rafaelfranca

rafaelfranca Jun 11, 2012

Owner

Applied at master and 3-2-stable.

Owner

rafaelfranca replied Jun 11, 2012

Applied at master and 3-2-stable.

@yahonda

This comment has been minimized.

Show comment Hide comment
@yahonda

yahonda Jun 11, 2012

Contributor

Thanks!

Contributor

yahonda replied Jun 11, 2012

Thanks!

Please sign in to comment.