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

feat: sql builder #36

Merged
merged 6 commits into from
Sep 16, 2022
Merged

feat: sql builder #36

merged 6 commits into from
Sep 16, 2022

Conversation

scottlepp
Copy link
Contributor

Add the standard sql query builder component from plugin-ui.

image

@scottlepp scottlepp requested review from yesoreyeram and a team September 15, 2022 21:05
package.json Outdated Show resolved Hide resolved
pkg/models/query.go Show resolved Hide resolved
pkg/models/query.go Outdated Show resolved Hide resolved
pkg/plugin/handlers_querydata.go Show resolved Hide resolved
pkg/plugin/handlers_querydata.go Outdated Show resolved Hide resolved
pkg/plugin/handlers_querydata.go Outdated Show resolved Hide resolved
src/components/AstraQueryEditor.tsx Outdated Show resolved Hide resolved
src/components/AstraQueryEditor.tsx Outdated Show resolved Hide resolved
src/datasource.ts Outdated Show resolved Hide resolved
src/types.ts Show resolved Hide resolved
@yesoreyeram
Copy link
Contributor

just an UX feedback.. we should restrict * from columns if already one or more columns exist.
image

Also, if there is no columns specified, we should default to *. WDYT?

Looks like GROUP BY doesn't honoured sometimes
image

Also, if possible restrict the group by fields to only supported group by fields.

image

@yesoreyeram
Copy link
Contributor

also it would be nice to show column types as description in the dropdown.

image

@yesoreyeram
Copy link
Contributor

Overall LGTM. 🎉

@scottlepp
Copy link
Contributor Author

@yesoreyeram

just an UX feedback.. we should restrict * from columns if already one or more columns exist.
possibly add later as a feature, seems like an edge case. doubt most will do this

Also, if there is no columns specified, we should default to *. WDYT?
good idea. will add this as a feature

Looks like GROUP BY doesn't honoured sometimes
Will create a bug

Also, if possible restrict the group by fields to only supported group by fields.
Not sure this is possible. Somehow would have to check the table constraints. Maybe possible with a schema query but could get complicated.

also it would be nice to show column types as description in the dropdown.
Yes would be a nice enhancement. This UI is based on Big Query that the designers did. But yes, it can be improved.

@scottlepp scottlepp merged commit 1287b31 into main Sep 16, 2022
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.

None yet

2 participants