fix: auto select cursor field for default incremental sync mode (#976)#985
Open
Saquib1973 wants to merge 10 commits into
Open
fix: auto select cursor field for default incremental sync mode (#976)#985Saquib1973 wants to merge 10 commits into
Saquib1973 wants to merge 10 commits into
Conversation
Saquib1973
requested a deployment
to
integration_tests
GitHub Actions
Waiting
Saquib1973
requested a deployment
to
integration_tests
GitHub Actions
Waiting
|
|
Contributor
Author
|
@nayanj98 my pr is ready for review |
Collaborator
|
Assigning @vaibhav-datazip to review your PR |
| convStream.SyncMode = types.CDC | ||
| } else if convStream.SupportedSyncModes.Exists(types.INCREMENTAL) { | ||
| convStream.SyncMode = types.INCREMENTAL | ||
| if convStream.CursorField == "" && convStream.AvailableCursorFields.Len() > 0 { |
Collaborator
There was a problem hiding this comment.
isn't cursorField = "" guard redundant ?
also can you add a crisp and clear comment here why this has been added and how it helps
Contributor
Author
There was a problem hiding this comment.
According to the issue, user can select cursorField during discover
- If cursorField already filled skip
- If the cursorField is empty and there is a availableCursorFields, set cursrorField to the first availableCursorField
Collaborator
There was a problem hiding this comment.
no I am saying can this ever happen that we get cursorField as non empty in discover function here
Contributor
Author
There was a problem hiding this comment.
In Discover(), every column is added to AvailableCursorFields using WithCursorField() while generating productSchema, but cursorField is never set.
Comment on lines
+148
to
+150
| if oldStream.Stream.CursorField != "" { | ||
| newStream.Stream.CursorField = oldStream.Stream.CursorField | ||
| } |
Collaborator
There was a problem hiding this comment.
add a test case for this in catalog_test as well
vaibhav-datazip
requested a deployment
to
integration_tests
GitHub Actions
Waiting
vaibhav-datazip
requested a deployment
to
integration_tests
GitHub Actions
Waiting
Saquib1973
requested a deployment
to
integration_tests
GitHub Actions
Waiting
Saquib1973
requested a deployment
to
integration_tests
GitHub Actions
Waiting
… incremental sync" This reverts commit 21372fe.
Saquib1973
requested a deployment
to
integration_tests
GitHub Actions
Waiting
Saquib1973
requested a deployment
to
integration_tests
GitHub Actions
Waiting
Signed-off-by: nayanj98 <nayan@datazip.io> Co-authored-by: nayanj98 <nayan@datazip.io> Co-authored-by: Cursor <cursoragent@cursor.com>
vaibhav-datazip
requested a deployment
to
integration_tests
GitHub Actions
Waiting
vaibhav-datazip
requested a deployment
to
integration_tests
GitHub Actions
Waiting
…uib1971/olake into fix-auto-select-cursor-field
Saquib1973
requested a deployment
to
integration_tests
GitHub Actions
Waiting
Saquib1973
requested a deployment
to
integration_tests
GitHub Actions
Waiting
Saquib1973
requested a deployment
to
integration_tests
GitHub Actions
Waiting
Saquib1973
requested a deployment
to
integration_tests
GitHub Actions
Waiting
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
5 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.
Description
Automatically assigns the first available cursor field when the default sync mode is
INCREMENTALduring the discovery phase. Additionally, updatesmergeCatalogsto prevent an old catalog's empty cursor field from overwriting the newly auto-selected default, while still preserving explicit user choices.Fixes #976
Type of change
How Has This Been Tested?
Tested locally using a PostgreSQL database configured without CDC support to force the
INCREMENTALfallback.discovercommand on PostgreSQL to verifystreams.jsoncorrectly populates thecursor_fieldwith the 0th available field instead of an empty string.synccommand on PostgreSQL to confirm the pipeline executes successfully without thezero-length delimited identifierSQL error.Screenshots or Recordings
2026-06-11.23-21-05.mp4
Documentation
Related PR's (If Any):
None