Make parameter length based on underlying column length except for complex ops
Why make this change?
Fixes issue #2626
What is this change?
Where a parameter is linked to an underlying column in the data model the length is stored When the query is executed the length is used to ensure parameters have consistent lengths
For certain operations the value is padded with escape characters or %s, these need to be allowed for. In those cases the length is set to -1 (max) in order that the extra characters are allowed for AND consistent lengths are maintained
How was this tested?
- [x] Integration Tests
Checking the resultant query is using the right length was done using trace. Not sure with the architecture whether it would be possible to mock at that level.
Sample Request(s)
See new tests TestFilterParamForStringFilterWorkWithComplexOp and TestFilterParamForStringFilterWorkWithNotContains
It’s to cover the edge case of matching with the LiKE that you get with contains and starts with, and also the escaping of values that occurs in the predicates. If you don’t you can end up with false positives and false negatives
Simon Sabin
From: aaronburtle @.> Sent: Thursday, March 20, 2025 9:19:26 PM To: Azure/data-api-builder @.> Cc: Simon Sabin @.>; Author @.> Subject: Re: [Azure/data-api-builder] Make parameter length based on underlying column length except for complex ops (PR #2627)
@aaronburtle commented on this pull request.
In src/Service.Tests/DatabaseSchema-MsSql.sqlhttps://github.com/Azure/data-api-builder/pull/2627#discussion_r2006469243:
@@ -514,7 +514,7 @@ SET IDENTITY_INSERT books ON INSERT INTO books(id, title, publisher_id) VALUES (1, 'Awesome book', 1234), (2, 'Also Awesome book', 1234), -(3, 'Great wall of china explained', 2345), +(3, 'Great wall of china explained]', 2345),
I am curious, why the extra "]"?
— Reply to this email directly, view it on GitHubhttps://github.com/Azure/data-api-builder/pull/2627#pullrequestreview-2704137237, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAJHM2Z7OZVBM5HTNT3F6SD2VMWF5AVCNFSM6AAAAABZOI6YHOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDOMBUGEZTOMRTG4. You are receiving this because you authored the thread.Message ID: @.***>
applied suggested changes
/azp run
Azure Pipelines successfully started running 6 pipeline(s).
Pipeline failures are just whitespace format.
/azp run
Azure Pipelines successfully started running 6 pipeline(s).
/azp run
Azure Pipelines successfully started running 6 pipeline(s).
/azp run
Azure Pipelines successfully started running 6 pipeline(s).
/azp run
Azure Pipelines successfully started running 6 pipeline(s).
/azp run
Azure Pipelines successfully started running 6 pipeline(s).
/azp run
Azure Pipelines successfully started running 6 pipeline(s).
/azp run
Azure Pipelines successfully started running 6 pipeline(s).
whats the state of this?
@Aniruddh25 @aaronburtle whats needed to get this merged in?
Running the tests to ensure they pass. Will merge this soon.
/azp run
Azure Pipelines successfully started running 6 pipeline(s).
/azp run
Azure Pipelines successfully started running 6 pipeline(s).