feat(core): Add support for boolean metadata attributes in FunctionalTranslator
#7407
Conversation
…provided value type
dosubot
bot
added
the
size:L
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
dosubot
bot
added
the
auto:improvement
| const sq6 = new StructuredQuery( | ||
| "", | ||
| new Comparison(Comparators.eq, "isReleased", true) | ||
| ); |
There was a problem hiding this comment.
This isn't technically a required test addition since I didn't modify the actual Comparison logic, but since I added support for boolean types in Comparison, I thought it was worthwhile to include it here just for posterity.
| @@ -17,6 +17,7 @@ const correctQuery = new StructuredQuery( | |||
| ]), | |||
| new Comparison(Comparators.lt, "length", 180), | |||
| new Comparison(Comparators.eq, "genre", "pop"), | |||
| new Comparison(Comparators.eq, "hasLyrics", true), | |||
There was a problem hiding this comment.
Same as this comment
| describe("FunctionalTranslator", () => { | ||
| const translator = new FunctionalTranslator(); | ||
|
|
||
| describe("getAllowedComparatorsForType", () => { |
There was a problem hiding this comment.
As mentioned in the PR description, I only added the bare minimum tests needed to ensure the modification to the existing functionality did not break the visitComparison logic.
| const value = inputValuesByAttribute[attribute]; | ||
| const validDocuments = validDocumentsByComparator[comparator]; | ||
| for (const validDocument of validDocuments) { | ||
| test(`${value} -> ${comparator} -> ${validDocument.metadata[attribute]}`, () => { |
There was a problem hiding this comment.
When a test fails, the generated test name looks like input_value -> comparator -> document_value, like shown in the below screenshot for a failed eq comparator check, where the input value is value and the document value is invalid-value.
jacoblee93
changed the title
boolean metadata attributes in FunctionalTranslatorboolean metadata attributes in FunctionalTranslator
|
@nick-w-nick seems like it's breaking some types in Any way we can make this backwards compatible? |
|
@jacoblee93 The type issues have been resolved, could you re-run the CI? Thanks! |
While adding support for
booleanas a type in thecastValuefunction to allow them to be used in metadata filters that are created within aStructuredQuery, I noticed that this would cause theFunctionalTranslatorimplementation to have a gap in functionality, where booleans were technically supported as filter inputs, but would not be handled correctly during the query translation without modifications.To remedy this, I've added a new utility method called
getAllowedComparatorsForTypewhich returns the allowed comparators that can be used with a given type. This enables guards around using incorrect combinations of values and comparators.Another added benefit of this utility is that we can easily get the list of comparators for each expected type, making it possible to generate tests for every comparator for each data type automatically. I've gone ahead and added an MVP set of tests for the
FunctionalTranslator, limited to thegetAllowedComparatorsForTypeandvisitComparisonmethods, to not overcrowd the PR with unrelated changes.