Skip to content

feat: add date.to_text for MySQL, Postgres, MSSQL and Clickhouse + doc - #3955

Merged
PrettyWood merged 12 commits into
mainfrom
date-to-string-others
Dec 19, 2023
Merged

feat: add date.to_text for MySQL, Postgres, MSSQL and Clickhouse + doc#3955
PrettyWood merged 12 commits into
mainfrom
date-to-string-others

Conversation

@PrettyWood

@PrettyWoodPrettyWood commented Dec 17, 2023

Copy link
Copy Markdown
Collaborator

tackles most of #366

@max-sixtymax-sixty left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Awesome, great docs!

Comment threadweb/book/src/reference/syntax/date-time/format-specifiers.md
Comment threadweb/book/src/reference/syntax/date-time/index.md Outdated
Comment threadweb/book/src/reference/syntax/date-time/index.md Outdated
@PrettyWood
PrettyWoodforce-pushed the fix/duckdb-date-to-string branch from 3b4bfd7 to bd1dfcbCompareDecember 18, 2023 12:16
Base automatically changed from fix/duckdb-date-to-string to mainDecember 18, 2023 19:39
@snth

snth commented Dec 18, 2023

Copy link
Copy Markdown
Member

On the language design aspect (should this comment go in #366 or here since that issue is closed?), when we created from_text in #286, we decided at the time that string was too computer sciency and we went with text instead, i.e. from_text rather than from_string (see #286 (comment)).

Therefore should this function be called date.to_text rather than date.to_string?

@PrettyWood

Copy link
Copy Markdown
CollaboratorAuthor

@snth We discussed this matter recently #3913 (comment) and string has been chosen as the go to. It can always be reverted and I agree we should be consistent (either string everywhere or text everywhere)

@PrettyWoodPrettyWood changed the title feat: add date to string for MySQL, Postgres and MSSQL + docfeat: add date to string for MySQL, Postgres, MSSQL and Clickhouse + docDec 18, 2023
@max-sixty

Copy link
Copy Markdown
Member

Therefore should this function be called date.to_text rather than date.to_string?

I was the primary advocate for the from_text...

On reflection, I think that was:

  • Marginal / a bit ambitious
  • Much better suited to from_X than here, because from_X involved a big block of text creating a relation, whereas this is creating a single value of type str

So I wouldn't really advocate for it here, and happy to align on string if that's the consensus


(OK, @PrettyWood just commented, I'll still post this but echoing his sentiment)

Comment threadCHANGELOG.md Outdated
Co-authored-by: Maximilian Roos <5635139+max-sixty@users.noreply.github.com>
Comment threadweb/book/src/reference/syntax/date-time/format-specifiers.md Outdated
@snth

snth commented Dec 18, 2023

Copy link
Copy Markdown
Member

I think our earlier discussion on from_text was sensible and we shouldn't be that quick to dismiss it.

The list in #3913 (comment) is impressive but are those languages aiming at the same kind of users as us? I would argue they are more aimed at developers while we are targeting Data Analysts who often come from a less computer science heavy background.

I keep coming back to Power Query M Language because I think it's a great example of a language that's aimed at the same kind of data munging that we target. Power Query M Language has a Text module and in the Date module there are Date.FromText and Date.ToText functions.

Pandas and Polars both use a .str namespace for their text manipulation functions (see https://pandas.pydata.org/docs/user_guide/text.html#string-methods and https://pola-rs.github.io/polars/user-guide/expressions/strings/).

I'm -1 on string.

PrettyWoodand others added 3 commits December 18, 2023 21:47
@PrettyWoodPrettyWood mentioned this pull request Dec 19, 2023
@PrettyWoodPrettyWood changed the title feat: add date to string for MySQL, Postgres, MSSQL and Clickhouse + docfeat: add date.to_text for MySQL, Postgres, MSSQL and Clickhouse + docDec 19, 2023

@max-sixtymax-sixty left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking really good!

@PrettyWood

Copy link
Copy Markdown
CollaboratorAuthor

@max-sixty I let you decide if you want to merge for the release or not

@max-sixty

Copy link
Copy Markdown
Member

@max-sixty I let you decide if you want to merge for the release or not

For sure! I was waiting for you to hit the button but happy to ofc

@PrettyWood
PrettyWood merged commit fa4b48b into mainDec 19, 2023
@PrettyWood
PrettyWood deleted the date-to-string-others branch December 19, 2023 20:14
Sign up for freeto 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

@PrettyWood@snth@max-sixty