From 4236689d27bab8c68b5aababda64e4f22223d591 Mon Sep 17 00:00:00 2001
From: Alan Guo Xiang Tan <gxtan1990@gmail.com>
Date: Mon, 10 Oct 2022 09:03:37 +0800
Subject: [PATCH] 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.
---
 plugin.rb                  |  8 ++---
 spec/data_explorer_spec.rb | 61 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 4 deletions(-)
 create mode 100644 spec/data_explorer_spec.rb

diff --git a/plugin.rb b/plugin.rb
index e9bea96..1118328 100644
--- a/plugin.rb
+++ b/plugin.rb
@@ -76,7 +76,7 @@ after_initialize do
     # Run a data explorer query on the currently connected database.
     #
     # @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
     #   explain - include a query plan in the result
     # @return [Hash]
@@ -124,9 +124,9 @@ SQL
 
           time_start = Time.now
 
-          # we probably want to rewrite this ... but for now reuse the working
-          # code we have
-          sql = DB.param_encoder.encode(sql, query_args)
+          # Using MiniSql::InlineParamEncoder directly instead of DB.param_encoder because current implementation of
+          # DB.param_encoder is meant for SQL fragments and not an entire SQL string.
+          sql = MiniSql::InlineParamEncoder.new(ActiveRecord::Base.connection.raw_connection).encode(sql, query_args)
 
           result = ActiveRecord::Base.connection.raw_connection.async_exec(sql)
           result.check # make sure it's done
diff --git a/spec/data_explorer_spec.rb b/spec/data_explorer_spec.rb
new file mode 100644
index 0000000..997eca2
--- /dev/null
+++ b/spec/data_explorer_spec.rb
@@ -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