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

Shell: Support for WITH clauses (or more complex queries) #239

Closed
SimonRichardson opened this issue Mar 31, 2023 · 7 comments · Fixed by #260
Closed

Shell: Support for WITH clauses (or more complex queries) #239

SimonRichardson opened this issue Mar 31, 2023 · 7 comments · Fixed by #260
Assignees
Labels
Feature New feature, not a bug

Comments

@SimonRichardson
Copy link
Member

The shell only supports queries that start with SELECT statements:

if strings.HasPrefix(strings.ToUpper(strings.TrimLeft(line, " ")), "SELECT") {
return s.processSelect(ctx, line)
} else {
return "", s.processExec(ctx, line)
}

Other queries can be made that don't start with a SELECT. It would be advantageous to add those to the shell. With dqlite being rolled out into production for Juju relatively soon, we're using the shell to inspect or remedy potential problems in the field. So using more of the sqlite SQL standard would be a good move forward.

@manadart
Copy link

Of course there's WITH. Anything else a query can begin with?

@SimonRichardson
Copy link
Member Author

Transactions at a query level might be useful at some point (BEGIN, COMMIT, ROLLBACK).

There is VACUUM but I'm unsure what will happen if you actually run that in dqlite?

@MathieuBordere
Copy link
Contributor

MathieuBordere commented Mar 31, 2023

Every query performed with the shell is also wrapped in a transaction right now, that's something that's also unwanted in many cases, we could add a startup option to disable that behavior.

@MathieuBordere
Copy link
Contributor

There is VACUUM but I'm unsure what will happen if you actually run that in dqlite?

VACUUM is currently not supported, see canonical/dqlite#435 (comment)

@jameinel
Copy link
Member

jameinel commented Apr 3, 2023

Presumably INSERT would be very useful, and UPDATE on a live system when you need to correct bad data.

@cole-miller
Copy link
Contributor

Ideally we'd avoid this kind of ad-hoc parsing of the SQL string entirely in go-dqlite and let the dqlite server take care of it. Since canonical/dqlite#477 dqlite's QUERY_SQL request can accomodate SQL strings that modify the database, which makes that potentially achievable.

@freeekanayaka
Copy link
Contributor

Ideally we'd avoid this kind of ad-hoc parsing of the SQL string entirely in go-dqlite and let the dqlite server take care of it. Since canonical/dqlite#477 dqlite's QUERY_SQL request can accomodate SQL strings that modify the database, which makes that potentially achievable.

+1

The shell should only use QUERY_SQL, via Go's Tx.Query() or another equivalent method.

As long as things like RETURNING are not used, this should already work without any other modification to the dqlite server.

Bonus points for fixing RETURNING as well, so we can close #192 as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature, not a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants