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.

Sign up for GitHub

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

Jump to bottom

As a developer I'd like some guidance on writing SQL upgrade scripts #5186

Closed
pdurbin opened this issue Oct 15, 2018 · 14 comments
Closed

As a developer I'd like some guidance on writing SQL upgrade scripts #5186

pdurbin opened this issue Oct 15, 2018 · 14 comments
Assignees
@kcondon
Labels
Component: Code Infrastructure formerly "Feature: Code Infrastructure"
Milestone
4.10 - Additional...

Comments

@pdurbin
Copy link
Member

pdurbin commented Oct 15, 2018

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
@mheppler
Copy link
Contributor

mheppler commented Oct 15, 2018 •
edited

I just ran into this myself locally. Found the necessary script from @qqmyers here 50fea8f.

ALTER TABLE datasetfieldtype ADD COLUMN uri text;
ALTER TABLE metadatablock ADD COLUMN namespaceuri text;

@pdurbin
Copy link
Member Author

pdurbin commented Oct 15, 2018

@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?

@qqmyers
Copy link
Member

qqmyers commented Oct 15, 2018

@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...

@mheppler
Copy link
Contributor

That's a very valid point @qqmyers and something we should consider moving forward, especially as we start ramping up the speed of releases with #4980.

@pdurbin
Copy link
Member Author

pdurbin commented Oct 15, 2018

@qqmyers here's what I do:

  • Look in scripts/database/upgrades to see if there's a script that's already been merged that indicates what the next release will be. If it exists, add my SQL updates statements to it.
  • If there is no SQL script in scripts/database/upgrades that takes us to the current released version to the next planned release, double check with @djbrooke what the next release number will be and then create a script with my updates.
  • Wait for my pull request to get merged and then let other developers know how to keep their dev environments working by sending an email to "dataverse-dev" like the one @sekmiller just sent: https://groups.google.com/d/msg/dataverse-dev/Vzfl3IxpTww/AutZIZRCBwAJ

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!!

@qqmyers qqmyers mentioned this issue Oct 15, 2018
Merged
5 tasks
@pdurbin
Copy link
Member Author

pdurbin commented Oct 15, 2018

@qqmyers pull request #5189 looks good for moving the SQL statements to the right place. Do you want me to add a bit about our process to the dev guide? (Some of the "here's what I do" above.) We could also defer messing with the dev guide.

@scolapasta
Copy link
Contributor

So to weigh in:
For a while I've been advocating we take this approach (naming db scripts based on issue and then compiling them all at the time we create a release). Too many times we've had to rename db scripts because we change our plan of what is next.

we will discuss it here as a team and can provide some guidance going forward.

@pdurbin pdurbin assigned scolapasta and unassigned pdurbin Oct 15, 2018
@pdurbin
Copy link
Member Author

pdurbin commented Oct 15, 2018

@scolapasta makes sense. I'm assigning this to you to sort out. As long as we document the process. Thanks!

@kcondon
Copy link
Contributor

kcondon commented Oct 15, 2018

@pdurbin So does this ticket cover actually renaming/ integrating this script into the update one then or is it more of a process discussion?

@pdurbin
Copy link
Member Author

pdurbin commented Oct 15, 2018

@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.

@scolapasta
Copy link
Contributor

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.

@scolapasta scolapasta removed their assignment Oct 15, 2018
@pdurbin
Copy link
Member Author

pdurbin commented Oct 15, 2018

Ok, I'll document the process we've been using since I got here.

@pdurbin pdurbin self-assigned this Oct 15, 2018
pdurbin added a commit to QualitativeDataRepository/dataverse that referenced this issue Oct 16, 2018
@pdurbin
avoid confusion by removing 3561-update.sql IQSS#5186
7c67734 
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.
@pdurbin
Copy link
Member Author

pdurbin commented Oct 16, 2018

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.

@pdurbin pdurbin changed the title PSQLException: ERROR: column "uri" does not exist on DatasetFieldType As a developer I'd like some guidance on writing SQL upgrade scripts Oct 16, 2018
pdurbin added a commit to QualitativeDataRepository/dataverse that referenced this issue Oct 16, 2018
@pdurbin
add SQL Upgrade Scripts section to Dev Guide IQSS#5186
b802c85
@pdurbin pdurbin removed their assignment Oct 16, 2018
@pdurbin
Copy link
Member Author

pdurbin commented Oct 16, 2018

I just added at "SQL Upgrade Scripts" section to the Dev Guide in b802c85

@qqmyers et al. you are welcome to give feedback!

@pdurbin pdurbin added the Component: Code Infrastructure formerly "Feature: Code Infrastructure" label Oct 17, 2018
@matthew-a-dunlap matthew-a-dunlap removed their assignment Oct 19, 2018
@kcondon kcondon self-assigned this Oct 19, 2018
@kcondon kcondon closed this as completed Oct 19, 2018
@HasanKhatib HasanKhatib mentioned this issue May 18, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kcondon kcondon

Labels
Component: Code Infrastructure formerly "Feature: Code Infrastructure"
Projects
None yet
Milestone
4.10 - Additional Data Transfer Options
Development

No branches or pull requests
7 participants
@pdurbin @mheppler @scolapasta @kcondon @qqmyers @djbrooke @matthew-a-dunlap