-
Notifications
You must be signed in to change notification settings - Fork 15
[DB-18017] Completed the implementation of TraverseAndExtractDefNames… #2992
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
Conversation
…FromDefElem to fetch the defNames values for other scenarios where the arg is not string
options:{def_elem:{defname:"security_barrier" arg:{string:{sval:"false"}} defaction:DEFELEM_UNSPEC location:57}} | ||
Extract all defnames from the def_eleme node | ||
*/ | ||
func ProtoAsAIndirectionNode(msg protoreflect.Message) (*pg_query.A_Indirection, bool) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just function rename
if tc := arg.GetTypeCast(); tc != nil { | ||
return NormalizeDefElemArgToString(tc.Arg) | ||
} | ||
return arg.String() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't a complete normalization, right?
As there could be other arg types like expression, function call
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any other known cases for def elements?
I added what i could find/think of.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added log line for fallback
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…FromDefElem to fetch the defNames values for other scenarios where the arg is not string
Describe the changes in this pull request
Describe if there are any user-facing changes
NA
How was this pull request tested?
Added unit tests.
Does your PR have changes in callhome/yugabyted payloads? If so, is the payload version incremented?
Does your PR have changes that can cause upgrade issues?