Skip to content
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

Support for raw SQL queries #184

Closed
noahlh opened this issue Apr 24, 2018 · 10 comments
Closed

Support for raw SQL queries #184

noahlh opened this issue Apr 24, 2018 · 10 comments

Comments

@noahlh
Copy link
Contributor

noahlh commented Apr 24, 2018

Proposal for discussion:

One of the things I really like about Granite is the philosophy that you should be able to get into the nitty gritty of SQL and let the ORM handle only the task of mapping the results to the object. The all method does a great job of this.

I'd like to propose abstracting one level further and adding a method called sql (or maybe raw?) which allows a full SQL query to be passed in and simply maps the results to the appropriate fields of a model.

An example based on an issue I ran into the the other night dealing with a legacy database in a project I'm building:

Assume I have a table called vehicles as such:

-----------------------------------------------
|  vehicleid  |  year  |  makeid  |  modelid  |
-----------------------------------------------
|     1       |  1999  |    54    |    65     |
|     2       |  2000  |    55    |    72     |
|     ...     |   ...  |    ...   |    ...    |
-----------------------------------------------

makeid and modelid are both references to foreign keys in the makes and models tables respectively. In those tables are naming columns (called makename and modelname).

If I want to build a single JOIN query to pull in a vehicle's year, make, and model names, I can construct the following SQL to do so:

SELECT vehicles.vehicleid, vehicles.yearid, makes.makename AS make, models.modelname AS model FROM vehicles JOIN make ON vehicles.makeid = makes.makeid JOIN model ON vehicles.modelid = models.modelid

There's no way currently to have Granite construct that query -- the JOIN part could be done manually, but by default the SELECT portion can only be constructed to access the fields from a single table (the model's table).

In my proposal, you could have a model as such:

class Vehicle < Granite::ORM::Base
  adapter pg

  primary vehicleid : Int32
  field yearid : Int32
  field make : String
  field model : String
end

And submit a query as such:

vehicles = Vehicle.sql("SELECT vehicles.vehicleid, vehicles.yearid, makes.makename AS make, models.modelname AS model FROM vehicles JOIN make ON vehicles.makeid = makes.makeid JOIN model ON vehicles.modelid = models.modelid")

With the resulting vehicles object, you could then access the respective fields as you would normally, i.e.

vehicles.each do |v|
  puts "This is a #{v.yearid} #{v.make} #{v.model}"
end

I've implemented a monkey-patched version of this in my own application as follows (PG only, for starters)

class Granite::Adapter::Pg < Granite::Adapter::Base
  def query(statement = "", params = [] of DB::Any, &block)
    statement = _ensure_clause_template(statement)
    log statement, params
    open do |db|
      db.query statement, params do |rs|
        yield rs
      end
    end
  end
end

module Granite::ORM::Querying
  def sql(clause = "", params = [] of DB::Any)
    rows = [] of self
    @@adapter.query(clause, params) do |results|
      results.each do
        rows << from_sql(results)
      end
    end
    return rows
  end
end

Thoughts on the above? If this is of interest, I'd be happy to take a stab at a proper PR w/ tests, etc.

@drujensen
Copy link
Member

@noahlh this is something I have been wanting to add. I recommend adding a new base class that is a View Only object and supports a select that provides a mapping to the fields. A view model.

This is a common pattern that will support any type of results including aggregate group by results like count.

We would remove the transactional module to remove save and delete from the object.

Let me know what u think.

@drujensen
Copy link
Member

@noahlh What about adding a select macro?

class Vehicle < Granite::ORM::Base
  adapter pg

  primary vehicleid : Int32
  field yearid : Int32
  field make : String
  field model : String

  select <<-SQL
    SELECT vehicles.vehicleid, vehicles.yearid, makes.makename AS make, models.modelname AS model FROM vehicles JOIN make ON vehicles.makeid = makes.makeid JOIN model ON vehicles.modelid = models.modelid
  SQL
end

That way, you can still perform queries:

model = "Tesla"
Vehicle.all("WHERE model = ?", model)

This will only need to override the default generated select statement so we could just check to see if one is defined and use it, otherwise generate one from the field definitions.

@noahlh
Copy link
Contributor Author

noahlh commented Apr 26, 2018

@noahlh this is something I have been wanting to add. I recommend adding a new base class that
is a View Only object and supports a select that provides a mapping to the fields. A view model.

This is a common pattern that will support any type of results including aggregate group by results
like count.

We would remove the transactional module to remove save and delete from the object.

Let me know what u think.

@drujensen I'm punching slightly above my weight class here so apologies if any of this is malformed, as it were.

So are you suggesting something like a new Granite::ORM::Viewmodel that sits alongside Granite::ORM::Base? And users would have to choose between the two when creating a new class depending on the functionality needed?

(replying to your other comment shortly)

@noahlh
Copy link
Contributor Author

noahlh commented Apr 26, 2018

@noahlh What about adding a select macro?

class Vehicle < Granite::ORM::Base
  adapter pg

  primary vehicleid : Int32
  field yearid : Int32
  field make : String
  field model : String

  select <<-SQL
   SELECT vehicles.vehicleid, vehicles.yearid, makes.makename AS make, models.modelname AS 
   model FROM vehicles JOIN make ON vehicles.makeid = makes.makeid JOIN model ON 
   vehicles.modelid = models.modelid
 SQL
end

That way, you can still perform queries:

model = "Tesla"
Vehicle.all("WHERE model = ?", model)

This will only need to override the default generated select statement so we could just check to see if one is defined and use it, otherwise generate one from the field definitions.

That's a very clever idea -- I like it a lot. The only limitation I see is that it (effectively) restricts you to one alternate SELECT per model, but I suppose that's probably a good thing? If you're getting into multiple different methods of populating a given model, you probably need multiple models. Or, perhaps building on what you suggested -- a View Model that sits between the Base model.

(In reading that over - I perhaps answered my own question and misunderstood your original suggestion).

Thoughts?

@faustinoaq
Copy link
Contributor

faustinoaq commented Apr 26, 2018

What about adding a select macro?

Isn't select a reserved crystal keyword?

select
when channel1.receive
  puts "Channel1!!!"
when channel2.receive
  puts "Channel2!!!"
end

See crystal keywords

@drujensen
Copy link
Member

drujensen commented May 12, 2018

The only limitation I see is that it (effectively) restricts you to one alternate SELECT per model

Right. I think this is a valid restriction for an ORM.

Or, perhaps building on what you suggested -- a View Model that sits between the Base model.

Correct. we will want a View Model that removes the save and destroy methods since this will only be to query and map the results to an object.

Isn't select a reserved crystal keyword?

@faustinoaq yeah, your right. maybe we could use query or something as the macro. Thanks for pointing that out.

@noahlh
Copy link
Contributor Author

noahlh commented May 16, 2018

Cool - all great thoughts - thanks. I'm going to work on the initial portion of this (the select/query/something macro) this weekend and will report back!

@noahlh noahlh mentioned this issue May 19, 2018
@noahlh
Copy link
Contributor Author

noahlh commented May 22, 2018

Update: Initial PR coming soon. Just writing up final new tests & README update and will submit as soon as complete.

@noahlh
Copy link
Contributor Author

noahlh commented Jun 6, 2018

Done & done with #215

@robacarp
Copy link
Member

robacarp commented Jun 9, 2018

Thanks @noahlh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants