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

Add link to Databricks type support in Delta Lake connector docs #14506

Merged
merged 3 commits into from
Oct 14, 2022

Conversation

jhlodin
Copy link
Contributor

@jhlodin jhlodin commented Oct 6, 2022

Description

The list of supported types in the Delta Lake connector is directly affected by what's supported in the Delta protocol. This PR links out to the Delta Transaction Log protocol's specification for more information about data type support.

Non-technical explanation

Provide link out to the Delta protocol documentation for more information about supported types in Delta Lake.

Release notes

(x) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label Oct 6, 2022
@jhlodin jhlodin requested review from ebyhr, mosabua and hashhar October 6, 2022 21:45
@github-actions github-actions bot added the docs label Oct 7, 2022
@@ -294,6 +294,10 @@ writing data. Data types may not map the same way in both directions between
Trino and the data source. Refer to the following sections for type mapping in
each direction.

See the `Databricks data types documentation
<https://docs.databricks.com/spark/latest/spark-sql/language-manual/sql-ref-datatypes.html>`_
for more information about supported types.
Copy link
Member

Choose a reason for hiding this comment

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

The databaricks docs won't know which types are supported by Trino

Copy link
Member

Choose a reason for hiding this comment

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

about available data types in Delta Lake.

Copy link
Member

Choose a reason for hiding this comment

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

the paragraphs says "supported" and doesn't indicate it's not about types supported by something else than Trino

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would "..about types supported by the Delta Lake connector" be specific enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I prefer Manfred's suggestion of "...for more information about supported data types in Delta Lake" since it's not about Trino, this connector, or any other Trino-specific component.

Copy link
Member

Choose a reason for hiding this comment

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

If I can nit-pick a little more, could be even more clear that we're talking about the Delta Lake format and not the connector:

"for more information about supported data types in the Delta Lake table format specification."

Either way, this is not the right link. I think you want: https://github.com/delta-io/delta/blob/master/PROTOCOL.md#primitive-types

The Databricks link includes all types that are in the SparkSQL language but not all of them are supported in DL

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reworded and updated the link to point to the Delta protocol specification.

@findepi findepi requested a review from alexjo2144 October 7, 2022 11:19
@jhlodin jhlodin force-pushed the jl/delta-databricks-types branch from 2ccc693 to 0aa2567 Compare October 12, 2022 18:46
@jhlodin jhlodin requested a review from findepi October 12, 2022 18:46
@findepi
Copy link
Member

findepi commented Oct 13, 2022

CI #14618

@findepi
Copy link
Member

findepi commented Oct 13, 2022

CI #14618 (second time)

@findepi findepi merged commit 80a70e4 into trinodb:master Oct 14, 2022
@github-actions github-actions bot added this to the 401 milestone Oct 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants