[Issue 69] Nullable Foreign Keys

[Markus Kahl]

I've created the following test:

https://github.com/machisuji/Squeryl/blob/issue69/src/test/scala/org/squeryl/tests/issues/Issue69.scala

It throws an Exception in line 31:

The object is already closed [90007-127]
errorCode: 90007, sqlState: 90007

It seems to be closed after line 29.
Apprently I'm doing something wrong here. In an older (though more
complex test) for issue 80
I got a "better" Exception at the line where the insert was done.
It said that the foreign key violated a constraint (since it was not
point to any existing primary key).

Any idea what's wrong?

[Markus Kahl]

Or maybe there's nothing wrong with it and the Exception
already is a symptom of the lacking support for optional foreign
keys?

On 10 Jan., 22:23, Markus Kahl <machis...@googlemail.com> wrote:
> I've created the following test:
>

> https://github.com/machisuji/Squeryl/blob/issue69/src/test/scala/org/...

[Maxime Lévesque]

This is the message you get when a connection is used after being close,
I suspect that nesting a transaction within a 'using' could be the cause,
but it is most certainly the interaction of specs with the transaction and session function.

As you can see the use of the specs framwework is very minimal,
and I don't quite have a pattern for it, but I think it might be better
to only use 'transaction' (I suspect that the 'session' I see in the scope
is a method defined in TestConnection...)

[Markus Kahl]

TestConnection looks like this:

https://github.com/machisuji/Squeryl/blob/issue69/src/test/scala/org/squeryl/tests/issues/TestConnection.scala

I get the same error (object already closed) if I don't nest the
transaction within
the using block.

It also does not seem be related to specs.
I've written the same without specs and it's the same result.

As it seems only the TestConnection was the problem.
I changed it now only a little and now the problem with
the closed connection is gone.

Now the "test" passes, meaning I can now start writing the actual
test.

On 11 Jan., 00:08, Maxime Lévesque <maxime.leves...@gmail.com> wrote:
> This is the message you get when a connection is used after being close,
> I suspect that nesting a transaction within a 'using' could be the cause,
> but it is most certainly the interaction of specs with the transaction and
> session function.
>
> As you can see the use of the specs framwework is very minimal,
> and I don't quite have a pattern for it, but I think it might be better
> to only use 'transaction' (I suspect that the 'session' I see in the scope
> is a method defined in TestConnection...)
>

[Markus Kahl]

See the latest commit for the changes made to TestConnection to make
it work:

https://github.com/machisuji/Squeryl/commit/3498caa5f86a12a850047cab35971c7cbc795f11

Also the tests. I don't know whether I've missed something, but the
first test
suggests that foreign keys in deed are optional (all tests pass).
The third "test" just prints the table to get an overview.

I think back then when I first tried it and thought it wasn't possible
I just made a mistake
to get that exception with the referential integrity violation
(parentId was no Option I think).

Well as it is now it's fine I'd say, is it not?

On 12 Jan., 00:18, Markus Kahl <machis...@googlemail.com> wrote:
> TestConnection looks like this:
>

> https://github.com/machisuji/Squeryl/blob/issue69/src/test/scala/org/...

[Maxime Lévesque]

The foriegn key declaration should also be modified (in the DDL), when the one side of the oneToMany is Option[] i.e. in :

ALTER TABLE <table identifier>
   ADD [ CONSTRAINT <constraint identifier> ]
     FOREIGN KEY ( <column expression> {, <column expression>}... )
      REFERENCES <table identifier> [ ( <column expression> {, <column expression>}... ) ]
      [ ON UPDATE <referential action> ]
      [ ON DELETE <referential action> ]

we should have  :
    ON UPDATE SET NULL
    ON DELETE SET NULL
   
  http://www.h2database.com/html/grammar.html#referential_constraint
  http://en.wikipedia.org/wiki/Foreign_key
(The code that drives the DDL generation is in Schema and DatabaseAdapter...)

Because if we allow the other end (the "one" side of the oneToMany) to be None,
it should translate to NULL in the DB.

So these specs should be added :

1)
"A node should, be able to tolerate the deletion of the parent" in {
    .... delete the parent of a node...
}

2)
"A node should, be able to tolerate the setting of it's foreign key to null" in {
    .... set parentId of a node to null...
}

Your test seems to do something similar to (1), but it looks like h2 is tolerating it

BTW, what database do you use besides H2 ?

Whatever DB it is, I'll take care of ensuring that it works in the other DBs.

Cheers

[Markus Kahl]

I've changed the test a little bit and added the specs you suggested,
well at least the second one:

https://github.com/machisuji/Squeryl/blob/issue69/src/test/scala/org/squeryl/tests/issues/Issue69.scala

All specs pass again.
I haven't got any database on my machine since I'm always using
embedded ones for
development. Usually HSQL DB.
Maybe I can try to run the tests with another db too if Squeryl
supports another one
with an embedded mode.

I will have a look at the ddl creation.

But regarding your first suggested spec:

Should it be tolerated? I mean even if it is optional to have a parent
(a foreign key), this does not invalidate referential integrity, does
it?

Regards,

Markus

> On Wed, Jan 12, 2011 at 6:18 AM, Markus Kahl <machis...@googlemail.com>wrote:
>
>
>
> > See the latest commit for the changes made to TestConnection to make
> > it work:
>

> >https://github.com/machisuji/Squeryl/commit/3498caa5f86a12a850047cab3...

[Maxime Lévesque]

The tests pass indeed... I wasn't thinking correctly about the foreign key constraint,
sorry for the confusion, as far as I can tell the issue should be closed, I will
go ahead and close it, if you see further problems with it you can reopen it.

Thanks

[Markus Kahl]

Then again would parent.delete ever work?
I don't know what the original intention of that method was.
Maybe it is supposed to "dereference" (set parentId of the referring
Node to None) before deleting the parent?
Although then ManyToOne#delete should also only work if the Node it is
called on
is the only one that has a reference to the to-be-deleted parent.

I mean the deleteAll for the Many side does make sense, since
integrity is not compromised
by doing that. Only if you delete the "one side" while there are still
rows on the "many side"
"pointing" to the "one side" should it fail.

Maybe I'm a little confused too. ;)

In any case I too think that this issue (optional foreign keys) can be
closed.

On 13 Jan., 05:36, Maxime Lévesque <maxime.leves...@gmail.com> wrote:
> The tests pass indeed... I wasn't thinking correctly about the foreign key
> constraint,
> sorry for the confusion, as far as I can tell the issue should be closed, I
> will
> go ahead and close it, if you see further problems with it you can reopen
> it.
>
> Thanks
>

> On Wed, Jan 12, 2011 at 5:16 PM, Markus Kahl <machis...@googlemail.com>wrote:
>
>
>
> > I've changed the test a little bit and added the specs you suggested,
> > well at least the second one:
>

> >https://github.com/machisuji/Squeryl/blob/issue69/src/test/scala/org/...

[Maxime Lévesque]

Deleting the "one" side makes sense when you have cascading delete,
but it should indeed fail otherwise. The intention behind parent.delete
is just to provide a bit of syntactic sugar ... it it causes more confusion then
I guess it was bad sugar ...  ;-) (causes mental cavities ;-))