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

Allow symbol (i.e. quoted identifier) as safe SQL string #36350

Merged
merged 1 commit into from May 28, 2019

Conversation

kamipo
Copy link
Member

@kamipo kamipo commented May 28, 2019

pluck(:id) / order(:id) are very common use case, and passed symbol
(i.e. quoted identifier) is obviously safe argument, but
:id.to_s.split(/\s*,\s*/).all? { |part| permit.match?(part) } is
useless and a bit expensive operation for each such safe symbols (will
make extra 2 mutable strings, 1 array, 1 proc).

This avoids the expensive operation to such safe symbols, it makes
pluck(:id) / order(:id) itself about 9% faster.

https://gist.github.com/kamipo/11d428b57f3629a72ae89c6f21721326

Before (93e6407):

Warming up --------------------------------------
    users.pluck(:id)     1.217k i/100ms
users.order(:id).to_sql  1.848k i/100ms
Calculating -------------------------------------
    users.pluck(:id)     12.239k (± 8.2%) i/s -     60.850k in   5.013839s
users.order(:id).to_sql  19.111k (± 7.5%) i/s -     96.096k in   5.064450s

After (this change):

Warming up --------------------------------------
    users.pluck(:id)     1.293k i/100ms
users.order(:id).to_sql  2.036k i/100ms
Calculating -------------------------------------
    users.pluck(:id)     13.257k (± 6.9%) i/s -     65.943k in   5.002568s
users.order(:id).to_sql  20.957k (± 7.6%) i/s -    105.872k in   5.086102s

`pluck(:id)` / `order(:id)` are very common use case, and passed symbol
(i.e. quoted identifier) is obviously safe argument, but
`:id.to_s.split(/\s*,\s*/).all? { |part| permit.match?(part) }` is
useless and a bit expensive operation for each such safe symbols (will
make extra 2 mutable strings, 1 array, 1 proc).

This avoids the expensive operation to such safe symbols, it makes
`pluck(:id)` / `order(:id)` itself about 9% faster.

https://gist.github.com/kamipo/11d428b57f3629a72ae89c6f21721326

Before (93e6407):

```
Warming up --------------------------------------
    users.pluck(:id)     1.217k i/100ms
users.order(:id).to_sql  1.848k i/100ms
Calculating -------------------------------------
    users.pluck(:id)     12.239k (± 8.2%) i/s -     60.850k in   5.013839s
users.order(:id).to_sql  19.111k (± 7.5%) i/s -     96.096k in   5.064450s
```

After (this change):

```
Warming up --------------------------------------
    users.pluck(:id)     1.293k i/100ms
users.order(:id).to_sql  2.036k i/100ms
Calculating -------------------------------------
    users.pluck(:id)     13.257k (± 6.9%) i/s -     65.943k in   5.002568s
users.order(:id).to_sql  20.957k (± 7.6%) i/s -    105.872k in   5.086102s
```
@kamipo kamipo merged commit 0a87d7c into rails:master May 28, 2019
@kamipo kamipo deleted the fast_pluck branch May 28, 2019 10:31
kamipo added a commit that referenced this pull request May 28, 2019
Allow symbol (i.e. quoted identifier) as safe SQL string
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant