search

jruby / activerecord-jdbc-adapter

JRuby's ActiveRecord adapter using JDBC.
BSD 2-Clause "Simplified" License
461 stars 385 forks source link

PG set stringtype=unspecified to handle string/uuid edge case #1087

Open imustafin opened 3 years ago

imustafin commented 3 years ago

Fixes #1086.

I'm not sure if changing stringtype is a good idea, but tests pass. I am now testing these changes in our projects.

I think that standard Rails with pg is working similarly by letting postgres to infer the needed types.

enebo commented 3 years ago

I updated this against 61-stable since against master I am seeing a lot of errors which most likely is a problem with rails HEAD.

enebo commented 3 years ago

@imustafin unfortunately we have a few consistent errors but this particular one looks like it added some new one related to prepared arity mismatch. Any chance stringtype is involved here?

@dr-itz If the above is not really an issue for this PR do you have an opinion here? Also @kares :)

https://app.travis-ci.com/github/jruby/activerecord-jdbc-adapter/jobs/527824796

imustafin commented 3 years ago

@enebo which tests are now failing comparing to 61-stable? I can't seem to find them.

I'll take a look at this PR (hopefully by Monday). Do you plan to make CI green soon on 61-stable?

enebo commented 3 years ago

@imustafin in the link above it is the tests like this:

"ActiveRecord::ConnectionAdapters::PostgreSQLAdapterTest#test_exec_with_binds:

ActiveRecord::StatementInvalid: ActiveRecord::JDBCError: org.postgresql.util.PSQLException: ERROR: bind message supplies 0 parameters, but prepared statement "" requires 1"

I did not see these in the head build for 61-stable.

imustafin commented 3 years ago

I think I see it at https://app.travis-ci.com/github/jruby/activerecord-jdbc-adapter/jobs/527821022#L373

enebo commented 3 years ago

@imustafin yeah. I just merged @kares review comment since it makes sense to honor settings. This will start a new CI run and we can see if we still see those couple of errors with this change or they are intermittent.

Faq commented 2 years ago

What is the status of this PR?

imustafin commented 2 years ago

I'm still here but haven't used activerecord-jdbc-adapter for a long time.

Maybe I can rebase this PR and we can resurrect the discussion.

@Faq do you want this to be merged? What is you use case? Is it similar to #1086?