Filter Implementation #28

ani-sha · 2020-09-07T12:46:24Z

Initial Version - Addition of filtering capabilities #16, few cases are yet to be validated.

wtrocki · 2020-09-07T19:06:40Z

engine/src/main/java/io/graphqlcrud/SQLQueryBuilderVisitor.java

+
+    public Condition visitIntValue(Value v, org.jooq.Field left, String conditionName) {
+        Condition c = null;
+        if(conditionName.equals("eq"))


Amazing job! I see you have all arguments covered. Is that ready for verification?

Have to implement in and between for float and int types.

rareddy · 2020-09-08T17:31:54Z

engine/src/test/java/io/graphqlcrud/SQLDataFetcherTest.java

+                "  ) \"accounts\"\n" +
+                "from PUBLIC.CUSTOMER \"g0\"\n" +
+                "where (\n" +
+                "  (\n" +


The result of this is not correct. it should be like lastname = 'smith' and firstname='John' and ssn = 'cst01002' a or as enclosing does not mean the attributes inside are separated by or. or is always adjunct operation where you could have said

{ customers (filter: { LASTNAME: { eq: "Smith" }, FIRSTNAME: { eq: "John" }, or: { SSN: { eq: "CST01002" } } } }

which should result in (lastname = 'smith' and firstname='John' ) or ssn = 'cst01002'

overall what I am saying or should not have had any effect on the query above

Thanks for pointing out that, I will correct it.

We found ourselves looking at the semantics of or operator recently in Graphback

/cc @craicoverflow if you want to share your experience.

yeah, We checked out competitors (AppSync) and their query output match that by @rareddy above.

@ani-sha did you make sure the previous original test as is should have behaved the way I listed above, as the test is not wrong the results were

@ani-sha keep the test case as is from before, make sure the results were correct.

rareddy · 2020-09-08T17:33:28Z

engine/src/test/java/io/graphqlcrud/SQLDataFetcherTest.java

+                "from PUBLIC.CUSTOMER \"g0\"\n" +
+                "where (\n" +
+                "  \"g0\".\"LASTNAME\" like ('%' || replace(\n" +
+                "    replace(\n" +


is JOOQ doing this conversion? seems cryptic, most databases have startsWith and contains kind of functions.

Agree, it's JOOQ. I didn't understand it as well.

rareddy · 2020-09-08T17:36:19Z

engine/src/main/java/io/graphqlcrud/SQLQueryBuilderVisitor.java

@@ -200,7 +188,7 @@ public void startVisitRootObject(Field rootField, GraphQLFieldDefinition rootDef
    }

    @Override
-    public void endVisitRootObject(Field rootFeild, GraphQLFieldDefinition rootDefinition, GraphQLObjectType type) {
+    public void endVisitRootObject(Field rootField, GraphQLFieldDefinition rootDefinition, GraphQLObjectType type) {


I need to learn how to spell field, I make this mistake quite often :)

rareddy · 2020-09-09T13:11:29Z

engine/src/main/java/io/graphqlcrud/SQLQueryBuilderVisitor.java

@@ -271,67 +273,130 @@ public void visitArgument(Field field, GraphQLFieldDefinition definition, GraphQ

    public Condition visitStringValue(Value v, org.jooq.Field left, String conditionName) {


Change the parameter Value v' to String` and remove the class castings below. When you are calling this method you already know the type just cast once there. Same for others below too

rareddy · 2020-09-09T13:14:01Z

engine/src/main/java/io/graphqlcrud/SQLQueryBuilderVisitor.java

+        Condition c = null;
+        switch (conditionName) {
+            case "between":
+                if (v.get(0) instanceof FloatValue)


even they are single line if or for routines wrap them with curly { braces, makes it easier to read the code

rareddy · 2020-09-09T13:15:49Z

engine/src/main/java/io/graphqlcrud/SQLQueryBuilderVisitor.java

+                    for (Value value : v)
+                        list.add(((IntValue) value).getValue());
+                    c = left.in(list);
+                }


put in a default in here and throw a runtimeexception saying wrong condition

rareddy · 2020-09-09T13:16:47Z

engine/src/main/java/io/graphqlcrud/SQLQueryBuilderVisitor.java

+                break;
+            case "ge":
+                c = left.ge(((FloatValue) v).getValue());
+                break;


put in default here and throw a RuntimeException saying wrong condition same at all other similar places.

rareddy · 2020-09-09T13:19:20Z

engine/src/main/java/io/graphqlcrud/SQLQueryBuilderVisitor.java

-                        visitCondition(finalConditionName, condition);
-                    });
+                    for(Object object : field.getValue().getChildren()) {
+                        if(object instanceof ObjectField) {


can there be any other value than ObjectField here? if yes capture that in else, you can throw an exception that case

Apart from ObjectField, we don't need to create Condition for others we just simply continue.

rareddy

LGTM

ani-sha added 2 commits September 7, 2020 18:07

Filters Implementation

7af9f40

Fix Typos

05e5d07

machi1990 mentioned this pull request Sep 7, 2020

Enhancement based on GraphQL CRUD capabilities #16

Open

5 tasks

wtrocki reviewed Sep 7, 2020

View reviewed changes

rareddy reviewed Sep 8, 2020

View reviewed changes

rareddy reviewed Sep 9, 2020

View reviewed changes

Implement IN and BETWEEN operator

baf521e

ani-sha force-pushed the validate-filter branch from c815665 to baf521e Compare September 9, 2020 14:33

Modified nested OR-AND queries & corrected results

475ca7d

rareddy approved these changes Sep 11, 2020

View reviewed changes

rareddy merged commit 13fe624 into graphqlcrud:master Sep 11, 2020

ani-sha deleted the validate-filter branch September 14, 2020 13:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Filter Implementation #28

Filter Implementation #28

ani-sha commented Sep 7, 2020

wtrocki Sep 7, 2020

ani-sha Sep 8, 2020

rareddy Sep 8, 2020

ani-sha Sep 9, 2020

machi1990 Sep 9, 2020

craicoverflow Sep 9, 2020

rareddy Sep 9, 2020

rareddy Sep 9, 2020

rareddy Sep 8, 2020

ani-sha Sep 9, 2020

rareddy Sep 8, 2020

rareddy Sep 9, 2020

rareddy Sep 9, 2020

rareddy Sep 9, 2020

rareddy Sep 9, 2020

rareddy Sep 9, 2020

ani-sha Sep 9, 2020

rareddy left a comment

		@@ -271,67 +273,130 @@ public void visitArgument(Field field, GraphQLFieldDefinition definition, GraphQ

		public Condition visitStringValue(Value v, org.jooq.Field left, String conditionName) {

Filter Implementation #28

Filter Implementation #28

Conversation

ani-sha commented Sep 7, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rareddy left a comment

Choose a reason for hiding this comment