Good point with the comparisons. I meant that after changing literal types to `VARCHAR` (and accepting that this brakes SQL standard) our comparisons/functions (like `concat`) would be more SQL standard compliant, without the need to fix them. Saying that our literals are `VARCHAR` and we are being consistent with handling them is better then providing broken `CHAR`. Also it should be a cheaper and way easier for us compared to finding all places that do not handle `CHAR` as they should and fixing them. As a side note, I generally consider `CHAR` as a pretty useless type, especially since unicode it's no longer fixed length.
Changing type of string literal would be easy in calcite (`org.apache.calcite.rex.RexBuilder#makePreciseStringLiteral`). On our side it's might a little bit be more tricky. I might be wrong here, but it might be enough to provide `RexShuttle` that do `CHAR` to `VARCHAR` type rewrite and apply it to `org.apache.calcite.rel.AbstractRelNode#accept(org.apache.calcite.rex.RexShuttle)` as a yet another "zero" step. Which also doesn't sound overly complicated. `shouldConvertRaggedUnionTypesToVarying` both with better `CHAR` support or `VARCHAR` literals looses it's purpose (and becomes cosmetic change), or doesn't it? Since I consider this our `CHAR` handling to be a serious problem, I would like it to be fixed (in one way or the other) before 1.7.0 release. If we could fix this properly, would you be fine with dropping this PR? If we would be running out of time, we could reevaluate this closer to feature freeze. [ Full content available at: https://github.com/apache/flink/pull/6519 ] This message was relayed via gitbox.apache.org for [email protected]
