fix: exclude manually added columns from copy #598
Merged
+121
−5
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Askir
temporarily deployed
to
internal-contributors
GitHub Actions
Inactive
Askir
force-pushed
the
jascha/allow-additional-columns
branch
from
5ce31c8 to
4bae8b1
Compare
Askir
temporarily deployed
to
internal-contributors
GitHub Actions
Inactive
jgpruitt
reviewed
Apr 1, 2025
Askir
force-pushed
the
jascha/allow-additional-columns
branch
from
4bae8b1 to
7784037
Compare
Askir
temporarily deployed
to
internal-contributors
GitHub Actions
Inactive
Askir
changed the title
Askir
force-pushed
the
jascha/allow-additional-columns
branch
from
7784037 to
f247916
Compare
Askir
temporarily deployed
to
internal-contributors
GitHub Actions
Inactive
Askir
force-pushed
the
jascha/allow-additional-columns
branch
from
f247916 to
350f583
Compare
Askir
temporarily deployed
to
internal-contributors
GitHub Actions
Inactive
cevian
requested changes
Apr 3, 2025
Agreed. But if we do this, we must construct the |
Askir
temporarily deployed
to
internal-contributors
GitHub Actions
Inactive
Askir
force-pushed
the
jascha/allow-additional-columns
branch
from
b2f5e1d to
712a710
Compare
Askir
temporarily deployed
to
internal-contributors
GitHub Actions
Inactive
Askir
force-pushed
the
jascha/allow-additional-columns
branch
from
712a710 to
fa13340
Compare
Askir
temporarily deployed
to
internal-contributors
GitHub Actions
Inactive
|
@cevian I updated the query. I don't really know how to write a test for this though, since you cant just add a column at any point in postgres, it's always added at the end. |
cevian
requested changes
Apr 9, 2025
| self.vectorizer.target_schema, | ||
| self.vectorizer.target_table, | ||
| target_columns, | ||
| ), | ||
| ) | ||
| self.copy_types = [row[0] for row in await cursor.fetchall()] |
There was a problem hiding this comment.
there is no guarantee right now of the order of the results. The query should return the column name, type and then we can finangle things into the right order
Askir
temporarily deployed
to
internal-contributors
GitHub Actions
Inactive
Askir
force-pushed
the
jascha/allow-additional-columns
branch
from
9366b7a to
312ebc4
Compare
Askir
temporarily deployed
to
internal-contributors
GitHub Actions
Inactive
cevian
approved these changes
Apr 10, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.