Optimize query building for array_length and array_upper #7754

mfussenegger · 2018-10-10T15:17:19Z

Q: select count(*) from t_arrays_biased_empty where array_length(xs, 1) > 0
C: 1
| Version |         Mean ±    Stdev |        Min |     Median |         Q3 |        Max |
|   V1    |      290.731 ±  113.990 |    247.121 |    266.169 |    287.443 |   1757.303 |
|   V2    |        1.481 ±    0.314 |      1.173 |      1.386 |      1.581 |      3.514 |
mean:   - 197.97%
median: - 197.93%
Likely significant

Q: select count(*) from t_arrays_biased_empty where array_length(xs, 1) >= 1
C: 1
| Version |         Mean ±    Stdev |        Min |     Median |         Q3 |        Max |
|   V1    |      289.019 ±   48.886 |    249.626 |    268.011 |    291.234 |    470.948 |
|   V2    |        4.097 ±    0.946 |      2.811 |      4.118 |      4.529 |     13.763 |
mean:   - 194.41%
median: - 193.95%
Likely significant

Q: select count(*) from t_arrays_biased_empty where array_length(xs, 1) > 1
C: 1
| Version |         Mean ±    Stdev |        Min |     Median |         Q3 |        Max |
|   V1    |      284.692 ±   47.274 |    247.780 |    267.656 |    285.270 |    476.669 |
|   V2    |       43.035 ±  112.856 |     22.808 |     24.378 |     27.115 |   1420.890 |
mean:   - 147.47%
median: - 166.61%
Likely significant

Q: select count(*) from t_arrays_biased_one_element where array_length(xs, 1) > 0
C: 1
| Version |         Mean ±    Stdev |        Min |     Median |         Q3 |        Max |
|   V1    |      398.286 ±   61.040 |    356.618 |    377.096 |    392.721 |    767.840 |
|   V2    |        2.269 ±    0.335 |      1.709 |      2.225 |      2.435 |      3.636 |
mean:   - 197.73%
median: - 197.65%
Likely significant

Q: select count(*) from t_arrays_biased_one_element where array_length(xs, 1) >= 1
C: 1
| Version |         Mean ±    Stdev |        Min |     Median |         Q3 |        Max |
|   V1    |      408.657 ±   85.833 |    359.427 |    379.455 |    397.453 |    815.517 |
|   V2    |        6.601 ±    0.990 |      4.734 |      6.943 |      7.169 |      9.026 |
mean:   - 193.64%
median: - 192.81%
Likely significant

Q: select count(*) from t_arrays_biased_one_element where array_length(xs, 1) > 1
C: 1
| Version |         Mean ±    Stdev |        Min |     Median |         Q3 |        Max |
|   V1    |      408.960 ±   76.523 |    360.013 |    379.550 |    404.113 |    801.026 |
|   V2    |       47.279 ±   57.332 |     28.225 |     33.883 |     38.301 |    396.388 |
mean:   - 158.55%
median: - 167.22%
Likely significant

Q: select count(*) from t_arrays_biased_5_elements where array_length(xs, 1) > 0
C: 1
| Version |         Mean ±    Stdev |        Min |     Median |         Q3 |        Max |
|   V1    |     1220.149 ±  201.066 |    991.450 |   1160.296 |   1181.234 |   2106.004 |
|   V2    |        2.294 ±    0.345 |      1.613 |      2.248 |      2.472 |      3.880 |
mean:   - 199.25%
median: - 199.23%
Likely significant

Q: select count(*) from t_arrays_biased_5_elements where array_length(xs, 1) >= 1
C: 1
| Version |         Mean ±    Stdev |        Min |     Median |         Q3 |        Max |
|   V1    |     1212.680 ±  191.316 |    991.358 |   1163.188 |   1188.586 |   2063.789 |
|   V2    |        7.037 ±    1.160 |      4.585 |      7.447 |      7.853 |      8.603 |
mean:   - 197.69%
median: - 197.46%
Likely significant

Q: select count(*) from t_arrays_biased_5_elements where array_length(xs, 1) > 1
C: 1
| Version |         Mean ±    Stdev |        Min |     Median |         Q3 |        Max |
|   V1    |     1210.043 ±  188.018 |    935.364 |   1161.068 |   1188.619 |   2090.637 |
|   V2    |     1127.600 ±  226.506 |    926.485 |   1061.582 |   1084.323 |   1968.013 |
mean:   -   7.05%
median: -   8.95%
Likely significant

Q: select count(*) from t_arrays_even where array_length(xs, 1) > 0
C: 1
| Version |         Mean ±    Stdev |        Min |     Median |         Q3 |        Max |
|   V1    |      584.994 ±   90.940 |    495.819 |    563.472 |    580.891 |   1024.237 |
|   V2    |        1.910 ±    0.333 |      1.372 |      1.857 |      2.117 |      3.282 |
mean:   - 198.70%
median: - 198.69%
Likely significant

Q: select count(*) from t_arrays_even where array_length(xs, 1) >= 1
C: 1
| Version |         Mean ±    Stdev |        Min |     Median |         Q3 |        Max |
|   V1    |      604.723 ±  122.134 |    494.548 |    562.449 |    590.167 |   1082.878 |
|   V2    |        5.944 ±    1.067 |      4.325 |      6.515 |      6.680 |      7.541 |
mean:   - 196.11%
median: - 195.42%
Likely significant

Q: select count(*) from t_arrays_even where array_length(xs, 1) > 1
C: 1
| Version |         Mean ±    Stdev |        Min |     Median |         Q3 |        Max |
|   V1    |      604.916 ±  110.365 |    495.054 |    566.245 |    617.784 |   1010.459 |
|   V2    |      259.545 ±   68.203 |    216.862 |    230.071 |    261.073 |    645.198 |
mean:   -  79.90%
median: -  84.43%
Likely significant

V1: "3.2.0-18d2e4e205b7217733cc8e38b5d731c8496b1ed6" ← master
V2: "3.2.0-624e3480fc1c6a47888b40ceb294e243c9c15b85" ← j/query-opt

User relevant changes are recorded in CHANGES.txt
Touched code is covered by tests
Documentation has been updated if necessary
CLA is signed

andreidan · 2018-10-11T08:33:24Z

sql/src/main/java/io/crate/lucene/ArrayLengthQuery.java

+        }
+        Number cmpNumber = (Number) ((Input) cmpSymbol).value();
+        if (cmpNumber == null) {
+            // TODO: this case is never reached due to early normalization + no-match detection


shall we drop this code block then?

I was wondering if I should add a unit test for this case so that it's hit or replace it with an assertion.

yeah, an assertion would be great

sql/src/main/java/io/crate/lucene/ArrayLengthQuery.java

seut · 2018-10-11T08:51:51Z

sql/src/main/java/io/crate/lucene/ArrayLengthQuery.java

+    private static IntUnaryOperator numValuesPerDocForBoolean(LeafReader reader, ColumnIdent column) {
+        SortedBinaryDocValues docValues;
+        try {
+            docValues = FieldData.toString(DocValues.getSortedNumeric(reader, column.fqn()));


isn't it irrelevant to wrap it into toString as long as we are only interested in the value count?

sql/src/main/java/io/crate/lucene/ArrayLengthQuery.java

sql/src/test/java/io/crate/lucene/ArrayLengthQueryBuilderTest.java

andreidan

lgtm, awesome improvement

seut

great, lgtm.

Various cases will be optimized, so they'll end up using different Lucene queries which is why we should test all different cases.

mfussenegger requested review from seut and andreidan October 11, 2018 08:13

andreidan reviewed Oct 11, 2018

View reviewed changes

sql/src/main/java/io/crate/lucene/ArrayLengthQuery.java Show resolved Hide resolved

seut reviewed Oct 11, 2018

View reviewed changes

andreidan reviewed Oct 11, 2018

View reviewed changes

sql/src/test/java/io/crate/lucene/ArrayLengthQueryBuilderTest.java Show resolved Hide resolved

andreidan approved these changes Oct 11, 2018

View reviewed changes

seut approved these changes Oct 11, 2018

View reviewed changes

Tests: Add more tests for array_length queries

405ec1a

Various cases will be optimized, so they'll end up using different Lucene queries which is why we should test all different cases.

mfussenegger force-pushed the j/query-opt branch from cfec665 to b546d9e Compare October 11, 2018 11:54

mfussenegger added the ready-to-merge Let Mergify merge the PR once approved and checks pass label Oct 11, 2018

mfussenegger force-pushed the j/query-opt branch from b546d9e to 78d3221 Compare October 11, 2018 12:46

mfussenegger mentioned this pull request Oct 11, 2018

Sql: Allow array counting #882

Closed

Optimize query building for array_length and array_upper

f6499ac

mfussenegger force-pushed the j/query-opt branch from 78d3221 to f6499ac Compare October 11, 2018 13:33

mergify bot merged commit 1bcf0e1 into master Oct 11, 2018

mfussenegger deleted the j/query-opt branch October 11, 2018 14:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize query building for array_length and array_upper #7754

Optimize query building for array_length and array_upper #7754

mfussenegger commented Oct 10, 2018 •

edited

Loading

andreidan Oct 11, 2018

mfussenegger Oct 11, 2018

andreidan Oct 11, 2018

seut Oct 11, 2018

andreidan left a comment

seut left a comment

Optimize query building for array_length and array_upper #7754

Optimize query building for array_length and array_upper #7754

Conversation

mfussenegger commented Oct 10, 2018 • edited Loading

andreidan Oct 11, 2018

Choose a reason for hiding this comment

mfussenegger Oct 11, 2018

Choose a reason for hiding this comment

andreidan Oct 11, 2018

Choose a reason for hiding this comment

seut Oct 11, 2018

Choose a reason for hiding this comment

andreidan left a comment

Choose a reason for hiding this comment

seut left a comment

Choose a reason for hiding this comment

mfussenegger commented Oct 10, 2018 •

edited

Loading