These are chat archives for rails-sqlserver/activerecord-sqlserver-adapter

25th
Jan 2015
Sean Griffin
@sgrif
Jan 25 2015 03:17
Random thing that I don't know if I mentioned here or not, but probably affects you -- I changed the RangeError check on the numeric types to type_cast_for_database, instead of type_cast_from_user. It'll be in 4.2.1
16 additions and 163 deletions.
Nice BTW
Ken Collins
@metaskills
Jan 25 2015 14:27
@sgrif Good to know. I will add that on my wrap up list.
I thought it was cool on type_cast_from_user.
Liekly I only have to change some of my tests.
@sgrif Quick question. Is there a way I can run a Ruby object thru my adapter and get back some form of sql type?
From what I can tell, 4.2 is WAY better at including a “column” with all binds. But I still have some guards in place which need to add the sql type hint for sp_executesql in a small case statement.
Nah… seems just the simplified type_to_sql
That’s cool.
Sean Griffin
@sgrif
Jan 25 2015 20:51
So the plan is to remove column from the binds completely, and use an Attribute object instead. An attribute has a name, type, and value. The column name should be all you need in order to do that sort of logic
Actually I guess you'd need the table name, as well. :\
Well I can make that happen
I've been hunting down the sources of nil columns in the binds, but really there's just no way to guarantee that we will always have a column object
But I can guarantee that we'll always have a name
Ken Collins
@metaskills
Jan 25 2015 20:53
I’m OK with not always having a column object.
Sean Griffin
@sgrif
Jan 25 2015 20:53
And since my goal on the Relation / Base side is to have column completely removed from the process of type casting, it makes sense that the first place you'd grab the column is in the adapter
Ken Collins
@metaskills
Jan 25 2015 20:53
Removing column from binds sounds horrible.
This 4.2 change may just burn me out again.
Not sure I can catfch up again for 5.0
Maybe be doing a pre release today.
Need to write an article on it too.
This made me happy.
rails-sqlserver/activerecord-sqlserver-adapter@b65e45e
May use that for the SS timestamp too… gonna look at your adivce and re-eval my substitute_at usage.
Sean Griffin
@sgrif
Jan 25 2015 20:56
Neat
Ken Collins
@metaskills
Jan 25 2015 20:56
This adapter may be Regexp free!
Shade grown too!
Sean Griffin
@sgrif
Jan 25 2015 20:57
And I'm happy to keep sharing the direction I'm working towards while it's still being designed, because you seem interested and seem to have valuable input. But I'd really rather not if I'm not going to get anything more constructive than "that sounds horrible".
Ken Collins
@metaskills
Jan 25 2015 20:58
Apologies.
Sean Griffin
@sgrif
Jan 25 2015 20:58
Also I have to deal with these changes on four adapters, so it's not like I'm blind to what changes will cause you pain
Ken Collins
@metaskills
Jan 25 2015 20:58
I’m a little burned now.
Please dont take that previous comment too seriously.
Sean Griffin
@sgrif
Jan 25 2015 20:59
I understand
Ken Collins
@metaskills
Jan 25 2015 20:59
To be honest, I would have said the same thing prior to Type stuff… and I freaking love it.
Sean Griffin
@sgrif
Jan 25 2015 20:59
And like I mentioned a few days ago, you've convinced me of the value of always having the information required to get the SQL type and other column metadata in type_cast and quote
My work is done then :)
LOL
Sean Griffin
@sgrif
Jan 25 2015 21:00
It just might not be the column object itself, because those methods may happen to be the first place that actually has access to them
Ken Collins
@metaskills
Jan 25 2015 21:00
You are indeed killing it… really really good work.
Cool, I’ll have to see the wisdom to appreciate it.
Sean Griffin
@sgrif
Jan 25 2015 21:01
Lol thanks
Ken Collins
@metaskills
Jan 25 2015 21:01
So far, this is the strongest adapter yet.
Once I made the decision to make FETCH happen.
Sean Griffin
@sgrif
Jan 25 2015 21:01
Lol
I am really glad I've gotten a chance to see what's been happening on your adapter first hand
Ken Collins
@metaskills
Jan 25 2015 21:02
Sweet, you been following along mostly? Thanks really nice of you man.
I’ve really appreciated a co-pilot.
I dont ofthen get to work with really smart people that I just implicitly trust.
Sincere compliment!
Sean Griffin
@sgrif
Jan 25 2015 21:03
Once I finish hashing out the last of these structure changes, we should have a longer chat about what needs to happen for the SQL server adapter to be built on public API
Ideally I'd like to see 5.0 be the last migration that requires a significant amount of work (without a deprecation cycle)
Ken Collins
@metaskills
Jan 25 2015 21:04
Indeed… so many topics.
Like… I’ve always liked coercing my own tests.
It feels wrong to pollute the AR tests and I am very partial with the method we have in place for this adapter. Maybe there is something in that for others.
Sean Griffin
@sgrif
Jan 25 2015 21:05
There's the obvious stuff like we need a public API to describe how a value gets converted to an Arel AST, with the ability to look at both the attribute name, and the value itself to determine the handler (and then a way to automatically register a handler by attribute name when the columns are looked up when it needs to be based on the SQL type)
Ken Collins
@metaskills
Jan 25 2015 21:05
Indeed.
Sean Griffin
@sgrif
Jan 25 2015 21:06
And then the plan is to have the 4 built-in adapters only use public API, and not actually rely on internals
Ken Collins
@metaskills
Jan 25 2015 21:06
Much dog feed eating.
Sean Griffin
@sgrif
Jan 25 2015 21:06
Ideally once that's accomplished, that should be 90% of what's needed for anyone else, but I'd love your input on what some of those APIs look like (I have yet to really start thinking about most of it)
I've been hoping more gem maintainers would be reaching out to me about getting the public APIs defined they need so that 4.2 can be the last version they have to rely on internals.
At this point the list of things that I definitely want to be able to build on public API in 5.0 are database adapters, and sharding gems
Ken Collins
@metaskills
Jan 25 2015 21:08
Ah, good ole fashion sharding gems. Much fun.
Well… my mind is mush, I’ve been on a death march for the 4.2 release.
Sean Griffin
@sgrif
Jan 25 2015 21:09
Lol
Take a break
Ken Collins
@metaskills
Jan 25 2015 21:09
I’d love to contribute to some API stuff in a few weeks if i have some solid ideas.
Mostly I would just examine pain and solve where things got hacky.
Yea… if you did not know...
Sean Griffin
@sgrif
Jan 25 2015 21:09
And things that break between versions. :)
Ken Collins
@metaskills
Jan 25 2015 21:10
Aaron was kind enought to forward a company my way for the SQL Server work.
Sean Griffin
@sgrif
Jan 25 2015 21:10
thoughtbot agreed to give me a few months to work on Rails 5 full time, and that starts tomorrow, so yeah a few weeks will probably be when I start digging into that more closely
Ken Collins
@metaskills
Jan 25 2015 21:10
So the past 4 weeks I have been doing this work as paid OS.
Sean Griffin
@sgrif
Jan 25 2015 21:10
That's really cool
And makes complete sense. I would guess that SQL Server is probably the most used third party adapter
Ken Collins
@metaskills
Jan 25 2015 21:11
It is… good windfall and good for my spirits too, I needed some OS love under my belt again.
Sean Griffin
@sgrif
Jan 25 2015 21:13
Right now I'm trying to just corral all of the sources of bind values. Tons of subtle bugs have been coming out for a while, where they were getting merged from different sources in the wrong order, or certain sources were dropped, etc
And the way binds from the join clause get added is completely ridiculous, and involves sticking them on the arel AST
Which then means that just accessing the binds on a Relation is actually wrong, and it should always be relation.arel.bind_values + relation.bind_values
Which is silly
Also I'm fairly certain that if you call where then having then where again, they'll be in the wrong order
And basically every part of Rails internally has to care about the internals of how they're represented and constructed
So I can't even look at changing how binds are represented because it's such a cluster atm...
At the very least I've got it to the point where nothing relies on bind_values and where_values being mutable
Ken Collins
@metaskills
Jan 25 2015 21:17
Interesting… not been that deep. FWIW, I really liked how schema creates can be done via an arel SelectManager.
Sean Griffin
@sgrif
Jan 25 2015 21:17
So the goal is to take the construction of each of those pieces and break them into a WhereClause and JoinClause object (having will probably just use an instance of WhereClause), and then we can do
def bind_values
  join_clause.binds + where_clause.binds + having_clause.binds
end
I'm pretty sure you can do just about every operation with a SelectManager. XD
Ken Collins
@metaskills
Jan 25 2015 21:22
OK… bravery test...
Bob Ross style.. seeing if I can yank the last hacky exec regexp for identity inserts. Will see how much internal or tests use the proper exec_insert method.
Sean Griffin
@sgrif
Jan 25 2015 21:25
So just to play devils advocate, would it make more sense to figure out why it's attempting to update a column that it's unable to? Is it something AR is attempting to do that it shouldn't be? If the user explicitly tried to do that, doesn't it make more sense to error?
Ken Collins
@metaskills
Jan 25 2015 21:26
We can update identity columns.
On create.
But we have to turn on identity inserts with a yield block.
That reminds me… did you see this...
I should have put that in core_ext. Anyways, that allows me to set the isolation level back again. For SQLServer, it is a session setting and does not revert on commit/rollback.
Either way, the hack was clean IMO.
Re: Identity inserts… were you only talking about updates?
The attributes method was the super clean way for identity to be ignored.
On INSERT, we can do them, but we have to wrap the call in a block that enables for the specific table.
...FF..........EEEEEEEEEEEEEEEEEEEE.............EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE..........EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE
Guess that did not work.
Sean Griffin
@sgrif
Jan 25 2015 21:29
Haha
Ken Collins
@metaskills
Jan 25 2015 21:29
Must be fixtures.
Still usign connection.execute
Sean Griffin
@sgrif
Jan 25 2015 21:29
Fixtures need to die so hard
Ken Collins
@metaskills
Jan 25 2015 21:30
Oh… have you heard my fixture rant? :)
Sean Griffin
@sgrif
Jan 25 2015 21:30
I have not
Ken Collins
@metaskills
Jan 25 2015 21:30
I love “fixtures”
But...
Sean Griffin
@sgrif
Jan 25 2015 21:30
My TL;DR problem with them is that they should not need to be special cased all over the place in the implementation
Ken Collins
@metaskills
Jan 25 2015 21:30
I’ve been harking on this topic for 6 years… I’m finally gonna put a talk together this year...
That goes over the concept.
My named_seeds gem builds on top of my first Rails contribution… way back in the day I made Fixtures#identify work the same on 1.8 and 1.9
But yea… I love fixtures as long as they go in with a factory… then I try to teach people to learn proper factories that hook into you own mother objects vs treating things like on big hash.
This all goes on to talk about leveraging transactional DB runs, with a known story, etc.
Sean Griffin
@sgrif
Jan 25 2015 21:32
Personally I think factories tend to be over-used
Ken Collins
@metaskills
Jan 25 2015 21:33
I agree.
Gimem a few months… will be presenting this all for the first time to Arlington DC groups… Jim Gay’s one.
You in Boston Sean?
Sean Griffin
@sgrif
Jan 25 2015 21:34
If constructing a valid record is exceedingly difficult in tests, it could be a sign of something that will be painful for your users. And if you need a whole tree of 8 associations for some record to exist, it's probably a sign that your design is flawed.
I'm in Denver
Let me know if there's a video of the talk though, I'd love to see it
Ken Collins
@metaskills
Jan 25 2015 21:34
Agreed on that too.
Will do.
Sean Griffin
@sgrif
Jan 25 2015 21:34
Or if you're giving it at a conference that we're both attending. :)
Ken Collins
@metaskills
Jan 25 2015 21:34
CustomInk has a Denver employee.
Sean Griffin
@sgrif
Jan 25 2015 21:34
Which reminds me I need to try and get a talk together about the types/attributes API for RailsConf
Ken Collins
@metaskills
Jan 25 2015 21:34
I plan to if accepted and I can get off my butt and do it.
Boom.
SQL Server => 587 🌟’s
Oracle Enhanced => 372 🌟’s
Sean Griffin
@sgrif
Jan 25 2015 21:39
!!!
Ken Collins
@metaskills
Jan 25 2015 21:39
Winning!
rails/rails@29b3e54
Now going thru my notes… FWIW, I tried putting a cache on our Utils.extract_identifiers. Caching around 600 micro objects did not seem to help.
Ken Collins
@metaskills
Jan 25 2015 21:51
@sgrif Can I tweet that it is cool of ThoughtBot to have you doing paid Rails 5 work for a few months?
Sean Griffin
@sgrif
Jan 25 2015 21:51
I don't see any reason why not
(The company name is never capitalized though)
Ken Collins
@metaskills
Jan 25 2015 21:52
Thanks… I’m a stickler for branding. So I’ll get it right :)
Sean Griffin
@sgrif
Jan 25 2015 22:17
Rofl. Relation#merge completely ignores the table name.
Developer.where(id: 0, projects: { id: 1 }).merge(id: 1) just completely barfs
Ken Collins
@metaskills
Jan 25 2015 22:19
I am really glad you are taking one for the team :)
Ken Collins
@metaskills
Jan 25 2015 22:42
@sgrif If I never super up in initialize_type_map would that break any YAML serialization?
None of those tests failed.
Sean Griffin
@sgrif
Jan 25 2015 22:43
I don't think so, but that assumes that none of the "standard" types work for you
Ken Collins
@metaskills
Jan 25 2015 22:43
I used no standard types… not directly anyway.
They are all SQLServer types that sub class.
I did that so I could allow TinyTDS native types to pass thru right from the C extension.
Sean Griffin
@sgrif
Jan 25 2015 22:44
I would still advise calling super, so you don't have to duplicate all the aliases
The type map was designed so if you replace the registration for "string" with your own type, all aliases will automatically update
Ken Collins
@metaskills
Jan 25 2015 22:46
Then I should super first.
Sean Griffin
@sgrif
Jan 25 2015 22:47
Yes
Ken Collins
@metaskills
Jan 25 2015 23:03
Crap… I’ve supported views since 3.1, gonna turn on supports_views? and cross my fingers.
Wow, there quote a few new ones in here… I have a TODO to turn on FKs.
Sean Griffin
@sgrif
Jan 25 2015 23:04
Lol
Ken Collins
@metaskills
Jan 25 2015 23:31
Ha, only failed one tests and its due to use doing better at views.