-
Notifications
You must be signed in to change notification settings - Fork 74
MLE-23473 Implement fromDocs #1898
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
base: develop
Are you sure you want to change the base?
Conversation
Add implementation of optic fromDocs to java client. Includes new context method to indicate current row for expressions.
|
Copyright Validation Results ⏭️ Skipped (Excluded) Files
✅ Valid Files
✅ All files have valid copyright headers! |
There was a problem hiding this 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 implements the fromDocs optic operation for the Java client, enabling dynamic mapping of semi-structured data (JSON/XML) into rows and columns without requiring TDE template deployment. This provides ad-hoc query capabilities similar to Virtual Template Views with enhanced flexibility.
Changes:
- Adds
fromDocs()methods with multiple overloads to support different parameter combinations - Introduces
columnBuilder()andcontext()helper functions for defining column specifications and accessing current row context - Implements new types:
PlanColumnBuilder,PlanContextExprCall, and their sequence variants
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| FromDocsTest.java | Comprehensive test suite covering basic usage, default values, expressions, geospatial queries, and vector operations with fromDocs |
| PlanContextExprCallSeq.java | Interface for sequence of context expression calls |
| PlanContextExprCall.java | Interface for individual context expression call instances |
| PlanColumnBuilderSeq.java | Interface for sequence of column builders |
| PlanColumnBuilder.java | Main interface defining column builder API with methods for xpath, type, nullable, expr, defaultValue, collation, dimension, and coordinateSystem |
| PlanBuilderSubImpl.java | Adds "from-docs" to the list of supported accessor constructors |
| PlanBuilderImpl.java | Implements fromDocs methods with validation, columnBuilder/context factory methods, xpath overloads, and internal implementation classes |
| ColumnBuilderImpl.java | Concrete implementation of PlanColumnBuilder using builder pattern with immutable operations |
| PlanBuilder.java | Public API declarations for fromDocs, columnBuilder, context, and xpath methods with comprehensive documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
|
|
||
| static class ColumnBuilderSeqListImpl extends PlanSeqListImpl implements PlanColumnBuilderSeq { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This and several of the next classes are unused and should be removed.
| * A sequence of column builders. | ||
| * See {@link com.marklogic.client.expression.PlanBuilder#columnBuilder()} | ||
| */ | ||
| public interface PlanColumnBuilderSeq { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While this is implemented, it isn't used by anything, so I think it can be safely deleted.
| * A sequence of context expression calls. | ||
| * See {@link com.marklogic.client.expression.PlanBuilder#context()} | ||
| */ | ||
| public interface PlanContextExprCallSeq { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above - not used by anything, should be deleted, don't know why the code generator spits this out.
| writeSet.add(newWriteOp(personUri, personDoc)); | ||
| testUris.add(personUri); | ||
|
|
||
| docMgr.write(writeSet); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great tests, but just add these as new docs in the ml-data area in the test-app. Then you don't need all this code to create the docs nor to delete them.
| * data (JSON/XML) into rows and columns without deploying a TDE template. | ||
| */ | ||
| @ExtendWith(RequiresML12.class) | ||
| public class FromDocsTest extends AbstractOpticUpdateTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be able to extend AbstractClientTest now. AbstractOpticUpdateTest was my first attempt at a useful base class that clears most everything out of the database before it runs. AbstractClientTest is now that class. It doesn't really matter since you're not inserting any docs in the tests that need to be deleted, but it's still good to extend.
Add implementation of optic fromDocs to java client. Includes new context method to indicate current row for expressions.