Use pager and allow configuration via \pset

[djellemah]

Which issue does this PR close?

• Closes Use a pager for query results in datafusion-cli #15596

Rationale for this change

Explained in issue. Common in other cli programs implementing querying of data.

What changes are included in this PR?

Add a pager (less by default). Check that it exists. Use it for query output.

Allow some manipulation of which pager program is used, and what its command line arguments are, via \pset (following psql usage of \pset)

Allow use of pager to be switched off.

Do not use a pager if stdout is redirected to a non-tty.

Are these changes tested?

Not in unit tests.

Are there any user-facing changes?

Yes. The purpose of these changes is to improve UX.

Not certain if the change from specifying format using \pset csv to using pset format csv is considered a breaking change.

In any case, \pset appears to come from psql, which uses subcommands to specify several areas of output configuration, one of which is pager. datafusion-cli should therefore have used \pset format whatever from the outset.

[berkaysynnada]

@djellemah thank you for working on this! Can we also add some tests to not break these features in the future? and there are some failures in CI

[alamb]

This PR has several CI failures so marking as a draft while they are addressed.
(I do this to make it easier to see what PRs are waiting on review)

[djellemah]

I've added commits to address the CI failures.

Of course you're welcome to add some tests to not break these features in the future :-)

[alamb]

Thank you for this contribution @djellemah -- it is a very nice feature to have

I've added commits to address the CI failures.

Of course you're welcome to add some tests to not break these features in the future :-)

As documented in the contribution guidelines https://datafusion.apache.org/contributor-guide/testing.html#testing

All new features should have test coverage

Thus I agree with @berkaysynnada that this feature should have tests before we merge it.

Since maintainer bandwidth is the most limited resource in this project, I suspect we might be waiting a while if we wait for a maintainer help write tests

[Weijun-H]

On f36029c, I added a test to verify the CLI execution on the pty. However, testing key presses like ‘w’ or ‘s’ in the test is tricky, and there’s additional complexity with waiting for other tests to finish.

[djellemah]

Seems to me there's a confluence of several factors here:

• testing this kind of functionality is not simple.

• it's user facing, so if it breaks somebody will notice and say something.

• it's not core functionality so the probability of erroneous behaviors causing severe consequences is fairly low.

• there is limited maintainer bandwidth.

• there is limited contributor bandwidth.

In my opinion, the decision matrix here is not quite the same as one would apply to core functionality.