Issue debug statement about "unknown OID" for PostgreSQL custom types. #14275

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
Contributor

joevandyk commented Mar 4, 2014

Changed from a warning.

I dunno if this can be put in 4.1 or earlier releases, but it would be really nice. I can't imagine it causing any problems.

@@ -143,7 +143,7 @@ def exec_query(sql, name = 'SQL', binds = [])
ftype = result.ftype i
fmod = result.fmod i
types[fname] = type_map.fetch(ftype, fmod) { |oid, mod|
- warn "unknown OID: #{fname}(#{oid}) (#{sql})"
+ ActiveRecord::Base.logger.debug "unknown OID: #{fname}(#{oid}) (#{sql})"
@senny

senny Mar 4, 2014

Member

can we use warn instead of debug? This shouldn't write to stderr as warn did.

@joevandyk

joevandyk Mar 4, 2014

Contributor

Would rather not. That would fill up production logs with thousands of
warnings a second.

On Tuesday, March 4, 2014, Yves Senn notifications@github.com wrote:

In
activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb:

@@ -143,7 +143,7 @@ def exec_query(sql, name = 'SQL', binds = [])
ftype = result.ftype i
fmod = result.fmod i
types[fname] = type_map.fetch(ftype, fmod) { |oid, mod|

  •          warn "unknown OID: #{fname}(#{oid}) (#{sql})"
    
  •          ActiveRecord::Base.logger.debug "unknown OID: #{fname}(#{oid}) (#{sql})"
    

can we use warn instead of debug? This shouldn't write to stderr as warndid.


Reply to this email directly or view it on GitHubhttps://github.com/rails/rails/pull/14275/files#r10261257
.

Contributor

joevandyk commented Mar 4, 2014

I'm curious why we need to log anything at all for this. What's the point?

Member

senny commented Mar 4, 2014

@joevandyk basically it's saying that the column is not properly supported and may not work as expected.

Contributor

joevandyk commented Mar 4, 2014

Was that a problem for people using activerecord < 4? That they didn't see
warnings for custom types?

Version 3 didn't warn about this.

Joe

On Tuesday, March 4, 2014, Yves Senn notifications@github.com wrote:

@joevandyk https://github.com/joevandyk basically it's saying that the
column is not properly supported and may not work as expected.


Reply to this email directly or view it on GitHubhttps://github.com/rails/rails/pull/14275#issuecomment-36683284
.

Member

senny commented Mar 4, 2014

@joevandyk we did not rely on the OID at that time. Also many PG specific stuff (like hstore) was introduced with Rails 4 and not present in 3.2. The main issue though is that there are still cases where we don't have to emit the warnings but the current initialization of the type map is not dynamic enough. I have several related issues and pull requests grouped up and assigned for 4.2. Like #13244 for example.

I think we should keep the warning and make the type map initialization better. Then we can see what other cases we need to tackle to remove it completely.

senny added a commit to senny/rails that referenced this pull request Apr 10, 2014

senny added a commit to senny/rails that referenced this pull request Apr 10, 2014

Member

senny commented Apr 10, 2014

@joevandyk see #14692 fora different solution to the log pollution. Let me know if that fix is not enough.

senny added a commit to senny/rails that referenced this pull request Apr 10, 2014

senny added a commit to senny/rails that referenced this pull request Apr 11, 2014

senny added a commit to senny/rails that referenced this pull request Apr 11, 2014

@senny senny closed this in #14692 Apr 11, 2014

senny added a commit that referenced this pull request Apr 13, 2014

PostgreSQL, warn once per connection per missing OID. Closes #14275.
[Yves Senn & Matthew Draper]

(cherry picked from commit 9f62344)

This is a partial backport of #14692.

Conflicts:
    activerecord/CHANGELOG.md
    activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
    activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb

senny added a commit that referenced this pull request Apr 13, 2014

PostgreSQL, warn once per connection per missing OID. Closes #14275.
[Yves Senn & Matthew Draper]

(cherry picked from commit 9f62344)

This is a partial backport of #14692.

Conflicts:
    activerecord/CHANGELOG.md
    activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
    activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb

Conflicts:
	activerecord/CHANGELOG.md
	activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb
	activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
	activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment