source-oracle: use rs_id and ssn for keyless tables #2453
Conversation
reworked our test suite a bit to make it faster, less brittle and shorter
| RSID: msg.RSID, | ||
| SSN: msg.SSN, |
There was a problem hiding this comment.
RSID + SSN related change
| @@ -553,6 +557,9 @@ func (s *replicationStream) receiveMessages(ctx context.Context, startSCN, endSC | |||
| return err | |||
| } | |||
|
|
|||
| // For some reason RSID comes with a space before and after it | |||
| msg.RSID = strings.TrimSpace(msg.RSID) | |||
|
|
|||
There was a problem hiding this comment.
RSID + SSN related change
|
|
||
| RSID string `json:"rs_id" jsonschema:"description=Record Set ID of the logical change"` | ||
|
|
||
| SSN int `json:"ssn" jsonschema:"description=SQL sequence number of the logical change"` |
There was a problem hiding this comment.
RSID + SSN related change
|
|
||
| // Artificial RSID and SSN during backfill | ||
| RSID: "0x000000.00000000.0000", | ||
| SSN: rowOffset, |
There was a problem hiding this comment.
RSID + SSN related change
| } | ||
| }, | ||
| "type": "object", | ||
| "required": [ | ||
| "schema", | ||
| "table", | ||
| "row_id" | ||
| "row_id", | ||
| "rs_id", |
There was a problem hiding this comment.
Are there any concerns about breakage to preexisting users when adding these as required? I think the answer is no (very few users at present and at least one seems to have automatic discovery disabled anyway), but just wanted to make sure this has been considered and we're confident no compatibility strategies are required.
There was a problem hiding this comment.
Yeah no real customers are using this seriously yet. Launchmetrics has been the only one who tried it but ended up not fitting their requirements and they are waiting for the batch connector
Description:
Workflow steps:
(How does one use this feature, and how has it changed)
Documentation links affected:
(list any documentation links that you created, or existing ones that you've identified as needing updates, along with a brief description)
Notes for reviewers:
(anything that might help someone review this PR)
This change is