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

Remove ignored_sql from SQLCounter by adding "TRANSACTION" to log name #36202

Merged
merged 1 commit into from
May 7, 2019

Conversation

yahonda
Copy link
Member

@yahonda yahonda commented May 7, 2019

Summary

Remove ignored_sql from SQLCounter by adding "TRANSACTION" to log name.

This commit adds "TRANSACTION" to savepoint and commit, rollback statements
because none of savepoint statements were removed by #36153 since they are not "SCHEMA" statements.

Although, only savepoint statements can be labeled as "TRANSACTION"
I think all of transaction related method should add this label.

Follow up #36153

This commit adds "TRANSACTION" to savepoint and commit, rollback statements
because none of savepoint statements were removed by rails#36153 since they are not "SCHEMA" statements.

Although, only savepoint statements can be labeled as "TRANSACTION"
I think all of transaction related method should add this label.

Follow up rails#36153
@@ -107,20 +107,12 @@ def clear_log; self.log = []; self.log_all = []; end

clear_log

self.ignored_sql = [/^SAVEPOINT/, /^ROLLBACK TO SAVEPOINT/, /^RELEASE SAVEPOINT/]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've realized #36153 accidentally removed /^BEGIN/, and /^COMMIT/.
Adding "TRANSACTION" to the /^SAVEPOINT/, /^RELEASE SAVEPOINT/, /^ROLLBACK/, /^BEGIN/, and /^COMMIT/ is fine to me.

@kamipo kamipo merged commit f10ad55 into rails:master May 7, 2019
@yahonda yahonda deleted the log_transaction branch May 8, 2019 15:04
yahonda added a commit to yahonda/oracle-enhanced that referenced this pull request Jun 6, 2019
yahonda added a commit to yahonda/oracle-enhanced that referenced this pull request Jun 7, 2019
yahonda added a commit to yahonda/oracle-enhanced that referenced this pull request Jun 18, 2019
yahonda added a commit to yahonda/oracle-enhanced that referenced this pull request Jun 19, 2019
yahonda added a commit to yahonda/oracle-enhanced that referenced this pull request Jun 21, 2019
Refer rails/rails#36153 and rails/rails#36202

Some of Oracle enhanced adapter unit tests get failed
because they depend on log output which is not logged since they are "SCHEMA".

To workaround these  failures overriding `ActiveRecord::LogSubscriber::IGNORE_PAYLOAD_NAMES`
in `spec/spec_helper.rb`, which causes these warnings.

```
$ bundle exec rspec
==> Loading config from ENV or use default
==> Running specs with MRI version 2.6.3
==> Effective ActiveRecord version 6.1.0.alpha
/home/yahonda/git/oracle-enhanced/spec/spec_helper.rb:112: warning: already initialized constant ActiveRecord::LogSubscriber::IGNORE_PAYLOAD_NAMES
/home/yahonda/git/rails/activerecord/lib/active_record/log_subscriber.rb:5: warning: previous definition of IGNORE_PAYLOAD_NAMES was here
```
yahonda added a commit to yahonda/oracle-enhanced that referenced this pull request Jun 21, 2019
Refer rails/rails#36153 and rails/rails#36202

Some of Oracle enhanced adapter unit tests get failed
because they depend on log output which is not logged since they are "SCHEMA".

To workaround these  failures overriding `ActiveRecord::LogSubscriber::IGNORE_PAYLOAD_NAMES`
in `spec/spec_helper.rb`, which causes these warnings.

```
$ bundle exec rspec
==> Loading config from ENV or use default
==> Running specs with MRI version 2.6.3
==> Effective ActiveRecord version 6.1.0.alpha
/home/yahonda/git/oracle-enhanced/spec/spec_helper.rb:112: warning: already initialized constant ActiveRecord::LogSubscriber::IGNORE_PAYLOAD_NAMES
/home/yahonda/git/rails/activerecord/lib/active_record/log_subscriber.rb:5: warning: previous definition of IGNORE_PAYLOAD_NAMES was here
```
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

2 participants