Skip to content

Conversation

@mmathieum
Copy link
Member

@mmathieum mmathieum commented Nov 20, 2025

@mmathieum mmathieum self-assigned this Nov 20, 2025
mmathieum added a commit to mtransitapps/parser that referenced this pull request Nov 20, 2025
mmathieum added a commit to mtransitapps/commons-android that referenced this pull request Nov 20, 2025
@mmathieum
Copy link
Member Author

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces functionality to index strings, likely for optimization, by adding a new strings table and related definitions. The changes are a good step towards improving performance.

My review has identified a few areas for improvement:

  • The new strings table's primary key definition can be improved to ensure it's an auto-incrementing primary key, which is crucial for data integrity.
  • The INSERT statement for the new table needs to be adjusted accordingly.
  • A minor copy-paste error was found in a region comment.
  • An unused constant with an unconventional name has been added.

I've provided specific suggestions to address these points.

@mmathieum mmathieum marked this pull request as ready for review November 24, 2025 18:47
@mmathieum mmathieum requested a review from Copilot November 24, 2025 18:55
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces string indexing functionality to optimize database storage by replacing repeated string values with indexed references. The implementation follows the approach described in the linked Transit blog post about reducing data size.

Key changes:

  • Adds utility extension functions for unescaping SQL strings
  • Creates a new strings table schema to store unique strings with auto-generated IDs
  • Defines column index arrays to identify which table columns should use string indexing
  • Adds a feature flag to control the export of indexed strings

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/main/java/org/mtransit/commons/sql/SQLUtils.kt Adds extension functions for unquoting and unescaping SQL strings
src/main/java/org/mtransit/commons/GTFSCommons.kt Defines string indexing schema, column mappings for route/direction/stop tables, and separator constant
src/main/java/org/mtransit/commons/FeatureFlags.kt Adds feature flag to enable/disable string export functionality

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@mmathieum mmathieum merged commit 9ee21b9 into master Nov 24, 2025
4 checks passed
@mmathieum mmathieum deleted the mm/strings branch November 24, 2025 20:16
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