-
Notifications
You must be signed in to change notification settings - Fork 176
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
pg sets the encoding on PG::Connection objects #280
Comments
Original comment by Lars Kanis (Bitbucket: larskanis, GitHub: larskanis). Thank you @eregon for notifying us and for your analysis! Storing the encoding in the metadata of a T_DATA object was a handy trick to save some space in the connection and the result. However if it's in the way of some future improvements in MRI, I think we can find an alternative solution for it. I'll check this and will make ruby-pg compatible with ruby-trunk. Regarding TruffleRuby, please let me know when it is ready to be supported by ruby-pg. We already had some Rubinius compatibility in the past for parts that are too MRI specific.Kind Regards, |
Original comment by Benoit Daloze (Bitbucket: eregon, GitHub: eregon). Yes. The fix should be:
|
This is fixed by commit cfb90ef |
Thanks for the fix and congrats on the GitHub migration. |
…bjects The previous solution used MRI's internal bits to store the connection encoding. This still works, but is only supported for String and some other types of VALUEs since ruby-2.6. Non-encoding capable objects are no longer allowed to use these bits. To save some bytes of struct size of PG::Result, we're using bit fields for enc_idx and autoclear. This patch also removes the cache from pgconn_external_encoding(). It was useful in the past, when the encoding retrievel was slow. In the meantime the cache did an improvement of factor 3 only. Therefore as external_encoding is a very infrequently called method, there is no need for a cache any longer. Fixes ged#280
Original report by Benoit Daloze (Bitbucket: eregon, GitHub: eregon).
pg sets the encoding with ENCODING_SET_INLINED/rb_enc_set_index on PG::Connection objects. Pg uses a macro PG_ENCODING_SET_NOCHECK() in pg.h.
This seems not supported in latest ruby trunk, as Koichi Sasada explicitly added a check against it in ruby/ruby@d0fb73a , and as a result the program crashes when rb_enc_set_index() is called on a non-encoding capable object.
To observe this, we need to change PG_ENCODING_SET_NOCHECK() to just call rb_enc_set_index() as ENCODING_SET_INLINED() doesn't contain the check currently, but likely has unsupported semantics and only works by luck that there are < 128 encodings.
Then with ruby-trunk we can see (the program assume a postgres db named "test" exists):
I think one solution is to store the encoding in a separate field, for instance in
t_pg_connection
. If you think Ruby should keep supporting rb_enc_set_index() on non-encoding-capable objects, please file an issue at https://bugs.ruby-lang.org/I'm just reporting this as I noticed the change by Koichi and knew from trying to run pg with TruffleRuby that pg uses rb_enc_set_index() on the PG::Connection object.
The text was updated successfully, but these errors were encountered: