<div dir="ltr"><div dir="ltr">I have pushed bunch of changes for non null constraint onto a branch [<a href="https://github.com/amitvc/herddb/tree/support-non-null-constraints">https://github.com/amitvc/herddb/tree/support-non-null-constraints</a>]<div>While looking at Table.java line - 443 I see that the primary key type is checked as 4 different ColumnTypes. With me introducing non null types this code should be changed to check against non null. PK should not be nullable type.</div><div>Current code -</div><div><pre style="background-color:rgb(43,43,43);color:rgb(169,183,198);font-family:Menlo;font-size:9pt"><span style="color:rgb(204,120,50)">if </span>(pk.<span style="color:rgb(152,118,170)">type </span>!= ColumnTypes.<span style="color:rgb(152,118,170);font-style:italic">STRING<br></span><span style="color:rgb(152,118,170);font-style:italic"> </span>&& pk.<span style="color:rgb(152,118,170)">type </span>!= ColumnTypes.<span style="color:rgb(152,118,170);font-style:italic">LONG<br></span><span style="color:rgb(152,118,170);font-style:italic"> </span>&& pk.<span style="color:rgb(152,118,170)">type </span>!= ColumnTypes.<span style="color:rgb(152,118,170);font-style:italic">INTEGER<br></span><span style="color:rgb(152,118,170);font-style:italic"> </span>&& pk.<span style="color:rgb(152,118,170)">type </span>!= ColumnTypes.<span style="color:rgb(152,118,170);font-style:italic">TIMESTAMP</span>) {<br> <span style="color:rgb(204,120,50)">throw new </span>IllegalArgumentException(<span style="color:rgb(106,135,89)">"primary key " </span>+ pkColumn + <span style="color:rgb(106,135,89)">" must be a string or long or integer or timestamp"</span>)<span style="color:rgb(204,120,50)">;<br></span>}</pre></div><div>Let me know what you think of the changes so far. I will add more unit tests.</div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Feb 26, 2019 at 7:58 PM Amit Chavan <<a href="mailto:achavan1@gmail.com">achavan1@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Thanks, Enrico for such a quick and insightful response. I will be working on the patch tonight.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Feb 26, 2019 at 3:00 AM <<a href="mailto:herddb-dev-request@lists.herddb.org" target="_blank">herddb-dev-request@lists.herddb.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Send herddb-dev mailing list submissions to<br>
<a href="mailto:herddb-dev@lists.herddb.org" target="_blank">herddb-dev@lists.herddb.org</a><br>
<br>
To subscribe or unsubscribe via the World Wide Web, visit<br>
<a href="http://lists.herddb.org/mailman/listinfo/herddb-dev" rel="noreferrer" target="_blank">http://lists.herddb.org/mailman/listinfo/herddb-dev</a><br>
or, via email, send a message with subject or body 'help' to<br>
<a href="mailto:herddb-dev-request@lists.herddb.org" target="_blank">herddb-dev-request@lists.herddb.org</a><br>
<br>
You can reach the person managing the list at<br>
<a href="mailto:herddb-dev-owner@lists.herddb.org" target="_blank">herddb-dev-owner@lists.herddb.org</a><br>
<br>
When replying, please edit your Subject line so it is more specific<br>
than "Re: Contents of herddb-dev digest..."<br>
<br>
<br>
Today's Topics:<br>
<br>
1. Support not-null constraints (Amit Chavan)<br>
2. Re: Support not-null constraints (Enrico Olivelli)<br>
<br>
<br>
----------------------------------------------------------------------<br>
<br>
Message: 1<br>
Date: Mon, 25 Feb 2019 23:11:25 -0800<br>
From: Amit Chavan <<a href="mailto:achavan1@gmail.com" target="_blank">achavan1@gmail.com</a>><br>
To: <a href="mailto:herddb-dev@lists.herddb.org" target="_blank">herddb-dev@lists.herddb.org</a><br>
Subject: [Herddb-dev] Support not-null constraints<br>
Message-ID:<br>
<CADsOBjMoMj2C-nGmDYVO+9BKGyAjta=9vHLLUFbzYs3-=<a href="mailto:E_ASQ@mail.gmail.com" target="_blank">E_ASQ@mail.gmail.com</a>><br>
Content-Type: text/plain; charset="utf-8"<br>
<br>
I am working on this ticket Issue-125<br>
<<a href="https://github.com/diennea/herddb/issues/125" rel="noreferrer" target="_blank">https://github.com/diennea/herddb/issues/125</a>>. Couple of questions -<br>
1. Currently ColumnTypes in herddb are part of the ColumnTypes class. I am<br>
thinking of using an enum<br>
E.g<br>
<br>
public enum ColumnTypes {<br>
<br>
NULLABLE_STRING(0, "nullable string"),<br>
LONG(1, "long"),<br>
INTEGER(2, "integer"),<br>
NULLABLE_BYTEARRAY(3, "nullable bytearray"),<br>
NULLABLE_TIMESTAMP(4, "nullable timestamp"),<br>
<br>
It breaks a lot of code which I am ok to fix. Want your opinion on this?<br>
<br>
2. Do we want to add support for null constraints on integers, longs and<br>
other data types. I have added support for string, byte array, timestamp.<br>
<br>
3. Once I make changes I want to test if we enforce the not null<br>
constraints in DDL queries. Any suggestions how to go about this?<br>
-------------- next part --------------<br>
An HTML attachment was scrubbed...<br>
URL: <<a href="http://lists.herddb.org/pipermail/herddb-dev/attachments/20190225/5e511da3/attachment-0001.html" rel="noreferrer" target="_blank">http://lists.herddb.org/pipermail/herddb-dev/attachments/20190225/5e511da3/attachment-0001.html</a>><br>
<br>
------------------------------<br>
<br>
Message: 2<br>
Date: Tue, 26 Feb 2019 09:09:55 +0100<br>
From: Enrico Olivelli <<a href="mailto:eolivelli@gmail.com" target="_blank">eolivelli@gmail.com</a>><br>
To: Herddb developers <<a href="mailto:herddb-dev@lists.herddb.org" target="_blank">herddb-dev@lists.herddb.org</a>><br>
Subject: Re: [Herddb-dev] Support not-null constraints<br>
Message-ID:<br>
<<a href="mailto:CACcefgen4obqqswfPVNUFVo4kQfOGQAYqqRbk4ZgSitg9mq%2B7w@mail.gmail.com" target="_blank">CACcefgen4obqqswfPVNUFVo4kQfOGQAYqqRbk4ZgSitg9mq+7w@mail.gmail.com</a>><br>
Content-Type: text/plain; charset="UTF-8"<br>
<br>
Hello Amit,<br>
<br>
Il giorno mar 26 feb 2019 alle ore 08:12 Amit Chavan<br>
<<a href="mailto:achavan1@gmail.com" target="_blank">achavan1@gmail.com</a>> ha scritto:<br>
><br>
> I am working on this ticket Issue-125. Couple of questions -<br>
> 1. Currently ColumnTypes in herddb are part of the ColumnTypes class. I am thinking of using an enum<br>
> E.g<br>
><br>
> public enum ColumnTypes {<br>
><br>
> NULLABLE_STRING(0, "nullable string"),<br>
> LONG(1, "long"),<br>
> INTEGER(2, "integer"),<br>
> NULLABLE_BYTEARRAY(3, "nullable bytearray"),<br>
> NULLABLE_TIMESTAMP(4, "nullable timestamp"),<br>
><br>
><br>
> It breaks a lot of code which I am ok to fix. Want your opinion on this?<br>
<br>
It is better to narrow down the patch to the minimum.<br>
We can create an issue and work on the 'enum' change as a separate ticket.<br>
<br>
><br>
> 2. Do we want to add support for null constraints on integers, longs and other data types. I have added support for string, byte array, timestamp.<br>
<br>
Priorities:<br>
- strings<br>
- int<br>
- long<br>
<br>
then:<br>
- timestamp<br>
<br>
byte array is not really well supported yet<br>
<br>
Anyway I think it is better to support all of them.<br>
<br>
><br>
> 3. Once I make changes I want to test if we enforce the not null constraints in DDL queries. Any suggestions how to go about this?<br>
<br>
<br>
First of all I suggest you to work at a lower level.<br>
HerdDB SQL layer is built on top of the Key-Value low level layer.<br>
<br>
Start from:<br>
<a href="https://github.com/diennea/herddb/blob/master/herddb-core/src/test/java/herddb/core/CreateTableTest.java" rel="noreferrer" target="_blank">https://github.com/diennea/herddb/blob/master/herddb-core/src/test/java/herddb/core/CreateTableTest.java</a><br>
<br>
This test case does not use SQL.<br>
We can support SQL as a second task.<br>
You can start checking from CreateTableStatement.java and so<br>
Table.java and so Column.java.<br>
No change are needed if you are simply introducing new datatypes.<br>
<br>
Let's start with an example, in order to keep the patch as small as<br>
possible, let's not rename every data type, but just add a new<br>
'INTEGER_NOTNULL"<br>
You are going to find all of the usages of ColumnTypes.INTEGER and<br>
there you will handle ColumnTypes.INTEGER_NOTNULL at the same way as<br>
ColumnTypes.INTEGER .<br>
<br>
Probably the only function change you should introduce is in RecordSerializer<br>
<a href="https://github.com/diennea/herddb/blob/82f81c30e036bf3575e676b55a5c019224a33ee8/herddb-core/src/main/java/herddb/codec/RecordSerializer.java#L283" rel="noreferrer" target="_blank">https://github.com/diennea/herddb/blob/82f81c30e036bf3575e676b55a5c019224a33ee8/herddb-core/src/main/java/herddb/codec/RecordSerializer.java#L283</a><br>
<a href="https://github.com/diennea/herddb/blob/82f81c30e036bf3575e676b55a5c019224a33ee8/herddb-core/src/main/java/herddb/codec/RecordSerializer.java#L344" rel="noreferrer" target="_blank">https://github.com/diennea/herddb/blob/82f81c30e036bf3575e676b55a5c019224a33ee8/herddb-core/src/main/java/herddb/codec/RecordSerializer.java#L344</a><br>
<br>
in those points you will have to fail (throw new<br>
IllegalArgumentException by now, a subclass of<br>
StatementExeceptionException in the future) if the object to be<br>
serialized is NULL.<br>
<br>
If you add a new case in CreateTableTest.java I expect to see a new<br>
@Test method which creates a table with an INTEGER_NOTNULL field (non<br>
in primary key) and it executes an INSERT, an UPDATE an assert that<br>
the exeception is thrown<br>
<br>
<br>
Recap:<br>
- start with only a INTEGER_NOTNULL<br>
- start creating a test, failing before your implementation<br>
- do the implementation<br>
- see the test passing<br>
- add tests cases for other "interesting" code paths touched by your<br>
change (like adding tests in RecordSerializer)<br>
<br>
<br>
Great to see you on this list !<br>
Cheers<br>
Enrico<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
> _______________________________________________<br>
> herddb-dev mailing list<br>
> <a href="mailto:herddb-dev@lists.herddb.org" target="_blank">herddb-dev@lists.herddb.org</a><br>
> <a href="http://lists.herddb.org/mailman/listinfo/herddb-dev" rel="noreferrer" target="_blank">http://lists.herddb.org/mailman/listinfo/herddb-dev</a><br>
<br>
<br>
------------------------------<br>
<br>
_______________________________________________<br>
herddb-dev mailing list<br>
<a href="mailto:herddb-dev@lists.herddb.org" target="_blank">herddb-dev@lists.herddb.org</a><br>
<a href="http://lists.herddb.org/mailman/listinfo/herddb-dev" rel="noreferrer" target="_blank">http://lists.herddb.org/mailman/listinfo/herddb-dev</a><br>
<br>
<br>
End of herddb-dev Digest, Vol 8, Issue 4<br>
****************************************<br>
</blockquote></div>
</blockquote></div>