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

Move processing of orderBy to runtime #2357

Open
wants to merge 28 commits into
base: main
Choose a base branch
from
Open

Move processing of orderBy to runtime #2357

wants to merge 28 commits into from

Conversation

benjie
Copy link
Member

@benjie benjie commented Feb 4, 2025

Description

Relates to:

Previously dynamic ordering would need to be evaluated at plantime, causing forks in the plan. This could get expensive.

This PR changes orderBy to be evaluated at runtime, allowing operation plans to be reused in more cases.

Performance impact

Small runtime cost, but helps avoid re-planning of operations.

Security impact

Helps to reduce the surface area of planning attacks.

Checklist

  • My code matches the project's code style and yarn lint:fix passes.
  • I've added tests for the new feature, and yarn test passes.
  • I have detailed the new feature in the relevant documentation.
  • I have added this feature to 'Pending' in the RELEASE_NOTES.md file (if one exists).
  • If this is a breaking change I've explained why.

Copy link

changeset-bot bot commented Feb 4, 2025

🦋 Changeset detected

Latest commit: 84395b9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages
Name Type
graphile-build Patch
graphile-build-pg Patch
postgraphile Patch
@dataplan/pg Patch
graphile-utils Patch
pgl Patch
graphile Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@benjie benjie force-pushed the export-extensions-directly branch from 473788d to ca506fc Compare February 4, 2025 15:42
@benjie benjie force-pushed the export-extensions-directly branch from ca506fc to fa005eb Compare February 4, 2025 19:33
@benjie benjie force-pushed the runtime-orderby branch 2 times, most recently from c3b5622 to 0bd16b6 Compare February 4, 2025 20:03
Base automatically changed from export-extensions-directly to main February 5, 2025 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🌳 Triage
Development

Successfully merging this pull request may close these issues.

1 participant