[
https://issues.apache.org/jira/browse/DRILL-6903?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Arina Ielchiieva updated DRILL-6903:
------------------------------------
Description:
SchemaBuilder code will be moved to the exec package from test in DRILL-6901.
There are a couple of improvements that can be done in the existing code:
*1. ColumnBuilder: OPTIONAL vs REQUIRED*
{{ColumnBuilder}} constructor sets mode as REQUIRED by default. We might
consider setting OPTIONAL as default.
[https://github.com/apache/drill/blob/master/exec/java-exec/src/test/java/org/apache/drill/test/rowSet/schema/ColumnBuilder.java#L40]
*2. ColumnBuilder: setScale method*
{{setScale}} method is a bit awkward, it requires precision as second
parameter. More natural to go with precision and then scale.
Suggestion is to have {{setPrecisionAndScale}} method instead which will
accept precision and scale as first and second parameters.
[https://github.com/apache/drill/blob/master/exec/java-exec/src/test/java/org/apache/drill/test/rowSet/schema/ColumnBuilder.java#L57]
*3. SchemaContainer: addColumn method*
{{addColumn}} method has parameter {{AbstractColumnMetadata}}, since we have
interface {{ColumnMetadata}}, it's better to operate on the interface level
rather than on the abstract class.
[https://github.com/apache/drill/blob/master/exec/java-exec/src/test/java/org/apache/drill/test/rowSet/schema/SchemaContainer.java#L28]
*4. MapBuilder / RepeatedListBuilder / UnionBuilder: buildCol method*
{{buildCol}} method is private in these classes. These classes create columns
and add them to the schema. There might be use cases when dev needs only column
and will add it to the schema when needed. Suggestion is to make {{buildCol}}
method public. Also all these classes require {{SchemaContainer}} as parent,
though when we only need them to build the column, second constructor can be
added without {{SchemaContainer}} parameter.
was:
SchemaBuilder code will be moved to the exec package from test in DRILL-6901.
There are a couple of improvements that can be done in the existing code:
*1. ColumnBuilder: OPTIONAL vs REQUIRED*
{{ColumnBuilder}} constructor sets mode as REQUIRED by default. We might
consider setting OPTIONAL as default.
https://github.com/apache/drill/blob/master/exec/java-exec/src/test/java/org/apache/drill/test/rowSet/schema/ColumnBuilder.java#L40
*2. ColumnBuilder: setScale method*
{{setScale}} method is a bit awkward, it requires precision as second
parameter. More natural to go with precision and then scale.
Suggestion is to have {{setPrecisionAndScale}} method instead which will accept
precision and scale as first and second parameters.
https://github.com/apache/drill/blob/master/exec/java-exec/src/test/java/org/apache/drill/test/rowSet/schema/ColumnBuilder.java#L57
*3. SchemaContainer: addColumn method*
{{addColumn}} method has parameter {{AbstractColumnMetadata}}, since we have
interface {{ColumnMetadata}}, it's better to operate on the interface level
rather than on the abstract class.
https://github.com/apache/drill/blob/master/exec/java-exec/src/test/java/org/apache/drill/test/rowSet/schema/SchemaContainer.java#L28
*4. MapBuilder / RepeatedListBuilder / UnionBuilder: buildCol method*
{{buildCol}} method is private in these classes. These classes create columns
and add them to the schema. There might be use cases when dev needs only column
and will add it to the schema when needed. Suggestion is to make {{buildCol}}
method public. Also all these classes require `SchemaContainer` as parent,
though when we only need them to build the column, second constructor can be
added without `SchemaContainer` parameter.
> SchemaBuilder improvements
> --------------------------
>
> Key: DRILL-6903
> URL: https://issues.apache.org/jira/browse/DRILL-6903
> Project: Apache Drill
> Issue Type: Bug
> Reporter: Arina Ielchiieva
> Priority: Major
> Fix For: 1.16.0
>
>
> SchemaBuilder code will be moved to the exec package from test in DRILL-6901.
> There are a couple of improvements that can be done in the existing code:
> *1. ColumnBuilder: OPTIONAL vs REQUIRED*
> {{ColumnBuilder}} constructor sets mode as REQUIRED by default. We might
> consider setting OPTIONAL as default.
>
> [https://github.com/apache/drill/blob/master/exec/java-exec/src/test/java/org/apache/drill/test/rowSet/schema/ColumnBuilder.java#L40]
> *2. ColumnBuilder: setScale method*
> {{setScale}} method is a bit awkward, it requires precision as second
> parameter. More natural to go with precision and then scale.
> Suggestion is to have {{setPrecisionAndScale}} method instead which will
> accept precision and scale as first and second parameters.
>
> [https://github.com/apache/drill/blob/master/exec/java-exec/src/test/java/org/apache/drill/test/rowSet/schema/ColumnBuilder.java#L57]
> *3. SchemaContainer: addColumn method*
> {{addColumn}} method has parameter {{AbstractColumnMetadata}}, since we have
> interface {{ColumnMetadata}}, it's better to operate on the interface level
> rather than on the abstract class.
>
> [https://github.com/apache/drill/blob/master/exec/java-exec/src/test/java/org/apache/drill/test/rowSet/schema/SchemaContainer.java#L28]
> *4. MapBuilder / RepeatedListBuilder / UnionBuilder: buildCol method*
> {{buildCol}} method is private in these classes. These classes create
> columns and add them to the schema. There might be use cases when dev needs
> only column and will add it to the schema when needed. Suggestion is to make
> {{buildCol}} method public. Also all these classes require
> {{SchemaContainer}} as parent, though when we only need them to build the
> column, second constructor can be added without {{SchemaContainer}} parameter.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
