GSOC'25 - MDEV-36100 Automatic embedding generation and "expensive" functions

[apostolis1]

This DRAFT PR is part of the GSOC'25 project "MDEV-36100 Generate vector embeddings automatically on INSERT".
This is a DRAFT PR, meant to be used for mentor feedback and will be updated as the project progresses.

There are two main aspects on this PR:

1. Plugin to automatically generate vector emebeddings
2. Avoidance of recalculation of stored expensive virtual columns

• The Jira issue number for this PR is: MDEV-36100

Description

TODO: This will be updated

Release Notes

The plugin uses the following session variables:

1. generate_embedding_openai_host: The API endpoint the request will be sent to
2. generate_embedding_openai_api_key: The API key used for the request, provided via HTTP Bearer authentication

and provides the following status variables to help system admins keep track of requests:

1. generate_embedding_openai_successful_http_requests
2. generate_embedding_openai_total_http_requests

How can this PR be tested?

This PR creates a new testing suite, gen_embedding,which includes tests both for the plugin and the expensive functionality.
Expensive tests have the prefix expensive_, to distinguish them from the plugin tests.

Basing the PR against the correct MariaDB version

• This is a new feature or a refactoring, and the PR is based against the main branch.
• This is a bug fix, and the PR is based against the earliest maintained branch in which the bug can be reproduced.

Future Work

In terms of the plugin, the status variables are not being reset by FLUSH STATUS, it remains to be investigated why that is and whether there is special support needed from the plugin.

As far as the stored expensive virtual columns are concerned:

• Currently, changes in the virtual columns data type in ALTER statements are not detected.
  So, something like the following incorrectly does not recompute the virtual column:

create table t (
    doc text,
    embedding vector(1536) as (generate_embedding_openai(doc, "text-embedding-ada-002")) stored
) engine=MyISAM;
alter table t modify embedding vector(2000);

• We would also like to avoid recomputing stored expensive virtual columns in replication setups whenever possible.
  Initially, we would want to do this for Row-Based Logging and then for Mixed Logging.

PR quality check

• I checked the CODING_STANDARDS.md file and my PR conforms to this where appropriate.
• For any trivial modifications to the PR, I am ok with the reviewer making the changes themselves.

[CLAassistant]

All committers have signed the CLA.

[apostolis1]

This check does not detect changes in the column's data type, for example changes like
alter table t modify embedding vector(2000);
What would be a good way to detect those?

[apostolis1]

For anyone interested, I have created this gist as well with some additional information on this work.

[vuvova]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a new MariaDB plugin, GENERATE_EMBEDDING_OPENAI, which utilizes libcurl to fetch text embeddings from OpenAI's API, alongside core server optimizations to avoid recomputing expensive virtual stored columns during ALTER TABLE operations when dependencies remain unchanged. Feedback on the changes highlights critical safety issues in the plugin, such as potential null-pointer dereferences when initializing curl or handling unconfigured variables, and an uninitialized variable that could cause undefined behavior. Additionally, the global counters require atomic operations to prevent race conditions, and the global signature change of Item::eq should be reverted to avoid a major ABI break. Finally, optimizations are suggested for json_path setup efficiency, and a fix is proposed for the ALTER TABLE logic to ensure vector dimensions are verified before skipping recomputation.

[gemini-code-assist]

If api_key or host is NULL, constructing a std::string from it or passing it to curl_easy_setopt will cause undefined behavior or a server crash. We should check both variables and handle them gracefully before proceeding. Note that empty strings should be explicitly allowed as they may represent the default state or be considered valid.

    const char *key_str= THDVAR(current_thd, api_key);
    const char *host_str= THDVAR(current_thd, host);
    if (!key_str)
    { 
      my_printf_error(1, "GENERATE_EMBEDDING_OPENAI: api_key is not set", ME_WARNING);
      null_value= true;
      return 1;
    }
    if (!host_str)
    {
      my_printf_error(1, "GENERATE_EMBEDDING_OPENAI: host is not set", ME_WARNING);
      null_value= true;
      return 1;
    }
    std::string response, model_name, authorization= std::string("Authorization: Bearer ") + key_str;

References

1. Validation logic for configuration variables should explicitly allow empty strings if they are considered valid or represent the default state.

[gemini-code-assist]

If curl_easy_init() fails and returns NULL, the subsequent curl_easy_setopt calls will dereference a null pointer and crash the server. We must check if hnd is NULL before using it.

    hnd= curl_easy_init();
    if (!hnd)
    {
      my_printf_error(1, "GENERATE_EMBEDDING_OPENAI: curl_easy_init() failed", ME_ERROR_LOG | ME_WARNING);
      null_value= true;
      goto cleanup;
    }

[gemini-code-assist]

The local variable p is completely uninitialized when passed to json_get_path_start, which expects an already initialized/parsed path. This will read garbage and cause undefined behavior or crashes. It should be initialized by copying json_path->p.

  json_path_t p= json_path->p;

[gemini-code-assist]

Since MariaDB is multi-threaded, these global counters are subject to race conditions and lost updates when multiple threads execute the function concurrently. We should use atomic operations to prevent race conditions.

#include <atomic>
std::atomic<ulonglong> curl_requests{0}; /* Number of total curl requests */
std::atomic<ulonglong> successful_curl_requests{0}; /* Number of successful curl requests */

[gemini-code-assist]

As noted in the TODO comment, we must detect changes in the field type/length (such as changing a vector dimension from 1536 to 1537). Since Virtual_column_info::is_equal only compares the expression and type handler, it does NOT detect changes in the field length/dimension. To prevent copying a mismatched vector directly without recomputation, we must also verify that (*ptr)->field_length == def->field->field_length.

        if (!to->check_dependencies_in_write_set(*ptr) && 
            def->field->vcol_info && 
            (*ptr)->field_length == def->field->field_length &&
            (*ptr)->vcol_info->is_equal(def->field->vcol_info, true))

[gemini-code-assist]

The model pointer has already been retrieved and checked for NULL on line 101. Calling args[1]->val_str() again is redundant, inefficient, and potentially unsafe if the item is not constant. We should reuse the existing model pointer.

    model_name= std::string(model->ptr(), model->length());

[gemini-code-assist]

Setting up the json_path on every call to read_json is highly inefficient. Since JSON_EMBEDDING_PATH is a constant, we can set up json_path once in fix_length_and_dec on the statement mem_root and reuse it.

    MEM_ROOT *root= thd->mem_root;
    json_path= (json_path_with_flags *) alloc_root(root, sizeof(json_path_with_flags));
    json_path->p.types_used= JSON_PATH_KEY_NULL;
    const uchar *s_p= (const uchar *) JSON_EMBEDDING_PATH;
    if (json_path_setup(&json_path->p, &my_charset_utf8mb4_general_ci, s_p,
                        s_p + strlen((const char *) s_p)))
      return true;

[gemini-code-assist]

Since the json_path is now set up once in fix_length_and_dec, we can completely remove the redundant setup from read_json.

[gemini-code-assist]

Changing the signature of Item::eq globally is a major ABI break that will affect all third-party storage engines and plugins overriding eq. Instead of changing the signature of Item::eq, we can preserve the original is_equivalent method in Virtual_column_info which clones the expression and renames the table/db names using rename_table_processor (which was deleted in this PR). This avoids any ABI break and keeps the change localized.

[gemini-code-assist]

Using request.splitlines()[-1] to extract the JSON body is fragile and will fail if the request has trailing whitespace/newlines or if the body itself contains literal newlines. Instead, use request.partition("\r\n\r\n") to reliably separate the headers from the body.

Suggested change

	request_line = request.splitlines()[0]
	method, path, _ = request_line.split()
	if path == "/success" and method in ("GET", "POST"):
	parsed_input = json.loads(request.splitlines()[-1])["input"]
	headers, _, body = request.partition("\r\n\r\n")
	request_line = headers.splitlines()[0]
	method, path, _ = request_line.split()
	if path == "/success" and method in ("GET", "POST"):
	parsed_input = json.loads(body)["input"]

[svoj]

This review spans only plugin/gen_embedding changes.

Coding style is inconsistent: spacing, lines over 80 characters, trailing spaces, variable names.

I'd move tests from plugin/gen_embedding/mysql-test/gen_embedding/t/ plugin/gen_embedding/mysql-test/gen_embedding/r/ to plugin/gen_embedding/mysql-test/gen_embedding.

I'd move the whole plugin either to plugin/ai/generate_embedding_openai or plugin/func_generate_embedding_openai.

[svoj]

Either use thd->alloc<json_path_with_flags>(1) or thd_alloc(thd, sizeof(json_path_with_flags)).

[svoj]

Function comment should be moved before function itself.

[svoj]

Function comment should be moved before function itself.

[svoj]

Resetting hnd and slist1 is redundant. cleanup label duplicates preceding code, this can be avoided.

[svoj]

Redundant variable.

[svoj]

I'd rather use base64 instead of float: 2-2.5x less data to transfer and much cheaper to parse.

[svoj]

I'd avoid this loop unless we really want to handle multiple embeddings at once. We probably don't within the scope of this SQL function.

[svoj]

Why not max_dimensions * sizeof(float)?

[svoj]

Given that most modern LLMs expose REST APIs, I’d prefer a generic function such as AI_EMBEDDING(data[, model]) that abstracts away provider-specific differences. This would avoid maintaining multiple plugins with nearly identical implementations.

[svoj]

The fact that OpenAI key is exposed via session variable needs to be reevaluated.

1. value may shortly become visible to SHOW PROCESSLIST
2. value may be written to query log
3. unsure if it matters, but DBA can't set per-server key without exposing it to users
4. are there other ways for the value to become unintentionally visible?