-
Notifications
You must be signed in to change notification settings - Fork 19
Add JGD2011 vertical compound CRS to srs.sql #754
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for the JGD2011 + JGD2011 (vertical) height compound coordinate reference system (EPSG:6697) and performs minor code formatting cleanup.
- Added JGD2011 + JGD2011 (vertical) height (EPSG:6697) spatial reference system to the GeoPackage initialization
- Reformatted a query string from multi-line to single-line format for consistency
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| nusamai-gpkg/src/sql/srs.sql | Added INSERT statement for JGD2011 compound CRS (EPSG:6697) with proper WKT definition |
| nusamai-gpkg/src/handler.rs | Reformatted query string to single line for consistency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| organization_coordsys_id, | ||
| definition | ||
| ) | ||
| VALUES | ||
| ( | ||
| 'JGD2011 + JGD2011 (vertical) height', | ||
| 6697, | ||
| 'EPSG', | ||
| 6697, | ||
| 'COMPOUNDCRS["JGD2011 + JGD2011 (vertical) height",GEOGCRS["JGD2011",DATUM["Japanese Geodetic Datum 2011",ELLIPSOID["GRS 1980",6378137,298.257222101,LENGTHUNIT["metre",1,ID["EPSG",9001]],ID["EPSG",7019]],ID["EPSG",1128]],CS[ellipsoidal,2,ID["EPSG",6422]],AXIS["Geodetic latitude (Lat)",north],AXIS["Geodetic longitude (Lon)",east],ANGLEUNIT["degree",0.0174532925199433,ID["EPSG",9102]],ID["EPSG",6668]],VERTCRS["JGD2011 (vertical) height",VDATUM["Japanese Geodetic Datum 2011 (vertical)",ID["EPSG",1131]],CS[vertical,1,ID["EPSG",6499]],AXIS["Gravity-related height (H)",up],LENGTHUNIT["metre",1,ID["EPSG",9001]],ID["EPSG",6695]],ID["EPSG",6697]]' | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: sqlx::query() executes only the first statement from srs.sql, preventing subsequent INSERTs, including new CRS entries, from being applied.
Severity: CRITICAL | Confidence: 1.00
🔍 Detailed Analysis
The srs.sql file contains multiple INSERT statements, but sqlx::query() is used to execute it. sqlx::query() is designed to execute only a single SQL statement. As a result, only the first INSERT statement will be executed, and subsequent statements, including the new JGD2011 entry (SRID 6697) and other existing CRS definitions, will not be inserted into the gpkg_spatial_ref_sys table. The database initialization will appear to succeed without error, but the new CRS will be unavailable for use.
💡 Suggested Fix
Use sqlx::raw_sql() for the multi-statement script, or split srs.sql into individual statements and execute them within a transaction, or use separate execute() calls for each statement.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: nusamai-gpkg/src/sql/srs.sql#L57-L74
Potential issue: The `srs.sql` file contains multiple `INSERT` statements, but
`sqlx::query()` is used to execute it. `sqlx::query()` is designed to execute only a
single SQL statement. As a result, only the first `INSERT` statement will be executed,
and subsequent statements, including the new JGD2011 entry (SRID 6697) and other
existing CRS definitions, will not be inserted into the `gpkg_spatial_ref_sys` table.
The database initialization will appear to succeed without error, but the new CRS will
be unavailable for use.
Did we get this right? 👍 / 👎 to inform future reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
📢 Thoughts on this report? Let us know! 🚀 New features to boost your workflow:
|
Close #753
What I did
Notes