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

WIP: Implement multiline query output #230

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

davisp
Copy link

@davisp davisp commented Aug 16, 2024

This is a first pass at adding support for matching query output line by line without attempting to parse column types and values. The general idea here is that we can use a query multiline directive to compare query results line by line, rather than attempting to parse and validate individual values. The idea being that this allows custom database drivers to just output formatted tables like such:

query multiline
select * from example_multiline
----
+---+---+---+
| a | b | c |
+---+---+---+
| 1 | 2 | 3 |
| 4 | 5 | 6 |
| 7 | 8 | 9 |
+---+---+---+

So far this PR appears to behave as intended by applying it to a personal project. The testing is extremely light, and there are a few different ways I could see implementing this feature. However, this seemed like the most direct approach after I spent at least 30m skimming the current implementation. I have verified that both matching and file updating work from the library implementation. I have not run this against the CLI version or attempted to add engine support. I can look into adding that as well if its desired.

The implementation is slightly wonky in that it just relies on the implementations of DB and AsyncDB to return a new DBOutput::MultiLine variant from the run(sql: &str) method. However, given the current state of those traits, I don't think there's much better to be done at the moment.

Speaking of those traits, and improving things, I think it'd be possible to do a progressive upgrade to those traits to add run_statement, run_query, and run_multiline_query methods that default just call run. However, I didn't feel like mixing up that level of change with this PR given this is already a bit half baked.

Let me know if there's any interest in me fleshing this out into a full PR with lots more test coverage and any CLI/engine stuff that needs to happen. Also, thanks for sqllogictest! Its a great tool that's proving rather useful!

This is a first pass at adding support for matching query output line by
line without attempting to parse column types and values. This just
relies on the database driver to return DBOutput::MultiLine variants.

Ideally we could inform the driver on every SQL run, but I think that's
part of a bigger refactor where we refactor the DB and AsyncDB traits so
that they're aware of whether we're expecting statement or results
output.

Signed-off-by: Paul J. Davis <[email protected]>
@skyzh skyzh self-requested a review November 10, 2024 20:35
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.

1 participant