bug(tianmu):improve the code quality and refactoring derived table optimize for tianmu(#1277, #1276, #1258) by isredstar · Pull Request #1281 · stoneatom/stonedb

isredstar · 2023-02-07T12:51:32Z

Summary about this PR

Issue Number: close #1247,#1276,#1258
refactor derived table optimize for tianmu
improve the code quality to modify the magic number

Tests Check List

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Changelog

Documentation

Affects user behaviors
Contains syntax changes
Contains variable changes
Contains experimental features

mergify · 2023-02-07T12:52:06Z

Thanks for the contribution!
I have applied any labels matching special text in your PR Changelog.

Please review the labels and make any necessary changes.

mergify · 2023-02-07T13:57:46Z

This pull request's title should follow requirements next. @isredstar please check it 👇.

Valid format:

fix(vc): fix sth..... (#3306)
  ^         ^---------^  ^----^
  |         |            |
  |         +            +-> you issue id.
  |         |
  |         +-> Summary in present tense.
  |
  +-------> Type: feat, fix, docs, workflow, style, refactor, test, website, chore

Valid types:

feat: new feature for stonedb
fix: bug fix for stonedb
docs: changes to the documentation
workflow: ci/cd in .github
perf: Changes to improve code performance
refactor: refactoring production code, eg. renaming a variable
style: formatting, missing semi colons, etc; no production code change
test: adding missing tests, refactoring tests; no production code change
website
chore: updating grunt tasks etc; no production code change

codecov · 2023-02-08T02:52:17Z

Codecov Report

Base: 0.00% // Head: 43.10% // Increases project coverage by +43.10% 🎉

Coverage data is based on head (3ef25d4) compared to base (fa5917a).
Patch coverage: 80.42% of modified lines in pull request are covered.

❗ Current head 3ef25d4 differs from pull request most recent head f62e2e9. Consider uploading reports for the commit f62e2e9 to get more accurate results

Additional details and impacted files

@@                 Coverage Diff                  @@
##           stonedb-5.7-dev    #1281       +/-   ##
====================================================
+ Coverage                 0   43.10%   +43.10%     
====================================================
  Files                    0     1830     +1830     
  Lines                    0   396311   +396311     
====================================================
+ Hits                     0   170839   +170839     
- Misses                   0   225472   +225472

Impacted Files	Coverage Δ
sql/sql_derived.cc	`95.89% <ø> (ø)`
sql/sql_lex.h	`53.06% <ø> (ø)`
sql/sql_optimizer.h	`74.54% <ø> (ø)`
sql/sql_select.h	`73.27% <ø> (ø)`
sql/table.h	`55.00% <ø> (ø)`
storage/tianmu/core/engine.h	`30.43% <ø> (ø)`
storage/tianmu/core/engine_execute.cpp	`58.46% <75.43%> (ø)`
sql/sql_optimizer.cc	`68.73% <81.60%> (ø)`
sql/sql_parse.cc	`61.84% <100.00%> (ø)`
sql/sql_select.cc	`69.41% <100.00%> (ø)`
... and 1832 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

lujiashun

suggest to refer to this script to format your code.( you should have installed clang-format, clang-format-diff.py)

#/bin/bash
echo $0
cd ..
cur=`pwd`"/"
cd  storage/tianmu
echo $cur
git status .|grep modified |awk '{print $2}'|xargs -i -n 1 clang-format  --style=file -i {}
git diff HEAD -U0 --no-color --src-prefix=$cur --dst-prefix=$cur | clang-format-diff.py -i -style=file

lujiashun · 2023-02-09T02:11:48Z

sql/sql_optimizer.cc

+        including those in semi-joins.
+      */
+      if (select_lex->materialized_derived_table_count)
+        {


blank space number was more than expected.

sql/sql_optimizer.cc

lujiashun · 2023-02-09T02:20:24Z

sql/sql_optimizer.h

  List<Item> *fields;
 } ROLLUP;

+enum class OptimizePhase // for Tianmu to indicate which optimization phase.


refer to googel style: https://google.github.io/styleguide/cppguide.html#Enumerator_Names

lujiashun · 2023-02-09T02:22:58Z

sql/sql_optimizer.cc

    {
-      if (part!=1 || cond->type()!=Item::SUBSELECT_ITEM)
-      {//TIANMU UPGRADE
+      if (phase !=OptimizePhase::Before_LOJ_Transform || cond->type()!=Item::SUBSELECT_ITEM)


add a blank space after '!="

wisehead · 2023-02-14T02:09:12Z

sql/sql_lex.h

  int optimize_for_tianmu();
-  int optimize_after_tianmu();
-  //END
+  int after_optimize_tianmu();


why rename the function name? what's the difference?

The optimize_after_tianmu has been as a variable name，so I think it is not suitable as a function name.

a function name should start with a verb. like set_value, update_row, I suggest rename the variable name.

…ptimize for tianmu(stoneatom#1277, stoneatom#1276, stoneatom#1258)

lujiashun

LGTM

mergify bot added the PR-feature feature for pull request label Feb 7, 2023

isredstar self-assigned this Feb 7, 2023

isredstar added this to the stonedb_5.7_v1.0.3 milestone Feb 7, 2023

isredstar changed the title ~~feat(tianmu):improve the code quality and refactoring derived table optimize for tianmu(#1277, #1276, #1258)~~ bug:improve the code quality and refactoring derived table optimize for tianmu(#1277, #1276, #1258) Feb 7, 2023

isredstar added A-bug Something isn't working and removed PR-feature feature for pull request labels Feb 7, 2023

isredstar changed the title ~~bug:improve the code quality and refactoring derived table optimize for tianmu(#1277, #1276, #1258)~~ bug(tianmu):improve the code quality and refactoring derived table optimize for tianmu(#1277, #1276, #1258) Feb 7, 2023

isredstar force-pushed the stonedb-5.7-dev branch 2 times, most recently from 3e93dce to 7341a45 Compare February 8, 2023 02:27

isredstar force-pushed the stonedb-5.7-dev branch from 7341a45 to 0531e62 Compare February 8, 2023 03:02

isredstar requested review from DandreChen, RingsC, duanfuxiang0, hustjieke, lujiashun and wisehead February 9, 2023 02:01

isredstar added PR-bug bug for pull request and removed A-bug Something isn't working labels Feb 9, 2023

lujiashun reviewed Feb 9, 2023

View reviewed changes

isredstar force-pushed the stonedb-5.7-dev branch from 0531e62 to c4a4933 Compare February 9, 2023 02:52

Nliver force-pushed the stonedb-5.7-dev branch from c4a4933 to 20358f9 Compare February 13, 2023 08:05

wisehead suggested changes Feb 14, 2023

View reviewed changes

isredstar force-pushed the stonedb-5.7-dev branch 5 times, most recently from a8637fe to 3bf8723 Compare February 15, 2023 10:20

isredstar force-pushed the stonedb-5.7-dev branch 3 times, most recently from c04a28b to 64fc98c Compare February 15, 2023 12:59

feat(tianmu):improve the code quality and refactoring derived table o…

3ef25d4

…ptimize for tianmu(stoneatom#1277, stoneatom#1276, stoneatom#1258)

isredstar force-pushed the stonedb-5.7-dev branch from 64fc98c to 3ef25d4 Compare February 15, 2023 14:03

wisehead approved these changes Feb 16, 2023

View reviewed changes

lujiashun approved these changes Feb 16, 2023

View reviewed changes

Merge branch 'stonedb-5.7-dev' into stonedb-5.7-dev

f62e2e9

mergify bot merged commit bdbf8d9 into stoneatom:stonedb-5.7-dev Feb 16, 2023

Uh oh!

Conversation

isredstar commented Feb 7, 2023

Summary about this PR

Tests Check List

Changelog

Documentation

Uh oh!

mergify bot commented Feb 7, 2023

Uh oh!

mergify bot commented Feb 7, 2023

Uh oh!

codecov bot commented Feb 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

lujiashun left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

isredstar Feb 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lujiashun left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Feb 8, 2023 •

edited

Loading

isredstar Feb 14, 2023 •

edited

Loading