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

documentation: foreign key constraints won't work in SQLite #209

Open
ksshannon opened this issue Feb 14, 2018 · 8 comments
Open

documentation: foreign key constraints won't work in SQLite #209

ksshannon opened this issue Feb 14, 2018 · 8 comments
Labels

Comments

@ksshannon
Copy link

@ksshannon ksshannon commented Feb 14, 2018

Foreign key constraints are disabled in SQLite by default. If a learner attempts to run the CREATE TABLE command in the lesson:

http://swcarpentry.github.io/sql-novice-survey/09-create/

that uses the foreign key constraints, everything will work fine. If the user then attempts to violate the constraint, no error is given. I propose that it is documented that foreign keys have to be enabled in SQLite (PRAGMA foreign_keys=ON), or drop the foreign key section as ambiguous. If foreign keys are tested by someone, the results can be confusing.

Example:

sqlite> CREATE TABLE t1(a INTEGER PRIMARY KEY, b INTEGER);
sqlite> CREATE TABLE t2(c INTEGER PRIMARY KEY, FOREIGN KEY(c) REFERENCES t1(a));
sqlite> INSERT INTO t1 VALUES(1,2);
sqlite> INSERT INTO t1 VALUES(3,4);
sqlite> INSERT INTO t2 VALUES(2); -- succeeds, but shouldn't due to foreign key constraint
sqlite> PRAGMA foreign_keys=ON;
sqlite> INSERT INTO t2 VALUES(4);
Error: FOREIGN KEY constraint failed
@remram44
Copy link
Contributor

@remram44 remram44 commented Feb 15, 2018

No real detail is given about the FOREIGN KEY constraint and there is no exercise that requires them to be enforced, so I don't know if adding details is necessary here. I agree that in a full episode about constraints or relationships, we would need to indicate this caveat.

@remram44 remram44 added the question label Feb 15, 2018
@ksshannon
Copy link
Author

@ksshannon ksshannon commented Feb 15, 2018

Agreed, it may be best to just remove it then?

@remram44
Copy link
Contributor

@remram44 remram44 commented Feb 15, 2018

We could take a different kind of constraint as an example. But then again, foreign keys are pretty common in the wild...

My personal opinion is that introducing constraints is a good idea, so that people at least know what to look for, even though we don't have time to cover them in details in a workshop. And that using foreign key constraints as an example is fine. I agree with you that this is a very high-level introduction that skips over many details.

What do you think?

@ksshannon
Copy link
Author

@ksshannon ksshannon commented Feb 15, 2018

We could take a different kind of constraint as an example. But then again, foreign keys are pretty common in the wild...

I agree that it is a good example.

My personal opinion is that introducing constraints is a good idea, so that people at least know what to look for, even though we don't have time to cover them in details in a workshop. And that using foreign key constraints as an example is fine. I agree with you that this is a very high-level introduction that skips over many details.

Agree with this too.

What do you think?

I had to update the sqlite3 code in the original description, I left out the inserts into t1 (please see above). My only worry is that SQLite doesn't act as expected with regard to foreign keys. This may be confusing, especially if a learner tries it with the tool they used in class. Perhaps the disclaimer:

Once again, exactly what constraints are available and what they’re called depends on which database manager we are using.

Is enough.

@r4space
Copy link
Contributor

@r4space r4space commented Feb 16, 2018

Could also add a some text to
reference.md about them

@ksshannon
Copy link
Author

@ksshannon ksshannon commented Feb 26, 2018

@njamescouk
Copy link
Contributor

@njamescouk njamescouk commented Feb 26, 2018

realised my comment was superfluous and deleted it.

@ksshannon
Copy link
Author

@ksshannon ksshannon commented Feb 26, 2018

rgaiacs added a commit to rgaiacs/swc-sql-novice-survey that referenced this issue Apr 17, 2018
Related with lesson-example/pull/172
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
4 participants