Fix stats/cost caching of group-referenced plan nodes#11858
Fix stats/cost caching of group-referenced plan nodes#11858findepi merged 3 commits intotrinodb:masterfrom
Conversation
core/trino-main/src/test/java/io/trino/cost/TestGetTableStatsOperations.java
Outdated
Show resolved
Hide resolved
core/trino-main/src/test/java/io/trino/metadata/CountingMetadataManagerWrapper.java
Outdated
Show resolved
Hide resolved
core/trino-main/src/test/java/io/trino/metadata/CountingMetadataManagerWrapper.java
Outdated
Show resolved
Hide resolved
.../trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergGetTableStatsOperations.java
Outdated
Show resolved
Hide resolved
core/trino-main/src/main/java/io/trino/cost/CachingStatsProvider.java
Outdated
Show resolved
Hide resolved
|
@lxynov i posted a few comments, i hope it won't be hard to accommodate to them. Please let me know when you're done. |
|
@findepi Sure, I just addressed them. Thanks! |
core/trino-main/src/main/java/io/trino/connector/InternalMetadataProvider.java
Outdated
Show resolved
Hide resolved
core/trino-main/src/main/java/io/trino/connector/InternalMetadataProvider.java
Outdated
Show resolved
Hide resolved
core/trino-main/src/test/java/io/trino/cost/TestGetTableStatsOperations.java
Outdated
Show resolved
Hide resolved
core/trino-main/src/test/java/io/trino/cost/TestGetTableStatsOperations.java
Outdated
Show resolved
Hide resolved
.../trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergGetTableStatsOperations.java
Outdated
Show resolved
Hide resolved
.../trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergGetTableStatsOperations.java
Outdated
Show resolved
Hide resolved
|
@findinpath @findepi Thanks for the review! I've addressed the comments. |
There was a problem hiding this comment.
don't initialize test resources before test class is actually being run by the test runner
init should be done in a @BeforeClass method
for this, extend from AbstractTestQueryFramework and it will do both setup and cleanup for you.
There was a problem hiding this comment.
Good point. AbstractTestQueryFramework is inside trino-testing module which trino-main doesn't have dependency on. So I used BasePlanTest. It also helps close the query runner. Please let me know if it works.
There was a problem hiding this comment.
BasePlanTest is not the right class to extends from (we're not testing plans here, after all).
Move the test to trino-tests instead.
There was a problem hiding this comment.
same here --
don't initialize test resources before test class is actually being run by the test runner
init should be done in a @BeforeClass method
for this, extend from AbstractTestQueryFramework and it will do both setup and cleanup for you.
...o-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergGetTableStatisticsOperations.java
Outdated
Show resolved
Hide resolved
...o-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergGetTableStatisticsOperations.java
Outdated
Show resolved
Hide resolved
...o-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergGetTableStatisticsOperations.java
Outdated
Show resolved
Hide resolved
...o-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergGetTableStatisticsOperations.java
Outdated
Show resolved
Hide resolved
|
@findepi Thanks for the review! Addressed the comments. |
|
Rebased to latest master. |
4 participants
Description
Fix/improvement.
Engine.
Reduce Iceberg metadata processing during cost-based query optimizations.
Related issues, pull requests, and links
#11708
#8659
Documentation
(x) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.
Release notes
( ) No release notes entries required.
(x) Release notes entries required with the following suggested text: