Commit Graph

202 Commits

Author SHA1 Message Date
Carlos Antonio da Silva
683e1c7d18 Fix typo in test name 2019-08-07 13:46:15 -03:00
Ryuta Kamizono
e62195e259 Fix GROUP BY aggregation alias to not duplicate "_" chars
c9e4c848 has one performance optimization for `aggregate_alias` to early
returning by `aggregate_alias.match?(/\A\w+\z/)`, but it is caused a
regression that failing deduplication for non word chars #36867.

I've quited the optimization and add a test to prevent a future
regression.

Fixes #36867.
2019-08-07 23:05:23 +09:00
Godfrey Chan
05300be635 Add tests for selecting aggregates
These tests verifies that aggregates like `AVG` can be selected as
"virtual attributes" on Active Record models and have the correct
column type.
2019-08-01 10:56:10 -07:00
Ryuta Kamizono
dddb331bd2 Do not use aliases in GROUP BY clause
It appears that Oracle does not allow using aliases in GROUP BY clause
unlike ORDER BY clause.

Fixes #36613.
2019-07-08 08:46:07 +09:00
Ryuta Kamizono
cb0299c9eb PostgreSQL: Fix GROUP BY with ORDER BY virtual count attribute
GROUP BY with virtual count attribute is invalid for almost all
databases, but it is valid for PostgreSQL, and it had worked until Rails
5.2.2, so it is a regression for Rails 5.2.3 (caused by 311f001).

I can't find perfectly solution for fixing this for now, but I would not
like to break existing apps, so I decided to allow referencing virtual
count attribute in ORDER BY clause when GROUP BY aggrigation (it partly
revert the effect of 311f001) to fix the regression #36022.

Fixes #36022.
2019-06-17 22:14:29 +09:00
Ryuta Kamizono
0542e0608f All modern adapters returns a numeric value as the result of numeric calculation 2019-06-11 01:49:48 +09:00
Yasuo Honda
86384ea889 Address test_pluck_columns_with_same_name failure due to nondeterministic sort order
```ruby
$ bundle exec rake test_postgresql
... snip ...
Failure:
CalculationsTest#test_pluck_columns_with_same_name [/home/yahonda/git/rails/activerecord/test/cases/calculations_test.rb:842]:
--- expected
+++ actual
@@ -1 +1 @@
-[["The First Topic", "The Second Topic of the day"], ["The Third Topic of the day", "The Fourth Topic of the day"]]
+[["The Third Topic of the day", "The Fourth Topic of the day"], ["The First Topic", "The Second Topic of the day"]]
```
2019-06-02 07:09:59 +00:00
Ryuta Kamizono
95e952626d Use capture_sql instead of assert_sql with no pattern
And no longer need to except SCHEMA SQLs manually since 0810c07.
2019-05-22 09:09:18 +09:00
Ryuta Kamizono
3453c5d4c0 Give up filling schema cache before assert_no_queries
This reverts commit a1ee4a9ff9d4a3cb255365310ead0dc7b739c6be.

Even if a1ee4a9 is applied, CI is still flakiness.

https://buildkite.com/rails/rails/builds/61252#2c090afa-aa84-4a2b-8b81-9f09219222c6/994-1005
https://buildkite.com/rails/rails/builds/61252#2e55bf83-1bde-44a2-a4f1-b5c3f6820fb4/929-938

Failing tests by whether schema cache is filled or not, it actually
means that whether SCHEMA SQLs are executed or not is not target for the
tests.

So I've reverted commit a1ee4a9 which filling schema cache before
`assert_no_queries`, and replace `assert_no_queries` to
`assert_queries(0)`.
2019-05-22 01:23:58 +09:00
Ryuta Kamizono
bf1494a101 Fix GROUP BY with calculate longer name field to respect table_alias_length
Follow up of c9e4c848eeeb8999b778fa1ae52185ca5537fffe.
2019-04-08 17:46:46 +09:00
Ryuta Kamizono
060e09dfb3 Fix count(:all) with eager loading and explicit select and order
This follows up ebc09ed9ad9a04338138739226a1a92c7a2707ee.

We've still experienced a regression for `size` (`count(:all)`) with
eager loading and explicit select and order when upgrading Rails to 5.1.

In that case, the eager loading enforces `distinct` to subselect but
still keep the custom select, it would cause the ORDER BY with DISTINCT
issue.

```
% ARCONN=postgresql bundle exec ruby -w -Itest test/cases/relations_test.rb -n test_size_with_eager_loading_and_custom_select_and_order
Using postgresql
Run options: -n test_size_with_eager_loading_and_custom_select_and_order --seed 8356

# Running:

E

Error:
RelationTest#test_size_with_eager_loading_and_custom_select_and_order:
ActiveRecord::StatementInvalid: PG::InvalidColumnReference: ERROR:  for SELECT DISTINCT, ORDER BY expressions must appear in select list
LINE 1: ..." ON "comments"."post_id" = "posts"."id" ORDER BY comments.i...
                                                             ^
```

As another problem on `distinct` is enforced, the result of `count`
becomes fewer than expected if `select` is given explicitly.

e.g.

```ruby
Post.select(:type).count
# => 11

Post.select(:type).distinct.count
# => 3
```

As long as `distinct` is enforced, we need to care to keep the result of
`count`.

This fixes both the `count` with eager loading problems.
2019-04-04 16:36:49 +09:00
Ryuta Kamizono
350ad01b81 Merge pull request #35361 from jvillarejo/fix_wrong_size_query_with_distinct_select
Fix different `count` calculation when using `size` with DISTINCT `select`
2019-02-27 01:04:20 +09:00
jvillarejo
fa2c61fc63 fixes different count calculation when using size manual select with DISTINCT
When using `select` with `'DISTINCT( ... )'` if you use method `size` on a non loaded relation it overrides the column selected by passing `:all` so it returns different value than count.

This fixes #35214
2019-02-26 12:21:14 -03:00
Ryuta Kamizono
a99e00452b Remove duplicated protected params definitions
Use "support/stubs/strong_parameters" instead.
2019-02-24 19:50:53 +09:00
Ryuta Kamizono
df2765ac03 More exercise tests for distinct count with group by 2019-02-24 08:04:52 +09:00
Ryuta Kamizono
0ee96d13de Fix pluck and select with custom attributes
Currently custom attributes are always qualified by the table name in
the generated SQL wrongly even if the table doesn't have the named
column, it would cause an invalid SQL error.

Custom attributes should only be qualified if the table has the same
named column.
2019-02-13 02:47:46 +09:00
Ryuta Kamizono
31ffbf8d50 All of queries should return correct result even if including large number
Currently several queries cannot return correct result due to incorrect
`RangeError` handling.

First example:

```ruby
assert_equal true, Topic.where(id: [1, 9223372036854775808]).exists?
assert_equal true, Topic.where.not(id: 9223372036854775808).exists?
```

The first example is obviously to be true, but currently it returns
false.

Second example:

```ruby
assert_equal topics(:first), Topic.where(id: 1..9223372036854775808).find(1)
```

The second example also should return the object, but currently it
raises `RecordNotFound`.

It can be seen from the examples, the queries including large number
assuming empty result is not always correct.

Therefore, This change handles `RangeError` to generate executable SQL
instead of raising `RangeError` to users to always return correct
result. By this change, it is no longer raised `RangeError` to users.
2019-01-18 16:01:07 +09:00
Rafael Mendonça França
91ddb30083
Do not allow passing the column name to sum when a block is passed 2019-01-17 16:08:33 -05:00
Rafael Mendonça França
67356f2034
Do not allow passing the column name to count when a block is passed 2019-01-17 16:08:33 -05:00
Alberto Almagro
3f2c865731 Revert "Fix NumericData.average test on ruby 2.6"
This reverts commit 89b4612ffc97e6648f5cf807906ae210e05acdda.
2019-01-04 00:32:33 +01:00
Ryuta Kamizono
600a66f60c Fix TypeError: no implicit conversion of Arel::Attributes::Attribute into String properly
This reverts 27c6c07 since `arel_attr.to_s` is not right way to avoid
the type error.

That to_s returns `"#<struct Arel::Attributes::Attribute ...>"`, there
is no reason to match the regex to the inspect form.

And also, the regex path is not covered by our test cases. I've tweaked
the regex for redundant part and added assertions for the regex path.
2019-01-02 04:15:32 +09:00
Gannon McGibbon
93f19071ad Fix join table column quoting with SQLite. 2018-12-05 11:56:14 -05:00
Abdallah Samman
89b4612ffc Fix NumericData.average test on ruby 2.6 2018-12-03 12:59:38 +02:00
Gannon McGibbon
192b7bcfac Redact SQL in errors
Move `ActiveRecord::StatementInvalid` SQL to error property.
Also add bindings as an error property.
2018-11-22 13:53:23 -05:00
Lachlan Sylvester
13f0d6ebd3 Avoid creating temporary arrays in ActiveRecord::Result#cast_values in order to speed up pluck 2018-06-19 11:11:46 +10:00
Ryuta Kamizono
63e35a1323 Fix GROUP BY queries to apply LIMIT/OFFSET after aggregations
If `eager_loading` is true, `apply_join_dependency` force applies
LIMIT/OFFSET before JOINs by `limited_ids_for` to keep parent records
count. But for aggregation queries, LIMIT/OFFSET should be applied after
aggregations the same as SQL semantics.

And also, we could not replace SELECT list by `limited_ids_for` when a
query has a GROUP BY clause. It had never been worked since it will
causes generating invalid SQL for MySQL, PostgreSQL, and probably most
backends.

```
% ARCONN=postgresql be ruby -w -Itest test/cases/calculations_test.rb -n test_group_by_with_limit
Using postgresql
Run options: -n test_group_by_with_limit --seed 20925

# Running:

E

Error:
CalculationsTest#test_group_by_with_limit:
ActiveRecord::StatementInvalid: PG::GroupingError: ERROR:  column "posts.id" must appear in the GROUP BY clause or be used in an aggregate function
LINE 1: SELECT  DISTINCT "posts"."id", "posts"."type" AS alias_0 FRO...                         ^
: SELECT  DISTINCT "posts"."id", "posts"."type" AS alias_0 FROM "posts" LEFT OUTER JOIN "comments" ON "comments"."post_id" = "posts"."id" GROUP BY "posts"."type" ORDER BY "posts"."type" ASC LIMIT $1
```

Fixes #8103.
Closes #27249.
2018-06-07 09:58:20 +09:00
Yuji Hanamura
6f4e66fefb Add delegate :pick, to: :all 2018-03-09 16:04:56 +09:00
Ryuta Kamizono
f1878fa06e Merge pull request #32005 from maschwenk/ar-distinct-order-count-regression
Active Record distinct & order #count regression
2018-02-27 22:22:04 +09:00
Max Schwenk
5120bc90c2
Distinct with order #count regression 2018-02-25 19:54:20 -08:00
Rafael Mendonça França
4c615a53e0 Add test to make sure pick works in a NullRelation 2018-02-12 15:15:16 -05:00
David Heinemeier Hansson
80cc0d323b
Add Relation#pick as short-hand for single-value plucks (#31941)
* Add Relation#pick as short-hand for single-value plucks
2018-02-09 10:30:19 -08:00
Ryuta Kamizono
ebc09ed9ad Fix count(:all) with eager loading and having an order other than the driving table
This is a regression caused by 6beb4de.

In PostgreSQL, ORDER BY expressions must appear in SELECT list when
using DISTINCT.

When using `count(:all)` with eager loading, Active Record enforces
DISTINCT to count the driving table records only. 6beb4de was caused the
regression because `count(:all)` with DISTINCT path no longer removes
ORDER BY.

We need to ignore ORDER BY when DISTINCT is enforced, otherwise not
always generated valid SQL for PostgreSQL.

Fixes #31783.
2018-01-25 14:12:58 +09:00
Ryuta Kamizono
a194c527fa Fix pluck with eager loading to respect offset 2018-01-07 11:13:15 +09:00
Ryuta Kamizono
c6cd9a59f2 Fix count(:all) to correctly work distinct with custom SELECT list
Currently `count(:all)` with `distinct` doesn't work correctly because
SELECT list is always replaced to `*` or primary key in that case even
if having custom SELECT list.

And also, PostgreSQL has a limitation that ORDER BY expressions must
appear in select list for SELECT DISTINCT.

Therefore, we should not replace custom SELECT list when using
`count(:all)` with `distinct`.

Closes #31277.
2017-12-20 06:41:10 +09:00
Ben Toews
798557145c try using regexes 2017-11-09 22:42:15 +10:30
Ben Toews
864b16063d allow Arel.sql() for pluck 2017-11-09 22:37:23 +10:30
Ryuta Kamizono
5668dc6b18 Fix COUNT(DISTINCT ...) for GROUP BY with ORDER BY and LIMIT
This is the fix for the regression of #29848.

In #29848, I've kept existing select list in the subquery for the count
if ORDER BY is given. But it had accidentally affect to GROUP BY
queries also. It should keep the previous behavior in that case.

Fixes #30886.
2017-10-14 13:36:51 +09:00
Yasuo Honda
cb2934ed79 sqlite3 adapter returns integer value which used to be string
`to_i` was added for SQLite3 adapter which did not handle number
but sqlite3 gem already supports it then `to_i` is unnecessary.

else condition is kept for adapters which return string,
i.e. mysql(not mysql2) and sqlserver.

Renamed `test_cache_does_not_wrap_string_results_in_arrays`
to `test_cache_does_not_wrap_results_in_arrays` to explain the
current behavior. most of adapters return integer, not only string.

* Refer these commits:

"future proofing the sqlite3 adapter code"
beda2d43d6

"Refactor calculation test to remove unneeded SQLite special case."
47d568ed3f

"no need to to_i, sqlite does that for us"
6cf44a1bd6
2017-09-01 15:57:02 +00:00
Rafael Mendonça França
feb1ddae02 Merge remote-tracking branch 'origin/master' into unlock-minitest 2017-08-01 17:34:14 -04:00
Ryuta Kamizono
0df0dfbfac Should keep the table name qualified * for distinct subquery 2017-07-22 08:40:16 +09:00
Ryuta Kamizono
a265d4b29c Fix COUNT(DISTINCT ...) with ORDER BY and LIMIT
Since #26972, `ORDER BY` is kept if `LIMIT` is presented for
performance. But in most SQL servers (e.g. PostgreSQL, SQL Server, etc),
`ORDER BY` expressions must appear in select list for `SELECT DISTINCT`.
We should not replace existing select list in that case.
2017-07-22 08:40:16 +09:00
Kir Shatrov
831be98f9a Use frozen-string-literal in ActiveRecord 2017-07-19 22:27:07 +03:00
Kasper Timm Hansen
aad42dce10 Merge branch 'master' into unlock-minitest 2017-07-15 21:17:27 +02:00
Eugene Kenny
6658e3746b Skip query cache for in_batches and friends
The `find_each`, `find_in_batches` and `in_batches` APIs usually operate
on large numbers of records, where it's preferable not to load them all
into memory at once.

If the query cache is enabled, it will hold onto the query results until
the end of the execution context (request/job), which means the memory
used is still proportional to the total number of records. These queries
are typically not repeated, so the query cache isn't desirable here.
2017-07-06 14:21:07 +01:00
Matthew Draper
87b3e226d6 Revert "Merge pull request #29540 from kirs/rubocop-frozen-string"
This reverts commit 3420a14590c0e6915d8b6c242887f74adb4120f9, reversing
changes made to afb66a5a598ce4ac74ad84b125a5abf046dcf5aa.
2017-07-02 02:15:17 +09:30
Kir Shatrov
cfade1ec7e Enforce frozen string in Rubocop 2017-07-01 02:11:03 +03:00
Koichi ITO
b8dea23c4a Fix test_pluck_without_column_names when using Oracle 2017-06-06 19:09:26 +09:00
yuuji.yaginuma
c40502f9f0 Load schema before assertion
Without this, test fails because the load schema when pluck is executed.

Steps to reproduce:

```
bin/test -a postgresql -w --seed 61689 test/cases/*test.rb -n "/^(?:InheritanceComputeTypeTest#(?:test_inheritance_new_with_subclass_as_default)|CalculationsTest#(?:test_pluck_loaded_relation))$/"

# Running:

.F

Failure:
CalculationsTest#test_pluck_loaded_relation [/home/yaginuma/program/rails/master_y_yagi/rails/activerecord/test/cases/calculations_test.rb:722]:
1 instead of 0 queries were executed.
Queries:
SELECT c.relname FROM pg_class c LEFT JOIN pg_namespace n ON n.oid = c.relnamespace WHERE n.nspname = ANY (current_schemas(false)) AND c.relname = 'companies' AND c.relkind IN ('r','v','m').
Expected: 0
  Actual: 1

bin/test test/cases/calculations_test.rb:7
```
2017-06-06 07:26:55 +09:00
Rafael França
908b136cf2 Merge pull request #26634 from kamipo/extract_numeric_data
Extract `NumericData` model for tests
2017-05-31 15:50:08 -04:00
Ryuta Kamizono
36417cf077 Deprecate passing arguments and block at the same time to count and sum in ActiveRecord::Calculations
`select`, `count`, and `sum` in `Relation` are also `Enumerable` method
that can be passed block. `select` with block already doesn't take
arguments since 4fc3366. This is follow up of that.
2017-05-29 09:02:45 +09:00