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

#3413 Shapefile format doesn't allow integers longer than 9 digits #3452

Merged
merged 9 commits into from
Jan 22, 2024

Conversation

SanyamSavla
Copy link
Collaborator

@SanyamSavla SanyamSavla commented Jan 2, 2024

Resolves #3413

Shapefile format doesn't allow integers longer than 9 digits

Fixed the bug!
Also changed the data type for "osmWayId" to BIGINT as INT data type does not take more than 9 digits.
Added Evolutions file for Database migration.

Things to check before submitting the PR
  • I've written a descriptive PR title.
  • I've added/updated comments for large or confusing blocks of code.

Copy link
Member

@misaugstad misaugstad left a comment

Choose a reason for hiding this comment

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

I added a couple comments regarding code style/cleanup. But the main issue is that this didn't work for me for numbers that don't fit as an Int.

You've fixed the issue where the numbers fit into a postgres/scala Int but are more than 9 digits. So the number 1234567890 now works when it didn't before.

But if I test the API with the number 12345678901, I get this error:
Execution exception[[PSQLException: Bad value for type int : 12345678901]]

Were you not getting this error when testing the API?

app/controllers/helper/ShapefilesCreatorHelper.java Outdated Show resolved Hide resolved
app/controllers/ProjectSidewalkAPIController.scala Outdated Show resolved Hide resolved
conf/evolutions/default/211.sql Outdated Show resolved Hide resolved
@misaugstad
Copy link
Member

Oh! And you can remove elements of the PR template that don't apply to you to keep it looking clean. I just edited yours as an example.

Copy link
Member

@misaugstad misaugstad left a comment

Choose a reason for hiding this comment

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

Okay there's no way that this code should be working for you unless you're testing on streets/neighborhoods that don't have any labels on them. You haven't updated all the locations where the OSM ID is used. If you do a search through the code for "osm_way_id", you'll see that it's used in GlobalAttributeTable.scala in two different case classes, both of which are still treading the OSM ID as an Int.

Copy link
Member

@misaugstad misaugstad left a comment

Choose a reason for hiding this comment

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

I found some more code that was meant for your other PR but removed it. Tested and it looks good, thank you!

@misaugstad misaugstad merged commit 4346b2f into develop Jan 22, 2024
@misaugstad misaugstad deleted the sanyam_v_savla branch January 22, 2024 21:09
@misaugstad misaugstad mentioned this pull request Jan 31, 2024
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.

Shapefile format doesn't allow integers longer than 9 digits
2 participants