Skip to content

OF-2956: Drop database table ofSASLAuthorized#2671

Merged
akrherz merged 2 commits intoigniterealtime:mainfrom
guusdk:OF-2956_Drop-unused-database-table
Feb 6, 2025
Merged

OF-2956: Drop database table ofSASLAuthorized#2671
akrherz merged 2 commits intoigniterealtime:mainfrom
guusdk:OF-2956_Drop-unused-database-table

Conversation

@guusdk
Copy link
Member

@guusdk guusdk commented Feb 6, 2025

The database table ofSASLAuthorized is unused, while its definition causes database installation/upgrade errors for some users.

This commit removes the database table completely, both for fresh installs of Openfire, as for updates.

The database table ofSASLAuthorized is unused, while its definition causes database installation/upgrade errors for some users.

This commit removes the database table completely, both for fresh installs of Openfire, as for updates.
@akrherz
Copy link
Member

akrherz commented Feb 6, 2025

causes database installation/upgrade errors for some users.

Lets then remove the schema upgrades done in script 19 for this table. Otherwise, those users are still stuck.

@guusdk
Copy link
Member Author

guusdk commented Feb 6, 2025

Hmm... won't that get into trouble when they then run script 37 which removes a then non-existing table?

At the same time we can't assume that everyone that will run script 37 hasn't run 19 yet.

Maybe we should fix the database creation in earlier scripts (for the deletion to be successful)?

@akrherz
Copy link
Member

akrherz commented Feb 6, 2025

I am just suggesting removing the schema modifications done in 19, not the table rename. My understanding is that the schema changes are what have been sometimes failing. This PR changes would not fix that.

@guusdk
Copy link
Member Author

guusdk commented Feb 6, 2025

I know of at least one person that had an issue with the table definition (notably, the index that includes the 4000-size text column). https://discourse.igniterealtime.org/t/this-rootcollectionnode-is-null/95133/7?u=guus

@akrherz
Copy link
Member

akrherz commented Feb 6, 2025

OK, so we don't have any examples of upgrade errors? If not, then my suggestion about 19 is moot.

We're removing this database script in upgrade 37, but we do that after reports of the original definition causing problems: on Oracle, under certain configuration, the primary key index on a 4000-wide column won't work.

In the off-chance that people upgrading in the future will still execute (very) old upgrade scripts (10 and 19), those scripts are modified by this commit to no longer introduce the problem. As 37 will delete the unused table anyway, those changes should be inconsequential.
@guusdk
Copy link
Member Author

guusdk commented Feb 6, 2025

I don't remember any (but that proves very, very little). In any case, I've now modified the older upgrade scripts in such a way that if there are still Openfire instances out there that haven't executed them, they run less risk of running into the issue that was reported.

@akrherz akrherz merged commit b99191a into igniterealtime:main Feb 6, 2025
15 checks passed
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