feat: querying from view works #3952

killme2008 · 2024-05-15T17:21:44Z

I hereby agree to the terms of the GreptimeDB CLA.

Refer to a related PR or issue link (optional)

#3729

What's changed and what's your intention?

Make querying view works.

Main changes:

Adds ViewInfoCache for caching ViewValueInfo. @WenyXu
Destroy view metadata in DropDatabaseCursor .
Extract and rewrite all table names in view logical plan. Saves the table names into view info.
Add SubstraitPlanDecoder trait and implement it in the query crate, which decodes the logical plan from bytes, takes care of the session state, etc.
Resolve view in DfTableSourceProvider. It queries the view info and tries to decode it, then wraps the logical plan into ViewTable.
Some refactoring: move the PromQL planner to the query crate, and remove some unused errors, etc.

Todo:

Supports column names(alias) for view.
Save the optimized logical plan for view. There are some serialization issues when saving the optimized plan. @waynexia

Checklist

I have written the necessary rustdoc comments.
I have added the necessary unit tests and integration tests.
This PR requires documentation updates.

Cargo.toml

src/catalog/src/table_source.rs

src/common/meta/src/ddl/drop_database/cursor.rs

src/common/meta/src/key/view_info.rs

codecov · 2024-05-21T15:01:10Z

Codecov Report

Attention: Patch coverage is 74.96791% with 195 lines in your changes are missing coverage. Please review.

Project coverage is 85.02%. Comparing base (85a2318) to head (1999314).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3952      +/-   ##
==========================================
- Coverage   85.37%   85.02%   -0.35%     
==========================================
  Files         987      991       +4     
  Lines      172141   172822     +681     
==========================================
- Hits       146965   146943      -22     
- Misses      25176    25879     +703

tests/cases/standalone/common/view/view.sql

zhongzc

I checked, basic LGTM

src/cache/src/lib.rs

fengjiachun · 2024-05-23T09:07:10Z

@WenyXu Could you please take a look at the cache section?

WenyXu · 2024-05-24T08:52:22Z

@WenyXu Could please take a look at the cache section?

The cache part, LGTM

killme2008 · 2024-05-26T15:19:16Z

@waynexia Please take a look.

fengjiachun

Rest LGTM, @waynexia PTAL

src/query/src/query_engine/default_serializer.rs

src/query/src/error.rs

src/query/src/query_engine/context.rs

src/query/src/query_engine/default_serializer.rs

waynexia

we should not depend on test-integration. where is it used?

src/common/meta/src/key/view_info.rs

src/query/src/query_engine/context.rs

src/common/query/src/logical_plan.rs

Co-authored-by: Ruihang Xia <[email protected]>

github-actions bot added the docs-not-required This change does not impact docs. label May 15, 2024

killme2008 force-pushed the query-view branch 2 times, most recently from 51a42c0 to 2669c3f Compare May 21, 2024 14:13

killme2008 marked this pull request as ready for review May 21, 2024 14:45

killme2008 requested review from zhongzc, waynexia, MichaelScofield, evenyag and a team as code owners May 21, 2024 14:45

killme2008 requested review from WenyXu and fengjiachun May 21, 2024 14:45