You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "ulysses-you (via GitHub)" <gi...@apache.org> on 2024/10/10 01:34:04 UTC
[PR] [spark] Support push down limit for primary key table [paimon]
ulysses-you opened a new pull request, #4299:
URL: https://github.com/apache/paimon/pull/4299
<!-- Please specify the module before the PR name: [core] ... or [flink] ... -->
### Purpose
<!-- Linking this pull request to the issue -->
For the primary key table, we still have a chance to push down limit if the split is raw convertible. This pr relaxes the push down limit condition to support primary key table.
<!-- What is the purpose of the change -->
### Tests
<!-- List UT and IT cases to verify this change -->
add test
### API and Format
<!-- Does this change affect API or storage format -->
no
### Documentation
<!-- Does this change introduce a new feature -->
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Support push down limit for primary key table [paimon]
Posted by "ulysses-you (via GitHub)" <gi...@apache.org>.
ulysses-you commented on PR #4299:
URL: https://github.com/apache/paimon/pull/4299#issuecomment-2404062040
thank you @wwj6591812 , addressed comments
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core][spark] Support push down limit for primary key table [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #4299:
URL: https://github.com/apache/paimon/pull/4299#issuecomment-2406468518
+1
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core][spark] Support push down limit for primary key table [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #4299:
URL: https://github.com/apache/paimon/pull/4299
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [spark] Support push down limit for primary key table [paimon]
Posted by "wwj6591812 (via GitHub)" <gi...@apache.org>.
wwj6591812 commented on code in PR #4299:
URL: https://github.com/apache/paimon/pull/4299#discussion_r1794592810
##########
paimon-core/src/main/java/org/apache/paimon/table/source/DataTableBatchScan.java:
##########
@@ -96,6 +96,27 @@ private StartingScanner.Result applyPushDownLimit(StartingScanner.Result result)
long scannedRowCount = 0;
SnapshotReader.Plan plan = ((ScannedResult) result).plan();
List<DataSplit> splits = plan.dataSplits();
+ if (splits.isEmpty()) {
+ return result;
+ }
+
+ // We first add the rawConvertible split to avoid merging, and if the row count
+ // is still less than limit number, then add split which is not rawConvertible.
+ splits.sort(
+ (x, y) -> {
+ if (x.rawConvertible() && y.rawConvertible()) {
+ return 0;
+ } else if (x.rawConvertible()) {
+ return -1;
+ } else {
+ return 1;
+ }
+ });
Review Comment:
Add a blank line.
##########
paimon-core/src/main/java/org/apache/paimon/table/source/DataTableBatchScan.java:
##########
@@ -96,6 +96,27 @@ private StartingScanner.Result applyPushDownLimit(StartingScanner.Result result)
long scannedRowCount = 0;
SnapshotReader.Plan plan = ((ScannedResult) result).plan();
List<DataSplit> splits = plan.dataSplits();
+ if (splits.isEmpty()) {
+ return result;
+ }
+
+ // We first add the rawConvertible split to avoid merging, and if the row count
+ // is still less than limit number, then add split which is not rawConvertible.
+ splits.sort(
+ (x, y) -> {
+ if (x.rawConvertible() && y.rawConvertible()) {
+ return 0;
+ } else if (x.rawConvertible()) {
+ return -1;
+ } else {
+ return 1;
+ }
+ });
+ // fast return if there is no rawConvertible split
Review Comment:
Why you can ignore pushDownLimit if there is no rawConvertible split?
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [spark] Support push down limit for primary key table [paimon]
Posted by "wwj6591812 (via GitHub)" <gi...@apache.org>.
wwj6591812 commented on PR #4299:
URL: https://github.com/apache/paimon/pull/4299#issuecomment-2403929078
I think this is a good point!
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [spark] Support push down limit for primary key table [paimon]
Posted by "ulysses-you (via GitHub)" <gi...@apache.org>.
ulysses-you commented on code in PR #4299:
URL: https://github.com/apache/paimon/pull/4299#discussion_r1794680201
##########
paimon-core/src/main/java/org/apache/paimon/table/source/DataTableBatchScan.java:
##########
@@ -96,6 +96,27 @@ private StartingScanner.Result applyPushDownLimit(StartingScanner.Result result)
long scannedRowCount = 0;
SnapshotReader.Plan plan = ((ScannedResult) result).plan();
List<DataSplit> splits = plan.dataSplits();
+ if (splits.isEmpty()) {
+ return result;
+ }
+
+ // We first add the rawConvertible split to avoid merging, and if the row count
+ // is still less than limit number, then add split which is not rawConvertible.
+ splits.sort(
+ (x, y) -> {
+ if (x.rawConvertible() && y.rawConvertible()) {
+ return 0;
+ } else if (x.rawConvertible()) {
+ return -1;
+ } else {
+ return 1;
+ }
+ });
+ // fast return if there is no rawConvertible split
Review Comment:
It follows previous behavior, if all splits are not rawConvertible, then we add all splits as the final plan. The semantics of pushDownLimit is just a best effort, the engine will do limit again.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core][spark] Support push down limit for primary key table [paimon]
Posted by "wwj6591812 (via GitHub)" <gi...@apache.org>.
wwj6591812 commented on code in PR #4299:
URL: https://github.com/apache/paimon/pull/4299#discussion_r1794995815
##########
paimon-core/src/main/java/org/apache/paimon/table/source/DataTableBatchScan.java:
##########
@@ -96,6 +96,27 @@ private StartingScanner.Result applyPushDownLimit(StartingScanner.Result result)
long scannedRowCount = 0;
SnapshotReader.Plan plan = ((ScannedResult) result).plan();
List<DataSplit> splits = plan.dataSplits();
+ if (splits.isEmpty()) {
+ return result;
+ }
+
+ // We first add the rawConvertible split to avoid merging, and if the row count
+ // is still less than limit number, then add split which is not rawConvertible.
+ splits.sort(
+ (x, y) -> {
+ if (x.rawConvertible() && y.rawConvertible()) {
+ return 0;
+ } else if (x.rawConvertible()) {
+ return -1;
+ } else {
+ return 1;
+ }
+ });
+ // fast return if there is no rawConvertible split
Review Comment:
OK.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [spark] Support push down limit for primary key table [paimon]
Posted by "wwj6591812 (via GitHub)" <gi...@apache.org>.
wwj6591812 commented on PR #4299:
URL: https://github.com/apache/paimon/pull/4299#issuecomment-2403930621
I think this is a good optimize.
Change title from "Spark" to "Core"，because Flink also use optimize.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core][spark] Support push down limit for primary key table [paimon]
Posted by "ulysses-you (via GitHub)" <gi...@apache.org>.
ulysses-you commented on PR #4299:
URL: https://github.com/apache/paimon/pull/4299#issuecomment-2404129084
cc @JingsongLi thank you
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core][spark] Support push down limit for primary key table [paimon]
Posted by "YannByron (via GitHub)" <gi...@apache.org>.
YannByron commented on PR #4299:
URL: https://github.com/apache/paimon/pull/4299#issuecomment-2423848297
Link to https://github.com/apache/paimon/issues/2404.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org