[
https://issues.apache.org/jira/browse/CALCITE-1026?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16579889#comment-16579889
]
Sergey Nuyanzin commented on CALCITE-1026:
------------------------------------------
Thank you for your response
I think at least for historical reason JSON is "preferred".
Based on the fact that in ModelHandler there is a call only to
{{ObjectMapper#readValue(java.lang.String, java.lang.Class<T>)}} from Jackson
and this is the same method for both JSON and YAML and on high level they work
with the same JsonRoot objects. Moreover I looked into jackson-dataformat-yaml
[1] they have tests for parsing JSON/YAML so I do not think we should write
tests both for JSON and YAML models each time a new model appears. But we
should if the logic of detection JSON or YAML changes (that is not covered by
jackson) for instance there will be added one more "inline:" like property.
I would agree that splitting ModelTest makes confused. May be it makes sense
not to split it but just to add several tests inside ModelTest to check if
JSON/YAML format is detected correctly.
Agree with necessity of docs updates at least the mentioned one
[https://calcite.apache.org/docs/adapter.html#jdbc-connect-string-parameters].
Will update it.
Agree on a point about line windows/linux endings - will think about tests
against it.
[1] https://github.com/FasterXML/jackson-dataformat-yaml
> Model in YAML format
> --------------------
>
> Key: CALCITE-1026
> URL: https://issues.apache.org/jira/browse/CALCITE-1026
> Project: Calcite
> Issue Type: Bug
> Reporter: Julian Hyde
> Assignee: Julian Hyde
> Priority: Major
>
> Models must currently be in JSON. YAML is more forgiving and flexible, so we
> should support YAML as a format.
> Jackson (versions 2.5 and above) can read YAML files and produce the same
> output as the corresponding JSON.
> As part of this change, we should add an example to
> http://calcite.apache.org/docs/model.html.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)