You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lens.apache.org by Srikanth Sundarrajan <sr...@hotmail.com> on 2016/09/21 14:14:08 UTC
Review Request 52119: LENS-1335 NullPointerException in
LensClient::getResults(QueryHandle query)
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52119/
-----------------------------------------------------------
Review request for lens.
Bugs: LENS-1335
https://issues.apache.org/jira/browse/LENS-1335
Repository: lens
Description
-------
NullPointerException in LensClient::getResults(QueryHandle query)
Diffs
-----
lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java e75fc0e
lens-client/src/main/java/org/apache/lens/client/LensClient.java 593cc08
Diff: https://reviews.apache.org/r/52119/diff/
Testing
-------
Modified tests to avoid using offending method.
Thanks,
Srikanth Sundarrajan
Re: Review Request 52119: LENS-1335 NullPointerException in
LensClient::getResults(QueryHandle query)
Posted by Srikanth Sundarrajan <sr...@hotmail.com>.
> On Sept. 22, 2016, 5:56 a.m., Amareshwari Sriramadasu wrote:
> > lens-client/src/main/java/org/apache/lens/client/LensClient.java, line 330
> > <https://reviews.apache.org/r/52119/diff/1/?file=1507243#file1507243line330>
> >
> > We would need an api to getResults for particular handle as well - to access it across clients, no? I see current caching may not work. But if not cached, client should fetch it from srver.
>
> Srikanth Sundarrajan wrote:
> Upon execution of a new query in lens client, a new statement object isn't created, so essentially, the statement object holds only LensQuery handle of last executed query. The current assumption in getLensStatement(queryHandle) will return query handle specific statement is broken. If there is merit in caching LensQuery objects of every query executed by a client/statement, then this requires a far deeper fix. I can change the scope of the issue and revise the patch accordingly, but we need to be convinced about the need to cache the LensQuery object. It seems unncessary to cache a LensQuery object particuarly if it is unfinished, as the state might change as the query makes progress. The caching can happen in the consuming application in such case given that the LensQuery for finished queries are immutable. Would like you hear your views before proceeding further.
>
> Rajat Khandelwal wrote:
> The query shouldn't be cached unless it's in a terminal state. I have incorporated this kind of caching in the python client.
Agreed. Caching unfinished LensQuery object doesn't seem like the right thing to do, will wait for @Amareshwari also to chime in (in case am missing something), and then we can move further on this.
- Srikanth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52119/#review149951
-----------------------------------------------------------
On Sept. 21, 2016, 2:14 p.m., Srikanth Sundarrajan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52119/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2016, 2:14 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1335
> https://issues.apache.org/jira/browse/LENS-1335
>
>
> Repository: lens
>
>
> Description
> -------
>
> NullPointerException in LensClient::getResults(QueryHandle query)
>
>
> Diffs
> -----
>
> lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java e75fc0e
> lens-client/src/main/java/org/apache/lens/client/LensClient.java 593cc08
>
> Diff: https://reviews.apache.org/r/52119/diff/
>
>
> Testing
> -------
>
> Modified tests to avoid using offending method.
>
>
> Thanks,
>
> Srikanth Sundarrajan
>
>
Re: Review Request 52119: LENS-1335 NullPointerException in
LensClient::getResults(QueryHandle query)
Posted by Srikanth Sundarrajan <sr...@hotmail.com>.
> On Sept. 22, 2016, 5:56 a.m., Amareshwari Sriramadasu wrote:
> > lens-client/src/main/java/org/apache/lens/client/LensClient.java, line 330
> > <https://reviews.apache.org/r/52119/diff/1/?file=1507243#file1507243line330>
> >
> > We would need an api to getResults for particular handle as well - to access it across clients, no? I see current caching may not work. But if not cached, client should fetch it from srver.
Upon execution of a new query in lens client, a new statement object isn't created, so essentially, the statement object holds only LensQuery handle of last executed query. The current assumption in getLensStatement(queryHandle) will return query handle specific statement is broken. If there is merit in caching LensQuery objects of every query executed by a client/statement, then this requires a far deeper fix. I can change the scope of the issue and revise the patch accordingly, but we need to be convinced about the need to cache the LensQuery object. It seems unncessary to cache a LensQuery object particuarly if it is unfinished, as the state might change as the query makes progress. The caching can happen in the consuming application in such case given that the LensQuery for finished queries are immutable. Would like you hear your views before proceeding further.
- Srikanth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52119/#review149951
-----------------------------------------------------------
On Sept. 21, 2016, 2:14 p.m., Srikanth Sundarrajan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52119/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2016, 2:14 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1335
> https://issues.apache.org/jira/browse/LENS-1335
>
>
> Repository: lens
>
>
> Description
> -------
>
> NullPointerException in LensClient::getResults(QueryHandle query)
>
>
> Diffs
> -----
>
> lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java e75fc0e
> lens-client/src/main/java/org/apache/lens/client/LensClient.java 593cc08
>
> Diff: https://reviews.apache.org/r/52119/diff/
>
>
> Testing
> -------
>
> Modified tests to avoid using offending method.
>
>
> Thanks,
>
> Srikanth Sundarrajan
>
>
Re: Review Request 52119: LENS-1335 NullPointerException in
LensClient::getResults(QueryHandle query)
Posted by Rajat Khandelwal <ra...@gmail.com>.
> On Sept. 22, 2016, 11:26 a.m., Amareshwari Sriramadasu wrote:
> > lens-client/src/main/java/org/apache/lens/client/LensClient.java, line 330
> > <https://reviews.apache.org/r/52119/diff/1/?file=1507243#file1507243line330>
> >
> > We would need an api to getResults for particular handle as well - to access it across clients, no? I see current caching may not work. But if not cached, client should fetch it from srver.
>
> Srikanth Sundarrajan wrote:
> Upon execution of a new query in lens client, a new statement object isn't created, so essentially, the statement object holds only LensQuery handle of last executed query. The current assumption in getLensStatement(queryHandle) will return query handle specific statement is broken. If there is merit in caching LensQuery objects of every query executed by a client/statement, then this requires a far deeper fix. I can change the scope of the issue and revise the patch accordingly, but we need to be convinced about the need to cache the LensQuery object. It seems unncessary to cache a LensQuery object particuarly if it is unfinished, as the state might change as the query makes progress. The caching can happen in the consuming application in such case given that the LensQuery for finished queries are immutable. Would like you hear your views before proceeding further.
The query shouldn't be cached unless it's in a terminal state. I have incorporated this kind of caching in the python client.
- Rajat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52119/#review149951
-----------------------------------------------------------
On Sept. 21, 2016, 7:44 p.m., Srikanth Sundarrajan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52119/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2016, 7:44 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1335
> https://issues.apache.org/jira/browse/LENS-1335
>
>
> Repository: lens
>
>
> Description
> -------
>
> NullPointerException in LensClient::getResults(QueryHandle query)
>
>
> Diffs
> -----
>
> lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java e75fc0e
> lens-client/src/main/java/org/apache/lens/client/LensClient.java 593cc08
>
> Diff: https://reviews.apache.org/r/52119/diff/
>
>
> Testing
> -------
>
> Modified tests to avoid using offending method.
>
>
> Thanks,
>
> Srikanth Sundarrajan
>
>
Re: Review Request 52119: LENS-1335 NullPointerException in
LensClient::getResults(QueryHandle query)
Posted by Srikanth Sundarrajan <sr...@hotmail.com>.
> On Sept. 22, 2016, 5:56 a.m., Amareshwari Sriramadasu wrote:
> > lens-client/src/main/java/org/apache/lens/client/LensClient.java, line 330
> > <https://reviews.apache.org/r/52119/diff/1/?file=1507243#file1507243line330>
> >
> > We would need an api to getResults for particular handle as well - to access it across clients, no? I see current caching may not work. But if not cached, client should fetch it from srver.
>
> Srikanth Sundarrajan wrote:
> Upon execution of a new query in lens client, a new statement object isn't created, so essentially, the statement object holds only LensQuery handle of last executed query. The current assumption in getLensStatement(queryHandle) will return query handle specific statement is broken. If there is merit in caching LensQuery objects of every query executed by a client/statement, then this requires a far deeper fix. I can change the scope of the issue and revise the patch accordingly, but we need to be convinced about the need to cache the LensQuery object. It seems unncessary to cache a LensQuery object particuarly if it is unfinished, as the state might change as the query makes progress. The caching can happen in the consuming application in such case given that the LensQuery for finished queries are immutable. Would like you hear your views before proceeding further.
>
> Rajat Khandelwal wrote:
> The query shouldn't be cached unless it's in a terminal state. I have incorporated this kind of caching in the python client.
>
> Srikanth Sundarrajan wrote:
> Agreed. Caching unfinished LensQuery object doesn't seem like the right thing to do, will wait for @Amareshwari also to chime in (in case am missing something), and then we can move further on this.
>
> Amareshwari Sriramadasu wrote:
> I totally agree, caching for LensQuery object is not required. I think current caching is there for cli command to accept commands without query handle to get details of last executed query.
>
> For the problem in the issue - getResults for a handle - client can always get results from server for the handle.
>
> Srikanth Sundarrajan wrote:
> Thanks Amareshwari & Rajat, Are there any changes suggested/required on this patch ?
Had offline conversation with Amareshwari and she rightly pointed out that the fix is actually ignoring the queryHandle and will more likely return incorrect results. Am working on the fix, will put up a revised patch. Thanks
- Srikanth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52119/#review149951
-----------------------------------------------------------
On Sept. 21, 2016, 2:14 p.m., Srikanth Sundarrajan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52119/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2016, 2:14 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1335
> https://issues.apache.org/jira/browse/LENS-1335
>
>
> Repository: lens
>
>
> Description
> -------
>
> NullPointerException in LensClient::getResults(QueryHandle query)
>
>
> Diffs
> -----
>
> lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java e75fc0e
> lens-client/src/main/java/org/apache/lens/client/LensClient.java 593cc08
>
> Diff: https://reviews.apache.org/r/52119/diff/
>
>
> Testing
> -------
>
> Modified tests to avoid using offending method.
>
>
> Thanks,
>
> Srikanth Sundarrajan
>
>
Re: Review Request 52119: LENS-1335 NullPointerException in
LensClient::getResults(QueryHandle query)
Posted by Amareshwari Sriramadasu <am...@apache.org>.
> On Sept. 22, 2016, 5:56 a.m., Amareshwari Sriramadasu wrote:
> > lens-client/src/main/java/org/apache/lens/client/LensClient.java, line 330
> > <https://reviews.apache.org/r/52119/diff/1/?file=1507243#file1507243line330>
> >
> > We would need an api to getResults for particular handle as well - to access it across clients, no? I see current caching may not work. But if not cached, client should fetch it from srver.
>
> Srikanth Sundarrajan wrote:
> Upon execution of a new query in lens client, a new statement object isn't created, so essentially, the statement object holds only LensQuery handle of last executed query. The current assumption in getLensStatement(queryHandle) will return query handle specific statement is broken. If there is merit in caching LensQuery objects of every query executed by a client/statement, then this requires a far deeper fix. I can change the scope of the issue and revise the patch accordingly, but we need to be convinced about the need to cache the LensQuery object. It seems unncessary to cache a LensQuery object particuarly if it is unfinished, as the state might change as the query makes progress. The caching can happen in the consuming application in such case given that the LensQuery for finished queries are immutable. Would like you hear your views before proceeding further.
>
> Rajat Khandelwal wrote:
> The query shouldn't be cached unless it's in a terminal state. I have incorporated this kind of caching in the python client.
>
> Srikanth Sundarrajan wrote:
> Agreed. Caching unfinished LensQuery object doesn't seem like the right thing to do, will wait for @Amareshwari also to chime in (in case am missing something), and then we can move further on this.
I totally agree, caching for LensQuery object is not required. I think current caching is there for cli command to accept commands without query handle to get details of last executed query.
For the problem in the issue - getResults for a handle - client can always get results from server for the handle.
- Amareshwari
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52119/#review149951
-----------------------------------------------------------
On Sept. 21, 2016, 2:14 p.m., Srikanth Sundarrajan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52119/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2016, 2:14 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1335
> https://issues.apache.org/jira/browse/LENS-1335
>
>
> Repository: lens
>
>
> Description
> -------
>
> NullPointerException in LensClient::getResults(QueryHandle query)
>
>
> Diffs
> -----
>
> lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java e75fc0e
> lens-client/src/main/java/org/apache/lens/client/LensClient.java 593cc08
>
> Diff: https://reviews.apache.org/r/52119/diff/
>
>
> Testing
> -------
>
> Modified tests to avoid using offending method.
>
>
> Thanks,
>
> Srikanth Sundarrajan
>
>
Re: Review Request 52119: LENS-1335 NullPointerException in
LensClient::getResults(QueryHandle query)
Posted by Srikanth Sundarrajan <sr...@hotmail.com>.
> On Sept. 22, 2016, 5:56 a.m., Amareshwari Sriramadasu wrote:
> > lens-client/src/main/java/org/apache/lens/client/LensClient.java, line 330
> > <https://reviews.apache.org/r/52119/diff/1/?file=1507243#file1507243line330>
> >
> > We would need an api to getResults for particular handle as well - to access it across clients, no? I see current caching may not work. But if not cached, client should fetch it from srver.
>
> Srikanth Sundarrajan wrote:
> Upon execution of a new query in lens client, a new statement object isn't created, so essentially, the statement object holds only LensQuery handle of last executed query. The current assumption in getLensStatement(queryHandle) will return query handle specific statement is broken. If there is merit in caching LensQuery objects of every query executed by a client/statement, then this requires a far deeper fix. I can change the scope of the issue and revise the patch accordingly, but we need to be convinced about the need to cache the LensQuery object. It seems unncessary to cache a LensQuery object particuarly if it is unfinished, as the state might change as the query makes progress. The caching can happen in the consuming application in such case given that the LensQuery for finished queries are immutable. Would like you hear your views before proceeding further.
>
> Rajat Khandelwal wrote:
> The query shouldn't be cached unless it's in a terminal state. I have incorporated this kind of caching in the python client.
>
> Srikanth Sundarrajan wrote:
> Agreed. Caching unfinished LensQuery object doesn't seem like the right thing to do, will wait for @Amareshwari also to chime in (in case am missing something), and then we can move further on this.
>
> Amareshwari Sriramadasu wrote:
> I totally agree, caching for LensQuery object is not required. I think current caching is there for cli command to accept commands without query handle to get details of last executed query.
>
> For the problem in the issue - getResults for a handle - client can always get results from server for the handle.
Thanks Amareshwari & Rajat, Are there any changes suggested/required on this patch ?
- Srikanth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52119/#review149951
-----------------------------------------------------------
On Sept. 21, 2016, 2:14 p.m., Srikanth Sundarrajan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52119/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2016, 2:14 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1335
> https://issues.apache.org/jira/browse/LENS-1335
>
>
> Repository: lens
>
>
> Description
> -------
>
> NullPointerException in LensClient::getResults(QueryHandle query)
>
>
> Diffs
> -----
>
> lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java e75fc0e
> lens-client/src/main/java/org/apache/lens/client/LensClient.java 593cc08
>
> Diff: https://reviews.apache.org/r/52119/diff/
>
>
> Testing
> -------
>
> Modified tests to avoid using offending method.
>
>
> Thanks,
>
> Srikanth Sundarrajan
>
>
Re: Review Request 52119: LENS-1335 NullPointerException in
LensClient::getResults(QueryHandle query)
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52119/#review149951
-----------------------------------------------------------
lens-client/src/main/java/org/apache/lens/client/LensClient.java (line 330)
<https://reviews.apache.org/r/52119/#comment217735>
We would need an api to getResults for particular handle as well - to access it across clients, no? I see current caching may not work. But if not cached, client should fetch it from srver.
- Amareshwari Sriramadasu
On Sept. 21, 2016, 2:14 p.m., Srikanth Sundarrajan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52119/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2016, 2:14 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1335
> https://issues.apache.org/jira/browse/LENS-1335
>
>
> Repository: lens
>
>
> Description
> -------
>
> NullPointerException in LensClient::getResults(QueryHandle query)
>
>
> Diffs
> -----
>
> lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java e75fc0e
> lens-client/src/main/java/org/apache/lens/client/LensClient.java 593cc08
>
> Diff: https://reviews.apache.org/r/52119/diff/
>
>
> Testing
> -------
>
> Modified tests to avoid using offending method.
>
>
> Thanks,
>
> Srikanth Sundarrajan
>
>