-
Notifications
You must be signed in to change notification settings - Fork 15
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
Support vector index for SQLAlchemy #65
Conversation
Signed-off-by: Wish <[email protected]>
Signed-off-by: Wish <[email protected]>
Signed-off-by: Wish <[email protected]>
Signed-off-by: Wish <[email protected]>
Signed-off-by: Wish <[email protected]>
f"vec_idx_{column.name}" | ||
) | ||
|
||
query = sqlalchemy.text(f"ALTER TABLE {table_name} SET TIFLASH REPLICA 1") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should setting a TiFlash replica only if it doesn't already exist, or what if this table already have a tilfash replica with 2?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this use case priority could be lowered, as I'm considering this:
If the user has already jump to SQL to manipulate the table, out of this driver's control, then maybe he don't need this driver's help to add a new index?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Users may exhibit various behaviors, such as manually setting Tiflash replicas via SQLAlchemy's sql.execute
when using this adapter. We should effectively handle these scenarios.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, good point. I will change this.
Co-authored-by: WD <[email protected]>
Signed-off-by: Wish <[email protected]>
Signed-off-by: Wish <[email protected]>
As an alternative to #63
After discussion with @JaySon-Huang, inspired by some other vector databases, I think we could simply introduce some side functions for creating indexes. The usage is as simple as using native SQLAlchemy interfaces, without the need to introduce another dialect like sqlalchemy-tidb.
Maintaining a new SQLAlchemy dialect is costy, as we need to support both SQLAlchemy 1.4 and SQLAlchemy 2.0, and support other TiDB features like AUTO_RANDOM.