Milesian111 commented on code in PR #29033:
URL: https://github.com/apache/flink/pull/29033#discussion_r3881266366
##########
flink-python/pyflink/dataframe/dataframe.py:
##########
@@ -542,6 +542,85 @@ def drop_duplicates(
distinct = drop_duplicates
unique = drop_duplicates
+ # ======================== Slicing ========================
+
+ @PublicEvolving()
+ def limit(self, n: int) -> "DataFrame":
+ """
+ Keep at most the first ``n`` rows.
+
+ This is a lazy transformation. Row order is deterministic only when
the underlying table
+ has an explicit ordering.
Review Comment:
Thanks for the suggestions.
I will clarify that a lazy transformation only builds and returns a new
DataFrame plan without starting a Flink job. Execution is triggered later by
an action such as collect() or to_pandas().
Ordering support is already tracked separately by FLINK-40421. FLIP-591
specifies DataFrame.sort(), which will delegate to the existing
Table.order_by() API. To avoid introducing two overlapping public
DataFrame APIs and to keep this PR scoped to FLINK-40422, I suggest leaving
sorting support to FLINK-40421. Once implemented, users will be able to
write:
df.sort("ts", descending=True).limit(100)
I will also reword the ordering note to clarify that, without explicit
sorting, the selected rows and their order are unspecified. Changes to the
underlying table content may naturally change the result, and fully stable
ordering also requires ordering keys that uniquely determine row order.
Would clarifying the lazy semantics and referencing FLINK-40421 address the
concern?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
