Added not reserved keyword. Works for both not in and not like by frahman5 · Pull Request #68 · filhodanuvem/gitql

frahman5 · 2018-10-12T19:12:25Z

The basic update I made is to add a "Not" field to the NodeIn and NodeLike types and update their assertion methods accordingly. The Not field acts as a flag telling the program whether to query with a normal In/Like or with a Not In/Not Like. If "Not" is set to true, then the assertion method returns the opposite of what it would return otherwise.

All the other changes just support the above structural update. I updated lexical to be able to process a "not" reserved keyword. And I updated the parser to be able to correctly read in the "not" and create the appropriate NodeIn and NodeLike objects. I also added some tests to parser_test.go to make sure that was all working.

I chose this route over the other route of creating entirely new NodeNotIn and NodeNotLike objects because:

it seems simpler, requiring less overall new code.
it feels more appropriate, since the 'not' keyword is not really asking the program to create a whole different kind of where condition, but is just modifying the upcoming not or like expression. And so it seems appropriate to reflect that in the code, using the inclusion of a 'not' keyword to modify existing NodeIn or NodeLike objects, rather than create new negative versions of them.
it would extend pretty easily if we were to add new reserved keywords, like between. In this model, if you wanted to support 'not between', you would just add a 'Not' field to the NodeBetween object and update the getWhereConds() functions appropriately.

Examples

A query with no where condition

// ♥ go run gitql.go cmd.go autocomplete.go "select author, message from commits limit 5'"
+---------------------+--------------------------------+
|       author        |            message             |
+---------------------+--------------------------------+
| frahman5            | Added not reserved keyword.    |
|                     | Functions for both not in and  |
|                     | not like                       |
+---------------------+--------------------------------+
| Claudson Oliveira   | Update supported go version on |
|                     | travis CI                      |
+---------------------+--------------------------------+
| Claudson Oliveira   | Release a minor release        |
+---------------------+--------------------------------+
| ia                  | all: gofmt (#65)               |
+---------------------+--------------------------------+
| Arumugam Jeganathan | Add 'like' functionality in    |
|                     | where clause (#64)             |
+---------------------+--------------------------------+

A query with a where condition that excludes several of the authors you'd otherwise see

// ♥ go run gitql.go cmd.go autocomplete.go "select author, message from commits where 'Claudson' not in author AND 'Arumugam' not in author limit 5'"
+---------------------+---------------------------------+
|       author        |             message             |
+---------------------+---------------------------------+
| frahman5            | Added not reserved keyword.     |
|                     | Functions for both not in and   |
|                     | not like                        |
+---------------------+---------------------------------+
| ia                  | all: gofmt (#65)                |
+---------------------+---------------------------------+
| Luiz Fernando Peres | Merge pull request #62 from     |
|                     | jsixface/feature/WhereNotEquals |
+---------------------+---------------------------------+
| Luiz Fernando Peres | Merge pull request #61 from     |
|                     | jsixface/bug/NilStarSelect      |
+---------------------+---------------------------------+
| Luiz Fernando Peres | Merge pull request #56 from     |
|                     | jsixface/bug/NilStarSelect      |
+---------------------+---------------------------------+

Using a not like to exclude any commits where the message begins with Add

// ♥ go run gitql.go cmd.go autocomplete.go "select author, message from commits where message not like '%Add' limit 5"
+---------------------+---------------------------------+
|       author        |             message             |
+---------------------+---------------------------------+
| Claudson Oliveira   | Update supported go version on  |
|                     | travis CI                       |
+---------------------+---------------------------------+
| Claudson Oliveira   | Release a minor release         |
+---------------------+---------------------------------+
| ia                  | all: gofmt (#65)                |
+---------------------+---------------------------------+
| Luiz Fernando Peres | Merge pull request #62 from     |
|                     | jsixface/feature/WhereNotEquals |
+---------------------+---------------------------------+
| Arumugam J          | register walk type              |
+---------------------+---------------------------------+

@luizperes :)

filhodanuvem

Awesome work @frahman5 🎉 .
I'm just waiting for @luizperes review and we can merge it.

filhodanuvem · 2018-10-12T20:10:51Z

 const T_ASC = 25
 const T_LIKE = 26
+const T_NOT = 27
+const T_NOT_OR_T_LIKE = 28


Did your Idea actually was T_IN_OR_T_LIKE, isn't? I'm not too comfortable seeing a token that is used just for error messages. I prefer to suggest just one of the options (T_NOT) and maybe we can discuss better in another thread how to improve the parser error to show more then one expected tokens. What do you think?

Yeah I wasn't too happy about that either. I didn't want to create an entirely different SyntaxError function though, so I squeezed in that token instead. I think you're right though. I'll change it to just suggest T_NOT for now, and we can leave the issue of leaving better parser errors for another day.

Just letting you know I pushed another commit with this fix :)

filhodanuvem · 2018-10-12T20:11:52Z

 	}
 }

+func TestWhereWithIn(t *testing.T) {


Wow, thanks a lot for that ❤️

filhodanuvem · 2018-10-12T20:12:36Z

+}
+
+func TestWhereWithNotIn(t *testing.T) {
+	New("Select message from commits where 'react' not in message")


Vue forever (?) 😆

filhodanuvem · 2018-10-12T21:36:54Z

@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>


Thank you for the change but you commit some IDE files :/ . Can you remove them?

filhodanuvem

Just because of the IDE files.

luizperes · 2018-10-12T21:47:26Z

 			T_IN:               "T_IN",
 			T_EOF:              "T_EOF",
 			T_ASC:              "T_ASC",
+			T_NOT: 				"T_NOT",


are these extra spaces only on github? otherwise, could you align them?

luizperes

LGTM (except for the extra non-needed files)

frahman5 · 2018-10-14T15:10:03Z

@luizperes @cloudson I'll take care of those IDE files and space issues by tomorrow

frahman5 · 2018-10-14T20:35:31Z

@luizperes @cloudson requested changes made. I've removed the IDE files and fixed the spacing on tokens. go :)

luizperes

Thank you so much for this PR @frahman5! It looks neat!

filhodanuvem · 2018-10-15T07:49:23Z

Thank you again 🤘

frahman5 · 2018-10-15T21:54:10Z

👍 😃

jsixface · 2018-10-18T17:14:38Z

I think this would close #49

filhodanuvem · 2018-10-18T19:27:34Z

Totally right @jsixface , thank you for the suggestion.

Added not reserved keyword. Functions for both not in and not like

2e98e26

filhodanuvem approved these changes Oct 12, 2018

View reviewed changes

removed all vestiges of T_NOT_OR_T_LIKE

78ba3e8

filhodanuvem reviewed Oct 12, 2018

View reviewed changes

filhodanuvem requested changes Oct 12, 2018

View reviewed changes

luizperes reviewed Oct 12, 2018

View reviewed changes

luizperes requested changes Oct 12, 2018

View reviewed changes

filhodanuvem changed the title ~~Added not reserved keyword. Works for both not in and not like.~~ Added not reserved keyword. Works for both not in and not like #49 Oct 12, 2018

filhodanuvem changed the title ~~Added not reserved keyword. Works for both not in and not like #49~~ Added not reserved keyword. Works for both not in and not like Oct 12, 2018

frahman5 added 6 commits October 14, 2018 16:31

Deleting IDE files (1)

4eb0cbe

Deleting IDE files (2)

dcb9122

Deleting IDE files (3)

824fedb

Deleting IDE files (4)

e9457dd

Deleting IDE files (5)

2006605

Fixing spaces on tokens.go

88581f6

filhodanuvem approved these changes Oct 14, 2018

View reviewed changes

luizperes approved these changes Oct 14, 2018

View reviewed changes

filhodanuvem merged commit a3f1a73 into filhodanuvem:develop Oct 15, 2018

Conversation

frahman5 commented Oct 12, 2018

Examples

Uh oh!

filhodanuvem left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

frahman5 Oct 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

filhodanuvem left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luizperes left a comment

Choose a reason for hiding this comment

Uh oh!

frahman5 commented Oct 14, 2018

Uh oh!

frahman5 commented Oct 14, 2018

Uh oh!

luizperes left a comment

Choose a reason for hiding this comment

Uh oh!

filhodanuvem commented Oct 15, 2018

Uh oh!

frahman5 commented Oct 15, 2018

Uh oh!

jsixface commented Oct 18, 2018

Uh oh!

filhodanuvem commented Oct 18, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

frahman5 Oct 12, 2018 •

edited

Loading