Skip to content

Report affected rows on postgres, and rewrite placeholders in linear time #8

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yogthos merged 2 commits into from
Aug 11, 2026
Merged

Report affected rows on postgres, and rewrite placeholders in linear time #8

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
15e8334
Report affected rows on postgres, and rewrite placeholders in linear …
Aug 11, 2026
ec58ee1
Do not read a $ inside an identifier as a dollar quote
Aug 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 80 additions & 0 deletions clj-test/jdbc/core_test.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,58 @@
(jdbc/execute! c "create table t (x integer)")
(jdbc/insert! c :t {:x 7})))

;; Rewriting ? to $N decides which parameter goes where, so a ? that is not
;; really a placeholder must neither be rewritten nor consume a number. These
;; run without a database, so the sqlite-only build covers them.
(println "postgres placeholder rewriting")
(let [rewrite (deref (resolve (symbol "jdbc.core" "pg-placeholders")))]
(doseq [[label in out]
[["no placeholders" "select 1" "select 1"]
["bare placeholder" "?" "$1"]
["several" "? ? ?" "$1 $2 $3"]
["two digits" "? ? ? ? ? ? ? ? ? ? ?" "$1 $2 $3 $4 $5 $6 $7 $8 $9 $10 $11"]
["string literal" "select '?' as a, ? as b" "select '?' as a, $1 as b"]
["doubled quote escape" "select 'it''s ?' , ?" "select 'it''s ?' , $1"]
["quoted identifier" "select \"c?\" from t where x = ?"
"select \"c?\" from t where x = $1"]
["doubled double quote" "select \"a\"\"?\" , ?" "select \"a\"\"?\" , $1"]
["line comment" "select ? -- is this right?\n, ?"
"select $1 -- is this right?\n, $2"]
["block comment" "select /* ? */ ? as c" "select /* ? */ $1 as c"]
["nested block comment" "select /* a /* ? */ ? */ ? as c"
"select /* a /* ? */ ? */ $1 as c"]
["dollar quoted" "select $$a?b$$ , ?" "select $$a?b$$ , $1"]
["dollar quoted with tag" "select $tag$ ? $tag$, ?" "select $tag$ ? $tag$, $1"]
["dollar quote after an operator" "select 1 where x = $$?$$ or y = ?"
"select 1 where x = $$?$$ or y = $1"]
;; postgres allows $ inside an identifier after the first character,
;; so a$b$c is one name rather than a dollar quote opening
["dollar inside an identifier" "select note from t where a$b$c = ?"
"select note from t where a$b$c = $1"]
["dollar inside an identifier, two params" "update t set note = ? where a$b$c = ?"
"update t set note = $1 where a$b$c = $2"]
["escape string" "select E'\\'?' , ?" "select E'\\'?' , $1"]
;; only a standalone E introduces backslash escapes; here the e ends
;; an identifier, so this is a plain literal and the backslash is
;; literal too, which leaves the ? after it a real placeholder
["e ending an identifier is not an escape string" "select code'a\\' , ?"
"select code'a\\' , $1"]
["unterminated literal" "select '?" "select '?"]
["unterminated comment" "select /* ?" "select /* ?"]]]
(check (str "placeholders, " label) out (rewrite in)))
;; Rebuilding the statement one character at a time made this quadratic: 40KB
;; of SQL already cost 129ms of pure string copying before the query was sent.
;; The bound is loose enough not to be flaky and still far under what
;; quadratic would need for half a megabyte.
(let [big (str "select ? /* " (apply str (repeat 500000 "x")) " */")
t0 (System/currentTimeMillis)
got (rewrite big)
ms (- (System/currentTimeMillis) t0)]
(check "placeholders, half a megabyte of sql rewrites correctly" true
(and (clojure.string/starts-with? got "select $1 /* ")
(= (count got) (inc (count big)))))
(check (str "placeholders, half a megabyte stays linear (" ms " ms)") true (< ms 5000))))

(when-let [pg-uri (System/getenv "JOLT_TEST_PG_URI")]
(println "jdbc.core over postgres (" pg-uri ")")
(with-open [conn (jdbc/connection pg-uri)]
Expand Down Expand Up @@ -114,6 +166,34 @@
(try
(jdbc/fetch conn "select * from jolt_missing_table")
(catch Exception _ :caught)))
;; execute! promises rows affected, and sqlite delivers it; postgres used to
;; return nil from all three of these
(check "pg execute! returns rows affected" 1
(jdbc/execute! conn ["insert into jolt_person (name, zip) values (?, ?)" "rows" 7]))
(check "pg update! returns rows affected" 1
(jdbc/update! conn :jolt_person {:zip 8} ["name = ?" "rows"]))
(check "pg delete! returns rows affected" 1
(jdbc/delete! conn :jolt_person ["name = ?" "rows"]))
(check "pg execute! returns 0 affected for ddl" 0
(jdbc/execute! conn "create table jolt_counts (x integer)"))
(check "pg execute! counts a multi-row update" 2
(do (jdbc/insert-multi! conn :jolt_counts [{:x 1} {:x 1} {:x 2}])
(jdbc/update! conn :jolt_counts {:x 9} ["x = ?" 1])))
(jdbc/execute! conn "drop table jolt_counts")
(check "pg ? inside a comment does not consume a parameter" "v"
(:c (jdbc/fetch-one conn ["select /* ? */ ? as c" "v"])))
(check "pg ? inside a quoted identifier is left alone" "v"
(:c (jdbc/fetch-one conn ["select ? as \"c\" /* ? */" "v"])))
;; postgres accepts $ inside an identifier, and reading a$b$c as a dollar
;; quote swallowed the rest of the statement and dropped the placeholder
(check "pg $ inside an identifier is not a dollar quote" "x"
(do (jdbc/execute! conn "drop table if exists jolt_dollar")
(jdbc/execute! conn "create table jolt_dollar (a$b$c integer, note text)")
(jdbc/execute! conn ["insert into jolt_dollar (a$b$c, note) values (?, ?)" 1 "x"])
(let [v (:note (jdbc/fetch-one conn
["select note from jolt_dollar where a$b$c = ?" 1]))]
(jdbc/execute! conn "drop table jolt_dollar")
v)))
(jdbc/execute! conn "drop table if exists jolt_payload")
(jdbc/execute! conn "create table jolt_payload (id serial primary key, content bytea not null)")
;; bytea reads back as text, in whichever format bytea_output names, so run
Expand Down
11 changes: 10 additions & 1 deletion clj/db/pg.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
(ffi/defcfn PQexecParams "PQexecParams" [:pointer :string :int :pointer :pointer :pointer :pointer :int] :pointer)
(ffi/defcfn PQresultStatus "PQresultStatus" [:pointer] :int)
(ffi/defcfn PQresultErrorMessage "PQresultErrorMessage" [:pointer] :string)
(ffi/defcfn PQcmdTuples "PQcmdTuples" [:pointer] :string)
(ffi/defcfn PQntuples "PQntuples" [:pointer] :int)
(ffi/defcfn PQnfields "PQnfields" [:pointer] :int)
(ffi/defcfn PQfname "PQfname" [:pointer :int] :string)
Expand Down Expand Up @@ -152,7 +153,15 @@
{:sql sql :jdbc/sql-error true}))))
res)))

(defn exec [conn sql params] (PQclear (run conn sql params)) nil)
(defn exec
"Run a statement and return the number of rows it affected. Commands that do not
report a count, DDL among them, give 0 — which is what sqlite3_changes reports
for those too."
[conn sql params]
(let [res (run conn sql params)
n (PQcmdTuples res)] ; must be read before PQclear
(PQclear res)
(or (when n (parse-long n)) 0)))

(defn- coerce [oid s]
(cond (int-oids oid) (parse-long s)
Expand Down
139 changes: 126 additions & 13 deletions clj/jdbc/core.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,17 +87,127 @@
(vector? q) [(first q) (vec (rest q))]
:else (throw (ex-info "query must be a string or sqlvec" {:q q}))))

;;; ? -> $N rewriting
;;
;; Which ? counts as a placeholder decides which parameter lands where, so a ?
;; that only looks like one has to be skipped without consuming a number. That
;; means recognising the constructs a ? can hide in: string literals, quoted
;; identifiers, line and block comments, dollar-quoted bodies, and E'' escape
;; strings. Each is skipped whole by the scanner below.

(defn- at
"The character at `i`, or nil past the end, so callers can compare without
bounds-checking first."
[sql len i]
(when (< i len) (nth sql i)))

(defn- digit? [c] (let [x (int c)] (and (>= x 48) (<= x 57))))

(defn- ident-char? [c]
(let [x (int c)]
(or (and (>= x 97) (<= x 122)) ; a-z
(and (>= x 65) (<= x 90)) ; A-Z
(and (>= x 48) (<= x 57)) ; 0-9
(= x 95) ; _
(> x 127)))) ; postgres allows non-ascii here

(defn- token-start?
"True when `i` begins a token rather than continuing an identifier. Postgres
allows $ inside an identifier after the first character, so a$b$c is one name
and not a dollar quote opening, and E only introduces an escape string when it
stands alone rather than ending a word like date'2020-01-01'. Its own lexer
draws the line in the same place."
[sql i]
(or (zero? i)
(let [p (nth sql (dec i))]
(not (or (ident-char? p) (= p \$))))))

(defn- skip-quoted
"Index just past the run of quote character `q` opening at `i`. A doubled quote
is an escaped one; `escapes?` additionally honours backslash escapes, which is
what distinguishes E'...' from a plain literal. An unterminated run ends at the
end of the statement rather than throwing."
[sql len i q escapes?]
(loop [j (inc i)]
(cond
(>= j len) len
(and escapes? (= (nth sql j) \\)) (recur (+ j 2))
(not= (nth sql j) q) (recur (inc j))
(= (at sql len (inc j)) q) (recur (+ j 2))
:else (inc j))))

(defn- skip-line-comment [sql len i]
(loop [j (+ i 2)]
(cond (>= j len) len
(= (nth sql j) \newline) j
:else (recur (inc j)))))

(defn- skip-block-comment
"Index just past the /* */ comment opening at `i`. Postgres nests these, so
track depth rather than stopping at the first */."
[sql len i]
(loop [j (+ i 2) depth 1]
(cond
(>= j len) len
(and (= (nth sql j) \/) (= (at sql len (inc j)) \*)) (recur (+ j 2) (inc depth))
(and (= (nth sql j) \*) (= (at sql len (inc j)) \/)) (if (= depth 1)
(+ j 2)
(recur (+ j 2) (dec depth)))
:else (recur (inc j) depth))))

(defn- dollar-tag-len
"Length of the $tag$ that opens at `i`, or nil when this $ does not open a
dollar quote. The tag follows unquoted-identifier rules, so it cannot start
with a digit, which is what keeps a positional $1 from being read as one."
[sql len i]
(loop [j (inc i)]
(let [c (at sql len j)]
(cond
(nil? c) nil
(= c \$) (- (inc j) i)
(and (= j (inc i)) (digit? c)) nil
(ident-char? c) (recur (inc j))
:else nil))))

(defn- skip-dollar-quoted [sql len i taglen]
(let [tag (subs sql i (+ i taglen))]
(if-let [close (str/index-of sql tag (+ i taglen))]
(+ close taglen)
len)))

(defn- pg-placeholders
"JDBC ? placeholders -> postgres $1..$N (skipping ? inside '...' literals)."
"JDBC ? placeholders -> postgres $1..$N. A ? inside a string literal, quoted
identifier, comment, dollar-quoted body or escape string is left as it is and
does not consume a number. Collects the pieces and joins them once, so the cost
is linear in the length of the statement."
[sql]
(loop [out "" i 0 n 1 in-str false]
(if (= i (count sql))
out
(let [c (subs sql i (inc i))]
(cond
(= c "'") (recur (str out c) (inc i) n (not in-str))
(and (= c "?") (not in-str)) (recur (str out "$" n) (inc i) (inc n) in-str)
:else (recur (str out c) (inc i) n in-str))))))
(let [len (count sql)]
(loop [i 0 from 0 pnum 1 pieces (transient [])]
(if (>= i len)
(str/join (persistent! (conj! pieces (subs sql from len))))
(let [c (nth sql i)
nxt (at sql len (inc i))]
(cond
(= c \?)
(recur (inc i) (inc i) (inc pnum)
(conj! (conj! pieces (subs sql from i)) (str "$" pnum)))

(= c \') (recur (skip-quoted sql len i \' false) from pnum pieces)
(= c \") (recur (skip-quoted sql len i \" false) from pnum pieces)

;; E'...' / e'...', where a backslash escapes the next character
(and (or (= c \E) (= c \e)) (= nxt \') (token-start? sql i))
(recur (skip-quoted sql len (inc i) \' true) from pnum pieces)

(and (= c \-) (= nxt \-)) (recur (skip-line-comment sql len i) from pnum pieces)
(and (= c \/) (= nxt \*)) (recur (skip-block-comment sql len i) from pnum pieces)

(= c \$)
(if-let [taglen (and (token-start? sql i) (dollar-tag-len sql len i))]
(recur (skip-dollar-quoted sql len i taglen) from pnum pieces)
(recur (inc i) from pnum pieces))

:else (recur (inc i) from pnum pieces)))))))

(defn- sqlite-eval [conn sql params]
(sqlite/query (:handle conn) sql params))
Expand Down Expand Up @@ -132,10 +242,11 @@
(case (:vendor conn)
:sqlite (do (sqlite-eval conn sql params)
(sqlite/changes (:handle conn)))
:postgresql (do (pg-eval conn sql params) nil)))))
:postgresql (pg-eval conn sql params)))))

(defn last-insert-id
"Driver-specific id of the last inserted row (sqlite: last_insert_rowid)."
"Driver-specific id of the last inserted row (sqlite: last_insert_rowid,
postgres: lastval, which needs the session to have used a sequence)."
[conn]
(case (:vendor conn)
:sqlite (sqlite/last-insert-rowid (:handle conn))
Expand All @@ -146,8 +257,10 @@
(defn- entity-str [entities x] (entities (if (keyword? x) (name x) (str x))))

(defn insert!
"Insert one row map. Returns the generated id (sqlite) / nil (postgres —
use \"... returning *\" with execute!/fetch for the row)."
"Insert one row map and return the generated id, from last_insert_rowid on
sqlite and lastval on postgres. Inserting into a postgres table with no
sequence therefore throws, since lastval has nothing to report — use
\"... returning ...\" with execute!/fetch for that case."
([conn table row] (insert! conn table row {}))
([conn table row opts]
(let [entities (get opts :entities identity)
Expand Down
Loading