[query] refactor: Table::read_plan does not need DatabendQueryContext anymore. #2088

drmingdrmer · 2021-10-06T17:50:56Z

I hereby agree to the terms of the CLA available at: https://databend.rs/policies/cla/

Summary

[query] refactor: Table::read_plan does not need DatabendQueryContext anymore.

Why:
Table is a low level concept in databend code base and should be a
dependency of some other crate such as common/planner.

This commit is one of the steps to remove the Table dependency on query.

Trait Table references DatabendQueryContext as an argument type.
In this commit it is replaced with another smaller type TableIOContext
so that in future Table can be moved out of crate query.

TableIOContext provides data-access support, exposes the runtime used
by query itself and provides two resource value: max thread number and
node list.

Add TableIOContext to provide everything a Table need to build a
plan or else.
Table::read_plan() use TableIOContext as argument to replace
DatabendQueryContext.
When calling read_plan(), a temporary TableIOContext is built out
of DatabendQueryContext.
DatabendQueryContext provides two additional supporting methods:
get_single_node_table_io_context() and
get_cluster_table_io_context().
fix: Introduce struct TableIOContext, to provide io handlers to let a table build ReadDataSourcePlan. This way the trait Table does not need to depend on DatabendQueryContext. #2072

Changelog

Improvement

Related Issues

… anymore. Why: `Table` is a low level concept in databend code base and should be a dependency of some other crate such as common/planner. This commit is one of the steps to remove the `Table` dependency on `query`. Trait `Table` references `DatabendQueryContext` as an argument type. In this commit it is replaced with another smaller type `TableIOContext` so that in future `Table` can be moved out of crate `query`. `TableIOContext` provides data-access support, exposes the runtime used by `query` itself and provides two resource value: max thread number and node list. - Add `TableIOContext` to provide everything a Table need to build a plan or else. - `Table::read_plan()` use `TableIOContext` as argument to replace `DatabendQueryContext`. - When calling `read_plan()`, a temporary `TableIOContext` is built out of DatabendQueryContext. - `DatabendQueryContext` provides two additional supporting methods: `get_single_node_table_io_context()` and `get_cluster_table_io_context()`. - fix: datafuselabs#2072

databend-bot · 2021-10-06T17:50:59Z

Thanks for the contribution!
I have applied any labels matching special text in your PR Changelog.

Please review the labels and make any necessary changes.

databend-bot · 2021-10-06T17:51:01Z

Thanks for the contribution!
I have applied any labels matching special text in your PR Changelog.

Please review the labels and make any necessary changes.

PsiACE · 2021-10-06T18:02:04Z

Looking good.

codecov-commenter · 2021-10-06T18:24:13Z

Codecov Report

Merging #2088 (8ab5910) into master (1480607) will increase coverage by 0%.
The diff coverage is 76%.

@@          Coverage Diff           @@
##           master   #2088   +/-   ##
======================================
  Coverage      68%     68%           
======================================
  Files         631     631           
  Lines       35239   35276   +37     
======================================
+ Hits        24060   24109   +49     
+ Misses      11179   11167   -12

Impacted Files	Coverage Δ
common/dal/src/data_accessor.rs	`0% <0%> (ø)`
query/src/catalogs/table.rs	`11% <ø> (ø)`
.../src/datasources/database/example/example_table.rs	`0% <ø> (ø)`
.../src/datasources/database/system/clusters_table.rs	`78% <ø> (ø)`
...y/src/datasources/database/system/configs_table.rs	`89% <ø> (ø)`
.../datasources/database/system/contributors_table.rs	`80% <ø> (ø)`
...y/src/datasources/database/system/credits_table.rs	`79% <ø> (ø)`
...src/datasources/database/system/databases_table.rs	`88% <ø> (ø)`
...y/src/datasources/database/system/engines_table.rs	`83% <ø> (ø)`
...src/datasources/database/system/functions_table.rs	`84% <ø> (ø)`
... and 40 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1480607...8ab5910. Read the comment docs.

BohuTANG · 2021-10-06T23:09:16Z

/lgtm

databend-bot · 2021-10-06T23:09:18Z

Approved! Thank you for the PR @drmingdrmer

databend-bot · 2021-10-06T23:09:47Z