forked from delta-io/delta
-
Notifications
You must be signed in to change notification settings - Fork 0
Add ServerSidePlanningClient interface and implementations #3
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.
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
murali-db
wants to merge
4
commits into
server-side-planning-1-rest-server-test-infra
Choose a base branch
from
server-side-planning-2-client-interface
base: server-side-planning-1-rest-server-test-infra
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Add ServerSidePlanningClient interface and implementations #3
murali-db
wants to merge
4
commits into
server-side-planning-1-rest-server-test-infra
from
server-side-planning-2-client-interface
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
murali-db
added a commit
that referenced
this pull request
Nov 2, 2025
This PR adds comprehensive integration tests that validate the entire server-side planning stack from DeltaCatalog through to data reading. Test Coverage: - Full stack integration: DeltaCatalog → ServerSidePlannedTable → Client → Data - SELECT query execution through server-side planning path - Aggregation queries (SUM, COUNT, GROUP BY) - Verification that normal path is unaffected when feature disabled Test Strategy: 1. Enable DeltaCatalog as Spark catalog 2. Create Parquet tables with test data 3. Enable forceServerSidePlanning flag 4. Configure ServerSidePlanningTestClientFactory 5. Execute queries and verify results 6. Verify scan plan discovered files Test Cases: - E2E full stack integration with SELECT query - E2E aggregation query (SUM, COUNT, GROUP BY) - Normal path verification (feature disabled) Assertions: - Query results are correct - Files are discovered via server-side planning - Aggregations produce correct values - Normal table loading works when feature disabled This completes the test pyramid: - PR #1: Test infrastructure (REST server) - PR #2: Client unit tests - PR #3: DSv2 Table unit and integration tests - PR #4: DeltaCatalog integration (no new tests - minimal change) - PR #5: Full stack E2E integration tests (this PR) All functionality is now fully tested from unit to integration level.
5 tasks
c49fd19 to
f9a93e1
Compare
murali-db
added a commit
that referenced
this pull request
Nov 3, 2025
This PR adds comprehensive integration tests that validate the entire server-side planning stack from DeltaCatalog through to data reading. Test Coverage: - Full stack integration: DeltaCatalog → ServerSidePlannedTable → Client → Data - SELECT query execution through server-side planning path - Aggregation queries (SUM, COUNT, GROUP BY) - Verification that normal path is unaffected when feature disabled Test Strategy: 1. Enable DeltaCatalog as Spark catalog 2. Create Parquet tables with test data 3. Enable forceServerSidePlanning flag 4. Configure ServerSidePlanningTestClientFactory 5. Execute queries and verify results 6. Verify scan plan discovered files Test Cases: - E2E full stack integration with SELECT query - E2E aggregation query (SUM, COUNT, GROUP BY) - Normal path verification (feature disabled) Assertions: - Query results are correct - Files are discovered via server-side planning - Aggregations produce correct values - Normal table loading works when feature disabled This completes the test pyramid: - PR #1: Test infrastructure (REST server) - PR #2: Client unit tests - PR #3: DSv2 Table unit and integration tests - PR #4: DeltaCatalog integration (no new tests - minimal change) - PR #5: Full stack E2E integration tests (this PR) All functionality is now fully tested from unit to integration level.
murali-db
added a commit
that referenced
this pull request
Nov 3, 2025
This PR adds comprehensive integration tests that validate the entire server-side planning stack from DeltaCatalog through to data reading. Test Coverage: - Full stack integration: DeltaCatalog → ServerSidePlannedTable → Client → Data - SELECT query execution through server-side planning path - Aggregation queries (SUM, COUNT, GROUP BY) - Verification that normal path is unaffected when feature disabled Test Strategy: 1. Enable DeltaCatalog as Spark catalog 2. Create Parquet tables with test data 3. Enable forceServerSidePlanning flag 4. Configure ServerSidePlanningTestClientFactory 5. Execute queries and verify results 6. Verify scan plan discovered files Test Cases: - E2E full stack integration with SELECT query - E2E aggregation query (SUM, COUNT, GROUP BY) - Normal path verification (feature disabled) Assertions: - Query results are correct - Files are discovered via server-side planning - Aggregations produce correct values - Normal table loading works when feature disabled This completes the test pyramid: - PR #1: Test infrastructure (REST server) - PR #2: Client unit tests - PR #3: DSv2 Table unit and integration tests - PR #4: DeltaCatalog integration (no new tests - minimal change) - PR #5: Full stack E2E integration tests (this PR) All functionality is now fully tested from unit to integration level.
2dcd2e8 to
43a3d0c
Compare
murali-db
added a commit
that referenced
this pull request
Nov 3, 2025
This PR adds comprehensive integration tests that validate the entire server-side planning stack from DeltaCatalog through to data reading. Test Coverage: - Full stack integration: DeltaCatalog → ServerSidePlannedTable → Client → Data - SELECT query execution through server-side planning path - Aggregation queries (SUM, COUNT, GROUP BY) - Verification that normal path is unaffected when feature disabled Test Strategy: 1. Enable DeltaCatalog as Spark catalog 2. Create Parquet tables with test data 3. Enable forceServerSidePlanning flag 4. Configure ServerSidePlanningTestClientFactory 5. Execute queries and verify results 6. Verify scan plan discovered files Test Cases: - E2E full stack integration with SELECT query - E2E aggregation query (SUM, COUNT, GROUP BY) - Normal path verification (feature disabled) Assertions: - Query results are correct - Files are discovered via server-side planning - Aggregations produce correct values - Normal table loading works when feature disabled This completes the test pyramid: - PR #1: Test infrastructure (REST server) - PR #2: Client unit tests - PR #3: DSv2 Table unit and integration tests - PR #4: DeltaCatalog integration (no new tests - minimal change) - PR #5: Full stack E2E integration tests (this PR) All functionality is now fully tested from unit to integration level.
murali-db
added a commit
that referenced
this pull request
Nov 3, 2025
This PR adds comprehensive integration tests that validate the entire server-side planning stack from DeltaCatalog through to data reading. Test Coverage: - Full stack integration: DeltaCatalog → ServerSidePlannedTable → Client → Data - SELECT query execution through server-side planning path - Aggregation queries (SUM, COUNT, GROUP BY) - Verification that normal path is unaffected when feature disabled Test Strategy: 1. Enable DeltaCatalog as Spark catalog 2. Create Parquet tables with test data 3. Enable forceServerSidePlanning flag 4. Configure ServerSidePlanningTestClientFactory 5. Execute queries and verify results 6. Verify scan plan discovered files Test Cases: - E2E full stack integration with SELECT query - E2E aggregation query (SUM, COUNT, GROUP BY) - Normal path verification (feature disabled) Assertions: - Query results are correct - Files are discovered via server-side planning - Aggregations produce correct values - Normal table loading works when feature disabled This completes the test pyramid: - PR #1: Test infrastructure (REST server) - PR #2: Client unit tests - PR #3: DSv2 Table unit and integration tests - PR #4: DeltaCatalog integration (no new tests - minimal change) - PR #5: Full stack E2E integration tests (this PR) All functionality is now fully tested from unit to integration level.
57e1430 to
37c120c
Compare
murali-db
added a commit
that referenced
this pull request
Nov 3, 2025
This PR adds comprehensive integration tests that validate the entire server-side planning stack from DeltaCatalog through to data reading. Test Coverage: - Full stack integration: DeltaCatalog → ServerSidePlannedTable → Client → Data - SELECT query execution through server-side planning path - Aggregation queries (SUM, COUNT, GROUP BY) - Verification that normal path is unaffected when feature disabled Test Strategy: 1. Enable DeltaCatalog as Spark catalog 2. Create Parquet tables with test data 3. Enable forceServerSidePlanning flag 4. Configure ServerSidePlanningTestClientFactory 5. Execute queries and verify results 6. Verify scan plan discovered files Test Cases: - E2E full stack integration with SELECT query - E2E aggregation query (SUM, COUNT, GROUP BY) - Normal path verification (feature disabled) Assertions: - Query results are correct - Files are discovered via server-side planning - Aggregations produce correct values - Normal table loading works when feature disabled This completes the test pyramid: - PR #1: Test infrastructure (REST server) - PR #2: Client unit tests - PR #3: DSv2 Table unit and integration tests - PR #4: DeltaCatalog integration (no new tests - minimal change) - PR #5: Full stack E2E integration tests (this PR) All functionality is now fully tested from unit to integration level.
a19cb4d to
da89365
Compare
37c120c to
dfc533e
Compare
da89365 to
c42f47e
Compare
dfc533e to
cf88294
Compare
tdas
reviewed
Nov 4, 2025
...in/scala/org/apache/spark/sql/delta/serverSidePlanning/IcebergServerSidePlanningClient.scala
Outdated
Show resolved
Hide resolved
tdas
reviewed
Nov 4, 2025
.../test/scala/org/apache/spark/sql/delta/serverSidePlanning/ServerSidePlanningTestClient.scala
Outdated
Show resolved
Hide resolved
tdas
reviewed
Nov 4, 2025
.../test/scala/org/apache/spark/sql/delta/serverSidePlanning/ServerSidePlanningTestClient.scala
Outdated
Show resolved
Hide resolved
tdas
reviewed
Nov 4, 2025
...in/scala/org/apache/spark/sql/delta/serverSidePlanning/IcebergServerSidePlanningClient.scala
Outdated
Show resolved
Hide resolved
tdas
reviewed
Nov 4, 2025
.../src/main/scala/org/apache/spark/sql/delta/serverSidePlanning/ServerSidePlanningClient.scala
Outdated
Show resolved
Hide resolved
tdas
reviewed
Nov 4, 2025
.../src/main/scala/org/apache/spark/sql/delta/serverSidePlanning/ServerSidePlanningClient.scala
Outdated
Show resolved
Hide resolved
tdas
reviewed
Nov 4, 2025
.../src/main/scala/org/apache/spark/sql/delta/serverSidePlanning/ServerSidePlanningClient.scala
Outdated
Show resolved
Hide resolved
tdas
reviewed
Nov 4, 2025
.../src/main/scala/org/apache/spark/sql/delta/serverSidePlanning/ServerSidePlanningClient.scala
Show resolved
Hide resolved
tdas
reviewed
Nov 4, 2025
.../test/scala/org/apache/spark/sql/delta/serverSidePlanning/MockServerSidePlanningClient.scala
Show resolved
Hide resolved
tdas
reviewed
Nov 4, 2025
spark/src/main/scala/org/apache/spark/sql/delta/sources/DeltaSQLConf.scala
Outdated
Show resolved
Hide resolved
Adds the client interface and factory for server-side planning: - ServerSidePlanningClient: Trait defining the planning contract - IcebergServerSidePlanningClient: REST implementation for Iceberg catalogs - IcebergServerSidePlanningClientFactory: Factory using reflection - ScanPlan/ScanFile: Simple data classes with no Iceberg dependencies - ServerSidePlanningClientSuite: Integration tests with IcebergRESTServer The interface lives in delta-spark module to avoid Iceberg dependencies, while the implementation lives in iceberg module where dependencies exist. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
cf88294 to
1f9a958
Compare
Changes made: - Rename IcebergServerSidePlanningClient to IcebergRESTCatalogPlanningClient - Rename IcebergServerSidePlanningClientFactory to IcebergRESTCatalogPlanningClientFactory - Remove schema field from ScanPlan (not in Iceberg REST API spec) - Remove partitionData field from ScanFile and add validation - Rename parameter 'namespace' to 'database' throughout - Remove FORCE_SERVER_SIDE_PLANNING config (moved to PR #5) - Simplify ServerSidePlanningTestClient (remove cloning/config logic) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This allows clients to be created with catalog-specific configuration by reading from spark.sql.catalog.<catalogName>.uri and token. Following the pattern used by UCCommitCoordinatorBuilder, this enables proper integration with Spark's catalog configuration system instead of hardcoding configuration keys. Changes: - Add buildForCatalog(spark, catalogName) to ServerSidePlanningClientFactory trait - Implement in IcebergRESTCatalogPlanningClientFactory to read catalog configs - Implement in MockServerSidePlanningClientFactory for testing - Implement in ServerSidePlanningTestClientFactory for testing - Add convenience method to ServerSidePlanningClientFactory object 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Fix scalastyle error by using toLowerCase(Locale.ROOT) instead of toLowerCase() in ServerSidePlanningTestClient. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR introduces the core client interface for server-side scan planning along with multiple implementations for different use cases (production
REST client, mocks for testing, and Spark-based client for integration tests).
Stack:
Changes
Core Interface (
spark/.../serverSidePlanning/)planScan(namespace, table)methodProduction Implementation (
iceberg/.../serverSidePlanning/)/v1/namespaces/{ns}/tables/{table}/planendpointPlanTableScanResponseto simpleScanPlandata classTest Implementations (
spark/src/test/.../serverSidePlanning/)input_file_name()to discover filesWhich Delta project/connector is this regarding?
Description
How was this patch tested?
Does this PR introduce any user-facing changes?