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

Sqlfluff linting existing clickhouse queries pt2 #6311

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Camyll
Copy link
Contributor

@Camyll Camyll commented Feb 20, 2025

lint existing clickhouse queries with new sqlfluff linter in a few separated PRs. (part two)

Copy link

vercel bot commented Feb 20, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Updated (UTC)
torchci ⬜️ Ignored (Inspect) Visit Preview Feb 21, 2025 3:37pm

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 20, 2025
@Camyll Camyll requested a review from clee2000 February 20, 2025 23:44
workflow.name
SELECT
COUNT(*) AS COUNT,
WORKFLOW.NAME AS NAME
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@clee2000 is this the correct behavior? I'm not too familiar with clickhouse so I wasn't sure if this syntax is correct

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I think all of the capitalizations it changed here should not be done

from default.workflow_job -- FINAL not needed since we just use this to filter a table that has already been FINALed
where
status = 'queued'
and created_at < (CURRENT_TIMESTAMP() - interval 5 minute)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@clee2000 do you know if we also want to have this behavior where if the file starts off with sql commands in lowercase that it makes everything else lowercase?

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO it would be better to always stick to either upper case or lower case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants