Skip to content

Commit

Permalink
Revert "Typecast literal expressions"
Browse files Browse the repository at this point in the history 
This reverts commit 1085957.
  • Loading branch information
@grassick
grassick committed Jan 20, 2021
1 parent 1085957 commit 9386b38
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 102 deletions.
23 changes: 2 additions & 21 deletions lib/JsonqlCompiler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -359,12 +359,12 @@ module.exports = JsonqlCompiler = /*#__PURE__*/function () {


if ((ref = (0, _typeof2["default"])(expr)) === "number" || ref === "string" || ref === "boolean") {
return this.compileLiteral(expr);
return new SqlFragment("?", [expr]);
}

switch (expr.type) {
case "literal":
return this.compileLiteral(expr.value);
return new SqlFragment("?", [expr.value]);

case "op":
return this.compileOpExpr(expr, aliases, ctes);
Expand Down Expand Up @@ -688,25 +688,6 @@ module.exports = JsonqlCompiler = /*#__PURE__*/function () {
throw new Error("Invalid alias '".concat(alias, "'"));
}
}
}, {
key: "compileLiteral",
value: function compileLiteral(value) {
// Postgres gives errors if the parameter type is unknown at times. e.g. sum(?) fails.
// To avoid, always cast value
if (typeof value === "number") {
return new SqlFragment("(?::numeric)", [value]);
}

if (typeof value === "string") {
return new SqlFragment("(?::text)", [value]);
}

if (typeof value === "boolean") {
return new SqlFragment("(?::boolean)", [value]);
}

return new SqlFragment("?", [value]);
}
}]);
return JsonqlCompiler;
}();
Expand Down
16 changes: 2 additions & 14 deletions src/JsonqlCompiler.coffee
View file
Original file line number Diff line number Diff line change
Expand Up @@ -284,11 +284,11 @@ module.exports = class JsonqlCompiler

# Literals
if typeof(expr) in ["number", "string", "boolean"]
return @compileLiteral(expr)
return new SqlFragment("?", [expr])

switch expr.type
when "literal"
return @compileLiteral(expr.value)
return new SqlFragment("?", [expr.value])
when "op"
return @compileOpExpr(expr, aliases, ctes)
when "field"
Expand Down Expand Up @@ -583,17 +583,5 @@ module.exports = class JsonqlCompiler
if not alias.match(/^[_a-zA-Z][a-zA-Z_0-9. :]*$/)
throw new Error("Invalid alias '#{alias}'")

compileLiteral: (value) ->
# Postgres gives errors if the parameter type is unknown at times. e.g. sum(?) fails.
# To avoid, always cast value
if typeof(value) == "number"
return new SqlFragment("(?::numeric)", [value])
if typeof(value) == "string"
return new SqlFragment("(?::text)", [value])
if typeof(value) == "boolean"
return new SqlFragment("(?::boolean)", [value])

return new SqlFragment("?", [value])

isInt = (x) ->
return typeof(x)=='number' and (x%1) == 0
134 changes: 67 additions & 67 deletions test/JsonqlCompilerTests.coffee
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe "JsonqlCompiler", ->
}

compiled = @compiler.compileQuery(query)
assert.equal compiled.sql, 'select (?::numeric) as "x" from ABC as "a_abc1"'
assert.equal compiled.sql, 'select ? as "x" from ABC as "a_abc1"'
assert.deepEqual compiled.params, [4]

it 'compiles distinct query', ->
Expand All @@ -43,7 +43,7 @@ describe "JsonqlCompiler", ->
}

compiled = @compiler.compileQuery(query)
assert.equal compiled.sql, 'select distinct (?::numeric) as "x" from ABC as "a_abc1"'
assert.equal compiled.sql, 'select distinct ? as "x" from ABC as "a_abc1"'
assert.deepEqual compiled.params, [4]

it 'compiles query with null select', ->
Expand Down Expand Up @@ -83,7 +83,7 @@ describe "JsonqlCompiler", ->
}

compiled = @compiler.compileQuery(query)
assert.equal compiled.sql, 'select a_abc1.P as "x" from ABC as "a_abc1" where (a_abc1.Q > (?::numeric))'
assert.equal compiled.sql, 'select a_abc1.P as "x" from ABC as "a_abc1" where (a_abc1.Q > ?)'
assert.deepEqual compiled.params, [5]

it 'compiles query with groupBy ordinals', ->
Expand Down Expand Up @@ -182,7 +182,7 @@ describe "JsonqlCompiler", ->
}

compiled = @compiler.compileQuery(query)
assert.equal compiled.sql, 'select (?::numeric) as "x" from ABC as "a_abc1" limit ?'
assert.equal compiled.sql, 'select ? as "x" from ABC as "a_abc1" limit ?'
assert.deepEqual compiled.params, [4, 10]

it 'compiles query with offset', ->
Expand All @@ -196,7 +196,7 @@ describe "JsonqlCompiler", ->
}

compiled = @compiler.compileQuery(query)
assert.equal compiled.sql, 'select (?::numeric) as "x" from ABC as "a_abc1" offset ?'
assert.equal compiled.sql, 'select ? as "x" from ABC as "a_abc1" offset ?'
assert.deepEqual compiled.params, [4, 10]

it 'compiles query with subquery query', ->
Expand All @@ -217,7 +217,7 @@ describe "JsonqlCompiler", ->
}

compiled = @compiler.compileQuery(query)
assert.equal compiled.sql, 'select a_abc1."q" as "x" from (select (?::numeric) as "q" from XYZ as "a_xyz1") as "a_abc1"'
assert.equal compiled.sql, 'select a_abc1."q" as "x" from (select ? as "q" from XYZ as "a_xyz1") as "a_abc1"'
assert.deepEqual compiled.params, [5]

it 'compiles query with subexpression', ->
Expand Down Expand Up @@ -262,7 +262,7 @@ describe "JsonqlCompiler", ->
}

compiled = @compiler.compileQuery(query)
assert.equal compiled.sql, 'with "a_wq" as (select (?::numeric) as "q" from XYZ as "a_xyz1") select a_abc1."q" as "x" from a_wq as "a_abc1"'
assert.equal compiled.sql, 'with "a_wq" as (select ? as "q" from XYZ as "a_xyz1") select a_abc1."q" as "x" from a_wq as "a_abc1"'
assert.deepEqual compiled.params, [5]

it 'compiles union all query', ->
Expand All @@ -288,7 +288,7 @@ describe "JsonqlCompiler", ->
}

compiled = @compiler.compileQuery(query)
assert.equal compiled.sql, '(select (?::numeric) as "x" from ABC as "a_abc1") union all (select (?::numeric) as "x" from ABC as "a_abc1")'
assert.equal compiled.sql, '(select ? as "x" from ABC as "a_abc1") union all (select ? as "x" from ABC as "a_abc1")'
assert.deepEqual compiled.params, [4, 5]

it "compiles reused alias", ->
Expand Down Expand Up @@ -433,13 +433,13 @@ describe "JsonqlCompiler", ->
assert.deepEqual fr.params, params

it 'literal', ->
@testExpr({ type: "literal", value: "abc" }, "(?::text)", ["abc"])
@testExpr({ type: "literal", value: "abc" }, "?", ["abc"])

it 'JSON literals', ->
@testExpr("abc", "(?::text)", ["abc"])
@testExpr(2.3, "(?::numeric)", [2.3])
@testExpr(true, "(?::boolean)", [true])
@testExpr(false, "(?::boolean)", [false])
@testExpr("abc", "?", ["abc"])
@testExpr(2.3, "?", [2.3])
@testExpr(true, "?", [true])
@testExpr(false, "?", [false])

it 'null', ->
@testExpr(null, "null", [])
Expand All @@ -451,106 +451,106 @@ describe "JsonqlCompiler", ->
describe "case", ->
it "does input case", ->
@testExpr({ type: "case", input: @a, cases: [{ when: @b, then: @c }]},
"case (?::numeric) when (?::numeric) then (?::numeric) end", [1, 2, 3]
"case ? when ? then ? end", [1, 2, 3]
)

it "does multiple case with else", ->
@testExpr({ type: "case", cases: [
{ when: @a, then: @b }
{ when: @c, then: @d }
], else: @e },
"case when (?::numeric) then (?::numeric) when (?::numeric) then (?::numeric) else (?::numeric) end", [1, 2, 3, 4, 5]
"case when ? then ? when ? then ? else ? end", [1, 2, 3, 4, 5]
)

describe "ops", ->
it '> < >= <= = <>', ->
@testExpr({ type: "op", op: ">", exprs: [@a, @b] }, "((?::numeric) > (?::numeric))", [1, 2])
@testExpr({ type: "op", op: "<", exprs: [@a, @b] }, "((?::numeric) < (?::numeric))", [1, 2])
@testExpr({ type: "op", op: ">=", exprs: [@a, @b] }, "((?::numeric) >= (?::numeric))", [1, 2])
@testExpr({ type: "op", op: "<=", exprs: [@a, @b] }, "((?::numeric) <= (?::numeric))", [1, 2])
@testExpr({ type: "op", op: "=", exprs: [@a, @b] }, "((?::numeric) = (?::numeric))", [1, 2])
@testExpr({ type: "op", op: "<>", exprs: [@a, @b] }, "((?::numeric) <> (?::numeric))", [1, 2])
@testExpr({ type: "op", op: ">", exprs: [@a, @b] }, "(? > ?)", [1, 2])
@testExpr({ type: "op", op: "<", exprs: [@a, @b] }, "(? < ?)", [1, 2])
@testExpr({ type: "op", op: ">=", exprs: [@a, @b] }, "(? >= ?)", [1, 2])
@testExpr({ type: "op", op: "<=", exprs: [@a, @b] }, "(? <= ?)", [1, 2])
@testExpr({ type: "op", op: "=", exprs: [@a, @b] }, "(? = ?)", [1, 2])
@testExpr({ type: "op", op: "<>", exprs: [@a, @b] }, "(? <> ?)", [1, 2])

it 'and', ->
@testExpr({ type: "op", op: "and", exprs: [] }, "", [])
@testExpr({ type: "op", op: "and", exprs: [@a] }, "(?::numeric)", [1])
@testExpr({ type: "op", op: "and", exprs: [@a, @b, @c] }, "((?::numeric) and (?::numeric) and (?::numeric))", [1, 2, 3])
@testExpr({ type: "op", op: "and", exprs: [{ type: "op", op: "and", exprs: [] }, { type: "op", op: "and", exprs: [@a] }] }, "(?::numeric)", [1])
@testExpr({ type: "op", op: "and", exprs: [@a] }, "?", [1])
@testExpr({ type: "op", op: "and", exprs: [@a, @b, @c] }, "(? and ? and ?)", [1, 2, 3])
@testExpr({ type: "op", op: "and", exprs: [{ type: "op", op: "and", exprs: [] }, { type: "op", op: "and", exprs: [@a] }] }, "?", [1])

it 'or', ->
@testExpr({ type: "op", op: "or", exprs: [] }, "", [])
@testExpr({ type: "op", op: "or", exprs: [@a] }, "(?::numeric)", [1])
@testExpr({ type: "op", op: "or", exprs: [@a, @b, @c] }, "((?::numeric) or (?::numeric) or (?::numeric))", [1, 2, 3])
@testExpr({ type: "op", op: "or", exprs: [@a] }, "?", [1])
@testExpr({ type: "op", op: "or", exprs: [@a, @b, @c] }, "(? or ? or ?)", [1, 2, 3])

it 'not', ->
@testExpr({ type: "op", op: "not", exprs: [@a] }, "(not (?::numeric))", [1])
@testExpr({ type: "op", op: "not", exprs: [@a] }, "(not ?)", [1])

it 'is null', ->
@testExpr({ type: "op", op: "is null", exprs: [@a] }, "((?::numeric) is null)", [1])
@testExpr({ type: "op", op: "is null", exprs: [@a] }, "(? is null)", [1])

it 'is not null', ->
@testExpr({ type: "op", op: "is not null", exprs: [@a] }, "((?::numeric) is not null)", [1])
@testExpr({ type: "op", op: "is not null", exprs: [@a] }, "(? is not null)", [1])

it 'in', ->
@testExpr({ type: "op", op: "in", exprs: [@a, @b] }, "((?::numeric) in (?::numeric))", [1, 2])
@testExpr({ type: "op", op: "in", exprs: [@a, @b] }, "(? in ?)", [1, 2])

it '+ - *', ->
@testExpr({ type: "op", op: "+", exprs: [@a, @b] }, "((?::numeric) + (?::numeric))", [1, 2])
@testExpr({ type: "op", op: "-", exprs: [@a, @b] }, "((?::numeric) - (?::numeric))", [1, 2])
@testExpr({ type: "op", op: "*", exprs: [@a, @b] }, "((?::numeric) * (?::numeric))", [1, 2])
@testExpr({ type: "op", op: "+", exprs: [@a, @b, @c] }, "((?::numeric) + (?::numeric) + (?::numeric))", [1, 2, 3])
@testExpr({ type: "op", op: "-", exprs: [@a, @b, @c] }, "((?::numeric) - (?::numeric) - (?::numeric))", [1, 2, 3])
@testExpr({ type: "op", op: "*", exprs: [@a, @b, @c] }, "((?::numeric) * (?::numeric) * (?::numeric))", [1, 2, 3])
@testExpr({ type: "op", op: "+", exprs: [@a, @b] }, "(? + ?)", [1, 2])
@testExpr({ type: "op", op: "-", exprs: [@a, @b] }, "(? - ?)", [1, 2])
@testExpr({ type: "op", op: "*", exprs: [@a, @b] }, "(? * ?)", [1, 2])
@testExpr({ type: "op", op: "+", exprs: [@a, @b, @c] }, "(? + ? + ?)", [1, 2, 3])
@testExpr({ type: "op", op: "-", exprs: [@a, @b, @c] }, "(? - ? - ?)", [1, 2, 3])
@testExpr({ type: "op", op: "*", exprs: [@a, @b, @c] }, "(? * ? * ?)", [1, 2, 3])

it '/', ->
@testExpr({ type: "op", op: "/", exprs: [@a, @b] }, "((?::numeric) / (?::numeric))", [1, 2])
@testExpr({ type: "op", op: "/", exprs: [@a, @b] }, "(? / ?)", [1, 2])

it '||', ->
@testExpr({ type: "op", op: "||", exprs: [@a, @b, @c] }, "((?::numeric) || (?::numeric) || (?::numeric))", [1, 2, 3])
@testExpr({ type: "op", op: "||", exprs: [@a, @b, @c] }, "(? || ? || ?)", [1, 2, 3])

it '~ ~* like ilike', ->
@testExpr({ type: "op", op: "~", exprs: [@a, @b] }, "((?::numeric) ~ (?::numeric))", [1, 2])
@testExpr({ type: "op", op: "~*", exprs: [@a, @b] }, "((?::numeric) ~* (?::numeric))", [1, 2])
@testExpr({ type: "op", op: "like", exprs: [@a, @b] }, "((?::numeric) like (?::numeric))", [1, 2])
@testExpr({ type: "op", op: "ilike", exprs: [@a, @b] }, "((?::numeric) ilike (?::numeric))", [1, 2])
@testExpr({ type: "op", op: "~", exprs: [@a, @b] }, "(? ~ ?)", [1, 2])
@testExpr({ type: "op", op: "~*", exprs: [@a, @b] }, "(? ~* ?)", [1, 2])
@testExpr({ type: "op", op: "like", exprs: [@a, @b] }, "(? like ?)", [1, 2])
@testExpr({ type: "op", op: "ilike", exprs: [@a, @b] }, "(? ilike ?)", [1, 2])

it '::text', ->
@testExpr({ type: "op", op: "::text", exprs: [@a] }, "((?::numeric)::text)", [1])
@testExpr({ type: "op", op: "::text", exprs: [@a] }, "(?::text)", [1])

it '[]', ->
@testExpr({ type: "op", op: "[]", exprs: [@a, @b] }, "(((?::numeric))[(?::numeric)])", [1, 2])
@testExpr({ type: "op", op: "[]", exprs: [@a, @b] }, "((?)[?])", [1, 2])

it '= any', ->
arr = { type: "literal", value: ["x", "y"] }
@testExpr({ type: "op", op: "=", modifier: "any", exprs: [@a, arr] }, "((?::numeric) = any(?))", [1, ["x", "y"]])
@testExpr({ type: "op", op: "=", modifier: "any", exprs: [@a, arr] }, "(? = any(?))", [1, ["x", "y"]])

it '->> #>>', ->
@testExpr({ type: "op", op: "->>", exprs: [@a, @b] }, "((?::numeric) ->> (?::numeric))", [1, 2])
@testExpr({ type: "op", op: "#>>", exprs: [@a, @b] }, "((?::numeric) #>> (?::numeric))", [1, 2])
@testExpr({ type: "op", op: "->>", exprs: [@a, @b] }, "(? ->> ?)", [1, 2])
@testExpr({ type: "op", op: "#>>", exprs: [@a, @b] }, "(? #>> ?)", [1, 2])

it 'between', ->
@testExpr({ type: "op", op: "between", exprs: [@a, @b, @c] }, "((?::numeric) between (?::numeric) and (?::numeric))", [1, 2, 3])
@testExpr({ type: "op", op: "between", exprs: [@a, @b, @c] }, "(? between ? and ?)", [1, 2, 3])

it 'aggregate expressions', ->
@testExpr({ type: "op", op: "avg", exprs: [@a] }, "avg((?::numeric))", [1])
@testExpr({ type: "op", op: "min", exprs: [@a] }, "min((?::numeric))", [1])
@testExpr({ type: "op", op: "max", exprs: [@a] }, "max((?::numeric))", [1])
@testExpr({ type: "op", op: "sum", exprs: [@a] }, "sum((?::numeric))", [1])
@testExpr({ type: "op", op: "count", exprs: [@a] }, "count((?::numeric))", [1])
@testExpr({ type: "op", op: "stdev", exprs: [@a] }, "stdev((?::numeric))", [1])
@testExpr({ type: "op", op: "stdevp", exprs: [@a] }, "stdevp((?::numeric))", [1])
@testExpr({ type: "op", op: "var", exprs: [@a] }, "var((?::numeric))", [1])
@testExpr({ type: "op", op: "varp", exprs: [@a] }, "varp((?::numeric))", [1])
@testExpr({ type: "op", op: "avg", exprs: [@a] }, "avg(?)", [1])
@testExpr({ type: "op", op: "min", exprs: [@a] }, "min(?)", [1])
@testExpr({ type: "op", op: "max", exprs: [@a] }, "max(?)", [1])
@testExpr({ type: "op", op: "sum", exprs: [@a] }, "sum(?)", [1])
@testExpr({ type: "op", op: "count", exprs: [@a] }, "count(?)", [1])
@testExpr({ type: "op", op: "stdev", exprs: [@a] }, "stdev(?)", [1])
@testExpr({ type: "op", op: "stdevp", exprs: [@a] }, "stdevp(?)", [1])
@testExpr({ type: "op", op: "var", exprs: [@a] }, "var(?)", [1])
@testExpr({ type: "op", op: "varp", exprs: [@a] }, "varp(?)", [1])
@testExpr({ type: "op", op: "count", exprs: [] }, "count(*)", [])
@testExpr({ type: "op", op: "count", modifier: "distinct", exprs: [@a] }, "count(distinct (?::numeric))", [1])
@testExpr({ type: "op", op: "unnest", exprs: [@a] }, "unnest((?::numeric))", [1])
@testExpr({ type: "op", op: "count", modifier: "distinct", exprs: [@a] }, "count(distinct ?)", [1])
@testExpr({ type: "op", op: "unnest", exprs: [@a] }, "unnest(?)", [1])
assert.throws () =>
@testExpr({ type: "op", op: "xyz", exprs: [@a] }, "xyz((?::numeric))", [1])
@testExpr({ type: "op", op: "xyz", exprs: [@a] }, "xyz(?)", [1])

it 'array_agg with orderBy', ->
orderBy = [{ expr: { type: "field", tableAlias: "abc", column: "x" }, direction: "asc" }]
expr = { type: "op", op: "array_agg", exprs: [@a], orderBy: orderBy }
@testExpr(expr, "array_agg((?::numeric) order by a_abc.X asc)", [1], { abc: "abc" })
@testExpr(expr, "array_agg(? order by a_abc.X asc)", [1], { abc: "abc" })

it 'aggregate over with partitionBy', ->
over = { partitionBy: [{ type: "field", tableAlias: "abc", column: "x" }] }
Expand Down Expand Up @@ -578,17 +578,17 @@ describe "JsonqlCompiler", ->

@testExpr({ type: "op", op: "exists", exprs:[
query
] }, 'exists (select (?::numeric) as "x" from ABC as "a_abc1")', [4])
] }, 'exists (select ? as "x" from ABC as "a_abc1")', [4])

it 'interval', ->
@testExpr({ type: "op", op: "interval", exprs: [@str] }, "(interval (?::text))", ["xyz"])
@testExpr({ type: "op", op: "interval", exprs: [@str] }, "(interval ?)", ["xyz"])

it 'at time zome', ->
@testExpr({ type: "op", op: "at time zone", exprs: [{ type: "op", op: "now", exprs: [] }, @str] }, "(now() at time zone (?::text))", ["xyz"])
@testExpr({ type: "op", op: "at time zone", exprs: [{ type: "op", op: "now", exprs: [] }, @str] }, "(now() at time zone ?)", ["xyz"])

describe "scalar", ->
it "simple scalar", ->
@testExpr({ type: "scalar", expr: @a, from: { type: "table", table: "abc", alias: "abc1" } }, '(select (?::numeric) from ABC as "a_abc1")', [1])
@testExpr({ type: "scalar", expr: @a, from: { type: "table", table: "abc", alias: "abc1" } }, '(select ? from ABC as "a_abc1")', [1])

it "scalar with orderBy expr", ->
@testExpr({
Expand All @@ -599,7 +599,7 @@ describe "JsonqlCompiler", ->
expr: @b
direction: "desc"
}]
}, '(select (?::numeric) from ABC as "a_abc1" order by (?::numeric) desc)', [1,2])
}, '(select ? from ABC as "a_abc1" order by ? desc)', [1,2])

it 'compiles scalar with withs', ->
withQuery = {
Expand All @@ -621,4 +621,4 @@ describe "JsonqlCompiler", ->
expr: @b
direction: "desc"
}]
}, '(with "a_wq" as (select (?::numeric) as "q" from XYZ as "a_xyz1") select (?::numeric) from a_wq as "a_abc1" order by (?::numeric) desc)', [5,1,2])
}, '(with "a_wq" as (select ? as "q" from XYZ as "a_xyz1") select ? from a_wq as "a_abc1" order by ? desc)', [5,1,2])

0 comments on commit 9386b38

Please sign in to comment.