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: convert table names to lowercase #446

Closed

Conversation

Rachelint
Copy link
Contributor

@Rachelint Rachelint commented Dec 2, 2022

Which issue does this PR close?

Part of #442

Rationale for this change

Convert all table name to lowercase to unify the behavior.

What changes are included in this PR?

As mentioned in title.

Are there any user-facing changes?

When user uses sdk to interact with ceresdb, the unquoted table name will be converted to lowercase now.

How does this change test

Test manually.

@Rachelint Rachelint force-pushed the convert-metrics-to-lowercase-in-router branch from 06cb112 to 6463f3b Compare December 5, 2022 08:01
@Rachelint Rachelint changed the title refactor: convert metrics to lowercase in router refactor: convert table names to lowercase Dec 6, 2022
@Rachelint Rachelint changed the title refactor: convert table names to lowercase feat: convert table names to lowercase Dec 6, 2022
@@ -13,11 +18,61 @@ pub async fn handle_route<Q>(
ctx: &HandlerContext<'_, Q>,
req: RouteRequest,
) -> Result<RouteResponse> {
// TODO: the case sensitive mode with quoted is not supported now, all table
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should file a new issue to track the changes about the case sensitivity. As for this PR, maybe we need a configuration option called case-insensitive = true to enable such conversion.

BTW, in the future changes, I am in favor of that CeresDB may be case-sensitive by default.

@ShiKaiWi ShiKaiWi closed this Dec 6, 2022
@Rachelint Rachelint deleted the convert-metrics-to-lowercase-in-router branch May 27, 2023 12:20
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.

2 participants