[GitHub] gianm commented on a change in pull request #6027: Make Parser.parseToMap() to return a mutable Map

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

gianm commented on a change in pull request #6027: Make Parser.parseToMap() to return a mutable Map
URL: https://github.com/apache/incubator-druid/pull/6027#discussion_r206743451
 
 

 ##########
 File path: java-util/src/main/java/io/druid/java/util/common/parsers/Parser.java
 ##########
 @@ -40,10 +40,13 @@ default void startFileFromBeginning()
   /**
    * Parse a String into a Map.  The result can be null which means the given input string will be ignored.
    *
+   * This method is defined to return LinkedHashMap instead of just Map to guarantee the mutability (that is needed
+   * for https://github.com/apache/incubator-druid/pull/6027) and the order of the entries.
+   *
    * @throws ParseException if the String cannot be parsed
    */
   @Nullable
-  Map<K, V> parseToMap(String input);
+  LinkedHashMap<K, V> parseToMap(String input);
 
 Review comment:
   Other comments aside, I don't think we should change this signature in a patch release. `Parser` is returned by `ParseSpec` which is an extension point. I think it's an oversight that Parser isn't yet tagged with ExtensionPoint too.

----------------------------------------------------------------
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

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org