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

sql: add eventlog for UDF relevant statements #88304

Merged

Conversation

chengxiong-ruan
Copy link
Contributor

Backport addresses #86058.

Release note: None
Release justification: low risk GA blocker

@chengxiong-ruan chengxiong-ruan requested review from a team September 20, 2022 21:35
@cockroach-teamcity
Copy link
Member

This change is Reviewable

}
event := eventpb.AlterFunctionOptions{
FunctionName: fnName.FQString(),
Options: fctx.CloseAndGetString(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

adding alter function details here is probably a overkill....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the event log json records the whole statement, so no need to book keeping redundant option details

@chengxiong-ruan chengxiong-ruan force-pushed the udf-log-event-for-udf-statements branch from 82bff78 to f62b44b Compare September 20, 2022 21:49
@postamar
Copy link

Why are there no tests?

@chengxiong-ruan
Copy link
Contributor Author

Why are there no tests?

oops, my bad, adding it now

@chengxiong-ruan chengxiong-ruan force-pushed the udf-log-event-for-udf-statements branch 2 times, most recently from 67b7741 to 355373b Compare September 21, 2022 16:21
@chengxiong-ruan
Copy link
Contributor Author

@postamar logic tests added

@chengxiong-ruan chengxiong-ruan force-pushed the udf-log-event-for-udf-statements branch from 355373b to 791b2c0 Compare September 21, 2022 16:42
Copy link

@postamar postamar left a comment

Choose a reason for hiding this comment

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

This LGTM but I don't have strong opinions about ddl event logging so you might want to solicit another review from someone who does.

@chengxiong-ruan
Copy link
Contributor Author

shipping it anyway. don't think it'd be a big deal as long as the event type are clear, desc id and ddl statement are well recorded.
TFTR!
bors r+

@craig
Copy link
Contributor

craig bot commented Sep 22, 2022

Build failed:

Backport addresses cockroachdb#86058.

Release note: None
Release justification: low risk GA blocker
@chengxiong-ruan chengxiong-ruan force-pushed the udf-log-event-for-udf-statements branch from 791b2c0 to 9f36dad Compare September 22, 2022 04:48
@chengxiong-ruan
Copy link
Contributor Author

bors r+

@craig
Copy link
Contributor

craig bot commented Sep 22, 2022

Build succeeded:

@craig craig bot merged commit 5dc6853 into cockroachdb:master Sep 22, 2022
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.

3 participants