Add documentation for Iceberg support in PrestoCPP #24741

agrawalreetika · 2025-03-17T18:46:17Z

Description

Add documentation for Iceberg support in PrestoCPP

Motivation and Context

Add documentation for Iceberg support in PrestoCPP

Impact

Iceberg documentation improvement

Test Plan

Contributor checklist

Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
Documented new properties (with its default value), SQL syntax, functions, or other functionality.
If release notes are required, they follow the release notes guidelines.
Adequate tests were added if applicable.
CI passed.

Release Notes

== NO RELEASE NOTE ==

auden-woolfson

lgtm

steveburnett

Thanks for the doc! Some nits and suggestions, nothing major.

presto-docs/src/main/sphinx/connector/iceberg.rst

presto-docs/src/main/sphinx/presto-cpp.rst

presto-docs/src/main/sphinx/connector/iceberg.rst

yingsu00

Thanks @agrawalreetika! I see PrestoCPP and Presto C++ were mixed. Let's unify them to Presto C++

yingsu00 · 2025-03-18T01:24:18Z

presto-docs/src/main/sphinx/connector/iceberg.rst

                                                        are ``HIVE``, ``HADOOP``, and ``NESSIE`` and ``REST``.

-``iceberg.hadoop.config.resources``                     The path(s) for Hadoop configuration resources.
+``iceberg.hadoop.config.resources``                     The path(s) for Hadoop configuration resources.                                                  Yes


The Presto C++ Support value is missing

yingsu00 · 2025-03-18T01:26:32Z

presto-docs/src/main/sphinx/connector/iceberg.rst

                                                        as a join with the data of the equality delete files.

-``iceberg.enable-parquet-dereference-pushdown``         Enable parquet dereference pushdown.                          ``true``
+``iceberg.enable-parquet-dereference-pushdown``         Enable parquet dereference pushdown.                          ``true``                           Yes


The Presto C++ Support value is missing

yingsu00 · 2025-03-18T01:29:00Z

presto-docs/src/main/sphinx/connector/iceberg.rst

                                                        statistics file cache.
-======================================================= ============================================================= ============
+======================================================= ============================================================= ================================== =================== =========================================

 Table Properties


Can you please also add a section for the C++ support for Table Properties? Just saying it's not suported because write is not implemented yet

yingsu00 · 2025-03-18T01:36:20Z

presto-docs/src/main/sphinx/connector/iceberg.rst

@@ -1511,13 +1587,23 @@ schema evolution, such as adding, dropping, and renaming columns. With schema
 evolution, users can evolve a table schema with SQL after enabling the Presto
 Iceberg connector.

+Presto C++ Support


This is for Partition Column Transform. Could you please add Presto C++ support as well? Thanks!

yingsu00 · 2025-03-18T01:44:07Z

presto-docs/src/main/sphinx/presto-cpp.rst


-* Supports reading and writing of DWRF and PARQUET file formats, supports reading ORC file format.
+Hive Connector - Iceberg Table Support


I think the following structure would be clearer
Supported Connectors

Hive connector

Hive table support

Iceberg table support

TPCH connector

....

Fuzzer connector

agrawalreetika · 2025-03-18T09:15:34Z

@steveburnett @yingsu00 Thanks for your review. I have made the changes based on your comments. Please have a look at your convenience.

hantangwangd

Thanks for the doc, it's very helpful. Only a couple of nit and little thing.

hantangwangd · 2025-03-18T09:50:27Z

presto-docs/src/main/sphinx/connector/iceberg.rst

+
+``SELECT``                     Yes           Yes          Read is supported in Presto C++ including those with positional delete files.
+
+``INSERT INTO``                Yes           No


nit: duplicate with line 1155.

hantangwangd · 2025-03-18T10:01:30Z

presto-docs/src/main/sphinx/presto-cpp.rst

+
+* Only read operations are supported for Iceberg tables.
+
+* The Iceberg connector supports both V1 and V2 tables, including those with positional delete files, but does not support equality delete files.


From a reader's perspective, I'm a little confused about the Iceberg connector here, do you think it's more appropriate to use something like Hive connector for Iceberg or something else?

So we need to create an iceberg catalog in Prestissimo as well for accessing Iceberg tables but the underline implementation on Velox is via Hive connector itself which is why I thought of keeping Hive Connector as main header and then different table support.

I am open for suggestions here whatever we all think is more clear.
@yingsu00 What's your opinion on this?

Sorry, I didn't describe it clearly. What I mean is, the content structure Hive connector/Iceberg Table Support is no problem, just the words Iceberg connector here may bring a little confusion. If I were a newer, I might wonder whether there is an Iceberg connector for presto C++ that I haven't noticed.

@agrawalreetika : If we use Java side Iceberg connector or catalog code, then its better to call it Iceberg Connector. The reuse of HiveConnector is an internal detail imo.

I think we could call it Iceberg connector because it's in Presto C++ point of view, not Velox point of view. But it would be helpful to explain that the Presto C++ Iceberg connector and catalog is backed up by Velox HiveConnector.

@hantangwangd Thanks for the clarification. So here as @aditi-pandit & @yingsu00 mentioned from Presto C++ point of view it's going to be a different iceberg connector with something like this -

connector.name=iceberg

That is why I thought calling it iceberg connector would be better in the Presto C++ document.

@hantangwangd @aditi-pandit @yingsu00 Please take a look at the document, its already updated based on the points we discussed here.

Got the point, thank you all for your perspective @aditi-pandit @yingsu00 @agrawalreetika.

steveburnett

Thanks for the update! I pulled the updated branch, ran a new local doc build, and reviewed again.

I found a few nits that aren't directly related to your new content here that I hope we can fix here, I don't think that fixing them will add a lot of work or delay the PR.

presto-docs/src/main/sphinx/connector/iceberg.rst

presto-docs/src/main/sphinx/presto-cpp.rst

steveburnett

LGTM! (docs)

Pull updated branch, new local doc build, looks good. Thank you!

agrawalreetika requested a review from yingsu00 March 17, 2025 18:46

agrawalreetika self-assigned this Mar 17, 2025

agrawalreetika requested review from steveburnett, elharo, hantangwangd, ZacBlanco and a team as code owners March 17, 2025 18:46

agrawalreetika requested a review from jaystarshot March 17, 2025 18:46

prestodb-ci added the from:IBM PR from IBM label Mar 17, 2025

prestodb-ci requested review from a team, nmahadevuni and auden-woolfson and removed request for a team March 17, 2025 18:46

github-actions bot added the docs label Mar 17, 2025

prestodb-ci requested a review from a team March 17, 2025 18:49

auden-woolfson previously approved these changes Mar 17, 2025

View reviewed changes

steveburnett requested changes Mar 17, 2025

View reviewed changes

yingsu00 reviewed Mar 18, 2025

View reviewed changes

agrawalreetika dismissed auden-woolfson’s stale review via aa5b2d3 March 18, 2025 09:09

agrawalreetika force-pushed the iceberg-native-doc branch 2 times, most recently from aa5b2d3 to 76c9f55 Compare March 18, 2025 09:14

hantangwangd reviewed Mar 18, 2025

View reviewed changes

steveburnett requested changes Mar 18, 2025

View reviewed changes

Add documentation for Iceberg support in PrestoCPP

ae78e53

agrawalreetika force-pushed the iceberg-native-doc branch from 76c9f55 to ae78e53 Compare March 18, 2025 15:07

hantangwangd approved these changes Mar 19, 2025

View reviewed changes

steveburnett approved these changes Mar 19, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add documentation for Iceberg support in PrestoCPP #24741

Add documentation for Iceberg support in PrestoCPP #24741

agrawalreetika commented Mar 17, 2025

auden-woolfson left a comment

steveburnett left a comment

yingsu00 left a comment •

edited

Loading

yingsu00 Mar 18, 2025

yingsu00 Mar 18, 2025

yingsu00 Mar 18, 2025

yingsu00 Mar 18, 2025

yingsu00 Mar 18, 2025

agrawalreetika commented Mar 18, 2025

hantangwangd left a comment

hantangwangd Mar 18, 2025

hantangwangd Mar 18, 2025

agrawalreetika Mar 18, 2025 •

edited

Loading

hantangwangd Mar 18, 2025

aditi-pandit Mar 18, 2025

yingsu00 Mar 18, 2025 •

edited

Loading

agrawalreetika Mar 19, 2025

hantangwangd Mar 19, 2025

steveburnett left a comment

steveburnett left a comment


		* Supports reading and writing of DWRF and PARQUET file formats, supports reading ORC file format.
		Hive Connector - Iceberg Table Support


		``SELECT`` Yes Yes Read is supported in Presto C++ including those with positional delete files.

		``INSERT INTO`` Yes No


		* Only read operations are supported for Iceberg tables.

		* The Iceberg connector supports both V1 and V2 tables, including those with positional delete files, but does not support equality delete files.

Add documentation for Iceberg support in PrestoCPP #24741

Are you sure you want to change the base?

Add documentation for Iceberg support in PrestoCPP #24741

Conversation

agrawalreetika commented Mar 17, 2025

Description

Motivation and Context

Impact

Test Plan

Contributor checklist

Release Notes

auden-woolfson left a comment

Choose a reason for hiding this comment

steveburnett left a comment

Choose a reason for hiding this comment

yingsu00 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agrawalreetika commented Mar 18, 2025

hantangwangd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agrawalreetika Mar 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yingsu00 Mar 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

steveburnett left a comment

Choose a reason for hiding this comment

steveburnett left a comment

Choose a reason for hiding this comment

yingsu00 left a comment •

edited

Loading

agrawalreetika Mar 18, 2025 •

edited

Loading

yingsu00 Mar 18, 2025 •

edited

Loading