Skip to content

Conversation

@tomtongue
Copy link
Contributor

This change adds how to run CTAS using DataFrame V2 API with specifying a table location. As a background, some iceberg users ask how to create an iceberg table by DataFrame V2 with specifying a table location, and it's not easy to find the information about the parameter.

@github-actions github-actions bot added the docs label Feb 3, 2023
Copy link
Member

@szehon-ho szehon-ho left a comment

Choose a reason for hiding this comment

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

This is great to document, I left a comment suggestion below.

.createOrReplace()
```

You can specify an Iceberg table location such as the `LOCATION` clause in SQL by add the `location` paramter to the `tableProperty`:
Copy link
Member

Choose a reason for hiding this comment

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

Nit: I know there's a bad example above, but typically we avoid 'you' or 'me' in documentation. you can see other reviews, for example: #4301 (comment)

Also, I'm not sure why we reference "LOCATION" clause in SQL, which is documented elsewhere.

How about:
The Iceberg table location can also be specified by the `location` table configuration

Let me know what you think

Copy link
Contributor Author

@tomtongue tomtongue Feb 27, 2023

Choose a reason for hiding this comment

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

Thanks for reviewing this and the great suggenstion, Szehon! The following your suggestion is simpler and makes sense. Based on your suggestion, I updated a bit; ... table configuration to table property. How about this?

The Iceberg table location can also be specified by the `location` table property.

For your comment;

Nit: I know there's a bad example above, but typically we avoid 'you' or 'me' in documentation. you can see other reviews, for example: #4301 (comment)

Thanks for pointing out, I understand it.

Also, I'm not sure why we reference "LOCATION" clause in SQL, which is documented elsewhere.

I tried saying the tableProperty("location", "...") is the same as LOCATION in SQL, but the suggestion is better.

Copy link
Member

Choose a reason for hiding this comment

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

Yes that will work. Thanks!

@tomtongue
Copy link
Contributor Author

tomtongue commented Feb 28, 2023

Thanks for reviewing! I updated the line based on the discussion. It would be great if you review it.

@szehon-ho szehon-ho merged commit a725dad into apache:master Feb 28, 2023
@szehon-ho
Copy link
Member

Merged, thanks @tomtongue !

@tomtongue tomtongue deleted the doc-dfv2-location branch October 26, 2023 01:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants