Issue #29: Query Rewriter Interface and Tests by kishore-narendran · Pull Request #75 · apache/texera

kishore-narendran · 2016-04-25T20:36:29Z

Started off with the creation of an interface for the QueryRewriter.

Added a class for the QueryRewriter that implements IOperator
Added a class FuzzyTokenizer that is the core class, that will perform the tokenizations
Added a new field in the TestConstants for testing purposes
Added a test class for QueryRewriter

I am a little puzzled, about how the QueryRewriter will return a list of rewritten queries as strings, when the expected is an ITuple. How do I proceed @chenlica @JavierJia ?

@shiladityasen

Merging master to yoda-query-rewriter after critical changes

kishore-narendran · 2016-04-25T20:38:01Z

Also, the checks are failing because of the SpanTupleTest.

chenlica · 2016-04-25T21:14:54Z

As the first step, you can include the rewritten queries as a list of strings.

If the test cases passed on the master, then it's your responsibility to look into the failed test cases in your branch.

Let's talk in person after the lecture today.

kishore-narendran · 2016-04-25T22:07:44Z

I fixed the failure professor. So, in that scenario is it okay if the QueryRewriter class doesn't extend IOperator for now?

chenlica · 2016-04-26T01:01:52Z

Per our discussion, we want all major features to be incorporated as an operator (by implementing the IOperator interface) in order to be a "good citizen" in the system to be used by other operators in a plan.

kishore-narendran · 2016-04-26T02:40:34Z

I have made changes such that our QueryRewriter implements the IOperator interface as per our discussion.

chenlica · 2016-04-26T04:47:09Z

Please first fix the "All checks have failed" problem.

kishore-narendran · 2016-04-26T06:49:58Z

@chenlica @JavierJia The interface and the tests are now complete.

chenlica · 2016-04-26T07:12:12Z

@sandeepreddy602 @rajesh9625 and @inkudo If you have time, can you review this PR?

JavierJia · 2016-04-26T07:24:22Z

+ */
+public class WordDictionary {
+
+    private static HashSet<String> wordDict;


why it is static?

Prof prefers Dictionary to Dict, could you change it to wordDictionary.

…ctionary; Added another test case

sandeepreddy602 · 2016-04-26T17:57:01Z

+	TEXT,
+
+	//A field that is the list of strings
+	STRING_LIST;


We need to use the generic list field here. Once our pull request #77 is merged into master, you can make these changes

Do you mean a list with a template and it can be any data type? @sandeepreddy602

@kishore-narendran ..Yes

sandeepreddy602 · 2016-04-27T04:55:00Z

@kishore-narendran .. Merged the pull request #77 to master. Please update this branch and make the changes accordingly.

kishore-narendran · 2016-04-27T20:23:48Z

@sandeepreddy602, I have merged master into this branch. Pending your final review, this can be merged into master too. @chenlica @JavierJia

sandeepreddy602 · 2016-04-27T21:15:11Z

@kishore-narendran Looks good to me.

chenlica · 2016-04-27T22:39:11Z

@kishore-narendran I will review it later today. After my approval, you can do the merge.

chenlica · 2016-04-28T04:53:16Z

+ * try the query rewriting.
+ *
+ * This includes query strings like - "newyorkcity", "horseshoe" and other
+ * strings which stem from misppelt, missing spaces in search queries


Fix "misppelt".

ss3n · 2016-04-29T02:46:40Z

@chenlica @JavierJia @sandeepreddy602 : We made the changes suggested after the last commit. Can you give our code another review to merge?

chenlica · 2016-04-29T04:39:48Z

+     */
+    @Override
+    public void close() throws Exception {
+        //Uncomment below line when FuzzyTokenizer has been created


Remove all "FuzzyTokenizer"-related code from this PR completely since it doesn't belong here. Add it in your next PR.

chenlica · 2016-04-29T04:47:54Z

@shiladityasen and @kishore-narendran I added more comments. Still the PR is NOT ready to merge.

General comments:

Remove all code NOT related to this PR (e.g., "FuzzyTokenizer"), even from the comments!
Be more careful about your comments;
This is your first major PR, and please be prepared for this type of rigorous reviews.

chenlica · 2016-04-29T06:44:08Z

+
+import java.util.List;
+
+public class ListField<T> implements IField {


Can you remind me the reason to have this class of list of fields?

This will be used, to return an ITuple that has a list of strings, that is the list of rewritten query strings that our operator sends.

chenlica · 2016-04-29T06:45:48Z

@kishore-narendran : I left a minor comment. After addressing it, you can go ahead to do the merge. Congratulations! :-)

chenlica · 2016-04-29T06:47:29Z

+ * Created by kishorenarendran on 25/04/16.
+ * QueryRewriter is an operator that converts a query search string which
+ * has faults in space placement to a list of meaningful search queries
+ * QueryRewriter internally uses the FuzzyTokenizer class to fulfill


Remove the sentence "QueryRewriter internally uses the FuzzyTokenizer class to fulfill its functionality" since it's related to the coming PR of implementation.

Done in 9bd7d7a professor!

kishore-narendran · 2016-04-29T06:56:06Z

@chenlica I have addressed the minor comment professor. Thank you! 😄
I am now merging.

ss3n and others added 5 commits April 25, 2016 11:05

Merge pull request #74 from TextDB/master

44583ff

Merging master to yoda-query-rewriter after critical changes

Added QueryRewriter and QueryParser classes

0e6aa59

Added new attribute to test constants

a44d90b

Class name refactoring, and input changed to search query string

3faaedd

Added FuzzyTokenizer class'

4b34848

Editing fields in SpanTupleTest.java to fix build

b40bbbc

kishore-narendran added 3 commits April 25, 2016 17:14

Reverted changes to TestConstant and SpanTupleTest'

d592dd6

Added StringListField

2180702

Updated interface to QueryRewriter class

352ca56

Added initial test for Query rewriter

dcab94d

shiladitya and others added 3 commits April 25, 2016 22:13

Attempting fix for Travis for Query Rewriter

f3d43ee

Fixing the build failure, minor refactors

b893b5c

Minor change to stubbed getParsedQueries function

b999cd9

kishore-narendran mentioned this pull request Apr 26, 2016

(Team 2) Design: Query Rewriter #29

Closed

JavierJia reviewed Apr 26, 2016
View reviewed changes

shiladitya added 2 commits April 26, 2016 00:44

Removed static type for wordDictionary; refactored wordDict to wordDi…

701e744

…ctionary; Added another test case

Commented some tests to pass Travis build

94f273d

sandeepreddy602 reviewed Apr 26, 2016
View reviewed changes

Merged branch 'master' into yoda-query-rewriter and fixed conflicts

9c275a6

kishore-narendran added the 2-query-rewritter label Apr 27, 2016

chenlica reviewed Apr 28, 2016
View reviewed changes

kishore-narendran added 2 commits April 28, 2016 00:11

Minor changes and removed implementation classes

9e39101

Added a few comments

c2ea67a

chenlica reviewed Apr 29, 2016
View reviewed changes

Removed all mentions of FuzzyTokenizer.java

5c76c68

chenlica reviewed Apr 29, 2016
View reviewed changes

Removed mention of FuzzyTokenizer.java

9bd7d7a

kishore-narendran merged commit cea37df into master Apr 29, 2016

kishore-narendran deleted the yoda-query-rewriter branch April 29, 2016 06:56


		import java.util.List;

		public class ListField<T> implements IField {

Conversation

kishore-narendran commented Apr 25, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kishore-narendran commented Apr 25, 2016

Uh oh!

chenlica commented Apr 25, 2016

Uh oh!

kishore-narendran commented Apr 25, 2016

Uh oh!

chenlica commented Apr 26, 2016

Uh oh!

kishore-narendran commented Apr 26, 2016

Uh oh!

chenlica commented Apr 26, 2016

Uh oh!

kishore-narendran commented Apr 26, 2016

Uh oh!

chenlica commented Apr 26, 2016

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!

sandeepreddy602 commented Apr 27, 2016

Uh oh!

kishore-narendran commented Apr 27, 2016

Uh oh!

sandeepreddy602 commented Apr 27, 2016

Uh oh!

chenlica commented Apr 27, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ss3n commented Apr 29, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chenlica commented Apr 29, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chenlica commented Apr 29, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kishore-narendran commented Apr 29, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

kishore-narendran commented Apr 25, 2016 •

edited

Loading