coderabbitai[bot]
commented
6 days ago Walkthrough
The recent changes involve refining the PermissionChecker trait and numerous other components by requiring direct UserInfoRef instead of Option<UserInfoRef>. This update streamlines user handling across various modules. Additionally, the change introduces RwLock for managing concurrent mutable access to session fields like catalog, schema, user_info, and timezone. These alterations aim to improve code clarity, concurrency, and overall performance.
Changes
| Files/Groups | Change Summaries |
|---|---|
src/auth/src/permission.rs |
Modified PermissionChecker trait to require UserInfoRef instead of Option<UserInfoRef>. |
src/auth/tests/mod.rs |
Updated check_permission in DummyPermissionChecker to accept UserInfoRef. |
src/common/datasource/Cargo.toml |
Added async feature to the orc-rust dependency. |
src/common/function/src/.../database.rs |
Changed current_schema to current_schema_raw in QueryContextBuilder instantiation. |
src/operator/src/expr_factory.rs and src/operator/src/... |
Replaced timezone with timezone_raw in QueryContextBuilder instantiation. |
src/pipeline/src/manager/table.rs, src/query/src/plan.rs |
Modified query_ctx and test_extract_full_table_names to use current_schema_raw in QueryContextBuilder. |
src/query/src/optimizer/type_conversion.rs |
Updated parameter in string_to_timestamp_ms function call to reference self.query_ctx.timezone(). |
src/query/src/sql.rs, src/servers/src/http/authorize.rs |
Changed timezone to timezone_raw and updated method calls and user handling logic. |
src/servers/src/export_metrics.rs, src/servers/src/... |
Modified current_schema to current_schema_raw. |
src/session/src/context.rs and src/session/src/lib.rs |
Introduced RwLock to manage session fields like current_schema, current_user, and timezone, replacing ArcSwap usage for better concurrency handling. |
Sequence Diagram(s)
Changes don't include a new feature or modifications to the control flow that necessitate a sequence diagram.
Poem
In the realm of rust and code, UserInfoRef now we hold, With the RwLock dance in place, Schemas and timezones join the race. A smoother flow, concurrency bright, Our sessions now dance with delight! 🐇✨
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
codecov[bot]
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
What's changed and what's your intention?
Follow up for #4218 . Need a rebase after #4218 merged.
Some of the properties, like schema and timezone, are modifiable within a session.
Our original design is to generate a read-only
QueryContext, then pass it with all the execution process path. This works well before we have statements that modify some of the session data, likeSET time_zoneandUSE.After that, we introduce a new method called
update_sessionto give back modified properties to session, after query execution. See https://github.com/GreptimeTeam/greptimedb/blob/b739c9fd10ba486726f6ee30d260792081eafd2f/src/servers/src/mysql/handler.rs#L107 . The implementation ofupdate_sessionhas to check each modifiable field and update the session. This is becauseArcSwapdata structure is not clonable. So we cannot share reference between the parentSessionand the childQueryContext.This main point of this patch is to change those modifiable fields from
ArcSwaptoArc<RwLock>so that we can share it between parent and child. Note that becauseSessionis associated with a signle connection, so actually there is no race condition with reading and updating those modifiable fields.This patch also include a feature flag fix for orc-rust to exclude it datafusion dependency.
Checklist
Summary by CodeRabbit
Refactor
ArcSwapwithArc<RwLock>for fields inSessionstruct, enhancing concurrent access management.Bug Fixes
Chores
asyncfeature fororc-rust, enhancing asynchronous capabilities.