ahyatt / ekg

The emacs knowledge graph, app for notes and structured data.
GNU General Public License v3.0
230 stars 19 forks source link

ekg-note-delete-by-id: remove title as well #112

Closed qingshuizheng closed 1 year ago

qingshuizheng commented 1 year ago

Hello @ahyatt , https://github.com/ahyatt/ekg/blob/d2f0e540b5e32553fec2352152513bb0fc82333a/ekg.el#L439

While inspecting db, I found some TITLE-related records remained in db after note deletion:

-- Loading resources from /Users/z/.sqliterc
SQLite version 3.42.0 2023-05-16 12:36:15
Enter ".help" for usage hints.
sqlite> select * from triples where subject is 33359085393;
subject      predicate     object      properties
-----------  ------------  ----------  ----------
33359085393  base/type     titled      ()        
33359085393  titled/title  "westlife"  (:index 0)
sqlite> 

I think 'titled types should be taken care as well here:

(defun ekg-note-delete-by-id (id)
  "Delete all note data associated with ID."
  (ekg-connect)
  (run-hook-with-args 'ekg-note-pre-delete-hook id)
  (triples-with-transaction
    ekg-db
    ;; (cl-loop for type in '(tagged text time-tracked) do
    (cl-loop for type in '(tagged text time-tracked titled) do     ; <- TITLED should be included
             (triples-remove-type ekg-db id type))
    (cl-loop for inline-id in (triples-subjects-with-predicate-object
                               ekg-db 'inline/for-text id)
             do (triples-remove-type ekg-db inline-id 'inline))
    (run-hook-with-args 'ekg-note-delete-hook id)))

With this↑ change, the titled-related records will also be deleted.

Note: The remaining titled-related items of all previous note deletions need to be cleaned up. (I found 74 remaining records in my db.)

Zheng

ahyatt commented 1 year ago

Thanks for reporting this. I've checked in a change (and a way to do the cleanup), but it isn't quite right in theory. What should happen is that we should encode dependency information into types in the triples package, and then have it just delete orphan types. That's something I'll look to add in a subsequent change in that package.

qingshuizheng commented 1 year ago

Thanks for the fix! 😃