Improve finding entity and field nodes#12
Improve finding entity and field nodes#12kungwangtotvs wants to merge 2 commits intotwilmes:masterfrom totvslabs:improve_finding_entity_and_field_nodes
Conversation
|
Thanks for the PR Kung. I'll take a look. |
| <junit.version>4.12</junit.version> | ||
| <tinkerpop.version>3.1.0-incubating</tinkerpop.version> | ||
| <calcite.version>1.5.0</calcite.version> | ||
| <calcite.version>1.6.0</calcite.version> |
There was a problem hiding this comment.
If upgrade to 1.8.0 and newer, there will be a JDBC error during planner's parse phase
| List<String> fieldNames = join.getRowType().getFieldNames(); | ||
| final String chosenField = fieldNames.get(fieldIndex); | ||
| return fieldMap.get(join).get(chosenField); | ||
| } else if (node instanceof EnumerableCalc) { |
There was a problem hiding this comment.
In my case, I always get this object as instance of EnumerableCalc instead of EnumerableJoin. By look deeper into the inputs, I will be able to get the converter out of these fields
| new RelWalker(root, fieldMapper); | ||
| final TraversalVisitor traversalVisitor = new TraversalVisitor(graph.traversal(), | ||
| scanMap, fieldMapper.getFieldMap()); | ||
| scanMap, fieldMapper.getFieldMap(), schemaConfig); |
There was a problem hiding this comment.
When doing 3 tables or more table joins, I see parser gives path that are joining two tables that has no relationship described in SchemaConfig, why? I think it should be problem is Calcite, here I was thinking of providing schema to checking in TraversalVisitor, but was not able to find left and right gremlin table in any of these nodes, so this will be a TODO item to check why it join two tables that has no relationship defined.
| } | ||
| final Property<Object> property = result.get(tableId). | ||
| property(tableIdToTableDefMap.get(tableId).getColumn(simpleFieldName.toLowerCase()).getPropertyName()); | ||
| TableColumn tableColumn = tableIdToTableDefMap.get(tableId).getColumn(simpleFieldName.toLowerCase()); |
There was a problem hiding this comment.
Sometimes I got null tableColumns here, so original statement will throw NPE in this case.
|
|
||
| if(!(left instanceof GremlinToEnumerableConverter)) { | ||
| left = fieldMap.get(left).get(leftAliasedColumn); | ||
| if (fieldMap.get(left) != null && fieldMap.get(left).containsKey(leftAliasedColumn)) { |
There was a problem hiding this comment.
In my own Gremlin implementation, I found left is always null when doing this simple get(leftAliasedColumn), so I have to look deeper to its joins to see what was involved in join and register them in tableTraversalMap, otherwise, later when doing get tableId, null will return and throw bad reference exception.
| } | ||
| if(!(right instanceof GremlinToEnumerableConverter)) { | ||
| right = fieldMap.get(join).get(rightAliasedColumn); | ||
| if (fieldMap.get(join) != null && fieldMap.get(join).containsKey(rightAliasedColumn)) { |
| @Override | ||
| public RelOptCost computeSelfCost(RelOptPlanner planner) { | ||
| return super.computeSelfCost(planner).multiplyBy(.1); | ||
| public RelOptCost computeSelfCost(RelOptPlanner planner, RelMetadataQuery mq) { |
There was a problem hiding this comment.
implementation changes from Calcite 1.5.0 to 1.6.0
| return value; | ||
| case "integer": | ||
| return ((Number) value).intValue(); | ||
| try { |
There was a problem hiding this comment.
I wonder if this is correct way of converting these types, but it works for me so far, same to the rest of changes
|
Would it be possible for you to provide the schema you're working with along with some queries? |
|
There are there tables involve in this query: the relationship is defined as: here is the SQL query: Although I did see where I can see the gremlin generated, but I would imaging it should be something like this: g.V().hasLabel("mdmcustomer").as("customers") But after SQL had been parsed, and creating traversal tree, it traverses from mdmticket to mdmopportunity. |
AN-822 Unit tests and checkstyle updates
2 participants
Here are changes that works for my gremlin implementation again our own database. Please help me review it. Thank you.