Re: [HACKERS] make more use of RoleSpec struct
On 12/28/16 9:53 AM, Alvaro Herrera wrote: > Peter Eisentraut wrote: >> Here is a small cleanup patch to make more use of the RoleSpec >> struct/node throughout the parser to avoid casts and make the code more >> self-documenting. > > This makes sense to me. I started to do this at some point when I was > writing RoleSpec; eventually got annoyed about fixing the fallout (I was > working to get somebody else's patch committed) and went back to how it > is. Glad to see it done. > > The only functional issue might be the removal of the IsA() checks. If > we don't cast any Node before passing it to any of those functions, > there should be no problem because any misfeasance will be reported as a > compile-time warning. Perhaps it's worth adding IsA() checks in loops > such as the one in roleSpecsToIds(). Committed with an extra Assert there for good measure. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] make more use of RoleSpec struct
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > Stephen Frost wrote: > > * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > > > > The only functional issue might be the removal of the IsA() checks. If > > > we don't cast any Node before passing it to any of those functions, > > > there should be no problem because any misfeasance will be reported as a > > > compile-time warning. Perhaps it's worth adding IsA() checks in loops > > > such as the one in roleSpecsToIds(). > > > > Maybe inside of an Assert() though..? > > Yeah, I was of two minds when writing that para. Maybe roleSpecsToIds > (and all similar functions, if any others exist) has a limited enough > caller base that it's trivial to audit that no bogus callsite exists. Well, I think an Assert() is fine to make sure someone doesn't add a new call that passes in something else where we don't have the compile-time type checking happening. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] make more use of RoleSpec struct
Stephen Frost wrote: > * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > > The only functional issue might be the removal of the IsA() checks. If > > we don't cast any Node before passing it to any of those functions, > > there should be no problem because any misfeasance will be reported as a > > compile-time warning. Perhaps it's worth adding IsA() checks in loops > > such as the one in roleSpecsToIds(). > > Maybe inside of an Assert() though..? Yeah, I was of two minds when writing that para. Maybe roleSpecsToIds (and all similar functions, if any others exist) has a limited enough caller base that it's trivial to audit that no bogus callsite exists. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] make more use of RoleSpec struct
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > Peter Eisentraut wrote: > > Here is a small cleanup patch to make more use of the RoleSpec > > struct/node throughout the parser to avoid casts and make the code more > > self-documenting. > > This makes sense to me. I started to do this at some point when I was > writing RoleSpec; eventually got annoyed about fixing the fallout (I was > working to get somebody else's patch committed) and went back to how it > is. Glad to see it done. > > The only functional issue might be the removal of the IsA() checks. If > we don't cast any Node before passing it to any of those functions, > there should be no problem because any misfeasance will be reported as a > compile-time warning. Perhaps it's worth adding IsA() checks in loops > such as the one in roleSpecsToIds(). Maybe inside of an Assert() though..? Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] make more use of RoleSpec struct
Peter Eisentraut wrote: > Here is a small cleanup patch to make more use of the RoleSpec > struct/node throughout the parser to avoid casts and make the code more > self-documenting. This makes sense to me. I started to do this at some point when I was writing RoleSpec; eventually got annoyed about fixing the fallout (I was working to get somebody else's patch committed) and went back to how it is. Glad to see it done. The only functional issue might be the removal of the IsA() checks. If we don't cast any Node before passing it to any of those functions, there should be no problem because any misfeasance will be reported as a compile-time warning. Perhaps it's worth adding IsA() checks in loops such as the one in roleSpecsToIds(). -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
