Implement generic Table for basic CRUD operations - Save multiple#33
Merged
Conversation
VojtechVitek
left a comment
Member
There was a problem hiding this comment.
Minor feedback / questions. Thank you. I think this is very decent already. 👍
| end := start + chunkSize | ||
| if end > len(insertRecords) { | ||
| end = len(insertRecords) | ||
| } |
Member
There was a problem hiding this comment.
What if you chose only one of the two insertRecords vs. insertIndices. Why do we need both?
Member
Author
There was a problem hiding this comment.
I need to update records in passed slice, and since I am chunking the requests, I am iterating over different slice.
Comment on lines
+132
to
+135
| // update original slice | ||
| for i, rr := range chunk { | ||
| records[insertIndices[start+i]] = rr | ||
| } |
Member
There was a problem hiding this comment.
I'm not following 100% why this is needed.
klaidliadon
pushed a commit
that referenced
this pull request
Mar 5, 2026
* save multiple * pr comments * return error instead of nil on nil record
VojtechVitek
added a commit
that referenced
this pull request
Apr 3, 2026
…equeuing (#30) * Implement generic Table for basic CRUD operations Co-authored-by: David Sedlacek <david.sedlacek@golang.cz> * Remove automatic 'deleted_at NULL' condition * Implement .LockForUpdate() using PostgreSQL's FOR UPDATE SKIP LOCKED pattern * Fix generic ID * Fix naming of generic types * Add tests for simple CRUD, complex transactions and LockForUpdate() * Fix TestRecordsWithJSONStruct test, since schema changed * Fix LockForUpdate test * Don't rely on wg.Go(), a feature from Go 1.25 * LockForUpdate that can pass transaction to update fn * Refactor LockForUpdate() to reuse tx if possible * Simplify data models further .Save() - variadic arg instead of .Save() and .SaveAll() .List() - renamed from .GetAll() .Get() - renamed from .GetOne() .LockForUpdates() - renamed from .LockForUpdate() .LockForUpdate() - renamed from .LockOneForUpdate() * Improve tests for async processing * Tests: Implement in-memory worker pattern via simple WaitGroup * A better "dequeue" abstraction defined on reviews table * Rename GetByIDs() to ListByIDs() * Fix updated_at field, thanks @shunkakinoki * PR feedback: Improve error annotations * Fix tests * Save multiple (#33) * save multiple * pr comments * return error instead of nil on nil record * Add iterator method for accounts and update tests * Rename types for clarity * add back truncateAllTables * Add IDColumn to table context and enhance WithTx tests * Add pagination support to Table with ListPaged and WithPaginator methods * feat: add Insert and Update methods in Table * feat: add RestoreByID and export timestamp interfaces - Add RestoreByID method that clears DeletedAt by passing zero time - Export HasSetCreatedAt, HasSetUpdatedAt, HasSetDeletedAt interfaces with godoc explaining the contract for each lifecycle hook - Update SetDeletedAt implementations to treat zero time as restore (nil) * fix: LockForUpdates missing return, saveAll hardcoded ID column, Iter missing rows.Err, saveOne missing SetCreatedAt - LockForUpdates: add missing return after existing-tx branch to prevent double execution of updateFn via a second transaction - saveAll: use t.IDColumn instead of hardcoded "id" for update WHERE clause - Iter: check rows.Err() after iteration loop to surface driver errors - saveOne: call SetCreatedAt on insert path (consistent with saveAll) - Use min() builtin for chunk bounds * fix(tests): race conditions in TestLockForUpdates and wrong ListByIDs slice - Move wg.Add(1) before goroutine dispatch in ProcessReview to prevent worker.Wait() returning early before all goroutines register - Replace shared ids[][]uint64 slice (data race across goroutines) with per-worker local slices merged under mutex - Fix articleIDs slice: make([]uint64, len) + append produced leading zeros, changed to make([]uint64, 0, len) * fix: ListPaged nil page panic Normalize nil page before passing to PrepareQuery/PrepareResult. PrepareQuery creates a local &Page{} but the caller's pointer stays nil, causing PrepareResult to panic on page.More assignment. * fix: LockForUpdates validate/timestamp, zero-value Paginator, ListPaged ordering - lockForUpdatesWithTx: call Validate() and SetUpdatedAt() on records after updateFn, matching Insert/Update/Save behavior - Page.SetDefaults: fall back to DefaultPageSize/MaxPageSize when PaginatorSettings has zero values (fixes zero-value Paginator capping page size to 0) - ListPaged: add IDColumn fallback ordering when no sort is configured, ensuring deterministic pagination - TestLockForUpdates: use assert instead of require in goroutine (require.FailNow is unsafe off the test goroutine) * fix: PrepareRaw empty ORDER BY and unbound limit/offset params - Skip ORDER BY clause when no sort columns are configured - Always inject pgx.NamedArgs for limit/offset even when args is empty * feat: Update, DeleteByID, HardDeleteByID return (bool, error) Return true when at least one row was affected, false when zero rows matched. Callers can now distinguish "success" from "not found" without reimplementing methods with RowsAffected checks. Breaking change: signatures go from error to (bool, error). * NOTICE: Experimental. Table and its methods are subject to change. --------- Co-authored-by: David Sedlacek <david.sedlacek@golang.cz> Co-authored-by: David Sedláček <david.littlefarmer@gmail.com> Co-authored-by: Alex Guerrieri <ag@sequence.xyz> Co-authored-by: Alex Guerrieri <klaidliadon@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Save multiple implementation.