flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] pnowojski edited a comment on issue #6519: [FLINK-9559] [table] The type of a union of CHAR columns of different lengths should be VARCHAR
Date Tue, 28 Aug 2018 20:12:26 GMT
pnowojski edited a comment 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-416647706
 
 
   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 proper fix superseding this PR? If we would be running out of time, we could
reevaluate this closer to feature freeze.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message