Skip to content

SAK-51950 Calendar slow query#277

Open
ottenhoff wants to merge 1 commit intosakaiproject:masterfrom
ottenhoff:SAK-51950
Open

SAK-51950 Calendar slow query#277
ottenhoff wants to merge 1 commit intosakaiproject:masterfrom
ottenhoff:SAK-51950

Conversation

@ottenhoff
Copy link
Contributor

@ottenhoff ottenhoff commented Sep 19, 2025

Summary by CodeRabbit

  • Documentation
    • Updated database conversion scripts for upgrades to add a new index on calendar events for both MySQL and Oracle.
    • The upgrade path now includes steps to create this index, improving performance for calendar loading and range-based event queries.
    • After applying the upgrade, users should notice faster retrieval of calendar events, especially when filtering by date ranges or viewing larger calendars.

@coderabbitai
Copy link

coderabbitai bot commented Sep 19, 2025

Walkthrough

Adds SAK-51950 index-creation snippets to MySQL and Oracle 23.4→23.5 conversion SQL scripts, creating an index on CALENDAR_EVENT over CALENDAR_ID, RANGE_START, RANGE_END, and EVENT_START, with start/end markers.

Changes

Cohort / File(s) Summary of Changes
Conversion SQL scripts
docs/conversion/sakai_23_4-23_5_mysql_conversion.sql, docs/conversion/sakai_23_4-23_5_oracle_conversion.sql
Added SAK-51950 blocks to create an index on CALENDAR_EVENT columns (CALENDAR_ID, RANGE_START, RANGE_END, EVENT_START). MySQL index named CALENDAR_EVENT_CID_RSTART_REND_ESTART; Oracle index named CAL_EVENT_CID_RST_REND_EST. Marked with -- START/END SAK-51950.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "SAK-51950 Calendar slow query" concisely includes the ticket ID and summarizes the primary intent—addressing a slow calendar query. The changeset (adding CALENDAR_EVENT indexes for MySQL and Oracle) matches that intent, so the title is fully related and informative for a quick scan of PR history. It is short, clear, and free of noise.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
docs/conversion/sakai_23_4-23_5_mysql_conversion.sql (1)

2-5: Make DDL rerunnable and reduce lock time

Consider ALTER TABLE with online options and guard for existing index to avoid failures during repeated conversions and to minimize blocking.

Apply if compatible with your supported MySQL version:

-CREATE INDEX CALENDAR_EVENT_CID_RSTART_REND_ESTART
-  ON CALENDAR_EVENT (CALENDAR_ID, RANGE_START, RANGE_END, EVENT_START);
+-- Create online if possible; no-op if already present
+-- (verify support for ALGORITHM/LOCK in your MySQL version)
+ALTER TABLE CALENDAR_EVENT
+  ADD INDEX CALENDAR_EVENT_CID_RSTART_REND_ESTART (CALENDAR_ID, RANGE_START, RANGE_END, EVENT_START)
+  ALGORITHM=INPLACE, LOCK=NONE;

Optionally precede with a check via INFORMATION_SCHEMA.STATISTICS in operational runbooks if you prefer to keep the SQL plain.

docs/conversion/sakai_23_4-23_5_oracle_conversion.sql (2)

2-5: Add ONLINE build and gather stats

To cut disruption during upgrades and ensure good plans, build the index ONLINE and collect stats post‑create.

Proposed adjustment:

-CREATE INDEX CAL_EVENT_CID_RST_REND_EST
-  ON CALENDAR_EVENT (CALENDAR_ID, RANGE_START, RANGE_END, EVENT_START);
+CREATE INDEX CAL_EVENT_CID_RST_REND_EST
+  ON CALENDAR_EVENT (CALENDAR_ID, RANGE_START, RANGE_END, EVENT_START)
+  ONLINE;
+BEGIN
+  DBMS_STATS.GATHER_INDEX_STATS(ownname => USER, indname => 'CAL_EVENT_CID_RST_REND_EST');
+END;
+/

2-5: Minor: unify index naming across dialects

Naming differs (CALENDAR_EVENT_CID_RSTART_REND_ESTART vs CAL_EVENT_CID_RST_REND_EST). Aligning helps ops/searchability in runbooks. Not blocking.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 85cd0cb and cccc831.

📒 Files selected for processing (2)
  • docs/conversion/sakai_23_4-23_5_mysql_conversion.sql (1 hunks)
  • docs/conversion/sakai_23_4-23_5_oracle_conversion.sql (1 hunks)
🔇 Additional comments (1)
docs/conversion/sakai_23_4-23_5_mysql_conversion.sql (1)

2-5: Confirm index column order vs slow query plan

If the slow path filters by CALENDAR_ID and ranges on RANGE_START/RANGE_END but orders by EVENT_START, consider leading EVENT_START after CALENDAR_ID to enable range scan + order without filesort. Please post the representative SQL and EXPLAIN to verify whether (CALENDAR_ID, RANGE_START, RANGE_END, EVENT_START) is optimal vs (CALENDAR_ID, EVENT_START, RANGE_START, RANGE_END) or a secondary index (CALENDAR_ID, EVENT_START).

@ern
Copy link
Contributor

ern commented Sep 30, 2025

when this is merged to branches will update this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants