Skip to content

fix: Drop include clause from list tables#13332

Merged
kodiakhq[bot] merged 3 commits into
cloudquery:mainfrom
erezrokah:refactor/drop_include_clause
Aug 28, 2023
Merged

fix: Drop include clause from list tables#13332
kodiakhq[bot] merged 3 commits into
cloudquery:mainfrom
erezrokah:refactor/drop_include_clause

Conversation

@erezrokah
Copy link
Copy Markdown
Member

@erezrokah erezrokah commented Aug 25, 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
We can't really follow that tip though

@erezrokah erezrokah requested a review from yevgenypats as a code owner August 25, 2023 13:08
@erezrokah erezrokah requested review from a team, hermanschaaf and mnorbury and removed request for a team, mnorbury and yevgenypats August 25, 2023 13:09
@hermanschaaf
Copy link
Copy Markdown
Contributor

Looks good, though I don't know if this should be marked as a refactor, as it does change behavior?

Comment thread plugins/destination/postgresql/client/list_tables.go Outdated
Co-authored-by: Alex Shcherbakov <candiduslynx@users.noreply.github.com>
@erezrokah
Copy link
Copy Markdown
Member Author

Looks good, though I don't know if this should be marked as a refactor, as it does change behavior?

Yeah, I'm not sure. It shouldn't have any user facing impact, but maybe I'll do a fix to play it safe

@erezrokah erezrokah changed the title refactor: Drop include clause from list tables fix: Drop include clause from list tables Aug 28, 2023
@erezrokah erezrokah added the automerge Automatically merge once required checks pass label Aug 28, 2023
@kodiakhq kodiakhq Bot merged commit 7b6369e into cloudquery:main Aug 28, 2023
kodiakhq Bot pushed a commit that referenced this pull request Aug 29, 2023
🤖 I have created a release *beep* *boop*
---


## [6.0.1](plugins-destination-postgresql-v6.0.0...plugins-destination-postgresql-v6.0.1) (2023-08-29)


### Bug Fixes

* **deps:** Update `github.com/cloudquery/arrow/go/v13` to `github.com/apache/arrow/go/v14` ([#13341](#13341)) ([feb8f87](feb8f87))
* **deps:** Update module github.com/cloudquery/plugin-sdk/v4 to v4.5.6 ([#13345](#13345)) ([a995a05](a995a05))
* Drop include clause from list tables ([#13332](#13332)) ([7b6369e](7b6369e))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
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

<!--
kodiakhq Bot pushed a commit that referenced this pull request Aug 30, 2023
Somehow the change in #13332 got reverted, I think unintentionally.

This is however necessary as part of a fix for #13412
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.

4 participants