Skip to content

Conversation

@rjrudin
Copy link
Contributor

@rjrudin rjrudin commented Feb 10, 2026

The plan is to dump eval support and just offer fromLexicons and fromView, but need to test out fromView a bit first.

Copilot AI review requested due to automatic review settings February 10, 2026 18:57
@github-actions
Copy link

github-actions bot commented Feb 10, 2026

Copyright Validation Results
Total: 7 | Passed: 6 | Failed: 0 | Skipped: 1 | at: 2026-02-10 19:39:58 UTC | commit: 130ba68

⏭️ Skipped (Excluded) Files

  • test-app/src/main/ml-schemas/tde/incrementalWriteHash.json

✅ Valid Files

  • marklogic-client-api/src/main/java/com/marklogic/client/datamovement/filter/IncrementalWriteConfig.java
  • marklogic-client-api/src/main/java/com/marklogic/client/datamovement/filter/IncrementalWriteEvalFilter.java
  • marklogic-client-api/src/main/java/com/marklogic/client/datamovement/filter/IncrementalWriteFilter.java
  • marklogic-client-api/src/main/java/com/marklogic/client/datamovement/filter/IncrementalWriteOpticFilter.java
  • marklogic-client-api/src/main/java/com/marklogic/client/datamovement/filter/IncrementalWriteViewFilter.java
  • marklogic-client-api/src/test/java/com/marklogic/client/datamovement/filter/IncrementalWriteTest.java

✅ All files have valid copyright headers!

Copy link

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

Adds support for incremental write hashing via an Optic fromView query (TDE view) as an alternative to lexicon-based retrieval, along with tests and a sample TDE template.

Changes:

  • Added a new IncrementalWriteViewFilter implementation that queries existing hashes from a TDE view via op.fromView.
  • Extended IncrementalWriteFilter.Builder with fromView(schemaName, viewName) and wiring in build().
  • Added test coverage for successful fromView usage and for a helpful failure when the view is invalid; added a sample TDE template for the view.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
test-app/src/main/ml-schemas/tde/incrementalWriteHash.json Adds a TDE template to expose URI/hash via a view used by fromView.
marklogic-client-api/src/test/java/com/marklogic/client/datamovement/filter/IncrementalWriteTest.java Adds tests for fromView and invalid view failure messaging.
marklogic-client-api/src/main/java/com/marklogic/client/datamovement/filter/IncrementalWriteViewFilter.java Implements view-based hash lookup using RowTemplate + Optic fromView.
marklogic-client-api/src/main/java/com/marklogic/client/datamovement/filter/IncrementalWriteFilter.java Adds builder support for configuring and constructing the view-based filter.

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

return filterDocuments(context, uri -> existingHashes.get(uri));
} catch (FailedRequestException e) {
String message = "Unable to query for existing incremental write hashes from view " + schemaName + "." + viewName + "; cause: " + e.getMessage();
throw new FailedRequestException(message, e.getFailedRequest());
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

The rethrown FailedRequestException drops the original exception as the cause, which makes debugging harder (stack trace/cause chain is lost). Prefer a constructor overload that accepts the cause, or initialize the cause on the newly created exception so the original e is preserved.

Suggested change
throw new FailedRequestException(message, e.getFailedRequest());
FailedRequestException fre = new FailedRequestException(message, e.getFailedRequest());
fre.initCause(e);
throw fre;

Copilot uses AI. Check for mistakes.
Comment on lines +2 to +24
"template": {
"description": "For incremental write that uses op.fromView instead of op.fromLexicons",
"context": "/doc",
"rows": [
{
"schemaName": "javaClient",
"viewName": "incrementalWriteHash",
"columns": [
{
"name": "uri",
"scalarType": "string",
"val": "xdmp:node-uri(.)"
},
{
"name": "hash",
"scalarType": "string",
"val": "xdmp:node-metadata-value(., 'incrementalWriteHash')",
"nullable": true
}
]
}
]
}
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

This JSON uses tab indentation. If the repo’s JSON/style conventions prefer spaces (common for JSON), consider normalizing indentation to match the rest of the project to reduce noisy diffs and keep formatting consistent.

Suggested change
"template": {
"description": "For incremental write that uses op.fromView instead of op.fromLexicons",
"context": "/doc",
"rows": [
{
"schemaName": "javaClient",
"viewName": "incrementalWriteHash",
"columns": [
{
"name": "uri",
"scalarType": "string",
"val": "xdmp:node-uri(.)"
},
{
"name": "hash",
"scalarType": "string",
"val": "xdmp:node-metadata-value(., 'incrementalWriteHash')",
"nullable": true
}
]
}
]
}
"template": {
"description": "For incremental write that uses op.fromView instead of op.fromLexicons",
"context": "/doc",
"rows": [
{
"schemaName": "javaClient",
"viewName": "incrementalWriteHash",
"columns": [
{
"name": "uri",
"scalarType": "string",
"val": "xdmp:node-uri(.)"
},
{
"name": "hash",
"scalarType": "string",
"val": "xdmp:node-metadata-value(., 'incrementalWriteHash')",
"nullable": true
}
]
}
]
}

Copilot uses AI. Check for mistakes.
The plan is to dump eval support and just offer fromLexicons and fromView, but need to test out fromView a bit first.

Did some refactoring too because the constructors had gotten so ugly - there's now an IncrementalWriteConfig class that holds all the inputs from the Builder, so that filter constructors only need that as an arg.
@rjrudin rjrudin merged commit 541b63e into develop Feb 10, 2026
4 checks passed
@rjrudin rjrudin deleted the feature/26918-tde branch February 10, 2026 21:22
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.

2 participants