Add the upsert logic for updating the recent update timestamp for spanner Node table#513
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds a last_update_timestamp column to the Node table schema and updates SpannerClient.java to populate this field with the commit timestamp during node mutations. The reviewer noted that existing databases will not automatically receive this new column, which could lead to runtime write failures, and suggested applying a manual migration or updating the initialization logic. Additionally, the reviewer recommended adding unit tests to verify the mutation builder logic.
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| CodeStyle | 2 minor |
🟢 Metrics 0 complexity · 2 duplication
Metric Results Complexity 0 Duplication 2
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
gmechali
left a comment
There was a problem hiding this comment.
Couple nits but LGTM otherwise.
I just want to be very careful with the rollout of this - if any instance trying to run this doesnt have the last_update_timestamp, I think it would fail.
So the question is.... is there any instance that doesnt have that column?
For DCP, we're fine to proceed. Any base DC spanner DBs without this column? @vish-cs @keyurva
This PR added the mutation logic to update the timestamp based on the insert/update logic from spanner table instruction https://docs.cloud.google.com/spanner/docs/commit-timestamp-postgresql#update-row