Rough sketch of extracting detailed exception info from a PSQLException

src/metabase/driver/postgres.clj

@@ -51,7 +51,9 @@
     Types)
    (java.time LocalDateTime OffsetDateTime OffsetTime)
    (org.postgresql.copy CopyManager)
-   (org.postgresql.jdbc PgConnection)))
+   (org.postgresql.jdbc PgConnection)
+   (org.postgresql.util PSQLException)
+   (org.postgresql.util ServerErrorMessage)))
 
 (set! *warn-on-reflection* true)
 
@@ -1116,3 +1118,63 @@
 (defmethod driver.sql/default-database-role :postgres
   [_ _]
   "NONE")
+
+;; TODO: dispatch extra-ex-info on a [driver (class ex)] tuple rather than just the driver and remove this method.
+(defmulti extra-ex-info-from class)
+
+(defmethod extra-ex-info-from :default
+  [_e]
+  nil)
+
+(defmethod extra-ex-info-from PSQLException
+  [^PSQLException e]
+  ;; This looks like a bunch of info, but apparently it depends on the Postgres server's configured log level what you
+  ;; get.
+  ;;
+  ;; https://jdbc.postgresql.org/documentation/publicapi/org/postgresql/util/ServerErrorMessage.html
+  ;; https://github.com/pgjdbc/pgjdbc/blob/2989c62c1986d0c94e3712f436ea307485c4162d/pgjdbc/src/main/java/org/postgresql/util/ServerErrorMessage.java#L174C5-L177C20
+  ;;
+  ;; As far as I can tell testing out different bad queries with the default Postgres docker image, only :position is
+  ;; consistently present and useful. :hint is sometimes useful (and of course :message), but it's not clear they are
+  ;; useful for anything other than displaying the text to the user, which we already do. :hint sometimes contains,
+  ;; e.g. the corrected table/column name if you type in the wrong name, but it's still embedded in a (presumably
+  ;; localized) string like: "Perhaps you meant to reference the column \"people.birth_date\"."
+  ;;
+  ;; TODO: Sometimes :position is 0, like when you pass a bad first arg to the `extract` function, :position will be
+  ;; zero. Is :position relative to the start of function's arg list, or is it simply unreliable in this case?
+  (when-let [^ServerErrorMessage sem (.getServerErrorMessage e)]
+    {:column            (.getColumn sem)
+     :constraint        (.getConstraint sem)
+     :data-type         (.getDatatype sem)
+     :detail            (.getDetail sem)
+     :file              (.getFile sem)
+     :hint              (.getHint sem)
+     :internal-position (.getInternalPosition sem)
+     :internal-query    (.getInternalQuery sem)
+     :line              (.getLine sem)
+     :message           (.getMessage sem)
+     :position          (.getPosition sem)
+     :routine           (.getRoutine sem)
+     :schema            (.getSchema sem)
+     :severity          (.getSeverity sem)
+     :sql-state         (.getSQLState sem)
+     :table             (.getTable sem)
+     :where             (.getWhere sem)}))
+
+(defn- adjust-error-position [pos sql _params]
+  ;; Subtract the length of the first line, which is the remark added by the default implementation
+  ;; of [[sql-jdbc.execute/inject-remark]]. Ideally, this would also handle adjustments for expanded params, snippets,
+  ;; card references etc. The params passed in here are just the values though, and the sql has already been expanded
+  ;; by this point, so there's not currently enough info to fully adjust the position.
+  (assert (and (str/starts-with? sql "--")
+               (pos-int? (str/index-of sql "\n"))))
+  ;; TODO: pos is sometimes zero... why?
+  (if (pos-int? pos)
+    (- pos (inc (str/index-of sql "\n")))
+    pos))
+
+(defmethod sql-jdbc.execute/extra-ex-info :postgres
+  [_driver sql params e]
+  ;; Return the full map for demo purposes. Possibly we would want to pair this down to just the useful info.
+  (when-let [extra-info (extra-ex-info-from e)]
+    (update extra-info :position adjust-error-position sql params)))

src/metabase/driver/sql_jdbc/execute.clj

@@ -715,6 +715,17 @@
   [_ sql remark]
   (str "-- " remark "\n" sql))
 
+;; TODO: move into [[metabase.driver]]?
+;; TODO: dispatch on [driver (class e)] instead?
+(defmulti extra-ex-info
+  "Get extra driver-specific exception info."
+  {:added "0.53.0", :arglists '([driver sql params e])}
+  driver/dispatch-on-initialized-driver
+  :hierarchy #'driver/hierarchy)
+
+(defmethod extra-ex-info :default [_driver _sql _params _e]
+  {})
+
 (defn execute-reducible-query
   "Default impl of [[metabase.driver/execute-reducible-query]] for sql-jdbc drivers."
   {:added "0.35.0", :arglists '([driver query context respond] [driver sql params max-rows context respond])}
@@ -742,7 +753,8 @@
                                                     {:driver driver
                                                      :sql    (str/split-lines (driver/prettify-native-form driver sql))
                                                      :params params
-                                                     :type   qp.error-type/invalid-query}
+                                                     :type   qp.error-type/invalid-query
+                                                     :extra-ex-info (extra-ex-info driver sql params e)}
                                                     e))))]
         (let [rsmeta           (.getMetaData rs)
               results-metadata {:cols (column-metadata driver rsmeta)}]

src/metabase/query_processor/middleware/catch_exceptions.clj

@@ -36,11 +36,13 @@
   [e]
   ;; `:is-curated` is a flag that signals whether the error message in `e` was approved by product
   ;; to be shown to the user. It is used by FE.
-  (let [{error-type :type, is-curated :is-curated, :as data} (ex-data e)]
+  (let [{error-type :type, is-curated :is-curated, {error-position :position} :extra-ex-info, :as data} (ex-data e)]
     (merge
      ((get-method format-exception Throwable) e)
      (when (qp.error-type/known-error-type? error-type)
        {:error_type error-type})
+     (when error-position
+       {:error_position error-position})
      (when is-curated
        {:error_is_curated is-curated})
      ;; TODO - we should probably change this key to `:data` so we're not mixing lisp-case and snake_case keys
@@ -51,6 +53,17 @@
   (assoc ((get-method format-exception Throwable) e)
          :state (.getSQLState e)))
 
+;; Rather than add new methods to the driver interface, we could do something like this instead, but it feels wrong to
+;; have driver-specific classes here. It also becomes gross/impossible if you want to extend support to other drivers,
+;; esp 3rd party. You also still have the problem of needing to adjust the error position to account for the possible
+;; snippets, params, "remarks", etc, and [[sql-jdbc.execute/inject-remark]] is driver-specific.
+#_(defmethod format-exception PSQLException
+    [^PSQLException e]
+    (merge ((get-method format-exception SQLException) e)
+           (when-let [^ServerErrorMessage sem (.getServerErrorMessage e)]
+             {:error_position (.getPosition sem)})))
+
+
 ;; TODO -- some of this logic duplicates the functionality of `clojure.core/Throwable->map`, we should consider
 ;; whether we can use that more extensively and remove some of this logic
 (defn- exception-chain
@@ -75,9 +88,11 @@
     (merge
      m
      (best-top-level-error maps)
-     ;; merge in the first error_type we see
+     ;; merge in the first error_type and/or error_position we see
      (when-let [error-type (some :error_type maps)]
        {:error_type error-type})
+     (when-let [error-position (some :error_position maps)]
+       {:error_position error-position})
      (when (seq more)
        {:via (vec more)}))))