Skip to content
This repository was archived by the owner on Jul 16, 2025. It is now read-only.

feat: add MariaDB store #342

Merged
merged 1 commit into from
Jun 26, 2025
Merged

Conversation

valtzu
Copy link
Contributor

@valtzu valtzu commented Jun 24, 2025

Related to #28

@valtzu valtzu force-pushed the add-mariadb-store branch 2 times, most recently from 2035405 to 032b197 Compare June 24, 2025 18:52
@chr-hertel chr-hertel requested a review from Copilot June 26, 2025 14:27
Copilot

This comment was marked as outdated.

Copy link
Member

@chr-hertel chr-hertel left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks for working on this.
Just one minor question - and what do you think about adding a compose.yaml to bring MariaDB to the repo for dev setup?

@valtzu valtzu force-pushed the add-mariadb-store branch 2 times, most recently from 5d6e995 to 93c0938 Compare June 26, 2025 15:13
composer.json Outdated
"codewithkyrian/chromadb-php": "^0.2.1 || ^0.3 || ^0.4",
"codewithkyrian/transformers": "^0.5.3",
"async-aws/bedrock-runtime": "^0.1.0",
"doctrine/dbal": "^4.2",
Copy link
Member

Choose a reason for hiding this comment

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

could also be, right?

Suggested change
"doctrine/dbal": "^4.2",
"doctrine/dbal": "^3.0 || ^4.0",

we won't see a difference tho without example or test :D

@valtzu valtzu force-pushed the add-mariadb-store branch from 93c0938 to 0e5f563 Compare June 26, 2025 15:41
@OskarStark OskarStark requested a review from Copilot June 26, 2025 15:43
Copy link

@Copilot 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 introduces a MariaDB-based vector store implementation for enhanced retrieval capabilities. Key changes include the addition of a new MariaDB store class with vector search support, an example demonstrating its usage, and corresponding updates to dependency configurations and environment settings.

  • Introduced MariaDB store class (Store.php) with methods for adding documents, querying, and initializing the underlying table.
  • Added an example usage script to demonstrate similarity search via the MariaDB store.
  • Updated composer.json, compose.yaml, and .env for MariaDB integration.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/Store/Bridge/MariaDB/Store.php New implementation for MariaDB vector store with query and initialization functions
examples/store/mariadb-similarity-search.php Example script to demonstrate similarity search using the MariaDB store
composer.json Dependency updates to support PDO and DBAL for MariaDB support
compose.yaml Docker configuration for MariaDB 11.7
.env Environment configuration for connecting to MariaDB

['title' => 'The Godfather', 'description' => 'The aging patriarch of an organized crime dynasty transfers control of his empire to his reluctant son.', 'director' => 'Francis Ford Coppola'],
];

// create embeddings and documents
Copy link
Preview

Copilot AI Jun 26, 2025

Choose a reason for hiding this comment

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

Consider initializing the $documents array before the foreach loop to ensure clarity and avoid potential undefined variable issues.

Suggested change
// create embeddings and documents
// create embeddings and documents
$documents = [];

Copilot uses AI. Check for mistakes.

@chr-hertel
Copy link
Member

Getting this error when executing the example:

PHP Fatal error:  Uncaught Doctrine\DBAL\Exception\DriverRequired: The options "driver" or "driverClass" are mandatory if no PDO instance is given to DriverManager::getConnection().

@valtzu valtzu force-pushed the add-mariadb-store branch from 0e5f563 to bd26570 Compare June 26, 2025 15:58
Copy link
Member

@chr-hertel chr-hertel left a comment

Choose a reason for hiding this comment

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

Let's merge this - thanks @valtzu!

@chr-hertel chr-hertel merged commit be704f7 into php-llm:main Jun 26, 2025
7 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants