Add facility to handle non null column type #359
Conversation
|
@eolivelli for the UT in jdbc module is it new class or I need to add more tests to existing classes of unit tests? |
| String data_type = ColumnTypes.typeToString(c.type); | ||
|
|
||
| result.add(RecordSerializer.makeRecord( | ||
| table, | ||
| "table_name", t.name, | ||
| "column_name", c.name, | ||
| "ordinal_position", pos++, | ||
| "is_nullable", pk ? 0 : 1, | ||
| "is_nullable", (pk || nonNullCType) ? DatabaseMetaData.columnNoNulls : DatabaseMetaData.columnNullable, |
There was a problem hiding this comment.
isn't "pl" reduntant here ?
if ("pk") nonNullCType is already true
There was a problem hiding this comment.
You are correct. I removed it.
|
@amitvc good idea to split your work in smaller subtasks |
|
In the context of this patch you can add only a testcase in SystemTablesTest JDBC clients won't be impacted directly, they will only "see" this new constraint, but it is already handled, as it was handled for pk columns. |
|
SystemTablesTest class is present in both core and jdbc. I am assuming adding tests in the one that is in the core project. |
…ests for systemtablestest
|
@eolivelli - I had to modify DDLSQLPlanner.java to add support for not null types when specified in query string. Also added UT to test my changes. |
| case LONG: | ||
| return NOTNULL_LONG; | ||
| default: | ||
| return type; |
There was a problem hiding this comment.
not sure this is the best return value, what about am IllegalArgumentException ?
Other wide it is better to add a Javadoc
There was a problem hiding this comment.
I modified the method to add more types that are supported in ColumnTypes.
…rted not null types
|
I left one last comment |
|
@eolivelli - Yes the scan won't get executed. I removed it to make it more obvious what we are testing in the UT |
|
@eolivelli thanks for the review. |
Following this checklist to help us incorporate your
contribution quickly and easily:
mvn clean verifyto make sure basic checks pass. A more thorough check willbe performed on your pull request automatically.
If your pull request is about ~20 lines of code you don't need to sign an iCLA if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.