Skip to content

[SPEC | CORE] : Allow table level override for scan planning #14867

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
singhpk234 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[SPEC | CORE] : Allow table level override for scan planning #14867

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1a25199
Core: Simplify REST scan planning to 2 modes (client/catalog)
sfc-gh-prsingh Feb 6, 2026
e886045
address steven feedback
sfc-gh-prsingh Feb 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 38 additions & 3 deletions core/src/main/java/org/apache/iceberg/rest/RESTCatalogProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,10 @@ private RESTCatalogProperties() {}

public static final String NAMESPACE_SEPARATOR = "namespace-separator";

// Enable planning on the REST server side
public static final String REST_SCAN_PLANNING_ENABLED = "rest-scan-planning-enabled";
public static final boolean REST_SCAN_PLANNING_ENABLED_DEFAULT = false;
// Configure scan planning mode
// Can be set by server in LoadTableResponse.config() for table-level override
public static final String SCAN_PLANNING_MODE = "scan-planning-mode";
public static final String SCAN_PLANNING_MODE_DEFAULT = ScanPlanningMode.CLIENT.modeName();

public static final String REST_SCAN_PLAN_ID = "rest-scan-plan-id";

Expand All @@ -59,4 +60,38 @@ public enum SnapshotMode {
ALL,
REFS
}

/**
* Enum to represent scan planning mode.
*
* <ul>
* <li>CLIENT - Use client-side scan planning
* <li>SERVER - Use server-side scan planning
* </ul>
*/
public enum ScanPlanningMode {
CLIENT("client"),
SERVER("server");

private final String modeName;

ScanPlanningMode(String modeName) {
this.modeName = modeName;
}

public String modeName() {
return modeName;
}

public static ScanPlanningMode fromString(String mode) {
for (ScanPlanningMode planningMode : values()) {
if (planningMode.modeName.equalsIgnoreCase(mode)) {
return planningMode;
}
}

throw new IllegalArgumentException(
String.format("Invalid scan planning mode: %s. Valid values are: client, server", mode));
}
}
}
47 changes: 34 additions & 13 deletions core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,6 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
private MetricsReporter reporter = null;
private boolean reportingViaRestEnabled;
private Integer pageSize = null;
private boolean restScanPlanningEnabled;
private CloseableGroup closeables = null;
private Set<Endpoint> endpoints;
private Supplier<Map<String, String>> mutationHeaders = Map::of;
Expand Down Expand Up @@ -281,12 +280,6 @@ public void initialize(String name, Map<String, String> unresolved) {
RESTCatalogProperties.NAMESPACE_SEPARATOR,
RESTUtil.NAMESPACE_SEPARATOR_URLENCODED_UTF_8);

this.restScanPlanningEnabled =
PropertyUtil.propertyAsBoolean(
mergedProps,
RESTCatalogProperties.REST_SCAN_PLANNING_ENABLED,
RESTCatalogProperties.REST_SCAN_PLANNING_ENABLED_DEFAULT);

this.tableCache = createTableCache(mergedProps);
this.closeables.addCloseable(this.tableCache);

Expand Down Expand Up @@ -584,7 +577,7 @@ private Supplier<BaseTable> createTableSupplier(

trackFileIO(ops);

RESTTable table = restTableForScanPlanning(ops, identifier, tableClient);
RESTTable table = restTableForScanPlanning(ops, identifier, tableClient, tableConf);
if (table != null) {
return table;
}
Expand All @@ -595,9 +588,35 @@ private Supplier<BaseTable> createTableSupplier(
}

private RESTTable restTableForScanPlanning(
TableOperations ops, TableIdentifier finalIdentifier, RESTClient restClient) {
// server supports remote planning endpoint and server / client wants to do server side planning
if (endpoints.contains(Endpoint.V1_SUBMIT_TABLE_SCAN_PLAN) && restScanPlanningEnabled) {
TableOperations ops,
TableIdentifier finalIdentifier,
RESTClient restClient,
Map<String, String> tableConf) {
// Get client-side and server-side scan planning modes
String planningModeClientConfig = properties().get(RESTCatalogProperties.SCAN_PLANNING_MODE);
String planningModeServerConfig = tableConf.get(RESTCatalogProperties.SCAN_PLANNING_MODE);

// Validate that client and server configs don't conflict
Copy link
Contributor

@amogh-jahagirdar amogh-jahagirdar Feb 12, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a choice here. We can either hard fail on conflicting configs like is done below or we can just let one override the other.

I think the least surprising thing from a client perspective is to just override using the client config in case of conflicts. On average I don't expect people to be fiddling with this config. In the chance they do, they're probably intentionally trying to set it and we should attempt to respect that. Even if there's some FGAC and creds and all aren't returned and it fails later on, that's better to me.

I guess I could go either way here (as long as we log there's a conflict and we're overriding using some mechanism), but I think any approach seems better than just hard failing since that seems unneccessary.

Copy link
Contributor Author

@singhpk234 singhpk234 Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the least surprising thing from a client perspective is to just override using the client config in case of conflicts

It can cause an issue in a sense that let say i am low resource client i can't run scan planning at all, so i say use catalog and the catalog says do client ? i believe if we let catalog judgement win it may lead to failure ?
But i think other way to look at it we any way override the client config with the server one if server says for some configs we can do the same then here ?
I am also open to both !

// Only validate if BOTH are explicitly set (not null)
if (planningModeClientConfig != null && planningModeServerConfig != null) {
Preconditions.checkState(
planningModeClientConfig.equalsIgnoreCase(planningModeServerConfig),
"Scan planning mode mismatch for table %s: client config=%s, server config=%s",
finalIdentifier,
planningModeClientConfig,
planningModeServerConfig);
}

// Determine effective mode: prefer server config if present, otherwise use client config
String effectiveModeConfig =
planningModeServerConfig != null ? planningModeServerConfig : planningModeClientConfig;
RESTCatalogProperties.ScanPlanningMode effectiveMode =
effectiveModeConfig != null
? RESTCatalogProperties.ScanPlanningMode.fromString(effectiveModeConfig)
: RESTCatalogProperties.ScanPlanningMode.CLIENT;

if (effectiveMode == RESTCatalogProperties.ScanPlanningMode.SERVER
&& endpoints.contains(Endpoint.V1_SUBMIT_TABLE_SCAN_PLAN)) {
return new RESTTable(
ops,
fullTableName(finalIdentifier),
Expand All @@ -610,6 +629,8 @@ private RESTTable restTableForScanPlanning(
properties(),
conf);
}

// Default to client-side planning
return null;
}

Expand Down Expand Up @@ -683,7 +704,7 @@ public Table registerTable(

trackFileIO(ops);

RESTTable restTable = restTableForScanPlanning(ops, ident, tableClient);
RESTTable restTable = restTableForScanPlanning(ops, ident, tableClient, tableConf);
if (restTable != null) {
return restTable;
}
Expand Down Expand Up @@ -952,7 +973,7 @@ public Table create() {

trackFileIO(ops);

RESTTable restTable = restTableForScanPlanning(ops, ident, tableClient);
RESTTable restTable = restTableForScanPlanning(ops, ident, tableClient, tableConf);
if (restTable != null) {
return restTable;
}
Expand Down
Loading