From 969b7094f22ac21a0c047b68cfb5a9070542e4a7 Mon Sep 17 00:00:00 2001
From: Rob Browning <rlb@puppet.com>
Date: Wed, 24 Apr 2024 15:19:08 -0500
Subject: [PATCH 01/20] (PDB-5739) certname-names-query: migrate to
certnames_status
cf. fda5d3c7671caf725dbc54e4533b31d88e0852d6
---
src/puppetlabs/puppetdb/query.clj | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/puppetlabs/puppetdb/query.clj b/src/puppetlabs/puppetdb/query.clj
index 9c9c21e6f3..0c03a90325 100644
--- a/src/puppetlabs/puppetdb/query.clj
+++ b/src/puppetlabs/puppetdb/query.clj
@@ -394,8 +394,8 @@
(defn certname-names-query [active]
(if active
- "SELECT name FROM certnames WHERE deactivated IS NULL AND expired IS NULL"
- "SELECT name FROM certnames WHERE deactivated IS NOT NULL OR expired IS NOT NULL") )
+ "SELECT certname FROM certnames_status WHERE deactivated IS NULL AND expired IS NULL"
+ "SELECT certname FROM certnames_status WHERE deactivated IS NOT NULL OR expired IS NOT NULL"))
(defn compile-resource-equality
"Compile an = operator for a resource query. `path` represents the field
From e0808f8c66b0f35e946ff6de3a0be027fc84557a Mon Sep 17 00:00:00 2001
From: Rob Browning <rlb@puppet.com>
Date: Wed, 24 Apr 2024 16:22:51 -0500
Subject: [PATCH 02/20] (PDB-5739) clj-jdbc-termination-ex?: require
SQLException
Previously this function could try to call .getSQLState on unrelated
exceptions.
---
src/puppetlabs/puppetdb/jdbc.clj | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/puppetlabs/puppetdb/jdbc.clj b/src/puppetlabs/puppetdb/jdbc.clj
index 560b33ee0d..5c78853e44 100644
--- a/src/puppetlabs/puppetdb/jdbc.clj
+++ b/src/puppetlabs/puppetdb/jdbc.clj
@@ -37,8 +37,9 @@
(and (instance? ExceptionInfo ex)
(let [{:keys [rollback handling]} (ex-data ex)]
(and rollback
+ (instance? SQLException handling)
(= (sql-state :admin-shutdown)
- (some-> handling .getSQLState))))))
+ (.getSQLState handling))))))
(defmacro with-db-connection [spec & body]
`(sql/with-db-connection [db# ~spec]
From 8b43bbe6516307a8731747a011deb282ab166ee7 Mon Sep 17 00:00:00 2001
From: Rob Browning <rlb@puppet.com>
Date: Tue, 30 Apr 2024 20:38:51 -0500
Subject: [PATCH 03/20] (PDB-5739) Fix command service request-shutdown
invocation
Noticed failing schema check in command-test output.
---
src/puppetlabs/puppetdb/command.clj | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/puppetlabs/puppetdb/command.clj b/src/puppetlabs/puppetdb/command.clj
index 9580370f1c..be749ed6a3 100644
--- a/src/puppetlabs/puppetdb/command.clj
+++ b/src/puppetlabs/puppetdb/command.clj
@@ -901,8 +901,9 @@
(let [msg (trs "Unable to shut down delayed command scheduler, requesting server shutdown")]
(log/error msg)
(shut-down-after-command-scheduler-unresponsive
+ ;; This fails request-shutdown schema validation during command-test
#(request-shutdown {:puppetlabs.trapperkeeper.core/exit
- {:status 2 :messages [msg *err*]}})))))
+ {:status 2 :messages [[msg *err*]]}})))))
(def delay-pool-size
"Thread count for delaying messages for retry, and broadcast pool
From 410177e2ad9fe913e59886fa11611c44972b74db Mon Sep 17 00:00:00 2001
From: Rob Browning <rlb@puppet.com>
Date: Wed, 24 Apr 2024 16:47:56 -0500
Subject: [PATCH 04/20] (PDB-5739) Prefer inventory in fact-related query docs
Since the other endpoints have to manufacture a synthetic
representation of the data (i.e. flattened key/values, etc.) and so
are likely to be more expensive, at least when a large number of
factsets is involved.
---
documentation/api/query/v4/fact-contents.markdown | 9 +++++----
documentation/api/query/v4/facts.markdown | 12 +++++++-----
documentation/api/query/v4/factsets.markdown | 8 +++++---
documentation/api/query/v4/inventory.markdown | 6 +++---
4 files changed, 20 insertions(+), 15 deletions(-)
diff --git a/documentation/api/query/v4/fact-contents.markdown b/documentation/api/query/v4/fact-contents.markdown
index c9f5477b1b..5ca41332f6 100644
--- a/documentation/api/query/v4/fact-contents.markdown
+++ b/documentation/api/query/v4/fact-contents.markdown
@@ -16,10 +16,11 @@ canonical: "/puppetdb/latest/api/query/v4/fact-contents.html"
[factsets]: ./factsets.markdown
[nodes]: ./nodes.markdown
-You can query fact information with greater power by using the `/fact-contents`
-endpoint. This endpoint provides the capability to descend into structured
-facts and query tree nodes deep within this data by using the concept of paths
-and values.
+The `/fact-contents` endpoint provides selective access to factset
+subtrees via `fact path`s. Note that the `inventory` endpoint will
+often provide more flexible and efficient access to the same
+information.
+
## Paths and Values
Structured facts can be thought of as trees. For example,
diff --git a/documentation/api/query/v4/facts.markdown b/documentation/api/query/v4/facts.markdown
index b167c73673..5e65b788aa 100644
--- a/documentation/api/query/v4/facts.markdown
+++ b/documentation/api/query/v4/facts.markdown
@@ -16,9 +16,11 @@ canonical: "/puppetdb/latest/api/query/v4/facts.html"
[fact-contents]: ./fact-contents.markdown
[nodes]: ./nodes.markdown
-You can query facts by making an HTTP request to the `/facts` endpoint.
-
-In Puppet's world, you only interact with facts from one node at a time, so any given fact consists of only a **fact name** and a **value.** But because PuppetDB interacts with a whole population of nodes, each PuppetDB fact also includes a **certname** and an **environment.**
+The `/facts` endpoint provides access to a represntation of node
+factsets where a result is returned for each top-level key in the
+node's structured factset. Note that the `inventory` endpoint will
+often provide more flexible and efficient access to the same
+information.
## `/pdb/query/v4/facts`
@@ -37,8 +39,8 @@ See [the AST query language page][ast].
### Query fields
-* `name` (string): the name of the fact.
-* `value` (string, numeric, Boolean): the value of the fact.
+* `name` (string): the top-level name of the fact.
+* `value` (json): the value of the fact.
* `certname` (string): the node associated with the fact.
* `environment` (string): the environment associated with the fact.
diff --git a/documentation/api/query/v4/factsets.markdown b/documentation/api/query/v4/factsets.markdown
index 6d5b08e1ef..fe9f32f299 100644
--- a/documentation/api/query/v4/factsets.markdown
+++ b/documentation/api/query/v4/factsets.markdown
@@ -18,9 +18,11 @@ canonical: "/puppetdb/latest/api/query/v4/factsets.html"
[producers]: ./producers.markdown
[nodes]: ./nodes.markdown
-You can query factsets by making an HTTP request to the `/factsets` endpoint.
-
-A factset is the set of all facts for a single certname.
+The `/factsets` endpoint provides access to a represntation of node
+factsets where each result includes the structured facts for a node
+broken down into a vector of top-level key/value pairs. Note that the
+`inventory` endpoint will often provide more flexible and efficient
+access to the same information.
## `/pdb/query/v4/factsets`
diff --git a/documentation/api/query/v4/inventory.markdown b/documentation/api/query/v4/inventory.markdown
index 95f8012fb1..11f3863c9d 100644
--- a/documentation/api/query/v4/inventory.markdown
+++ b/documentation/api/query/v4/inventory.markdown
@@ -20,9 +20,9 @@ canonical: "/puppetdb/latest/api/query/v4/inventory.html"
[query]: query.markdown
[ast]: ./ast.markdown
-The `/inventory` endpoint enables an alternative query syntax for digging into
-structured facts, and can be used instead of the `facts`, `fact-contents`, and
-`factsets` endpoints for most fact-related queries.
+The `/inventory` endpoint provides an alternate and potentially more
+efficient way to access structured facts as compared to the `facts`,
+`fact-contents`, and `factsets` endpoints.
## `/pdb/query/v4/inventory`
From 17ee84df3b7029998ee1642cbfe3434fba8157d5 Mon Sep 17 00:00:00 2001
From: Rob Browning <rlb@puppet.com>
Date: Tue, 30 Apr 2024 12:55:25 -0500
Subject: [PATCH 05/20] (PDB-5739) fact-query->sql: change stable||volatile to
union
Since we've observed that the || operation can be expensive, and we
don't actually want a full factset map here (we want the individual
top-level subtrees), rewrite it as a union. This shouldn't change
anything material since pdb ensures the two key sets are disjoint.
This also assumes that it's not important to be compatible with the
existing stable||volatile index here. The existing query could only
have been using the index if postgres is clever enough to see all the
way through the name -> key (via jsonb_each/.*) operations to identify
cases where the index might be relevant.
---
src/puppetlabs/puppetdb/query.clj | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/puppetlabs/puppetdb/query.clj b/src/puppetlabs/puppetdb/query.clj
index 0c03a90325..5b90328180 100644
--- a/src/puppetlabs/puppetdb/query.clj
+++ b/src/puppetlabs/puppetdb/query.clj
@@ -385,8 +385,10 @@
FROM (SELECT certname,
producer_timestamp,
environment_id,
- (jsonb_each((stable||volatile))).*
- FROM factsets) fs
+ facts.*
+ FROM factsets,
+ lateral (select * from jsonb_each(stable)
+ union all select * from jsonb_each(volatile)) as facts) fs
LEFT JOIN environments env ON fs.environment_id = env.id
) AS facts
WHERE %s" (column-map->sql fact-columns) where)]
From b0e92a4eb384a7dffc285b58f26ed14403f23617 Mon Sep 17 00:00:00 2001
From: Rob Browning <rlb@puppet.com>
Date: Tue, 30 Apr 2024 12:55:25 -0500
Subject: [PATCH 06/20] (PDB-5739) fact-contents-core: change stable||volatile
to union
Since we've observed that the || operation can be expensive, and we
don't actually want a full factset map here (we want the individual
top-level subtrees), rewrite it as a union. This shouldn't change
anything material since pdb ensures the two key sets are disjoint.
This also assumes that it's not important to be compatible with the
existing stable||volatile index here. The existing query could only
have been using the index if postgres is clever enough to see all the
way through the recursive CTE, name -> key (via jsonb_each/.*)
operations to identify cases where the index might be relevant.
---
src/puppetlabs/puppetdb/query_eng/engine.clj | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/puppetlabs/puppetdb/query_eng/engine.clj b/src/puppetlabs/puppetdb/query_eng/engine.clj
index c17e24dae5..bea22a60a2 100644
--- a/src/puppetlabs/puppetdb/query_eng/engine.clj
+++ b/src/puppetlabs/puppetdb/query_eng/engine.clj
@@ -546,12 +546,11 @@
" left join lateral ("
" with recursive flattened_one (parent_path, parent_types, key, value, type) as ("
" select"
- " array[]::text[],"
- " '',"
- " (jsonb_each(fs.stable||fs.volatile)).*,"
- " 's'"
+ " array[]::text[], '', facts.*, 's'"
+ " from (select * from jsonb_each(fs.stable)"
+ " union all select * from jsonb_each(fs.volatile)) as facts"
" union all"
- ;; -- jsonb_each().* expands into key and value columns
+ ;; -- jsonb_each().* expands into key and value columns via facts.*
" select"
" parent_path || flattened_one.key,"
" parent_types || flattened_one.type,"
From 1ef77367b1c144486993d07b0fb96f76c6b4c068 Mon Sep 17 00:00:00 2001
From: Rob Browning <rlb@puppet.com>
Date: Wed, 24 Apr 2024 18:09:36 -0500
Subject: [PATCH 07/20] (PDB-5739) factsets-query-base: change stable||volatile
to union
Since we've observed that the || operation can be expensive, and we
don't actually want a full factset map here (we want the individual
top-level subtrees), rewrite it as a union. This shouldn't change
anything material since pdb ensures the two key sets are disjoint.
This also assumes that it's not important to be compatible with the
existing stable||volatile index here. The existing query could only
have been using the index if postgres is clever enough to see all the
way through the json_agg -> json_build_object -> name -> key (via
jsonb_each/.*) operations to identify cases where the index might be
relevant.
---
src/puppetlabs/puppetdb/query_eng/engine.clj | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/puppetlabs/puppetdb/query_eng/engine.clj b/src/puppetlabs/puppetdb/query_eng/engine.clj
index bea22a60a2..c40d563718 100644
--- a/src/puppetlabs/puppetdb/query_eng/engine.clj
+++ b/src/puppetlabs/puppetdb/query_eng/engine.clj
@@ -1373,7 +1373,10 @@
[(hsql-hash-as-href "fs.certname" :factsets :facts)
:href]]
:from [[{:select [[:key :name] :value :fs.certname]
- :from [[[:raw "jsonb_each(fs.volatile || fs.stable)"]]]}
+ :from [[{:union-all
+ [{:select :* :from [[[:jsonb_each :fs.stable]]]}
+ {:select :* :from [[[:jsonb_each :fs.volatile]]]}]}
+ :t-union]]}
:t]]}
:facts_data]]}
:join-deps #{:fs}}
From 18d7b8432e30b62fa267a8e7aef577e3ad49c7bc Mon Sep 17 00:00:00 2001
From: Rob Browning <rlb@puppet.com>
Date: Thu, 25 Apr 2024 11:29:04 -0500
Subject: [PATCH 08/20] (PDB-5739) drop-local-unused-joins-from-query: include
cross-joins
Add a missing hsql case.
---
src/puppetlabs/puppetdb/query_eng/engine.clj | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/puppetlabs/puppetdb/query_eng/engine.clj b/src/puppetlabs/puppetdb/query_eng/engine.clj
index c40d563718..623a37e37f 100644
--- a/src/puppetlabs/puppetdb/query_eng/engine.clj
+++ b/src/puppetlabs/puppetdb/query_eng/engine.clj
@@ -3281,7 +3281,8 @@
function-reqs (extract-function-deps plan)
required? (set/union proj-reqs where-reqs function-reqs)
join-key? (if (empty? (:call plan))
- #{:full-join
+ #{:cross-join
+ :full-join
:join
:left-join
:merge-full-join
From 47fb1012e7bfb664e93529114b783c707bb5933d Mon Sep 17 00:00:00 2001
From: Rob Browning <rlb@puppet.com>
Date: Thu, 25 Apr 2024 11:36:55 -0500
Subject: [PATCH 09/20] (PDB-5739) facts-query: change stable||volatile to
union
Since we've observed that the || operation can be expensive, and we
don't actually want a full factset map here (we want the individual
top-level subtrees), rewrite it as a union. This shouldn't change
anything material since pdb ensures the two key sets are disjoint.
This also assumes that it's not important to be compatible with the
existing stable||volatile index here. The existing query could only
have been using the index if postgres is clever enough to see through
the key (via jsonb_each/.*) operation to identify cases where the
index might be relevant.
---
src/puppetlabs/puppetdb/query_eng/engine.clj | 17 +++++++++++------
.../puppetdb/http/query_logging_test.clj | 4 ++--
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/src/puppetlabs/puppetdb/query_eng/engine.clj b/src/puppetlabs/puppetdb/query_eng/engine.clj
index 623a37e37f..92a9efb0fa 100644
--- a/src/puppetlabs/puppetdb/query_eng/engine.clj
+++ b/src/puppetlabs/puppetdb/query_eng/engine.clj
@@ -520,15 +520,20 @@
:queryable? true
:field :fs.value
:join-deps #{:fs}}}
- :selection {:from [[[:raw
- (str "(select certname,"
- " environment_id,"
- " (jsonb_each((stable||volatile))).*"
- " from factsets)")]
+ ;; Consider rewrite-fact-query when making any changes here
+ :selection {:select [:certname :environment_id :key :value]
+ :from [[{:select [:certname :environment_id
+ :facts.key :facts.value]
+ :from :factsets
+ ;; fixme: don't need cross-join?
+ :cross-join [[[:lateral
+ {:union-all
+ [{:select :* :from [[[:jsonb_each :stable]]]}
+ {:select :* :from [[[:jsonb_each :volatile]]]}]}]
+ :facts]]}
:fs]]
:left-join [[:environments :env]
[:= :fs.environment_id :env.id]]}
-
:relationships (merge certname-relations
{"environments" {:local-columns ["environment"]
:foreign-columns ["name"]}
diff --git a/test/puppetlabs/puppetdb/http/query_logging_test.clj b/test/puppetlabs/puppetdb/http/query_logging_test.clj
index 7709721c23..4b1a6d304e 100644
--- a/test/puppetlabs/puppetdb/http/query_logging_test.clj
+++ b/test/puppetlabs/puppetdb/http/query_logging_test.clj
@@ -82,7 +82,7 @@
"foo"]
[["/v4" ["from" "facts"]]
"\"from\",\"facts\""
- "(jsonb_each((stable||volatile)))"
+ " JSONB_EACH(stable) UNION ALL "
nil]
;; stream-query-result
[["/v4/catalogs/myhost.localdomain" [] {:origin "bar"}]
@@ -168,7 +168,7 @@
;; match the AST/SQL logs for facts query
(is (logs-include? logs "\"from\",\"facts\""))
- (is (logs-include? logs "(jsonb_each((stable||volatile)))")))))))))))
+ (is (logs-include? logs " JSONB_EACH(stable) UNION ALL ")))))))))))
(deftest no-PuppetDBServer-tk-service-queries-are-logged-when-log-queries-is-false
(tk-log/with-logged-event-maps logs
From d299c7aeef04aa38b1a67e69b1d51e4904e5ab36 Mon Sep 17 00:00:00 2001
From: Rob Browning <rlb@puppet.com>
Date: Thu, 25 Apr 2024 13:25:52 -0500
Subject: [PATCH 10/20] (PDB-5739) -plan->sql AndExpression/OrExpression:
produce vector
Produce an hsql vector, not a lazy seq.
---
src/puppetlabs/puppetdb/query_eng/engine.clj | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/puppetlabs/puppetdb/query_eng/engine.clj b/src/puppetlabs/puppetdb/query_eng/engine.clj
index 92a9efb0fa..e77fc528c7 100644
--- a/src/puppetlabs/puppetdb/query_eng/engine.clj
+++ b/src/puppetlabs/puppetdb/query_eng/engine.clj
@@ -1740,11 +1740,11 @@
AndExpression
(-plan->sql [expr options]
- (concat [:and] (map #(-plan->sql % options) (:clauses expr))))
+ (apply vector :and (map #(-plan->sql % options) (:clauses expr))))
OrExpression
(-plan->sql [expr options]
- (concat [:or] (map #(-plan->sql % options) (:clauses expr))))
+ (apply vector :or (map #(-plan->sql % options) (:clauses expr))))
NotExpression
(-plan->sql [expr options]
From c9bd7ca1d0c877c831396a6c3cc9c1c458913fa2 Mon Sep 17 00:00:00 2001
From: Rob Browning <rlb@puppet.com>
Date: Thu, 25 Apr 2024 13:39:15 -0500
Subject: [PATCH 11/20] (PDB-5739) query-eng.engine: convert various raw
operations to hsql
---
src/puppetlabs/puppetdb/query_eng/engine.clj | 28 +++++++++++---------
test/puppetlabs/puppetdb/query_eng_test.clj | 2 +-
2 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/src/puppetlabs/puppetdb/query_eng/engine.clj b/src/puppetlabs/puppetdb/query_eng/engine.clj
index e77fc528c7..70007d64bf 100644
--- a/src/puppetlabs/puppetdb/query_eng/engine.clj
+++ b/src/puppetlabs/puppetdb/query_eng/engine.clj
@@ -423,7 +423,9 @@
(assoc-in [:projections "expires_facts"]
{:type :boolean
:queryable? true
- :field [:raw "coalesce(certname_fact_expiration.expire, true)"]
+ :field [:coalesce
+ :certname_fact_expiration.expire
+ [:inline true]]
:join-deps #{:certname_fact_expiration}})
(assoc-in [:projections "expires_facts_updated"]
{:type :timestamp
@@ -1000,7 +1002,7 @@
:join-deps #{:ci}}}
:selection {:from [:certnames]
:join [[{:select [:certname_id
- [[:raw "array_agg(array[catalog_inputs.type, name])"] :inputs]]
+ [[:array_agg [:array [:catalog_inputs.type :name]]] :inputs]]
:from [:catalog_inputs]
:group-by [:certname_id]} :ci]
[:= :certnames.id :ci.certname_id]]
@@ -1373,7 +1375,9 @@
"facts" {:type :queryable-json
:queryable? true
:field {:select [(h/row-to-json :facts_data)]
- :from [[{:select [[[:raw "json_agg(json_build_object('name', t.name, 'value', t.value))"]
+ :from [[{:select [[[:json_agg [:json_build_object
+ [:inline "name"] :t.name
+ [:inline "value"] :t.value]]
:data]
[(hsql-hash-as-href "fs.certname" :factsets :facts)
:href]]
@@ -1502,15 +1506,15 @@
[selection node-purge-ttl]
(let [timestamp (-> node-purge-ttl t/ago t/to-string)]
(assoc selection
- :with {:inactive_nodes {:select [[:certname]]
- :from [:certnames_status]
- ;; Since we use our own bespoke parameter extraction, we cannot use any parameters
- ;; in this wrapping honeysql cte, so we have to format the string ourselves.
- ;; If we can switch to using honeysql for parameter extraction, we can define this
- ;; filter in honeysql and let it convert the timestamps to SQL.
- :where [:raw
- (str "(deactivated IS NOT NULL AND deactivated > '" timestamp "')"
- " OR (expired IS NOT NULL and expired > '" timestamp "')")]}
+ :with {:inactive_nodes
+ {:select [[:certname]]
+ :from [:certnames_status]
+ ;; Because have bespoke parameter extraction,
+ ;; honeysql must not generate any parameters for
+ ;; this cte (i.e. everything must be inline).
+ :where [:or
+ [:and [:<> :deactivated nil] [:> :deactivated [:inline timestamp]]]
+ [:and [:<> :expired nil] [:> :expired [:inline timestamp]]]]}
;; the postgres query planner currently blindly applies De Morgan's law if
;; it can identify that we have negated these is-not null checks, such as
;; in the default query for active nodes. So in addition to relying on the
diff --git a/test/puppetlabs/puppetdb/query_eng_test.clj b/test/puppetlabs/puppetdb/query_eng_test.clj
index 4158ff6b76..720ef0aa93 100644
--- a/test/puppetlabs/puppetdb/query_eng_test.clj
+++ b/test/puppetlabs/puppetdb/query_eng_test.clj
@@ -68,7 +68,7 @@
(deftest test-plan-cte
(is (re-matches
- #"WITH inactive_nodes AS \(SELECT certname FROM certnames_status WHERE \(deactivated IS NOT NULL AND deactivated > '\d\d\d\d-\d\d-\d\dT\d\d:\d\d:\d\d\.\d\d\dZ'\) OR \(expired IS NOT NULL and expired > '\d\d\d\d-\d\d-\d\dT\d\d:\d\d:\d\d\.\d\d\dZ'\)\), not_active_nodes AS \(SELECT certname FROM certnames_status WHERE \(deactivated IS NOT NULL\) OR \(expired IS NOT NULL\)\) SELECT table.foo AS \"foo\" FROM table WHERE \(\? = \?\)"
+ #"WITH inactive_nodes AS \(SELECT certname FROM certnames_status WHERE \(\(deactivated IS NOT NULL\) AND \(deactivated > '\d\d\d\d-\d\d-\d\dT\d\d:\d\d:\d\d\.\d\d\dZ'\)\) OR \(\(expired IS NOT NULL\) AND \(expired > '\d\d\d\d-\d\d-\d\dT\d\d:\d\d:\d\d\.\d\d\dZ'\)\)\), not_active_nodes AS \(SELECT certname FROM certnames_status WHERE \(deactivated IS NOT NULL\) OR \(expired IS NOT NULL\)\) SELECT table\.foo AS \"foo\" FROM table WHERE \(\? = \?\)"
(-> {:projections {"foo" {:type :string
:queryable? true
:field :table.foo}}
From db5a70bc7ff6e2eccae08227b45bfad445b447a7 Mon Sep 17 00:00:00 2001
From: Rob Browning <rlb@puppet.com>
Date: Thu, 25 Apr 2024 13:46:11 -0500
Subject: [PATCH 12/20] (PDB-5739) -plan->sql Query: nest via :nest not (str
...)
---
src/puppetlabs/puppetdb/query_eng/engine.clj | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/puppetlabs/puppetdb/query_eng/engine.clj b/src/puppetlabs/puppetdb/query_eng/engine.clj
index 70007d64bf..650624b907 100644
--- a/src/puppetlabs/puppetdb/query_eng/engine.clj
+++ b/src/puppetlabs/puppetdb/query_eng/engine.clj
@@ -1634,7 +1634,7 @@
true (sql/format :allow-dashed-names? true)
true first)]
(if (:subquery? query)
- [:raw (str " ( " sql " ) ")]
+ [:nest [:raw sql]]
sql)))
InExpression
From 31a9755898fc0477eec49fa47bdb9a2461a8cd90 Mon Sep 17 00:00:00 2001
From: Rob Browning <rlb@puppet.com>
Date: Thu, 25 Apr 2024 16:20:19 -0500
Subject: [PATCH 13/20] (PDB-5739) jsonb-path-binary-expression: convert to
hsql
---
src/puppetlabs/puppetdb/scf/storage_utils.clj | 30 +++++++++++--------
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/src/puppetlabs/puppetdb/scf/storage_utils.clj b/src/puppetlabs/puppetdb/scf/storage_utils.clj
index 7c42bc3a7b..003267522c 100644
--- a/src/puppetlabs/puppetdb/scf/storage_utils.clj
+++ b/src/puppetlabs/puppetdb/scf/storage_utils.clj
@@ -364,19 +364,23 @@
because -> is not indexable (with GIN) but ? is. Assumes a GIN index on the
column supplied."
[op column qmarks]
- (if (= "~" (name op))
- (let [path-elts (cons column qmarks)
- path (apply str
- (str/join "->" (butlast path-elts))
- (when-let [x (last path-elts)] ["->>" x]))]
- [:raw (str/join \space
- [(str "((" path ")") (name op) "(?#>>'{}')"
- "and" column "??" "?)"])])
- (let [delimited-qmarks (str/join "->" qmarks)]
- [:raw (str/join \space
- [(str "(" column "->" delimited-qmarks ")")
- (name op) "?"
- "and" column "??" "?"])])))
+ ;; Internal expectations
+ (when-not (or (vector? column) (keyword? column) (string? column))
+ (throw (ex-info "invalid column type" {:value column})))
+ (when-not (keyword? op)
+ (throw (ex-info "invalid operator type" {:value op})))
+ (let [column (if (string? column) (keyword column) column)]
+ (if (= "~" (name op))
+ (let [path (cond
+ (empty? qmarks) column
+ (empty? (rest qmarks)) [:->> column (first qmarks)]
+ :else [:->> (apply vector :-> column (butlast qmarks)) (last qmarks)])]
+ [:and
+ [:nest [op path [:nest [:#>> [:raw "?"] [:inline "{}"]]]]]
+ [:? column [:raw "?"]]])
+ [:and
+ [op (apply vector :-> column qmarks) [:raw "?"]]
+ [:? column [:raw "?"]]])))
(defn jsonb-scalar-cast
[typ]
From 7e00bac85c89359c8b6e204e2a9cee22f5508eff Mon Sep 17 00:00:00 2001
From: Rob Browning <rlb@puppet.com>
Date: Thu, 25 Apr 2024 16:53:12 -0500
Subject: [PATCH 14/20] (PDB-5739) hsql-hash-as-str hsql-hash-as-href: convert
to hsql
---
src/puppetlabs/puppetdb/query_eng/engine.clj | 30 +++++++++-----------
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/src/puppetlabs/puppetdb/query_eng/engine.clj b/src/puppetlabs/puppetdb/query_eng/engine.clj
index 650624b907..47803bc5c2 100644
--- a/src/puppetlabs/puppetdb/query_eng/engine.clj
+++ b/src/puppetlabs/puppetdb/query_eng/engine.clj
@@ -67,6 +67,7 @@
(def field-schema (s/conditional keyword? s/Keyword
map? {:select s/Any s/Any s/Any}
#(= :raw (first %)) [(s/one (s/eq :raw) "raw") (s/one s/Str "sql")]
+ vector? [s/Any]
:else [s/Keyword]))
; The use of 'column' here is a misnomer. This refers to the result of
@@ -242,19 +243,16 @@
(map-invert pdb-fns->pg-fns))
(defn hsql-hash-as-str
- [column-keyword]
- (->> column-keyword
- name
- su/sql-hash-as-str
- (conj [:raw])))
+ [column-kw]
+ [:encode [:cast column-kw :bytea] [:inline "hex"]])
(defn hsql-hash-as-href
[entity parent child]
- [:raw (str "format("
- (str "'/pdb/query/v4/" (name parent) "/%s/" (name child) "'")
- ", "
- entity
- ")")])
+ [:format
+ [:inline (str "/pdb/query/v4/" (name parent) "/%s/" (name child))]
+ (if (string? entity)
+ [:raw entity]
+ entity)])
(defn hsql-uuid-as-str
[column-keyword]
@@ -734,7 +732,7 @@
:field {:select [(h/row-to-json :t)]
:from [[{:select
[[(h/coalesce :metrics (h/scast :metrics_json :jsonb)) :data]
- [(hsql-hash-as-href (su/sql-hash-as-str "hash") :reports :metrics)
+ [(hsql-hash-as-href (hsql-hash-as-str :hash) :reports :metrics)
:href]]} :t]]}
:join-deps #{:reports}}
"logs" {:type :json
@@ -742,7 +740,7 @@
:field {:select [(h/row-to-json :t)]
:from [[{:select
[[(h/coalesce :logs (h/scast :logs_json :jsonb)) :data]
- [(hsql-hash-as-href (su/sql-hash-as-str "hash") :reports :logs)
+ [(hsql-hash-as-href (hsql-hash-as-str :hash) :reports :logs)
:href]]} :t]]}
:join-deps #{:reports}}
"receive_time" {:type :timestamp
@@ -795,7 +793,7 @@
:field {:select [(h/row-to-json :event_data)]
:from [[{:select
[[(json-agg-row :t) :data]
- [(hsql-hash-as-href (su/sql-hash-as-str "hash") :reports :events) :href]]
+ [(hsql-hash-as-href (hsql-hash-as-str :hash) :reports :events) :href]]
:from [[{:select
[:re.status
:re.timestamp
@@ -888,7 +886,7 @@
:queryable? false
:field {:select [(h/row-to-json :resource_data)]
:from [[{:select [[(json-agg-row :t) :data]
- [(hsql-hash-as-href "c.certname" :catalogs :resources) :href]]
+ [(hsql-hash-as-href :c.certname :catalogs :resources) :href]]
:from [[{:select [[(hsql-hash-as-str :cr.resource) :resource]
:cr.type :cr.title :cr.tags :cr.exported :cr.file :cr.line
[(h/scast :rpc.parameters :json) :parameters]]
@@ -903,7 +901,7 @@
:queryable? false
:field {:select [(h/row-to-json :edge_data)]
:from [[{:select [[(json-agg-row :t) :data]
- [(hsql-hash-as-href "c.certname" :catalogs :edges) :href]]
+ [(hsql-hash-as-href :c.certname :catalogs :edges) :href]]
:from [[{:select [[:sources.type :source_type] [:sources.title :source_title]
[:targets.type :target_type] [:targets.title :target_title]
[:edges.type :relationship]]
@@ -1379,7 +1377,7 @@
[:inline "name"] :t.name
[:inline "value"] :t.value]]
:data]
- [(hsql-hash-as-href "fs.certname" :factsets :facts)
+ [(hsql-hash-as-href :fs.certname :factsets :facts)
:href]]
:from [[{:select [[:key :name] :value :fs.certname]
:from [[{:union-all
From 853b806418ad9dc3e760b49f5944b6f0c80e4f83 Mon Sep 17 00:00:00 2001
From: Rob Browning <rlb@puppet.com>
Date: Thu, 25 Apr 2024 17:10:53 -0500
Subject: [PATCH 15/20] (PDB-5739) fn-binary-expression-hsql: convert to hsql
---
src/puppetlabs/puppetdb/query_eng/engine.clj | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/src/puppetlabs/puppetdb/query_eng/engine.clj b/src/puppetlabs/puppetdb/query_eng/engine.clj
index 47803bc5c2..11f6d92213 100644
--- a/src/puppetlabs/puppetdb/query_eng/engine.clj
+++ b/src/puppetlabs/puppetdb/query_eng/engine.clj
@@ -1593,16 +1593,15 @@
provided value. The operator, function name and its arguments must
have already been validated."
[op function args]
- (let [quoted-args
+ (let [args
(case function
- ("sum" "avg" "min" "max" "count" "jsonb_typeof") (mapv jdbc/double-quote args)
- "to_char" [(jdbc/double-quote (first args)) (jdbc/single-quote (second args))]
+ ("sum" "avg" "min" "max" "count" "jsonb_typeof") (map keyword args)
+ "to_char" [(-> args first keyword) [:inline (second args)]]
(throw (IllegalArgumentException. (tru "{0} is not a valid function"
(pr-str function)))))]
- [:raw (format "%s(%s) %s ?"
- function
- (str/join ", " quoted-args)
- op)]))
+ [(keyword op)
+ (apply vector (keyword function) args)
+ [:raw "?"]]))
(defn munge-query-ordering
[clauses]
From a0bf73b4186796265e5779669e5529dc785458ac Mon Sep 17 00:00:00 2001
From: Rob Browning <rlb@puppet.com>
Date: Thu, 25 Apr 2024 17:54:42 -0500
Subject: [PATCH 16/20] (PDB-5739) JsonContainsExpression: convert to hsql
---
src/puppetlabs/puppetdb/query_eng/engine.clj | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/src/puppetlabs/puppetdb/query_eng/engine.clj b/src/puppetlabs/puppetdb/query_eng/engine.clj
index 11f6d92213..2456742e88 100644
--- a/src/puppetlabs/puppetdb/query_eng/engine.clj
+++ b/src/puppetlabs/puppetdb/query_eng/engine.clj
@@ -6,7 +6,7 @@
[clojure.tools.logging :as log]
[honey.sql :as sql]
[honey.sql.helpers :as hsql]
- [honey.sql.pg-ops] ;; require to enable postgres ops
+ [honey.sql.pg-ops :as pgop] ;; require to enable postgres ops
[puppetlabs.kitchensink.core :as ks]
[puppetlabs.puppetdb.facts :as facts]
[puppetlabs.puppetdb.honeysql :as h]
@@ -1651,16 +1651,13 @@
:from [[(-plan->sql subquery options) :sub]]}]))
JsonContainsExpression
- (-plan->sql [{:keys [field column-data array-in-path]} _opts]
- (let [f (if (= :raw (:field-type column-data))
- (-> column-data :field second)
- field)]
- ;; This distinction is necessary (at least) because @> cannot
- ;; traverse into arrays, but should be otherwise preferred
- ;; because it can use an index.
- (if array-in-path
- [:raw (format "%s #> ? = ?" f)]
- [:raw (format "%s @> ?" f)])))
+ (-plan->sql [{:keys [column-data array-in-path]} _opts]
+ ;; This distinction is necessary (at least) because @> cannot
+ ;; traverse into arrays, but should be otherwise preferred
+ ;; because it can use an index.
+ (if array-in-path
+ [:= [:#> (:field column-data) [:raw "?"]] [:raw "?"]]
+ [pgop/at> (:field column-data) [:raw "?"]]))
FnBinaryExpression
(-plan->sql [{:keys [function args operator]} _opts]
From 979afa86e5df2ffffa2ee8936602a290b59dca10 Mon Sep 17 00:00:00 2001
From: Rob Browning <rlb@puppet.com>
Date: Thu, 25 Apr 2024 18:04:43 -0500
Subject: [PATCH 17/20] (PDB-5739) -plan->sql NullExpression: convert to hsql
---
src/puppetlabs/puppetdb/query_eng/engine.clj | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/src/puppetlabs/puppetdb/query_eng/engine.clj b/src/puppetlabs/puppetdb/query_eng/engine.clj
index 2456742e88..bf8cb169df 100644
--- a/src/puppetlabs/puppetdb/query_eng/engine.clj
+++ b/src/puppetlabs/puppetdb/query_eng/engine.clj
@@ -1724,17 +1724,19 @@
(-plan->sql [{:keys [column] :as expr} options]
(s/validate column-schema column)
(let [queryable-json? (= :queryable-json (:type column))
+ hsql-col? (and (vector? (:field column))
+ (not (= :raw (-> column :field first))))
lhs (if queryable-json?
- (name (h/extract-sql (:field column)))
+ (if hsql-col?
+ (:field column)
+ (name (h/extract-sql (:field column))))
(-plan->sql (:field column) options))
json? (or queryable-json? (= :jsonb-scalar (:type column)))]
- (if (:null? expr)
- (if json?
- (su/jsonb-null? lhs true)
- [:is lhs nil])
- (if json?
- (su/jsonb-null? lhs false)
- [:is-not lhs nil]))))
+ (if json?
+ (if hsql-col?
+ [(if (:null? expr) := :<>) [:jsonb_typeof lhs] [:inline "null"]]
+ (su/jsonb-null? lhs (boolean (:null? expr))))
+ [(if (:null? expr) :is :is-not) lhs nil])))
AndExpression
(-plan->sql [expr options]
From f3ecea669e446ee8532287f6f4a10125d0a286c6 Mon Sep 17 00:00:00 2001
From: Rob Browning <rlb@puppet.com>
Date: Mon, 29 Apr 2024 14:45:22 -0500
Subject: [PATCH 18/20] (PDB-5739) create-json-path-extraction: convert to hsql
---
src/puppetlabs/puppetdb/jdbc.clj | 12 -----
src/puppetlabs/puppetdb/query_eng/engine.clj | 49 +++++++++++---------
test/puppetlabs/puppetdb/query_eng_test.clj | 8 ++--
3 files changed, 32 insertions(+), 37 deletions(-)
diff --git a/src/puppetlabs/puppetdb/jdbc.clj b/src/puppetlabs/puppetdb/jdbc.clj
index 5c78853e44..5eb390dd4e 100644
--- a/src/puppetlabs/puppetdb/jdbc.clj
+++ b/src/puppetlabs/puppetdb/jdbc.clj
@@ -436,18 +436,6 @@
(.setType "text[]")
(.setValue (str \{ (str/join \, quoted) \})))))
-;; Q: move/replace?
-(defn create-json-path-extraction
- "Given a base json field and a path of keys to traverse, construct the proper
- SQL query of the form base->'key'->'key2'..."
- [field path]
- (str field
- (when (seq path)
- (->> (map single-quote path)
- (str/join "->")
- ;; prefix for the first arrow in field->'key1'...
- (str "->")))))
-
;; TODO this is no longer used by the main query engine (engine.clj), but it is
;; still used by the legacy query engines and should be removed once it is
;; handled by honeysql everywhere
diff --git a/src/puppetlabs/puppetdb/query_eng/engine.clj b/src/puppetlabs/puppetdb/query_eng/engine.clj
index bf8cb169df..e485a75357 100644
--- a/src/puppetlabs/puppetdb/query_eng/engine.clj
+++ b/src/puppetlabs/puppetdb/query_eng/engine.clj
@@ -2270,25 +2270,34 @@
(contains? (ks/keyset user-name->query-rec-name)
(first expr)))
+(defn create-json-path-extraction
+ "Given a base json field and a path of keys to traverse, construct the proper
+ SQL query of the form base->'key'->'key2'..."
+ [field path]
+ (when (string? field) ; internal expectation
+ (throw (ex-info "invalid field type" {:value field})))
+ ;; The str is to make sure 5 comes through as '5' -- see comments below.
+ (apply vector :-> field (map #(vector :inline (str %)) path)))
+
(defn create-json-subtree-projection
[[top & path :as _parsed-field] projections]
- (let [stringify (fn [{:keys [field field-type]}]
- (case field-type
- :keyword (name field)
- :raw (second field)
- (throw (IllegalArgumentException.
- (tru "Cannot determine field type for field ''{0}''" field)))))]
- {:type :json
- :queryable? false
- :field [:raw
- (jdbc/create-json-path-extraction
- ;; Extract the original field as a string
- (-> top :name projections stringify)
- (map #(case (:kind %)
- ::parse/named-field-part (:name %))
- path))]
- ;; json-path will be something like ("facts" "kernel" ...)
- :join-deps (get-in projections [(:name top) :join-deps])}))
+ {:type :json
+ :queryable? false
+ :field (create-json-path-extraction
+ ;; Extract the original field as a string
+ (let [{:keys [field field-type]} (-> top :name projections)]
+ (case field-type
+ (:keyword :raw) field
+ (do
+ (when-not (vector? field)
+ (throw (IllegalArgumentException.
+ (tru "Cannot determine field type for field ''{0}''" field))))
+ field)))
+ (map #(case (:kind %)
+ ::parse/named-field-part (:name %))
+ path))
+ ;; json-path will be something like ("facts" "kernel" ...)
+ :join-deps (get-in projections [(:name top) :join-deps])})
(defn create-extract-node*
"Returns a `query-rec` that has the correct projection for the given
@@ -2655,10 +2664,8 @@
cinfo (get-in query-rec [:projections (:name top)])]
(if (and (= :queryable-json (:type cinfo))
(seq path))
- (let [field (name (h/extract-sql (:field cinfo)))
- json-path (->> (map :name path)
- (jdbc/create-json-path-extraction field)
- (conj [:raw]))]
+ (let [field (:field cinfo)
+ json-path (create-json-path-extraction field (map :name path))]
(map->NullExpression {:column (assoc cinfo :field json-path)
:null? value}))
(map->NullExpression {:column cinfo :null? value})))
diff --git a/test/puppetlabs/puppetdb/query_eng_test.clj b/test/puppetlabs/puppetdb/query_eng_test.clj
index 720ef0aa93..336a446b09 100644
--- a/test/puppetlabs/puppetdb/query_eng_test.clj
+++ b/test/puppetlabs/puppetdb/query_eng_test.clj
@@ -176,26 +176,26 @@
(deftest test-extract-json-subtree-compiles
(testing "with differing levels of subtrees"
- (is (re-find #"SELECT \(fs.stable\|\|fs.volatile\)->'os' AS \"facts\.os\" .*FROM factsets"
+ (is (re-find #"SELECT \(fs.stable\|\|fs.volatile\) -> 'os' AS \"facts\.os\" .*FROM factsets"
(->> ["extract" "facts.os"]
(compile-user-query->sql inventory-query)
:results-query
first)))
- (is (re-find #"SELECT \(fs.stable\|\|fs.volatile\)->'os'->'family' AS \"facts\.os\.family\" .*FROM factsets"
+ (is (re-find #"SELECT \(fs.stable\|\|fs.volatile\) -> 'os' -> 'family' AS \"facts\.os\.family\" .*FROM factsets"
(->> ["extract" "facts.os.family"]
(compile-user-query->sql inventory-query)
:results-query
first))))
(testing "when field is raw sql"
- (is (re-find #"SELECT \(fs.stable\|\|fs.volatile\)->'trusted'->'certname' AS \"trusted\.certname\" .*FROM factsets"
+ (is (re-find #"SELECT \(fs.stable\|\|fs.volatile\)->'trusted' -> 'certname' AS \"trusted\.certname\" .*FROM factsets"
(->> ["extract" "trusted.certname"]
(compile-user-query->sql inventory-query)
:results-query
first))))
(testing "when field is a keyword"
- (is (re-find #"SELECT rpc.parameters->'foo' AS \"parameters\.foo\" .*FROM catalog_resources"
+ (is (re-find #"SELECT rpc.parameters -> 'foo' AS \"parameters\.foo\" .*FROM catalog_resources"
(->> ["extract" "parameters.foo"]
(compile-user-query->sql resources-query)
:results-query
From e38db4b8ab0e73291e17fee52ef65cbbf5ad5429 Mon Sep 17 00:00:00 2001
From: Rob Browning <rlb@puppet.com>
Date: Mon, 29 Apr 2024 15:17:47 -0500
Subject: [PATCH 19/20] (PDB-5739) inventory-query: convert stable||volatile to
hsql
---
src/puppetlabs/puppetdb/query_eng/engine.clj | 10 ++++------
test/puppetlabs/puppetdb/query_eng_test.clj | 11 ++++++-----
2 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/src/puppetlabs/puppetdb/query_eng/engine.clj b/src/puppetlabs/puppetdb/query_eng/engine.clj
index e485a75357..fcf9ab8cde 100644
--- a/src/puppetlabs/puppetdb/query_eng/engine.clj
+++ b/src/puppetlabs/puppetdb/query_eng/engine.clj
@@ -281,14 +281,12 @@
"facts" {:type :queryable-json
:projectable-json? true
:queryable? true
- :field [:raw "(fs.stable||fs.volatile)"]
- :field-type :raw
+ :field [:nest [:|| :fs.stable :fs.volatile]]
:join-deps #{:fs}}
"trusted" {:type :queryable-json
:projectable-json? true
:queryable? true
- :field [:raw "(fs.stable||fs.volatile)->'trusted'"]
- :field-type :raw
+ :field [:-> [:|| :fs.stable :fs.volatile] [:inline "trusted"]]
:join-deps #{:fs}}}
:selection {:from [[:factsets :fs]]
@@ -1666,8 +1664,8 @@
JsonbPathBinaryExpression
(-plan->sql [{:keys [field value column-data operator]} _opts]
(su/jsonb-path-binary-expression operator
- (if (= :raw (:field-type column-data))
- (-> column-data :field second)
+ (if (vector? (:field column-data))
+ (:field column-data)
field)
value))
diff --git a/test/puppetlabs/puppetdb/query_eng_test.clj b/test/puppetlabs/puppetdb/query_eng_test.clj
index 336a446b09..a134a6d028 100644
--- a/test/puppetlabs/puppetdb/query_eng_test.clj
+++ b/test/puppetlabs/puppetdb/query_eng_test.clj
@@ -176,19 +176,19 @@
(deftest test-extract-json-subtree-compiles
(testing "with differing levels of subtrees"
- (is (re-find #"SELECT \(fs.stable\|\|fs.volatile\) -> 'os' AS \"facts\.os\" .*FROM factsets"
+ (is (re-find #"SELECT \(fs.stable \|\| fs.volatile\) -> 'os' AS \"facts\.os\" .*FROM factsets"
(->> ["extract" "facts.os"]
(compile-user-query->sql inventory-query)
:results-query
first)))
- (is (re-find #"SELECT \(fs.stable\|\|fs.volatile\) -> 'os' -> 'family' AS \"facts\.os\.family\" .*FROM factsets"
+ (is (re-find #"SELECT \(fs.stable \|\| fs.volatile\) -> 'os' -> 'family' AS \"facts\.os\.family\" .*FROM factsets"
(->> ["extract" "facts.os.family"]
(compile-user-query->sql inventory-query)
:results-query
first))))
(testing "when field is raw sql"
- (is (re-find #"SELECT \(fs.stable\|\|fs.volatile\)->'trusted' -> 'certname' AS \"trusted\.certname\" .*FROM factsets"
+ (is (re-find #"SELECT \(\(fs.stable \|\| fs.volatile\) -> 'trusted'\) -> 'certname' AS \"trusted\.certname\" .*FROM factsets"
(->> ["extract" "trusted.certname"]
(compile-user-query->sql inventory-query)
:results-query
@@ -201,14 +201,15 @@
:results-query
first)))))
+;; ??? (and check all other || and/or jsonb_each in tests)
(deftest index-hit-for-trusted-facts-on-inventory
(testing "facts.trusted.extensions and trusted.extensions should generate the same SQL"
- (is (re-find #"WHERE \(fs.stable\|\|fs.volatile\) @> ?"
+ (is (re-find #"WHERE \(fs.stable \|\| fs.volatile\) @> ?"
(->> ["extract" [] ["=" "facts.trusted.extensions.foo" "bar"]]
(compile-user-query->sql inventory-query)
:results-query
first)))
- (is (re-find #"WHERE \(fs.stable\|\|fs.volatile\) @> ?"
+ (is (re-find #"WHERE \(fs.stable \|\| fs.volatile\) @> ?"
(->> ["extract" [] ["=" "trusted.extensions.foo" "bar"]]
(compile-user-query->sql inventory-query)
:results-query
From 9df5ae255014e0f0b838014c3a0bcb15ba1d189c Mon Sep 17 00:00:00 2001
From: Rob Browning <rlb@puppet.com>
Date: Mon, 13 May 2024 16:06:01 -0500
Subject: [PATCH 20/20] (PDB-5745) rewrite-fact-query: convert to hsql
---
src/puppetlabs/puppetdb/query_eng/engine.clj | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/src/puppetlabs/puppetdb/query_eng/engine.clj b/src/puppetlabs/puppetdb/query_eng/engine.clj
index fcf9ab8cde..a34c4ea83c 100644
--- a/src/puppetlabs/puppetdb/query_eng/engine.clj
+++ b/src/puppetlabs/puppetdb/query_eng/engine.clj
@@ -3017,15 +3017,16 @@
;; and thus still have no results.
(let [fact-name (name-constraint clause)]
[(update facts-query :selection assoc
- :from [[[:raw (str "(select certname,"
- " environment_id, "
- " ?::text as key, "
- " (stable||volatile)->? as value"
- " from factsets"
- " where (stable||volatile) ?? ?"
- ")")]
+ :from [[{:select [:certname :environment_id
+ [[:cast [:raw "?" ] :text] :key]
+ [[[:coalesce
+ [:-> :volatile [:raw "?"]]
+ [:-> :stable [:raw "?"]]]]
+ :value]]
+ :from :factsets
+ :where [[:? [:|| :stable :volatile] [:raw "?"]]]}
:fs]]
- :selection-params [fact-name fact-name fact-name])
+ :selection-params [fact-name fact-name fact-name fact-name])
user-query])
:else
[query-rec user-query])))