[AMORO-2417] Support partition filter on optimizing plan #2436
Conversation
github-actions
bot
added
module:core
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #2436 +/- ##
============================================
- Coverage 52.93% 52.91% -0.02%
- Complexity 4420 4423 +3
============================================
Files 534 535 +1
Lines 30578 30618 +40
Branches 2994 2997 +3
============================================
+ Hits 16185 16202 +17
- Misses 13097 13132 +35
+ Partials 1296 1284 -12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| public void testUnkeyedConvertPartitionStructLikeToDataFilter() throws IOException { | ||
| Assume.assumeTrue(getArcticTable().isUnkeyedTable()); | ||
| UnkeyedTable table = getArcticTable().asUnkeyedTable(); | ||
| logger.info("Partition Spec:" + getArcticTable().spec()); |
There was a problem hiding this comment.
I'm also not very clear about the necessity of printing logs here.
There was a problem hiding this comment.
Without this log, I can't tell which test used which partition spec. For example, if an error occurs in a case, we will not be able to tell which transform case has the error.
There was a problem hiding this comment.
How about changing the test parameters rather than printing the log?
core/src/test/java/com/netease/arctic/utils/TestKeyedExpressionUtil.java
Outdated
Show resolved
Hide resolved
...server/src/main/java/com/netease/arctic/server/optimizing/scan/KeyedTableFileScanHelper.java
Outdated
Show resolved
Hide resolved
…' into optimizing-plan-partition-filter
ams/server/src/main/java/com/netease/arctic/server/optimizing/plan/OptimizingPlanner.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/netease/arctic/utils/TablePropertyUtil.java
Outdated
Show resolved
Hide resolved
| public void testUnkeyedConvertPartitionStructLikeToDataFilter() throws IOException { | ||
| Assume.assumeTrue(getArcticTable().isUnkeyedTable()); | ||
| UnkeyedTable table = getArcticTable().asUnkeyedTable(); | ||
| logger.info("Partition Spec:" + getArcticTable().spec()); |
There was a problem hiding this comment.
I'm also not very clear about the necessity of printing logs here.
ams/server/src/main/java/com/netease/arctic/server/optimizing/scan/TableFileScanHelper.java
Show resolved
Hide resolved
Fixed in the new commit. |
| * @param partitions the collection of partition data | ||
| * @return data filter converted from partition data | ||
| */ | ||
| public static Expression convertPartitionDataToDataFilter( |
There was a problem hiding this comment.
Both methods are named convertPartitionDataToDataFilter. I think they can be distinguished.
| * @param partition the partition data | ||
| * @return data filter converted from partition data | ||
| */ | ||
| public static Expression convertPartitionDataToDataFilter( |
There was a problem hiding this comment.
add @VisibleForTesting? This method is not referenced externally
|
Sorry, the PR has been merged into the master branch. Thanks for your review. Looks like some minor imperfections that maybe could be ignored? |
ok! |
* support partition filter on scan helper * apply spotless * fix comment * fix comment * fix comment * fix comment * fix comment * change test param --------- Co-authored-by: wangtaohz <103108928+wangtaohz@users.noreply.github.com>
Why are the changes needed?
Close #2417.
Brief change log
Pair<Integer, StructLike>to represent the partition instead of a String.ExpressionUtilused to convert partition data to a data filter and apply it as a partition filter.How was this patch tested?
Add some test cases that check the changes thoroughly including negative and positive cases if possible
Add screenshots for manual tests if appropriate
Run test locally before making a pull request
Documentation