Skip to content
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

Check for table's name uniquess in core instead of fetching all tables in front when upserting #11600

Merged
merged 1 commit into from
Mar 25, 2025

Conversation

Fraggle
Copy link
Contributor

@Fraggle Fraggle commented Mar 25, 2025

Description

When we upsert a table we were fetching all the tables to check for uniqueness of datasource & name which is computationnaly intensive.
This PR move the check to core in a much more efficient way.

Tests

Local

Risk

Low, very low usage of this endpoint outside of controlled connectors code.

Deploy Plan

Deploy core & front

@Fraggle Fraggle requested review from spolu and fontanierh March 25, 2025 17:02
Copy link

github-actions bot commented Mar 25, 2025

Warnings
⚠️

Files in **/api/v1/ have been modified and the PR has the documentation-ack label.
Don't forget to run npm run docs and use the Deploy OpenAPI Docs Github action to update https://docs.dust.tt/reference.

Generated by 🚫 dangerJS against 1dd3a5d

Copy link
Contributor

@spolu spolu left a comment

Choose a reason for hiding this comment

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

LGTM assuming it's indexed?

@Fraggle Fraggle added the documentation-ack Used to acknowledge that documentation is up-to-date with this PR label Mar 25, 2025
@Fraggle
Copy link
Contributor Author

Fraggle commented Mar 25, 2025

LGTM assuming it's indexed?

It's not and I don't know why, especially since @fontanierh mention that it has to be unique in the table schema.
I plan to ask him (my guess is that it needed some migration)

@spolu
Copy link
Contributor

spolu commented Mar 25, 2025

Let's add an index for that no? Not necessarily unique to avoid any issue. I'm more concerned by perf?

@Fraggle Fraggle merged commit d493a41 into main Mar 25, 2025
9 checks passed
@Fraggle Fraggle deleted the sflory-stop-fetching-all-tables-when-upserting branch March 25, 2025 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation-ack Used to acknowledge that documentation is up-to-date with this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants