Export schemas #4260

Anish9901 · 2025-02-13T14:15:02Z

Lets you export your schemas along with its table contents.

Checklist

My pull request has a descriptive title (not a vague title like Update index.md).
My pull request targets the develop branch of the repository
My commit messages follow best practices.
My code follows the established code style of the repository.
I added tests for the changes I made (if applicable).
I added or updated documentation (if applicable).
I tried running the project locally and verified that there are no
visible errors.

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

mathemancer

Overall, pretty good. I have some changes to request for the actual pg_dump command.

mathemancer · 2025-02-17T15:35:03Z

mathesar/views/export.py

+            '-U', conn.info.user,
+            '-d', conn.info.dbname,
+            '-n', sql.Identifier(schema_name).as_string(),
+            '-O'  # Don't include owner info in the dump


I think you should add the --inserts flag here. We should prioritize compatibility over performance, unless the performance really becomes a problem for someone.

I'd also like you to look into whether there's a flag to add that will get rid of the SET commands at the start of the file. Those can also cause problems when loading, and aren't usually needed.

I'd also like you to look into whether there's a flag to add that will get rid of the SET commands at the start of the file.

There is no such flag, unfortunately, users would have to take those statements out manually. Apparently, postgres includes those statements to make sure that upon restore, the server behaves exactly the same as when the dump was created.

Given this, and the fact that we use GENERATED BY DEFAULT AS IDENTITY for id columns, would it make sense for us to prioritize compatibility over performance? The performance for someone dumping their local database and restoring it on a remote server would be terrible if there's a lot of data. Assuming this being a fairly common use case for someone using our product.

Anish9901 · 2025-02-19T13:44:37Z

@mathemancer As discussed in our 1:1, the dumps will now include the mathesar_types schema if any columns in the schema depends on our custom types.

mathemancer

LGTM.

For context, for anyone watching, we're going to assume that we'll have postgres-client installed on the service layer for the foreseeable future.

mathemancer · 2025-02-20T09:26:04Z

@pavish we'll wait to merge this till you review the front end changes.

pavish

@Anish9901

The export fails whenever the schema has at least one table that the user does not have select privileges on. For this scenario, I'd either expect the "Export" button to not be enabled, show a valid error, or export only the tables that the user has access to.
- I do think there might be additional cases where export would fail due to permission related issues and we have to test it all.
I'd like the Export button to be placed within a dropdown menu. For eg., Similar to the dropdown in the Database Page which contains the Disconnect Database option.
- The chances that the user would export the entire schema as a SQL file is low that it does not warrant us placing the Export button in such a prominent location.

Anish9901 · 2025-02-20T17:01:10Z

@pavish I agree with your critique,

"Export" button to not be enabled, show a valid error, or export only the tables that the user has access to.

As far as export behavior is concerned, out of these three, I'd prefer the first option. I don't think this feature would be of much use to the user if we only dump a subset of tables from their schema. They could do that anyway with per table CSV export.

I'd like the Export button to be placed within a dropdown menu. For eg., Similar to the dropdown in the Database Page which contains the Disconnect Database option.

I agree.

That said, it would take me a while to figure out these changes on the frontend, and given that we want to cut our release tomorrow, would you be able to take over the frontend changes required? So that we can get this in before the feature freeze?

zackkrida · 2025-02-20T21:41:20Z

@Anish9901 I do think @pavish's feedback should block this PR from inclusion in the current release if it can't be resolved by end of tomorrow. If it can, great!

I also think the triple-dot menu that triggers a dropdown, like that on the DB page, is also a good choice for the design.

Lastly, when I think about use cases for functionality like this, I can imagine there are users who would only want to export the structure of a schema and not just the data, depending on the scenario. It would be nice to offer both options in the future. There's an incongruity in the current approach where we offer csv data export at the table-level but sql data export at the schema level and both of these are labeled with the same "Export" button. I could see that being confusing. But I think it can be optimized in the future.

pavish · 2025-02-21T07:44:57Z

@Anish9901 The UI changes are not my primary concern. I'm not entirely sure about our product strategy with exporting schemas. We've jumped right into implementation on this rather than having a product level discussion on the requirements.

The fact that export would fail even if one table is owned by another user is not a good idea. I'd rather have the functionality working for all users, and export only the data that the user has access to. Until we have that figured out, the other reasonable option is to limit this functionality to database superusers only.

I'd rather wait a couple days to resolve this (and get it into the next release) rather than rushing the PR just to get it into this release.

cc: @zackkrida

pavish · 2025-02-21T11:47:19Z

@Anish9901 @zackkrida @mathemancer

I've made the frontend changes in this PR to move the export button into a dropdown: efbae44.
- Also the option is called 'Export SQL' instead of 'Export' to better differentiate it from the Export csv functionality we have for tables.
I've made changes to display this dropdown and the export option only for database superusers in 1d01f70.
- This ensures that the export would not end up in a failure.
- This should suffice for users only working with a single role (including anyone creating databases from within Mathesar), and for other users working with a database superuser role.
- Since the export is a SQL file, I think this is a reasonable restriction to have.
- Note: This check is only done on the frontend. I think the backend is fine as-is since the request will fail if the user does not have access to a table in the schema.

With these changes, I think this PR could go into 0.2.1. I'll leave it to @zackkrida and @Anish9901 to make the call.

However, I also think we should discuss this feature holistically and it's actual usecase/user requirements soon after, I'm not entirely convinced about it's usefulness with the current implementation.

zackkrida · 2025-02-21T13:03:03Z

@Anish9901 @pavish @mathemancer:

I also think the PR could be merged in it's current state. I really appreciate the improvements you've made, @pavish. However, I think it is best to draft this PR and save it for the next release for the following reasons:

It isn't clear to me yet who this functionality is for and what goal its intended to serve. When we do include it in a release I'd like to be able to give a clear impression of the value it offers to our users. Furthermore, if some discussion from the team and a refinement of this PR could make it significantly more useful to users, I'd like to take the extra time to do so before we include it in a release.

I'll draft this now just so it's clear that it shouldn't be merged in the short term.

Anish9901 · 2025-02-21T20:40:08Z

@pavish @zackkrida Here are some of my thoughts:

The problem with exporting a subset of tables as SQL:

When dumping a subset of tables, using -t flag, the information about the schema structure gets lost.
- This means not only the schema structure definition is lost, but any objects other than the table, like functions, custom types, etc which are stored within the schema, that the table might depend upon would be lost. Without these objects, the restore might fail, and the dump will be useless.
If someone only has SELECT only on a referer table, but not on the referent(s), the information about the constraints is still included in the dump. And the restore would eventually fail if the referent table(s) are not present on target db.
For dumping a table successfully, you not only need SELECT on that table you also need, but you also need SELECT on the sequences used by that table.
Given that a lot of our users want READ ONLY setups, and the fact that we don't even support granting permissions on the sequences through mathesar, this feature would be useless for someone with only SELECT on a table.
Exporting CSV on the other hand requires only SELECT privilege on the table and is more portable, and accessible for someone wanting to use it in other applications(e.g. Excel, Airtable).
All this to say, that if anyone with limited privileges on the db wants their data out of postgres, SQL is probably not a format that would be suitable for them. Also, having "schema exports" without schema structure definition makes no sense to me.

Who is this feature useful for?

Anyone trying out mathesar for the first time or a long time user who has their data in one of our internal dockerized db wanting to move their data to a more robust postgres platform would use this feature.
Admins wanting a regular backup of their data, that can be easily restored. Given that we are still in beta, and are prone to make mistakes, it would induce confidence in users if backups can be taken easily.

How can we extend this in the future?

Allowing to restore these dumps easily through mathesar (would require design and probably would take more than a couple release cycles).
Allowing to export all the accessible tables as CSVs within a zip/tar file in one click from the schema page.
[Brent's idea] Have some metadata that would allow us to restore mathesar specific metadata along with the dump, e.g. Explorations.

get prototype working!

ec8782f

seancolsen added this to the v0.2.1 milestone Feb 13, 2025

Anish9901 added 8 commits February 17, 2025 15:50

Merge branch 'develop' into schema_exports

527795d

add schema export button

700f011

fix lint errors for SchemaPage.svelte

1c45391

add url for schema exports

1f6568e

add functions for dumping schema to a .sql file

719b6da

remove prototype files

2935104

revert export.py

e77631e

some readability improvements

abfaf0a

Anish9901 marked this pull request as ready for review February 17, 2025 13:42

Anish9901 changed the title ~~Export schemas[WIP]~~ Export schemas Feb 17, 2025

Anish9901 requested review from mathemancer and pavish February 17, 2025 13:53

Anish9901 assigned mathemancer and pavish Feb 17, 2025

Anish9901 added the pr-status: review A PR awaiting review label Feb 17, 2025

mathemancer requested changes Feb 17, 2025

View reviewed changes

Anish9901 added 5 commits February 19, 2025 17:27

Merge branch 'develop' into schema_exports

8ae2b3c

add sql function to determine custom type dependency

0ce48c3

wire dependency func from python to sql

fb5b794

use the right command for pg_dump based on custom type dependency

6232e47

update all_mathesar_objects table

8fe9548

Anish9901 requested a review from mathemancer February 19, 2025 13:20

add tests for schema_has_custom_type_dependency

5ec5699

mathemancer approved these changes Feb 20, 2025

View reviewed changes

Merge branch 'develop' into schema_exports

86c728e

pavish requested changes Feb 20, 2025

View reviewed changes

pavish assigned Anish9901 and unassigned mathemancer and pavish Feb 20, 2025

pavish added pr-status: revision A PR awaiting follow-up work from its author after review and removed pr-status: review A PR awaiting review labels Feb 20, 2025

pavish added 2 commits February 21, 2025 16:19

Merge branch 'develop' into schema_exports

3110c2f

Move export schema button to dropdown

efbae44

pavish force-pushed the schema_exports branch from 35b4812 to efbae44 Compare February 21, 2025 11:11

Show export schema sql option only when current role is superuser

1d01f70

pavish approved these changes Feb 21, 2025

View reviewed changes

zackkrida marked this pull request as draft February 21, 2025 13:03

zackkrida modified the milestones: v0.2.1, v0.2.2 Feb 21, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Export schemas #4260

Export schemas #4260

Anish9901 commented Feb 13, 2025 •

edited

Loading

mathemancer left a comment

mathemancer Feb 17, 2025

Anish9901 Feb 17, 2025 •

edited

Loading

Anish9901 commented Feb 19, 2025

mathemancer left a comment

mathemancer commented Feb 20, 2025 •

edited

Loading

pavish left a comment

Anish9901 commented Feb 20, 2025

zackkrida commented Feb 20, 2025

pavish commented Feb 21, 2025 •

edited

Loading

pavish commented Feb 21, 2025

zackkrida commented Feb 21, 2025

Anish9901 commented Feb 21, 2025

Export schemas #4260

Are you sure you want to change the base?

Export schemas #4260

Conversation

Anish9901 commented Feb 13, 2025 • edited Loading

Checklist

Developer Certificate of Origin

mathemancer left a comment

Choose a reason for hiding this comment

mathemancer Feb 17, 2025

Choose a reason for hiding this comment

Anish9901 Feb 17, 2025 • edited Loading

Choose a reason for hiding this comment

Anish9901 commented Feb 19, 2025

mathemancer left a comment

Choose a reason for hiding this comment

mathemancer commented Feb 20, 2025 • edited Loading

pavish left a comment

Choose a reason for hiding this comment

Anish9901 commented Feb 20, 2025

zackkrida commented Feb 20, 2025

pavish commented Feb 21, 2025 • edited Loading

pavish commented Feb 21, 2025

zackkrida commented Feb 21, 2025

Anish9901 commented Feb 21, 2025

The problem with exporting a subset of tables as SQL:

Who is this feature useful for?

How can we extend this in the future?

Anish9901 commented Feb 13, 2025 •

edited

Loading

Anish9901 Feb 17, 2025 •

edited

Loading

mathemancer commented Feb 20, 2025 •

edited

Loading

pavish commented Feb 21, 2025 •

edited

Loading