New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix large number search in Postgres databases #22205
Conversation
|
Since blackbox tests aren't really in a working state right now I've tested searching large numbers with every database provider.
I would appreciate hints on running the oracle docker we have in the development compose file as it fails with some networking issues on my end. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oracle and Postgres work as expected ✅
MS SQL still seems to have issues around bigInts and searching in my tests, will dive a bit deeper as you had issues getting that one running
Error: select top (@p0) [test_2].[id], [test_2].[big_int], [test_2].[int], [test_2].[string] from [test_2] where (([test_2].[id] = @p1) or ([test_2].[big_int] = @p2) or ([test_2].[int] = @p3) or LOWER([test_2].[string]) LIKE @P4) order by [test_2].[id] asc - Bigint must be safe integer or must be passed as string, saw 9223372036854775000
select top (@p0) [test_2].[id], [test_2].[big_int], [test_2].[int], [test_2].[string] from [test_2] where (([test_2].[id] = @p1) or ([test_2].[big_int] = @p2) or ([test_2].[int] = @p3) or LOWER([test_2].[string]) LIKE @P4) order by [test_2].[id] asc - The conversion of the nvarchar value '9223372036854775000' overflowed an int column.
// maybe call this "addSearchCondition"? | ||
orWhere(dbQuery: Knex.QueryBuilder, collection: string, name: string, value: NumericValue): Knex.QueryBuilder { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since orWhere
is pretty generic what do you think of something like addSearchCondition
api/src/utils/apply-query.ts
Outdated
@@ -844,11 +858,25 @@ export async function applySearch( | |||
}); | |||
} | |||
|
|||
function validateNumber(value: string, parsed: number) { | |||
if (isNaN(parsed) || !Number.isFinite(parsed)) return false; | |||
// move to its own util |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reminder to myself or hannes to move this to a util file
b514275
to
160b982
Compare
Scope
What's changed:
Potential Risks / Drawbacks
None
Review Notes / Questions
applySearch
function, I am unsure if that is the right location for it or if this should be dropped?Fixes #17946