Skip to content

Feature/elasticsearch handler update: fix array support and standardize format#11552

Merged
tino097 merged 5 commits into
mindsdb:releases/26.1.0from
richardokonicha:feature/elasticsearch-handler-update
Mar 24, 2026
Merged

Feature/elasticsearch handler update: fix array support and standardize format#11552
tino097 merged 5 commits into
mindsdb:releases/26.1.0from
richardokonicha:feature/elasticsearch-handler-update

Conversation

@richardokonicha
Copy link
Copy Markdown
Contributor

@richardokonicha richardokonicha commented Sep 10, 2025

Description

Improve the Elasticsearch handler by fixing array field handling, adding Data Catalog support for enterprise integration, and standardizing documentation format to match MindsDB conventions.

This PR addresses the "Arrays not supported" error that users encounter when querying Elasticsearch indices with array fields, implements the three required Data Catalog methods
(get_column_statistics, get_primary_keys, get_foreign_keys), and ensures the handler follows MindsDB's standard documentation format.

Demo Video:
Array support showcase and feature overview
[Elastic search demo with data catalog feature]
(https://youtu.be/FGfXqtKxdWY)

Type of change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ⚡ New feature (non-breaking change which adds functionality)
  • 📄 This change requires a documentation update

Verification Process

To ensure the changes are working as expected:

  • Test Location: /mindsdb/integrations/handlers/elasticsearch_handler/tests/
  • Test Coverage: 48 tests (6 handler + 10 Data Catalog + 32 query tests)
  • Verification Steps:
    1. Connect to Elasticsearch cluster with array fields
    2. Run SELECT * FROM elasticsearch_conn.products WHERE product_id = '12345' (arrays converted to JSON strings)
    3. Test Data Catalog: SELECT * FROM mindsdb.get_column_statistics('elasticsearch_conn', 'products')
    4. Use SELECT table_name FROM information_schema.tables WHERE table_schema = 'elasticsearch_conn' for schema discovery
    5. Verify handler automatically falls back to Search API when SQL API fails
  • CI Testing: mindsdb-handlers-monitor PR #13

Additional Media:

  • I have attached a brief loom video or screenshots showcasing the new functionality or change.

Checklist:

  • My code follows the style guidelines (PEP 8) of MindsDB.
  • I have appropriately commented on my code, especially in complex areas.
  • Necessary documentation updates are either made or tracked in issues.
  • Relevant unit and integration tests are updated or added.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Sep 10, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@entelligence-ai-pr-reviews
Copy link
Copy Markdown
Contributor

🔒 Entelligence AI Vulnerability Scanner

❌ Security analysis failed: Security analysis failed: 403: blocked: error while getting the team: team is blocked

Comment thread mindsdb/integrations/handlers/elasticsearch_handler/elasticsearch_handler.py Outdated
Comment thread mindsdb/integrations/handlers/elasticsearch_handler/elasticsearch_handler.py Outdated
Comment thread mindsdb/integrations/handlers/elasticsearch_handler/elasticsearch_handler.py Outdated
@richardokonicha
Copy link
Copy Markdown
Contributor Author

I have read the CLA Document and I hereby sign the CLA

github-actions Bot added a commit that referenced this pull request Sep 29, 2025
@richardokonicha richardokonicha force-pushed the feature/elasticsearch-handler-update branch 2 times, most recently from 3eeae43 to 4b1dea7 Compare October 22, 2025 16:01
@richardokonicha richardokonicha requested a review from a team as a code owner October 22, 2025 16:01
@richardokonicha richardokonicha changed the base branch from main to develop October 23, 2025 08:15
@richardokonicha richardokonicha force-pushed the feature/elasticsearch-handler-update branch from 610da93 to be158ee Compare October 23, 2025 16:35
ZoranPandovski
ZoranPandovski previously approved these changes Nov 3, 2025
Copy link
Copy Markdown
Member

@ZoranPandovski ZoranPandovski left a comment

Choose a reason for hiding this comment

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

Thanks, please check my comments

Comment thread mindsdb/integrations/handlers/elasticsearch_handler/requirements.txt Outdated
Comment thread mindsdb/integrations/handlers/elasticsearch_handler/requirements.txt Outdated
Comment thread mindsdb/integrations/handlers/elasticsearch_handler/elasticsearch_handler.py Outdated
Comment thread mindsdb/integrations/handlers/elasticsearch_handler/elasticsearch_handler.py Outdated
Comment thread mindsdb/integrations/handlers/elasticsearch_handler/elasticsearch_handler.py Outdated
Comment thread mindsdb/integrations/handlers/elasticsearch_handler/README.md Outdated
Comment thread mindsdb/integrations/handlers/elasticsearch_handler/elasticsearch_handler.py Outdated
Comment thread mindsdb/integrations/handlers/elasticsearch_handler/elasticsearch_handler.py Outdated
Comment thread mindsdb/integrations/handlers/elasticsearch_handler/elasticsearch_handler.py Outdated
Comment thread mindsdb/integrations/handlers/elasticsearch_handler/tests/run_tests.py Outdated
richardokonicha added a commit to richardokonicha/mindsdb that referenced this pull request Nov 5, 2025
…tabaseHandler

Migrate Elasticsearch handler to inherit from MetaDatabaseHandler to comply with data catalog specifications per review feedback on PR mindsdb#11552.

Key changes:
- Change base class from DatabaseHandler to MetaDatabaseHandler
- Rename methods with meta_ prefix per data catalog API requirements
- Add meta_get_tables and meta_get_columns required methods
- Update column names to uppercase specification (TABLE_NAME, COLUMN_NAME, etc.)
- Calculate NULL_PERCENTAGE as percentage (0.0-100.0) instead of count
- Support multiple tables via Optional[List[str]] parameter
- Remove data catalog documentation from README
- Delete deprecated handler test files, consolidate tests in unit/handlers
- Update all 18 unit tests to match new API
- Maintain backward compatibility by preserving old methods

All tests passing (18/18) and pre-commit checks passed.
@richardokonicha
Copy link
Copy Markdown
Contributor Author

@MinuraPunchihewa All comments addressed! Here's a summary:

Removed Data Catalog documentation sections from README
Changed inheritance to MetaDatabaseHandler
Updated all column names to uppercase specification
Renamed methods with meta_ prefix and updated signatures
Added meta_get_tables and meta_get_columns methods
Consolidated all tests to tests/unit/handlers/test_elasticsearch.py

Copy link
Copy Markdown
Contributor Author

@richardokonicha richardokonicha left a comment

Choose a reason for hiding this comment

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

I have consolidated all tests to tests/unit/handlers/test_elasticsearch.py

Copy link
Copy Markdown
Contributor

@MinuraPunchihewa MinuraPunchihewa left a comment

Choose a reason for hiding this comment

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

@richardokonicha This looks good. The only other ask that I have is for you to test this with our open-source agents. This will be a good smoke test for whether it will actually work with Minds, which is what we want. You can follow these steps:

*As a prerequisite, set the MINDSDB_DATA_CATALOG_ENABLED environment variable to true before starting MindsDB:
export MINDSDB_DATA_CATALOG_ENABLED=1

  1. Create a MindsDB database for Elasticsearch.
  2. Create an Agent connected to the Elasticsearch database:
CREATE AGENT my_agent
USING
    model = {
        "provider": "openai",
        "model_name" : "gpt-4o",
        "api_key": "<your-api-key>"
    },
    data = {
         "tables": ["elasticsearch_conn.<table-1>", "elasticsearch_conn.<table-2>"]
    },
    prompt_template='
        <an-explanation-of-the-data-contained-in-your-tables>
    ';
  1. Ask a few questions and evaluate if the answers are accurate.

If you can post a few screenshots of this working well, I think we should be good to go.

@MinuraPunchihewa
Copy link
Copy Markdown
Contributor

@richardokonicha For the tests to run, you will also need to add a line to install the dependencies for the Elasticsearch integration to the workflow here. You just need to add .[elasticsearch] to the end (also add a forward slash to the previous line).

@richardokonicha
Copy link
Copy Markdown
Contributor Author

@MinuraPunchihewa this branch has been updated and now all tests pass gracefully, here is a short video i made to demo this elastic search functionality https://youtu.be/FGfXqtKxdWY

@tino097 tino097 changed the base branch from develop to releases/26.1.0 March 17, 2026 12:18
@tino097
Copy link
Copy Markdown
Contributor

tino097 commented Mar 17, 2026

@richardokonicha hey man, can you please resolve the issues with this PR ?

…d add to CI

- Fix array field handling (convert to JSON strings)
- Add Data Catalog support (get_column_statistics, get_primary_keys, get_foreign_keys)
- Standardize documentation format
- Add elasticsearch to tests_unit.yml HANDLERS_TO_INSTALL for CI testing

Addresses maintainer feedback on PR mindsdb#11552
@richardokonicha richardokonicha force-pushed the feature/elasticsearch-handler-update branch from 4be4ec4 to aa03cd9 Compare March 19, 2026 01:30
@richardokonicha
Copy link
Copy Markdown
Contributor Author

@tino097 Hey! Branch is synced with releases/26.1.0 — all conflicts resolved and elasticsearch handler is intact.

Changes in this PR:

  • ✅ Array field handling (convert arrays to JSON strings)
  • ✅ Data Catalog support (meta_get_tables, meta_get_columns, get_column_statistics, get_primary_keys, get_foreign_keys)
  • ✅ Elasticsearch added to tests_unit.yml HANDLERS_TO_INSTALL for CI
  • ✅ MetaDatabaseHandler inheritance
  • ✅ Consolidated tests at tests/unit/handlers/test_elasticsearch.py
  • ✅ Demo video: https://youtu.be/FGfXqtKxdWY

Ready for review.

Comment thread mindsdb/integrations/handlers/elasticsearch_handler/tests/run_tests.py Outdated
Comment thread .github/workflows/tests_unit.yml Outdated
@richardokonicha
Copy link
Copy Markdown
Contributor Author

@tino097 Done — removed elasticsearch from HANDLERS_TO_INSTALL in the unit test workflow (1463a17).

Regarding run_tests.py — that file doesn't exist on this branch. Tests were already moved to tests/unit/handlers/test_elasticsearch.py per the earlier review. The comment may be referencing the old diff.

Let me know if anything else needs adjusting!

@richardokonicha richardokonicha requested a review from tino097 March 20, 2026 17:46
@tino097 tino097 merged commit 869db5f into mindsdb:releases/26.1.0 Mar 24, 2026
16 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 24, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants