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

WIP: order latest first COMPASS-6706 #6361

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
Draft

WIP: order latest first COMPASS-6706 #6361

wants to merge 11 commits into from

Conversation

lerouxb
Copy link
Contributor

@lerouxb lerouxb commented Oct 14, 2024

TODO:

  • make sure we send the default order through to all relevant places. ie. explain

Super hacky, will need lots of cleanup but at least we can try it out.

{ _id: -1 } for normal collections, undefined whenever the _id_ index does not exist. ie. timeseries and views.

@@ -7,7 +7,7 @@ export async function countDocuments(
dataService: DataService,
preferences: PreferencesAccess,
ns: string,
filter: BSONObject,
filter: BSONObject | undefined,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

drive-by

export const fetchDocuments: (
dataService: DataService,
serverVersion: string,
isDataLake: boolean,
fetchDocumentsOptions: FetchDocumentsOptions,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is getting ugly 😆

@lerouxb lerouxb changed the title order latest first WIP: order latest first Oct 16, 2024
@lerouxb lerouxb changed the title WIP: order latest first WIP: order latest first COMPASS-6706 Oct 16, 2024
collectionStats: CollectionStats | null
): Sort | undefined {
if (collectionStats?.index_details._id_) {
return { _id: -1 };
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm now not sure. It seems like { $natural: -1 } is actually just as fast as { $natural: 1 } *(the default) at least in most cases. If it is in MOST cases we'd have to just change this logic and return { $natural: -1 }. If it works and is fine in ALL cases, then most of this PR is probably not needed. We could just always use that as a default and we don't even need this function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But then.. I don't think we want $natural at all because it's just how documents are sorted in wiredtiger, which means any order really. For example, an update can bump an old document to the top of the natural order.

@@ -94,6 +94,7 @@
"hadron-type-checker": "^7.2.3",
"jsondiffpatch": "^0.5.0",
"lodash": "^4.17.21",
"mongodb": "^6.9.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just for the Sort type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant