Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip query result from rewritten queries #4633

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

ray6080
Copy link
Contributor

@ray6080 ray6080 commented Dec 14, 2024

Description

Query results from rewritten queries should only be kept as internal one, but not exposed as query result to users.

@ray6080 ray6080 requested a review from acquamarin December 14, 2024 06:55
Copy link

codecov bot commented Dec 14, 2024

Codecov Report

Attention: Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 87.46%. Comparing base (2bb28c2) to head (5832a0a).

Files with missing lines Patch % Lines
src/include/parser/statement.h 66.66% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4633   +/-   ##
=======================================
  Coverage   87.46%   87.46%           
=======================================
  Files        1370     1370           
  Lines       57912    57915    +3     
  Branches     7210     7210           
=======================================
+ Hits        50651    50655    +4     
+ Misses       7092     7091    -1     
  Partials      169      169           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

Benchmark Result

Master commit hash: 2bb28c28819a081c63229f651b447c7a2653fd54
Branch commit hash: e1aa3d53c267f05e79239444a68afabc2e8ff426

Query Group Query Name Mean Time - Commit (ms) Mean Time - Master (ms) Diff
aggregation q24 646.28 654.40 -8.13 (-1.24%)
aggregation q28 11998.74 12365.31 -366.57 (-2.96%)
filter q14 126.56 133.10 -6.54 (-4.91%)
filter q15 124.34 134.69 -10.36 (-7.69%)
filter q16 303.16 306.93 -3.76 (-1.23%)
filter q17 446.68 455.36 -8.68 (-1.91%)
filter q18 1945.29 1936.22 9.08 (0.47%)
filter zonemap-node 87.30 94.77 -7.47 (-7.88%)
filter zonemap-node-lhs-cast 86.13 94.88 -8.75 (-9.22%)
filter zonemap-rel 5702.04 5415.07 286.97 (5.30%)
fixed_size_expr_evaluator q07 569.04 578.82 -9.79 (-1.69%)
fixed_size_expr_evaluator q08 802.00 806.06 -4.06 (-0.50%)
fixed_size_expr_evaluator q09 803.51 809.88 -6.37 (-0.79%)
fixed_size_expr_evaluator q10 235.40 244.55 -9.14 (-3.74%)
fixed_size_expr_evaluator q11 227.87 237.48 -9.61 (-4.05%)
fixed_size_expr_evaluator q12 225.55 233.86 -8.30 (-3.55%)
fixed_size_expr_evaluator q13 1457.23 1485.24 -28.00 (-1.89%)
fixed_size_seq_scan q23 108.24 125.00 -16.76 (-13.41%)
join q29 600.18 639.31 -39.13 (-6.12%)
join q30 1469.17 1592.03 -122.86 (-7.72%)
join q31 5.64 4.68 0.95 (20.36%)
join SelectiveTwoHopJoin 53.44 55.08 -1.64 (-2.99%)
ldbc_snb_ic q35 2553.62 2663.93 -110.31 (-4.14%)
ldbc_snb_ic q36 557.33 519.73 37.60 (7.23%)
ldbc_snb_is q32 3.20 5.78 -2.58 (-44.61%)
ldbc_snb_is q33 12.84 13.62 -0.78 (-5.74%)
ldbc_snb_is q34 1.03 1.22 -0.19 (-15.40%)
multi-rel multi-rel-large-scan 1293.65 1405.14 -111.48 (-7.93%)
multi-rel multi-rel-lookup 20.71 31.58 -10.87 (-34.43%)
multi-rel multi-rel-small-scan 91.20 89.21 2.00 (2.24%)
order_by q25 132.07 137.18 -5.10 (-3.72%)
order_by q26 450.45 457.64 -7.19 (-1.57%)
order_by q27 1455.83 1465.38 -9.54 (-0.65%)
recursive_join recursive-join-bidirection 303.55 286.09 17.46 (6.10%)
recursive_join recursive-join-dense 7454.52 5568.82 1885.70 (33.86%)
recursive_join recursive-join-path 24221.14 23972.72 248.43 (1.04%)
recursive_join recursive-join-sparse 14906.29 14774.18 132.11 (0.89%)
recursive_join recursive-join-trail 7390.71 6285.52 1105.18 (17.58%)
scan_after_filter q01 169.17 178.31 -9.14 (-5.13%)
scan_after_filter q02 154.72 163.58 -8.86 (-5.42%)
shortest_path_ldbc100 q37 84.86 88.54 -3.68 (-4.15%)
shortest_path_ldbc100 q38 371.88 347.84 24.04 (6.91%)
shortest_path_ldbc100 q39 59.43 59.23 0.20 (0.34%)
shortest_path_ldbc100 q40 442.98 454.80 -11.83 (-2.60%)
var_size_expr_evaluator q03 2060.07 2070.25 -10.19 (-0.49%)
var_size_expr_evaluator q04 2230.04 2231.07 -1.03 (-0.05%)
var_size_expr_evaluator q05 2608.53 2617.03 -8.50 (-0.32%)
var_size_expr_evaluator q06 1350.55 1323.27 27.28 (2.06%)
var_size_seq_scan q19 1444.64 1451.24 -6.60 (-0.45%)
var_size_seq_scan q20 2720.13 2368.65 351.49 (14.84%)
var_size_seq_scan q21 2289.31 2268.49 20.82 (0.92%)
var_size_seq_scan q22 124.15 127.41 -3.26 (-2.56%)

Copy link
Contributor

@andyfengHKU andyfengHKU left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add tests if possible.

@@ -38,6 +41,7 @@ class Statement {

private:
common::StatementType statementType;
bool skipResult_;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would call this internal_. And also comment, internal statements are those rewritten from input and whose result should not be rendered on client side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants