fix(gitlab): always re-collect MR commits regardless of MR updated_at… - #8959
Merged
Conversation
… to prevent missing commits in incremental runs
Contributor
Author
|
Hi @klesh could you please review and approve pull request? |
Contributor
|
Please fix the failed checks first. |
Contributor
Author
|
Hi @klesh — I've fixed the build. 5c66376 corrects the if-brace syntax error in GetMergeRequestsIterator that was breaking lint, unit-test, and the e2e checks; it now compiles cleanly (go build ./plugins/gitlab/... passes locally) and I've merged latest main. The workflows are showing "awaiting approval" — could you approve the run so CI can execute, then re-review? Thanks! |
bujjibabukatta
force-pushed
the
fix/#8818
branch
2 times, most recently
from
August 11, 2026 09:08
8bac968 to
5c66376
Compare
klesh
approved these changes
Aug 12, 2026
klesh
left a comment
klesh
left a comment
Contributor
There was a problem hiding this comment.
LGTM.
Thanks for your contribution.
DoDiODev
added a commit
to DoDiODev/devlake
that referenced
this pull request
Aug 12, 2026
…tal filter apache#8959 changed `GetMergeRequestsIterator` to filter merge requests by `GREATEST(gmr.gitlab_updated_at, COALESCE(gmr.commit_updated_at, gmr.gitlab_updated_at))`, but `commit_updated_at` does not exist on `_tool_gitlab_merge_requests`: there is neither a model field nor a migration script, and nothing ever writes the value. As a result every incremental "Collect MR Notes" run (the only caller that passes a stateful collector; the MR commit collector passes nil) aborts with: Error 1054 (42S22): Unknown column 'gmr.commit_updated_at' in 'where clause' This makes the intended behaviour actually work instead of reverting it: * add `CommitUpdatedAt` to `GitlabMergeRequest` plus a migration script * maintain the column in the MR commit extractor, setting it to the latest authored date of the MR's commits, so MRs that received new commits without their own `updated_at` being bumped (e.g. force-pushes) are picked up again * the collector filter is unchanged and now resolves against a real column Verified on MySQL 8 and PostgreSQL 14 (extractor UPDATE and the GREATEST filter produce identical results on both).
klesh
pushed a commit
that referenced
this pull request
Aug 14, 2026
…tal filter (#9046) #8959 changed `GetMergeRequestsIterator` to filter merge requests by `GREATEST(gmr.gitlab_updated_at, COALESCE(gmr.commit_updated_at, gmr.gitlab_updated_at))`, but `commit_updated_at` does not exist on `_tool_gitlab_merge_requests`: there is neither a model field nor a migration script, and nothing ever writes the value. As a result every incremental "Collect MR Notes" run (the only caller that passes a stateful collector; the MR commit collector passes nil) aborts with: Error 1054 (42S22): Unknown column 'gmr.commit_updated_at' in 'where clause' This makes the intended behaviour actually work instead of reverting it: * add `CommitUpdatedAt` to `GitlabMergeRequest` plus a migration script * maintain the column in the MR commit extractor, setting it to the latest authored date of the MR's commits, so MRs that received new commits without their own `updated_at` being bumped (e.g. force-pushes) are picked up again * the collector filter is unchanged and now resolves against a real column Verified on MySQL 8 and PostgreSQL 14 (extractor UPDATE and the GREATEST filter produce identical results on both).
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
2 participants
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.
When running incremental collection, GetMergeRequestsIterator filters merge requests using gitlabupdatedat > since.
When running incremental collection, GetMergeRequestsIterator filters merge requests using gitlab_updated_at > since. This works fine for notes and reviews — if the MR hasn't been touched, nothing new to collect. But for commits it's the wrong filter. Pushing new commits to a MR's source branch doesn't update gitlab_updated_at on the MR itself, so those MRs get skipped entirely and their new commits are never fetched. This is exactly why full refresh works but normal collect misses data.
The fix is to pass nil instead of apiCollector when calling GetMergeRequestsIterator in the commit collector. Passing nil skips the time filter so every MR is scanned for commits on each run, matching full refresh behaviour. The MR commits endpoint is lightweight so the performance impact is minimal. Notes and reviews collectors are unchanged — they still pass apiCollector and keep their incremental filtering, which is correct for those entities.