ref(pdo): fix boundary/coercion bugs and polish wrapper DX

CHANGELOG.md

@@ -24,6 +24,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - `pdo/get-attribute` / `pdo/set-attribute` now dispatch on the handle: pass a connection (as before) or a statement to reach `PDOStatement::getAttribute` / `setAttribute` ([#12]).
 - `pdo/error-code` / `pdo/error-info` now dispatch on the handle: pass a statement to read `PDOStatement::errorCode` / `errorInfo` ([#14]).
+- **BC** `pdo/error-code` now returns the SQLSTATE **string** (e.g. `"23000"`, `"HY000"`) instead of an int, and `pdo/error-info`'s first element is likewise the SQLSTATE string (the `driver-code` second element stays an int). SQLSTATE is a 5-character code; the old `intval` corrupted non-numeric states like `HY000` (→ `0`) and `42S02` (→ `42`).
+- **BC** `pdo/last-insert-id` and `pdo/insert` now return the id as a **string** (as PDO reports it) instead of coercing to int. This is lossless for big integers and PostgreSQL named sequences; call `php/intval` at the call site if you need a number.
+- **BC** `pdo/get-available-drivers` no longer takes a connection argument - `PDO::getAvailableDrivers` is static. Call `(pdo/get-available-drivers)`.
+
+### Fixed
+
+- `pdo/connect` and `pdo/prepare` now convert a Phel options map (with integer `\PDO/ATTR_*` keys) into a PHP array; previously passing an options map raised a `TypeError` because `phel->php` cannot convert integer map keys.
+- `pdo/bind-value` / `pdo/bind-param` now keep an integer `column` as a 1-based positional index instead of stringifying it, fixing positional (`?`) parameter binding.
+- `pdo/insert` now rejects an empty `row` map with an `InvalidArgumentException` instead of generating invalid SQL.
+- `pdo/set-fetch-mode` now throws an `InvalidArgumentException` on unsupported arity (more than 2 extra args) instead of silently doing nothing.
 
 ## [0.1.0] - 2026-05-13
 

README.md

@@ -32,7 +32,7 @@ Requires PHP `>=8.4` and `phel-lang/phel-lang ^0.37`.
 
 ;; Insert a row from a map
 (pdo/insert conn :t1 {:name "lisp"})
-;; => 3   ; new last-insert-id
+;; => "3"   ; new last-insert-id (string, as PDO reports it)
 ```
 
 `pdo/fetch` returns the row as a map keyed by column keyword, or `nil` when no rows remain.
@@ -74,15 +74,15 @@ All functions live in the `phel.pdo` namespace.
 | `exec` | `(exec conn sql)` | Execute SQL, return number of affected rows. |
 | `query` | `(query conn sql & [fetch-mode])` | Run SQL without placeholders, return a statement. |
 | `prepare` | `(prepare conn sql & [options])` | Prepare a statement for later `execute`. |
-| `insert` | `(insert conn table row)` | Insert `row` into `table` via a prepared statement and return the new `last-insert-id`. Identifiers must match `[A-Za-z_][A-Za-z0-9_]*`. |
+| `insert` | `(insert conn table row)` | Insert a non-empty `row` map into `table` via a prepared statement and return the new `last-insert-id` (string). Identifiers must match `[A-Za-z_][A-Za-z0-9_]*`. |
 | `quote` | `(quote conn string & [type])` | Quote a string for safe embedding in SQL. |
-| `last-insert-id` | `(last-insert-id conn)` | ID of the last inserted row. |
+| `last-insert-id` | `(last-insert-id conn)` | ID of the last inserted row, as a string (as PDO reports it). |
 | `begin` / `commit` / `rollback` | `(begin conn)` … | Transaction control. |
 | `in-transaction` | `(in-transaction conn)` | `true` if a transaction is active. |
 | `with-transaction` | `(with-transaction conn & body)` | Run `body` in a transaction: commit + return last value, or rollback + re-throw. Runs inline if already in a transaction. |
 | `get-attribute` / `set-attribute` | `(get-attribute handle attr)` / `(set-attribute handle attr value)` | PDO attribute access; `handle` is a connection or a statement. |
-| `get-available-drivers` | `(get-available-drivers conn)` | Vector of installed PDO drivers. |
-| `error-code` | `(error-code handle)` | SQLSTATE of the last operation; `handle` is a connection or a statement. |
+| `get-available-drivers` | `(get-available-drivers)` | Vector of installed PDO drivers (static; no connection needed). |
+| `error-code` | `(error-code handle)` | SQLSTATE string of the last operation; `handle` is a connection or a statement. |
 | `error-info` | `(error-info handle)` | `[sqlstate driver-code driver-message]`; `handle` is a connection or a statement. |
 
 ### Statement

docs/recipes.md

@@ -115,10 +115,12 @@ Check state with `(pdo/in-transaction conn)`.
 ```clojure
 (pdo/exec conn "insert into t1 (name) values ('phel')")
 (pdo/last-insert-id conn)
-;; => 1
+;; => "1"
 ```
 
-Returns an `int`. On PostgreSQL pass the sequence name to raw PDO (not yet wrapped - drop into `(php/-> (conn :pdo) (lastInsertId "seq"))` for now).
+Returns a `string` (as PDO reports it) - lossless for big integers and named
+sequences; coerce with `php/intval` when you need a number. On PostgreSQL pass the
+sequence name to raw PDO (not yet wrapped - drop into `(php/-> (conn :pdo) (lastInsertId "seq"))` for now).
 
 ## Quoting
 
@@ -140,11 +142,11 @@ Pass a type as the third arg if you need something other than `PARAM_STR`.
   (pdo/exec conn "insert into t1 (id, name) values (1, 'dup')")
   (catch \PDOException _e nil))
 
-(pdo/error-code conn)   ; => 23000
-(pdo/error-info conn)   ; => [23000 19 "UNIQUE constraint failed: t1.id"]
+(pdo/error-code conn)   ; => "23000"
+(pdo/error-info conn)   ; => ["23000" 19 "UNIQUE constraint failed: t1.id"]
 ```
 
-`error-info` returns `[sqlstate driver-code driver-message]` with the first two coerced to int.
+`error-code` returns the SQLSTATE string. `error-info` returns `[sqlstate driver-code driver-message]` - `sqlstate` is the SQLSTATE string, `driver-code` the driver-specific integer.
 
 ## Attributes
 
@@ -160,7 +162,7 @@ Read or change PDO attributes per connection:
 ## Available drivers
 
 ```clojure
-(pdo/get-available-drivers conn)
+(pdo/get-available-drivers)
 ;; => ["mysql" "sqlite" ...]
 ```
 

docs/troubleshooting.md

@@ -83,14 +83,16 @@ phel-pdo sets `ERRMODE_EXCEPTION` in `connect`. If you've overridden it:
 
 …then errors go silent. Either revert it (`\PDO/ERRMODE_EXCEPTION`) or read `pdo/error-code` / `pdo/error-info` after every call. The default is the saner mode for almost every app.
 
-## `last-insert-id` returns 0 on PostgreSQL
+## `last-insert-id` is wrong on PostgreSQL
 
 PostgreSQL needs the sequence name. The wrapped `pdo/last-insert-id` calls `lastInsertId()` with no args. Until a sequence-aware wrapper lands, drop down to raw PDO:
 
 ```clojure
-(php/intval (php/-> (conn :pdo) (lastInsertId "t1_id_seq")))
+(php/-> (conn :pdo) (lastInsertId "t1_id_seq"))   ; => "42" (string)
 ```
 
+`pdo/last-insert-id` returns the value as a string (as PDO does); coerce with `php/intval` only when you actually need a number.
+
 ## Method I want isn't wrapped
 
 Check the "Not implemented yet" block at the bottom of `src/pdo/statement.phel`. The escape hatch is always:

src/pdo.phel

@@ -9,6 +9,13 @@
   [handle]
   (if (statement? handle) (handle :stmt) (handle :pdo)))
 
+(defn- options->php
+  "Builds a PHP array from a Phel options map. Unlike phel->php it preserves
+  integer keys, which PDO attribute maps use (e.g. \\PDO/ATTR_TIMEOUT)."
+  [options]
+  (apply php-associative-array
+         (mapcat (fn [[k v]] [k (phel->php v)]) options)))
+
 (defn ^bool set-attribute
   "Set an attribute on a connection or statement"
   [handle attribute value]
@@ -23,7 +30,7 @@
   "Connect database and return connection object.
   Throws a PDOException if the attempt to connect to the requested database fails."
   [dsn & [username password options]]
-  (let [conn (connection (php/new \PDO dsn username password options))]
+  (let [conn (connection (php/new \PDO dsn username password (options->php (or options {}))))]
     (set-attribute conn \PDO/ATTR_ERRMODE \PDO/ERRMODE_EXCEPTION)
     conn))
 
@@ -33,6 +40,8 @@
   closes a connection it did not open. Pass {:apply-defaults true} to set
   phel-pdo's ERRMODE_EXCEPTION default on the wrapped handle."
   [pdo & [options]]
+  (when-not (php/instanceof pdo \PDO)
+    (throw (php/new \InvalidArgumentException "from-connection expects a \\PDO instance")))
   (let [conn (connection pdo)]
     (when (get options :apply-defaults)
       (set-attribute conn \PDO/ATTR_ERRMODE \PDO/ERRMODE_EXCEPTION))
@@ -50,7 +59,7 @@
   "Prepares a statement for execution and returns a statement object"
   [conn stmt & [options]]
   (statement
-   (php/-> (conn :pdo) (prepare stmt (phel->php (or options {}))))))
+   (php/-> (conn :pdo) (prepare stmt (options->php (or options {}))))))
 
 (defn query
   "Prepares and executes an SQL statement without placeholders"
@@ -78,10 +87,12 @@
   [conn string & [type]]
   (php/-> (conn :pdo) (quote string (or type \PDO/PARAM_STR))))
 
-(defn ^int last-insert-id
-  "Returns the ID of the last inserted row or sequence value"
+(defn ^string last-insert-id
+  "Returns the ID of the last inserted row or sequence value, as a string (as PDO
+  reports it). Returning the raw string is lossless for big integers and named
+  sequences; coerce with php/intval at the call site when you need a number"
   [conn]
-  (php/intval (php/-> (conn :pdo) (lastInsertId))))
+  (php/-> (conn :pdo) (lastInsertId)))
 
 (defn- check-ident [s]
   (if (= 1 (php/preg_match "/^[A-Za-z_][A-Za-z0-9_]*$/" s))
@@ -91,10 +102,13 @@
 (defn- join-csv [xs]
   (php/implode ", " (phel->php (into [] xs))))
 
-(defn ^int insert
-  "Inserts a row built from a map into table and returns the new last-insert-id.
-  Table and column names must be plain identifiers; values are bound as parameters."
+(defn ^string insert
+  "Inserts a row built from a map into table and returns the new last-insert-id
+  (a string, as PDO reports it). Table and column names must be plain identifiers;
+  values are bound as parameters."
   [conn table row]
+  (when (empty? row)
+    (throw (php/new \InvalidArgumentException "insert row map must not be empty")))
   (let [tbl  (check-ident (name table))
         cols (into [] (for [k :in (keys row)] (check-ident (name k))))
         sql  (str "INSERT INTO " tbl
@@ -103,17 +117,20 @@
     (-> (prepare conn sql) (execute row))
     (last-insert-id conn)))
 
-(defn ^int error-code
-  "Fetch the SQLSTATE associated with the last operation on a connection or statement"
+(defn ^string error-code
+  "Fetch the SQLSTATE associated with the last operation on a connection or statement.
+  SQLSTATE is a 5-character code (e.g. \"23000\", \"HY000\"), returned as a string"
   [handle]
-  (php/intval (php/-> (handle-target handle) (errorCode))))
+  (php/-> (handle-target handle) (errorCode)))
 
 (defn error-info
-  "Fetch extended error information associated with the last operation on a connection or statement"
+  "Fetch extended error information associated with the last operation on a connection
+  or statement, as [sqlstate driver-code driver-message]. sqlstate is the SQLSTATE
+  string; driver-code is the driver-specific integer"
   [handle]
   (let [[sqlstate driver-code driver-message]
         (values (php->phel (php/-> (handle-target handle) (errorInfo))))]
-    [(php/intval sqlstate) (php/intval driver-code) driver-message]))
+    [sqlstate (php/intval driver-code) driver-message]))
 
 (defn ^bool in-transaction
   "Checks if inside a transaction"
@@ -139,8 +156,9 @@
              (throw e#)))))))
 
 (defn get-available-drivers
-  "Return an array of available PDO drivers"
-  [conn]
-  (values (php->phel (php/-> (conn :pdo) (getAvailableDrivers)))))
+  "Return a vector of available PDO driver names. PDO::getAvailableDrivers is
+  static, so no connection is needed"
+  []
+  (values (php->phel (php/:: \PDO (getAvailableDrivers)))))
 
 (load "pdo/statement")

src/pdo/statement.phel

@@ -12,17 +12,20 @@
     (row->map row)))
 
 (defn fetch-column
-  "Returns a single column from the next row of a result set"
+  "Returns a single column from the next row of a result set. Returns false when
+  the cursor is exhausted (PDO's sentinel); a nil means the column was SQL NULL"
   [stmt & [column]]
   (php/-> (stmt :stmt) (fetchColumn (or column 0))))
 
 (defn fetch-all
-  "Fetches the remaining rows from a result set"
+  "Fetches the remaining rows from a result set as a seq of maps"
   [stmt]
-  (map row->map (php/-> (stmt :stmt) (fetchAll \PDO/FETCH_ASSOC))))
+  (map row->map (or (php/-> (stmt :stmt) (fetchAll \PDO/FETCH_ASSOC)) [])))
 
 (defn statement-seq
-  "Returns a lazy seq of the statement's remaining rows as maps, fetched one at a time"
+  "Returns a lazy seq of the statement's remaining rows as maps, fetched one at a
+  time. The cursor stays open until the seq is fully consumed; on drivers with
+  cursor limits, realise it fully or call close-cursor when done early"
   [stmt]
   (lazy-seq
    (when-let [row (fetch stmt)]
@@ -51,19 +54,25 @@
   stmt)
 
 (defn ^bool next-rowset
-  "Advances to the next rowset of a multi-rowset statement; false when none remain"
+  "Advances to the next rowset of a multi-rowset statement; true when another
+  rowset is available, false when none remain. Throws PDOException on drivers
+  that don't support multiple rowsets (e.g. SQLite)"
   [stmt]
   (php/-> (stmt :stmt) (nextRowset)))
 
 (defn set-fetch-mode
   "Sets the default fetch mode for the statement. Extra args match the mode
-  (e.g. column index for FETCH_COLUMN, class name + ctor args for FETCH_CLASS)"
+  (e.g. column index for FETCH_COLUMN, class name + ctor args for FETCH_CLASS).
+  Note: fetch / fetch-all always request FETCH_ASSOC, so this default applies to
+  the statement's native iteration, not those wrappers"
   [stmt mode & args]
   (let [s (stmt :stmt)]
     (case (count args)
       0 (php/-> s (setFetchMode mode))
       1 (php/-> s (setFetchMode mode (phel->php (first args))))
-      2 (php/-> s (setFetchMode mode (phel->php (first args)) (phel->php (second args))))))
+      2 (php/-> s (setFetchMode mode (phel->php (first args)) (phel->php (second args))))
+      (throw (php/new \InvalidArgumentException
+                      (str "set-fetch-mode accepts at most 2 extra args, got " (count args))))))
   stmt)
 
 (defn ^int column-count
@@ -82,18 +91,26 @@
   (when-let [meta (php/-> (stmt :stmt) (getColumnMeta column))]
     (row->map meta)))
 
+(defn- param-key
+  "PDO parameter identifier: an integer is a 1-based positional index (kept as-is);
+  anything else is a named placeholder coerced to a string."
+  [column]
+  (if (int? column) column (str column)))
+
 (defn bind-value
-  "Binds a value to a parameter"
+  "Binds a value to a parameter (named placeholder or 1-based positional index)"
   [stmt column value & [type]]
   (php/-> (stmt :stmt)
-          (bindValue (str column) value (or type \PDO/PARAM_STR)))
+          (bindValue (param-key column) value (or type \PDO/PARAM_STR)))
   stmt)
 
 (defn bind-param
-  "Binds a parameter to a value, applied at execution time"
+  "Binds a value to a parameter. The value is captured at call time: Phel values
+  are immutable, so unlike PHP's by-reference bindParam there is nothing to mutate
+  before execute - for fresh values per run, re-bind or pass a params map to execute"
   [stmt column value & [type]]
   (php/-> (stmt :stmt)
-          (bindParam (str column) value (or type \PDO/PARAM_STR)))
+          (bindParam (param-key column) value (or type \PDO/PARAM_STR)))
   stmt)
 
 (defn ^string debug-dump-params