anse1 / emacs-libpq

An Emacs 25 module for accessing postgres via libpq.
GNU General Public License v3.0
22 stars 4 forks source link

Do you recommend using handle in each function using database? #15

Open gnusupport opened 3 years ago

gnusupport commented 3 years ago

At first I have made libraries that connect and fetch and handle was &optional. If there was no handle standard *db* handle was used. Programs could access one database.

Then I have developed separate Emacs package that uses the same database but due to its nature could also use separate database and other databases outside of my local computer.

As then I had 2 programs connecting to local database and complexities arrive in future when using multiple databases in the same time, I am of opinion that each call to function that uses database should also transfer the handle, so that handle is not just global variable.

Do you agree on this?

Handle was here optional and it worked well until I started making two connections to database:

(defun rcd-sql (sql &optional handle)
  "Sends SQL queries to PostgreSQL database and returns results"
  (let ((handle (if handle handle *cf*)))
    (condition-case err
    (pq:query handle sql)
      (error
       (if (string-match "^ERROR:  syntax error" (cdr err))
       (progn
         (if (fboundp 'speak) (speak (cdr err)))
         (message (cdr err)))
     ;; re-throw
     (signal (car err) (cdr err)))))))

(defun rcd-sql-list (sql &optional handle)
  "Returns list of lists instead of vectors"
  (let ((list '()))
    (dolist (i (apply 'rcd-sql (list sql handle)) (reverse list))
      (cond ((eq (type-of i) 'vector) (push (append i '()) list))
        (t (push i list))))))

(defun rcd-sql-first (sql &optional handle)
  "Returns first entry from SQL query"
  (car (apply 'rcd-sql (list sql handle))))

My opinion is that I should not make it optional but require handle each time. Is that what you also recommend?

anse1 commented 3 years ago

GNU Support writes:

My opinion is that I should not make it optional but require handle each time. Is that what you also recommend?

For code as generic as the one you listed I think non-optional is the better implementation.

However, when your package implements a mode where database connections are associated with buffers in that mode, I think the most elegant solution is to use buffer-local variables (see make-local-variable, make-variable-buffer-local).

You may still have to pass the context eventually when your functions could run in a context such as timers, hooks, etc but you could then pass the buffer handle around instead of the database handle.

gnusupport commented 3 years ago

That is something new to me to think about. I did not think that database connection would be associated to buffer. I do not even know how to do that, do you have practical example?

Now I have made it work and I am using (defvar cf "Handle") where this cf is used only by specific file or files, but other hs handle is used by other file. That is good compromise as that way I do not need to inject cf as database handle to each function.

Functions in file using cf as handle can then use it as global variables, calling those generic functions and I do not need to call `(do-something cf)

(defun do-something () (rcd-sql sql cf))

That way multiple programs will not have problems with each other, and I can make packages.

You may still have to pass the context eventually when your functions could run in a context such as timers, hooks, etc but you could then pass the buffer handle around instead of the database handle.

I wish to see your use case practically if you have anything.

gnusupport commented 3 years ago

I use handles as global variables, per package, that works well. I have only the problem when there is server interruption, for example server stopped and started. So I have to improve generic function to recognize that and to re-connect again. That may need passing a symbol to handle so that generic function recognizes it.

My generic functions are here:

RCD Database Basics https://hyperscope.link/3/7/4/9/3/RCD-Database-Basics-37493.html

anse1 commented 3 years ago

GNU Support writes:

I use handles as global variables, per package, that works well. I have only the problem when there is server interruption, for example server stopped and started. So I have to improve generic function to recognize that

hmyes. Ideally you'd check whether the SQLSTATE starts with "08" - these are all kinds of connection failures. You can't check that though since my code does throw an error instead of a pq:error in thie case and the former lacks the SQLSTATE error data :-/ I shall fix that.

and to re-connect again. That may need passing a symbol to handle so that generic function recognizes it.

There's no need for extra parameters, you can invoke pq:reset on a failed connection and libpq will attempt to re-connect. You need to be careful though that your session state is restored. For example your client code might have had a transaction in progress and if you catch the connection failure and reconnect without telling anyone you are outside of a transaction and all statements are auto-committed, messing with your client code.

Similarly, there may be session_variables that need restoring, e.g. when a custom statement_timeout is set as mentioned in an earlier issue, this needs to be re-set after calling pq:reset on a connection.

regards, Andreas

gnusupport commented 3 years ago

I have tried that, I think it worked.

For example your client code might have had a transaction in progress and if you catch the connection failure and reconnect without telling anyone you are outside of a transaction and all statements are auto-committed, messing with your client code.

That is understandable, I can make sure of that. My inserts or updates are usually of shortest nature, and those which take long time usually update only temporary information. But what would be good practice is to use BEGIN and COMMIT to prevent such problems. Good reminder!

Similarly, there may be session_variables that need restoring, e.g. when a custom statement_timeout is set as mentioned in an earlier issue, this needs to be re-set after calling pq:reset on a connection.

Aha, those session variables, yes, I get it. Also good to think of it.

Thanks.