Skip to content

Conversation

whhe
Copy link
Member

@whhe whhe commented Jul 13, 2022

fnmatch is used to match pattern of change events, while regex is used for snapshot reading.

In this pr, I make the log client to fetch all the data at first and do filter with regex in the process part.

@whhe
Copy link
Member Author

whhe commented Jul 21, 2022

Please help review this pr when you are available. @leonardBang @GOODBOY008 @Jiabao-Sun

The ci failure is not related to my modification.

Copy link
Contributor

@Jiabao-Sun Jiabao-Sun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @whhe for the excellent work to push this feature forward.

From my point of view, I think we can do some optimizations mentioned in the comments, especially in the scenario of a single database or a single table.

In addition, using not fully qualified table regex does not seem to be able to partially filter the tables with same name but in different databases. Shall we use the fully qualified tableList like MySQL CDC connector?

CC @leonardBang

@whhe
Copy link
Member Author

whhe commented Jul 21, 2022

Thanks @whhe for the excellent work to push this feature forward.

From my point of view, I think we can do some optimizations mentioned in the comments, especially in the scenario of a single database or a single table.

In addition, using not fully qualified table regex does not seem to be able to partially filter the tables with same name but in different databases. Shall we use the fully qualified tableList like MySQL CDC connector?

CC @leonardBang

Thanks for your advice.

I used regex here at the beginning and so I keep it in this pr. And it make sense to use fully qualified table list, the monitor list can be more accurate in this way. I would change it asap.

Copy link
Contributor

@Jiabao-Sun Jiabao-Sun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @whhe. I left a few comments.

@whhe
Copy link
Member Author

whhe commented Jul 27, 2022

@Jiabao-Sun Docs updated, please take a look when you are available.

@Jiabao-Sun
Copy link
Contributor

Thanks @whhe for the update.
Hi @leonardBang, do you have some suggestions?

@whhe
Copy link
Member Author

whhe commented Jul 28, 2022

@leonardBang Comments addressed. PTAL.

Copy link
Contributor

@leonardBang leonardBang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @whhe for the update, LGTM. Btw, the logic in OceanBaseRichSourceFunction is too long to read, we can split it in the future.

@leonardBang leonardBang merged commit 2de4903 into apache:master Aug 2, 2022
ChaomingZhangCN pushed a commit to ChaomingZhangCN/flink-cdc that referenced this pull request Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants