Skip to content

Fix: sort document reference by long type id #8673

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Dec 17, 2024
Merged

Fix: sort document reference by long type id #8673

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
dc2f945
handle numeric document ids
milaGGL Dec 10, 2024
f8c91ee
handle 64bit longs in document key
milaGGL Dec 10, 2024
0c6bd90
format
milaGGL Dec 10, 2024
f9a3df1
Update path.ts
milaGGL Dec 10, 2024
85d78a3
add largest long numbers
milaGGL Dec 10, 2024
ff54671
wrap documentIds into ''
milaGGL Dec 12, 2024
32cdcb0
add "__id" to tests
milaGGL Dec 16, 2024
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
12 changes: 11 additions & 1 deletion packages/firestore/src/local/memory_remote_document_cache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ interface MemoryRemoteDocumentCacheEntry {
size: number;
}

/**
* The smallest value representable by a 64-bit signed integer (long).
*/
const MIN_LONG_VALUE = '-9223372036854775808';

type DocumentEntryMap = SortedMap<DocumentKey, MemoryRemoteDocumentCacheEntry>;
function documentEntryMap(): DocumentEntryMap {
return new SortedMap<DocumentKey, MemoryRemoteDocumentCacheEntry>(
Expand Down Expand Up @@ -171,7 +176,12 @@ class MemoryRemoteDocumentCacheImpl implements MemoryRemoteDocumentCache {
// Documents are ordered by key, so we can use a prefix scan to narrow down
// the documents we need to match the query against.
const collectionPath = query.path;
const prefix = new DocumentKey(collectionPath.child(''));
// Document keys are ordered first by numeric value ("__id<Long>__"),
// then lexicographically by string value. Start the iterator at the minimum
// possible Document key value.
const prefix = new DocumentKey(
collectionPath.child('__id' + MIN_LONG_VALUE + '__')
);
const iterator = this.docs.getIteratorFrom(prefix);
while (iterator.hasNext()) {
const {
Expand Down
55 changes: 44 additions & 11 deletions packages/firestore/src/model/path.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
* limitations under the License.
*/

import { Integer } from '@firebase/webchannel-wrapper/bloom-blob';

import { debugAssert, fail } from '../util/assert';
import { Code, FirestoreError } from '../util/error';

Expand Down Expand Up @@ -163,28 +165,59 @@ abstract class BasePath<B extends BasePath<B>> {
return this.segments.slice(this.offset, this.limit());
}

/**
* Compare 2 paths segment by segment, prioritizing numeric IDs
* (e.g., "__id123__") in numeric ascending order, followed by string
* segments in lexicographical order.
*/
static comparator<T extends BasePath<T>>(
p1: BasePath<T>,
p2: BasePath<T>
): number {
const len = Math.min(p1.length, p2.length);
for (let i = 0; i < len; i++) {
const left = p1.get(i);
const right = p2.get(i);
if (left < right) {
return -1;
}
if (left > right) {
return 1;
const comparison = BasePath.compareSegments(p1.get(i), p2.get(i));
if (comparison !== 0) {
return comparison;
}
}
if (p1.length < p2.length) {
return Math.sign(p1.length - p2.length);
}

private static compareSegments(lhs: string, rhs: string): number {
const isLhsNumeric = BasePath.isNumericId(lhs);
const isRhsNumeric = BasePath.isNumericId(rhs);

if (isLhsNumeric && !isRhsNumeric) {
// Only lhs is numeric
return -1;
}
if (p1.length > p2.length) {
} else if (!isLhsNumeric && isRhsNumeric) {
// Only rhs is numeric
return 1;
} else if (isLhsNumeric && isRhsNumeric) {
// both numeric
return BasePath.extractNumericId(lhs).compare(
BasePath.extractNumericId(rhs)
);
} else {
// both non-numeric
if (lhs < rhs) {
return -1;
}
if (lhs > rhs) {
return 1;
}
return 0;
}
return 0;
}

// Checks if a segment is a numeric ID (starts with "__id" and ends with "__").
private static isNumericId(segment: string): boolean {
return segment.startsWith('__id') && segment.endsWith('__');
}

private static extractNumericId(segment: string): Integer {
return Integer.fromString(segment.substring(4, segment.length - 2));
}
}

Expand Down
181 changes: 180 additions & 1 deletion packages/firestore/test/integration/api/database.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ import {
withTestDocAndInitialData,
withNamedTestDbsOrSkipUnlessUsingEmulator,
toDataArray,
checkOnlineAndOfflineResultsMatch
checkOnlineAndOfflineResultsMatch,
toIds
} from '../util/helpers';
import { DEFAULT_SETTINGS, DEFAULT_PROJECT_ID } from '../util/settings';

Expand Down Expand Up @@ -2245,4 +2246,182 @@ apiDescribe('Database', persistence => {
});
});
});

describe('sort documents by DocumentId', () => {
it('snapshot listener sorts query by DocumentId same way as get query', async () => {
const testDocs = {
'A': { a: 1 },
'a': { a: 1 },
'Aa': { a: 1 },
'7': { a: 1 },
'12': { a: 1 },
'__id7__': { a: 1 },
'__id12__': { a: 1 },
'__id-2__': { a: 1 },
'_id1__': { a: 1 },
'__id1_': { a: 1 },
'__id': { a: 1 },
// largest long numbers
'__id9223372036854775807__': { a: 1 },
'__id9223372036854775806__': { a: 1 },
// smallest long numbers
'__id-9223372036854775808__': { a: 1 },
'__id-9223372036854775807__': { a: 1 }
};

return withTestCollection(persistence, testDocs, async collectionRef => {
const orderedQuery = query(collectionRef, orderBy(documentId()));
const expectedDocs = [
'__id-9223372036854775808__',
'__id-9223372036854775807__',
'__id-2__',
'__id7__',
'__id12__',
'__id9223372036854775806__',
'__id9223372036854775807__',
'12',
'7',
'A',
'Aa',
'__id',
'__id1_',
'_id1__',
'a'
];

const getSnapshot = await getDocsFromServer(orderedQuery);
expect(toIds(getSnapshot)).to.deep.equal(expectedDocs);

const storeEvent = new EventsAccumulator<QuerySnapshot>();
const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
const watchSnapshot = await storeEvent.awaitEvent();
expect(toIds(watchSnapshot)).to.deep.equal(expectedDocs);

unsubscribe();
});
});

it('snapshot listener sorts filtered query by DocumentId same way as get query', async () => {
const testDocs = {
'A': { a: 1 },
'a': { a: 1 },
'Aa': { a: 1 },
'7': { a: 1 },
'12': { a: 1 },
'__id7__': { a: 1 },
'__id12__': { a: 1 },
'__id-2__': { a: 1 },
'_id1__': { a: 1 },
'__id1_': { a: 1 },
'__id': { a: 1 },
// largest long numbers
'__id9223372036854775807__': { a: 1 },
'__id9223372036854775806__': { a: 1 },
// smallest long numbers
'__id-9223372036854775808__': { a: 1 },
'__id-9223372036854775807__': { a: 1 }
};

return withTestCollection(persistence, testDocs, async collectionRef => {
const filteredQuery = query(
collectionRef,
orderBy(documentId()),
where(documentId(), '>', '__id7__'),
where(documentId(), '<=', 'Aa')
);
const expectedDocs = [
'__id12__',
'__id9223372036854775806__',
'__id9223372036854775807__',
'12',
'7',
'A',
'Aa'
];

const getSnapshot = await getDocsFromServer(filteredQuery);
expect(toIds(getSnapshot)).to.deep.equal(expectedDocs);

const storeEvent = new EventsAccumulator<QuerySnapshot>();
const unsubscribe = onSnapshot(filteredQuery, storeEvent.storeEvent);
const watchSnapshot = await storeEvent.awaitEvent();
expect(toIds(watchSnapshot)).to.deep.equal(expectedDocs);
unsubscribe();
});
});

// eslint-disable-next-line no-restricted-properties
(persistence.gc === 'lru' ? describe : describe.skip)('offline', () => {
it('SDK orders query the same way online and offline', async () => {
const testDocs = {
'A': { a: 1 },
'a': { a: 1 },
'Aa': { a: 1 },
'7': { a: 1 },
'12': { a: 1 },
'__id7__': { a: 1 },
'__id12__': { a: 1 },
'__id-2__': { a: 1 },
'_id1__': { a: 1 },
'__id1_': { a: 1 },
'__id': { a: 1 },
// largest long numbers
'__id9223372036854775807__': { a: 1 },
'__id9223372036854775806__': { a: 1 },
// smallest long numbers
'__id-9223372036854775808__': { a: 1 },
'__id-9223372036854775807__': { a: 1 }
};

return withTestCollection(
persistence,
testDocs,
async collectionRef => {
const orderedQuery = query(collectionRef, orderBy(documentId()));
let expectedDocs = [
'__id-9223372036854775808__',
'__id-9223372036854775807__',
'__id-2__',
'__id7__',
'__id12__',
'__id9223372036854775806__',
'__id9223372036854775807__',
'12',
'7',
'A',
'Aa',
'__id',
'__id1_',
'_id1__',
'a'
];
await checkOnlineAndOfflineResultsMatch(
orderedQuery,
...expectedDocs
);

const filteredQuery = query(
collectionRef,
orderBy(documentId()),
where(documentId(), '>', '__id7__'),
where(documentId(), '<=', 'Aa')
);
expectedDocs = [
'__id12__',
'__id9223372036854775806__',
'__id9223372036854775807__',
'12',
'7',
'A',
'Aa'
];
await checkOnlineAndOfflineResultsMatch(
filteredQuery,
...expectedDocs
);
}
);
});
});
});
});
Loading