gdal icon indicating copy to clipboard operation
gdal copied to clipboard

[Feature Request] Implement OGR Driver for Dameng Database

Open Ha1fm0oN opened this issue 1 year ago • 8 comments

Feature description

As a spatial data developer working with DM Database. I need an OGR to implement spatial data conversion with other data sources.

Currently, I have successfully implemented the ogr2ogr conversion functionality, but I am facing challenges in implementing other essential parts of the driver.

I have tested the ogr2ogr conversion with a few test cases and it seems to be working fine. However, I need guidance on how to proceed with full driver support, including read/write operations, error handling, and potential optimizations.

Questions:

What are the key steps I should follow to expand my current ogr2ogr implementation into a full driver? Are there any existing drivers that I can use as a reference or starting point? What are the best practices for testing an OGR driver, especially using pytest?

Additional context

My Branch:https://github.com/Ha1fm0oN/DMForOgr

Ha1fm0oN avatar Jul 28 '24 11:07 Ha1fm0oN

A few hints:

  • basic write support involves implementing GDALDataset::ICreateLayer(), OGRLayer::CreateField() and OGRLayer::ICreateFeature()
  • I don't know DM Database to be able to point to the most relevant driver. But if it is a SQL database, you may have a look at the PostGIS (ogr/ogrsf_frmts/pg) or MySQL (ogr/ogrsf_frmts/mysql) drivers. But that's mostly to see what the methods I mentioned above may look, not necessarily as a copy&paste model. Every driver implementation is different
  • Bunch of general advice at https://gdal.org/development/dev_practices.html and https://gdal.org/development/testing.html
  • For testing, using the test_ogrsf utility is a good way to check a minimum level of compliance (you may look for "test_ogrsf" in autotest/ogr/ for how to invoke it), but it is far from being exhaustive.

rouault avatar Jul 28 '24 15:07 rouault

Thank you very much for your hints. I was working on this project in my spare time, so the progress has been very slow. Recently, I have modified some of the ogr_DM driver code and completed the compilation of ogr2ogr. I have tested the related functions of ogr2ogr and have encountered some problems:

I completed the build using cmake on a Windows system. During the build process, I found that if I specified DM_INCLUDE_DIR and DM_LIBRARY_RELEASE, the build could be completed normally. However, if I did not set these two options, the build would encounter an error. These two options seem to have become mandatory, and I think it may have been caused by an error in my writing of FindDM.cmake. However, I am not sure where the error lies. I wrote it based on FindPostgreSQL.cmake and made some modifications.

For testing, I used Visual Studio to debug the test_ogrsf project locally, and encountered issues during debugging. It always had problems with INFO: DM: TestCreate skipped. It seems that there was an issue with GetMetadataItem (GDAL_DCAP_VIRTUALIO) and GetMetadataItem (GDAL_DCAP_CREATE), and I am not sure how to modify it. I added the relevant SetMetadataItem in RegisterOGRDM() and set it to YES, but it seems not work.


I have already resolved the first issue, where I mistakenly used ogr_optinald_river in the \ogr\ogrsf_frmts\CMakeLists.txt. The problem was resolved when I switched to ogr_dependent_driver. But I have a question about it. I am using ogr_dependent_driver (dm "DMGEO2" "). Will this have any impact?

Ha1fm0oN avatar Sep 03 '24 06:09 Ha1fm0oN

I still have some questions about using the test_ogrsf utility for testing. When I am testing another database, PostgreSQL, it shows: "INFO: PostgreSQL: TestCreate skip." The INFO is the same as the DM database I am testing. Should I still use test_ogrsf to test my ogr_dm code?

Ha1fm0oN avatar Oct 11 '24 09:10 Ha1fm0oN

"INFO: PostgreSQL: TestCreate skip."

You can't "create" a PostgreSQL datasource with the driver. That would mean creating a new PostgreSQL database. The PG driver can only create layers/table, hence this TestCreate test is skipped

Should I still use test_ogrsf to test my ogr_dm code?

yes, that's strongly recommended. This is one of the easiest way to check "compliance" of your driver code w.r.t expectations of users of the OGR API. This is not sufficient to guarantee correct implementation, but it is a minimum.

rouault avatar Oct 11 '24 12:10 rouault

I don't think there's a point to keep that ticket opened. I'm not sure how strong the appetite of the project is for an upstream driver for a new exotic database. You'll have to make the case for it to the community: cf https://gdal.org/en/stable/development/rfc/rfc85_policy_code_additions.html

rouault avatar Jan 23 '25 18:01 rouault

Hello Rouault,  Thank you for your previous feedback. I have now taken over the maintenance and further development of the driver, and the project has been moved to the following repository: https://github.com/DM-WYL/DMForOgr. 

I don't think there's a point to keep that ticket opened. I'm not sure how strong the appetite of the project is for an upstream driver for a new exotic database. You'll have to make the case for it to the community: cf https://gdal.org/en/stable/development/rfc/rfc85_policy_code_additions.html

Although Dameng database may not be widely known internationally, it is one of the leading database providers in the Chinese market. It is actively used in areas such as smart cities, natural resources, emergency management, and public services — fields where spatial data support is essential.  In the latest update, I have successfully passed the test_ogrsf for the currently implemented features. I am committed to maintaining this driver and will continue to adapt it for future requirements. Given its real-world applications and the availability of a free trial version, I believe that even partial support at this stage is a meaningful step toward fuller integration.  I would be very grateful if you could reconsider including the driver in the main GDAL repository. Thank you for your time and consideration.

DM-WYL avatar Oct 19 '25 13:10 DM-WYL

@DM-WYL On a very very quick skimming through your branch, I don't see any automated tests in autotest/ogr for the driver, or use documentation in https://github.com/DM-WYL/DMForOgr/tree/master/doc/source/drivers/vector I believe it would be better to rename the driver as "dameng" because "dm" is too short. I'd suggest you enable github actions in your fork and fix issues that it wil spot. One blocking point though is that https://github.com/DM-WYL/DMForOgr/blob/master/ogr/ogrsf_frmts/dm/ogrdmtransform.cpp contains code from PostGIS LWGeom library, and doesn't mention its GPL licence nor its copyright holder(s). That's a big NO NO. GDAL source tree cannot contain code under GPL. If your purpose it to parse / generate WKB, you could use GDAL's OGRGeometry classes for that. Once all that is resolved, you can submit a pull request so that it gets a chance to be more thoroughly reviewed.

rouault avatar Oct 19 '25 14:10 rouault

Thank you for the feedback. I will implement the suggested changes and run the tests again promptly.

DM-WYL avatar Oct 19 '25 16:10 DM-WYL