I took a very quick look. This looks very well structured (with nice abstractions Principal, Grant and Revoke commands, and I like how you have made the parser extensible). This is definitely worth reviewing and getting to complation.
* Terminology. You have made Grant and Revoke sub-classes of Privilege. I would rename Privilege to AuthCommand (or something), because a privilege is a ’thing you can do’ and grant and revoke are ‘requests to change the things you can do’. * CalcitePrincipal is used in too many places. We should just use Principal. E.g. CalciteSchema.getAccessType(CalcitePrincipal) should be getAccessType(Principal). We want to make it easy for people to plug in their own access scheme. CalcitePrincipal can be used for tests and simple demos. Should CatalogReader.getAllowedAccess() be changed to CatalogReader.getAllowedAccess(Principal)? I can see arguments both ways. One would require CatalogReader to be a filtered view for the current statement’s principal(s). I’ll add these comments to the Jira case. I encourage others to have a similar ‘quick look’ so that we can uncover the major issues quickly, before we move to more detailed review. Julian > On Aug 1, 2023, at 5:00 AM, Hongyu Guo <guo_hon...@pku.edu.cn> wrote: > > Hi, community! > > I’ve submitted a PR for "supporting authorization via GRANT and REVOKE DDL > commands.” > > In this PR, I implemented basic access control for calcite by adding GRANT > and REVOKE syntax. > > JIRA case link: https://issues.apache.org/jira/browse/CALCITE-5816 > > Github PR link: https://github.com/apache/calcite/pull/3284 > > > Can someone make a review? > > This is the first new feature I submitted in the community. > > I need some feedback and suggestions to improve it. Thank you! > > Best > Hongyu Guo
