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

Assistant Builder: Can't select both remote and non-remote tables on the same Table Query action #11520

Merged
merged 1 commit into from
Mar 21, 2025

Conversation

PopDaph
Copy link
Contributor

@PopDaph PopDaph commented Mar 21, 2025

Description

The code that filters the datasource views to ensure if you're in table query you select only non-remote or only remote tables was not working anymore. We could select tables from both kind and end up with this error:

Screenshot 2025-03-21 at 10 59 48

This PR fixes the filtering logic + use the filtered data source views on the search endpoint to ensure we're searching applying the proper filter.

Tests

Locally.

Risk

Break the data source selector on the Builder.
Can be safely rollback.

Deploy Plan

Deploy front.

Copy link
Contributor

@Fraggle Fraggle left a comment

Choose a reason for hiding this comment

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

What exactly was the bug in the code ? (hard to tell with the diff)

@PopDaph
Copy link
Contributor Author

PopDaph commented Mar 21, 2025

The filtering logic was not applied, on prod you can select both snowflake and some tables in Notion.

First fix is to make sure the filtering is applied: you can check on front-edge
-> We were looping on orderDatasourceViews and not on filtered ones.

Second fix is to use the filtered datasource views on the search endpoint.

@PopDaph PopDaph marked this pull request as ready for review March 21, 2025 10:49
Copy link
Contributor

@philipperolet philipperolet left a comment

Choose a reason for hiding this comment

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

Great, LGTM 🙏 👍

@PopDaph PopDaph merged commit a619078 into main Mar 21, 2025
18 of 21 checks passed
@PopDaph PopDaph deleted the builder-fix-incompatible-tables branch March 21, 2025 13:07
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.

3 participants