Improve finding entity and field nodes #12

kungwangtotvs · 2017-03-13T18:36:14Z

Here are changes that works for my gremlin implementation again our own database. Please help me review it. Thank you.

…java.lang.Number

twilmes · 2017-03-15T01:06:47Z

Thanks for the PR Kung. I'll take a look.

kungwangtotvs · 2017-03-15T01:39:49Z

pom.xml

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


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

kungwangtotvs · 2017-03-15T01:44:32Z

src/main/java/org/twilmes/sql/gremlin/processor/FieldMapVisitor.java

@@ -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) {


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

kungwangtotvs · 2017-03-15T01:45:06Z

src/main/java/org/twilmes/sql/gremlin/processor/GremlinCompiler.java

@@ -121,7 +121,7 @@ public String explain(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);


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.

kungwangtotvs · 2017-03-15T01:45:39Z

src/main/java/org/twilmes/sql/gremlin/processor/JoinQueryExecutor.java

@@ -185,8 +186,8 @@ public JoinQueryExecutor(final RelNode node,
                        }
                    }
                }
-                final Property<Object> property = result.get(tableId).
-                        property(tableIdToTableDefMap.get(tableId).getColumn(simpleFieldName.toLowerCase()).getPropertyName());
+                TableColumn tableColumn = tableIdToTableDefMap.get(tableId).getColumn(simpleFieldName.toLowerCase());


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

kungwangtotvs · 2017-03-15T01:46:07Z

src/main/java/org/twilmes/sql/gremlin/processor/TraversalVisitor.java

@@ -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)) {


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.

kungwangtotvs · 2017-03-15T01:46:22Z

src/main/java/org/twilmes/sql/gremlin/processor/TraversalVisitor.java

        } 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)) {


Same reason as above

kungwangtotvs · 2017-03-15T01:46:41Z

src/main/java/org/twilmes/sql/gremlin/rel/GremlinToEnumerableConverter.java

@@ -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) {


implementation changes from Calcite 1.5.0 to 1.6.0

kungwangtotvs · 2017-03-15T01:47:25Z

src/main/java/org/twilmes/sql/gremlin/schema/TableUtil.java

@@ -61,11 +61,23 @@ public static Object convertType(Object value, TableColumn column) {
            case "string":
                return value;
            case "integer":
-                return ((Number) value).intValue();
+                try {


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

twilmes · 2017-03-15T13:57:33Z

Would it be possible for you to provide the schema you're working with along with some queries?

kungwangtotvs · 2017-03-15T18:00:09Z

There are there tables involve in this query:
mdmcustomer, mdmticket and mdmopportunity.

the relationship is defined as:
mdmcustomer.mdmcompanycode <= mdmticket.mdmcompanycode
mdmcustomer.mdmcompanycode <= mdmopportunity.mdmcompanycode

here is the SQL query:
select c.mdmcompanycode, count(i.mdmcompanycode), count(o.mdmcompanycode)
from mdmcustomer as c join mdmticket as i on c.mdmcompanycode = i.mdmcompanycode
join mdmopportunity as o on o.mdmcompanycode = c.mdmcompanycode
group by c.mdmcompanycode

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")
.match(
__.as("c").both("has Opportunity").count().as("opportunities"),
__.as("c").both("has_ticket").count().as("tickets"))
.select("customers", "opportunities", "tickets")

But after SQL had been parsed, and creating traversal tree, it traverses from mdmticket to mdmopportunity.

AN-822 Unit tests and checkstyle updates

kungwangtotvs added 2 commits March 13, 2017 00:18

improve finding nodes for entities and fields during join

5f2f67d

fix java.lang.ClassCastException: java.lang.String cannot be cast to …

6ce97a9

…java.lang.Number

kungwangtotvs commented Mar 15, 2017

View reviewed changes

twilmes pushed a commit that referenced this pull request Nov 5, 2021

Merge pull request #12 from lyndonb-bq/lyndon/unit-test-updates

bced0f1

AN-822 Unit tests and checkstyle updates

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve finding entity and field nodes #12

Improve finding entity and field nodes #12

kungwangtotvs commented Mar 13, 2017

twilmes commented Mar 15, 2017

kungwangtotvs Mar 15, 2017

kungwangtotvs Mar 15, 2017 •

edited

Loading

kungwangtotvs Mar 15, 2017

kungwangtotvs Mar 15, 2017

kungwangtotvs Mar 15, 2017

kungwangtotvs Mar 15, 2017

kungwangtotvs Mar 15, 2017

kungwangtotvs Mar 15, 2017

twilmes commented Mar 15, 2017

kungwangtotvs commented Mar 15, 2017

Improve finding entity and field nodes #12

Are you sure you want to change the base?

Improve finding entity and field nodes #12

Conversation

kungwangtotvs commented Mar 13, 2017

twilmes commented Mar 15, 2017

kungwangtotvs Mar 15, 2017

Choose a reason for hiding this comment

kungwangtotvs Mar 15, 2017 • edited Loading

Choose a reason for hiding this comment

kungwangtotvs Mar 15, 2017

Choose a reason for hiding this comment

kungwangtotvs Mar 15, 2017

Choose a reason for hiding this comment

kungwangtotvs Mar 15, 2017

Choose a reason for hiding this comment

kungwangtotvs Mar 15, 2017

Choose a reason for hiding this comment

kungwangtotvs Mar 15, 2017

Choose a reason for hiding this comment

kungwangtotvs Mar 15, 2017

Choose a reason for hiding this comment

twilmes commented Mar 15, 2017

kungwangtotvs commented Mar 15, 2017

kungwangtotvs Mar 15, 2017 •

edited

Loading