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

Allow Insert/Update/Delete for Views on Browse Data tab #141

Closed
asperax opened this issue Oct 31, 2014 · 33 comments
Closed

Allow Insert/Update/Delete for Views on Browse Data tab #141

asperax opened this issue Oct 31, 2014 · 33 comments

Comments

@asperax
Copy link

@asperax asperax commented Oct 31, 2014

This is a feature request to allow Insert/Update/Delete user interface capability for Views on Browse Data tab.

Views are normally read-only in SQLite, however, SQLite supports using triggers on Views, which handle the write operations attempted against the View. When this is the case, the absence of a user interface for editing the view means you have to write SQL code to do so.

To avoid confusion, you might consider interrogating existing triggers for an "...INSTEAD OF [INSERT/UPDATE/DELETE] on ..." trigger to determine what user interface options to expose. But if this is too complex, it might just be better to expose the options and return the result from the database engine, whether that's an error (because the view is read-only and doesn't have a trigger for the action) or otherwise. Of course this may conflict with your easy-to-use design philosophy (some users might dislike having the user interface indicate that edits are possible when they are not), in which case I guess the enhancement should be rejected.

@justinclift
Copy link
Member

@justinclift justinclift commented Oct 31, 2014

Yeah, not sure about this. Martin/Rene?

We might want to reject this for now, but consider it again at some future point?

@nowox
Copy link

@nowox nowox commented Apr 28, 2017

It is been two Years, can we reopen this issue?

Consider this database:

BEGIN TRANSACTION;
CREATE TABLE Foo (a, b, c);
INSERT INTO `Foo` (a,b,c) VALUES (1,2,3);
CREATE TRIGGER update_bar
INSTEAD OF UPDATE OF a ON Bar
BEGIN
  UPDATE Foo SET a = NEW.a
  WHERE b=NEW.b;
END;
CREATE VIEW Bar AS SELECT a, b FROM Foo ORDER BY b;
COMMIT;

With sqlite3 It is legit to write:

UPDATE Bar SET a = 23 WHERE b = 2;

However, it is not possible to do it so on a View on DB Browser for SQLite.

It would be a huge plus for me.

@justinclift
Copy link
Member

@justinclift justinclift commented Apr 28, 2017

It's pretty likely we'll support them at some point. But it's not likely for the upcoming release (3.10.0), and we still a fair amount we need to get done for that. 😕

@nowox
Copy link

@nowox nowox commented Apr 30, 2017

What amount of work this feature would represent? What are the concerned files? If I wanna build DB Browser for SQLite on Windows is there a VS project?

@justinclift
Copy link
Member

@justinclift justinclift commented Apr 30, 2017

This takes you through the complete developer environment setup process for Windows, along with getting DB4S built and running:

Does that help? 😄

@nowox
Copy link

@nowox nowox commented Apr 30, 2017

It's tough, but well documented ^^

@justinclift
Copy link
Member

@justinclift justinclift commented Apr 30, 2017

If you're up for getting it done (takes a few hours, though that's mostly downloading and waiting for installs to finish), be aware that some of the paths in the setup guide no longer match the ones hard coded in the source. They're not far off, and it's easy to fix if you know to watch out for it. 😉

The paths used in the source at the moment are: https://github.com/sqlitebrowser/sqlitebrowser/blob/master/CMakeLists.txt#L18-L30

(that's for compiling on 64 bit windows)

Notice the "5.7" in the Qt path? That'd likely need updating to "5.8". Similar for the OpenSSL and SQLite paths in that highlighted section too, just make sure they match the directory structure you create when you go through the guide. 😄

@nowox
Copy link

@nowox nowox commented Apr 30, 2017

Great!

@MKleusberg
Copy link
Member

@MKleusberg MKleusberg commented May 12, 2017

@nowox Working on this now. Turns out to be trickier than expected but I might be able to push a first version this evening.

MKleusberg added a commit that referenced this issue May 12, 2017
This adds a new context menu option that allows unlocking views for
updating. This requires appropriate triggers to be in place and the user
to type in a column name that can be used as a 'primary key' for the
view. By default views are still locked from editing.

Inserting into and deleting from views isn't supported yet.

See issue #141.
@MKleusberg
Copy link
Member

@MKleusberg MKleusberg commented May 12, 2017

Progress:

  • Update
  • Insert
  • Delete

Delete seems easy enough, insert however more tricky.

By default, views are still locked from editing because most databases don't have triggers in place for editing views. This means you have to enable editing for a view by browsing it, right clicking on the column headers, and clicking "Unlock view editing". You'll then be asked for the name of a unique column in that view. That's because we somehow have to decide how to address rows in the view and there doesn't seem to be any good way to find out a suitable column automatically.

@nowox
Copy link

@nowox nowox commented May 14, 2017

Wooh that's nice. I should give a try

@lpirl lpirl mentioned this issue Jun 15, 2017
4 of 13 tasks complete
MKleusberg added a commit that referenced this issue Jun 7, 2018
If a view has been unlocked for editing by specifiying a pseudo primary
key, with this commit you can now delete records from the view if an
appropriate trigger exists.

See issue #141.
@MKleusberg
Copy link
Member

@MKleusberg MKleusberg commented Jun 7, 2018

It's been a long time but I have just added support for deleting from a view 😄

MKleusberg added a commit that referenced this issue Jun 7, 2018
If a view has been unlocked for editing by specifiying a pseudo primary
key, with this commit you can now delete records from the view if an
appropriate trigger exists.

See issue #141.
@nowox
Copy link

@nowox nowox commented Jun 7, 2018

@justinclift
Copy link
Member

@justinclift justinclift commented Jun 7, 2018

We need a few people to run through this new code to try it out, and report any bugs or other weirdness.

Is that something you'd be up for? If so, this new code will be in our nightly builds from tomorrow onwards. 😄

@MKleusberg
Copy link
Member

@MKleusberg MKleusberg commented Aug 16, 2018

I'll just leave a link to PR #1477 here because I think we can finally tackle inserting records into views when that PR is merged. Haven't thought this through though, so maybe there are still more preparations needed after that 😉

@mgrojo
Copy link
Member

@mgrojo mgrojo commented Oct 12, 2018

There are problems with view editing in the alpha:

  • The pseudo PK seems to not be applied again when switching back to the view (you cannot edit and the Delete Record button is not reenabled) or after refreshing.
  • The 'Unlock view editing' check state persists when switching between views.

At first sight I don't get where is the problem in the code.

@shaun-ba
Copy link

@shaun-ba shaun-ba commented Oct 12, 2018

@justinclift @MKleusberg When i assign a unique column, then try to edit a cell i get the error:

"Error changing data: cannot modify Shields-die_number because it is a view"

My intention is to edit cells within a view, and add entries. This is my intention because I was told a view is the only way to sort the database using multiple columns.

mgrojo added a commit that referenced this issue Oct 12, 2018
This solves these two problems that seem to have being introduced in
d5a0490

- The unlocked-view status was not applied again when switching back to the
view (you cannot edit and the Delete Record button is not reenabled) or
after refreshing.

- The 'Unlock view editing' check state persisted when switching between
views.

See issue #141.
MKleusberg added a commit that referenced this issue Oct 19, 2018
This solves these two problems that seem to have being introduced in
d5a0490

- The unlocked-view status was not applied again when switching back to the
view (you cannot edit and the Delete Record button is not reenabled) or
after refreshing.

- The 'Unlock view editing' check state persisted when switching between
views.

See issue #141.
MKleusberg added a commit that referenced this issue Oct 19, 2018
This solves these two problems that seem to have being introduced in
d5a0490

- The unlocked-view status was not applied again when switching back to the
view (you cannot edit and the Delete Record button is not reenabled) or
after refreshing.

- The 'Unlock view editing' check state persisted when switching between
views.

See issue #141.
@chrisjlocke
Copy link
Contributor

@chrisjlocke chrisjlocke commented Dec 3, 2018

@MKleusberg - changing data in a view still displays an error (this is from the nightly from 3 Dec)

image

Deleting from a view displays the same (albeit differently worded) error

image

Also note the error in the last SQL - mee_id is a primary key (and was selected as such when unlocking the view) but it is not populated in the 'IN' statement.
The third record (mee_id=3) was selected for deletion.

image

Red herring: The currently selected cell is empty. Selecting any cell creates the same error. So don't say, "select another cell and try again." 😉

@mgrojo
Copy link
Member

@mgrojo mgrojo commented Dec 3, 2018

@chrisjlocke How is the view defined? Have you set the "INSTEAD OF" triggers?

@chrisjlocke
Copy link
Contributor

@chrisjlocke chrisjlocke commented Dec 4, 2018

Just a straight view from a select. No triggers defined. Triggers are required if wanting to amend a view outside of Db4S, but not if via DB4S, correct?

@mgrojo
Copy link
Member

@mgrojo mgrojo commented Dec 4, 2018

No, we actually need the triggers. The update is performed by SQLite and it is requested by DB4S in the same way as if it were a table. That's why we need to define a key for the view, in order to do our usual update query:

UPDATE "main"."myView" SET "field1"=? WHERE "key"='value';

And SQLite requires the triggers to make the actual insertion in the appropriate table(s). Maybe it should be stated explicitly (or even checked) in the application. For the moment, I've updated the Wiki page to mention it.

@MKleusberg
Copy link
Member

@MKleusberg MKleusberg commented Dec 5, 2018

Manuel is right about the triggers. You need to define them yourself because we can't keep track of what row comes from where and which cell is connected to which. That's actually difficult enough that I'm not even sure we can even check for the existence of such a trigger in a reliable way 😉 However, the empty IN(...) list you're mentioning is indeed a problem. I'll look into that in a second.

MKleusberg added a commit that referenced this issue Dec 5, 2018
This was probably broken by f1e01dd.

See issue #141.
@MKleusberg
Copy link
Member

@MKleusberg MKleusberg commented Dec 5, 2018

Should be fixed now 😄

@justinclift
Copy link
Member

@justinclift justinclift commented Dec 5, 2018

Just started the Windows nightly builders, so new builds for today should be available in an hour or so. 😄

@cristilupascu
Copy link
Contributor

@cristilupascu cristilupascu commented Feb 22, 2019

Are there any updates for the Insert functionality for views? I would really love this feature and I am thinking of trying to implement it, if no one is working on it at the moment.

@mgrojo
Copy link
Member

@mgrojo mgrojo commented Feb 22, 2019

@cristilupascu Not sure which is the current state (specially after this @MKleusberg's comment) and haven't tested myself. I assume by your comment, that this isn't still working. I'm not working on it. Don't know about @MKleusberg, but I'd say that he isn't either.

@cristilupascu
Copy link
Contributor

@cristilupascu cristilupascu commented Feb 22, 2019

@mgrojo I see. Then I'll start looking through the code and see if I can figure it out. At the moment, the insert button is always disabled for views.

@justinclift
Copy link
Member

@justinclift justinclift commented Feb 23, 2019

Thanks @cristilupascu, that'd be awesome. 😄

cristilupascu added a commit to cristilupascu/sqlitebrowser that referenced this issue Mar 3, 2019
cristilupascu added a commit to cristilupascu/sqlitebrowser that referenced this issue Mar 3, 2019
Add Insert for Views using AddRecordDialog (sqlitebrowser#141)
cristilupascu added a commit to cristilupascu/sqlitebrowser that referenced this issue Mar 3, 2019
cristilupascu added a commit to cristilupascu/sqlitebrowser that referenced this issue Mar 3, 2019
@cristilupascu
Copy link
Contributor

@cristilupascu cristilupascu commented Mar 3, 2019

Uff, sorry for all those references. I am pretty sure I added the Insert functionality without breaking anything for tables. Right now, for views, the AddRecordDialog is always used when inserting. The other method (inserting a row directly into the table) seems too complex to modify, so I won't be working on it.

@justinclift
Copy link
Member

@justinclift justinclift commented Mar 4, 2019

@cristilupascu Sounds like it'll be a good start. 😄

MKleusberg added a commit that referenced this issue Mar 6, 2019
@MKleusberg
Copy link
Member

@MKleusberg MKleusberg commented Mar 6, 2019

I have just merged @cristilupascu's pull request 😄 So starting with tomorrow's nightly build there should be inserting into views too now. Somebody still around to test this? We could finally close this issue then 😉

@cristilupascu Using the AddRecordDialog was also the way which I thought makes most sense. Inserting a row directly into the table probably won't ever work 100% correctly for views. Also I imagine some people might have an ON INSERT but no ON UPDATE trigger and the approach to insert a row directly requires both. So if no problems pop up in the tests, I think we can consider this finished 😄

@MKleusberg
Copy link
Member

@MKleusberg MKleusberg commented Aug 10, 2019

Because I haven't heard of any problems with this, I'll finally close this issue 😄 I'd suggest considering this as done. So if there is any (remaining or new) problem with editing views, let's open a dedicated issue for that problem.

@bam80
Copy link

@bam80 bam80 commented Jan 23, 2020

Please check if it's still an issue here: #1653 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
9 participants