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

Modifying Attached Table Twice Returns Error #1133

Closed
chrisjlocke opened this issue Sep 12, 2017 · 12 comments
Closed

Modifying Attached Table Twice Returns Error #1133

chrisjlocke opened this issue Sep 12, 2017 · 12 comments
Labels

Comments

@chrisjlocke
Copy link
Contributor

@chrisjlocke chrisjlocke commented Sep 12, 2017

Details for the issue

OK, another "but this is never going to happen in real life!!" case, but thought I'd mention it....
If you attach a database twice using different names* (who would do that?) and try to amend one of the tables in one of the attached databases, DB4S can't save the changes.

image

I guess you can't prevent it, but didn't know if the error could be 'sugared' somewhat for the user, so they had an inkling that they couldn't do what they're trying to do.

* For the avoidance of doubt, this was my schema.

image

a, b, and c are all the same attached database - just different names. The 'main' database is a different database (so I'm not attaching itself, if that makes sense...)

Useful extra information

I'm opening this issue because:

  • DB4S is crashing
  • DB4S has a bug
  • DB4S needs a feature
  • DB4S has another problem

I'm using DB4S on:

  • Windows: ( version: ___ )
  • Linux: ( distro: ___ )
  • Mac OS: ( version: ___ )
  • Other: ___

I'm using DB4S version:

  • 3.10.99 (nightly from 12 Sep)
  • 3.10.0-beta*
  • 3.9.1
  • Other: ___

I have also:

@justinclift
Copy link
Member

@justinclift justinclift commented Sep 12, 2017

It's probably preventable by keeping track of the path names to attached databases, and having some kind of special handling (generate an error?) when a subsequent attach attempts to use the same one.

@chrisjlocke
Copy link
Contributor Author

@chrisjlocke chrisjlocke commented Sep 12, 2017

I can kind of see where you'd want to attach a database twice (rare occassion) but the chances of you then wanting to amend that database are even rarer! ;)

MKleusberg added a commit that referenced this issue Oct 5, 2017
See issue #1133.
@MKleusberg
Copy link
Member

@MKleusberg MKleusberg commented Oct 5, 2017

Yeah, I can't come up with any reasonable way to make this work either. So I have just added some code now which forbids attaching the same database twice. It's definitely better to just prevent the entire issue this way. Are you ok to double check, @chrisjlocke? Sorry for all the testing I ask for today 😉

@chrisjlocke
Copy link
Contributor Author

@chrisjlocke chrisjlocke commented Oct 6, 2017

Using the latest nightly allowed me to attach the same database twice?
The date of the build was 06-Oct-2017 03:46. Am I too early?

Just to confirm, while they're the same database, I'm giving them different names in the 'Specify a name' dialog.

image

@MKleusberg
Copy link
Member

@MKleusberg MKleusberg commented Oct 6, 2017

Hmm... It should work with that build. I hope it's not a Windows/Linux thing 😦 Can you attach the database twice, then go to the Execute SQL tab and run

PRAGMA database_list;

and copy the results in here?

@chrisjlocke
Copy link
Contributor Author

@chrisjlocke chrisjlocke commented Oct 6, 2017

image

@MKleusberg
Copy link
Member

@MKleusberg MKleusberg commented Oct 6, 2017

Hmm, that doesn't help as much as I would have wanted 😆 I do have an idea though...

@chrisjlocke
Copy link
Contributor Author

@chrisjlocke chrisjlocke commented Oct 6, 2017

image

MKleusberg added a commit that referenced this issue Oct 6, 2017
@MKleusberg
Copy link
Member

@MKleusberg MKleusberg commented Oct 6, 2017

Where did you find this picture of me?!

I've just pushed a commit which (hopefully) makes the path and file name comparison case-independent on Windows. Maybe this already fixes your issue, so it's worth testing it again tomorrow. And if it doesn't fix your issue the commit is still fixing some bug at least 😄

@MKleusberg
Copy link
Member

@MKleusberg MKleusberg commented Oct 29, 2017

Any update here, @chrisjlocke ? 😄

@chrisjlocke
Copy link
Contributor Author

@chrisjlocke chrisjlocke commented Oct 29, 2017

Sorry - missed this one.

Yes, this has now been 'locked down' so the original error can't occur.

image

Thanks!

@MKleusberg
Copy link
Member

@MKleusberg MKleusberg commented Oct 29, 2017

Awesome, thanks for confirming 😄

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

Successfully merging a pull request may close this issue.

None yet
3 participants