-
Notifications
You must be signed in to change notification settings - Fork 88
Static Code Generator #96
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a modern static code generator for PostgreSQL versioning triggers along with comprehensive end-to-end tests and updated documentation.
- Adds new SQL files for static trigger generation and automatic event‐trigger based re‐rendering.
- Updates the versioning functions, test suites (static, legacy, integration, and event triggers), and CI/CD configurations.
Reviewed Changes
Copilot reviewed 25 out of 29 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| versioning_tables_metadata.sql | Adds a metadata table for managing versioned table configurations. |
| versioning_function*.sql | Updates timestamp format in the versioning functions. |
| test/* | Extensive additions/updates to end-to-end tests covering all versioning features. |
| generate_static_versioning_trigger.sql | Introduces a static trigger generator with advanced conflict mitigation and migration mode. |
| event_trigger_versioning.sql | Adds an event trigger for automatic re-rendering upon table schema changes. |
| README.md and other config files | Updates documentation and project configurations (CI, ESLint, Prettier, etc.). |
Comments suppressed due to low confidence (2)
test/e2e/test-static-generator.ts:98
- Consider converting the value to a number before comparison to ensure type consistency between the inserted bigint and its assertion.
deepStrictEqual(mainResult.rows[0].a, '3')
jest.config.json:10
- Verify that the testRegex configuration matches the naming convention used for your test files (e.g., if your tests use a .ts suffix rather than .spec.ts).
"testRegex": ".spec.ts$",
|
|
||
| IF TG_OP = 'UPDATE' OR TG_OP = 'DELETE' OR (%6$L AND TG_OP = 'INSERT') THEN | ||
| IF NOT %6$L THEN | ||
| -- Ignore rows already modified in the current transaction |
Copilot
AI
Jul 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Add inline comments to explain the rationale behind comparing OLD.xmin with the txid_current() modulo expression to improve future maintainability and clarity.
| -- Ignore rows already modified in the current transaction | |
| -- Ignore rows already modified in the current transaction | |
| -- OLD.xmin represents the transaction ID of the row's creation. | |
| -- txid_current() returns the current transaction ID, and the modulo operation (%% (2^32)) | |
| -- handles transaction ID wraparound in PostgreSQL, which occurs after 2^32 transactions. | |
| -- This comparison ensures that rows modified in the current transaction are skipped. |
…static-trigger-generator
|
I'm thinking we should probably consider abandoning these changes, just because they are nearly 70 days old. What do you think? |
Let's say that the fact that this code has been here for some time doesn't alone justify closing the PR. On the other hand, the main reason why this code has been here for some time is that there isn't an easy way to test this, outside of the automated tests that you put in place. So the path forward is slightly tricky, because I think we can agree that we don't want to entirely replace the current implementation, but rather offer an alternative solution. Yet I'm not sure on the basis of what users would choose which one to use. Any thoughts on that? |
|
I'll have to mull that over. |
No rush 👌 |
|
Closing, for now |
I've written a new version of temporal_tables Open Source project.
The motivation for the rewrite is my discovery of the number of procedural language SQL calls that are being done for every single row.
The legacy version is designed to work with any table and makes a tremendous amount of calls along the way, which is expensive.
The modern implementation uses a code-generator approach to building table-specific triggers that reduce the number of actual run time calls in the trigger dramatically.
The result is a new version that runs in half the time (or twice as fast!) as the old version.
To keep things in sync when the table changes i.e. regenerate the trigger, an event trigger is created that intercepts ALTER TABLE statements and regenerates the trigger(s) as needed.
Closes #95