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

30th
Jan 2015
Ken Collins
@metaskills
Jan 30 2015 15:35
And there goes @sgrif on the CoffeeScript topic too!
Sean Griffin
@sgrif
Jan 30 2015 15:44
I like function currying. XD
Which is the main reason I still use CS.
(foo) -> (bar) -> vs function(foo) { return function(bar) { return ... }}
Hey random idea that I had this morning that I would like your opinion on: support for check constraints in the migration DSL with an API that matches the validations API
Ken Collins
@metaskills
Jan 30 2015 15:47
Hmmm… I need more to go on.
Sean Griffin
@sgrif
Jan 30 2015 15:48
t.integer :price, check: { greater_than: 0 }
`CREATE TABLE products
Ken Collins
@metaskills
Jan 30 2015 15:48
Whoa… is that a totally new syntax?
(keep going)
Sean Griffin
@sgrif
Jan 30 2015 15:49
It would be, yeah and it would add `CHECK (price > 1) to the column definition
Er 0
CREATE TABLE products (numeric price CHECK (price > 0))
Gah I swapped the column name/type there but I think that gets it across
(That's pg syntax)
Ken Collins
@metaskills
Jan 30 2015 15:51
Interesting, despite being a DB adapter author, I am weak on SQL. Just googled check constraints and learned.
Sean Griffin
@sgrif
Jan 30 2015 15:51
Yeah I was just kinda wondering if it was possible to move all of our validations to the DB (more or less)
Ken Collins
@metaskills
Jan 30 2015 15:52
I drank that DHH kool-aid a long time about about the DB being dumb.
Being in large companies and legacy systems, I get it from the DB perspective.
Sean Griffin
@sgrif
Jan 30 2015 15:53
See, I only partially agree there. I think that user facing errors are definitely a good use case for app level validations, but AR can't truly verify data integrity.
Ken Collins
@metaskills
Jan 30 2015 15:53
Yea… so this would be neat, especially given your recent work.
The cast type could pre validate.
Sean Griffin
@sgrif
Jan 30 2015 15:53
The worst offenders being uniqueness and fks, but as long as save(validate: false) and other ways to skip validations exist, all validations aren't truly validating
Ken Collins
@metaskills
Jan 30 2015 15:53
But it does move validation validation from the model and updates to that change to migrations.
True.
I wonder if this has feature creep for validation functions.
Sean Griffin
@sgrif
Jan 30 2015 15:54
I guess its just a separation of validations that are truly required for data integrity vs stuff that's for user facing errors
Ken Collins
@metaskills
Jan 30 2015 15:54
First SQL Server page I pulled up shows eager beaver DBA funcion examples.
Would the API include a way to ignore or distinguish these via the error objects?
Sean Griffin
@sgrif
Jan 30 2015 15:55
E.g. your app won't break if the user has an empty string for a name, so I would validate that on the model only, but it would break if it got nil so I would have a not null constraint on the DB
Ken Collins
@metaskills
Jan 30 2015 15:56
Might be interesting to know if a DB constraint validatoin did indeed orginate from the DB vs user model-land validation.
Sean Griffin
@sgrif
Jan 30 2015 15:56
No, this would just be data integrity purposes since the DB wouldn't let you save so there could be no API to ignore them.
Same as fks or not null today
Or a unique index
Ken Collins
@metaskills
Jan 30 2015 15:57
But they would be user facing eventually if hit upon and displayed via @obj.errors
That’s not a bad thing, just TOL.
Sean Griffin
@sgrif
Jan 30 2015 15:58
Sure. The issue with actually trying to handle them in AR is that valid? starts to lie.
Though we could maybe infer model validations from the schema
Ken Collins
@metaskills
Jan 30 2015 15:59
Given that varying characters are now not limited.
Any length constraint via the model is not a real lie.
Sean Griffin
@sgrif
Jan 30 2015 16:01
Can you elaborate?
Ken Collins
@metaskills
Jan 30 2015 16:07
Oh.. back.
Well, you had said…
is that valid? starts to lie..
And it depends on what you mean by that. A DB could have 4000 chars to use in a nvarchar() type.
But my model says I only want max 25 chars for valid. Maybe the DB is legacy.
Sean Griffin
@sgrif
Jan 30 2015 16:09
Right. Yeah, there's definitely cases where that could be true today
Ken Collins
@metaskills
Jan 30 2015 16:09
So is that a lie?
Sean Griffin
@sgrif
Jan 30 2015 16:10
So more specifically I was trying to say it would increase the number of cases where valid? could return true, but the record will not be able to save successfully, if you have CHECK (price > 0) at the DB level, but no validation on the model
Ken Collins
@metaskills
Jan 30 2015 16:10
Yea… so if I created that in a migration, then the onus would be on me to limit which is a type of check constraint.
Sean Griffin
@sgrif
Jan 30 2015 16:10
So having us rescue the DB error and then add an error to the model would be confusing
But one option could be that we add the validation automatically during schema inference
Ken Collins
@metaskills
Jan 30 2015 16:10
Oh, great point.
Yea, rescuing would be very anti-performant.
Sean Griffin
@sgrif
Jan 30 2015 16:10
That, too
Basically I've been thinking about the future of shoulda matchers
Ken Collins
@metaskills
Jan 30 2015 16:11
I remember learning my lesson for exceptions for control flow in the adapter before :)
Sean Griffin
@sgrif
Jan 30 2015 16:11
Because I think most of what it provides is useless
IMO tests exist to either catch mistakes, or influence design
Ken Collins
@metaskills
Jan 30 2015 16:11
Whoa, cool topic… how did we get here :)
Agree.
Sean Griffin
@sgrif
Jan 30 2015 16:11
and it { should validate_presence_of(:name) } does neither. It will never influence my design, and if I was going to make a mistake in the validation, I'll make the same mistake in the test since they're 1-1
And that test may as well just grep the source code
Ken Collins
@metaskills
Jan 30 2015 16:12
should_have_db_column
LOL
Sean Griffin
@sgrif
Jan 30 2015 16:12
I do think the allow_value matcher is really useful, since it lets you test complex validations or interactions between them
And make sure it actually was the right column failing to validate, while just setting the value and asserting valid? could be because of an unrelated validation
But one matcher that I think could be cool is it { should verify_integrity_at_the_database_level }
Which today would do things like look for a belongs_to without a foreign key
Or a uniqueness validation without a unique index
Or a presence validation without a null: false constraint
Ken Collins
@metaskills
Jan 30 2015 16:14
Hmm… i see it now.
TBH, I shy away front units this low.
Or maybe I just have not modeled that hard lately.
Sean Griffin
@sgrif
Jan 30 2015 16:17
I guess it's more of a linter than anything else
For things that I consider to be best practices
Which I think most of the people at thoughtbot agree with me on
Ken Collins
@metaskills
Jan 30 2015 16:18
Yea, I totally get that.
Sean Griffin
@sgrif
Jan 30 2015 16:52
So can I get your input on the whole columns being passed to quote/type_castthing? I'm trying to think of the best way to do this. The reason I removed them was to make it clear to other developers that the majority of type casting logic belongs elsewhere. e.g. You should not see that it's a date column, and perform date parsing when given a string. But as per our discussions, looking at the SQL type and seeing that we need to use different quoting logic is completely valid behavior.
Ken Collins
@metaskills
Jan 30 2015 16:52
NP, will take a me a few.
Doing meetings now.
Sean Griffin
@sgrif
Jan 30 2015 16:52
No worries, whenever you have time
Just trying to figure out if there is a way to pass the needed information, but still express what behavior is appropriate for the connection adapter
Since (if we assume the SQL type is all you'll ever need which I know is not the case) even the SQL type could be used to do the wrong thing
Sean Griffin
@sgrif
Jan 30 2015 17:06
      # If you are having to call this function, you are likely doing something
      # wrong. The column does not have sufficient type information if the user
      # provided a custom type on the class level either explicitly (via
      # `attribute`) or implicitly (via `serialize`,
      # `time_zone_aware_attributes`). In almost all cases, the sql type should
      # only be used to change quoting behavior, when the primitive to
      # represent the type doesn't sufficiently reflect the differences
      # (varchar vs binary) for example. The type used to get this primitive
      # should have been provided before reaching the connection adapter.
      def type_cast_from_sql_type(sql_type, value)
Ken Collins
@metaskills
Jan 30 2015 18:20
I think I am starting to see the wisdom in this.
One test I strugged with in the adapter was when using a custom attribute on the model that had atoms_in_the_universe or something like that.
Basically, this is when my intercepted delegation of of type casting from the DB went to var and avoided type casting from the user.
This is all to say that… educating what a method is for and why is a huge win.
In fact, I still think I have to learn when to do it better.
Sean Griffin
@sgrif
Jan 30 2015 18:23
Yeah, ideally the vast majority of the work is already done by the time it reaches the connection adapter
Ken Collins
@metaskills
Jan 30 2015 18:23
And I felt at odds with some of the system and penelized because TinyTDS does all the work before it even gets to ActiveRecord.
Sean Griffin
@sgrif
Jan 30 2015 18:24
For primitives
Ken Collins
@metaskills
Jan 30 2015 18:24
Yes. Only coming from the DB tho.
Sean Griffin
@sgrif
Jan 30 2015 18:24
Right. Sqlite3 is the same, so is one of the mysqls.
PG and the other MySQL are the only two that return strings for primitives
Ken Collins
@metaskills
Jan 30 2015 18:25
Yea, I modeled TinyTDS heavily after Brian’s MySQL2 gem.
Sean Griffin
@sgrif
Jan 30 2015 18:25
And that's fine, we aren't trying to be at odds with that.
Though if there is much of a runtime cost to doing it in the gem, it'd be better to let AR handle it
Since we're lazy about it
But that really shouldn't affect primitives at all
The simplest type which really has any expense to it is dates
Ken Collins
@metaskills
Jan 30 2015 18:26
I have not had time to evaluate that. Could be a premature optimization, but I feel that is not true.
I remember I was bothered by ActiveRecord::Result object too.
One idea I had for TinyTDS was to leverage the aspect that as you called #each on the result, you were actually reding data from the wire.
Sean Griffin
@sgrif
Jan 30 2015 18:28
Yeah, I'm looking to do something similarly lazy in AR::Result as well
Ken Collins
@metaskills
Jan 30 2015 18:28
Idea being that you could if you wanted to, itterate over a large ActiveRecord data set and not incure memory blaot.
Sean Griffin
@sgrif
Jan 30 2015 18:28
The problem is that for all the database gems which don't do that, we can't take advantage of it
So we have to do the find_in_batches approach
Ken Collins
@metaskills
Jan 30 2015 18:29
Yuppers
Michael Lang
@mwlang
Jan 30 2015 20:21
you know...sequel (the other ORM) does a lot of these things really well. I've been toying with writing a "sequel adapter" for some time now and as I look through some of the more modern AR code, I am almost more compelled to give it a whirl.
Seems like it would be a boon to write one adapter, write no hard-coded SQL anywhere, and connect to anything Sequel can connect to.
Ken Collins
@metaskills
Jan 30 2015 20:22
They already have a TinyTDS adapter
Michael Lang
@mwlang
Jan 30 2015 20:22
yeah, I know...I'm saying write a "sequel" adapter for AR. :-)
btw, added that README and starting to outline a blog post for the backported tinytds code.
one of the reasons you said you were dropping < 2012 support is because of the offset/limit features being missing from earlier SQL Servers...however, Sequel has solved this issue either using the DBMS native facilities or emulating it, so again, sequel as an adapter has some compelling wins from my perspective.
Ken Collins
@metaskills
Jan 30 2015 20:49
@mwlang How so?
I mean, we emulated it via standard ROW_NUMBER functions.
Since Rails 1.0.
If that is what they are doing, then it still does not make sense for us moving forward.
We have to do so much more than ActiveRecord and the theory of “just use ROW_NUMBER” breaks down and polluted many other places in the adapter.
TL;DR - ROW_NUMBER was a cancer for us, not a pancea.
Aside, I started using ROW_NUMBER in the adapter around 3.0 since 2000 did not have that.
4.2 is the new 3.0 :)
Ken Collins
@metaskills
Jan 30 2015 20:59
When 3.0 was released, people wanted their emulated select TOP inverse order hacks. Which was the crazies of all cause the windowing functions performance got worse the further out your page was.
Love that illustration
Ken Collins
@metaskills
Jan 30 2015 21:05
This makes me think of that 1.2 app on 2000 you were talking about. If they have decent side tables they are windowing thru, a move to 3/4.1 would be epic for them
Not to mention, 2000 support was dropped by M$ long long ago.
I just sent os@microsoft.com a email this morning.
Asking that they give us Azure instances for testing.
Michael Lang
@mwlang
Jan 30 2015 21:07
Ah, I wasn't suggesting it for this here adapter...was more thinking to tackle it for my own fun...but more to the point, sequel has a nice arel-like syntax and you can call DB[:mytable].limit(10).offset(10) whether its SQL Server 2000, 2005, 2008, or 2012 and sequel, uses the best strategy for each, IIRC.
I bet azure is fun. I've always wondered what a really scaled out SQL Server infrastructure feels like to work with. Fortunately, I am doing less and less SQL Server and primarily because I'm starting to let contracts go that the clients are still on 2000 and 2005.
and, yeah, I know row_number is a cancer. I encountered its nastiness when putting together the backported tinytds adapter.
Ken Collins
@metaskills
Jan 30 2015 21:11
At one time I tested Azure. But that is when my work place paid for it.
It had its oddities. But we should be just fine on it.