Skip to content

Conversation

@Sevenannn
Copy link

@Sevenannn Sevenannn commented Nov 12, 2024

Which issue does this PR close?

Rationale for this change

When unparsing CharacterLengthFunc scalar function to DuckDB & SQLite syntax SQL query, the CharacterLengthFunc scalar function need to be unparsed to length, since length is the equivalence of CharacterLengthFunc in DuckDB & SQLite.

DuckDB length()
SQLite length()

What changes are included in this PR?

  • Add struct DuckDBDialect, which implements the Dialect trait
  • Add new method character_length_style for Dialect trait for determining the CharacterLengthStyle to use for a dialect
  • Add helper function character_length_to_sql, which unparses the CharacterLengthFunc to length or character_length based on CharacterLengthStyle

Are these changes tested?

Yes

Are there any user-facing changes?

No

@Sevenannn Sevenannn marked this pull request as ready for review November 13, 2024 00:03
@Sevenannn Sevenannn requested a review from a team November 13, 2024 00:05
Copy link

@peasee peasee left a comment

Choose a reason for hiding this comment

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

A couple minor comments for naming. Otherwise looks good!

@Sevenannn Sevenannn merged commit 0c3bbc3 into spiceai-43 Nov 13, 2024
@Sevenannn Sevenannn deleted the qianqian/fix-character-scalar branch November 13, 2024 01:49
Sevenannn added a commit that referenced this pull request Nov 14, 2024
* Fix duckdb & sqlite character_length scalar unparsing

* Add comments

* Update CharacterLengthStyle::SQLStandard to CharacterLengthExtractStyle::CharacterLength
phillipleblanc pushed a commit that referenced this pull request Nov 18, 2024
* Fix duckdb & sqlite character_length scalar unparsing (#59)

* Fix duckdb & sqlite character_length scalar unparsing

* Add comments

* Update CharacterLengthStyle::SQLStandard to CharacterLengthExtractStyle::CharacterLength

* Fix clippy error
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