CloverDew commented on PR #10890:
URL: https://github.com/apache/seatunnel/pull/10890#issuecomment-4562844939

   > I just noticed an issue that needs to be confirmed by @CloverDew.
   > 
   > The lower bound of the DDL history query is inconsistent with the 
semantics of the main reading window, causing events at the fromLsn boundary to 
be missed. SqlServerStreamingChangeEventSource.java:235-245
   > 
   > ### Problem Description
   > The main code clearly states that "reading interval is inclusive". For 
this reason, `incrementLsn()` is executed first in the steady state. For the 
initial run or recovery window, `fromLsn` is directly assigned as 
`lastProcessedPosition.getCommitLsn()`.
   > 
   > However, the newly added DDL history query adopts a different logic:
   > 
   > This excludes DDL statements whose LSN is exactly equal to fromLsn. In 
other words, the data reading window follows a closed interval semantics, while 
the DDL reading window uses a left-open and right-closed interval. The 
semantics of these two windows are now inconsistent.
   > 
   > ### Potential Risks
   > When the task performs the initial incremental read, starts from a 
specified offset, or replays incomplete transactions after recovery, if a DDL 
statement corresponds exactly to fromLsn:
   > 
   > * Data rows may be re-read.
   > * The corresponding schema event will be lost.
   > * The downstream system will subsequently process data with new column 
layouts based on the old schema, or directly fail to capture required schema 
evolution.
   > 
   > This is a correctness issue in the main workflow, and it does not only 
occur in edge cases.
   > 
   > ### Improvement Suggestion
   > Adjust the lower bound of the DDL history query to adopt the same closed 
interval semantics as the main reading window, namely `ddl_lsn >= ? AND ddl_lsn 
<= ?`. The `incrementLsn()` logic will still be used in the steady state to 
prevent duplicate processing.
   > 
   > If a fix is required, please submit another Pull Request. I have just 
merged this PR.
   
   okay, thank you for reviewing. I've created an issue to track this problem.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to