You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Madhan Neethiraj <ma...@apache.org> on 2017/07/19 18:40:11 UTC
Review Request 60980: ATLAS-1961: Basic search improvement in use of
Solr index for attribute filtering (# 3)
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60980/
-----------------------------------------------------------
Review request for atlas.
Bugs: ATLAS-1961
https://issues.apache.org/jira/browse/ATLAS-1961
Repository: atlas
Description
-------
- fixed NPE when search includes a free-text query & a classification that has large number of sub-types
- fixed entity search in Solr to include state filter
Diffs
-----
repository/src/main/java/org/apache/atlas/discovery/ClassificationSearchProcessor.java 77b2c7c2
repository/src/main/java/org/apache/atlas/discovery/EntitySearchProcessor.java 50376ef1
repository/src/main/java/org/apache/atlas/discovery/FullTextSearchProcessor.java 83368c2b
repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java 596b43bc
Diff: https://reviews.apache.org/r/60980/diff/1/
Testing
-------
- validated that the fix addressed the error cases listed above
Thanks,
Madhan Neethiraj
Re: Review Request 60980: ATLAS-1961: Basic search improvement in use
of Solr index for attribute filtering (# 3)
Posted by Madhan Neethiraj <ma...@apache.org>.
> On July 19, 2017, 7:42 p.m., Apoorv Naik wrote:
> > repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java
> > Line 181 (original), 181 (patched)
> > <https://reviews.apache.org/r/60980/diff/1/?file=1779714#file1779714line181>
> >
> > Do it make more sense to pass a boolean flag here to control the addition of the state to the query ?
> >
> > EntityProcessor will pass true whereas ClassificationProcessor will pass false
I choose to read from context, as it is already available in SearchProcessor - avoid the need to pass additional parameter.
- Madhan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60980/#review180967
-----------------------------------------------------------
On July 19, 2017, 9:59 p.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60980/
> -----------------------------------------------------------
>
> (Updated July 19, 2017, 9:59 p.m.)
>
>
> Review request for atlas.
>
>
> Bugs: ATLAS-1961
> https://issues.apache.org/jira/browse/ATLAS-1961
>
>
> Repository: atlas
>
>
> Description
> -------
>
> - fixed NPE when search includes a free-text query & a classification that has large number of sub-types
> - fixed entity search in Solr to include state filter
>
>
> Diffs
> -----
>
> repository/src/main/java/org/apache/atlas/discovery/ClassificationSearchProcessor.java 77b2c7c2
> repository/src/main/java/org/apache/atlas/discovery/EntitySearchProcessor.java 50376ef1
> repository/src/main/java/org/apache/atlas/discovery/FullTextSearchProcessor.java 83368c2b
> repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java 596b43bc
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java cd9a47ad
>
>
> Diff: https://reviews.apache.org/r/60980/diff/2/
>
>
> Testing
> -------
>
> - validated that the fix addressed the error cases listed above
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
Re: Review Request 60980: ATLAS-1961: Basic search improvement in use
of Solr index for attribute filtering (# 3)
Posted by Apoorv Naik <na...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60980/#review180967
-----------------------------------------------------------
repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java
Line 181 (original), 181 (patched)
<https://reviews.apache.org/r/60980/#comment256348>
Do it make more sense to pass a boolean flag here to control the addition of the state to the query ?
EntityProcessor will pass true whereas ClassificationProcessor will pass false
- Apoorv Naik
On July 19, 2017, 6:40 p.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60980/
> -----------------------------------------------------------
>
> (Updated July 19, 2017, 6:40 p.m.)
>
>
> Review request for atlas.
>
>
> Bugs: ATLAS-1961
> https://issues.apache.org/jira/browse/ATLAS-1961
>
>
> Repository: atlas
>
>
> Description
> -------
>
> - fixed NPE when search includes a free-text query & a classification that has large number of sub-types
> - fixed entity search in Solr to include state filter
>
>
> Diffs
> -----
>
> repository/src/main/java/org/apache/atlas/discovery/ClassificationSearchProcessor.java 77b2c7c2
> repository/src/main/java/org/apache/atlas/discovery/EntitySearchProcessor.java 50376ef1
> repository/src/main/java/org/apache/atlas/discovery/FullTextSearchProcessor.java 83368c2b
> repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java 596b43bc
>
>
> Diff: https://reviews.apache.org/r/60980/diff/1/
>
>
> Testing
> -------
>
> - validated that the fix addressed the error cases listed above
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
Re: Review Request 60980: ATLAS-1961: Basic search improvement in use
of Solr index for attribute filtering (# 3)
Posted by Apoorv Naik <na...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60980/#review180964
-----------------------------------------------------------
Ship it!
Ship It!
- Apoorv Naik
On July 19, 2017, 6:40 p.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60980/
> -----------------------------------------------------------
>
> (Updated July 19, 2017, 6:40 p.m.)
>
>
> Review request for atlas.
>
>
> Bugs: ATLAS-1961
> https://issues.apache.org/jira/browse/ATLAS-1961
>
>
> Repository: atlas
>
>
> Description
> -------
>
> - fixed NPE when search includes a free-text query & a classification that has large number of sub-types
> - fixed entity search in Solr to include state filter
>
>
> Diffs
> -----
>
> repository/src/main/java/org/apache/atlas/discovery/ClassificationSearchProcessor.java 77b2c7c2
> repository/src/main/java/org/apache/atlas/discovery/EntitySearchProcessor.java 50376ef1
> repository/src/main/java/org/apache/atlas/discovery/FullTextSearchProcessor.java 83368c2b
> repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java 596b43bc
>
>
> Diff: https://reviews.apache.org/r/60980/diff/1/
>
>
> Testing
> -------
>
> - validated that the fix addressed the error cases listed above
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
Re: Review Request 60980: ATLAS-1961: Basic search improvement in use
of Solr index for attribute filtering (# 3)
Posted by Apoorv Naik <na...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60980/#review181000
-----------------------------------------------------------
Ship it!
Ship It!
- Apoorv Naik
On July 19, 2017, 9:59 p.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60980/
> -----------------------------------------------------------
>
> (Updated July 19, 2017, 9:59 p.m.)
>
>
> Review request for atlas.
>
>
> Bugs: ATLAS-1961
> https://issues.apache.org/jira/browse/ATLAS-1961
>
>
> Repository: atlas
>
>
> Description
> -------
>
> - fixed NPE when search includes a free-text query & a classification that has large number of sub-types
> - fixed entity search in Solr to include state filter
>
>
> Diffs
> -----
>
> repository/src/main/java/org/apache/atlas/discovery/ClassificationSearchProcessor.java 77b2c7c2
> repository/src/main/java/org/apache/atlas/discovery/EntitySearchProcessor.java 50376ef1
> repository/src/main/java/org/apache/atlas/discovery/FullTextSearchProcessor.java 83368c2b
> repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java 596b43bc
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java cd9a47ad
>
>
> Diff: https://reviews.apache.org/r/60980/diff/2/
>
>
> Testing
> -------
>
> - validated that the fix addressed the error cases listed above
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
Re: Review Request 60980: ATLAS-1961: Basic search improvement in use
of Solr index for attribute filtering (# 3)
Posted by Madhan Neethiraj <ma...@apache.org>.
> On July 21, 2017, 3:12 a.m., Apoorv Naik wrote:
> > repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java
> > Line 56 (original), 56 (patched)
> > <https://reviews.apache.org/r/60980/diff/3-4/?file=1780199#file1780199line56>
> >
> > Should we add all solr reserved characters here ?
> >
> > + - && || ! ( ) { } [ ] ^ " ~ * ? : /
> >
> > Otherwise overall it looks good.
For these special characters (or keywords - && ||), Solr recommends to escape with a backslash. However, we use double-quotes when value includes one of "@/ " - which serves well for our searches. This needs to be investigated further. I would suggest we take this up with Solr experts (mailing lists?).
- Madhan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60980/#review181094
-----------------------------------------------------------
On July 21, 2017, 1:39 a.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60980/
> -----------------------------------------------------------
>
> (Updated July 21, 2017, 1:39 a.m.)
>
>
> Review request for atlas.
>
>
> Bugs: ATLAS-1961
> https://issues.apache.org/jira/browse/ATLAS-1961
>
>
> Repository: atlas
>
>
> Description
> -------
>
> - fixed NPE when search includes a free-text query & a classification that has large number of sub-types
> - fixed entity search in Solr to include state filter
>
>
> Diffs
> -----
>
> repository/src/main/java/org/apache/atlas/discovery/EntitySearchProcessor.java a3525c93
> repository/src/main/java/org/apache/atlas/discovery/FullTextSearchProcessor.java 1b19a0e2
> repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java 7950127d
>
>
> Diff: https://reviews.apache.org/r/60980/diff/4/
>
>
> Testing
> -------
>
> - validated that the fix addressed the error cases listed above
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
Re: Review Request 60980: ATLAS-1961: Basic search improvement in use
of Solr index for attribute filtering (# 3)
Posted by Apoorv Naik <na...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60980/#review181094
-----------------------------------------------------------
repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java
Line 56 (original), 56 (patched)
<https://reviews.apache.org/r/60980/#comment256520>
Should we add all solr reserved characters here ?
+ - && || ! ( ) { } [ ] ^ " ~ * ? : /
Otherwise overall it looks good.
- Apoorv Naik
On July 21, 2017, 1:39 a.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60980/
> -----------------------------------------------------------
>
> (Updated July 21, 2017, 1:39 a.m.)
>
>
> Review request for atlas.
>
>
> Bugs: ATLAS-1961
> https://issues.apache.org/jira/browse/ATLAS-1961
>
>
> Repository: atlas
>
>
> Description
> -------
>
> - fixed NPE when search includes a free-text query & a classification that has large number of sub-types
> - fixed entity search in Solr to include state filter
>
>
> Diffs
> -----
>
> repository/src/main/java/org/apache/atlas/discovery/EntitySearchProcessor.java a3525c93
> repository/src/main/java/org/apache/atlas/discovery/FullTextSearchProcessor.java 1b19a0e2
> repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java 7950127d
>
>
> Diff: https://reviews.apache.org/r/60980/diff/4/
>
>
> Testing
> -------
>
> - validated that the fix addressed the error cases listed above
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
Re: Review Request 60980: ATLAS-1961: Basic search improvement in use
of Solr index for attribute filtering (# 3)
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60980/
-----------------------------------------------------------
(Updated July 21, 2017, 1:39 a.m.)
Review request for atlas.
Changes
-------
updates to handle spaces in tag-names, while searcing in Solr index
Bugs: ATLAS-1961
https://issues.apache.org/jira/browse/ATLAS-1961
Repository: atlas
Description
-------
- fixed NPE when search includes a free-text query & a classification that has large number of sub-types
- fixed entity search in Solr to include state filter
Diffs (updated)
-----
repository/src/main/java/org/apache/atlas/discovery/EntitySearchProcessor.java a3525c93
repository/src/main/java/org/apache/atlas/discovery/FullTextSearchProcessor.java 1b19a0e2
repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java 7950127d
Diff: https://reviews.apache.org/r/60980/diff/4/
Changes: https://reviews.apache.org/r/60980/diff/3-4/
Testing
-------
- validated that the fix addressed the error cases listed above
Thanks,
Madhan Neethiraj
Re: Review Request 60980: ATLAS-1961: Basic search improvement in use
of Solr index for attribute filtering (# 3)
Posted by Apoorv Naik <na...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60980/#review181076
-----------------------------------------------------------
Ship it!
Looks good to me
- Apoorv Naik
On July 20, 2017, 8:31 a.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60980/
> -----------------------------------------------------------
>
> (Updated July 20, 2017, 8:31 a.m.)
>
>
> Review request for atlas.
>
>
> Bugs: ATLAS-1961
> https://issues.apache.org/jira/browse/ATLAS-1961
>
>
> Repository: atlas
>
>
> Description
> -------
>
> - fixed NPE when search includes a free-text query & a classification that has large number of sub-types
> - fixed entity search in Solr to include state filter
>
>
> Diffs
> -----
>
> repository/src/main/java/org/apache/atlas/discovery/ClassificationSearchProcessor.java b6e0de5c
> repository/src/main/java/org/apache/atlas/discovery/EntitySearchProcessor.java 6f629eb4
> repository/src/main/java/org/apache/atlas/discovery/FullTextSearchProcessor.java 22d91e04
> repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java 2e75dfeb
>
>
> Diff: https://reviews.apache.org/r/60980/diff/3/
>
>
> Testing
> -------
>
> - validated that the fix addressed the error cases listed above
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
Re: Review Request 60980: ATLAS-1961: Basic search improvement in use
of Solr index for attribute filtering (# 3)
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60980/
-----------------------------------------------------------
(Updated July 20, 2017, 8:31 a.m.)
Review request for atlas.
Changes
-------
updated to choose appropriate start-offset, to handle 'state filter' (to exclude deleted entities)
Bugs: ATLAS-1961
https://issues.apache.org/jira/browse/ATLAS-1961
Repository: atlas
Description
-------
- fixed NPE when search includes a free-text query & a classification that has large number of sub-types
- fixed entity search in Solr to include state filter
Diffs (updated)
-----
repository/src/main/java/org/apache/atlas/discovery/ClassificationSearchProcessor.java b6e0de5c
repository/src/main/java/org/apache/atlas/discovery/EntitySearchProcessor.java 6f629eb4
repository/src/main/java/org/apache/atlas/discovery/FullTextSearchProcessor.java 22d91e04
repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java 2e75dfeb
Diff: https://reviews.apache.org/r/60980/diff/3/
Changes: https://reviews.apache.org/r/60980/diff/2-3/
Testing
-------
- validated that the fix addressed the error cases listed above
Thanks,
Madhan Neethiraj
Re: Review Request 60980: ATLAS-1961: Basic search improvement in use
of Solr index for attribute filtering (# 3)
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60980/
-----------------------------------------------------------
(Updated July 19, 2017, 9:59 p.m.)
Review request for atlas.
Changes
-------
Updates to reuse temp lists during search iterations
Bugs: ATLAS-1961
https://issues.apache.org/jira/browse/ATLAS-1961
Repository: atlas
Description
-------
- fixed NPE when search includes a free-text query & a classification that has large number of sub-types
- fixed entity search in Solr to include state filter
Diffs (updated)
-----
repository/src/main/java/org/apache/atlas/discovery/ClassificationSearchProcessor.java 77b2c7c2
repository/src/main/java/org/apache/atlas/discovery/EntitySearchProcessor.java 50376ef1
repository/src/main/java/org/apache/atlas/discovery/FullTextSearchProcessor.java 83368c2b
repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java 596b43bc
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java cd9a47ad
Diff: https://reviews.apache.org/r/60980/diff/2/
Changes: https://reviews.apache.org/r/60980/diff/1-2/
Testing
-------
- validated that the fix addressed the error cases listed above
Thanks,
Madhan Neethiraj