Skip to content

Clickhouse Dialect - Support BackQuoted Identifiers#5457

Merged
greg-finley merged 4 commits intosqlfluff:mainfrom
kaiyannameighu:kai/back-quoted-identifiers
Dec 11, 2023
Merged

Clickhouse Dialect - Support BackQuoted Identifiers#5457
greg-finley merged 4 commits intosqlfluff:mainfrom
kaiyannameighu:kai/back-quoted-identifiers

Conversation

@kaiyannameighu
Copy link
Copy Markdown
Contributor

Brief summary of the change made

Fixes #5356
Fixes #4637

Prior to this change, backquoted identifiers failed to parse when using the Clickhouse Dialect.

Examples as identified in tests.

This became a bigger issue as of the release of dbt-clickhouse v1.6.1 yesterday (2023/12/4), which expands dbt references with backticks.

Are there any other side effects of this change that we should be aware of?

I don't think so

Pull Request checklist

  • Please confirm you have completed any of the necessary steps below.

  • Included test cases to demonstrate any code changes, which may be one or more of the following:

    • .yml rule test cases in test/fixtures/rules/std_rule_cases.
    • .sql/.yml parser test cases in test/fixtures/dialects (note YML files can be auto generated with tox -e generate-fixture-yml).
    • Full autofix test cases in test/fixtures/linter/autofix.
    • Other.
  • Added appropriate documentation for the change.

  • Created GitHub issues for any relevant followup/future enhancements if appropriate.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 5, 2023

Coverage Results ✅

Name    Stmts   Miss  Cover   Missing
-------------------------------------
TOTAL   16924      0   100%

222 files skipped due to complete coverage.

@coveralls
Copy link
Copy Markdown

coveralls commented Dec 5, 2023

Coverage Status

coverage: 100.0%. remained the same
when pulling aca2b0d on kaiyannameighu:kai/back-quoted-identifiers
into c9fa71b on sqlfluff:main.

Copy link
Copy Markdown
Contributor

@greg-finley greg-finley left a comment

Choose a reason for hiding this comment

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

Looking good, one cleanup request.

clickhouse_dialect = ansi_dialect.copy_as("clickhouse")
clickhouse_dialect.sets("unreserved_keywords").update(UNRESERVED_KEYWORDS)

clickhouse_dialect.add(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we just incorporate BackQuotedIdentifierSegment into the existing clickhouse_dialect.add?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@greg-finley Ah! I thought the ordering was important here. Done and cleaned up the multiple .replace clauses as well

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great, thanks! Yeah it's a bit weird how it compiles.

Copy link
Copy Markdown
Contributor

@greg-finley greg-finley left a comment

Choose a reason for hiding this comment

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

Thank you!

@greg-finley greg-finley added this pull request to the merge queue Dec 11, 2023
Merged via the queue into sqlfluff:main with commit 4668f1d Dec 11, 2023
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.

Linting error when using aliases in backticks Clickhouse Dialect does not support backticks as identifier quotes

3 participants