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

SQL over CSV with tests and docs #1146

Merged
merged 9 commits into from
Aug 27, 2024
Merged

Conversation

stuioco
Copy link
Contributor

@stuioco stuioco commented Aug 25, 2024

Added support for SELECT, UPDATE, DELETE over CSV data sources in csvSQL template function.

Intention is to allow users to simulate a persistent CRUD datastore in a straightforward manner.

This in addition to template function earlier for simpler datasource manipulation.

Have included tests and documentation.

I am not sure how the documentation will render once compiled.

@stuioco stuioco requested a review from tommysitu August 25, 2024 15:44
@kapishmalik
Copy link
Collaborator

Cool. This is nice 👍

core/templating/datasource.go Outdated Show resolved Hide resolved
core/templating/template_helpers.go Outdated Show resolved Hide resolved
core/templating/template_helpers.go Outdated Show resolved Hide resolved
core/templating/template_helpers.go Outdated Show resolved Hide resolved
core/templating/template_helpers.go Outdated Show resolved Hide resolved
core/templating/template_helpers.go Outdated Show resolved Hide resolved
core/templating/templating_test backup.txt Outdated Show resolved Hide resolved
core/templating/templating_test.go Outdated Show resolved Hide resolved
conditions := query.Conditions
setClauses := query.SetClauses
for i, row := range (*data)[1:] {
rowMap := mapRow(headers, row)
Copy link
Member

Choose a reason for hiding this comment

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

I started to think that it might be better to store the data as []map[string]string rather than [][]string if we are doing a lot of querying.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed - might be worth doing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not done - should be simple enough to refactor but quite a lot of changes

Copy link
Member

Choose a reason for hiding this comment

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

let's leave it for another PR.

@stuioco stuioco requested a review from tommysitu August 26, 2024 17:44
Copy link
Member

@tommysitu tommysitu left a comment

Choose a reason for hiding this comment

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

new templating functions only, LGTM

@tommysitu tommysitu merged commit 8fe2370 into SpectoLabs:master Aug 27, 2024
3 checks passed
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