libenchao commented on PR #20140: URL: https://github.com/apache/flink/pull/20140#issuecomment-1207632475
@qingwei91 Thanks for your contribution. For the current design, I see you use `ValueLiteralExpression#toString()` to generate the string for literals. This may work for some cases, but not for all cases IMHO. Consider following cases: 1. `ValueLiteralExpression#toString()` uses Flink dialect to represent string, and it hard-coded the quote with `'`. However, in many DBMS, `'` is not the only choice. 2. `ValueLiteralExpression#toString()` only handles special character `'` using escape. In many DBMS, they need more special character handing, e.g. [mysql driver](https://github.com/mysql/mysql-connector-j/blob/release/8.0/src/main/protocol-impl/java/com/mysql/cj/protocol/a/StringValueEncoder.java#L72) 3. For other types, e.g. `TIMESTAMP`, `DATE`, `TIME`, `INTERVAL` and so on, they may suffers from this too, because we cannot assume that all DB dialects handle them in the same way. Another more general way to handle this is to use `PrepareStatement.setXXX` just like we already did in `TableSimpleStatementExecutor` and `JdbcRowDataLookupFunction`. WDYT? -- 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]
