singhpk234 commented on code in PR #13400:
URL: https://github.com/apache/iceberg/pull/13400#discussion_r2553759572
##########
core/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java:
##########
@@ -594,7 +594,11 @@ default boolean shouldPlanTableScanAsync(TableScan
tableScan) {
}
protected PlanningBehavior planningBehavior() {
- return new PlanningBehavior() {};
+ return this.planningBehavior == null ? new PlanningBehavior() {} :
planningBehavior;
+ }
+
+ protected void setPlanningBehavior(PlanningBehavior behavior) {
+ this.planningBehavior = behavior;
Review Comment:
> Are we not able to override the planning behavior just once in a manner
which will force the client to exercise all the paths
It is possible ! but having a setter simplifies the test specially since we
have tests with time travel and scans with multiple which would make assertions
lengthy, IMHO
Approach 1 (check on table name prefix or some property, not preferred ?):
```java
default boolean shouldPlanTableScanAsync(TableScan tableScan) {
return tableScan.table.name.startsWith("async");
}
}
```
Approach 2 (based on fileScanTasks count decide async / sync)
but test such as TT and multiple scan task make this adding which will
have > 2 FS task this means i need to have table with lot of FS tasks and
corresponding assertion
```java
default boolean shouldPlanTableScanAsync(TableScan tableScan) {
List<FileScanTask> fs = tableScan.planFiles()
if (fs.size() > 2) {
// async plan
}
}
}
```
presently for testing sync or async for me i just for this via the setter
and server sets it automatically as part of testSetup.
Given this i don't have a strong preference for setter and totally open to
make changes per your recommendation, please let me know wdyt of approach 1 or
2 or have a complete other approach, happy to make the changes accordingly,
(haven't resolved feedback from before as well
https://github.com/apache/iceberg/pull/13400#discussion_r2538882579)
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
