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

Let's not present usage pop-ups for unsupported functions in custom expression interface #35156

Closed
lbrdnk opened this issue Oct 27, 2023 · 1 comment · Fixed by #39766
Closed
Assignees
Labels
.Backend .Frontend Priority:P2 Average run of the mill bug Querying/GUI Query builder catch-all, including simple mode .Team/QueryingComponents Type:Bug Product defects
Milestone

Comments

@lbrdnk
Copy link
Contributor

lbrdnk commented Oct 27, 2023

Issue #34993 requires us to fix percentile support for mariadb, saying that user tried to use percentile in the custom expression interface but translation to sql which they've got was not right. However we do not support :percentile-expressions for mariadb on the backend. Let alone, mariadb supports only window version on percetile_disc and currently query builder also does not support use of window functions (related issue is #9393).

I believe the fact, that they were seeing the usage pop-up after writing in percentile( made them think it is something we are able to correctly compute from query builder and that's false.

  1. Not showing that pop-up could make us less prone to get bug reports for bugs which are not bugs.
  2. Same case for completion of arguments to functions that are not supported.

With regards to the completion of function name -- This is something we do not do for unsupported functions and that's right.

Attached screenshots contain (1) no completion when writing percentile and (2) pop-up after entring in percentile( and (3) completion of arguments that should imo happen only for supported functions.

Screenshot 2023-10-27 at 16 02 40 Screenshot 2023-10-27 at 16 02 46 Screenshot 2023-10-27 at 16 02 56

And super ideal behavior would be to not even enable user to generate request for query containing use of a function that is not supported by a driver or make backend return eg. 418 when request like this is processed.

@lbrdnk lbrdnk added .Frontend .Needs Triage .Task Not a part of any Epic, used by the Task Issue Template labels Oct 27, 2023
@ranquild ranquild added Type:Bug Product defects Priority:P2 Average run of the mill bug Querying/GUI Query builder catch-all, including simple mode .Team/42 and removed .Needs Triage .Task Not a part of any Epic, used by the Task Issue Template labels Oct 27, 2023
@kamilmielnik
Copy link
Contributor

kamilmielnik commented Jan 24, 2024

I can still reproduce it in master, 3e4a40e.

Frontend does not know which functions are supported by which database, so this would need backend work as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
.Backend .Frontend Priority:P2 Average run of the mill bug Querying/GUI Query builder catch-all, including simple mode .Team/QueryingComponents Type:Bug Product defects
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants