claudiuinberlin has quit [Quit: My MacBook has gone to sleep. ZZZzzz…]
claudiuinberlin has joined #jruby
claudiuinberlin has quit [Quit: My MacBook has gone to sleep. ZZZzzz…]
claudiuinberlin has joined #jruby
<rdubya>
It looks like AR dropped support for postgres < 9.1 with Rails 5.0, is that something arjdbc should do as well or should it continue to support older versions?
<enebo>
rdubya: good question
<enebo>
rdubya: you see I made a change last night for BigDecimal
<enebo>
rdubya: I did not test on postgresql but did I think using sqlite3 (should be fine though)
<enebo>
rdubya: how old is postgres < 9.1?
<rdubya>
yeah saw that this morning, once that PR I created is merged in, I'll get up to date with the main branch and make sure its passing in postgres
<enebo>
rdubya: looks like EOL for 9.1 is 9/16
<enebo>
err 09/2016
<enebo>
rdubya: so not too long in the tooth but I bet day 0 is much scarier in the db world than Java
<rdubya>
It looks like AR still supports 9.1 but nothing below that
<enebo>
looks like they EOL a version once a year
<enebo>
9.0 was EOL 09/2015
<rdubya>
Makes sense that arjdbc would drop support for it then? Since it hasn't been supported for over 2 years?
<enebo>
rdubya: my take (and perhaps kares knows more about users of arjdbc) is that we should just worry about this if it comes up
<rdubya>
ok
<enebo>
rdubya: I suspect we can hack in support if it ends up mattering
<rdubya>
as of right now, I don't think there is anything that specifically breaks because of it, though, I don't have postgres 9.0 installed to test
<rdubya>
there are already conditionals for 8, 8.2, 9, and 9.2
<enebo>
rdubya: yeah 8.2 was EOL in 2011
<enebo>
rdubya: but I guess it depends on how many there are
<enebo>
rdubya: I do not want to kill users over having ancient dbs (and some back office apps may be way out of date) but I also would like to get rid of as many code paths as possible for maintenance
<rdubya>
enebo: that makes sense
<enebo>
rdubya: another thing I have been thinking about but have not tried (a did a small experiement with sqlite3) was updating to latest jdbc drivers
<enebo>
the sqlite3 one was fairly old but I was in flux when I did it so it was poor time to try it
<rdubya>
I think kares was upgrading some of them in the master branch, not sure how far he was going
<enebo>
yeah
<rdubya>
enebo: it would be nice to get upgraded, I think there are some things in the newer sqlite drivers that could simplify some of the date management stuff that currently has to be done manually
<enebo>
yeah I think sqlite fixes some transaction stuff too
<rdubya>
I created an issue yesterday with jruby core, that I believe is messing with one of the postgres tests for handling dates
<rdubya>
I think it is just the way the test is written that is causing it to fail, because the AR tests don't fail on it, but wasn't sure
<enebo>
rdubya: yeah I guess the more it is isolated the easier it will be to understand
<enebo>
rdubya: fact that AR doesn't fail is ???
<rdubya>
yeah, I tracked it down to being if you create a time and convert it to a datetime and back again you won't get the original time
<rdubya>
it ends up with a timezone that is +04:56 or something like that
<enebo>
rdubya: so something MRI does differently yet AR will still pass
<rdubya>
yeah
<enebo>
weird and it is not historical date?
<enebo>
those have shown occasional differences and sometimes now even ones we know who is right with
<rdubya>
i tested with years between -1 and 3, let me verify if it happens with ones more recent
<rdubya>
enebo: I'll update the issue, looks like it happens with dates that are less than the year 1884
<enebo>
rdubya: ah ok so historical but near enough where some users could trip over it
<enebo>
rdubya: I see one data type switched to wanting Integer/Fixnum
<enebo>
rdubya: in the sqlite3 adapter I tried to rearrange as many methods so they could be a literal copy of AR code or just require the AR code itself
<enebo>
rdubya: one question you added interval ... does that fix our tests or Rails tests?
<rdubya>
enebo: that fixes the AR tests, i'm not sure why our code needed it since it wasn't added to AR until 5.1 but the schema dumper wasn't recognizing "interval" as a valid type without it
<rdubya>
i'm trying to go the same route, with postgres, if I can reuse AR code I will otherwise I try to make it as similar as possible
<enebo>
rdubya: heh yeah my guiding light on the work is I want all same methods to match as much as possible between AR source and ours
<enebo>
rdubya: interval may be something weird we need to add but I meant mostly that if your changes to index support match AR's code then this should all be fine
<enebo>
rdubya: If we are using different SQL because it supports 8.0 of postgresql or some reason like that then we should think hard on whether that is worth the cost (I am a little more towards favoring maintenance side of things)
<enebo>
>95% of sqlite3 Ruby code is a 100% copy of AR code right now (hoping to get some change in Rails to kill all this code)
<rdubya>
enebo: yeah, this is all basically right out of the AR code, unfortunately we have to tweak the returned values or I could have just use AR's code
<enebo>
postgresql unfortunately is much more complicated
<enebo>
rdubya: oh do we? can we fix that?
<enebo>
I remember they have some boxed datastructure for some crap out of the native adapter
<rdubya>
we have to convert the value from a string to a boolean, and test for a PGobject
<enebo>
row is a JDBC result set?
<enebo>
we could do this in Java if not right?
<rdubya>
Let me double check
<enebo>
heh it is hard to remember where query is :)
<enebo>
if this uses mapQueryResult then I wonder if this can get fixed there
<rdubya>
enebo: looks like it is running through jdbcToRuby
<rdubya>
I'm still a little unclear on the difference between execute_query and execute_query_raw and the need for each
<rdubya>
this runs through execute_query_raw
<enebo>
rdubya: ah indeed
<enebo>
rdubya: yeah I do not know why either but I would like to know why and maybe remove one
<enebo>
heh "* NOTE: since 1.3 this behaves like <code>execute_query</code> in AR-JDBC 1.2"
<rdubya>
lol
<enebo>
well that probably is not true for us any more based on changes I made alone
<enebo>
I would try calling non-raw version and then consider mapQueryResult to fix that output
<enebo>
although it is possible they behave the same but one is quicker
<enebo>
but I strongly suspect they do not behave the same
<enebo>
and I also think that contract is probably hard to maintain too
<enebo>
by strong believe because I definitely only changed one and not the other
<enebo>
it may be that way on master
<rdubya>
yeah, I think they have diverged
<enebo>
so another take is even if we slow some stuff down atm we can always revisit performance and then maybe make a convention for when we decide to do it differently
<rdubya>
yeah
<rdubya>
Is it alright if I keep this PR as is for now and then we can look at fixing that up as a bigger task that tries to reconcile those two paths?
<enebo>
something where we spell out what to keep track of wrt AR code
<enebo>
rdubya: yeah but I would identify in comments which code differs a a FIXME
<rdubya>
ok I'll add that in
<enebo>
I do that in sqlite3 which is helpful for next time I diff
<rdubya>
good idea, I'll try to do a better job of marking the differences with comments
<enebo>
rdubya: yeah great. I have not tried to update for 9.1 but of course that is destined to be a new branch
<enebo>
I will not bother trying until we get 9.0 more done
<enebo>
I am hoping 9.1 is a tiny amount of work
<enebo>
mostly just changing what diffed
<rdubya>
yeah, I think if we can get a solid 5.0 in place, at least people can start testing that out and can get off of 4.2 now that it is EOL
<rdubya>
then like you said, the minor point update will hopefully be much smaller changes
<enebo>
rdubya: one last thing I would like is to mark adapters with an Annotaion in RubyJdbcConnection
<enebo>
like @Sqlite3 next to each method I use
<enebo>
rdubya: I will add some today
<enebo>
rdubya: I am frightened to change anything which has dependency on potentially multiple adapters
<enebo>
I know that might be a bit noisy but at this point we only plan on 3 adapters and it would be nice to know
<enebo>
being in a mixed two lang environment this traceability is very hard
<rdubya>
enebo: I agree, its really hard to see what gets used where and makes me afraid to make some changes that should probably be made as well lol
<rdubya>
enebo: just merged in that PR so I'll update my branch and test out your changes now
<enebo>
rdubya: great thanks
<rdubya>
enebo: another obscure one that I dug into a bit but put on the back burner is a couple tests that are testing that a transaction gets rolled back if the thread is killed
<rdubya>
i've tracked it down to where it looks like the 'ensure' block of the transaction is executing in the context of the thread and it has a state of "run" so the code is trying to commit the transaction
<rdubya>
and since the thread is still alive and the connection is still there, the commit is successful instead of causing a rollback
<enebo>
rdubya: ah yeah sqlite3 has that one still
<enebo>
rdubya: ok that is good to know...we may need to PR Rails and work around it
<rdubya>
ok
<enebo>
jesus rubocop
<enebo>
- if ENV["LAWS"] && !ENV["LAWS"].match(/\A\s*\z/) + if !ENV["LAWS"]&.match(/\A\s*\z/)
<enebo>
Am I the only person who finds the suggestion confusing looking
<rdubya>
no, that is hard to follow
<enebo>
I guess ! is the whole expr but it looks super weird with &. and a hash lookup
<enebo>
funny this may just be how we see &.
<enebo>
the second half of the original is basically the replacement with an extra '&'
<rdubya>
yeah
<enebo>
Some idioms do eventually win like symbol proc
<rdubya>
One of the things I had trouble with while learning ruby (and I think the &.) adds to it, is trying to learn what all those mean
<rdubya>
you can't really google "&."
<rdubya>
but i guess most languages have a long list of those types of things that you just have to find a good reference for
<rdubya>
enebo: looks like your changes are good, I don't see any new broken postgres tests
<rdubya>
enebo: another error that I see occasionally pop up in random tests is: ActiveRecord::StatementInvalid: ConcurrencyError: Detected invalid array contents due to unsynchronized modifications with concurrent users: SET client_min_messages TO 'warning'
<rdubya>
guessing that may be related to some threading issues as well, but I haven't been able to reproduce them
<enebo>
rdubya: yeah don't be surprised if AR has threading issues
<enebo>
rdubya: we may be able to figure it out and send a PR if so
claudiuinberlin has quit [Quit: My MacBook has gone to sleep. ZZZzzz…]
vtunka has quit [Quit: Leaving]
shellac has quit [Quit: Leaving]
lanceball is now known as lance|afk
claudiuinberlin has joined #jruby
tax has joined #jruby
claudiuinberlin has quit [Quit: My MacBook has gone to sleep. ZZZzzz…]
claudiuinberlin has joined #jruby
claudiuinberlin has quit [Client Quit]
<GitHub31>
[jruby-openssl] drcapulet opened pull request #146: Normalize all constants in CipherStrings as public final static (master...alexc-public) https://git.io/vdoak
drbobbeaty has quit [Quit: My MacBook Pro has gone to sleep. ZZZzzz…]
lanceball is now known as lance|afk
<GitHub142>
[jruby] walerian777 opened pull request #4814: Changed error thrown by Dir.entries method from ENOENT to ENOTDIR (master...dir-error-inconsistency) https://git.io/vdKYw