Conversation
This commit refactors duplicate functions to improve code structure and adds a new `requirePositiveDuplicates` check. Introduced HTML documentation for `duplicate` and `duplicateRows` APIs.
zaleslaw
marked this pull request as ready for review
October 1, 2026 13:58
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1962
duplicateandduplicateRowshad no KDocs, no dedicated tests, and a website page without data examples. While covering them, we found that the four functions handledn <= 0in four different ways, one of which silently lost data. This PR fixes that and documents the contract.core/.../impl/api/duplicate.ktn > 0check at the entry of every overloadcore/.../api/duplicate.ktcore/.../documentation/DocumentationUrls.ktduplicatepagecore/.../test/.../api/duplicate.ktDuplicateTestssamples/.../api/DuplicateSamples.kt,samples/build.gradle.kts:samplesdocs/StardustDocs/topics/duplicate.md,_shadow_resources.md,resources/api/duplicate/docs/StardustDocs/topics/appendDuplicate.mdduplicateKDoc, page and tests use one dataset (
name/age: Alice 15, Bob 20, Charlie 30).A, A, B, B)duplicateRowskeeps column types;DataRow.duplicatemakes a nullable column non-nullable when the row value is notnullDataFrame.duplicateFrameColumnwithncopies;.concat()gives all rows, then all rows againnboundsn = 1keeps one copy of everything;n <= 0throws in all four functionsNote for the reviewer
Bugs fixed in the implementation (all four functions now throw
IllegalArgumentException: Number of duplicates must be greater than 0, but was <n>):duplicateRows(n) { condition }withn = 0or a negativensilently removed every matching row:df.duplicateRows(-1) { age > 18 }returned onlyAliceand droppedBobandCharlie, with no error. Pinned byduplicateRows with a condition rejects n that is not positive instead of removing matching rows.DataFrame.duplicate(n)andDataRow.duplicate(n)with a negativenfailed withIllegalArgumentException: Illegal Capacity: -1, an accidental message fromArrayList. Withn = 0they returned an emptyFrameColumnor an emptyDataFrame. Pinned byDataFrame duplicate rejects n that is not positiveandDataRow duplicate rejects n that is not positive.duplicateRows(n)checkednonce per column, so a dataframe without columns acceptedn = 0without an error, while one with columns threw. Pinned byduplicateRows rejects n that is not positive.All four tests failed before the fix.
The fix: one
requirePositiveDuplicates(n)at the entry of each impl function, with the message the unconditional overload already used. For the overload with a condition, the check sits in the@PublishedApiduplicateRowsImpl(n, indices), not in the publicinlinebody, so code compiled against the old inline function gets the check too. Signatures are unchanged, so the.apidump is unchanged as well.Behaviour change for callers:
duplicate(0),DataRow.duplicate(0)andduplicateRows(0) { … }now throw instead of returning an empty or reduced result. We chose to throw forn = 0too, to matchduplicateRows(n), which already threw.Found and left as is:
DataRow.duplicatetakes each column's nullability from the row value:Int?becomesIntwhen the value is notnull. This is now documented and pinned by a test.Beyond the ticket:
:coreto:samples, because rendered result tables needDataFrameSampleHelper.appendDuplicate.mdis updated: its link text said "duplicate selected rows", butduplicatealso repeats all rows and whole dataframes.DocumentationUrls(#duplicaterows,#duplicate-on-a-datarow,#duplicate-on-a-dataframe) come from the new headings of this page.