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

feat: short sql length setting #16502

Merged
merged 1 commit into from
Sep 29, 2024

Conversation

arkzuse
Copy link
Contributor

@arkzuse arkzuse commented Sep 23, 2024

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Summary

New setting for short length sql.

Tests

  • Unit Test
  • Logic Test
  • Benchmark Test
  • No Test

Type of change

  • Bug Fix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Breaking Change (fix or feature that could cause existing functionality not to work as expected)
  • Documentation Update
  • Refactoring
  • Performance Improvement
  • Other (please describe):

This change is Reviewable

@arkzuse arkzuse changed the title feature: short sql length setting feat: short sql length setting Sep 23, 2024
@github-actions github-actions bot added the pr-feature this PR introduces a new feature to the codebase label Sep 23, 2024
@arkzuse arkzuse marked this pull request as ready for review September 23, 2024 14:04
@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. C-feature Category: feature C-setting-changed Category: setting changed, need note in release labels Sep 23, 2024
@BohuTANG BohuTANG marked this pull request as draft September 24, 2024 00:59
@BohuTANG
Copy link
Member

Hi @arkzuse

Thanks for the contribution!
This PR is not yet ready for review, so I've converted it to a draft. Please mark it as 'READY' when you believe it's ready for review.

@arkzuse
Copy link
Contributor Author

arkzuse commented Sep 24, 2024

@BohuTANG sorry for the oversight. What can I do to make it review ready?

@BohuTANG
Copy link
Member

BohuTANG commented Sep 24, 2024

@BohuTANG sorry for the oversight. What can I do to make it review ready?

Currently, the settings are only defined. We need to implement them in the short_sql func:
https://github.com/datafuselabs/databend/blob/8eaf57b52cc56929291d5d3b25c696c0bd4a2252/src/common/base/src/base/string.rs#L196-L198

Something like short_sql(sql String, max_length: usize) ...

You can refer to other settings for guidance on how to get and set values.

Finally, please add a test for this PR. Once all tests pass, it will be ready for review, and the tests can reference the other settings.

@BohuTANG
Copy link
Member

For you reference:
BohuTANG@a3642b0

@arkzuse arkzuse closed this Sep 26, 2024
@arkzuse arkzuse deleted the short-sql-default-setting branch September 26, 2024 07:21
@BohuTANG
Copy link
Member

@arkzuse Hi, could you clarify why the PR was closed? Let me know if you need any help to continue.

@arkzuse
Copy link
Contributor Author

arkzuse commented Sep 26, 2024

@BohuTANG I didn't close it. I am working on this pr and interested to contribute in future.

@arkzuse
Copy link
Contributor Author

arkzuse commented Sep 26, 2024

Oh i deleted my local brach so it closed...
sorry for inconvenience

@arkzuse arkzuse restored the short-sql-default-setting branch September 26, 2024 10:13
@arkzuse arkzuse reopened this Sep 26, 2024
@arkzuse arkzuse marked this pull request as ready for review September 28, 2024 08:28
@BohuTANG BohuTANG self-requested a review September 29, 2024 02:25
@BohuTANG
Copy link
Member

@arkzuse Thanks for your contribution!

@BohuTANG BohuTANG merged commit 5bfafbe into databendlabs:main Sep 29, 2024
83 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature Category: feature C-setting-changed Category: setting changed, need note in release pr-feature this PR introduces a new feature to the codebase size:XS This PR changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feature: short sql length setting
2 participants