Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/3829
Hi @lincoln-lil, I think it is a very good idea to strive for consistency
with JDBC. Here is what I think about the proposed methods:
- `sqlQuery(sql: String): Table`: Looks good to me
- `sqlUpdate(sql: String): Option[Long]`: This method requires an optional
`QueryConfig` object (either as optional argument or in a second method). We
also need a Java version that does not use `Option` (maybe encode `None` as
`-1L`). We should also think about whether we want the Scala and Java APIs to
be consistent.
- `sql(sql: String): Option[Table]`: I'm not sure about this method. I
think it would rather confuses than helps. IMO, there is a clear separation
between `sqlQuery` and `sqlUpdate` that users have to be aware of. This is even
true, if they would use `sql()` because they would need to handle the returned
`Option`. Moreover, we would need to change the signature of a method which is
a more significant change than deprecating the method.
I think the decision about this API change should not be done in this PR
but discussed on the dev@f.a.o mailing list. Would you like to start a
[DISCUSS] thread and propose your ideas there?
Regarding the issue of distinguishing `SELECT` queries from `INSERT` or
`UPDATE` queries, we can change `FlinkPlannerImpl.parse()` to call
`SqlParser.parseQuery()` instead of `SqlParser.parseStmt()`. `parseQuery()`
accepts `SELECT` queries + `UNION`, `INTERSECT` and `EXCEPT`.
Thanks, Fabian
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
