Skip to content

refactor: Clean up list tables method#13331

Merged
kodiakhq[bot] merged 2 commits into
cloudquery:mainfrom
erezrokah:refactor/clean_up_list_tables
Aug 25, 2023
Merged

refactor: Clean up list tables method#13331
kodiakhq[bot] merged 2 commits into
cloudquery:mainfrom
erezrokah:refactor/clean_up_list_tables

Conversation

@erezrokah
Copy link
Copy Markdown
Member

Summary

exclude was always passed as an empty list. Also we can use tables.TableNames()

where = fmt.Sprintf("AND pg_class.relname IN (%s)", c.inClause(include))
}
if len(exclude) > 0 {
where = fmt.Sprintf("AND pg_class.relname NOT IN (%s)", c.inClause(exclude))
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think there was a bug here. There should be a space between the include and exclude AND clauses

@erezrokah erezrokah requested review from candiduslynx and removed request for yevgenypats August 25, 2023 12:53
@erezrokah erezrokah added the automerge Automatically merge once required checks pass label Aug 25, 2023
@kodiakhq kodiakhq Bot merged commit 11b51dd into cloudquery:main Aug 25, 2023
@erezrokah erezrokah deleted the refactor/clean_up_list_tables branch August 25, 2023 13:00
kodiakhq Bot pushed a commit that referenced this pull request Aug 28, 2023

#### Summary

Follow up to #13331.
On AWS this means we search an array of ~600 items. Since at the moment we only have a single `MigrateTableBatch` call for all the tables, no point of having the `WHERE` clause (and even with multiple migrate batches might be faster just to query everything).

See also https://www.postgresql.org/docs/current/arrays.html:
![image](https://github.com/cloudquery/cloudquery/assets/26760571/698909ad-3110-42bd-a9be-5f6364514483)
We can't really follow that tip though

<!--
hermanschaaf pushed a commit that referenced this pull request Aug 29, 2023

#### Summary

Follow up to #13331.
On AWS this means we search an array of ~600 items. Since at the moment we only have a single `MigrateTableBatch` call for all the tables, no point of having the `WHERE` clause (and even with multiple migrate batches might be faster just to query everything).

See also https://www.postgresql.org/docs/current/arrays.html:
![image](https://github.com/cloudquery/cloudquery/assets/26760571/698909ad-3110-42bd-a9be-5f6364514483)
We can't really follow that tip though

<!--
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge Automatically merge once required checks pass

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants