davidradl commented on PR #79: URL: https://github.com/apache/flink-connector-jdbc/pull/79#issuecomment-1881759642
> @davidradl thanks for your efforts Based on findings I created a PR to your branch [davidradl#1](https://github.com/davidradl/flink-connector-jdbc/pull/1) which passes all the tests I've tested so far > > It still keeps your main idea around this PR the thing that probably makes sense to mention > > 1. Use arrays instead of lists in `org.apache.flink.connector.jdbc.table.ParameterizedPredicate` > 2. Concatenate strings in `org.apache.flink.connector.jdbc.table.ParameterizedPredicate#combine` only once, respect cases with multiple predicates on both sides > 3. Introduce `org.apache.flink.connector.jdbc.table.JdbcFilterPushdownPreparedStatementVisitor.FilterPushdownFunction` which allows to make the code more concise especially for `org.apache.flink.connector.jdbc.table.JdbcFilterPushdownPreparedStatementVisitor#visit(org.apache.flink.table.expressions.CallExpression)` > 4. introduce parameterized tests in `org.apache.flink.connector.jdbc.table.ParameterizedPredicatePlanTest` Hi @snuyanzin , Thanks for the changes - you were very quick :-) . I have made one review comment. I have not had time to test your fix. From a quick glance it looks good. I am ok with merging it - if you could have a look at the review comment. I will not be able to do proper testing on this until Thursday. If you need to merge into main before then to move the release though its release candidates, I could test against main. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
