[GitHub] [calcite] vlsi commented on a change in pull request #1632: [CALCITE-3560] Additional calcite.util.Source implementation for generic text source (eg. CharSource)

[GitBox <gi...@apache.org>]

vlsi commented on a change in pull request #1632: [CALCITE-3560] Additional calcite.util.Source implementation for generic text source (eg. CharSource)
URL: https://github.com/apache/calcite/pull/1632#discussion_r353943555
 
 

 ##########
 File path: example/csv/src/main/java/org/apache/calcite/adapter/csv/JsonEnumerator.java
 ##########
 @@ -67,15 +67,8 @@ static JsonDataConverter deduceRowType(RelDataTypeFactory typeFactory, Source so
       objectMapper.configure(JsonParser.Feature.ALLOW_UNQUOTED_FIELD_NAMES, true)
           .configure(JsonParser.Feature.ALLOW_SINGLE_QUOTES, true)
           .configure(JsonParser.Feature.ALLOW_COMMENTS, true);
-      if (source.file().exists() && source.file().length() > 0) {
-        if ("file".equals(source.protocol())) {
-          //noinspection unchecked
-          jsonObj = objectMapper.readValue(source.file(), Object.class);
-        } else {
-          //noinspection unchecked
-          jsonObj = objectMapper.readValue(source.url(), Object.class);
-        }
-      }
+
+      jsonObj = objectMapper.readValue(source.reader(), Object.class);
 
 Review comment:
   Technically speaking, this makes error messages harder to understand. For instance, `objectMapper.readValue(File,...` can produce targeted exception messages and refer to the file and the line number. However, if you abstract that behind `reader`, then targeted exceptions are not possible

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services