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

Make SQL::Abstract work. #2276

Merged
merged 1 commit into from
Feb 7, 2024
Merged

Conversation

drgrice1
Copy link
Member

@drgrice1 drgrice1 commented Dec 9, 2023

This removes the optional dependence on SQL::Abstract::Classic or a version of SQL::Abstract prior to 2.000000, and only uses version 2.000000 or newer of SQL::Abstract.

For this to work the capability of a field override needs to be removed. There doesn't seem to be a reliable way to do that with the newer versions of SQL::Abstract, although I didn't try very hard. I also don't see the need for having a field override. The only field that has an override is the key_not_a_keyfield or key field. Why was that ever done? Did someone think that MySQL would make a field a database key field if it had the name "key"? Did someone think it would be confusing to have a field named "key" that was not a database key field? It was probably the latter, but in my opinion the field override was even more confusing.

The key field in the key table does not contain any permanent data, so upgrading courses does not require any special handling. The data from the old key_not_a_keyfield field can be safely dropped, and the new key field used in its place. You can leave the key_not_a_keyfield field in the database if you want to switch back and forth between other pull requests that use the key_not_a_keyfield field.

Copy link
Member

@pstaabp pstaabp left a comment

Choose a reason for hiding this comment

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

Just for documentation, see #1236. This appears to fix the script that wasn't working in that issue.

@drgrice1 drgrice1 force-pushed the sql-abstract branch 2 times, most recently from db3871a to 6992f9a Compare January 8, 2024 22:51
@drgrice1 drgrice1 force-pushed the sql-abstract branch 4 times, most recently from f746f89 to e078337 Compare January 21, 2024 12:07
This removes the optional dependence on SQL::Abstract::Classic or a
version of SQL::Abstract prior to 2.000000, and only uses version
2.000000 or newer of SQL::Abstract.

For this to work the capability of a field override needs to be removed.
There doesn't seem to be a reliable way to do that with the newer
versions of SQL::Abstract, although I didn't try very hard.  I also
don't see the need for having a field override.  The only field that has
an override is the `key_not_a_keyfield` or `key` field.  Why was that
ever done?  Did someone think that MySQL would make a field a database
key field if it had the name "key"?  Did someone think it would be
confusing to have a field named "key" that was not a database key field?
It was probably the latter, but in my opinion the field override was
even more confusing.

The `key` field in the `key` table does not contain any permanent data,
so upgrading courses does not require any special handling.  The data
from the old `key_not_a_keyfield` field can be safely dropped, and the
new `key` field used in its place.  You can leave the
`key_not_a_keyfield` field in the database if you want to switch back
and forth between other pull requests that use the `key_not_a_keyfield`
field.
@pstaabp pstaabp merged commit 53d8d5e into openwebwork:develop Feb 7, 2024
1 check passed
@drgrice1 drgrice1 deleted the sql-abstract branch February 7, 2024 23:14
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.

3 participants