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

Spark 3.5: Fix Javadoc in ColumnarBatchUtil #12058

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

huaxingao
Copy link
Contributor

@huaxingao huaxingao commented Jan 22, 2025

@huaxingao huaxingao changed the title Fix Javadoc in ColumnarBatchUtil Spark 3.5: Fix Javadoc in ColumnarBatchUtil Jan 22, 2025
@huaxingao
Copy link
Contributor Author

@@ -31,10 +31,13 @@ public class ColumnarBatchUtil {

private ColumnarBatchUtil() {}

// spotless:off
/**
Copy link
Contributor

Choose a reason for hiding this comment

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

I just realized the example actually doesn't fit anymore as we only compute the mapping, not the isDeleted array. Also, what if we use <pre> blocks to avoid disabling Spotless?

/**
 * Builds a row ID mapping inside a batch to skip deleted rows.
 *
 * <pre>
 * Initial state
 * Data values: [v0, v1, v2, v3, v4, v5, v6, v7]
 * Row ID mapping: [0, 1, 2, 3, 4, 5, 6, 7]
 *
 * Apply position deletes
 * Position deletes: 2, 6
 * Row ID mapping: [0, 1, 3, 4, 5, 7, -, -] (6 live records)
 *
 * Apply equality deletes
 * Equality deletes: v1, v2, v3
 * Row ID mapping: [0, 4, 5, 7, -, -, -, -] (4 live records)
 * </pre>
 *
 * @param columnVectors the array of column vectors for the batch
 * @param deletes the delete filter containing delete information
 * @param rowStartPosInBatch the starting position of the row in the batch
 * @param batchSize the size of the batch
 * @return the mapping array and the number of live rows, or {@code null} if nothing is deleted
*/

I think this example would be more descriptive as it is clear what the data values are and which records are removed by equality deletes.

Copy link
Contributor

@aokolnychyi aokolnychyi Jan 23, 2025

Choose a reason for hiding this comment

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

Let's also give a similar example in the buildIsDeleted method below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Thanks!

@github-actions github-actions bot removed the INFRA label Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants