[ 
https://issues.apache.org/jira/browse/TINKERPOP-848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17429641#comment-17429641
 ] 

ASF GitHub Bot commented on TINKERPOP-848:
------------------------------------------

amatiushkin commented on a change in pull request #1485:
URL: https://github.com/apache/tinkerpop/pull/1485#discussion_r730338605



##########
File path: 
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphml/GraphMLReader.java
##########
@@ -138,9 +143,11 @@ public void readGraph(final InputStream graphInputStream, 
final Graph graphToWri
                         case GraphMLTokens.DATA:
                             final String key = reader.getAttributeValue(null, 
GraphMLTokens.KEY);
                             final String dataAttributeName = keyIdMap.get(key);
+                            final String defaultValue = defaultValues.get(key);
 
                             if (dataAttributeName != null) {
-                                final String value = reader.getElementText();
+                                String elementValue =  reader.getElementText();
+                                final String value = elementValue.length() == 
0 && defaultValue.length() != 0 ? defaultValue : elementValue;

Review comment:
       I need to better understand the intention to move default value 
assignment under `XMLEvent.END_ELEMENT`.
   It would require to make an extra call to `typeCastValue(...)` for default 
values as well before making an assignment.  I also might need to create new 
loop or re-use stream step to map default values to a key. It also looks to me 
that value initialization and coercion is nicely located as part of  
`XMLEvent.START_ELEMENT` . DATA case.
   
   What's the benefit of decoupling default value processing from element 
properties construction?
   
    




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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


> Support default attribute values in GraphMLReader
> -------------------------------------------------
>
>                 Key: TINKERPOP-848
>                 URL: https://issues.apache.org/jira/browse/TINKERPOP-848
>             Project: TinkerPop
>          Issue Type: Improvement
>          Components: io
>    Affects Versions: 3.0.2-incubating
>            Reporter: Pavel Klinov
>            Priority: Trivial
>   Original Estimate: 2h
>  Remaining Estimate: 2h
>
> Looking at the code of GraphMLReader I see that it doesn't support default 
> values of attributes, which are allowed by the GraphML spec. This is a bit 
> annoying especially if the input defines default values for attributes which 
> are used for mandatory data, e.g. edge labels. 
> One small example is the sample graph at [1]. "d_e" is the label attribute 
> with a default value. There're <edge .. /> elements w/o body later in the 
> document and reading those will throw a "java.lang.IllegalArgumentException: 
> Label can not be null" exception (if the vendor considers edge labels 
> mandatory).
> I'd personaly squash both keyIdMap and keyTypesMap into a single String -> 
> AttrInfo map, where AttrInfo would contain information about the data 
> attribute name, type, and the default value.
> [1] http://www.eecs.wsu.edu/~yyao/DirectedStudyI/Datasets/AS/sample.graphml



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to