Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Client reset imposes stricter character limits on table name #7177

Closed
sync-by-unito bot opened this issue Dec 1, 2023 · 0 comments · Fixed by #7175
Closed

Client reset imposes stricter character limits on table name #7177

sync-by-unito bot opened this issue Dec 1, 2023 · 0 comments · Fixed by #7175
Assignees

Comments

@sync-by-unito
Copy link

sync-by-unito bot commented Dec 1, 2023

Slack Thread captured from #privategroup by [~[email protected]]

  • [~[email protected]]: Looking at a HELP ticket - this app is getting an error about a table name being too long during client reset, but not during normal bootstrap
    {"@t":"2023-11-29T16:27:43.2410074-06:00","@m":"Connection[2]: Session[5]: A fatal error occurred during client reset: 'class name too long (version: 0, last_integrated_remote_version: 0, origin_file_ident: 0, timestamp: 0). Please contact support.'","@l":"Error","SourceContext":"RealmSdk"}
    It appears that <https://github.com/realm/realm-core/blob/bc487096035a25e4d80e9b81e17d912d004047dd/src/realm/sync/instruction_applier.cpp#L88-L94|this method>, which is only used during client reset, enforces a max length of 57 characters (i assume the -6 comes from class_), and the app does indeed have a table with 58 characters. any idea why the normal bootstrap allows this table name?
  • [email protected]: because by the time you're able to bootstrap you've already created all your tables/columns?
  • [email protected]: i don't know why that function is only used in client resets though
  • [~[email protected]]: sorry, by "bootstrap" i meant the non-client reset case.
  • [~[email protected]]: like when they connect and do schema exchange outside of client reset, they don't hit this invariant
  • [email protected]: yeah, i think this may just be a case of the shrugs. going back 3 years that function is still unused
  • [email protected]: i don't know if there was a historical reason for only allowing 57 characters.
  • [~[email protected]]: but wouldn't the validation fail here in the non-client reset case?
    Table* Group::do_add_table(StringData name, Table::Type table_type, bool do_repl)
    or does that name not include the class_ prefix?
  • [~[email protected]]: there's an off-by-one error there
  • [~[email protected]]: should be class_name.size() > Group::max_table_name_length - 6
  • [email protected]: ah, the best kind of error.
  • [~[email protected]]: ooooo wow 🤦 . that explains why a 58-character table name slips through the cracks 🙃
  • [~[email protected]]: wait, that would still fail though?
    class_name.size() = 58
    58 > 63 - 6 and 58 >= 63-6

in the other codepath
name.size() = 64
64 > 63 <-- why is this okay? or does name not include class_ here either?

  • [~[email protected]]: what 64 character long name do they have? the longest table name I can find in the logs attached to that ticket is class_FieldResultDoc_tableResults_tableResults_checkListOptions , which is 63
  • [~[email protected]]: The table name is ECQTSFIBERRESULTDOC_WAVELENGTHRESULTS_MEDIALIST_CUSTOMTAGS
  • [email protected]: did y'all figure this one out?
  • [~[email protected]]: i don't think so. i can file an rcore so it doesn't get lost
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant