[ 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)