Skip to content

feat: remove backtrace from sql::error::Error#966

Merged
evenyag merged 2 commits intoGreptimeTeam:developfrom
etolbakov:remove-backtrace-from-sql-error
Feb 11, 2023
Merged

feat: remove backtrace from sql::error::Error#966
evenyag merged 2 commits intoGreptimeTeam:developfrom
etolbakov:remove-backtrace-from-sql-error

Conversation

@etolbakov
Copy link
Copy Markdown
Collaborator

I hereby agree to the terms of the GreptimeDB CLA

What's changed and what's your intention?

Removed backtrace from Error enum.

Please explain IN DETAIL what the changes are in this PR and why they are needed:

  • this is a cleanup task. As per Remove backtrace from sql::error::Error #855 most backtraces in sql::error::Error are unnecessary and might hurt performance since the parser would always return them while parsing an invalid SQL.

Checklist

  • Docs are not needed in this case
  • Need a bit of a steer with the necessary unit tests and integration tests.

Refer to a related PR or issue link

#855

Comment thread src/sql/src/error.rs
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 10, 2023

Codecov Report

Merging #966 (d10f8a3) into develop (c0d3533) will decrease coverage by 0.26%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop     #966      +/-   ##
===========================================
- Coverage    86.05%   85.79%   -0.26%     
===========================================
  Files          457      456       -1     
  Lines        61739    61684      -55     
===========================================
- Hits         53129    52923     -206     
- Misses        8610     8761     +151     
Flag Coverage Δ
rust 85.79% <ø> (-0.26%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/sql/src/error.rs 79.36% <ø> (ø)
src/object-store/src/test_util.rs 0.00% <0.00%> (-100.00%) ⬇️
src/datanode/src/instance.rs 41.56% <0.00%> (-14.91%) ⬇️
tests-integration/src/test_util.rs 79.47% <0.00%> (-9.33%) ⬇️
src/servers/src/grpc.rs 86.95% <0.00%> (-4.35%) ⬇️
src/promql/src/planner.rs 85.13% <0.00%> (-2.59%) ⬇️
src/datatypes/src/types/timestamp_type.rs 77.64% <0.00%> (-1.18%) ⬇️
src/meta-srv/src/service/heartbeat.rs 86.17% <0.00%> (-1.07%) ⬇️
src/mito/src/table.rs 88.08% <0.00%> (-0.52%) ⬇️
src/storage/src/write_batch.rs 99.26% <0.00%> (-0.25%) ⬇️
... and 21 more

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

@evenyag
Copy link
Copy Markdown
Contributor

evenyag commented Feb 10, 2023

Nice job! @etolbakov The Rustfmt check failed, could you fix it?

Copy link
Copy Markdown
Member

@waynexia waynexia left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

@etolbakov
Copy link
Copy Markdown
Collaborator Author

Thanks @evenyag 🙏
sorry about the formatting issue! will make sure that it won't be a problem later on.

Copy link
Copy Markdown
Contributor

@evenyag evenyag left a comment

Choose a reason for hiding this comment

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

LGTM

@evenyag evenyag merged commit 7d6f4cd into GreptimeTeam:develop Feb 11, 2023
@etolbakov etolbakov deleted the remove-backtrace-from-sql-error branch February 11, 2023 06:56
paomian pushed a commit to paomian/greptimedb that referenced this pull request Oct 19, 2023
* feat: remove backtrace from sql::error::Error

* fix: address formatting issues

---------

Co-authored-by: Evgeny Tolbakov <evgeny.tolbakov@jpmorgan.com>
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