-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Docs: Fix MERGE INTO example in Getting Started #14943
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
Docs: Fix MERGE INTO example in Getting Started #14943
Conversation
| ```sql | ||
| MERGE INTO local.db.target t USING (SELECT * FROM updates) u ON t.id = u.id | ||
| WHEN MATCHED THEN UPDATE SET t.count = t.count + u.count | ||
| WHEN MATCHED THEN UPDATE SET t.data = u.data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make this truly runnable (and align with TestRoundTrip’s TODO about the getting-started doc not being runnable), should we change local.db.target → local.db.table and define updates before the MERGE?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Updated the Getting Started doc to use local.db.table instead of local.db.target and added explicit CREATE TABLE and INSERT INTO statements for the source and updates tables to make the example runnable.
|
|
||
| // Run through our Doc's Getting Started Example | ||
| // TODO Update doc example so that it can actually be run, modifications were required for this | ||
| // test suite to run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shall we also update 3.4/4.0/4.1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated. Removed the TODO comments from all Spark versions (3.4, 3.5, 4.0, 4.1).
|
Thanks @varun-lakhyani for the PR! |
Thanks for the review and merging this. |
Description
This resolves the TODO noted in
spark/v3.5/spark-runtime/src/integration/java/org/apache/iceberg/spark/TestRoundTrip.java:41.Fixed broken
MERGE INTOexample in Spark Getting Started documentation that referenced a non-existentcountcolumn.What was wrong
The original example attempted to update a
countcolumn:However, the table schema defined earlier in the guide is
(id bigint, data string), making this example invalid.Table was defined as
local.db.tablenotlocal.db.targetso updated this inMERGE INTOWhat changed
Updated the example to use the correct
datacolumn: