Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Comments are not allowed in mapping #1394

Closed
dadoonet opened this issue Oct 13, 2011 · 18 comments
Closed

Comments are not allowed in mapping #1394

dadoonet opened this issue Oct 13, 2011 · 18 comments

Comments

@dadoonet
Copy link
Member

Hi,

I would like to be able to send comment in mapping as I managed mappings in files.
So by now, it's not allowed to send that kind of mapping :

curl -XPUT http://localhost:9200/twitter/tweet/_mapping -d @tweet.json

with tweet.json content

/* Test comment */
{
  "tweet":{
    "properties":{
      "user":{
        "type":"string" // This a comment on field
      }
    }
  }
}

You get an error like :

{"error":"MapperParsingException[Failed to parse mapping definition]; nested: JsonParseException[Unexpected character ('/' (code 47)): maybe a (non-standard) comment? (not recognized as one since Feature 'ALLOW_COMMENTS' not enabled for parser)\n at [Source: /* Test comment */{  \"tweet\":{    \"properties\":{      \"user\":{        \"type\":\"string\"      }    }  }}; line: 1, column: 2]]; ","status":400}

I think it's only need to set Jackson property as follow

objectMapper.configure(JsonParser.Feature.ALLOW_COMMENTS, true);

Could it have any side effects ?

Thanks.
David.

@kimchy
Copy link
Member

kimchy commented Oct 14, 2011

That won't work, we might be able to parse it with comments (thanks to jackson, its not a valid json), but, the way elasticsearch works is that it parses the mappings into its own structure, and then serialize it based on it back to the json string representation (thus causing comments to be lost...).

@karussell
Copy link
Contributor

or probably skip lines beginning with # before parsing?

@kimchy
Copy link
Member

kimchy commented Oct 15, 2011

@karussell the problem is not with parsing, as that can be handled, its with how elasticsearch handles mapping. The json provided gets parsed into elasticsearch own mapping model, and then serialized back into the json (to form a consistent form of the mapping json, and only output whats applicable). There is no handling for comments in the mapping data structures so they can be serialized back.

@dadoonet
Copy link
Member Author

@kimchy : I don't need to get comments back from ES. It's okay to loose it.
I just need to be able to manage my mapping definition files in my VCS and I would like to share comments with other developpers.

@kimchy
Copy link
Member

kimchy commented Oct 18, 2011

I see, I can enable comments in json parsing, just want to check the parsing overhead costs...

@dadoonet
Copy link
Member Author

Nice. I can understand that you can not allow comments only for _mapping endpoint, so it can have side effects on other endpoints.
So, if there is any cost, as it's only a request feature for "confort" and it's not standard JSON, we can forget it !!!

@apatrida
Copy link
Contributor

I have similar issue with a really long mapping JSON file and without comments it is evil to manage. Having them stripped on return is no problem, it is the source material that requires them, not echo'ing or retrieval of the result

@Mpdreamz
Copy link
Member

Mpdreamz commented Apr 2, 2012

+1 for enabling comments. I am also ok with not getting them back from the _mapping endpoint.

@kkrauth
Copy link

kkrauth commented Apr 22, 2012

Same here. My mapping file is heavily commented so that I remember why certain fields are configured the way they are. Also no need to have the comments preserved in the ES internal representation as I assume most people would have the original mapping stored in a VCS.

For testing purposes, I use the following (using Jackson) to strip out comments from my config and apply it to ES

JsonFactory jf = new JsonFactory();
jf.enable(JsonParser.Feature.ALLOW_COMMENTS);
ObjectMapper om = new ObjectMapper(jf);
JsonNode root = om.readTree("{/*Some comment*/\"field\":\"value\"}");
String noComments = om.writeValueAsString(root);
assertEquals("{\"field\":\"value\"}", noComments);

Which gives me an idea, if enabling JsonParser.Feature.ALLOW_COMMENTS globally could incur a penalty overhead, why not enable it for mappings only? I would imagine mappings don't get changed often, and it seems that comments are most useful precisely in that scenario.

@sp-sensaria
Copy link

Putting in my +1. Mappings get lengthy and complicated pretty quickly (and often involve a lot of little compromises whose rationalizations are quickly lost to time). It would be very helpful to be able to annotate them inline.

@simplechris
Copy link
Contributor

+1 - It'd be really helpful to be able to annotate my mapping files inline.

@kimchy kimchy closed this as completed in 6f7253c Jan 7, 2013
@lukas-vlcek
Copy link
Contributor

Found an interesting article discussing comments in JSON. It might be useful to keep the link here.
http://bolinfest.com/essays/json.html

@Hubbitus
Copy link

+1

3 similar comments
@knoxxs
Copy link

knoxxs commented Sep 10, 2015

+1

@sgeeroms
Copy link

+1

@wenma
Copy link

wenma commented Aug 16, 2017

+1

@theBull
Copy link

theBull commented Apr 20, 2018

@kkrauth your suggestion worked for me for multi-line and single-line comments.

String unparsed1 = "/*this\n is\n a\n multi-line\n comment*/ {\"foo\": 1 /*another comment*/}";
String unparsed2 = "// single-line comment\n{\n  \"name\": \"1\"\t\t\n}";
JsonFactory jsonFactory = new JsonFactory();
jsonFactory.enable(JsonParser.Feature.ALLOW_COMMENTS);
ObjectMapper mapper = new ObjectMapper(jsonFactory);

// converting the json strings into JsonNodes
JsonNode parsed1 = mapper.readTree(unparsed2);
parsed1.get("foo"); // will contain 1

JsonNode parsed2 = mapper.readTree(unparsed2);
parsed2.get("name"); // will contain "1"

Sorry for the shoddy example; on a tight timeline at work but I hope this helps someone.

@dingmike
Copy link

dingmike commented Jul 2, 2018

haha

williamrandolph pushed a commit to williamrandolph/elasticsearch that referenced this issue Jun 4, 2020
elastic#1368 introduces a plugin that is documents scala apis
for java developers. This plugin does not support 2.10 and
this variant was missed in the initial testing.

This commit for 7.x conditionally uses this plugin based on the
scala major version. 2.10 support has been removed for 8.x.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests