-
Notifications
You must be signed in to change notification settings - Fork 495
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
As a developer I'd like some guidance on writing SQL upgrade scripts #5186
Comments
@mheppler oh! It's called "scripts/database/5043-update.sql". Thanks! Is this the new way? Shall we document it in the dev guide? And explain the new way in a post to the "dataverse-dev" list? |
@pdurbin I think I just put it in that way since I didn't know what version it would become part of... If I should have done it differently, let me know... |
@qqmyers here's what I do:
Does that make sense? I could make a pull request to document this. Are you interested in making a pull request to remove scripts/database/5043-update.sql and put the statements in scripts/database/upgrades/upgrade_v4.9.4_to_v4.10.sql? I'm happy to do this if you want. Are you interested in emailing dataverse-dev about the update? I can do this too. Finally, since I'm bothering you, can you please take a quick peek at #5185? Thanks!! |
So to weigh in: we will discuss it here as a team and can provide some guidance going forward. |
@scolapasta makes sense. I'm assigning this to you to sort out. As long as we document the process. Thanks! |
@pdurbin So does this ticket cover actually renaming/ integrating this script into the update one then or is it more of a process discussion? |
@kcondon good question. Without a process I'm blocked on making SQL updates. I'm attempting to use the new process in pull request #5195 but as @scolapasta indicated we will be discussing this as a team. |
For this issue, we can use the current process of using the update script, if people prefer, so we can get it done and over. The more general discussion will happen in the next few weeks, I hope. |
Ok, I'll document the process we've been using since I got here. |
This script is referenced from https://github.com/IQSS/dataverse/releases/tag/v4.8 and it's unclear why it was not part of upgrade_v4.7.1_to_v4.8.sql New contributors have found the presence of this script in the "scripts/database" directory confusing and have assumed that the process they should follow is to create a similar script, not knowing about our upgrade_v4.7.1_to_v4.8.sql process.
As discussed at standup this morning I'll be adding some documentation of our existing process for writing SQL upgrade script. I'll tweak the title of this issue to reflect this. At standup we also agreed to remove scripts/database/3561-update.sql which leads contributors astray into thinking that's what we want. I did this in 7c67734 and had the thought that we could move the content into upgrade_v4.7.1_to_v4.8.sql and update the release notes at https://github.com/IQSS/dataverse/releases/tag/v4.8 but didn't want to make this decision unilaterally. |
I just switched to the "develop" branch and could not deploy due to this error:
Caused by: Exception [EclipseLink-4002] (Eclipse Persistence Services - 2.5.2.v20140319-9ad6abd): org.eclipse.persistence.exceptions.DatabaseException
Internal Exception: org.postgresql.util.PSQLException: ERROR: column "uri" does not exist
Position: 184
Error Code: 0
Call: SELECT ID, ADVANCEDSEARCHFIELDTYPE, ALLOWCONTROLLEDVOCABULARY, ALLOWMULTIPLES, description, DISPLAYFORMAT, DISPLAYONCREATE, DISPLAYORDER, FACETABLE, FIELDTYPE, name, REQUIRED, title, uri, VALIDATIONFORMAT, WATERMARK, METADATABLOCK_ID, PARENTDATASETFIELDTYPE_ID FROM DATASETFIELDTYPE ORDER BY ID
Query: ReadAllQuery(referenceClass=DatasetFieldType sql="SELECT ID, ADVANCEDSEARCHFIELDTYPE, ALLOWCONTROLLEDVOCABULARY, ALLOWMULTIPLES, description, DISPLAYFORMAT, DISPLAYONCREATE, DISPLAYORDER, FACETABLE, FIELDTYPE, name, REQUIRED, title, uri, VALIDATIONFORMAT, WATERMARK, METADATABLOCK_ID, PARENTDATASETFIELDTYPE_ID FROM DATASETFIELDTYPE ORDER BY ID")
It looks like the "uri" field was added to the "datasetfieldtype" table in 319c8d9 as part of pull request #5047 which was merged on Friday afternoon.
As of e948af2
scripts/database/upgrades/upgrade_v4.9.4_to_v4.10.sql
on the "develop" branch does not contain any SQL statements to add the new "uri" column so the definition of done for this issue is:We could expand the scope slightly to say that the dev guide should explain the procedure above. Perhaps "database migrations" deserves its own page? We should probably also mention the need to write SQL update scripts and let others know about them in http://guides.dataverse.org/en/4.9.4/developers/version-control.html#how-to-make-a-pull-request
The text was updated successfully, but these errors were encountered: