twalthr commented on issue #8087: [FLINK-12029][table] Add column operations for TableApi URL: https://github.com/apache/flink/pull/8087#issuecomment-483186032 Thanks for the contribution @hequn8128. I haven't had a look at the code but at the API again. I still find the `columns(...) vs. -columns(...)` confusing. Why not making it more explicit and fluent, e.g. `withColumns(...) vs. withoutColumns(...)`. There are a couple of advantages: 1. It reads nicely: "select with columns 1 to 5" or "udagg with columns 1 to 5". 2. When I saw the design doc the first time, I thought users can do things like `columns(1 to 5)-columns(2)` (so not just unary minus). 2. It avoids ambiguity. Because a column operation with a single column could be misinterpreted. Take `select(-columns('a))` for a schema `('a, 'b)` as an example. Given that a `columns(...)` is just defined as a replacement operation. This could be interpreted as `select(-'a)` or `select('b)`. 3. We already need to keep a new Java DSL in mind. `withColumns()` and `withoutColumns()` can be easily expressed in Java. The current design would look like `select(minus(columns(...)))`. What do you think?
---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
