From 304df6ac42e93235f3c14518f87ea47f834cde59 Mon Sep 17 00:00:00 2001 From: Nick Franken Date: Tue, 21 Feb 2017 15:26:32 -0600 Subject: [PATCH 1/7] aggregate working, just need to write more tests --- .crystal-version | 2 +- spec/repo_spec.cr | 15 +++++++++++++++ src/crecto/adapters/mysql_adapter.cr | 24 ++++++++++++++++++++++++ src/crecto/adapters/postgres_adapter.cr | 24 ++++++++++++++++++++++++ src/crecto/repo.cr | 16 +++++++++++++++- 5 files changed, 79 insertions(+), 2 deletions(-) diff --git a/.crystal-version b/.crystal-version index 144996e..1db0ede 100644 --- a/.crystal-version +++ b/.crystal-version @@ -1 +1 @@ -0.20.3 +0.21.0 \ No newline at end of file diff --git a/spec/repo_spec.cr b/spec/repo_spec.cr index 59c47c9..02b64b3 100644 --- a/spec/repo_spec.cr +++ b/spec/repo_spec.cr @@ -125,6 +125,21 @@ describe Crecto do end end + describe "#aggregate" do + it "should do things" do + x = Crecto::Repo.aggregate(User, :sum, :id) + puts x + puts x.class + end + + it "should do more things" do + query = Crecto::Repo::Query.where(name: "test") + x = Crecto::Repo.aggregate(User, :count, :id, query) + puts x + puts x.class + end + end + describe "#get" do it "should return a user" do now = Time.now diff --git a/src/crecto/adapters/mysql_adapter.cr b/src/crecto/adapters/mysql_adapter.cr index 6dbd854..459e261 100644 --- a/src/crecto/adapters/mysql_adapter.cr +++ b/src/crecto/adapters/mysql_adapter.cr @@ -60,6 +60,26 @@ module Crecto end end + def self.aggregate(queryable, ag, field) + @@CRECTO_DB = DB.open(ENV["MYSQL_URL"]) if @@CRECTO_DB.nil? + @@CRECTO_DB.as(DB::Database).scalar(build_aggregate_query(queryable, ag, field)) + end + + def self.aggregate(queryable, ag, field, query : Crecto::Repo::Query) + @@CRECTO_DB = DB.open(ENV["MYSQL_URL"]) if @@CRECTO_DB.nil? + + params = [] of DbValue | Array(DbValue) + q = [build_aggregate_query(queryable, ag, field)] + q.push joins(queryable, query, params) if query.joins.any? + q.push wheres(queryable, query, params) if query.wheres.any? + q.push or_wheres(queryable, query, params) if query.or_wheres.any? + q.push order_bys(query) if query.order_bys.any? + q.push limit(query) unless query.limit.nil? + q.push offset(query) unless query.offset.nil? + + @@CRECTO_DB.as(DB::Database).scalar(q.join(" "), params) + end + def self.execute_query(query_string, params) @@CRECTO_DB = DB.open(ENV["MYSQL_URL"]) if @@CRECTO_DB.nil? @@CRECTO_DB.as(DB::Database).query(query_string, params) @@ -89,6 +109,10 @@ module Crecto execute_query(q.join(" "), [id]) end + private def self.build_aggregate_query(queryable, ag, field) + "SELECT #{ag}(#{field}) from #{queryable.table_name}" + end + private def self.all(queryable, query) params = [] of DbValue | Array(DbValue) diff --git a/src/crecto/adapters/postgres_adapter.cr b/src/crecto/adapters/postgres_adapter.cr index c55af8c..7c20032 100644 --- a/src/crecto/adapters/postgres_adapter.cr +++ b/src/crecto/adapters/postgres_adapter.cr @@ -60,6 +60,26 @@ module Crecto end end + def self.aggregate(queryable, ag, field) + @@CRECTO_DB = DB.open(ENV["PG_URL"]) if @@CRECTO_DB.nil? + @@CRECTO_DB.as(DB::Database).scalar(build_aggregate_query(queryable, ag, field)) + end + + def self.aggregate(queryable, ag, field, query : Crecto::Repo::Query) + @@CRECTO_DB = DB.open(ENV["PG_URL"]) if @@CRECTO_DB.nil? + params = [] of DbValue | Array(DbValue) + + q = [build_aggregate_query(queryable, ag, field)] + q.push joins(queryable, query, params) if query.joins.any? + q.push wheres(queryable, query, params) if query.wheres.any? + q.push or_wheres(queryable, query, params) if query.or_wheres.any? + q.push order_bys(query) if query.order_bys.any? + q.push limit(query) unless query.limit.nil? + q.push offset(query) unless query.offset.nil? + + @@CRECTO_DB.as(DB::Database).scalar(position_args(q.join(" ")), params) + end + def self.execute(query_string, params) @@CRECTO_DB = DB.open(ENV["PG_URL"]) if @@CRECTO_DB.nil? @@CRECTO_DB.as(DB::Database).query(query_string, params) @@ -79,6 +99,10 @@ module Crecto execute(q.join(" "), [id]) end + private def self.build_aggregate_query(queryable, ag, field) + "SELECT #{ag}(#{field}) from #{queryable.table_name}" + end + private def self.all(queryable, query) params = [] of DbValue | Array(DbValue) diff --git a/src/crecto/repo.cr b/src/crecto/repo.cr index fd44868..56ad78f 100644 --- a/src/crecto/repo.cr +++ b/src/crecto/repo.cr @@ -233,7 +233,7 @@ module Crecto # Repo.delete_all(User, query) # ``` def self.delete_all(queryable, query = Query.new) - query = ADAPTER.run(:delete_all, queryable, query) + ADAPTER.run(:delete_all, queryable, query) end # Run aribtrary sql queries. `query` will cast the output as that @@ -263,6 +263,20 @@ module Crecto ADAPTER.run(:sql, sql, params).as(DB::ResultSet) end + # Calculate the given aggregate `ag` over the given `field` + # Aggregate `ag` must be one of (:avg, :count, :max, :min:, :sum) + def self.aggregate(queryable, ag : Symbol, field : Symbol | String) + raise InvalidOption.new("Aggregate must be one of :avg, :count, :max, :min:, :sum") unless [:avg, :count, :max, :min, :sum].includes?(ag) + + ADAPTER.aggregate(queryable, ag, field) + end + + def self.aggregate(queryable, ag : Symbol, field : Symbol | String, query : Crecto::Repo::Query) + raise InvalidOption.new("Aggregate must be one of :avg, :count, :max, :min:, :sum") unless [:avg, :count, :max, :min, :sum].includes?(ag) + + ADAPTER.aggregate(queryable, ag, field, query) + end + private def self.add_preloads(results, queryable, preloads) preloads.each do |preload| case queryable.association_type_for_association(preload) From 9495751baf2a37cbbc7ac0b2352a3afb8b98d565 Mon Sep 17 00:00:00 2001 From: Nick Franken Date: Tue, 21 Feb 2017 15:28:59 -0600 Subject: [PATCH 2/7] use crystal-pg master for now --- shard.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shard.yml b/shard.yml index 4010e54..e4d4198 100644 --- a/shard.yml +++ b/shard.yml @@ -18,4 +18,4 @@ development_dependencies: pg: github: will/crystal-pg - version: 0.13.1 \ No newline at end of file + branch: master \ No newline at end of file From a725592ee6060c69af3375ceea029011bd7d6ed3 Mon Sep 17 00:00:00 2001 From: Nick Franken Date: Tue, 21 Feb 2017 15:43:42 -0600 Subject: [PATCH 3/7] tests init, updated readme to include aggregate functions, updated shard to use latest crystal-pg --- README.md | 7 +++++++ shard.yml | 2 +- spec/repo_spec.cr | 49 +++++++++++++++++++++++++++++++++++++--------- src/crecto/repo.cr | 4 ++-- 4 files changed, 50 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 06d7961..540d4d1 100644 --- a/README.md +++ b/README.md @@ -169,6 +169,13 @@ users[0].posts # has_many relation is preloaded posts = Crecto::Repo.all(Post, Crecto::Query.new, preload: [:user]) posts[0].user # belongs_to relation preloaded + +# +# Aggregate functions +# +# can use the following aggregate functions: :avg, :count, :max, :min:, :sum +Crecto::Repo.aggregate(User, :count, :id) +Crecto::Repo.aggregate(User, :avg, :age, Crecto::Repo::Query.where(name: 'Bill')) ``` ## Contributing diff --git a/shard.yml b/shard.yml index e4d4198..27493e1 100644 --- a/shard.yml +++ b/shard.yml @@ -18,4 +18,4 @@ development_dependencies: pg: github: will/crystal-pg - branch: master \ No newline at end of file + version: 0.13.2 \ No newline at end of file diff --git a/spec/repo_spec.cr b/spec/repo_spec.cr index 02b64b3..54fe0f3 100644 --- a/spec/repo_spec.cr +++ b/spec/repo_spec.cr @@ -126,17 +126,48 @@ describe Crecto do end describe "#aggregate" do - it "should do things" do - x = Crecto::Repo.aggregate(User, :sum, :id) - puts x - puts x.class + describe "without a query" do + pending "should return the correct :avg" do + + end + + pending "should return the correct :count" do + + end + + pending "should return the correct :max" do + + end + + pending "should return the correct :min" do + + end + + pending "should return the correct :sum" do + + end end - it "should do more things" do - query = Crecto::Repo::Query.where(name: "test") - x = Crecto::Repo.aggregate(User, :count, :id, query) - puts x - puts x.class + describe "with a query" do + pending "should return the correct :avg" do + + end + + pending "should return the correct :count" do + + end + + pending "should return the correct :max" do + + end + + pending "should return the correct :min" do + + end + + pending "should return the correct :sum" do + + end end end diff --git a/src/crecto/repo.cr b/src/crecto/repo.cr index 56ad78f..1c53696 100644 --- a/src/crecto/repo.cr +++ b/src/crecto/repo.cr @@ -265,13 +265,13 @@ module Crecto # Calculate the given aggregate `ag` over the given `field` # Aggregate `ag` must be one of (:avg, :count, :max, :min:, :sum) - def self.aggregate(queryable, ag : Symbol, field : Symbol | String) + def self.aggregate(queryable, ag : Symbol, field : Symbol) raise InvalidOption.new("Aggregate must be one of :avg, :count, :max, :min:, :sum") unless [:avg, :count, :max, :min, :sum].includes?(ag) ADAPTER.aggregate(queryable, ag, field) end - def self.aggregate(queryable, ag : Symbol, field : Symbol | String, query : Crecto::Repo::Query) + def self.aggregate(queryable, ag : Symbol, field : Symbol, query : Crecto::Repo::Query) raise InvalidOption.new("Aggregate must be one of :avg, :count, :max, :min:, :sum") unless [:avg, :count, :max, :min, :sum].includes?(ag) ADAPTER.aggregate(queryable, ag, field, query) From ccfc4acfc778a0c3d1882e7e0838b973bb4bb398 Mon Sep 17 00:00:00 2001 From: Nick Franken Date: Tue, 21 Feb 2017 18:01:52 -0600 Subject: [PATCH 4/7] aggregate done --- spec/helper_methods.cr | 12 +++ spec/repo_spec.cr | 111 +++++++++++++++++++----- src/crecto/adapters/postgres_adapter.cr | 1 - src/crecto/model.cr | 3 +- 4 files changed, 104 insertions(+), 23 deletions(-) create mode 100644 spec/helper_methods.cr diff --git a/spec/helper_methods.cr b/spec/helper_methods.cr new file mode 100644 index 0000000..f1ab1f4 --- /dev/null +++ b/spec/helper_methods.cr @@ -0,0 +1,12 @@ +def quick_create_user(name) + user = User.new + user.name = name + Crecto::Repo.insert(user).instance +end + +def quick_create_user_with_things(name, things) + user = User.new + user.name = name + user.things = things + Crecto::Repo.insert(user).instance +end diff --git a/spec/repo_spec.cr b/spec/repo_spec.cr index 54fe0f3..7cd7dfe 100644 --- a/spec/repo_spec.cr +++ b/spec/repo_spec.cr @@ -1,4 +1,5 @@ require "./spec_helper" +require "./helper_methods" describe Crecto do describe "Repo" do @@ -127,46 +128,116 @@ describe Crecto do describe "#aggregate" do describe "without a query" do - pending "should return the correct :avg" do - + it "should return the correct :avg" do + Crecto::Repo.delete_all(Post) + Crecto::Repo.delete_all(User) + quick_create_user_with_things("test", 9) + quick_create_user_with_things("test", 10) + quick_create_user_with_things("test", 11) + + Crecto::Repo.aggregate(User, :avg, :things).as(PG::Numeric).to_f.should eq 10.0 end - pending "should return the correct :count" do - + it "should return the correct :count" do + Crecto::Repo.delete_all(Post) + Crecto::Repo.delete_all(User) + quick_create_user_with_things("test", 9) + quick_create_user_with_things("test", 10) + quick_create_user_with_things("test", 11) + + Crecto::Repo.aggregate(User, :count, :id).should eq 3 end - pending "should return the correct :max" do - + it "should return the correct :max" do + Crecto::Repo.delete_all(Post) + Crecto::Repo.delete_all(User) + quick_create_user_with_things("test", 9) + quick_create_user_with_things("test", 10) + quick_create_user_with_things("test", 11) + + Crecto::Repo.aggregate(User, :max, :things).should eq 11 end - pending "should return the correct :min" do - + it "should return the correct :min" do + Crecto::Repo.delete_all(Post) + Crecto::Repo.delete_all(User) + quick_create_user_with_things("test", 9) + quick_create_user_with_things("test", 10) + quick_create_user_with_things("test", 11) + + Crecto::Repo.aggregate(User, :min, :things).should eq 9 end - pending "should return the correct :sum" do - + it "should return the correct :sum" do + Crecto::Repo.delete_all(Post) + Crecto::Repo.delete_all(User) + quick_create_user_with_things("test", 9) + quick_create_user_with_things("test", 10) + quick_create_user_with_things("test", 11) + + Crecto::Repo.aggregate(User, :sum, :things).should eq 30 end end describe "with a query" do - pending "should return the correct :avg" do - + it "should return the correct :avg" do + Crecto::Repo.delete_all(Post) + Crecto::Repo.delete_all(User) + quick_create_user_with_things("test", 9) + quick_create_user_with_things("test", 10) + quick_create_user_with_things("test", 11) + quick_create_user_with_things("nope", 12) + query = Crecto::Repo::Query.where(name: "test") + + Crecto::Repo.aggregate(User, :avg, :things, query).as(PG::Numeric).to_f.should eq 10.0 end - pending "should return the correct :count" do - + it "should return the correct :count" do + Crecto::Repo.delete_all(Post) + Crecto::Repo.delete_all(User) + quick_create_user_with_things("test", 9) + quick_create_user_with_things("test", 10) + quick_create_user_with_things("test", 11) + quick_create_user_with_things("nope", 12) + query = Crecto::Repo::Query.where(name: "test") + + Crecto::Repo.aggregate(User, :count, :id, query).should eq 3 end - pending "should return the correct :max" do - + it "should return the correct :max" do + Crecto::Repo.delete_all(Post) + Crecto::Repo.delete_all(User) + quick_create_user_with_things("test", 9) + quick_create_user_with_things("test", 10) + quick_create_user_with_things("test", 11) + quick_create_user_with_things("nope", 12) + query = Crecto::Repo::Query.where(name: "test") + + Crecto::Repo.aggregate(User, :max, :things, query).should eq 11 end - pending "should return the correct :min" do - + it "should return the correct :min" do + Crecto::Repo.delete_all(Post) + Crecto::Repo.delete_all(User) + quick_create_user_with_things("test", 9) + quick_create_user_with_things("test", 10) + quick_create_user_with_things("test", 11) + quick_create_user_with_things("nope", 12) + query = Crecto::Repo::Query.where(name: "test") + + Crecto::Repo.aggregate(User, :min, :things, query).should eq 9 end - pending "should return the correct :sum" do - + it "should return the correct :sum" do + Crecto::Repo.delete_all(Post) + Crecto::Repo.delete_all(User) + quick_create_user_with_things("test", 9) + quick_create_user_with_things("test", 10) + quick_create_user_with_things("test", 11) + quick_create_user_with_things("nope", 12) + query = Crecto::Repo::Query.where(name: "test") + + Crecto::Repo.aggregate(User, :sum, :things, query).should eq 30 end end end diff --git a/src/crecto/adapters/postgres_adapter.cr b/src/crecto/adapters/postgres_adapter.cr index 7c20032..2f2673b 100644 --- a/src/crecto/adapters/postgres_adapter.cr +++ b/src/crecto/adapters/postgres_adapter.cr @@ -68,7 +68,6 @@ module Crecto def self.aggregate(queryable, ag, field, query : Crecto::Repo::Query) @@CRECTO_DB = DB.open(ENV["PG_URL"]) if @@CRECTO_DB.nil? params = [] of DbValue | Array(DbValue) - q = [build_aggregate_query(queryable, ag, field)] q.push joins(queryable, query, params) if query.joins.any? q.push wheres(queryable, query, params) if query.wheres.any? diff --git a/src/crecto/model.cr b/src/crecto/model.cr index c50564e..e922d36 100644 --- a/src/crecto/model.cr +++ b/src/crecto/model.cr @@ -1,6 +1,5 @@ module Crecto - - #Your data models should extend `Crecto::Model`: + # Your data models should extend `Crecto::Model`: # # `class User < Crecto::Model` # -or- From d6f72edc4cff65f8873b83f41811e54a783a20d3 Mon Sep 17 00:00:00 2001 From: Nick Franken Date: Tue, 21 Feb 2017 18:13:35 -0600 Subject: [PATCH 5/7] dynamic helper method --- spec/helper_methods.cr | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/spec/helper_methods.cr b/spec/helper_methods.cr index f1ab1f4..dde2fc4 100644 --- a/spec/helper_methods.cr +++ b/spec/helper_methods.cr @@ -4,9 +4,11 @@ def quick_create_user(name) Crecto::Repo.insert(user).instance end -def quick_create_user_with_things(name, things) +{% for x in ["things", "nope", "yep", "some_date", "pageviews"] %} +def quick_create_user_with_{{x.id}}(name, {{x.id}}) user = User.new user.name = name - user.things = things + user.{{x.id}} = {{x.id}} Crecto::Repo.insert(user).instance end +{% end %} From ce33361e4883efcee9ff732149fbcd47ce5f9246 Mon Sep 17 00:00:00 2001 From: Nick Franken Date: Tue, 21 Feb 2017 18:22:20 -0600 Subject: [PATCH 6/7] added test case for error --- README.md | 1 + spec/repo_spec.cr | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/README.md b/README.md index 540d4d1..ff9fd5c 100644 --- a/README.md +++ b/README.md @@ -61,6 +61,7 @@ require "crecto" - [ ] Transactions / Multi - [ ] Association / dependent options (`dependent: :delete_all`, `dependent: :nilify_all`, etc) - [ ] Unique constraint +- [ ] Combine database adapters (base class). Currently there is unecessary duplication ## Usage diff --git a/spec/repo_spec.cr b/spec/repo_spec.cr index 7cd7dfe..f2a85c7 100644 --- a/spec/repo_spec.cr +++ b/spec/repo_spec.cr @@ -127,6 +127,12 @@ describe Crecto do end describe "#aggregate" do + it "should raise InvalidOption with an invalid option" do + expect_raises Crecto::InvalidOption do + Crecto::Repo.aggregate(User, :blurb, :id) + end + end + describe "without a query" do it "should return the correct :avg" do Crecto::Repo.delete_all(Post) From ee8eb3ce72a1cf05f519e7dff79bfe4d0cc2a3bd Mon Sep 17 00:00:00 2001 From: Nick Franken Date: Tue, 21 Feb 2017 18:34:37 -0600 Subject: [PATCH 7/7] explicit json require, necessary for mysql users --- src/crecto.cr | 1 + 1 file changed, 1 insertion(+) diff --git a/src/crecto.cr b/src/crecto.cr index b117257..f19ec52 100644 --- a/src/crecto.cr +++ b/src/crecto.cr @@ -1,3 +1,4 @@ +require "json" require "./crecto/errors/*" require "./crecto/schema/*" require "./crecto/adapters/*"