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

Show dialog when importing tables via browser, give users options to control import #60875

Merged
merged 24 commits into from
Mar 10, 2025
Merged

Show dialog when importing tables via browser, give users options to control import #60875

merged 24 commits into from
Mar 10, 2025

Conversation

nyalldawson
Copy link
Collaborator

This PR is a step towards porting the DB Manager "Import vector table" functionality into core/browser. I've taken extreme care here to ensure that everything is kept generic and that the GUI is all dynamically built based on the available capabilities of the destination database provider. This means that we gain native import vector table control for postgres, hana, sql server, gpkg, spatialite + the OGR database formats (eg ESRI File Geodatabase) in one shot, and that any future extensions to this dialog will automatically apply to all relevant destination providers. *

To facilitate this, I've added new API to the database connections classes. I've removed a lot of duplicate copy/paste code from the various QgsDataItemGuiProvider subclasses and moved them to generic methods in QgsDataItemGuiProviderUtils.

Some notes:

  • Part of this work was based on efforts by @JanCaha to port the DB Manager Postgres import functionality to browser
  • The new dialog will show when dragging and dropping a SINGLE VECTOR TABLE only onto a database schema (or connection item, if the connection doesn't support schemas). You won't see it if you drag and drop multiple tables at once -- that still does an immediately import without any customisation or prompts. I've left this as I don't want to risk UI regressions for users who need to import many tables at once, where showing a dialog per table would be annoying. (At some stage we could/should make a similar import dialog designed around multiple-table import to handle this situation)
  • The dialog will show when dragging and dropping a layer from the browser, OR a layer from the current project
  • You can also show the dialog by right-clicking a schema item (or non-schema supporting connection item) and selecting "Import Vector Table...", giving you the choice of importing any of the layers from the current project.
  • Unlike the DB Manager feature, which blocks the application and does the import in the main thread, this uses a background task to do the actual import leaving QGIS nice and responsive.
  • Supported functionality is:
    • Renaming the destination table. The destination table will default to the same name as the import table/layer, but users can change this as desired
    • Replace destination table checkbox (unchecked by default)
    • Set the primary key name for the created table. Defaults to the original layer primary key name, or a default provider-specific name if the original layer does not have a primary key. Only shown if the destination supports primary key naming.
    • Set the geometry column name. Defaults to the original layer geometry column name, or a default provider-specific name if the original layer does not have a named geometry column. Only shown if the destination supports geometry column naming and the source is a spatial layer.
    • Set the destination layer CRS. Only shown if the source layer is a spatial layer. Defaults to the original layer CRS.
    • Set the output table comment. Only shown if the destination provider supports this (which is currently only the postgres provider). Defaults to the input layer metadata description/abstract.
  • Some functionality from the DB Manager Import Vector Table is NOT currently supported. This is:
    • Overriding the source layer CRS. In my view this is confusing, niche functionality which should be handled outside of this dialog, eg by setting the layer's CRS manually.
    • Similarly, the ability to override the source layer's encoding is not supported. Again, this is confusing + niche and should instead just be set for the source map layer directly when required.
    • "Do not promote to multi-part" option. I think this logic needs re-thinking to make it generic and not specific to postgres database import. In db manager it means that eg a multipolygon source layer will be imported as a (not multi)polygon layer on the database, and that the import will error out if ANY geometries are found with more than one part. It's badly named for a start, as it implies that by leaving the box unchecked that a promotion to multipart geometries will ALWAYS occur. At the very least this option should be renamed to "demote to single part" and only enabled when the source layer is a multipart geometry type, but this needs to be discussed and rethought before we do this. (It's out of scope for my current work, I will not be doing this)
    • "Convert field names to lowercase". In a follow up PR I will expose the ability to exclude source layers from the import and rename them -- after this is done we could instead have a button in the dialog which just renames all the layers to lowercase (out of scope for my current work).
    • "Create spatial index". The API for spatial index creation is messy -- we have a method in QgsVectorDataProvider to do this, but only implemented by SQL Server and Oracle providers. We have an alternative API in QgsAbstractDatabaseProviderConnection but only implemented by Oracle, Postgres, GeoPackage and Spatialite. Right now the QgsVectorLayerExporter class used by browser vector imports just ALWAYS uses the vector provider method when exporting tables, so we ALWAYS get spatial indexes for tables imported for SQL Server + Oracle providers. This is just too messy and we need to consolidate on one spatial index creation method implemented by all database providers before exposing this choice in the dialog. (This is out of scope for my current work)
    • "Import only selected features" -- I'll be sending through a follow up PR after this one with extra options for filtering the imported layer, including extent based filter + expression based filter, and I'll implement selection based filtering at this time.
    • There's no way to hit a "..." button and pick a non-project layer to import. This should be done by showing a dialog (or QgsPanelWidget) with a browser tree for picking the source layer. It's out of scope for my current work.

image

Sponsored by City of Canning

  • Note that this functionality does not currently work for oracle databases, simple because the oracle provider is still using deprecated functionality from QgsDataItem and surprisingly does not yet implement a QgsDataItemGuiProvider! 🙀 There's nothing here which prevents Oracle from just working as soon as someone implements that class for oracle data items.

Fixes #24194

nyalldawson added 19 commits March 5, 2025 15:01
@nyalldawson
[mssql] Move browser handle drop logic to GUI provider
f36a6b7
@nyalldawson
[hana] Move browser handle drop logic to GUI provider
261e288
@nyalldawson
[postgres] Move browser handle drop logic to GUI provider
a8b02da
@nyalldawson
Pass GUI context onto handleDrop implementations
884856f
@nyalldawson
Preparatory work for generic import database table dialog
263318c
@nyalldawson
Start on generic import vector layer dialog
33e2d9c
@nyalldawson
Move some logic to dialog, create connection method
b063f90
@nyalldawson
Create QgsAbstractDatabaseProviderConnection::createVectorLayerExport…
dec9c93 
…erDestinationUri

Creates a URI for use with QgsVectorLayerExporter corresponding to given destination
table options for the backend. The URI format and extra options which
need to be passed to QgsVectorLayerExporter differ from provider to
provider, so this new method gives us a consistent, generic method we
can call to safely generate the right URI and options in a
cross-provider way.

Implemented for all database connection providers.
@nyalldawson
Create generic method to handle single vector layer drop for providers
af388f9 
Create QgsDataItemGuiProviderUtils::handleDropUriForConnection, which
handles single vector item drop on a database provider connection
item for ALL database providers in a consistent way.

This first shows QgsDbImportVectorLayerDialog allowing the user
to customise the imported table details, and then runs the
export in a background task.
@nyalldawson
[api] Add connection metadata methods for default geometry column nam…
fae02dc 
…e, import capabilities
@nyalldawson
Add metadata methods for primary key capabilities
5e3d3fd
@nyalldawson
Allow customising primary key and geometry column name
de4b665
@nyalldawson
Allow setting destination layer CRS
f70e86b
@nyalldawson
Add provider connection method to set table comment
3443d64 
Currently supported for Postgres provider only
@nyalldawson
Allow setting table comment for imported tables
2a775ac
@nyalldawson
Memory safety
2c09f9d
@nyalldawson
Add replace destination table if exists option
d4971e2
@nyalldawson
Db import dialog can directly import project layers
af01da2
@nyalldawson
[browser] Add direct "Import Vector Layer" context menu for db items
39b9a38 
To accompany the existing drag-and-drop support for layers, this action
just directly opens the Import Layer dialog and allows users to import
one of the layers from the current project to a destination db/schema
@github-actions github-actions bot added this to the 3.44.0 milestone Mar 6, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 6, 2025 •
edited
Loading

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit f74ec9d)

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit f74ec9d)

@JanCaha
Copy link
Contributor

JanCaha commented Mar 7, 2025

Looks great @nyalldawson . Seems to handle all the cases 👍

@nyalldawson nyalldawson closed this Mar 8, 2025
@nyalldawson nyalldawson reopened this Mar 8, 2025
@nyalldawson nyalldawson reopened this Mar 10, 2025
@nyalldawson
Copy link
Collaborator Author

Unrelated test failure

@nyalldawson nyalldawson merged commit 852bfe7 into qgis:master Mar 10, 2025
31 of 32 checks passed
@nyalldawson nyalldawson deleted the browser_export_options branch March 10, 2025 04:40
@DelazJ DelazJ added the Needs Documentation When merging a labeled PR, an issue will be created in the Doc repo. label Mar 10, 2025
@qgis-bot
Copy link
Collaborator

@nyalldawson
This pull request has been tagged as requiring documentation.

A documentation ticket will be opened at https://github.com/qgis/QGIS-Documentation when this PR is merged.

Please update the description (not the comments) with helpful description and screenshot to help the work from documentors.
Also, any commit having [needs-doc] or [Needs Documentation] in will see its message pushed to the issue, so please be as verbose as you can.

Thank you!

@qgis-bot
Copy link
Collaborator

@nyalldawson
A documentation ticket has been opened at qgis/QGIS-Documentation#9721
It is your responsibility to visit this ticket and add as much detail as possible for the documentation team to correctly document this change.
Thank you!

@qgis-bot qgis-bot mentioned this pull request Mar 10, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexbruy alexbruy alexbruy approved these changes

Assignees
No one assigned
Labels
Feature Needs Documentation When merging a labeled PR, an issue will be created in the Doc repo. QGIS Browser
Projects
None yet
Milestone
3.44.0
Development

Successfully merging this pull request may close these issues.

Add a rename option when copying layers by drag and drop in the browser
5 participants
@nyalldawson @JanCaha @qgis-bot @alexbruy @DelazJ