oikewll commented on PR #4471:
URL: https://github.com/apache/spark/pull/4471#issuecomment-5011888571

   ## Review — [SPARK-5686][SQL] Add show current roles command in HiveQl
   
   **Summary:** Adds the `TOK_SHOW_SET_ROLE` token to the `nativeCommands` list 
in `HiveQl.scala`, so that the statement is delegated to Hive as a native 
command instead of failing during Spark SQL parsing. Minimal, low-risk one-line 
change.
   
   ### Analysis
   - **Correctness:** The placement is right — grouped alongside the other 
authorization tokens (`TOK_GRANT`, `TOK_REVOKE`, `TOK_SHOW_GRANT`, 
`TOK_SHOW_ROLE_GRANT`). Adding it to `nativeCommands` is the correct mechanism, 
since these role/grant statements are handled by Hive rather than Spark's own 
execution path.
   - **Scope note:** In Hive's grammar, `TOK_SHOW_SET_ROLE` is emitted by 
**both** `SHOW CURRENT ROLES` **and** `SET ROLE <role>`. So this change 
actually enables passthrough for `SET ROLE` as well, not just the "show current 
roles" described in the title/description. That's arguably a nice bonus, but 
worth calling out explicitly so the broadened surface is intentional and 
documented.
   - **Tests:** No test is included. This is consistent with how sibling tokens 
in this list were added (pure passthrough entries), and CI reported "passes all 
tests / adds no public classes," so it's acceptable here — though a small 
negative parse test (statement no longer throws) would have documented the 
intent.
   - **Risk:** Very low. Merges cleanly, no public API change, no behavioral 
change to existing statements.
   
   ### Verdict
   Correct and appropriately minimal. The only substantive follow-up would be 
documenting that the same token also covers `SET ROLE`. Consistent with the 
surrounding code and the merge outcome.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to