Skip to content
This repository was archived by the owner on Oct 10, 2025. It is now read-only.

Remove count from evaluator local state - #4878

Merged
andyfengHKU merged 2 commits into
masterfrom
remove-evaluator-local-state
Feb 12, 2025
Merged

Remove count from evaluator local state#4878
andyfengHKU merged 2 commits into
masterfrom
remove-evaluator-local-state

Conversation

@andyfengHKU

@andyfengHKU andyfengHKU commented Feb 10, 2025

Copy link
Copy Markdown
Contributor

Description

This PR removes count from evaluator local state. The previous way of directly modifying an evaluator local state is way too hack. This PR instead provides a new API evaluate(count) specifically for this case. evaluate(count) is used in the following 2 cases

  • evaluate constant for count times as default value.
  • evaluate nextVal() for count times as default value for SERIAL column.
    So we only implement it for the above two types of evaluator.

In the long term, we still need to

  • remove EvaluatorLocalState. This is already doable but since it touches too many files, I will refactor with a separate PR.
  • introduce functionLocalState and move count out of FunctionBindData

Fixes # (issue)

Contributor agreement

@andyfengHKU
andyfengHKU requested a review from ray6080 February 10, 2025 16:02
@github-actions

github-actions Bot commented Feb 10, 2025

Copy link
Copy Markdown

Benchmark Result

Master commit hash: 3b37b0845e07fa51e47f9dfecf4f2f87ee399e42
Branch commit hash: 94173add3a9469d390de1517f4489dba51555472

Query Group Query Name Mean Time - Commit (ms) Mean Time - Master (ms) Diff
aggregation q24 744.74 731.20 13.54 (1.85%)
aggregation q28 6404.26 6339.96 64.31 (1.01%)
filter q14 126.17 118.57 7.60 (6.41%)
filter q15 123.52 115.52 8.00 (6.92%)
filter q16 302.64 309.00 -6.36 (-2.06%)
filter q17 449.24 444.76 4.48 (1.01%)
filter q18 1947.46 1911.24 36.22 (1.90%)
filter zonemap-node 88.94 83.03 5.91 (7.12%)
filter zonemap-node-lhs-cast 92.11 81.52 10.59 (12.99%)
filter zonemap-node-null 91.31 80.69 10.62 (13.16%)
filter zonemap-rel 5498.83 5471.28 27.55 (0.50%)
fixed_size_expr_evaluator q07 572.05 563.42 8.63 (1.53%)
fixed_size_expr_evaluator q08 805.03 795.32 9.71 (1.22%)
fixed_size_expr_evaluator q09 801.14 796.53 4.61 (0.58%)
fixed_size_expr_evaluator q10 236.81 228.96 7.85 (3.43%)
fixed_size_expr_evaluator q11 228.79 222.04 6.75 (3.04%)
fixed_size_expr_evaluator q12 226.45 217.14 9.31 (4.29%)
fixed_size_expr_evaluator q13 1455.54 1447.51 8.03 (0.55%)
fixed_size_seq_scan q23 110.47 105.86 4.61 (4.36%)
join q29 758.55 715.70 42.86 (5.99%)
join q30 10185.97 9894.64 291.33 (2.94%)
join q31 7.92 7.14 0.77 (10.82%)
join SelectiveTwoHopJoin 50.35 55.23 -4.88 (-8.84%)
ldbc_snb_ic q35 2577.92 2689.46 -111.54 (-4.15%)
ldbc_snb_ic q36 486.51 474.01 12.50 (2.64%)
ldbc_snb_is q32 2.43 5.73 -3.29 (-57.52%)
ldbc_snb_is q33 13.94 15.37 -1.43 (-9.29%)
ldbc_snb_is q34 1.51 1.18 0.33 (27.83%)
multi-rel multi-rel-large-scan 1316.25 1397.44 -81.19 (-5.81%)
multi-rel multi-rel-lookup 33.78 31.59 2.19 (6.94%)
multi-rel multi-rel-small-scan 91.76 55.71 36.05 (64.71%)
order_by q25 130.85 123.06 7.79 (6.33%)
order_by q26 446.53 440.62 5.91 (1.34%)
order_by q27 1432.39 1437.25 -4.87 (-0.34%)
recursive_join recursive-join-bidirection 300.69 315.22 -14.53 (-4.61%)
recursive_join recursive-join-dense 7430.49 7359.64 70.85 (0.96%)
recursive_join recursive-join-path 24291.36 24295.00 -3.64 (-0.01%)
recursive_join recursive-join-sparse 1063.92 1047.62 16.30 (1.56%)
recursive_join recursive-join-trail 7372.75 7333.16 39.59 (0.54%)
scan_after_filter q01 179.11 168.37 10.74 (6.38%)
scan_after_filter q02 160.65 151.14 9.51 (6.29%)
shortest_path_ldbc100 q37 88.63 100.48 -11.85 (-11.79%)
shortest_path_ldbc100 q38 313.90 387.24 -73.34 (-18.94%)
shortest_path_ldbc100 q39 63.73 64.98 -1.25 (-1.92%)
shortest_path_ldbc100 q40 345.53 462.65 -117.12 (-25.31%)
var_size_expr_evaluator q03 2120.91 2093.01 27.90 (1.33%)
var_size_expr_evaluator q04 2248.54 2232.23 16.31 (0.73%)
var_size_expr_evaluator q05 2648.90 2663.57 -14.67 (-0.55%)
var_size_expr_evaluator q06 1357.65 1321.14 36.51 (2.76%)
var_size_seq_scan q19 1477.50 1443.32 34.18 (2.37%)
var_size_seq_scan q20 2319.23 2341.89 -22.66 (-0.97%)
var_size_seq_scan q21 2337.68 2302.94 34.74 (1.51%)
var_size_seq_scan q22 131.16 126.92 4.24 (3.34%)

@andyfengHKU
andyfengHKU merged commit 98adae4 into master Feb 12, 2025
@andyfengHKU
andyfengHKU deleted the remove-evaluator-local-state branch February 12, 2025 06:38
acquamarin pushed a commit that referenced this pull request Feb 19, 2025
@andyfengHKU @acquamarin
Remove count from evaluator local state (#4878)
06e491e 
* Remove evaluator local state

* Run clang-format

---------

Co-authored-by: CI Bot <andyfengHKU@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@benjaminwinger benjaminwinger Awaiting requested review from benjaminwinger benjaminwinger is a code owner
1 more reviewer
@ray6080 ray6080 ray6080 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@andyfengHKU @ray6080