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

Fix issue of returning incorrect entities when querying table with int64 values #2386

Merged
merged 1 commit into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ Table:

- Fail the insert entity request with double property whose value is greater than MAX_VALUE (Issue #2387)

Table:

- Fixed issue of returning incorrect entities when querying table with int64 values. (issue #2385)

## 2023.12 Version 3.29.0

General:
Expand Down
95 changes: 95 additions & 0 deletions src/table/persistence/QueryInterpreter/QueryNodes/BigNumberNode.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
import { IQueryContext } from "../IQueryContext";
import IQueryNode from "./IQueryNode";
import ValueNode from "./ValueNode";

/**
* Represents a constant value which is stored in its underlying JavaScript representation.
*
* This is used to hold boolean, number, and string values that are provided in the query.
* For example, the query `PartitionKey eq 'foo'` would contain a `ConstantNode` with the value `foo`.
*/
export default class BigNumberNode extends ValueNode {
get name(): string {
return "BigNumber";
}

compare(context: IQueryContext, other: IQueryNode): number {
const thisValue = this.evaluate(context) as string;
const otherValue = other.evaluate(context) as string;

if (thisValue === undefined || otherValue === undefined || otherValue === null) {
return NaN;
}

if (thisValue.startsWith("-")) {
// Compare two negative number
if (otherValue.startsWith("-")) {
return -(this.comparePositiveNumber(thisValue.substring(1), otherValue.substring(1)));
}
else {
// Could be two 0s formated with -000 and 000
if (this.trimZeros(thisValue.substring(1)).length === 0
&& this.trimZeros(otherValue).length === 0) {
return 0;
}
else {
return -1;
}
}
}
else {
// Could be two 0s formated with -000 and 000
if (otherValue.startsWith("-")) {
if (this.trimZeros(thisValue.substring(1)).length === 0
&& this.trimZeros(otherValue).length === 0) {
return 0;
}
else {
return 1;
}
}
else {
return this.comparePositiveNumber(thisValue, otherValue);
}
}
}

comparePositiveNumber(thisValue: string, otherValue: string): number {
const thisNumberValue = this.trimZeros(thisValue);
const otherNumberValue = this.trimZeros(otherValue);

if (thisNumberValue.length < otherNumberValue.length) {
return -1
}
else if (thisNumberValue.length > otherNumberValue.length) {
return 1;
}

let index = 0;
while (index < thisNumberValue.length) {
if (thisNumberValue[index] < otherNumberValue[index]) {
return -1;
}
else if (thisNumberValue[index] > otherNumberValue[index]) {
return 1;
}
++index
}

return 0;
}

trimZeros(numberString: string): string {
let index = 0;
while (index < numberString.length) {
if (numberString[index] === '0') {
++index;
}
else {
break;
}
}

return numberString.substring(index);
}
}
3 changes: 2 additions & 1 deletion src/table/persistence/QueryInterpreter/QueryParser.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { QueryLexer, QueryTokenKind } from "./QueryLexer";
import AndNode from "./QueryNodes/AndNode";
import BigNumberNode from "./QueryNodes/BigNumberNode";
import BinaryDataNode from "./QueryNodes/BinaryDataNode";
import ConstantNode from "./QueryNodes/ConstantNode";
import DateTimeNode from "./QueryNodes/DateTimeNode";
Expand Down Expand Up @@ -246,7 +247,7 @@ class QueryParser {

if (token.value!.endsWith("L")) {
// This is a "long" number, which should be represented by its string equivalent
return new ConstantNode(token.value!.substring(0, token.value!.length - 1));
return new BigNumberNode(token.value!.substring(0, token.value!.length - 1));
} else {
return new ConstantNode(parseFloat(token.value!));
}
Expand Down
103 changes: 103 additions & 0 deletions tests/table/apis/table.entity.query.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1397,4 +1397,107 @@ describe("table Entity APIs test - using Azure/data-tables", () => {

await tableClient.deleteTable();
});

it("23. should find the correct long int, @loki", async () => {
const tableClient = createAzureDataTablesClient(
testLocalAzuriteInstance,
getUniqueName("longint")
);
const partitionKey = createUniquePartitionKey("");
const testEntity: TableTestEntity =
entityFactory.createBasicEntityForTest(partitionKey);

await tableClient.createTable({ requestOptions: { timeout: 60000 } });
let result = await tableClient.createEntity(testEntity);

const anotherPartitionKey = createUniquePartitionKey("");
const anotherEntity: TableTestEntity =
entityFactory.createBasicEntityForTest(anotherPartitionKey);
anotherEntity.int64Field = { value: "1234", type: "Int64" };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to check the entity will have value as "1234L"?


result = await tableClient.createEntity(anotherEntity);
assert.ok(result.etag);

for await (const entity of tableClient
.listEntities<TableTestEntity>({
queryOptions: {
filter: `int64Field gt 1233L and int64Field lt 1235L`
}
})) {
assert.deepStrictEqual(entity.int64Field, 1234n);
}

await tableClient.deleteTable();
});

it("24. should find the correct negative long int, @loki", async () => {
const tableClient = createAzureDataTablesClient(
testLocalAzuriteInstance,
getUniqueName("longint")
);
const partitionKey = createUniquePartitionKey("");
const testEntity: TableTestEntity =
entityFactory.createBasicEntityForTest(partitionKey);
testEntity.int64Field = { value: "-12345", type: "Int64" };

await tableClient.createTable({ requestOptions: { timeout: 60000 } });
let result = await tableClient.createEntity(testEntity);

const anotherPartitionKey = createUniquePartitionKey("");
const anotherEntity: TableTestEntity =
entityFactory.createBasicEntityForTest(anotherPartitionKey);
anotherEntity.int64Field = { value: "-1234", type: "Int64" };

result = await tableClient.createEntity(anotherEntity);
assert.ok(result.etag);

for await (const entity of tableClient
.listEntities<TableTestEntity>({
queryOptions: {
filter: `int64Field lt -1233L and int64Field gt -1235L`
}
})) {
assert.deepStrictEqual(entity.int64Field, -1234n);
}

await tableClient.deleteTable();
});

it("25. should find the correct negative long int, @loki", async () => {
const tableClient = createAzureDataTablesClient(
testLocalAzuriteInstance,
getUniqueName("longint")
);
const partitionKey = createUniquePartitionKey("");
const testEntity: TableTestEntity =
entityFactory.createBasicEntityForTest(partitionKey);
testEntity.int64Field = { value: "12345", type: "Int64" };

await tableClient.createTable({ requestOptions: { timeout: 60000 } });
let result = await tableClient.createEntity(testEntity);

const anotherPartitionKey = createUniquePartitionKey("");
const anotherEntity: TableTestEntity =
entityFactory.createBasicEntityForTest(anotherPartitionKey);
anotherEntity.int64Field = { value: "-1234", type: "Int64" };

result = await tableClient.createEntity(anotherEntity);
assert.ok(result.etag);

let count = 0;

for await (const entity of tableClient
.listEntities<TableTestEntity>({
queryOptions: {
filter: `int64Field gt -1235L`
}
})) {
entity;
++count;
}

assert.deepStrictEqual(count, 2);

await tableClient.deleteTable();
});
});
4 changes: 2 additions & 2 deletions tests/table/unit/query.parser.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -260,12 +260,12 @@ describe("Query Parser", () => {
{
name: "Correctly handles longs",
originalQuery: "myInt lt 123.01L",
expectedQuery: "(lt (id myInt) \"123.01\")"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: We might can add a case to validate long query here. Might can do it after this release, as the test case already covered long.

expectedQuery: "(lt (id myInt) (BigNumber 123.01))"
},
{
name: "Correctly handles longs with a negative sign",
originalQuery: "myInt gt -123.01L",
expectedQuery: "(gt (id myInt) \"-123.01\")"
expectedQuery: "(gt (id myInt) (BigNumber -123.01))"
}
])

Expand Down
21 changes: 16 additions & 5 deletions tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"preserveConstEnums": true,
"sourceMap": true,
"newLine": "LF",
"target": "es2017",
"target": "ES2020",
"moduleResolution": "node",
"noUnusedLocals": true,
"noUnusedParameters": false,
Expand All @@ -16,13 +16,24 @@
"declarationMap": true,
"importHelpers": true,
"declarationDir": "./typings",
"lib": ["es5", "es6", "es7", "esnext", "dom"],
"lib": [
"es5",
"es6",
"es7",
"esnext",
"dom"
],
"esModuleInterop": true,
"downlevelIteration": true,
"useUnknownInCatchVariables": false,
"skipLibCheck": true,
},
"compileOnSave": true,
"exclude": ["node_modules"],
"include": ["./src/**/*.ts", "./tests/**/*.ts"]
}
"exclude": [
"node_modules"
],
"include": [
"./src/**/*.ts",
"./tests/**/*.ts"
]
}
Loading