[GitHub] [flink] luoyuxia commented on pull request #21676: [FLINK-30662][table] Planner supports delete

2023-01-19 Thread GitBox
luoyuxia commented on PR #21676: URL: https://github.com/apache/flink/pull/21676#issuecomment-1397093090 Thanks for the detail check. Sorry for missing some thing while merge these commits to two commit. Now, I fix it, each commit should compile and pass test sucessfully. Thank you

[GitHub] [flink] luoyuxia commented on pull request #21676: [FLINK-30662][table] Planner supports delete

2023-01-18 Thread GitBox
luoyuxia commented on PR #21676: URL: https://github.com/apache/flink/pull/21676#issuecomment-1387033925 @lincoln-lil Thanks for your reviewing. I have addressed your comments and test failure. Sorry for the some wrong commit messages in the `3~5` commits. They shouldn't be

[GitHub] [flink] luoyuxia commented on pull request #21676: [FLINK-30662][table] Planner supports delete

2023-01-17 Thread GitBox
luoyuxia commented on PR #21676: URL: https://github.com/apache/flink/pull/21676#issuecomment-1386571875 @lincoln-lil Thanks for your reviewing. I have addressed your comments. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub

[GitHub] [flink] luoyuxia commented on pull request #21676: [FLINK-30662][table] Planner supports delete

2023-01-17 Thread GitBox
luoyuxia commented on PR #21676: URL: https://github.com/apache/flink/pull/21676#issuecomment-1386565864 > one more question for the 2nd row-level delete: consider the delete query without a filter: "DELETE FROM t", does it work under 'REMAINING_ROWS' mode? the generated plan contains an

[GitHub] [flink] luoyuxia commented on pull request #21676: [FLINK-30662][table] Planner supports delete

2023-01-17 Thread GitBox
luoyuxia commented on PR #21676: URL: https://github.com/apache/flink/pull/21676#issuecomment-1386391577 > > > can the new DeleteFromFilterOperation extend the SinkModifyOperation? > > > > > > I think we can make DeleteFromFilterOperation implement ModifyOperation since

[GitHub] [flink] luoyuxia commented on pull request #21676: [FLINK-30662][table] Planner supports delete

2023-01-17 Thread GitBox
luoyuxia commented on PR #21676: URL: https://github.com/apache/flink/pull/21676#issuecomment-1385110220 @lincoln-lil Thanks for reviewing. I have addressed your comments about the first commits. The test failure is about the error message donesn't match, ``` Expecting message to

[GitHub] [flink] luoyuxia commented on pull request #21676: [FLINK-30662][table] Planner supports delete

2023-01-17 Thread GitBox
luoyuxia commented on PR #21676: URL: https://github.com/apache/flink/pull/21676#issuecomment-1385105621 > can the new DeleteFromFilterOperation extend the SinkModifyOperation? I think we can make DeleteFromFilterOperation implement ModifyOperation since `SinkModifyOperation` contains