Skip to content

Surface truncated flag from SEA result manifest#1351

Open
tom-s-powell wants to merge 4 commits intodatabricks:mainfrom
tom-s-powell:tp/truncated
Open

Surface truncated flag from SEA result manifest#1351
tom-s-powell wants to merge 4 commits intodatabricks:mainfrom
tom-s-powell:tp/truncated

Conversation

@tom-s-powell
Copy link
Copy Markdown
Contributor

@tom-s-powell tom-s-powell commented Mar 27, 2026

Description

When using SEA it would be helpful to understand whether results have been truncated to ensure we don't silently consume incomplete results. Propagating this flag into DatabricksResultSetMetaData similar to isCloudFetchUsed would be helpful.

I guess in theory the JDBC driver could fail loudly if results are truncated, but appreciate that's a behaviour change and perhaps use of SEA is expected to match behaviour via the API and it's a user's responsibility to behave appropriately.

Testing

Unit tests have been added.

@tom-s-powell tom-s-powell marked this pull request as ready for review March 27, 2026 12:29
Copilot AI review requested due to automatic review settings March 27, 2026 12:29
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR surfaces the SEA result manifest’s truncated flag through DatabricksResultSetMetaData so JDBC consumers can detect when SEA results are incomplete/truncated (similar to isCloudFetchUsed).

Changes:

  • Add truncated state to DatabricksResultSetMetaData and expose it via getIsTruncated().
  • Populate truncated from SEA ResultManifest.getTruncated() (defaulting to false when unset).
  • Add/extend unit + integration tests to assert getIsTruncated() behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
src/main/java/com/databricks/jdbc/api/impl/DatabricksResultSetMetaData.java Adds truncated metadata field/getter and initializes it from SEA result manifests.
src/test/java/com/databricks/jdbc/api/impl/DatabricksResultSetMetaDataTest.java Adds unit coverage for SDK/SEA manifest truncated being null/true/false.
src/test/java/com/databricks/jdbc/integration/fakeservice/tests/SqlExecApiHybridResultsIntegrationTests.java Extends integration assertions to ensure hybrid SEA results are not marked truncated.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 502 to 509
this.statementId = statementId;
this.isCloudFetchUsed = false;
this.totalRows = -1;
this.chunkCount = -1L;
this.columns = columnsBuilder.build();
this.columnNameIndex = CaseInsensitiveImmutableMap.copyOf(columnNameToIndexMap);
this.truncated = false;
}
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

This constructor now sets chunkCount to -1L. Since chunkCount is only used as nullable metadata (e.g., telemetry’s total_chunks_present), using null for "not applicable" avoids negative values showing up in logs/metrics.

Copilot uses AI. Check for mistakes.
@gopalldb
Copy link
Copy Markdown
Collaborator

gopalldb commented Apr 1, 2026

Can you please sync the branch from main?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants