stoty commented on a change in pull request #150:
URL: https://github.com/apache/calcite-avatica/pull/150#discussion_r695508542
##########
File path: core/src/main/java/org/apache/calcite/avatica/ColumnMetaData.java
##########
@@ -360,6 +361,8 @@ public ColumnMetaData setRep(Rep rep) {
builder.put(rep.clazz, rep);
}
builder.put(byte[].class, BYTE_STRING);
+ //The lookup code is not smart enough to handle subclasses
+ builder.put(BigDecimal.class, NUMBER);
Review comment:
So I spent some more time looking at this.
AFAICT, adding a new ColumnMetaData.Rep value would not affect backwards
compatibility with Protobuf, as we're already putting and expecting
Common.Rep.BIG_DECIMAL on the wire anyway.
However, with JSON we do put (the string value of) ColumnMetaData.Rep on
the wire and try to parse it on the other side, so adding a new a value (using
BIG_DECIMAL instead of NUMBER) would break backwards compatibility with older
JSON servers, which could not parse that.
see https://calcite.apache.org/avatica/docs/json_reference.html#typedvalue
The current solution doesn't break backwards comatibility
WDYT @joshelser ?
--
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]
