From 66eeb784fbec4ecfe79b4fd43365a0d484af5722 Mon Sep 17 00:00:00 2001 From: Yacine Petitprez Date: Mon, 13 Aug 2018 07:54:34 +0700 Subject: [PATCH] Fix #28 : Add double quote around schema, table and column name to allow reserved keywords usage on it. --- CHANGELOG.md | 16 +++- spec/extensions/jsonb_spec.cr | 22 ++--- spec/model/cache_spec.cr | 2 +- spec/model/model_spec.cr | 22 ++--- spec/model/scope_spec.cr | 6 +- spec/sql/delete_spec.cr | 22 ++--- spec/sql/insert_spec.cr | 10 +-- spec/sql/select_spec.cr | 106 ++++++++++++----------- src/clear/expression/expression.cr | 22 +++-- src/clear/expression/nodes/function.cr | 11 +++ src/clear/expression/nodes/raw.cr | 10 +++ src/clear/expression/nodes/variable.cr | 6 +- src/clear/model/collection.cr | 6 +- src/clear/model/modules/class_methods.cr | 16 +++- src/clear/model/modules/has_relations.cr | 25 +++--- src/clear/model/reflection/column.cr | 3 +- src/clear/model/reflection/table.cr | 42 ++++----- src/clear/sql/delete_query.cr | 7 +- src/clear/sql/fragment/column.cr | 4 +- src/clear/sql/fragment/from.cr | 4 +- src/clear/sql/insert_query.cr | 17 ++-- src/clear/sql/logger.cr | 2 +- src/clear/sql/query/group_by.cr | 9 +- src/clear/sql/query/join.cr | 2 + src/clear/sql/query/order_by.cr | 16 ++-- src/clear/sql/query/where.cr | 10 +-- src/clear/sql/select_builder.cr | 2 +- src/clear/sql/sql.cr | 13 ++- 28 files changed, 261 insertions(+), 172 deletions(-) create mode 100644 src/clear/expression/nodes/function.cr create mode 100644 src/clear/expression/nodes/raw.cr diff --git a/CHANGELOG.md b/CHANGELOG.md index 90de99b8f..788302d20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,11 +3,25 @@ ## Features - Improved Model build/create methods, allowing to pass arguments instead of NamedTuple - ## Bug fixes +- Escaping table, columns and schema name to allow Clear to works on any model names. + - This is very demanding work as it turns out table and columns naming are used everywhere + in the ORM. Please give me feedback in case of any issues ! ## Breaking changes - Renaming `insert` method on `InsertQuery` to `values`, making API more elegant. +- Usage of `var` in Expression engine has been changed and is now different from raw: + - `var` provide simple way to construct `[schema].table.field` structure, + with escaped table, field and schema keywords. + - `raw` works as usual, printing the raw string fragment to you condition. + - Therefore: + ```crystal + where{ var("a.b") == 1 } # Wrong now! => WHERE "a.b" = 1 + # Must be changed by: + where{ var("a", "b") == 1 } # OR + where{ raw("a.b") } + ``` + TL;DR, if you currently use `var` function, please use `raw` instead from now. # v0.3 diff --git a/spec/extensions/jsonb_spec.cr b/spec/extensions/jsonb_spec.cr index 9082fd973..903ff7947 100644 --- a/spec/extensions/jsonb_spec.cr +++ b/spec/extensions/jsonb_spec.cr @@ -46,48 +46,48 @@ module JSONBSpec it "use -> operator when it cannot test presence" do Clear::SQL.select("*").from("users") .where { data.jsonb("personal email").cast("text").like "%@gmail.com" }.to_sql - .should eq %(SELECT * FROM users WHERE (data->'personal email'::text LIKE '%@gmail.com')) + .should eq %(SELECT * FROM users WHERE ("data"->'personal email'::text LIKE '%@gmail.com')) # v-- Complex call Clear::SQL.select.from("users") .where { data.jsonb("test") == call_function(data.jsonb("a.b.c")) }.to_sql - .should eq %(SELECT * FROM users WHERE (data->'test' = call_function(data->'a'->'b'->'c'))) + .should eq %(SELECT * FROM users WHERE ("data"->'test' = call_function("data"->'a'->'b'->'c'))) end it "uses @> operator when it can !" do - Clear::SQL.select("*").from("users") + Clear::SQL.select("*").from(:users) .where { (data.jsonb("security.role") == "admin") & (data.jsonb("security.level") == 1) }.to_sql - .should eq "SELECT * FROM users WHERE (data @> '{\"security\":{\"role\":\"admin\"}}' AND " + - "data @> '{\"security\":{\"level\":1}}')" + .should eq "SELECT * FROM \"users\" WHERE (\"data\" @> '{\"security\":{\"role\":\"admin\"}}' AND " + + "\"data\" @> '{\"security\":{\"level\":1}}')" end it "check existence of a key" do - Clear::SQL.select.from("users") + Clear::SQL.select.from(:users) .where { data.jsonb_key_exists?("test") } .to_sql - .should eq "SELECT * FROM users WHERE (data ? 'test')" + .should eq "SELECT * FROM \"users\" WHERE (\"data\" ? 'test')" Clear::SQL.select.from("users") .where { data.jsonb("a").jsonb_key_exists?("test") } .to_sql - .should eq "SELECT * FROM users WHERE (data->'a' ? 'test')" + .should eq "SELECT * FROM users WHERE (\"data\"->'a' ? 'test')" end it "check existence of any key" do - Clear::SQL.select.from("users") + Clear::SQL.select.from(:users) .where { data.jsonb_any_key_exists?(["a", 0]) } .to_sql - .should eq "SELECT * FROM users WHERE (data ?| array['a', 0])" + .should eq "SELECT * FROM \"users\" WHERE (\"data\" ?| array['a', 0])" end it "check existence of all keys" do Clear::SQL.select.from("users") .where { data.jsonb("a").jsonb_all_keys_exists?(["a", "b"]) } .to_sql - .should eq "SELECT * FROM users WHERE (data->'a' ?& array['a', 'b'])" + .should eq "SELECT * FROM users WHERE (\"data\"->'a' ?& array['a', 'b'])" end end end diff --git a/spec/model/cache_spec.cr b/spec/model/cache_spec.cr index ebcf01b8f..5bc1f8cb0 100644 --- a/spec/model/cache_spec.cr +++ b/spec/model/cache_spec.cr @@ -55,7 +55,7 @@ module CacheSpec end Clear::Model::QueryCache.cache_hitted.should eq(4) # Number of posts - Category.query.with_users { |q| q.order_by("users.id": :asc) }.order_by("id": :asc).each do |c| + Category.query.with_users { |q| q.order_by("users.id", "asc") }.order_by("id", "asc").each do |c| c.users.each do |user| c.id.not_nil! user.id.not_nil! diff --git a/spec/model/model_spec.cr b/spec/model/model_spec.cr index 5d6d8794b..0e50e230f 100644 --- a/spec/model/model_spec.cr +++ b/spec/model/model_spec.cr @@ -414,12 +414,12 @@ module ModelSpec p = Post.create!({title: "Post about Dogs", user_id: u.id, category_id: c.id}) # Categories should return 1, as we remove duplicate + u.categories.to_sql.should eq "SELECT DISTINCT ON (\"model_categories\".\"id\") \"model_categories\".* " + + "FROM \"model_categories\" " + + "INNER JOIN \"model_posts\" ON " + + "((\"model_posts\".\"category_id\" = \"model_categories\".\"id\")) " + + "WHERE (\"model_posts\".\"user_id\" = 1)" u.categories.count.should eq(1) - u.categories.to_sql.should eq "SELECT DISTINCT ON (model_categories.id) model_categories.* " + - "FROM model_categories " + - "INNER JOIN model_posts ON " + - "((model_posts.category_id = model_categories.id)) " + - "WHERE (model_posts.user_id = 1)" end end end @@ -433,9 +433,9 @@ module ModelSpec Post.create!({title: "A Post", user_id: u.id}) - Post.query.join("model_users") { model_posts.user_id == model_users.id }.to_sql - .should eq "SELECT model_posts.* FROM model_posts INNER JOIN model_users " + - "ON ((model_posts.user_id = model_users.id))" + Post.query.join(:model_users) { model_posts.user_id == model_users.id }.to_sql + .should eq "SELECT \"model_posts\".* FROM \"model_posts\" INNER JOIN \"model_users\" " + + "ON ((\"model_posts\".\"user_id\" = \"model_users\".\"id\"))" end end @@ -445,11 +445,11 @@ module ModelSpec u = User.create!({first_name: "Join User"}) Post.create!({title: "A Post", user_id: u.id}) - user_with_a_post_minimum = User.query.distinct.join("model_posts") { model_posts.user_id == model_users.id } + user_with_a_post_minimum = User.query.distinct.join(:model_posts) { model_posts.user_id == model_users.id } user_with_a_post_minimum.to_sql.should eq \ - "SELECT DISTINCT model_users.* FROM model_users INNER JOIN " + - "model_posts ON ((model_posts.user_id = model_users.id))" + "SELECT DISTINCT \"model_users\".* FROM \"model_users\" INNER JOIN " + + "\"model_posts\" ON ((\"model_posts\".\"user_id\" = \"model_users\".\"id\"))" user_with_a_post_minimum.with_posts.each { } # Should just execute end diff --git a/spec/model/scope_spec.cr b/spec/model/scope_spec.cr index 65ed148a0..99934eeb1 100644 --- a/spec/model/scope_spec.cr +++ b/spec/model/scope_spec.cr @@ -45,11 +45,11 @@ module ScopeSpec ScopeModel.create! #Without value - ScopeModel.no_value.to_sql.should eq("SELECT * FROM scope_models WHERE (value IS NULL)") + ScopeModel.no_value.to_sql.should eq("SELECT * FROM \"scope_models\" WHERE (\"value\" IS NULL)") ScopeModel.no_value.count.should eq 1 - ScopeModel.with_value(1).to_sql.should eq("SELECT * FROM scope_models WHERE (value = 1)") + ScopeModel.with_value(1).to_sql.should eq("SELECT * FROM \"scope_models\" WHERE (\"value\" = 1)") ScopeModel.with_value(1).count.should eq 1 - ScopeModel.with_values(1,2,3).where{id < 10}.to_sql.should eq("SELECT * FROM scope_models WHERE value IN (1, 2, 3) AND (id < 10)") + ScopeModel.with_values(1,2,3).where{id < 10}.to_sql.should eq("SELECT * FROM \"scope_models\" WHERE \"value\" IN (1, 2, 3) AND (\"id\" < 10)") ScopeModel.with_values(1,2,3).count.should eq 3 end diff --git a/spec/sql/delete_spec.cr b/spec/sql/delete_spec.cr index f8d9e1e5f..528136661 100644 --- a/spec/sql/delete_spec.cr +++ b/spec/sql/delete_spec.cr @@ -15,11 +15,11 @@ module DeleteSpec def complex_query Clear::SQL.select.from(:users) - .join(:role_users) { role_users.user_id == users.id } - .join(:roles) { role_users.role_id == roles.id } - .where({role: ["admin", "superadmin"]}) - .order_by({priority: :desc, name: :asc}) - .limit(1) + .join(:role_users) { role_users.user_id == users.id } + .join(:roles) { role_users.role_id == roles.id } + .where({role: ["admin", "superadmin"]}) + .order_by({priority: :desc, name: :asc}) + .limit(1) end describe "Clear::SQL" do @@ -30,12 +30,12 @@ module DeleteSpec end it "can create a delete with where parameter" do - r = delete_request.from("table").where({id: complex_query}) - r.to_sql.should eq "DELETE FROM table WHERE id IN (SELECT * " + - "FROM users " + - "INNER JOIN role_users ON ((role_users.user_id = users.id)) " + - "INNER JOIN roles ON ((role_users.role_id = roles.id)) " + - "WHERE role IN ('admin', 'superadmin') " + + r = delete_request.from(:table).where({id: complex_query}) + r.to_sql.should eq "DELETE FROM \"table\" WHERE \"id\" IN (SELECT * " + + "FROM \"users\" " + + "INNER JOIN \"role_users\" ON ((\"role_users\".\"user_id\" = \"users\".\"id\")) " + + "INNER JOIN \"roles\" ON ((\"role_users\".\"role_id\" = \"roles\".\"id\")) " + + "WHERE \"role\" IN ('admin', 'superadmin') " + "ORDER BY priority DESC, name ASC " + "LIMIT 1)" end diff --git a/spec/sql/insert_spec.cr b/spec/sql/insert_spec.cr index 5a5f543ad..09be8d30b 100644 --- a/spec/sql/insert_spec.cr +++ b/spec/sql/insert_spec.cr @@ -13,29 +13,29 @@ module InsertSpec describe "InsertQuery" do it "can build an insert" do insert_request.values({a: "c", b: 12}).to_sql.should eq( - "INSERT INTO users (a, b) VALUES ('c', 12)" + "INSERT INTO \"users\" (\"a\", \"b\") VALUES ('c', 12)" ) end it "can build an insert from sql" do insert_request.values( - Clear::SQL.select.from("old_users") + Clear::SQL.select.from(:old_users) .where { old_users.id > 100 } ).to_sql.should eq ( - "INSERT INTO users (SELECT * FROM old_users WHERE (old_users.id > 100))" + "INSERT INTO \"users\" (SELECT * FROM \"old_users\" WHERE (\"old_users\".\"id\" > 100))" ) end it "can build an empty insert?" do insert_request.to_sql.should eq ( - "INSERT INTO users DEFAULT VALUES" + "INSERT INTO \"users\" DEFAULT VALUES" ) end it "can insert unsafe values" do insert_request.values({created_at: Clear::Expression.unsafe("NOW()")}) .to_sql - .should eq "INSERT INTO users (created_at) VALUES (NOW())" + .should eq "INSERT INTO \"users\" (\"created_at\") VALUES (NOW())" end end end diff --git a/spec/sql/select_spec.cr b/spec/sql/select_spec.cr index dc2095a31..613db6e1a 100644 --- a/spec/sql/select_spec.cr +++ b/spec/sql/select_spec.cr @@ -17,8 +17,8 @@ module SelectSpec def complex_query select_request.from(:users) - .join(:role_users) { var("role_users.user_id") == users.id } - .join(:roles) { var("role_users.role_id") == var("roles.id") } + .join(:role_users) { var("role_users", "user_id") == users.id } + .join(:roles) { var("role_users", "role_id") == var("roles", "id") } .where({role: ["admin", "superadmin"]}) .order_by({priority: :desc, name: :asc}) .limit(50) @@ -39,7 +39,7 @@ module SelectSpec it "can transfert to delete method" do r = select_request.select("*").from(:users).where { raw("users.id") > 1000 } - r.to_delete.to_sql.should eq "DELETE FROM users WHERE (users.id > 1000)" + r.to_delete.to_sql.should eq "DELETE FROM \"users\" WHERE (users.id > 1000)" end describe "the SELECT clause" do @@ -54,12 +54,13 @@ module SelectSpec end it "can select using variables" do - r = select_request.select({sum: "SUM(quantity)", count: "COUNT(*)"}) + r = select_request.select("SUM(quantity) AS sum", "COUNT(*) AS count") + # No escape with string, escape must be done manually r.to_sql.should eq "SELECT SUM(quantity) AS sum, COUNT(*) AS count" end it "can select using multiple strings" do - r = select_request.select("user_id AS uid", "column AS some_cool_stuff") + r = select_request.select({uid: "user_id", some_cool_stuff: "column"}) r.to_sql.should eq "SELECT user_id AS uid, column AS some_cool_stuff" end @@ -77,12 +78,12 @@ module SelectSpec describe "the FROM clause" do it "can build simple from" do r = select_request.from(:users) - r.to_sql.should eq "SELECT * FROM users" + r.to_sql.should eq "SELECT * FROM \"users\"" end it "can build multiple from" do r = select_request.from(:users, :posts) - r.to_sql.should eq "SELECT * FROM users, posts" + r.to_sql.should eq "SELECT * FROM \"users\", \"posts\"" end it "can build named from" do @@ -101,8 +102,8 @@ module SelectSpec end it "can stack" do - r = select_request.from("x").from("y") - r.to_sql.should eq "SELECT * FROM x, y" + r = select_request.from("x").from(:y) + r.to_sql.should eq "SELECT * FROM x, \"y\"" end it "can be cleared" do @@ -123,15 +124,15 @@ module SelectSpec # Simple CTE cte = select_request.from(:users_info).where("x > 10") sql = select_request.from(:ui).with_cte("ui", cte).to_sql - sql.should eq "WITH ui AS (SELECT * FROM users_info WHERE x > 10) SELECT * FROM ui" + sql.should eq "WITH ui AS (SELECT * FROM \"users_info\" WHERE x > 10) SELECT * FROM \"ui\"" # Complex CTE cte1 = select_request.from(:users_info).where { a == b } cte2 = select_request.from(:just_another_table).where { users_infos.x == just_another_table.w } sql = select_request.with_cte({ui: cte1, at: cte2}).from(:at).to_sql - sql.should eq "WITH ui AS (SELECT * FROM users_info WHERE (a = b))," + - " at AS (SELECT * FROM just_another_table WHERE (" + - "users_infos.x = just_another_table.w)) SELECT * FROM at" + sql.should eq "WITH ui AS (SELECT * FROM \"users_info\" WHERE (\"a\" = \"b\"))," + + " at AS (SELECT * FROM \"just_another_table\" WHERE (" + + "\"users_infos\".\"x\" = \"just_another_table\".\"w\")) SELECT * FROM \"at\"" end end @@ -139,33 +140,33 @@ module SelectSpec context "using simple engine" do it "can use simple equals" do r = select_request.from(:users).where({user_id: 1}) - r.to_sql.should eq "SELECT * FROM users WHERE (user_id = 1)" + r.to_sql.should eq "SELECT * FROM \"users\" WHERE (\"user_id\" = 1)" end it "can use `in` operators in case of array" do r = select_request.from(:users).where({user_id: [1, 2, 3, 4, "hello"]}) - r.to_sql.should eq "SELECT * FROM users WHERE user_id IN (1, 2, 3, 4, 'hello')" + r.to_sql.should eq "SELECT * FROM \"users\" WHERE \"user_id\" IN (1, 2, 3, 4, 'hello')" end it "can write where with string" do r = select_request.from(:users).where("a = b") - r.to_sql.should eq "SELECT * FROM users WHERE a = b" + r.to_sql.should eq "SELECT * FROM \"users\" WHERE a = b" end it "manages ranges" do select_request.from(:users).where({x: 1..4}).to_sql - .should eq "SELECT * FROM users WHERE (x >= 1 AND x <= 4)" + .should eq "SELECT * FROM \"users\" WHERE (\"x\" >= 1 AND \"x\" <= 4)" select_request.from(:users).where({x: 1...4}).to_sql - .should eq "SELECT * FROM users WHERE (x >= 1 AND x < 4)" + .should eq "SELECT * FROM \"users\" WHERE (\"x\" >= 1 AND \"x\" < 4)" end it "can prepare query" do r = select_request.from(:users).where("a LIKE ?", ["hello"]) - r.to_sql.should eq "SELECT * FROM users WHERE a LIKE 'hello'" + r.to_sql.should eq "SELECT * FROM \"users\" WHERE a LIKE 'hello'" r = select_request.from(:users).where("a LIKE ?", {"hello"}) - r.to_sql.should eq "SELECT * FROM users WHERE a LIKE 'hello'" + r.to_sql.should eq "SELECT * FROM \"users\" WHERE a LIKE 'hello'" end it "raises exception with prepared query" do @@ -177,7 +178,7 @@ module SelectSpec it "can prepare query with tuple" do r = select_request.from(:users).where("a LIKE :hello AND b LIKE :world", {hello: "h", world: "w"}) - r.to_sql.should eq "SELECT * FROM users WHERE a LIKE 'h' AND b LIKE 'w'" + r.to_sql.should eq "SELECT * FROM \"users\" WHERE a LIKE 'h' AND b LIKE 'w'" end it "raises exception if a tuple element is not found" do @@ -188,78 +189,78 @@ module SelectSpec end it "can prepare group by query" do - select_request.select("role").from(:users).group_by("role").order_by("role").to_sql.should eq \ - "SELECT role FROM users GROUP BY role ORDER BY role ASC" + select_request.select("role").from(:users).group_by(:role).order_by(:role).to_sql.should eq \ + "SELECT role FROM \"users\" GROUP BY \"role\" ORDER BY \"role\" ASC" end end context "using expression engine" do it "can use different comparison and arithmetic operators" do r = select_request.from(:users).where { users.id > 1 } - r.to_sql.should eq "SELECT * FROM users WHERE (users.id > 1)" + r.to_sql.should eq "SELECT * FROM \"users\" WHERE (\"users\".\"id\" > 1)" r = select_request.from(:users).where { users.id < 1 } - r.to_sql.should eq "SELECT * FROM users WHERE (users.id < 1)" + r.to_sql.should eq "SELECT * FROM \"users\" WHERE (\"users\".\"id\" < 1)" r = select_request.from(:users).where { users.id >= 1 } - r.to_sql.should eq "SELECT * FROM users WHERE (users.id >= 1)" + r.to_sql.should eq "SELECT * FROM \"users\" WHERE (\"users\".\"id\" >= 1)" r = select_request.from(:users).where { users.id <= 1 } - r.to_sql.should eq "SELECT * FROM users WHERE (users.id <= 1)" + r.to_sql.should eq "SELECT * FROM \"users\" WHERE (\"users\".\"id\" <= 1)" r = select_request.from(:users).where { users.id * 2 == 1 } - r.to_sql.should eq "SELECT * FROM users WHERE ((users.id * 2) = 1)" + r.to_sql.should eq "SELECT * FROM \"users\" WHERE ((\"users\".\"id\" * 2) = 1)" r = select_request.from(:users).where { users.id / 2 == 1 } - r.to_sql.should eq "SELECT * FROM users WHERE ((users.id / 2) = 1)" + r.to_sql.should eq "SELECT * FROM \"users\" WHERE ((\"users\".\"id\" / 2) = 1)" r = select_request.from(:users).where { users.id + 2 == 1 } - r.to_sql.should eq "SELECT * FROM users WHERE ((users.id + 2) = 1)" + r.to_sql.should eq "SELECT * FROM \"users\" WHERE ((\"users\".\"id\" + 2) = 1)" r = select_request.from(:users).where { users.id - 2 == 1 } - r.to_sql.should eq "SELECT * FROM users WHERE ((users.id - 2) = 1)" + r.to_sql.should eq "SELECT * FROM \"users\" WHERE ((\"users\".\"id\" - 2) = 1)" r = select_request.from(:users).where { -users.id < -1000 } - r.to_sql.should eq "SELECT * FROM users WHERE (-users.id < -1000)" + r.to_sql.should eq "SELECT * FROM \"users\" WHERE (-\"users\".\"id\" < -1000)" end it "can use expression engine equal" do r = select_request.from(:users).where { users.id == var("test") } - r.to_sql.should eq "SELECT * FROM users WHERE (users.id = test)" + r.to_sql.should eq "SELECT * FROM \"users\" WHERE (\"users\".\"id\" = \"test\")" end it "can use expression engine not equals" do r = select_request.from(:users).where { users.id != 1 } - r.to_sql.should eq "SELECT * FROM users WHERE (users.id <> 1)" + r.to_sql.should eq "SELECT * FROM \"users\" WHERE (\"users\".\"id\" <> 1)" end it "can use expression engine not null" do r = select_request.from(:users).where { users.id != nil } - r.to_sql.should eq "SELECT * FROM users WHERE (users.id IS NOT NULL)" + r.to_sql.should eq "SELECT * FROM \"users\" WHERE (\"users\".\"id\" IS NOT NULL)" end it "can use expression engine null" do r = select_request.from(:users).where { users.id == nil } - r.to_sql.should eq "SELECT * FROM users WHERE (users.id IS NULL)" + r.to_sql.should eq "SELECT * FROM \"users\" WHERE (\"users\".\"id\" IS NULL)" end it "can stack with `AND` operator" do now = Time.now r = select_request.from(:users).where { users.id == nil }.where { - var("users.updated_at") >= now + var("users", "updated_at") >= now } - r.to_sql.should eq "SELECT * FROM users WHERE (users.id IS NULL) " + - "AND (users.updated_at >= #{Clear::Expression[now]})" + r.to_sql.should eq "SELECT * FROM \"users\" WHERE (\"users\".\"id\" IS NULL) " + + "AND (\"users\".\"updated_at\" >= #{Clear::Expression[now]})" end it "can use subquery into where clause" do r = select_request.from(:users).where { users.id.in?(complex_query.clear_select.select(:id)) } - r.to_sql.should eq "SELECT * FROM users WHERE users.id IN (" + - "SELECT id FROM users INNER JOIN role_users ON " + - "((role_users.user_id = users.id)) INNER JOIN roles" + - " ON ((role_users.role_id = roles.id)) WHERE role IN" + + r.to_sql.should eq "SELECT * FROM \"users\" WHERE \"users\".\"id\" IN (" + + "SELECT \"id\" FROM \"users\" INNER JOIN \"role_users\" ON " + + "((\"role_users\".\"user_id\" = \"users\".\"id\")) INNER JOIN \"roles\"" + + " ON ((\"role_users\".\"role_id\" = \"roles\".\"id\")) WHERE \"role\" IN" + " ('admin', 'superadmin') ORDER BY priority DESC, " + "name ASC LIMIT 50 OFFSET 50)" end it "can build locks" do r = select_request.from(:users).with_lock("FOR UPDATE") - r.to_sql.should eq "SELECT * FROM users FOR UPDATE" + r.to_sql.should eq "SELECT * FROM \"users\" FOR UPDATE" r = select_request.from(:users).with_lock("FOR SHARE") - r.to_sql.should eq "SELECT * FROM users FOR SHARE" + r.to_sql.should eq "SELECT * FROM \"users\" FOR SHARE" end it "can use & as AND and | as OR" do @@ -268,31 +269,32 @@ module SelectSpec (raw("users.role") == "superadmin") } - r.to_sql.should eq "SELECT * FROM users WHERE (((users.id > 100) " + + r.to_sql.should eq "SELECT * FROM \"users\" WHERE (((users.id > 100) " + "AND (users.visible = TRUE)) OR (users.role = 'superadmin'))" end it "can check presence into array" do r = select_request.from(:users).where { raw("users.id").in?([1, 2, 3, 4]) } - r.to_sql.should eq "SELECT * FROM users WHERE users.id IN (1, 2, 3, 4)" + r.to_sql.should eq "SELECT * FROM \"users\" WHERE users.id IN (1, 2, 3, 4)" end it "can check presence into range" do # Simple number select_request.from(:users).where { users.id.in?(1..3) }.to_sql - .should eq "SELECT * FROM users WHERE (users.id >= 1 AND users.id <= 3)" + .should eq "SELECT * FROM \"users\" WHERE (\"users\".\"id\" >= 1 AND \"users\".\"id\" <= 3)" # Date range. range = 2.day.ago..1.day.ago select_request.from(:users).where { created_at.in?(range) }.to_sql - .should eq "SELECT * FROM users WHERE " + - "(created_at >= #{Clear::Expression[range.begin]} AND" + - " created_at <= #{Clear::Expression[range.end]})" + .should eq "SELECT * FROM \"users\" WHERE " + + "(\"created_at\" >= #{Clear::Expression[range.begin]} AND" + + " \"created_at\" <= #{Clear::Expression[range.end]})" # Exclusive range select_request.from(:users).where { users.id.in?(1...3) }.to_sql - .should eq "SELECT * FROM users WHERE (users.id >= 1 AND users.id < 3)" + .should eq "SELECT * FROM \"users\" WHERE (\"users\".\"id\" >= 1" + + " AND \"users\".\"id\" < 3)" end end end diff --git a/src/clear/expression/expression.cr b/src/clear/expression/expression.cr index 0723320cc..6d5940a04 100644 --- a/src/clear/expression/expression.cr +++ b/src/clear/expression/expression.cr @@ -206,14 +206,22 @@ class Clear::Expression # IDEA: raw should accept array splat as second parameters and the "?" keyword # def raw(x) - Node::Variable.new(x.to_s) + Node::Raw.new(x.to_s) end - # Alias for `raw` + # Flag the content as variable. + # Variables are escaped with double quotes # - # See `raw` - def var(x) - raw(x) + def var(*parts) + _var(parts) + end + + private def _var(parts : Tuple, pos = parts.size-1) + if pos == 0 + Node::Variable.new(parts[pos].to_s) + else + Node::Variable.new(parts[pos].to_s, _var(parts, pos-1)) + end end # Because many postgresql operators are not transcriptable in Crystal lang, @@ -230,8 +238,8 @@ class Clear::Expression macro method_missing(call) {% if call.args.size > 0 %} - args = {{call.args}}.map{ |x| Clear::Expression[x] }.join(", ") - return Node::Variable.new("{{call.name.id}}(#{args})") + args = {{call.args}}.map{ |x| Clear::Expression[x] } + return Node::Function.new("{{call.name.id}}", args) {% else %} return Node::Variable.new({{call.name.id.stringify}}) {% end %} diff --git a/src/clear/expression/nodes/function.cr b/src/clear/expression/nodes/function.cr new file mode 100644 index 000000000..d9dbb9ef5 --- /dev/null +++ b/src/clear/expression/nodes/function.cr @@ -0,0 +1,11 @@ +require "./node" + +# +# +class Clear::Expression::Node::Function < Clear::Expression::Node + def initialize( @name : String, @args : Array(String) ); end + + def resolve + {@name, "(", @args.join(", "), ")"}.join + end +end diff --git a/src/clear/expression/nodes/raw.cr b/src/clear/expression/nodes/raw.cr new file mode 100644 index 000000000..5bb3bf996 --- /dev/null +++ b/src/clear/expression/nodes/raw.cr @@ -0,0 +1,10 @@ +require "./node" + +# Raw unsafe SQL fragment. +class Clear::Expression::Node::Raw < Clear::Expression::Node + def initialize(@raw : String); end + + def resolve + @raw + end +end diff --git a/src/clear/expression/nodes/variable.cr b/src/clear/expression/nodes/variable.cr index 0e9531cbb..2a5213763 100644 --- a/src/clear/expression/nodes/variable.cr +++ b/src/clear/expression/nodes/variable.cr @@ -15,8 +15,6 @@ require "./node" # # ``` class Clear::Expression::Node::Variable < Clear::Expression::Node - def initialize(@a : String); end - def initialize(@name : String, @parent : Variable? = nil); end macro method_missing(call) @@ -31,9 +29,9 @@ class Clear::Expression::Node::Variable < Clear::Expression::Node def resolve parent = @parent if parent - {parent.resolve, ".", @name}.join + {parent.resolve, ".", Clear::SQL.escape(@name)}.join else # nil - @name + Clear::SQL.escape(@name) end end end diff --git a/src/clear/model/collection.cr b/src/clear/model/collection.cr index c573c085b..7960456b4 100644 --- a/src/clear/model/collection.cr +++ b/src/clear/model/collection.cr @@ -34,7 +34,7 @@ module Clear::Model @wheres = [] of Clear::Expression::Node, @havings = [] of Clear::Expression::Node, @windows = [] of {String, String}, - @group_bys = [] of String, + @group_bys = [] of SQL::Symbolic, @order_bys = [] of Clear::SQL::Query::OrderBy::Record, @limit = nil, @offset = nil, @@ -263,7 +263,7 @@ module Clear::Model # Get the first row from the collection query. # if not found, return `nil` def first(fetch_columns = false) : T? - order_by("#{T.pkey}", "ASC") unless T.pkey.nil? || order_bys.any? + order_by(Clear::SQL.escape("#{T.pkey}"), "ASC") unless T.pkey.nil? || order_bys.any? limit(1).fetch do |hash| return T.factory.build(hash, persisted: true, cache: @cache, fetch_columns: fetch_columns) @@ -281,7 +281,7 @@ module Clear::Model protected def join_impl(name, type, clear_expr) # TODO: not sure about that... if @columns.empty? - self.select("#{T.table}.*") + self.select("#{Clear::SQL.escape(T.table)}.*") end super(name, type, clear_expr) diff --git a/src/clear/model/modules/class_methods.cr b/src/clear/model/modules/class_methods.cr index 6b5a38138..fe898ee20 100644 --- a/src/clear/model/modules/class_methods.cr +++ b/src/clear/model/modules/class_methods.cr @@ -15,12 +15,26 @@ module Clear::Model::ClassMethods class_property table : Clear::SQL::Symbolic = self.name.underscore.gsub(/::/, "_").pluralize + # Schema of this model + class_property schema : Clear::SQL::Symbolic? = nil + + # Compose the "schema"."table" key for PG + def self.esc_schema_table + if s = schema + {schema, table}.map{ |x| Clear::SQL.escape(x.to_s) }.join(".") + else + # Default schema + Clear::SQL.escape(table) + end + + end + class_property pkey : String = "id" class Collection < Clear::Model::CollectionBase(\{{@type}}); end def self.query - Collection.new.use_connection(connection).from(table) + Collection.new.use_connection(connection).from(self.esc_schema_table) end def self.find(x) diff --git a/src/clear/model/modules/has_relations.cr b/src/clear/model/modules/has_relations.cr index 733b1e51e..be5a368d5 100644 --- a/src/clear/model/modules/has_relations.cr +++ b/src/clear/model/modules/has_relations.cr @@ -51,14 +51,17 @@ module Clear::Model::HasRelations %primary_key = {{(primary_key || "#{relation_type}.pkey").id}} %foreign_key = {{foreign_key}} || ( {{@type}}.table.to_s.singularize + "_id" ) + %table = {{@type}}.esc_schema_table #SELECT * FROM foreign WHERE foreign_key IN ( SELECT primary_key FROM users ) - sub_query = self.dup.clear_select.select("#{{{@type}}.table}.#{%primary_key}") + sub_query = self.dup.clear_select.select( + { %table, Clear::SQL.escape(%primary_key) }.join(".") + ) @cache.active "{{method_name}}" {{relation_type}}.query.where{ raw(%foreign_key).in?(sub_query) }.each(fetch_columns: true) do |mdl| @cache.set( - "{{@type}}.{{method_name}}", mdl.attributes[%foreign_key], [mdl] + "#{%table}.{{method_name}}", mdl.attributes[%foreign_key], [mdl] ) end end @@ -87,13 +90,13 @@ module Clear::Model::HasRelations cache = @cache - qry = {{relation_type}}.query.select("#{%final_table}.*") - .join(%through_table){ - var("#{%through_table}.#{%through_key}") == var("#{%final_table}.#{%final_pkey}") + qry = {{relation_type}}.query.select("#{Clear::SQL.escape(%final_table)}.*") + .join(Clear::SQL.escape(%through_table)){ + var(%through_table, %through_key) == var(%final_table, %final_pkey) }.where{ # FIXME: self.id or self.pkey ? - var("#{%through_table}.#{%own_key}") == self.id - }.distinct("#{%final_table}.#{%final_pkey}") + var(%through_table, %own_key) == self.id + }.distinct("#{Clear::SQL.escape(%final_table)}.#{Clear::SQL.escape(%final_pkey)}") if cache && cache.active?("{{method_name}}") @@ -122,11 +125,11 @@ module Clear::Model::HasRelations sub_query = self.dup.clear_select.select("#{{{@type}}.table}.#{self_type.pkey}") qry = {{relation_type}}.query.join(%through_table){ - var("#{%through_table}.#{%through_key}") == var("#{%final_table}.#{%final_pkey}") + var(%through_table, %through_key) == var(%final_table, %final_pkey) }.where{ - var("#{%through_table}.#{%own_key}").in?(sub_query) - }.distinct.select( "#{%final_table}.*", - "#{%through_table}.#{%own_key} AS __own_id" + var(%through_table, %own_key).in?(sub_query) + }.distinct.select( "#{Clear::SQL.escape(%final_table)}.*", + "#{Clear::SQL.escape(%through_table)}.#{Clear::SQL.escape(%own_key)} AS __own_id" ) block.call(qry) diff --git a/src/clear/model/reflection/column.cr b/src/clear/model/reflection/column.cr index 86a7b98fa..654c63c70 100644 --- a/src/clear/model/reflection/column.cr +++ b/src/clear/model/reflection/column.cr @@ -3,7 +3,8 @@ class Clear::Reflection::Column include Clear::Model - self.table = "information_schema.columns" + self.schema = "information_schema" + self.table = "columns" self.read_only = true column table_catalog : String diff --git a/src/clear/model/reflection/table.cr b/src/clear/model/reflection/table.cr index e429832b9..9b9e2f1cf 100644 --- a/src/clear/model/reflection/table.cr +++ b/src/clear/model/reflection/table.cr @@ -3,7 +3,9 @@ class Clear::Reflection::Table include Clear::Model - self.table = "information_schema.tables" + self.table = "tables" + self.schema = "information_schema" + self.read_only = true column table_catalog : String @@ -14,7 +16,7 @@ class Clear::Reflection::Table scope(:public) { where { table_schema == "public" } } scope(:tables_only) { where { table_type == "BASE TABLE" } } - scope(:views_only) { where{ table_type == "VIEW" } } + scope(:views_only) { where { table_type == "VIEW" } } has_many columns : Clear::Reflection::Column, foreign_key: "table_name", primary_key: "table_name" @@ -45,27 +47,27 @@ class Clear::Reflection::Table o = {} of String => Array(String) SQL.select({ - index_name: "i.relname", - column_name: "a.attname" + index_name: "i.relname", + column_name: "a.attname", }) - .from({t: "pg_class", i: "pg_class", ix: "pg_index", a: "pg_attribute"}) - .where { - (t.oid == ix.indrelid) & - (i.oid == ix.indexrelid) & - (a.attrelid == t.oid) & - (a.attnum == raw("ANY(ix.indkey)")) & - (t.relkind == "r") & - (t.relname == self.table_name) - } - .order_by("t.relname": :asc, "i.relname": :asc) - .fetch do |h| - col = h["column_name"].to_s - v = h["index_name"].to_s + .from({t: "pg_class", i: "pg_class", ix: "pg_index", a: "pg_attribute"}) + .where { + (t.oid == ix.indrelid) & + (i.oid == ix.indexrelid) & + (a.attrelid == t.oid) & + (a.attnum == raw("ANY(ix.indkey)")) & + (t.relkind == "r") & + (t.relname == self.table_name) + } + .order_by("t.relname").order_by("i.relname") + .fetch do |h| + col = h["column_name"].to_s + v = h["index_name"].to_s - arr = o[col]? ? o[col] : (o[col] = [] of String) + arr = o[col]? ? o[col] : (o[col] = [] of String) - arr << v - end + arr << v + end return o end diff --git a/src/clear/sql/delete_query.cr b/src/clear/sql/delete_query.cr index d34cbe281..27b438569 100644 --- a/src/clear/sql/delete_query.cr +++ b/src/clear/sql/delete_query.cr @@ -18,7 +18,10 @@ class Clear::SQL::DeleteQuery end def to_sql - raise Clear::ErrorMessages.query_building_error("Delete Query must have a `from` clause.") if @from.nil? - ["DELETE FROM #{@from}", print_wheres].compact.join(" ") + raise Clear::ErrorMessages.query_building_error("Delete Query must have a `from` clause.") unless from = @from + + from = from.is_a?(Symbol) ? SQL.escape(from.to_s) : from + + ["DELETE FROM", from, print_wheres].compact.join(" ") end end diff --git a/src/clear/sql/fragment/column.cr b/src/clear/sql/fragment/column.cr index 5abf96b39..641fbfd14 100644 --- a/src/clear/sql/fragment/column.cr +++ b/src/clear/sql/fragment/column.cr @@ -17,8 +17,10 @@ module Clear::SQL def to_sql v = value case v - when Symbolic + when String [v, @var].compact.join(" AS ") + when Symbol + [SQL.escape(v.to_s), @var].compact.join(" AS ") when SQL::SelectBuilder ["( #{v.to_sql} )", @var].compact.join(" AS ") else diff --git a/src/clear/sql/fragment/from.cr b/src/clear/sql/fragment/from.cr index 3578db098..1aa713b43 100644 --- a/src/clear/sql/fragment/from.cr +++ b/src/clear/sql/fragment/from.cr @@ -11,7 +11,9 @@ module Clear::SQL def to_sql v = value case v - when Symbolic + when Symbol + [Clear::SQL.escape(v), @var].compact.join(" AS ") + when String [v, @var].compact.join(" AS ") when SQL::SelectBuilder raise Clear::ErrorMessages.query_building_error("Subquery `from` clause must have variable name") if @var.nil? diff --git a/src/clear/sql/insert_query.cr b/src/clear/sql/insert_query.cr index a78555679..7fc8fbfe9 100644 --- a/src/clear/sql/insert_query.cr +++ b/src/clear/sql/insert_query.cr @@ -23,17 +23,17 @@ class Clear::SQL::InsertQuery alias Inserable = ::Clear::SQL::Any | BigInt | BigFloat | Time getter keys : Array(Symbolic) = [] of Symbolic getter values : SelectBuilder | Array(Array(Inserable)) = [] of Array(Inserable) - getter! table : Selectable + getter! table : Symbol | String getter returning : String? - def initialize(@table : Selectable) + def initialize(@table : Symbol | String) end - def initialize(@table : Selectable, values) + def initialize(@table : Symbol | String, values) self.values(values) end - def into(@table : Selectable) + def into(@table : Symbol | String) end def fetch(connection_name : String = "default", &block : Hash(String, ::Clear::SQL::Any) -> Void) @@ -151,7 +151,7 @@ class Clear::SQL::InsertQuery end protected def print_keys - @keys.any? ? "(" + @keys.map(&.to_s).join(", ") + ")" : nil + @keys.any? ? "(" + @keys.map{ |x| Clear::SQL.escape(x.to_s) }.join(", ") + ")" : nil end protected def print_values @@ -164,8 +164,11 @@ class Clear::SQL::InsertQuery end def to_sql - raise QueryBuildingError.new "You must provide a `into` clause" if @table.nil? - o = ["INSERT INTO", @table, print_keys] + raise QueryBuildingError.new "You must provide a `into` clause" unless table = @table + + table = Clear::SQL.escape(table.to_s) + + o = ["INSERT INTO", table, print_keys] v = @values case v when SelectBuilder diff --git a/src/clear/sql/logger.cr b/src/clear/sql/logger.cr index c70d7087c..6ab6014e2 100644 --- a/src/clear/sql/logger.cr +++ b/src/clear/sql/logger.cr @@ -13,7 +13,7 @@ module Clear::SQL::Logger LEADING LIMIT LEFT LOCALTIME LOCALTIMESTAMP NATURAL NEW NOT NULL OFF OFFSET OLD ON ONLY OR ORDER OUTER PLACING PRIMARY REFERENCES RELEASE RETURNING RIGHT ROLLBACK SAVEPOINT SELECT SESSION_USER SET SOME SYMMETRIC - TABLE THEN TO TRAILING TRUE UNION UNIQUE UPDATE USER USING VALUES + TABLE THEN TO TRAILING TRIGGER TRUE UNION UNIQUE UPDATE USER USING VALUES WHEN WHERE WINDOW )) diff --git a/src/clear/sql/query/group_by.cr b/src/clear/sql/query/group_by.cr index 7ad19ff8f..fbdf2cca6 100644 --- a/src/clear/sql/query/group_by.cr +++ b/src/clear/sql/query/group_by.cr @@ -1,18 +1,19 @@ module Clear::SQL::Query::GroupBy - getter group_bys : Array(String) + getter group_bys : Array(Symbolic) def clear_group_bys @group_bys.clear change! end - def group_by(column) + def group_by(column : Symbolic) @group_bys << column change! - end + end + protected def print_group_bys return unless @group_bys.any? - "GROUP BY " + @group_bys.join(", ") + "GROUP BY " + @group_bys.map{ |x| x.is_a?(Symbol) ? SQL.escape(x) : x }.join(", ") end end diff --git a/src/clear/sql/query/join.cr b/src/clear/sql/query/join.cr index d009ae39c..0618fefc0 100644 --- a/src/clear/sql/query/join.cr +++ b/src/clear/sql/query/join.cr @@ -4,6 +4,8 @@ module Clear::SQL::Query::Join end protected def join_impl(name : Symbolic, type, clear_expr) + name = ( name.is_a?(Symbol) ? Clear::SQL.escape(name.to_s) : name ) + joins << Clear::SQL::Join.new(name, clear_expr, type) change! end diff --git a/src/clear/sql/query/order_by.cr b/src/clear/sql/query/order_by.cr index 602d5d247..e3f121a8a 100644 --- a/src/clear/sql/query/order_by.cr +++ b/src/clear/sql/query/order_by.cr @@ -21,16 +21,13 @@ module Clear::SQL::Query::OrderBy change! end - def order_by( x : Array(Record) ) + def order_by(x : Array(Record)) @order_bys = x change! end def order_by(**tuple) - tuple.each do |k, v| - @order_bys << Record.new(k.to_s, _order_by_to_symbol(v.to_s)) - end - change! + order_by(tuple) end def order_by(tuple : NamedTuple) @@ -40,13 +37,18 @@ module Clear::SQL::Query::OrderBy change! end - def order_by(expression, direction="ASC") + def order_by(expression : Symbol, direction = "ASC") + @order_bys << Record.new(SQL.escape(expression.to_s), _order_by_to_symbol(direction)) + change! + end + + def order_by(expression : String, direction = "ASC") @order_bys << Record.new(expression, _order_by_to_symbol(direction)) change! end protected def print_order_bys return unless @order_bys.any? - "ORDER BY " + @order_bys.map{ |r| [r.op, r.dir.to_s.upcase].join(" ") }.join(", ") + "ORDER BY " + @order_bys.map { |r| [r.op, r.dir.to_s.upcase].join(" ") }.join(", ") end end diff --git a/src/clear/sql/query/where.cr b/src/clear/sql/query/where.cr index fd0e8d812..4fd473f48 100644 --- a/src/clear/sql/query/where.cr +++ b/src/clear/sql/query/where.cr @@ -44,16 +44,16 @@ module Clear::SQL::Query::Where # ``` # - the `IN` operator if compared with an array: # ```crystal - # query.where(x: [1, 2]) # WHERE x in (1,2) + # query.where({x: [1, 2]}) # WHERE x in (1,2) # ``` # - the `>=` and `<=` | `<` if compared with a range: # ```crystal - # query.where(x: (1..4)) # WHERE x >= 1 AND x <= 4 - # query.where(x: (1...4)) # WHERE x >= 1 AND x < 4 + # query.where({x: (1..4)}) # WHERE x >= 1 AND x <= 4 + # query.where({x: (1...4)}) # WHERE x >= 1 AND x < 4 # ``` # - You also can put another select query as argument: # ```crystal - # query.where(x: another_select) # WHERE x IN (SELECT ... ) + # query.where({x: another_select}) # WHERE x IN (SELECT ... ) # ``` def where(x : NamedTuple) x.each do |k, v| @@ -122,7 +122,7 @@ module Clear::SQL::Query::Where # where("ADD_SOME_DANGEROUS_SQL_HERE") # WHERE ADD_SOME_DANGEROUS_SQL_HERE # ``` def where(str : String) - @wheres << Clear::Expression::Node::Variable.new(str) + @wheres << Clear::Expression::Node::Raw.new(str) change! end diff --git a/src/clear/sql/select_builder.cr b/src/clear/sql/select_builder.cr index c2efecbf8..4f401f64a 100644 --- a/src/clear/sql/select_builder.cr +++ b/src/clear/sql/select_builder.cr @@ -28,7 +28,7 @@ module Clear::SQL::SelectBuilder @wheres = [] of Clear::Expression::Node, @havings = [] of Clear::Expression::Node, @windows = [] of {String, String}, - @group_bys = [] of String, + @group_bys = [] of Symbolic, @order_bys = [] of Clear::SQL::Query::OrderBy::Record, @limit = nil, @offset = nil, diff --git a/src/clear/sql/sql.cr b/src/clear/sql/sql.cr index 3d121733a..3e233374f 100644 --- a/src/clear/sql/sql.cr +++ b/src/clear/sql/sql.cr @@ -58,11 +58,22 @@ module Clear alias Symbolic = String | Symbol alias Selectable = Symbolic | Clear::SQL::SelectBuilder - # Sanitize the + # Sanitize def sanitize(x : String, delimiter = "''") Clear::Expression[x] end + # Escape the expression, double quoting it. + # + # It allows use of reserved keywords as table or column name + def escape(x : String|Symbol) + "\"" + x.to_s.gsub("\"", "\"\"") + "\"" + end + + def unsafe(x) + Clear::Expression::UnsafeSql.new(x) + end + def init(url : String) @@connections["default"] = DB.open(url) end