pnowojski commented on issue #6519: [FLINK-9559] [table] The type of a union of CHAR columns of different lengths should be VARCHAR URL: https://github.com/apache/flink/pull/6519#issuecomment-415699060 @hequn8128 may I ask what is the use case that you are trying to solve? I'm asking because I have mixed feelings with this: 1. as it is, it brakes standard, which is bad 2. adding configuration option `shouldConvertRaggedUnionTypesToVarying`, which by default would be `true` is equally bad 3. making it by default `false` leaves us with an option that's very hard to explain to the users what does it to. I would guess almost nobody would use it 4. having more configuration rises the complexity of the system - more features that we need to maintain, support, refactor, test etc. This can quickly become a nightmare. 5. it seems like very cosmetic change. In most cases it doesn't matter, since either the result is being printed or being stored in an external system that has it's own schema. As long as in this external system column is defined as `VARCHAR(x)` this issue doesn't matter. Isn't the scope of this limited purely to unit tests, where we need to compare to spaces? 6. if we agree to introduce this change, should we also agree when next time someone reports that he/she would like to have things like: - `123.456` literal being double or decimal? - `'abc'` literal being `CHAR(3)` or `VARCHAR(3)`? - `'A ' || 'B '` returning `'AB'` or `'A B '` - 'NULL == 0` being true, false or null? - `SUBSTRING('abc' FROM -2 FOR 4)` should yield 'a' or 'bc'? and the list goes on and on. We definitely can not provide a switch for all of such quirks. Having one switch like `mysql-compatibility-mode: true` would have more sense, however it's impossible to implement/maintain for different reasons. That's why I don't like changing the default behaviour and I see more problems then positives by introducing this as a configurable option. However I see one issue here that would solve this problem. Since Flink does not support `CHAR(x)` anywhere besides string literals, and our `CHAR(x)` support seems to be broken (for example `'A ' || 'B '` should return `'A B '`), maybe we should treat all string literals as varchars?
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on 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
