Skip to content

Document session properties specific for the Delta Lake connector#17331

Merged
mosabua merged 3 commits intotrinodb:masterfrom
findinpath:docs/delta-session-properties
Jun 16, 2023
Merged

Document session properties specific for the Delta Lake connector#17331
mosabua merged 3 commits intotrinodb:masterfrom
findinpath:docs/delta-session-properties

Conversation

@findinpath
Copy link
Copy Markdown
Contributor

Description

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:

@cla-bot cla-bot bot added the cla-signed label May 3, 2023
@findinpath findinpath self-assigned this May 3, 2023
@github-actions github-actions bot added the docs label May 3, 2023
@findinpath findinpath requested a review from mosabua May 3, 2023 08:33
@findinpath findinpath force-pushed the docs/delta-session-properties branch from 7a5c95a to 3769f9d Compare May 3, 2023 08:37
@cla-bot cla-bot bot added the cla-signed label May 3, 2023
@findinpath findinpath force-pushed the docs/delta-session-properties branch from 3769f9d to 0f039ea Compare May 8, 2023 16:59
@findinpath findinpath requested a review from ebyhr May 9, 2023 07:10
Copy link
Copy Markdown
Member

@mosabua mosabua left a comment

Choose a reason for hiding this comment

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

Need to look in more detail .. but I have my doubts that it makes sense to have this list that is a mix of different topics Parquet reader, stats, ..

Why dont we just document the catalog session properties with the catalog properties they belong to? And with the relevant separate sections for different topics...

@m57lyra is working on this resstructuring for all the object storage connectors so we should sync up on how to best proceed

@findinpath findinpath force-pushed the docs/delta-session-properties branch from 0f039ea to d8e536d Compare May 10, 2023 05:02
@findinpath
Copy link
Copy Markdown
Contributor Author

I have my doubts that it makes sense to have this list that is a mix of different topics Parquet reader, stats, ..

If the properties are available in the session, they should appear in the documentation.
A potential Trino user shouldn't need to actually run Trino/ go through the code to discover them.

@mosabua
Copy link
Copy Markdown
Member

mosabua commented May 11, 2023

I have my doubts that it makes sense to have this list that is a mix of different topics Parquet reader, stats, ..

If the properties are available in the session, they should appear in the documentation. A potential Trino user shouldn't need to actually run Trino/ go through the code to discover them.

You misunderstood me. What I am saying is we should NOT have them documented as a list of session properties for various topics. Instead we should do what we do everywhere else in the docs and document the catalog property (or global property) and the session property in the same place. So a table of Parquet config properties should list all catalog properties related to Parquet reading and writing and such .. and mention the name of the catalog session property in the same section.

Like https://trino.io/docs/current/admin/properties-query-management.html for global properties

Or https://trino.io/docs/current/connector/hudi.html#general-configuration where in fact the Parquet ones that work across connectors will be pulled out into the separate Parquet page (or fragment) .. @m57lyra is working on that

@findinpath findinpath force-pushed the docs/delta-session-properties branch from 70f79d3 to 4baeac9 Compare May 14, 2023 16:01
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.

Relevant commit 6eb42f2
cc @raunaqmorarka

@findinpath findinpath requested a review from mosabua May 14, 2023 16:02
@findinpath findinpath force-pushed the docs/delta-session-properties branch from 4baeac9 to 56127b7 Compare June 16, 2023 08:05
@findinpath findinpath requested a review from m57lyra June 16, 2023 08:06
@findinpath
Copy link
Copy Markdown
Contributor Author

@mosabua / @m57lyra CTAL over this docs PR?

@mosabua mosabua merged commit 7d9265b into trinodb:master Jun 16, 2023
@github-actions github-actions bot added this to the 420 milestone Jun 16, 2023
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.

2 participants