[jira] [Comment Edited] (IGNITE-6702) Get rid of values deserialization in H2TreeIndex.getRowCount

["Vladimir Ozerov (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/IGNITE-6702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16265026#comment-16265026 ] 

Vladimir Ozerov edited comment on IGNITE-6702 at 11/24/17 8:12 AM:
-------------------------------------------------------------------

[~kirill.shirokov], my comments:
1) I did a number of changes in the code to better align it with our styling rules and general guidelines:
1.1) Variables should be abbreviated according to our abbreviation rules
1.2) It is better to avoid long chains of method calls on a single line because it makes debug and log analysis harder.
1.3) Semantically different lines should have blank line in betwee,
2) {{H2TreeIndex#filter}} - there was potential NPE because thread-local context could be {{null}}. To avoid that I moved cache filter logic to a common method used in several places.
3) {{H2TreeIndex#filter}} - TODOs are not allowed in the code. 
4) {{H2TreeIndex#filter}} - please avoid anonymous classes because according to our experience it may lead to unexpected issues with binary compatibility. It is better to rework it to top-level class or at least inner static class.
5) The most important - please confirm that we have enough tests for {{COUNT(*)}} with different cache modes ({{PARTITIONED}}, {{PARTITIONED}} with backups, {{REPLICATED}}, {{LOCAL}}, one nodes, several nodes). If no, let's create a set of tests covering all these scenarios.

The rest looks good to me.


was (Author: vozerov):
[~kirill.shirokov], my comments:
1) I did a number of changes in the code to better align it with our styling rules and general guidelines:
1.1) Variables should be abbreviated according to our abbreviation rules
1.2) It is better to avoid long chains of method calls on a single line because it makes debug and log analysis harder.
1.3) Semantically different lines should have blank line in betwee,
2) {{H2TreeIndex#filter}} - there was potential NPE because thread-locl context could be {{null}}. To avoid that I moved cache filter logic to a common method use in several places.
3) {{H2TreeIndex#filter}} - TODOs are not allowed in the code. 
4) {{H2TreeIndex#filter}} - please avoid anonymous classes because according to our experience it may lead to unexpected issues with binary compatibility. It is better to rework it to top-level class or at least inner static class.
5) The most important - please confirm that we have enough tests for {{COUNT(*)}} with different cache modes ({{PARTITIONED}}, {{PARTITIONED}} with backups, {{REPLICATED}}, {{LOCAL}}, one nodes, several nodes). If no, let's create a set of tests covering all these scenarios.

> Get rid of values deserialization in H2TreeIndex.getRowCount
> ------------------------------------------------------------
>
>                 Key: IGNITE-6702
>                 URL: https://issues.apache.org/jira/browse/IGNITE-6702
>             Project: Ignite
>          Issue Type: Improvement
>          Components: sql
>            Reporter: Semen Boikov
>            Assignee: Kirill Shirokov
>              Labels: performance
>             Fix For: 2.4
>
>
> It seems H2TreeIndex.getRowCount is called for 'select count(*) from x' queries, now it is implemented via BPlusTree.find(x, x) method which deserializes all values. Actually values are not needed there, need implement method on BPlusTree computing size without deserialization.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)