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

fix(core): support order by id #2233

Merged
merged 2 commits into from Jun 21, 2023
Merged

fix(core): support order by id #2233

merged 2 commits into from Jun 21, 2023

Conversation

liuxiaocs7
Copy link
Member

@liuxiaocs7 liuxiaocs7 commented Jun 19, 2023

Purpose of the PR

Main Changes

Verifying these changes

  • This change is a trivial rework / code cleanup without any test coverage.

(or)

  • This change is already covered by existing tests, such as (please describe tests).

(or)

  • This change added tests and can be verified as follows:

    • Add UT.

Does this PR potentially affect the following parts?

  • Nope
  • Dependencies (add/update license info)
  • Modify configurations
  • The public API
  • Other affects (typed here)

Documentation Status

  • Doc - TODO
  • Doc - Done
  • Doc - NO Need

@codecov
Copy link

codecov bot commented Jun 19, 2023

Codecov Report

Merging #2233 (228f699) into master (b836426) will increase coverage by 0.01%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##             master    #2233      +/-   ##
============================================
+ Coverage     68.64%   68.66%   +0.01%     
  Complexity      977      977              
============================================
  Files           498      498              
  Lines         40681    40682       +1     
  Branches       5681     5681              
============================================
+ Hits          27927    27934       +7     
+ Misses        10055    10047       -8     
- Partials       2699     2701       +2     
Impacted Files Coverage Δ
...va/org/apache/hugegraph/structure/HugeElement.java 74.20% <0.00%> (+1.93%) ⬆️

... and 9 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@imbajin imbajin requested review from javeme and zyxxoo June 19, 2023 07:45
@imbajin imbajin changed the title chore: add order by id support fix: support order by id Jun 19, 2023
@imbajin imbajin changed the title fix: support order by id fix(core): support order by id Jun 19, 2023
javeme
javeme previously approved these changes Jun 20, 2023
Copy link
Contributor

@javeme javeme left a comment

Choose a reason for hiding this comment

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

LGTM

@javeme
Copy link
Contributor

javeme commented Jun 20, 2023

please note ci error:

Caused by: com.mysql.cj.exceptions.WrongArgumentException:
The connection property 'useSSL' acceptable values are: 'TRUE', 'FALSE', 'YES' or 'NO'.
 The value 'disable' is not acceptable.

prefer to update ssl_mode in a separate PR.

@liuxiaocs7
Copy link
Member Author

please note ci error:

Caused by: com.mysql.cj.exceptions.WrongArgumentException:
The connection property 'useSSL' acceptable values are: 'TRUE', 'FALSE', 'YES' or 'NO'.
 The value 'disable' is not acceptable.

prefer to update ssl_mode in a separate PR.

got it, thanks!

Copy link
Contributor

@javeme javeme left a comment

Choose a reason for hiding this comment

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

LGTM

@imbajin imbajin merged commit 5227248 into apache:master Jun 21, 2023
18 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Bug] Inconsistent behavior when sorting vertices or edges
3 participants