Skip to content

feat: Support custom table and column names for JdbcChatMemoryRepository #3006

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sunyuhan1998
Copy link
Contributor

The JdbcChatMemoryRepository currently only supports predefined table and column names when in use, which limits its usability for users. We need the ability to customize these names. This PR addresses that by supporting custom configuration of these properties during build time.

…d for storing chat history when constructing the JdbcChatMemoryRepository.

Signed-off-by: Sun Yuhan <1085481446@qq.com>
@sunyuhan1998
Copy link
Contributor Author

Hi @ThomasVitale
What do you think about this?

Signed-off-by: Sun Yuhan <1085481446@qq.com>
@ThomasVitale
Copy link
Contributor

@sunyuhan1998 thanks so much, this is really needed! I was just mentioning it in #2806 (comment) where MSSQL requires different queries.

What do you think about making it more flexible by accepting the custom queries directly (eg via some SchemaBuilder/QueryProvider interface or something like that)? Different databases might require queries in a format that might not fit the current parametrisation (eg Oracle). Though I understand that doing so would require some changes in how we build the JdbcTemplate calls. It's tricky :)

@joshlong do you have some inputs for this change?

@sunyuhan1998
Copy link
Contributor Author

@sunyuhan1998 thanks so much, this is really needed! I was just mentioning it in #2806 (comment) where MSSQL requires different queries.

What do you think about making it more flexible by accepting the custom queries directly (eg via some SchemaBuilder/QueryProvider interface or something like that)? Different databases might require queries in a format that might not fit the current parametrisation (eg Oracle). Though I understand that doing so would require some changes in how we build the JdbcTemplate calls. It's tricky :)

@joshlong do you have some inputs for this change?

@ThomasVitale Yes, I also noticed that the current approach using fixed SQL statements is somewhat rigid. It seems necessary to support user-defined custom queries. I think we have two possible approaches:

  1. When building the JdbcChatMemoryRepository, instead of requiring a JdbcTemplate as a parameter, we could accept a QueryProvider as you suggested. Different implementations of QueryProvider could handle different database types, and if a JdbcTemplate is needed, it should be referenced within the QueryProvider.
  2. With the above approach, I start to wonder about the purpose of ChatMemoryRepository. It seems like it's merely forwarding the conversation memory content that needs to be saved from ChatMemory. Perhaps instead of having a generic JdbcChatMemoryRepository, we should directly implement separate repositories for each database type (e.g., OracleChatMemoryRepository, MsSqlChatMemoryRepository, etc.)?
    What do you think?

@ThomasVitale
Copy link
Contributor

@sunyuhan1998 I agree with you. Especially about the second bullet point. To ensure enough flexibility, we might make JdbcChatMemoryRepository a bit redundant. At that point, a DB-specific implementation might be better.

@sunyuhan1998
Copy link
Contributor Author

@sunyuhan1998 I agree with you. Especially about the second bullet point. To ensure enough flexibility, we might make JdbcChatMemoryRepository a bit redundant. At that point, a DB-specific implementation might be better.

@ThomasVitale I want to try something like this: use JdbcChatMemoryRepository as a base class, but make all parameters (column names, table names, etc.) as open and configurable as possible in the builder.
Then, I'd create several subclasses that extend it to adapt to different databases — for example, MariadbChatMemoryRepository and OracleChatMemoryRepository.
What do you think of this approach? If you think it sounds feasible, I'll start working on implementing it. Or, do you have any better ideas?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants