Repository navigation
Replies: 8 comments 5 replies
|
I will give it more thought, but my initial comments are:
|
|
Hi @GeePa67 Thanks a lot for the valuable feedback.
Commit early opensak.transaction(function()
opensak.update("GC1", { user_flag = true })
if done then return end -- commits what was done so far
opensak.update("GC2", { user_flag = true })
end)Cancel (roll back) without ending the macro. Wrap the call in local ok, err = pcall(opensak.transaction, function()
opensak.update("GC1", { user_flag = true })
if not opensak.confirm("Keep the changes?") then error("cancelled", 0) end
end)
-- ok == false, err == "cancelled", GC1 unchangedWithout Undo only part. A nested
A real new table within the current OSAK DB is not allowed
What works instead
|
|
@nagisml Thanks for your well described outline. As I said elsewhere, I am currently travelling for several weeks and don't really have time and access to go through my macros systematically, so all feedback is out of my head.
As to your questions:
I hope this feedback is helpful. |
|
You might want to get feedback from Mike Wood (lignumaqua) as well on this. He has authored some of the more sophisticated GSAK macros. His findstatgen macro creates a FoundStatsSQLLite.db3, for example. There is also a number of ATTACH statements in the code. I don't know all of how it is used and what it contains and maybe there is another way to do what is being done in Lua, but Mike would be the best to give a view on it. |
|
Hi @GeePa67 @urs-beeli @AgreeDK Thanks again for the feedback. The branch has been updated based on your feedback Lua macros: new SQL and write features (
|
| Read | Add | Change |
|---|---|---|
opensak.logs(code) |
opensak.insert_log(code, fields) |
opensak.update_log(id, fields) |
opensak.waypoints(code) |
opensak.insert_waypoint(code, fields) |
opensak.update_waypoint(id, fields) |
opensak.attributes(code) |
opensak.insert_attribute(code, fields) |
opensak.update_attribute(id, fields) |
opensak.trackables(code) |
opensak.insert_trackable(code, fields) |
opensak.update_trackable(id, fields) |
-- add a note log and a final waypoint
local id = opensak.insert_log("GC12345", { type = "Write note", date = "2026-10-11", text = "Final solved" })
opensak.insert_waypoint("GC12345", { prefix = "FN", type = "Final Location", lat = 47.36872, lon = 8.54093 })
-- change rows found by reading them first (each row has an id)
for _, w in ipairs(opensak.waypoints("GC12345")) do
if w.prefix == "PK" then opensak.update_waypoint(w.id, { comment = "free parking" }) end
endfalseclears a field, as for caches.- OpenSAK updates the cache's counts and log dates itself.
- The usual write permission applies.
- The read functions also take the
databaseoption.
3. A database per macro: opensak.macro_db()
opensak.macro_db() creates <macro name>.db3 next to the macro file (if needed) and attaches it as schema macro. In it the macro has full access: CREATE/DROP/ALTER tables, indexes, views and triggers, INSERT/UPDATE/DELETE, and PRAGMA macro.user_version. The data stays between runs. One statement can combine the active database and the macro database.
opensak.macro_db()
opensak.sql("CREATE TABLE IF NOT EXISTS macro.seen (code TEXT PRIMARY KEY, first_seen TEXT)")
opensak.sql("INSERT OR IGNORE INTO macro.seen SELECT gc_code, date('now') FROM caches")
-- caches that are new since the macro first ran
for _, r in ipairs(opensak.sql(
"SELECT c.gc_code, c.name FROM caches c JOIN macro.seen s ON s.code = c.gc_code "
.. "WHERE s.first_seen = date('now')")) do
print(r.gc_code, r.name)
end- Create objects with the
macro.prefix, since unqualified names refer to the active database first. - Needs no permission. The macro must be saved, because the file is named after it.
- Changes inside
opensak.transaction()are kept or undone together with those of the active database. - A plain SQL
ATTACHstays refused, so a macro can't open any other file on disk.
4. Faster transactions and opensak.resync()
Inside opensak.transaction(), the columns OpenSAK maintains (distance, bearing, log/waypoint/trackable counts, log dates) are now recalculated once, just before the commit, instead of after every write. This is a big speed-up for loops with many writes, e.g. 2000 insert_log calls on one cache: 4.5 s → 1.8 s.
Until the commit, reads inside the transaction still see the old values. Call opensak.resync() when a later read depends on them:
opensak.transaction(function()
for i = 1, 3 do opensak.insert_log("GC12345", { type = "Write note" }) end
opensak.resync() -- recalculate now
print(opensak.cache("GC12345").log_count) -- already includes the 3 new logs
end)Outside a transaction nothing changes: each write is recalculated before its own commit.
Bulk updates with sql_write()
Not new, but a common question: a bulk change doesn't need a Lua loop. One statement is one transaction and is recalculated in one go:
opensak.sql_write("UPDATE caches SET user_flag = 1 WHERE found = 1")
local codes = opensak.codes() -- caches of the current filter
opensak.sql_write("UPDATE caches SET user_flag = 1 WHERE gc_code IN ("
.. string.rep("?", #codes, ", ") .. ")", codes)|
Thanks @nagisml for opening this as a discussion before the PR, and for turning the feedback around so quickly. Thanks also to @GeePa67 and @urs-beeli (especially while travelling!) for the input. This is exactly the kind of review I hoped we'd get from people who actually write GSAK macros. Overall I'm very positive. The direction is right, and the safeguards (SQLite authorizer instead of text inspection, rollback on error/cancel, protected derived fields, no 1. 2. Logs, waypoints, attributes, trackables and 3. Approval model: this is where I'd like to tighten things a bit
The first option is my preference. It costs the user one extra click per new macro, which I think is acceptable for write access. Happy to hear arguments either way. 4.
5. Delete 6. ATTACH / complex GSAK macros From my side you're welcome to open the PR when you're ready. The approval-model question doesn't need to block that, but I'd like it settled before we merge. |
|
I reached out to Mike Wood (@lignumaqua) as @GeePa67 suggested. Mike has written some of the most advanced GSAK macros (FindStatGen and others), and he kindly gave detailed answers. Here's a summary, because it adds a dimension we haven't fully covered yet. 1. Mike suggests a companion database per cache database
His reasons:
2. FindStatGen is the other pattern, which The gap: FSG temporarily attaches several standard cache databases while it runs. Found and placed caches are often kept in separate databases. In the current proposal, (Porting FSG itself isn't realistic. It's 20,000+ lines and depends on a server back end on gsak.net for maps and charts. But it's a good test of the pattern.) 3. Whatever we choose must be included in backups (Mike's point, and relevant to the backup epic #942). Questions for discussion, mainly @nagisml:
Mike also sent a sample FSG database so we can see a real-world schema. It may contain personal log data, so I'll share it privately rather than here. @nagisml, I'll send it to you directly if it's useful. |
|
Thanks @nagisml, the On the approval model (3), your GSAK comparison helped me settle it. In GSAK, installing a macro was the trust decision. OpenSAK has no install step, since you just drop a So my proposal is to make the approval per macro instead of per database:
This gives fewer prompts than today for people with many databases, matches @urs-beeli's view that trust is decided once per macro, and closes the gap where any later macro could write to a database that was approved earlier. The one open detail is whether an "Always" approval should reset if the macro file is replaced or moved. Keying it on the file path seems fine to me for now. I'd rather not re-prompt every time someone edits their own macro. @GeePa67 @urs-beeli, any objections? |
Uh oh!
There was an error while loading. Please reload this page.
Hi @AgreeDK @urs-beeli @GeePa67 and everybody else
I'd like an early review and early feedback of a new set of macro functions before I open a PR. It's not final; I'd rather change direction now than after a PR review. The branch is
nagisml/OpenSAK→feature/lua-cache-write(based onbeta), and everything is documented in the generated API reference:📄 API documentation, version 3. Start with Changing caches.
Until now, macros could mostly read (only corrected coordinates and moving/copying caches changed anything). With this branch, macros can change most cache data. That's a big step, so I'm mainly asking for a judgement on two questions:
What's new in API version 3
opensak.update(code, fields)opensak.insert{...}opensak.sql_write(query, params)INSERTorUPDATEon the cache tables see #1033opensak.transaction(fn)fntogether, or undo all of them on an error (like GSAK'sTransactioncommand)opensak.in_transaction()InTransaction())Existing write functions, for context:
set_corrected,clear_corrected,create_database,switch_database,move_caches,copy_caches.Example:
Safeguards built in
set_corrected/clear_corrected.sql_writerefusesDELETE, and a trigger catches the rowsINSERT OR REPLACEwould delete.sql_writelimits: onlyINSERT/UPDATEoncaches,user_notes,waypoints,logs,attributes,trackables. No schema changes, noATTACH, no writingPRAGMAs, noBEGIN/COMMIT(useopensak.transaction()), 60 s timeout per statement. This is enforced by SQLite's authorizer, not by inspecting the SQL text.Where I'm unsure — opinions welcome
sql_write: keep, restrict or remove? It's the most powerful function. It can change many caches at once, includinglogsandwaypoints. Should it be limited tocaches/user_notes, or removed in favour ofupdate/insertonly?logs,waypoints,attributes,trackablesvia SQL. Useful (e.g. importing own logs or waypoints from a CSV), but these tables are normally filled only by imports. Too risky?insert{}for new caches. Fine, or should macros only change existing caches?Related
INSERT INTO cachesonly needs 5 instead of 21 columns (sql_writeworks without it, butopensak.insert{}is the easier way to add a cache for now)There's no PR yet. Comments here or on the branch are both fine. Thanks for taking a look!
All reactions