FIX: Errors when running query due to PG template patterns or comments (#189)
Before this fix, the use of PG template patterns containing ":" or the use of "?" in comments in the SQL will result in an error being raised because `DB.param_encoder.encode` calls ActiveRecord's `sanitize_sql_array` which is meant for SQL fragments and not an entire SQL string. Instead we change data-explorer to use `MiniSql::InlineParamEncoder` instead which takes into account of template patterns and does not trip on `?` which is a special param encoding character used by ActiveRecord.
This commit is contained in:
parent
729e5a2add
commit
4236689d27
|
@ -76,7 +76,7 @@ after_initialize do
|
||||||
# Run a data explorer query on the currently connected database.
|
# Run a data explorer query on the currently connected database.
|
||||||
#
|
#
|
||||||
# @param [DataExplorer::Query] query the Query object to run
|
# @param [DataExplorer::Query] query the Query object to run
|
||||||
# @param [Hash] params the colon-style query parameters to pass to AR
|
# @param [Hash] params the colon-style query parameters for the query
|
||||||
# @param [Hash] opts hash of options
|
# @param [Hash] opts hash of options
|
||||||
# explain - include a query plan in the result
|
# explain - include a query plan in the result
|
||||||
# @return [Hash]
|
# @return [Hash]
|
||||||
|
@ -124,9 +124,9 @@ SQL
|
||||||
|
|
||||||
time_start = Time.now
|
time_start = Time.now
|
||||||
|
|
||||||
# we probably want to rewrite this ... but for now reuse the working
|
# Using MiniSql::InlineParamEncoder directly instead of DB.param_encoder because current implementation of
|
||||||
# code we have
|
# DB.param_encoder is meant for SQL fragments and not an entire SQL string.
|
||||||
sql = DB.param_encoder.encode(sql, query_args)
|
sql = MiniSql::InlineParamEncoder.new(ActiveRecord::Base.connection.raw_connection).encode(sql, query_args)
|
||||||
|
|
||||||
result = ActiveRecord::Base.connection.raw_connection.async_exec(sql)
|
result = ActiveRecord::Base.connection.raw_connection.async_exec(sql)
|
||||||
result.check # make sure it's done
|
result.check # make sure it's done
|
||||||
|
|
|
@ -0,0 +1,61 @@
|
||||||
|
# frozen_string_literal: true
|
||||||
|
|
||||||
|
describe DataExplorer do
|
||||||
|
describe '.run_query' do
|
||||||
|
fab!(:topic) { Fabricate(:topic) }
|
||||||
|
|
||||||
|
it 'should run a query that includes PG template patterns' do
|
||||||
|
sql = <<~SQL
|
||||||
|
WITH query AS (
|
||||||
|
SELECT TO_CHAR(created_at, 'yyyy:mm:dd') AS date FROM topics
|
||||||
|
) SELECT * FROM query
|
||||||
|
SQL
|
||||||
|
|
||||||
|
query = DataExplorer::Query.create!(name: "some query", sql: sql)
|
||||||
|
|
||||||
|
result = described_class.run_query(query)
|
||||||
|
|
||||||
|
expect(result[:error]).to eq(nil)
|
||||||
|
expect(result[:pg_result][0]["date"]).to eq(topic.created_at.strftime("%Y:%m:%d"))
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'should run a query containing a question mark in the comment' do
|
||||||
|
sql = <<~SQL
|
||||||
|
WITH query AS (
|
||||||
|
SELECT id FROM topics -- some SQL ? comment ?
|
||||||
|
) SELECT * FROM query
|
||||||
|
SQL
|
||||||
|
|
||||||
|
query = DataExplorer::Query.create!(name: "some query", sql: sql)
|
||||||
|
|
||||||
|
result = described_class.run_query(query)
|
||||||
|
|
||||||
|
expect(result[:error]).to eq(nil)
|
||||||
|
expect(result[:pg_result][0]["id"]).to eq(topic.id)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'can run a query with params interpolation' do
|
||||||
|
topic2 = Fabricate(:topic)
|
||||||
|
|
||||||
|
sql = <<~SQL
|
||||||
|
-- [params]
|
||||||
|
-- int :topic_id = 99999999
|
||||||
|
WITH query AS (
|
||||||
|
SELECT
|
||||||
|
id,
|
||||||
|
TO_CHAR(created_at, 'yyyy:mm:dd') AS date
|
||||||
|
FROM topics
|
||||||
|
WHERE topics.id = :topic_id
|
||||||
|
) SELECT * FROM query
|
||||||
|
SQL
|
||||||
|
|
||||||
|
query = DataExplorer::Query.create!(name: "some query", sql: sql)
|
||||||
|
|
||||||
|
result = described_class.run_query(query, { "topic_id" => topic2.id.to_s })
|
||||||
|
|
||||||
|
expect(result[:error]).to eq(nil)
|
||||||
|
expect(result[:pg_result].to_a.size).to eq(1)
|
||||||
|
expect(result[:pg_result][0]["id"]).to eq(topic2.id)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
Loading…
Reference in New Issue