Skip to content
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

Guid queries on table returns all rows without value #2169

Closed
FranceyD opened this issue Sep 15, 2023 · 3 comments
Closed

Guid queries on table returns all rows without value #2169

FranceyD opened this issue Sep 15, 2023 · 3 comments
Assignees
Labels
alignment Alignment between Azurite with Azure Storage production bug Something isn't working table-query

Comments

@FranceyD
Copy link

FranceyD commented Sep 15, 2023

Which service(blob, file, queue, table) does this issue concern?

Table

Which version of the Azurite was used?

3.25, 3.25.1 or 3.26. No problem with Azurite 3.24 or lower

Where do you get Azurite? (npm, DockerHub, NuGet, Visual Studio Code Extension)

DockerHub and npm

What's the Node.js version?

14.21.3

What problem was encountered?

Doing a query with a specified guid returns all rows with the specified guid, but also all rows where there is no value in the column.
On Table storage and on Azurite < 3.25, only the rows with the specified guid were returned.

Steps to reproduce the issue?

Create a table. Add some rows only with a partition key and row key. Do a query on any empty column with any guid like this: AnyColumn eq guid'43a29cc9-cc32-4f6c-a40b-1ada7f4bc0b0'. You receive all rows of the table.

If possible, please provide the debug log using the -d parameter, replacing <pathtodebuglog> with an appropriate path for your OS, or review the instructions for docker containers:

-d "<pathtodebuglog>"

Please be sure to remove any PII or sensitive information before sharing!
The debug log will log raw request headers and bodies, so that we can replay these against Azurite using REST and create tests to validate resolution.

Have you found a mitigation/solution?

Yes, using Azurite 3.24

Edit: I Fixed an error with Azurite version. The issue appeared with Azurite 3.25, not Azurite 3.25.1

@blueww blueww added bug Something isn't working alignment Alignment between Azurite with Azure Storage production table-query labels Sep 18, 2023
@blueww blueww self-assigned this Sep 18, 2023
@blueww
Copy link
Member

blueww commented Sep 18, 2023

@notheotherben

I can repro this issue.
It looks the issue related with the recent table query PR from you.

The problem looks in function compare():

compare(context: IQueryContext, other: IQueryNode): number {

When "otherValue" is undefined, "thisValue < otherValue" and "thisValue > otherValue" both are false, so finally 0 is returned which means "thisValue" equals to "otherValue", this is also not correct.
However, when "otherValue" is undefined, it looks return -1, 0, 1 in function compare() are all wrong (since all comparison should fail in this case, include eq, gt, lt, ge, le, ne). So not sure how to fix this issue, looks any return value from the function compare() is not correct.
We might should do some null check in function evaluate() before call function compare(). But this fix will impact all kinds of value node, I am little worry about the regression risk.

@notheotherben
Would you please help to look at this issue and fix it?
Feel free to let me know if any assistance from us needed.

@notheotherben
Copy link
Member

Thank you @FranceyD and @blueww for reporting and investigating the cause of this, I've made the relevant changes to the compare() implementation in #2173 and these should be localized to the GUID implementation (and DateTime, which follows the same pattern) to ensure we avoid the risks associated with changes to evaluate(). I've expanded the test suite both to reproduce this issue (prior to the patch) and to confirm that the changes do not introduce any other problems based on the current test suite's coverage.

@blueww
Copy link
Member

blueww commented Sep 19, 2023

@notheotherben

Thanks for the quick fix!
The fix PR generally looks good to me. Just a suggestion for test case raised.

@blueww blueww closed this as completed in c962ffc Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alignment Alignment between Azurite with Azure Storage production bug Something isn't working table-query
Projects
None yet
Development

No branches or pull requests

3 participants