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 setting of 'optimizer' GUC to the extension #1128

Open
wants to merge 4 commits into
base: feature/ADBDEV-6552
Choose a base branch
from

Conversation

whitehawk
Copy link

@whitehawk whitehawk commented Nov 20, 2024

Move setting of 'optimizer' GUC to the extension

Problem:
File 'guc_gp.c' contained a compile-time dependency on Orca, related to
'optimizer' GUC. It prevented moving of all Orca's code into a shared lib and
making its work transparent to the GPDB core.

This patch:

  1. Sets the default value for 'optimizer' GUC to 'false'.
  2. Adds enabling of 'optimizer' GUC in gp_orca extension shared lib.
  3. Updates check in 'check_optimizer()' function. Now it utilizes runtime check
    of planner_hook presence to distinguish if an external planner is available or
    not.

Note: the semantic of 'optimizer' GUC has changed. Previously it was used
solely to enable & disable Orca. Now it is used to enable & disable any general
external planner. The name of GUC hasn't been changed, because too many places
in code and infrastructure rely on it, and we aim not to blow up the size of
the patch.

@whitehawk whitehawk marked this pull request as ready for review November 20, 2024 04:06
@dkovalev1
Copy link

It looks good. The only question left is: will there be a possibility to have more than two optimizers? As we know, PG12 also has geqo at least. I understand that this may be premature, and not relevant for this particular task but anyway.

@whitehawk
Copy link
Author

It looks good. The only question left is: will there be a possibility to have more than two optimizers? As we know, PG12 also has geqo at least. I understand that this may be premature, and not relevant for this particular task but anyway.

We can support many planners, but not simultaneously in runtime. The decision which one to use now can be done by switching between shared libraries. I think that now we do not aim to support Standard + 2 external planners at the same time.

andr-sokolov
andr-sokolov previously approved these changes Nov 21, 2024
@andr-sokolov andr-sokolov dismissed their stale review November 21, 2024 13:23

Conflicting files

@whitehawk
Copy link
Author

Fixed conflicts.

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.

4 participants