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.

Sign up for GitHub

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

Jump to bottom

Improve finding entity and field nodes #12

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Improve finding entity and field nodes #12

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5f2f67d
improve finding nodes for entities and fields during join
kungwangtotvs Mar 13, 2017
6ce97a9
fix java.lang.ClassCastException: java.lang.String cannot be cast to …
kungwangtotvs Mar 13, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<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>
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If upgrade to 1.8.0 and newer, there will be a JDBC error during planner's parse phase

</properties>

<dependencies>
Expand Down
15 changes: 15 additions & 0 deletions src/main/java/org/twilmes/sql/gremlin/processor/FieldMapVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.twilmes.sql.gremlin.processor;

import org.apache.calcite.adapter.enumerable.EnumerableCalc;
import org.twilmes.sql.gremlin.rel.GremlinToEnumerableConverter;
import org.apache.calcite.adapter.enumerable.EnumerableJoin;
import org.apache.calcite.rel.RelNode;
Expand Down Expand Up @@ -95,6 +96,20 @@ private GremlinToEnumerableConverter getConverter(int fieldIndex, String field,
List<String> fieldNames = join.getRowType().getFieldNames();
final String chosenField = fieldNames.get(fieldIndex);
return fieldMap.get(join).get(chosenField);
} else if (node instanceof EnumerableCalc) {
Copy link
Author

@kungwangtotvs kungwangtotvs Mar 15, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

final EnumerableCalc calc = (EnumerableCalc) node;
for (RelNode relNode : calc.getInputs()) {
if (relNode instanceof EnumerableJoin) {
return getConverter(fieldIndex, field, relNode);
} else {
try {
GremlinToEnumerableConverter converter = (GremlinToEnumerableConverter) relNode;
return converter;
} catch (ClassCastException e) {

}
}
}
}
return null;
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/twilmes/sql/gremlin/processor/GremlinCompiler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public List<Object> execute(String sql) {
final FieldMapVisitor fieldMapper = new FieldMapVisitor();
new RelWalker(root, fieldMapper);
final TraversalVisitor traversalVisitor = new TraversalVisitor(graph.traversal(),
scanMap, fieldMapper.getFieldMap());
scanMap, fieldMapper.getFieldMap(), schemaConfig);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

new RelWalker(root, traversalVisitor);

traversal = TraversalBuilder.buildMatch(graph.traversal(), traversalVisitor.getTableTraversalMap(),
Expand Down
9 changes: 5 additions & 4 deletions src/main/java/org/twilmes/sql/gremlin/processor/JoinQueryExecutor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.twilmes.sql.gremlin.rel.GremlinToEnumerableConverter;
import org.twilmes.sql.gremlin.rel.GremlinTraversalScan;
import org.twilmes.sql.gremlin.rel.GremlinTraversalToEnumerableRelConverter;
import org.twilmes.sql.gremlin.schema.TableColumn;
import org.twilmes.sql.gremlin.schema.TableDef;
import org.twilmes.sql.gremlin.schema.TableUtil;
import org.apache.calcite.adapter.enumerable.EnumerableInterpretable;
Expand Down Expand Up @@ -153,8 +154,8 @@ public List<Object> run() {
}

private List<Object> project(Map<String, String> fieldToTableMap, List<String> fields,
List<Map<String, ? extends Element>> results,
Map<String, TableDef> tableIdToTableDefMap) {
List<Map<String, ? extends Element>> results,
Map<String, TableDef> tableIdToTableDefMap) {
final List<Object> rows = new ArrayList<>(results.size());
Map<String, String> labelTableIdMap = new HashMap<>();
for (Map.Entry<String, TableDef> entry : tableIdToTableDefMap.entrySet()) {
Expand Down Expand Up @@ -185,8 +186,8 @@ private List<Object> project(Map<String, String> fieldToTableMap, List<String> f
}
}
}
final Property<Object> property = result.get(tableId).
property(tableIdToTableDefMap.get(tableId).getColumn(simpleFieldName.toLowerCase()).getPropertyName());
TableColumn tableColumn = tableIdToTableDefMap.get(tableId).getColumn(simpleFieldName.toLowerCase());
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes I got null tableColumns here, so original statement will throw NPE in this case.

final Property<Object> property = tableColumn == null ? Property.empty() : result.get(tableId).property(tableColumn.getPropertyName());
if(!(property instanceof EmptyProperty || property instanceof EmptyVertexProperty)) {
if(result.get(tableId).label().equals(tableIdToTableDefMap.get(tableId).label)) {
val = property.value();
Expand Down
37 changes: 34 additions & 3 deletions src/main/java/org/twilmes/sql/gremlin/processor/TraversalVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.apache.calcite.rel.RelNode;
import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal;
import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
import org.twilmes.sql.gremlin.schema.SchemaConfig;

import java.util.ArrayList;
import java.util.HashMap;
Expand All @@ -38,6 +39,7 @@ public class TraversalVisitor implements RelVisitor {

private final Map<GremlinToEnumerableConverter, List<RelNode>> scanMap;
private final Map<EnumerableJoin, Map<String, GremlinToEnumerableConverter>> fieldMap;
private final SchemaConfig schemaConfig; // may use to check if join is valid
private Integer id = 0;
private Map<GremlinToEnumerableConverter, String> tableIdMap = new HashMap<>();
private Map<String, GremlinToEnumerableConverter> tableIdConverterMap = new HashMap<>();
Expand All @@ -50,9 +52,11 @@ public class TraversalVisitor implements RelVisitor {

public TraversalVisitor(GraphTraversalSource traversalSource,
Map<GremlinToEnumerableConverter, List<RelNode>> scanMap,
Map<EnumerableJoin, Map<String, GremlinToEnumerableConverter>> fieldMap) {
Map<EnumerableJoin, Map<String, GremlinToEnumerableConverter>> fieldMap,
SchemaConfig schemaConfig) {
this.scanMap = scanMap;
this.fieldMap = fieldMap;
this.schemaConfig = schemaConfig;
}

public List<Pair<String, String>> getJoinPairs() { return joinPairs; }
Expand All @@ -71,6 +75,9 @@ public void visit(RelNode node) {
RelNode left = join.getLeft();
RelNode right = join.getRight();

// TODO: should we check schema to see if the join is valid?
// found some cases the join happens to two tables that has no relationship

final Integer leftKeyId = join.getLeftKeys().get(0);
final Integer rightKeyId = join.getRightKeys().get(0);

Expand All @@ -82,15 +89,39 @@ public void visit(RelNode node) {
final String rightAliasedColumn = rightJoinFields.get(rightKeyId);

if(!(left instanceof GremlinToEnumerableConverter)) {
left = fieldMap.get(left).get(leftAliasedColumn);
if (fieldMap.get(left) != null && fieldMap.get(left).containsKey(leftAliasedColumn)) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

left = fieldMap.get(left).get(leftAliasedColumn);
} else {
Map<String, GremlinToEnumerableConverter> leftMap = fieldMap.get(left);
if (leftMap == null) {
for(EnumerableJoin ej : fieldMap.keySet()) {
if (ej.getLeft().equals(left)) {
left = fieldMap.get(ej).get(leftAliasedColumn);
List<RelNode> scan = scanMap.get(left);
GraphTraversal leftTraversal = TraversalBuilder.toTraversal(scan);
traversals.add(leftTraversal);
tableTraversalMap.put(getTableId((GremlinToEnumerableConverter) left), leftTraversal);
break;
}
}
} else {
left = leftMap.get(leftAliasedColumn);
}
}
} else {
List<RelNode> scan = scanMap.get(left);
GraphTraversal leftTraversal = TraversalBuilder.toTraversal(scan);
traversals.add(leftTraversal);
tableTraversalMap.put(getTableId((GremlinToEnumerableConverter) left), leftTraversal);
}
if(!(right instanceof GremlinToEnumerableConverter)) {
right = fieldMap.get(join).get(rightAliasedColumn);
if (fieldMap.get(join) != null && fieldMap.get(join).containsKey(rightAliasedColumn)) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same reason as above

right = fieldMap.get(join).get(rightAliasedColumn);
List<RelNode> scan = scanMap.get(right);
GraphTraversal rightTraversal = TraversalBuilder.toTraversal(scan);
traversals.add(rightTraversal);
tableTraversalMap.put(getTableId((GremlinToEnumerableConverter) right), rightTraversal);
}
} else {
List<RelNode> scan = scanMap.get(right);
GraphTraversal rightTraversal = TraversalBuilder.toTraversal(scan);
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/org/twilmes/sql/gremlin/rel/GremlinToEnumerableConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.apache.calcite.plan.*;
import org.apache.calcite.rel.RelNode;
import org.apache.calcite.rel.convert.ConverterImpl;
import org.apache.calcite.rel.metadata.RelMetadataQuery;

import java.util.List;

Expand All @@ -50,8 +51,8 @@ public RelNode copy(RelTraitSet traitSet, List<RelNode> inputs) {
}

@Override
public RelOptCost computeSelfCost(RelOptPlanner planner) {
return super.computeSelfCost(planner).multiplyBy(.1);
public RelOptCost computeSelfCost(RelOptPlanner planner, RelMetadataQuery mq) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

implementation changes from Calcite 1.5.0 to 1.6.0

return super.computeSelfCost(planner, mq).multiplyBy(.1);
}

@Override
Expand Down
18 changes: 15 additions & 3 deletions src/main/java/org/twilmes/sql/gremlin/schema/TableUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,23 @@ public static Object convertType(Object value, TableColumn column) {
case "string":
return value;
case "integer":
return ((Number) value).intValue();
try {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this is correct way of converting these types, but it works for me so far, same to the rest of changes

return Integer.valueOf(String.valueOf(value));
} catch ( NumberFormatException e ) {
return value;
}
case "long":
return ((Number) value).longValue();
try {
return Long.valueOf(String.valueOf(value));
} catch ( NumberFormatException e ) {
return value;
}
case "double":
return ((Number) value).doubleValue();
try {
return Double.valueOf(String.valueOf(value));
} catch ( NumberFormatException e ) {
return value;
}
case "boolean":
if(value instanceof Number) {
if(value.equals(0)) {
Expand Down