Repository navigation
fix(rdb): lowercase filter values for every operator on text fields - #463
Open
RaphaelFakhri wants to merge 1 commit into
Open
RaphaelFakhri wants to merge 1 commit into
RaphaelFakhri wants to merge 1 commit into
Conversation
This file contains hidden or 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
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.
Description of the Change
Problem
TableQuery.Initmaps text fields toLOWER(column::text), butDataFilterlowercases a string value only forlikeandnot like. A filter such as{"field":"title","operator":"=","value":"Recon Example"}comparesLOWER(title::text) = 'Recon Example'and never matches, and the same value with!=matches every row. Theinoperator already lowercases its items, so the same value behaves differently depending on the operator.Solution
DataFilterlowercases a string value for every operator when the field is a text field, using the sameisNumbericFieldcheck thatInituses to decide whether to wrap the column inLOWER(...). Numeric,*_idand*_atfields keep their values unchanged, andlike/not likestill wrap the value in%.Closes #462
Type of Change
Areas Affected
Testing and Verification
Test Configuration
Test Steps
PENTAGI_TEST_DSN.go test -tags postgres -race -count=1 -run TestTable_ ./pkg/server/rdb/frombackend.table.goand run the same command.Test Results
The new
TestTable_Query_ComparesTextFieldsWhateverTheCaseOfTheValuelives intable_live_test.go(thepostgrestier), becauseInitemitsLOWER(column::text)and sqlite cannot parse the::textcast. It filters a table holdingRecon ExampleandOtherthroughTableQuery.Querywith=,!=,likeandin.Without the fix:
With the fix, all 4 subtests pass.
go test -race ./pkg/server/rdb/ ./pkg/server/services/passes, andgolangci-lintv2.12.2 reports 0 issues onpkg/server/rdbwith and without thepostgrestag.Security Considerations
None. The value is still passed as a bound parameter.
Performance Impact
None. One
strings.ToLowerper text filter value.Documentation Updates
Deployment Notes
None.
Checklist
Code Quality
go fmtandgo vet(for Go code)pnpm run lint(for TypeScript/JavaScript code)Security
Compatibility
Documentation
Additional Notes
Filters that already worked are unchanged: a lower-case value gives the same result as before, and
like,not likeandinwere already lowercased.