Skip to content

Add AI functions#24963

Merged
electrum merged 1 commit intotrinodb:masterfrom
electrum:ai
Feb 18, 2025
Merged

Add AI functions#24963
electrum merged 1 commit intotrinodb:masterfrom
electrum:ai

Conversation

@electrum
Copy link
Copy Markdown
Member

@electrum electrum commented Feb 9, 2025

Description

This adds AI functions that invoke an LLM by calling Ollama, Anthropic, or OpenAI (or compatible APIs). The functions are modeled after the Databricks SQL functions.

Testing

The tests utilize Hoverfly to execute against recorded responses from the provider APIs. The responses can be updated by changing the test class from @HoverflySimulate to @HoverflyCapture, providing a valid API key, and then running the modified tests. We have a custom Hoverfly JUnit 5 extension that stores responses separately for each test method, allowing to easily update the response for a single method.

Release notes

(x) Release notes are required, with the following suggested text:

## General
* Add AI functions. ({issue}`24963`)

@cla-bot cla-bot bot added the cla-signed label Feb 9, 2025
@github-actions github-actions bot added the docs label Feb 9, 2025
@electrum electrum force-pushed the ai branch 2 times, most recently from 15f9e59 to b6c8ec3 Compare February 9, 2025 17:56
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's rather use strings instead of enums.
There are more AI services out there and limiting the providers to only the ones we know of seems a limiting factor.

I'd salute having a more pluggable way to think about AI providers.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We don't support plugins for plugins in Trino. Adding new providers will require modifying the code, so anyone doing that can easily add their own enum.

Using an enum means that users will get an immediate config validation error if the value doesn't exist. I've been changing other similar string configs to enums.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also .. I am sure we are happy to expand to other providers but with Ollama we support a LOT already

@mosabua
Copy link
Copy Markdown
Member

mosabua commented Feb 10, 2025

Awesome ..

Copy link
Copy Markdown
Member

@mosabua mosabua left a comment

Choose a reason for hiding this comment

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

Gave the docs a review for starters.. very exciting to see this come alive.

@mosabua
Copy link
Copy Markdown
Member

mosabua commented Feb 10, 2025

Fyi @alaturqua

@alaturqua
Copy link
Copy Markdown
Contributor

Great to see this is being implemented.
I had developed a version some time ago.

https://github.com/alaturqua/trino-ai

@alaturqua
Copy link
Copy Markdown
Contributor

Open webui is using ollama to serve many models locally with openai interface. And a possibility to connect to openai, groq and similar ai services.

Might be worth to take a look at.

https://github.com/open-webui/open-webui

@electrum electrum force-pushed the ai branch 2 times, most recently from 11e4c17 to 8f9a6fd Compare February 15, 2025 05:26
@electrum
Copy link
Copy Markdown
Member Author

I removed ai_similarity function since that likely requires a specialized model. All of the LLMs that I tried returned somewhat random results, that varied between invocations for the same inputs, and varied significantly between models.

@electrum electrum requested a review from mosabua February 15, 2025 05:29
@electrum electrum force-pushed the ai branch 2 times, most recently from c26f64d to 0cedb38 Compare February 17, 2025 04:26
@electrum
Copy link
Copy Markdown
Member Author

Updated to include support for Anthropic and add OpenTelemetry tracing.

Copy link
Copy Markdown
Member

@dain dain left a comment

Choose a reason for hiding this comment

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

Nice!

Copy link
Copy Markdown
Member

@mosabua mosabua left a comment

Choose a reason for hiding this comment

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

A couple of minor adjustments for the docs. Code wise I think @dain already looked in detail so there is no need for me to dig in more.

I expect that all this will evolve quickly so I am okay with merging and follow up PRs for docs and any further changes.

@electrum electrum merged commit 6ce9e41 into trinodb:master Feb 18, 2025
@electrum electrum deleted the ai branch February 18, 2025 20:48
@Override
protected String generateCompletion(String model, String prompt)
{
URI uri = uriBuilderFrom(endpoint)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi @electrum, I'm wondering why here decided to build raw http request rather than using libs from com.openai.client.OpenAIClient?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We use the Airlift HTTP client because it’s used everywhere else in Trino and supports our standard configuration system. It already has extensive configs for things like TLS certificates that people often need to customize for their system. If we used a different client, then we’d have a long process of adding configs as people need them.

Also, we support backends other than OpenAI, and for something this simple, we don’t want to add a third party dependency.

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

Development

Successfully merging this pull request may close these issues.

7 participants