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

feat: only collecting all tables on demand in influxql planner #854

Merged

Conversation

Rachelint
Copy link
Contributor

@Rachelint Rachelint commented Apr 22, 2023

Which issue does this PR close?

Closes #
Part of #830

Rationale for this change

Now, we collect and keep all tables in influxql planner even though it is unused.
It is wasteful to do this. So in this pr, I make it just do such works on demand.

What changes are included in this PR?

  • Add all_tables method in MetaProvider.
  • Just call MetaProvider::all_tables to get tables while they are actually needed(such as while influxql's SchemaProvider::table_names is called).

Are there any user-facing changes?

None.

How does this change test

Test by ut.

@Rachelint Rachelint force-pushed the enhance-all-tables-getting-of-influxql branch from 164d9c5 to cc0872a Compare April 23, 2023 01:53
query_frontend/src/influxql/planner.rs Show resolved Hide resolved
query_frontend/src/provider.rs Show resolved Hide resolved
query_frontend/src/provider.rs Outdated Show resolved Hide resolved
@Rachelint Rachelint force-pushed the enhance-all-tables-getting-of-influxql branch from 6c08d7d to 4ce7193 Compare April 24, 2023 03:26
@Rachelint Rachelint force-pushed the enhance-all-tables-getting-of-influxql branch from 4f954cb to 48e5ed0 Compare April 24, 2023 05:17
@Rachelint Rachelint force-pushed the enhance-all-tables-getting-of-influxql branch from a88ccc1 to b6dce20 Compare April 24, 2023 07:21
Copy link
Contributor

@jiacai2050 jiacai2050 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Rachelint Rachelint merged commit a465768 into apache:main Apr 24, 2023
chunshao90 pushed a commit to chunshao90/ceresdb that referenced this pull request May 15, 2023
…e#854)

* add `all_tables` interface in `MetaProvider`.

* make all tables will only be collected while calling table names of `SchemaProvider` of influxql.

* remove some useless comments and params.

* fix style and refactor some error descriptions.

* add the custom impl for `table_exists` method in influxql SchemaProvider.

* return error when schema/catalog not exist in `all_table` of `MetaProvider`.

* add comments of `all_tables` in `MetaProvider`.

* add comment for influxql planner and remove useless Result wrapping.
@Rachelint Rachelint deleted the enhance-all-tables-getting-of-influxql branch May 27, 2023 12:18
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.

2 participants