Distinguish SystemAccessControl methods not exposing query id

[findepi]

Description

Before the change, SystemSecurityContext was provided to every method in SystemAccessControl. This was problematic for implementations which need to be query-aware (e.g. for audit or caching purposes), because there was no type system support helping to determine whether ID is present or not. This commit makes SystemSecurityContext.queryId non-optional. For methods where query ID is not passed, simple Identity is now passed, since this is the only other field in the SystemSecurityContext. This is a breaking change because doing it in backwards compatible way would be quite laborious.

[kokosing]

% comment

[kokosing]

I think we should go with deprecation step and avoid backward incompatible change here.

[findepi]

How would you envision getQueryId method in the security context class? would it return optional or not?
i think it would require spreading out changes over multiple versions with plugins updating more than once. I think this is the case where lack of compat is justified.

[kokosing]

Let it be.

[djsstarburst]

I generally like this change. I think this is one of those cases where an argument that can be Optional.empty() buys us nothing, and obscures the intent.

I don't have an opinion on the backwards-compatibliity question.

[wannte]

Hi, @findepi

I tried to use trino version 427, I found that this breaking changes makes unexpected result when using with Apache Ranger.

Apache Ranger override SystemAccessControl method, but now, base methods were changed in Trino 427.

Do you have any idea to handle this issue?

https://github.com/apache/ranger/blob/master/ranger-trino-plugin-shim/src/main/java/org/apache/ranger/authorization/trino/authorizer/RangerSystemAccessControl.java

[2416210017]

Hi, @findepi

I tried to use trino version 427, I found that this breaking changes makes unexpected result when using with Apache Ranger.

Apache Ranger override SystemAccessControl method, but now, base methods were changed in Trino 427.

Do you have any idea to handle this issue?

https://github.com/apache/ranger/blob/master/ranger-trino-plugin-shim/src/main/java/org/apache/ranger/authorization/trino/authorizer/RangerSystemAccessControl.java

Hello,@wannte , I used<trino. version>427</trino. version>to compile ranger, but still reported the same error. May I ask if there is anything wrong?
Error log:
[ERROR] /app/ranger/ranger-release-ranger-2.3.0/plugin-trino/src/main/java/org/apache/ranger/authorization/trino/authorizer/RangerSystemAccessControl.java:[615,71] error: cannot find symbol [ERROR] symbol: class CatalogSchemaTableName [ERROR] location: class RangerSystemAccessControl [ERROR] /app/ranger/ranger-release-ranger-2.3.0/plugin-trino/src/main/java/org/apache/ranger/authorization/trino/authorizer/RangerSystemAccessControl.java:[628,35] error: cannot find symbol [ERROR] symbol: class SystemSecurityContext [ERROR] location: class RangerSystemAccessControl [ERROR] /app/ranger/ranger-release-ranger-2.3.0/plugin-trino/src/main/java/org/apache/ranger/authorization/trino/authorizer/RangerSystemAccessControl.java:[628,66] error: cannot find symbol [ERROR] -> [Help 1] [ERROR] [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch. [ERROR] Re-run Maven using the -X switch to enable full debug logging. [ERROR] [ERROR] For more information about the errors and possible solutions, please read the following articles: [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException [ERROR] [ERROR] After correcting the problems, you can resume the build with the command [ERROR] mvn <args> -rf :ranger-trino-plugin

[findepi]

i wish i could help. i don't know where the ranger integration is maintained and by whom

[wannte]

hi @2416210017 ,

As you check RangerSystemAccessControl, it implements trino's SystemAccessControl. The change of this pull-request (which changed function signature) makes the ranger's implementation fail. You have to customize Ranger's implementation according to Trinos's change.

[2416210017]

hi @2416210017 ,

As you check RangerSystemAccessControl, it implements trino's SystemAccessControl. The change of this pull-request (which changed function signature) makes the ranger's implementation fail. You have to customize Ranger's implementation according to Trinos's change.

Hello,@wannte
Do you have any modified code or installation packages?