CNF96 commented on PR #10416: URL: https://github.com/apache/seatunnel/pull/10416#issuecomment-3828420948
> > @LiJie20190102 Hi, the relevant logic has been fully covered by existing tests, see: [PostgresCDCIT](https://github.com/apache/seatunnel/blob/dev/seatunnel-e2e/seatunnel-connector-v2-e2e/connector-cdc-postgres-e2e/src/test/java/org/apache/seatunnel/connectors/seatunnel/cdc/postgres/PostgresCDCIT.java#L330,) so no duplicate test cases are written. > > The original code logic has two issues: first, it will throw exceptions in Chinese OS because PostgreSQL's exception messages are automatically adapted to the system language, causing the relevant judgment to fail; second, the original logic itself is not rigorous and has potential risks. > > I have reviewed the relevant code of Flink CDC at `https://github.com/apache/flink-cdc/blob/release-3.5/flink-cdc-connect/flink-cdc-source-connectors/flink-connector-postgres-cdc/src/main/java/org/apache/flink/cdc/connectors/postgres/source/fetch/PostgresScanFetchTask.java`. Is it possible to simply add `if (slotInfo == null)` instead of modifying `replicationConnection.createReplicationSlot().orElse(null)` <img alt="image" width="809" height="515" src="https://private-user-images.githubusercontent.com/53458004/543154158-26d640e2-bfde-48c2-aa32-2f369c2328ad.png?jwt=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Njk4NjM3NjcsIm5iZiI6MTc2OTg2MzQ2NywicGF0aCI6Ii81MzQ1ODAwNC81NDMxNTQxNTgtMjZkNjQwZTItYmZkZS00OGMyLWFhMzItMmYzNjljMjMyOGFkLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNjAxMzElMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFt ei1EYXRlPTIwMjYwMTMxVDEyNDQyN1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTlhODViOGE5NjVjNGNhZTU3NWJkYWZmMzZjZTgwYTkxMjM1OWVhODgyYjZkM2M3MDY4MzQ1NzBjODc1Yzc1YTQmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.D0rUNzoULRZ1YFIcxUrnctADHLrKXDrU_6ZCPHEfSyQ"> Thanks a lot for your professional review and valuable suggestion! I fully accept this optimization plan. The core reason is that your approach follows the principle of minimal code modification — directly adding the null judgment if (slotInfo == null) avoids unnecessary changes to the original replicationConnection.createReplicationSlot().orElse(null) method call, which keeps the current code structure intact, reduces the potential risk of introducing new bugs due to method modification, and can still accurately achieve the null check effect required for the business logic. -- 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]
