Bug: Fix multi-lines printing issue for datafusion-cli #14954

zhuqi-lucas · 2025-03-01T05:31:52Z

Which issue does this PR close?

Initial PR:

Datafusion-cli: Redesign the datafusion-cli execution and print, make it totally streaming printing without memory overhead #14877

Rationale for this change

Revert the reverted PR Revert Datafusion-cli: Redesign the datafusion-cli execution and print, make it totally streaming printing without memory overhead #14948
Fix multi-lines printing issue for datafusion-cli

What changes are included in this PR?

Revert the reverted PR
Fix multi-lines printing issue for datafusion-cli

Are these changes tested?

Yes

Are there any user-facing changes?

Fix the explain format

…and print, make it totally streaming printing without memory overhead (apache#14948)" This reverts commit 382e232.

zhuqi-lucas · 2025-03-01T05:39:38Z

Sorry for the inconvenience!

We can review and merge this PR after the release: cc @alamb @xudong963

Revert the reverted PR
Fix multi-lines printing issue for datafusion-cli

Meanwhile, i think more forks can also test it, i am not sure if we will have more bugs come since it's a totally new way for datafusion-cli, and i have added more testing cases, thanks!

zhuqi-lucas · 2025-03-01T11:47:28Z

Updated testing result:

DataFusion CLI v45.0.0
> create table foo(x int, y int) as values (1,2), (3,4);
0 row(s) fetched.
Elapsed 0.026 seconds.

> explain select * from foo where x = 4;
+---------------+-------------------------------------------------------+
| plan_type     | plan                                                  |
+---------------+-------------------------------------------------------+
| logical_plan  | Filter: foo.x = Int32(4)                              |
|               |   TableScan: foo projection=[x, y]                    |
| physical_plan | CoalesceBatchesExec: target_batch_size=8192           |
|               |   FilterExec: x@0 = 4                                 |
|               |     DataSourceExec: partitions=1, partition_sizes=[1] |
+---------------+-------------------------------------------------------+
2 row(s) fetched.
Elapsed 0.022 seconds.

> explain verbose select * from foo where x = 4;
+------------------------------------------------------------+-------------------------------------------+
| plan_type                                                  | plan                                      |
+------------------------------------------------------------+-------------------------------------------+
| initial_logical_plan                                       | Projection: *                             |
|                                                            |   Filter: foo.x = Int64(4)                |
|                                                            |     TableScan: foo                        |
| logical_plan after inline_table_scan                       | SAME TEXT AS ABOVE                        |
| logical_plan after expand_wildcard_rule                    | Projection: foo.x, foo.y                  |
|                                                            |   Filter: foo.x = Int64(4)                |
|                                                            |     TableScan: foo                        |
| logical_plan after resolve_grouping_function               | SAME TEXT AS ABOVE                        |
| logical_plan after type_coercion                           | Projection: foo.x, foo.y                  |
|                                                            |   Filter: CAST(foo.x AS Int64) = Int64(4) |
|                                                            |     TableScan: foo                        |
| analyzed_logical_plan                                      | SAME TEXT AS ABOVE                        |
| logical_plan after eliminate_nested_union                  | SAME TEXT AS ABOVE                        |
| logical_plan after simplify_expressions                    | SAME TEXT AS ABOVE                        |
| logical_plan after unwrap_cast_in_comparison               | Projection: foo.x, foo.y                  |
|                                                            |   Filter: foo.x = Int32(4)                |
|                                                            |     TableScan: foo                        |
| logical_plan after replace_distinct_aggregate              | SAME TEXT AS ABOVE                        |
| logical_plan after eliminate_join                          | SAME TEXT AS ABOVE                        |
| logical_plan after decorrelate_predicate_subquery          | SAME TEXT AS ABOVE                        |
| logical_plan after scalar_subquery_to_join                 | SAME TEXT AS ABOVE                        |
| logical_plan after extract_equijoin_predicate              | SAME TEXT AS ABOVE                        |
| logical_plan after eliminate_duplicated_expr               | SAME TEXT AS ABOVE                        |
| logical_plan after eliminate_filter                        | SAME TEXT AS ABOVE                        |
| logical_plan after eliminate_cross_join                    | SAME TEXT AS ABOVE                        |
| logical_plan after common_sub_expression_eliminate         | SAME TEXT AS ABOVE                        |
| logical_plan after eliminate_limit                         | SAME TEXT AS ABOVE                        |
| logical_plan after propagate_empty_relation                | SAME TEXT AS ABOVE                        |
| logical_plan after eliminate_one_union                     | SAME TEXT AS ABOVE                        |
| logical_plan after filter_null_join_keys                   | SAME TEXT AS ABOVE                        |
| logical_plan after eliminate_outer_join                    | SAME TEXT AS ABOVE                        |
| logical_plan after push_down_limit                         | SAME TEXT AS ABOVE                        |
| logical_plan after push_down_filter                        | SAME TEXT AS ABOVE                        |
| logical_plan after single_distinct_aggregation_to_group_by | SAME TEXT AS ABOVE                        |
| logical_plan after simplify_expressions                    | SAME TEXT AS ABOVE                        |
| logical_plan after unwrap_cast_in_comparison               | SAME TEXT AS ABOVE                        |
| logical_plan after common_sub_expression_eliminate         | SAME TEXT AS ABOVE                        |
| logical_plan after eliminate_group_by_constant             | SAME TEXT AS ABOVE                        |
| logical_plan after optimize_projections                    | Filter: foo.x = Int32(4)                  |
|                                                            |   TableScan: foo projection=[x, y]        |
| logical_plan after eliminate_nested_union                  | SAME TEXT AS ABOVE                        |
| logical_plan after simplify_expressions                    | SAME TEXT AS ABOVE                        |
| logical_plan after unwrap_cast_in_comparison               | SAME TEXT AS ABOVE                        |
| logical_plan after replace_distinct_aggregate              | SAME TEXT AS ABOVE                        |
| logical_plan after eliminate_join                          | SAME TEXT AS ABOVE                        |
| logical_plan after decorrelate_predicate_subquery          | SAME TEXT AS ABOVE                        |
| logical_plan after scalar_subquery_to_join                 | SAME TEXT AS ABOVE                        |
| logical_plan after extract_equijoin_predicate              | SAME TEXT AS ABOVE                        |
| logical_plan after eliminate_duplicated_expr               | SAME TEXT AS ABOVE                        |
| .                                                          | .                                         |
| .                                                          | .                                         |
| .                                                          | .                                         |
+------------------------------------------------------------+-------------------------------------------+
78 row(s) fetched. (First 40 displayed. Use --maxrows to adjust)
Elapsed 0.005 seconds.

alamb · 2025-03-01T13:11:48Z

Thanks @zhuqi-lucas -- I'll try and check this out soon (likely tomorrow)

alamb · 2025-03-03T12:04:48Z

datafusion-cli/src/print_format.rs

@@ -209,14 +211,175 @@ impl PrintFormat {
        }
        Ok(())
    }
+
+    #[allow(clippy::too_many_arguments)]
+    pub fn process_batch(


Given that we now have some more time, I wonder if you could potentially consider refactoring this code now.

Code clean for new datafusion-cli streaming printing logic #14886

I found it somewhat hard to understand when I was trying to debug why the explain plans were not showing up reasonably.

Thank you @alamb for this suggestion, may after this PR finalizing. We can open the #14961 to my branch? Is it a common way for us to collaborate?

alamb · 2025-03-03T12:05:34Z

datafusion-cli/src/print_options.rs

@@ -127,9 +107,125 @@ impl PrintOptions {
        Ok(())
    }

+    pub async fn print_table_batch(


Could you add some comments here, as well as how it is different than print_batches

alamb · 2025-03-03T12:06:09Z

datafusion-cli/src/print_options.rs

@@ -110,7 +90,7 @@ impl PrintOptions {
        self.format


One thing I noticed when reviewing this code is that there are different passes for print_batches and print_table_batch

I wonder if there is some way to combine them (so we always use the same printing logic)

zhuqi-lucas · 2025-03-04T04:04:27Z

Thanks @zhuqi-lucas -- I'll try and check this out soon (likely tomorrow)

Thank you @alamb for review and good suggestion! I will try to address it today.

zhuqi-lucas · 2025-03-04T09:38:34Z

I am continuing polish the code besides the #14886 which will add the streaming state struct. I believe the code is more readable now.

zhuqi-lucas · 2025-03-05T03:27:55Z

datafusion-cli/src/print_options.rs

+    ///
+    /// The query_start_time is used to calculate the elapsed time for the query.
+    /// The schema is used to print the header.
+    pub async fn print_stream(


Unified to one API for all cases.

zhuqi-lucas · 2025-03-05T05:52:15Z

datafusion-cli/tests/cli_integration.rs

@@ -51,6 +51,163 @@ fn init() {
    ["--command", "show datafusion.execution.batch_size", "--format", "json", "-q", "-b", "1"],
    "[{\"name\":\"datafusion.execution.batch_size\",\"value\":\"1\"}]\n"
 )]
+


Add rich testing cases in cli_integration, it can avoid regression for datafusion-cli changes.

zhuqi-lucas added 2 commits March 1, 2025 13:09

Revert "Revert Datafusion-cli: Redesign the datafusion-cli execution …

9f7ecf9

…and print, make it totally streaming printing without memory overhead (apache#14948)" This reverts commit 382e232.

Bug: Fix multi-lines printing issue for datafusion-cli

5a2a6a4

zhuqi-lucas marked this pull request as draft March 1, 2025 05:31

Fix clippy

7b01905

zhuqi-lucas marked this pull request as ready for review March 1, 2025 11:52

alamb reviewed Mar 3, 2025

View reviewed changes

This was referenced Mar 3, 2025

Weekly Plan (Andrew Lamb) March 3, 2025 #14978

Open

datafusion-cli: add streaming state for printing logic #14961

Draft

zhuqi-lucas added 11 commits March 4, 2025 12:28

Merge remote-tracking branch 'upstream/main' into issue_14953

016bb74

Change the code first version

3fb3d9d

code clean for unified function

4edd79e

Remove unused print batches

068e53f

Refactor testing

a454620

Add cleaer name

407cab2

Add cli testing

aad9c32

Add more cli testing

ab2820d

Fix clippy

a98ff18

Add rich testing

647e2dd

Add create table case

24fd2a1

Add more comments

e779a09

zhuqi-lucas commented Mar 5, 2025

View reviewed changes

zhuqi-lucas added 2 commits March 5, 2025 13:26

Add json limit testing

1aaf48e

Merge remote-tracking branch 'upstream/main' into issue_14953

dd68b88

zhuqi-lucas commented Mar 5, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug: Fix multi-lines printing issue for datafusion-cli #14954

Bug: Fix multi-lines printing issue for datafusion-cli #14954

zhuqi-lucas commented Mar 1, 2025 •

edited by alamb

Loading

zhuqi-lucas commented Mar 1, 2025 •

edited

Loading

zhuqi-lucas commented Mar 1, 2025

alamb commented Mar 1, 2025

alamb Mar 3, 2025

zhuqi-lucas Mar 4, 2025

alamb Mar 3, 2025

alamb Mar 3, 2025

zhuqi-lucas commented Mar 4, 2025

zhuqi-lucas commented Mar 4, 2025 •

edited

Loading

zhuqi-lucas Mar 5, 2025

zhuqi-lucas Mar 5, 2025

Bug: Fix multi-lines printing issue for datafusion-cli #14954

Are you sure you want to change the base?

Bug: Fix multi-lines printing issue for datafusion-cli #14954

Conversation

zhuqi-lucas commented Mar 1, 2025 • edited by alamb Loading

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

zhuqi-lucas commented Mar 1, 2025 • edited Loading

zhuqi-lucas commented Mar 1, 2025

alamb commented Mar 1, 2025

alamb Mar 3, 2025

Choose a reason for hiding this comment

zhuqi-lucas Mar 4, 2025

Choose a reason for hiding this comment

alamb Mar 3, 2025

Choose a reason for hiding this comment

alamb Mar 3, 2025

Choose a reason for hiding this comment

zhuqi-lucas commented Mar 4, 2025

zhuqi-lucas commented Mar 4, 2025 • edited Loading

zhuqi-lucas Mar 5, 2025

Choose a reason for hiding this comment

zhuqi-lucas Mar 5, 2025

Choose a reason for hiding this comment

zhuqi-lucas commented Mar 1, 2025 •

edited by alamb

Loading

zhuqi-lucas commented Mar 1, 2025 •

edited

Loading

zhuqi-lucas commented Mar 4, 2025 •

edited

Loading