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

23rd
Jan 2015
Ken Collins
@metaskills
Jan 23 2015 19:36
@sgrif Thinking out loud… I wonder if I should have not followed the PG type examples and used ActiveRecord::Type::SQLServer as my type namespace vs ActiveRecord::ConnectionAdapters::SQLServer::Type. Seems like it would be easier for people to use coerce if needed. Much like NumericData does in the ActiveRecord tests.
So… attribute :world_population, Type::SQLServer::BigInteger.new
Sean Griffin
@sgrif
Jan 23 2015 19:36
Not sure. The way PG happened was just due to where those classes lived before I started working on this, rather than a decision that was explicitly made
I don't really have an opinion on whether that was better or worse
Ken Collins
@metaskills
Jan 23 2015 19:37
Yea, same here, just stuck out at me.
People would have to use ConnectionAdapters::SQLServer::Type now for mine.
Sean Griffin
@sgrif
Jan 23 2015 19:38
You could always do
module Type
  SQLServer = ConnectionAdapters::SQLServer::Type
end
Grrrr
Ken Collins
@metaskills
Jan 23 2015 19:38
And I can ot really redefine Type in attributes. But if I nested, then I would get the alias for free.
I can.
That’s a damn good idea :)
Thanks man!
Sean Griffin
@sgrif
Jan 23 2015 19:39
There's also some missing pieces in what the attributes API will look like
David (to my protest) insists that the API allow passing symbols, instead of an object
Passing a type object will definitely always be valid
But users will probably be able to do attribute :foo, :string
Which means the adapters will need to be able to override what :string translates to
And likely register their own
Ken Collins
@metaskills
Jan 23 2015 19:40
Well, if symbols, I’d have to patch some lookup to mine.
Yea.
That’s the benefit.
Sean Griffin
@sgrif
Jan 23 2015 19:40
Don't worry, I'll give you a stable API to use
You shouldn't need to patch anything
Ken Collins
@metaskills
Jan 23 2015 19:40
Oh, I’m good.
Just coercing tests now.
Sean Griffin
@sgrif
Jan 23 2015 19:40
Cool
Ken Collins
@metaskills
Jan 23 2015 19:41
The NumericaData being defined in a few places exposed a situation to me… I think I can work around it.
BasicsTest#test_numeric_fields [/Users/kencollins/.rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/bundler/gems/rails-a6cfe96cb88e/activerecord/test/cases/base_test.rb:1023]:
Expected #<BigDecimal:7feef6a1d140,'0.6E10',9(27)> to be a kind of Integer, not BigDecimal.
Sean Griffin
@sgrif
Jan 23 2015 19:42
I really need to add the ability to turn off automatic schema lookup
Ken Collins
@metaskills
Jan 23 2015 19:42
My day has been going great!
Once I found this...
Sean Griffin
@sgrif
Jan 23 2015 19:43
      def test_attributes_which_are_invalid_for_database_can_still_be_reassigned        type_which_cannot_go_to_the_database = Type::Value.new        def type_which_cannot_go_to_the_database.type_cast_for_database(*)          raise        end        klass = Class.new(ActiveRecord::Base) do
          self.table_name = 'posts'
          attribute :foo, type_which_cannot_go_to_the_database
        end
        model = klass.new

        model.foo = "foo"
        model.foo = "bar"

        assert_equal "bar", model.foo
      end
    end
Ken Collins
@metaskills
Jan 23 2015 19:43
I’ve been working till now with random tests order.
Sean Griffin
@sgrif
Jan 23 2015 19:43
Hah
Ok I don't know why that formatting got messed up
Regardless I hate that I have to set the table name
For a test which has nothing to do with persistence
I guess that'll get fixed when I move this to Active Model. :\
Ken Collins
@metaskills
Jan 23 2015 19:44
Yea, but I get it.
I’ve made a point to really clean up SS tests.
I’ve moved all models and helpers out.
Namespaced everyting in ARTest or my own SSTest prefix.
Sean Griffin
@sgrif
Jan 23 2015 19:46
Yeah, we've been doing the same in the Rails code base. New tests tend to just define a table/class in the test file or in the test itself, rather than having shared fixtures all over the place
Otherwise we get that
Ken Collins
@metaskills
Jan 23 2015 19:50
Ug… I know.
Ken Collins
@metaskills
Jan 23 2015 20:01
@sgrif One thing that occured to me that you likely know...
Sean Griffin
@sgrif
Jan 23 2015 20:01
I gotta go record my podcast, I'll be back in an hour.
Ken Collins
@metaskills
Jan 23 2015 20:02
When I was doing all our own type objects, there was a time where I had an initialize that set defaults. So that way you did not have to remember to always instantiate a certain way.
NP!
That strategy had a few edge case bugs.
Sean Griffin
@sgrif
Jan 23 2015 21:11
Can you elaborate further?
Ken Collins
@metaskills
Jan 23 2015 21:34
Sure, for example our BigInteger type. There is no other limit than 8.
So I thought to myself, why am I passing that in when I register the type. I should override initialize and specifically set that limit.
Sean Griffin
@sgrif
Jan 23 2015 21:34
Those are going to get changed to kwargs, next time I touch the file
Ken Collins
@metaskills
Jan 23 2015 21:34
So people would not be required to remember that.
But… I can’t do that.
Once I changed my bigint to internalize the @limit...
Sean Griffin
@sgrif
Jan 23 2015 21:35
def initialize(options = {})
  options = options.merge(limit: 8)
  super
end
?
Ken Collins
@metaskills
Jan 23 2015 21:35
I can’t.
Running tests now...
Ken Collins
@metaskills
Jan 23 2015 21:42
Hmmm… seems to be working now… maybe because I have a few more alias types. I think the issues was that bigint(8) was the type I returned from DB reflection, but it is never quoted or used that way in SQL for bind var declarations.
So I’m good now… will make notes if not. Thanks.