Skip to content

Loading…

Firebird supports encoding option #760

Closed
wants to merge 1 commit into from

2 participants

@gvieira

Hi,

Since fb gem now supports string encoding in connect method, it would be nice to have this option for firebird.

Please let me know if you need any other information.

Regards,
Vieira

@jeremyevans
Owner

Sequel's firebird adapter only supported wishdev's old fb fork last I checked. Have you actually tested this with the current fb driver?

I can probably test this tomorrow. Assuming it doesn't break things, I'll merge it.

@gvieira

You're right. the wishdev fork really doesn't support encoding. Anyway, I didn't know about it and did install the official version. It looks like everything worked well. Is there any kind of test I can make to see if it is really needed to use this old fork?

Thanks for your quick response!

@jeremyevans
Owner

If the official fb driver now works correctly with the firebird adapter, that's great. It's been a few years since I last checked, but I know it didn't work back then. I'll test it out in a bit and seen how it works. Thanks!

@gvieira

Nice to hear it! Anything I can do to help, please let me know.

@jeremyevans
Owner

Updating to the official fb driver causes failures in the dataset integation tests to go from less than 20% to over 90%. How heavily have you used the firebird adapter with the official fb driver?

I'm afraid I can't accept this patch as it would give users the false impression that it works on the official fb driver. I think the first step before this patch could be accepted is to get the firebird adapter working with the official fb driver, which will require changes to one or the other. The best way to start diagnosing issues is to run the dataset integration tests:

SEQUEL_INTEGRATION_URL="firebird://SYSDBA:password@localhost//path/to/test/database.fdb" ruby -S spec -b spec/integration/dataset_test.rb

Over 90% of these should fail, so you would need to figure out why, and either fix it in the firebird adapter or the fb driver depending on the source of the problem.

Assuming you are able to get it to work with the official fb driver and get the dataset integration test failure rate down to around 20%, I can certainly accept the encoding patch then.

@gvieira

Sure I understand. Actually, all I've done with firebird is some simple query like SELECT * FROM TABLE WHERE ID IN (1, 2, 3). This is probably the reason why it worked for me. Do you think I can have any issues with these queries types?
Anyway, now I know how to test the adapter, so I can try and fix the original repo.

@jeremyevans
Owner

I'm guessing simple selects will probably be fine. I think part of the problem is that something is causing the underlying connection to go into a bad state, where all queries end up getting rejected. Fixing that may cause most of the failures to go away. That's just a guess, though.

@gvieira

Got it! Going to check it this weekend. See ya!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 22, 2014
  1. @gvieira
Showing with 2 additions and 1 deletion.
  1. +2 −1 lib/sequel/adapters/firebird.rb
View
3 lib/sequel/adapters/firebird.rb
@@ -18,7 +18,8 @@ def connect(server)
Fb::Database.new(
:database => "#{opts[:host]}:#{opts[:database]}",
:username => opts[:user],
- :password => opts[:password]).connect
+ :password => opts[:password],
+ :encoding => opts[:encoding]).connect
end
def disconnect_connection(conn)
Something went wrong with that request. Please try again.