mihaibudiu commented on code in PR #3429:
URL: https://github.com/apache/calcite/pull/3429#discussion_r1327984728
##########
core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java:
##########
@@ -1409,8 +1411,20 @@ public static SqlNode toSql(RexLiteral literal) {
() -> "literal " + literal
+ " has null SqlTypeFamily, and is SqlTypeName is " +
typeName);
switch (family) {
- case CHARACTER:
+ case CHARACTER: {
+ NlsString value = literal.getValueAs(NlsString.class);
+ if (value != null) {
+ String charsetName = value.getCharsetName();
+ if (charsetName != null
+ &&
charsetName.equals(CalciteSystemProperty.DEFAULT_CHARSET.value())) {
+ // Do not set the charset if it is the same as the default charset
+ charsetName = null;
Review Comment:
Before this change it was always null, which is wrong.
A null charset seems to have the same meaning as the default charset.
But when you unparse them, they look different: the null charset shows just
a literal, e.g., 'x', but the default charset (at least the default default
charset) shows up as `_ISO-8859-1'`x'`
So if I leave the charset always there many tests need to be changed.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]