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

release-23.2: changefeedccl: Correctly handle stable CDC query functions #117577

Open
wants to merge 1 commit into
base: release-23.2
Choose a base branch
from

Conversation

blathers-crl[bot]
Copy link

@blathers-crl blathers-crl bot commented Jan 9, 2024

Backport 1/1 commits from #117520 on behalf of @miretskiy.

/cc @cockroachdb/release


Custom CDC query functions rely on having "annotations" configured. Prior to this change, these annotations were configured when the CDC query was being evaluated (for each event). However, CDC query also needs to be evaluated when e.g. the changefeed is being created. In this case, the correct annotations were not configured, resulting in failure to create changefeed that use stable, custom CDC function. At this point, there is only one such function: changefeed_creation_time.

This PR refactors and cleans up how semantic and evalution contexts are configured. This now happens in a single place -- namely the withPlanner helper so that correct information is configured at all times.

Fixes #115245

Release note (enterprise change): Fix CDC query to correctly handle changefeed_creation_time() function.


Release justification:

Custom CDC query functions rely on having "annotations" configured.
Prior to this change, these annotations were configured when the
CDC query was being evaluated (for each event).  However, CDC
query also needs to be evaluated when e.g. the changefeed is
being created.  In this case, the correct annotations were not
configured, resulting in failure to create changefeed
that use stable, custom CDC function.  At this point, there
is only one such function: `changefeed_creation_time`.

This PR refactors and cleans up how semantic and evalution
contexts are configured.  This now happens in a single place --
namely the `withPlanner` helper so that correct information
is configured at all times.

Fixes #115245

Release note (enterprise change): Fix CDC query to correctly
handle `changefeed_creation_time()` function.
@blathers-crl blathers-crl bot requested a review from a team as a code owner January 9, 2024 20:31
@blathers-crl blathers-crl bot force-pushed the blathers/backport-release-23.2-117520 branch from cbe6330 to 316db9d Compare January 9, 2024 20:31
@blathers-crl blathers-crl bot requested review from jayshrivastava and removed request for a team January 9, 2024 20:31
@blathers-crl blathers-crl bot added blathers-backport This is a backport that Blathers created automatically. O-robot Originated from a bot. labels Jan 9, 2024
Copy link
Author

blathers-crl bot commented Jan 9, 2024

Thanks for opening a backport.

Please check the backport criteria before merging:

  • Backports should only be created for serious
    issues
    or test-only changes.
  • Backports should not break backwards-compatibility.
  • Backports should change as little code as possible.
  • Backports should not change on-disk formats or node communication protocols.
  • Backports should not add new functionality (except as defined
    here).
  • Backports must not add, edit, or otherwise modify cluster versions; or add version gates.
  • All backports must be reviewed by the owning areas TL and one additional
    TL. For more information as to how that review should be conducted, please consult the backport
    policy
    .
If your backport adds new functionality, please ensure that the following additional criteria are satisfied:
  • There is a high priority need for the functionality that cannot wait until the next release and is difficult to address in another way.
  • The new functionality is additive-only and only runs for clusters which have specifically “opted in” to it (e.g. by a cluster setting).
  • New code is protected by a conditional check that is trivial to verify and ensures that it only runs for opt-in clusters. State changes must be further protected such that nodes running old binaries will not be negatively impacted by the new state (with a mixed version test added).
  • The PM and TL on the team that owns the changed code have signed off that the change obeys the above rules.
  • Your backport must be accompanied by a post to the appropriate Slack
    channel (#db-backports-point-releases or #db-backports-XX-X-release) for awareness and discussion.

Also, please add a brief release justification to the body of your PR to justify this
backport.

@blathers-crl blathers-crl bot added the backport Label PR's that are backports to older release branches label Jan 9, 2024
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link

Reminder: it has been 3 weeks please merge or close your backport!

@cockroach-teamcity
Copy link
Member

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Yevgeniy Miretskiy seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link

github-actions bot commented Apr 8, 2024

Reminder: it has been 3 weeks please merge or close your backport!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Label PR's that are backports to older release branches blathers-backport This is a backport that Blathers created automatically. no-backport-pr-activity O-robot Originated from a bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants