eskabetxe commented on PR #160:
URL:
https://github.com/apache/flink-connector-jdbc/pull/160#issuecomment-2792150160
I agree with @davidradl — `JdbcDynamicTableSource` should not have any
direct reference to a specific dialect, and we need to add a unit test for
this..
Since we're assuming that getLimitClause will always append the clause at
the end of the query, I suggest introducing a new method in `JdbcDialect`:
```
String addLimitClause(String query, long limit);
```
We can then provide a default implementation in `AbstractDialect`:
```
@Override
public String addLimitClause(String query, long limit) {
return String.format("%s %s", query, dialect.getLimitClause(limit));
}
```
Changing in `JdbcDynamicTableSource`
```
if (limit >= 0) {
query = dialect.addLimitClause(query, limit);
}
```
For dialects like SQL Server, this method can be overridden to inject the
limit clause appropriately. For example:
```
// SqlServerDialect
@Override
public String addLimitClause(String query, long limit) {
return query.replace("SELECT", String.format("SELECT TOP %s", limit));
}
```
Optionally, the `if (limit >= 0)` check in `JdbcDynamicTableSource` could be
moved into this new method for better encapsulation, but I’ve left it out here
for simplicity.
--
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
