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

Fixes the alias issue of struct_pack function #4894

Merged
merged 3 commits into from
Feb 13, 2025
Merged

Conversation

acquamarin
Copy link
Collaborator

We used to set the alias of struct-pack function parameters as their field names.
E.g. {'x': y} => We set the alias of y as x
This causes a bug in nested aggregation:

with count(5) as x return count({y: x});

This PR fixes it by introducing the optionalParams field in the functionExpression

@acquamarin acquamarin changed the title This PR fixes the alias issue of struct_pack function Fixes the alias issue of struct_pack function Feb 12, 2025
Copy link

Benchmark Result

Master commit hash: dfabf90eab17ec0dc0f87d18464152412e1fd8ee
Branch commit hash: 06f49a38c40ee5da75e938afa02403a600df6c80

Query Group Query Name Mean Time - Commit (ms) Mean Time - Master (ms) Diff
aggregation q24 755.25 736.85 18.40 (2.50%)
aggregation q28 6370.78 6358.04 12.73 (0.20%)
filter q14 142.51 128.29 14.22 (11.09%)
filter q15 141.75 126.50 15.24 (12.05%)
filter q16 319.28 306.18 13.09 (4.28%)
filter q17 461.08 446.79 14.28 (3.20%)
filter q18 1956.16 1922.93 33.23 (1.73%)
filter zonemap-node 105.24 88.87 16.37 (18.42%)
filter zonemap-node-lhs-cast 105.02 90.75 14.27 (15.73%)
filter zonemap-node-null 104.60 90.66 13.94 (15.38%)
filter zonemap-rel 5382.11 5394.15 -12.04 (-0.22%)
fixed_size_expr_evaluator q07 598.15 581.95 16.20 (2.78%)
fixed_size_expr_evaluator q08 822.33 801.57 20.76 (2.59%)
fixed_size_expr_evaluator q09 826.67 803.44 23.23 (2.89%)
fixed_size_expr_evaluator q10 254.17 236.67 17.49 (7.39%)
fixed_size_expr_evaluator q11 246.60 229.64 16.96 (7.38%)
fixed_size_expr_evaluator q12 248.06 231.70 16.36 (7.06%)
fixed_size_expr_evaluator q13 1473.35 1465.25 8.10 (0.55%)
fixed_size_seq_scan q23 130.54 111.76 18.78 (16.80%)
join q29 716.46 703.37 13.09 (1.86%)
join q30 9860.81 11083.57 -1222.76 (-11.03%)
join q31 5.88 9.98 -4.09 (-41.03%)
join SelectiveTwoHopJoin 53.47 59.99 -6.52 (-10.86%)
ldbc_snb_ic q35 2658.48 2607.02 51.47 (1.97%)
ldbc_snb_ic q36 459.43 485.56 -26.13 (-5.38%)
ldbc_snb_is q32 7.13 4.47 2.66 (59.44%)
ldbc_snb_is q33 15.30 14.83 0.47 (3.19%)
ldbc_snb_is q34 1.15 1.25 -0.10 (-7.74%)
multi-rel multi-rel-large-scan 1328.50 1392.59 -64.09 (-4.60%)
multi-rel multi-rel-lookup 33.36 32.54 0.82 (2.54%)
multi-rel multi-rel-small-scan 64.69 102.16 -37.47 (-36.68%)
order_by q25 148.29 131.92 16.37 (12.41%)
order_by q26 472.36 452.45 19.91 (4.40%)
order_by q27 1415.12 1420.37 -5.26 (-0.37%)
recursive_join recursive-join-bidirection 307.16 296.22 10.93 (3.69%)
recursive_join recursive-join-dense 7402.51 7444.01 -41.50 (-0.56%)
recursive_join recursive-join-path 24239.77 24117.33 122.44 (0.51%)
recursive_join recursive-join-sparse 1059.81 1057.45 2.37 (0.22%)
recursive_join recursive-join-trail 7430.59 7418.08 12.51 (0.17%)
scan_after_filter q01 186.87 175.01 11.86 (6.78%)
scan_after_filter q02 173.44 159.85 13.59 (8.50%)
shortest_path_ldbc100 q37 91.93 97.65 -5.72 (-5.85%)
shortest_path_ldbc100 q38 386.69 377.28 9.42 (2.50%)
shortest_path_ldbc100 q39 65.90 64.85 1.05 (1.62%)
shortest_path_ldbc100 q40 438.72 464.15 -25.43 (-5.48%)
var_size_expr_evaluator q03 2116.90 2149.45 -32.55 (-1.51%)
var_size_expr_evaluator q04 2277.64 2203.44 74.19 (3.37%)
var_size_expr_evaluator q05 2689.17 2620.11 69.06 (2.64%)
var_size_expr_evaluator q06 1336.85 1345.39 -8.55 (-0.64%)
var_size_seq_scan q19 1499.75 1459.82 39.93 (2.74%)
var_size_seq_scan q20 2396.89 2352.12 44.78 (1.90%)
var_size_seq_scan q21 2361.08 2311.06 50.01 (2.16%)
var_size_seq_scan q22 130.28 128.13 2.15 (1.68%)

Copy link

codecov bot commented Feb 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.53%. Comparing base (dfabf90) to head (3a0566f).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4894   +/-   ##
=======================================
  Coverage   86.52%   86.53%           
=======================================
  Files        1403     1403           
  Lines       60519    60529   +10     
  Branches     7433     7435    +2     
=======================================
+ Hits        52364    52376   +12     
+ Misses       7986     7983    -3     
- Partials      169      170    +1     

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

src/include/function/function.h Outdated Show resolved Hide resolved
Copy link

Benchmark Result

Master commit hash: dfabf90eab17ec0dc0f87d18464152412e1fd8ee
Branch commit hash: 4e3b96284507e717ff34962ca6cb7a25d7023abe

Query Group Query Name Mean Time - Commit (ms) Mean Time - Master (ms) Diff
aggregation q24 724.45 736.85 -12.40 (-1.68%)
aggregation q28 6339.88 6358.04 -18.16 (-0.29%)
filter q14 127.43 128.29 -0.86 (-0.67%)
filter q15 122.03 126.50 -4.47 (-3.53%)
filter q16 305.11 306.18 -1.08 (-0.35%)
filter q17 445.04 446.79 -1.75 (-0.39%)
filter q18 1951.76 1922.93 28.83 (1.50%)
filter zonemap-node 90.23 88.87 1.35 (1.52%)
filter zonemap-node-lhs-cast 89.04 90.75 -1.70 (-1.88%)
filter zonemap-node-null 88.35 90.66 -2.30 (-2.54%)
filter zonemap-rel 5385.36 5394.15 -8.79 (-0.16%)
fixed_size_expr_evaluator q07 579.64 581.95 -2.31 (-0.40%)
fixed_size_expr_evaluator q08 828.18 801.57 26.61 (3.32%)
fixed_size_expr_evaluator q09 809.50 803.44 6.06 (0.75%)
fixed_size_expr_evaluator q10 243.87 236.67 7.20 (3.04%)
fixed_size_expr_evaluator q11 236.95 229.64 7.30 (3.18%)
fixed_size_expr_evaluator q12 234.10 231.70 2.40 (1.03%)
fixed_size_expr_evaluator q13 1480.93 1465.25 15.69 (1.07%)
fixed_size_seq_scan q23 115.75 111.76 3.99 (3.57%)
join q29 710.10 703.37 6.73 (0.96%)
join q30 10127.51 11083.57 -956.07 (-8.63%)
join q31 8.23 9.98 -1.75 (-17.54%)
join SelectiveTwoHopJoin 58.80 59.99 -1.19 (-1.98%)
ldbc_snb_ic q35 2597.23 2607.02 -9.78 (-0.38%)
ldbc_snb_ic q36 448.08 485.56 -37.48 (-7.72%)
ldbc_snb_is q32 6.70 4.47 2.23 (49.88%)
ldbc_snb_is q33 15.16 14.83 0.33 (2.22%)
ldbc_snb_is q34 1.34 1.25 0.09 (7.48%)
multi-rel multi-rel-large-scan 1307.64 1392.59 -84.95 (-6.10%)
multi-rel multi-rel-lookup 21.31 32.54 -11.23 (-34.52%)
multi-rel multi-rel-small-scan 78.83 102.16 -23.33 (-22.84%)
order_by q25 133.60 131.92 1.68 (1.27%)
order_by q26 480.30 452.45 27.84 (6.15%)
order_by q27 1392.47 1420.37 -27.90 (-1.96%)
recursive_join recursive-join-bidirection 295.11 296.22 -1.12 (-0.38%)
recursive_join recursive-join-dense 6439.65 7444.01 -1004.36 (-13.49%)
recursive_join recursive-join-path 24075.63 24117.33 -41.70 (-0.17%)
recursive_join recursive-join-sparse 1059.99 1057.45 2.55 (0.24%)
recursive_join recursive-join-trail 6911.26 7418.08 -506.82 (-6.83%)
scan_after_filter q01 173.18 175.01 -1.83 (-1.04%)
scan_after_filter q02 160.47 159.85 0.62 (0.39%)
shortest_path_ldbc100 q37 87.90 97.65 -9.75 (-9.99%)
shortest_path_ldbc100 q38 369.27 377.28 -8.01 (-2.12%)
shortest_path_ldbc100 q39 69.65 64.85 4.80 (7.41%)
shortest_path_ldbc100 q40 421.60 464.15 -42.55 (-9.17%)
var_size_expr_evaluator q03 2081.08 2149.45 -68.37 (-3.18%)
var_size_expr_evaluator q04 2240.64 2203.44 37.19 (1.69%)
var_size_expr_evaluator q05 2626.13 2620.11 6.02 (0.23%)
var_size_expr_evaluator q06 1338.00 1345.39 -7.39 (-0.55%)
var_size_seq_scan q19 1452.58 1459.82 -7.24 (-0.50%)
var_size_seq_scan q20 2319.14 2352.12 -32.97 (-1.40%)
var_size_seq_scan q21 2283.16 2311.06 -27.91 (-1.21%)
var_size_seq_scan q22 126.90 128.13 -1.24 (-0.96%)

@acquamarin acquamarin merged commit b3e085f into master Feb 13, 2025
25 checks passed
@acquamarin acquamarin deleted the optional-params branch February 13, 2025 04:06
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.

2 participants