Skip to content

feat: complete OracleDocumentStore operations#3179

Merged
davidsbatista merged 19 commits intodeepset-ai:mainfrom
yugborana:feature/oracle-integration-completion
Apr 23, 2026
Merged

feat: complete OracleDocumentStore operations#3179
davidsbatista merged 19 commits intodeepset-ai:mainfrom
yugborana:feature/oracle-integration-completion

Conversation

@yugborana
Copy link
Copy Markdown
Contributor

@yugborana yugborana commented Apr 19, 2026 •
edited by davidsbatista
Loading

Related Issues

Proposed Changes:

I finalized the Oracle integration by completing all missing Document Store functions, fixing underlying datatype bugs, and implementing their asynchronous equivalents using asyncio.to_thread to prevent thread-blocking during database calls.

How did you test it?

To ensure total reliability, we rely on 28 unit tests executing entirely in mocked memory to validate parsing logic and component structure, supplemented by 105 real-database integration tests driven by Haystack's strict compliance mixins proving complete systemic alignment. For manual verification, we orchestrated an end-to-end Python script that connects to the live Oracle 23ai Docker container, injects diverse real-world metadata, merges patches, manipulates keyword indexing, and audits the async output states.

Checklist

  • I have read the contributors guidelines and the code of conduct - yes
  • I have updated the related issue with new insights and changes - yes
  • I added unit tests and updated the docstrings - yes
  • I've used one of the conventional commit types for my PR title: fix:, feat:, build:, chore:, ci:, docs:, style:, refactor:, perf:, test:. - yes

@yugborana yugborana requested a review from a team as a code owner April 19, 2026 05:03
@yugborana yugborana requested review from anakin87 and removed request for a team April 19, 2026 05:03
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 19, 2026 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions Bot added the type:documentation Improvements or additions to documentation label Apr 19, 2026
@yugborana
Copy link
Copy Markdown
Contributor Author

yugborana commented Apr 19, 2026

@davidsbatista @julian-risch

@yugborana
Copy link
Copy Markdown
Contributor Author

yugborana commented Apr 19, 2026

@bogdankostic @sjrl

@anakin87 anakin87 requested review from davidsbatista and removed request for anakin87 April 20, 2026 07:26
@davidsbatista davidsbatista changed the title feat: complete OracleDocumentStore methods, hybrid search, and async … feat: complete OracleDocumentStore operations Apr 20, 2026
@davidsbatista
Copy link
Copy Markdown
Contributor

davidsbatista commented Apr 20, 2026

@yugborana, thanks for your contribution. I will start the review still this week.

NOTE: There's no need to tag anyone immediately; once the PR is open, one of us will be assigned to review it.

@yugborana
Copy link
Copy Markdown
Contributor Author

yugborana commented Apr 20, 2026

Thanks for the reply. Sorry for tagging immediately.

@davidsbatista
Merge branch 'main' into feature/oracle-integration-completion
baf0c5e
@davidsbatista
Merge branch 'main' into feature/oracle-integration-completion
2949c7d
@davidsbatista
Merge branch 'main' into feature/oracle-integration-completion
9be93ba
@davidsbatista
wip
fe670cb
@davidsbatista
fixing possible SQL injection via unsanitized field_path with regex v…
b4c6c60 
…alidation
@davidsbatista
adding logging statment when _ensure_keyword_index fails
5ca3fb2
@davidsbatista
fixing logging formatting
abd01c6
@davidsbatista
make get_metadata_field_unique_values params passing consistent with …
d64943c 
…the rest of the code
@davidsbatista
keyword and embedding retrieval fail following the same principle as …
575314b 
…pgvector
@davidsbatista
fix for the scenario where an existing database already exists and us…
3e40c43 
…er wants to use it
@davidsbatista
delete_table following the same failing pattern as for keyword and em…
f9e5398 
…bedding retrieval
@davidsbatista
fixing all docstrings and logging statments
e0423b1
@davidsbatista
formatting
a064f82
@davidsbatista
fixing tests
93c72f9
@davidsbatista
Copy link
Copy Markdown
Contributor

davidsbatista commented Apr 23, 2026

Thanks for your contribution @yugborana

I had to do some more adjustments,making sure there's no SQL injections are possible, passing up some exceptions to the caller when they occur, logging statements.

One thing to keep in mind is the docstrings, check how they are done in the other parts of the code. They are then rendered and used as part of our API doc. I rewrote them completely.

davidsbatista
davidsbatista approved these changes Apr 23, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@davidsbatista davidsbatista left a comment

Choose a reason for hiding this comment

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

Looks good - did some adjustments

@yugborana
Copy link
Copy Markdown
Contributor Author

yugborana commented Apr 23, 2026

Thanks for giving your time. I will try to refine my process of contributing. Review was pretty fast and also saw for the first time that maintainers are themselves doing changes to a contributor's code and making it properly work.
I suppose it is good to go for merge?

@davidsbatista davidsbatista merged commit 5c0d0ea into deepset-ai:main Apr 23, 2026
12 checks passed
@davidsbatista
Copy link
Copy Markdown
Contributor

davidsbatista commented Apr 23, 2026

Thanks for giving your time. I will try to refine my process of contributing. Review was pretty fast and also saw for the first time that maintainers are themselves doing changes to a contributor's code and making it properly work. I suppose it is good to go for merge?

Yes, I just merged it. In some occasions we do the changes ourselves, in this case they were too many, but I hope you can learn from some of the adjustments I did.

@yugborana
Copy link
Copy Markdown
Contributor Author

yugborana commented Apr 23, 2026

Any other issues you could point me to where I can work...

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

Reviewers

@davidsbatista davidsbatista davidsbatista approved these changes

Assignees

No one assigned

Labels

type:documentation Improvements or additions to documentation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

OracleDocumentStore missing methods and test coverage

3 participants

@yugborana @CLAassistant @davidsbatista