search

amazon-ion / ion-go

A Go implementation of Amazon Ion.
https://amazon-ion.github.io/ion-docs/
Apache License 2.0
175 stars 31 forks source link

Support for System Symbols as Annotations in Text Writer #94

Closed matthewh-BQ closed 4 years ago

matthewh-BQ commented 4 years ago

Text Writer and Text Reader have a concept of SymbolTable. This introduces the concept into the Text Writer first by bringing down the SymbolTable to the base writer. When a writer is constructed, a default system table is created and then referenced to resolve any Symbols as annotations.

matthewh-BQ commented 4 years ago

@therapon : As discussed earlier. I think there was some clarification on whether this is an issue or not. Current behaviour keeps system symbols annotations as $1, $2, etc. This is currently failing on the TextRoundTrip test because the text writer output differs from the binary writer output. The Binary Writer performs the resolution, so there is a mismatch.

therapon commented 4 years ago

@therapon : As discussed earlier. I think there was some clarification on whether this is an issue or not. Current behaviour keeps system symbols annotations as $1, $2, etc. This is currently failing on the TextRoundTrip test because the text writer output differs from the binary writer output. The Binary Writer performs the resolution, so there is a mismatch.

@matthewh-BQ I looked at the Ion Java library and I think I know what's tripping us up here. We need to clarify in the API the following two cases when adding an annotation

  1. Add an annotation and the value to add is a string. The intent here (I think) is "here is the text given to you as a string and I want this text to be added to the list of annotations."
  2. Add an annotation and the value to add is a Symbol Token. The intent here (I think) is "here is the symbol token, trust me I know what I am doing and how symbol tables work".

This leads to 2 methods/function one for each case.

The first case is the one we expect to be used more commonly, take this string and add it to the list of annotations for the current element in the writer. The input is a string and therefore the code will take the string and create a new symbol token with a new SID in the symbol table

Using Ion Java's IonWriter#addTypeAnnotation and passing as argument "$1" the Ion Java implementation creates a new symbol token for the string "$1" and writes the annotation to the output as '$1':: ....

The second case, which requires clients to know what symbol tokens are and how they work in Ion, takes as input a symbol token which is used as is and will be resolved with the current symbol table.

My proposal at this point is to create the API for the 1st use case (here is a string with the text that I want added as an annotation to the list of annotation's of this value). The second use case I think we can address later.

matthewh-BQ commented 4 years ago

Closing draft PR as per discussion. SymbolTokens and SymbolTables will address the second case. The first case is already supported.