fix: sanitize foreign schemas #1058
Merged
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
|
Note: I disabled some lint checks because we are (hopefully) moving to use prettier in nodejs in #1042 and these checks disagreed with prettier's formatting. |
wjones127
reviewed
Mar 4, 2024
There was a problem hiding this comment.
What do you think of putting the sanitize* functions in their own module / file?
There was a problem hiding this comment.
Yeah, that's a good idea. I've put these into a sanitize.ts file.
wjones127
approved these changes
Mar 4, 2024
eddyxu
approved these changes
Mar 4, 2024
westonpace
force-pushed
the
fix/sanitize-foreign-schemas
branch
from
bb1a773
to
f7527d3
Compare
wjones127
approved these changes
Mar 4, 2024
raghavdixit99
pushed a commit
to raghavdixit99/lancedb
that referenced
this pull request
Apr 5, 2024
Arrow-js uses brittle `instanceof` checks throughout the code base. These fail unless the library instance that produced the object matches exactly the same instance the vectordb is using. At a minimum, this means that a user using arrow version 15 (or any version that doesn't match exactly the version that vectordb is using) will get strange errors when they try and use vectordb. However, there are even cases where the versions can be perfectly identical, and the instanceof check still fails. One such example is when using `vite` (e.g. vitejs/vite#3910) This PR solves the problem in a rather brute force, but workable, fashion. If we encounter a schema that does not pass the `instanceof` check then we will attempt to sanitize that schema by traversing the object and, if it has all the correct properties, constructing an appropriate `Schema` instance via deep cloning.
raghavdixit99
pushed a commit
to raghavdixit99/lancedb
that referenced
this pull request
Apr 5, 2024
Arrow-js uses brittle `instanceof` checks throughout the code base. These fail unless the library instance that produced the object matches exactly the same instance the vectordb is using. At a minimum, this means that a user using arrow version 15 (or any version that doesn't match exactly the version that vectordb is using) will get strange errors when they try and use vectordb. However, there are even cases where the versions can be perfectly identical, and the instanceof check still fails. One such example is when using `vite` (e.g. vitejs/vite#3910) This PR solves the problem in a rather brute force, but workable, fashion. If we encounter a schema that does not pass the `instanceof` check then we will attempt to sanitize that schema by traversing the object and, if it has all the correct properties, constructing an appropriate `Schema` instance via deep cloning.
westonpace
added a commit
that referenced
this pull request
Apr 5, 2024
Arrow-js uses brittle `instanceof` checks throughout the code base. These fail unless the library instance that produced the object matches exactly the same instance the vectordb is using. At a minimum, this means that a user using arrow version 15 (or any version that doesn't match exactly the version that vectordb is using) will get strange errors when they try and use vectordb. However, there are even cases where the versions can be perfectly identical, and the instanceof check still fails. One such example is when using `vite` (e.g. vitejs/vite#3910) This PR solves the problem in a rather brute force, but workable, fashion. If we encounter a schema that does not pass the `instanceof` check then we will attempt to sanitize that schema by traversing the object and, if it has all the correct properties, constructing an appropriate `Schema` instance via deep cloning.
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.
Arrow-js uses brittle
instanceofchecks throughout the code base. These fail unless the library instance that produced the object matches exactly the same instance the vectordb is using. At a minimum, this means that a user using arrow version 15 (or any version that doesn't match exactly the version that vectordb is using) will get strange errors when they try and use vectordb.However, there are even cases where the versions can be perfectly identical, and the instanceof check still fails. One such example is when using
vite(e.g. vitejs/vite#3910)This PR solves the problem in a rather brute force, but workable, fashion. If we encounter a schema that does not pass the
instanceofcheck then we will attempt to sanitize that schema by traversing the object and, if it has all the correct properties, constructing an appropriateSchemainstance via deep cloning.