You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@metamodel.apache.org by GitBox <gi...@apache.org> on 2020/07/02 07:23:25 UTC

[GitHub] [metamodel] arjansh commented on a change in pull request #241: (METAMODEL-1226) Metadata for numeric data types does not include num…

arjansh commented on a change in pull request #241:
URL: https://github.com/apache/metamodel/pull/241#discussion_r448797969



##########
File path: core/src/main/java/org/apache/metamodel/schema/ImmutableColumn.java
##########
@@ -71,12 +72,13 @@
      *            whether the column is a primary key or not
      */
     public ImmutableColumn(String name, ColumnType type, Table table, int columnNumber, Integer columnSize,
-            String nativeType, Boolean nullable, String remarks, boolean indexed, String quote, boolean primaryKey) {
+                           Integer decimalDigits, String nativeType, Boolean nullable, String remarks, boolean indexed, String quote, boolean primaryKey) {

Review comment:
       By adding a parameter to this constructor, the code will cause compilation errors in other project using this constructor. Can you please add an extra constructor with the signature of the original version of this constructor, which invokes this constructor with a default value for `decimalDigits`? In that manner you also don't have to change all the unit test code.

##########
File path: core/src/main/java/org/apache/metamodel/schema/MutableColumn.java
##########
@@ -70,9 +71,10 @@ public MutableColumn(String name, ColumnType type, Table table, int columnNumber
     }
 
     public MutableColumn(String name, ColumnType type, Table table, int columnNumber, Integer columnSize,
-            String nativeType, Boolean nullable, String remarks, boolean indexed, String quote) {
+                         Integer decimalDigits, String nativeType, Boolean nullable, String remarks, boolean indexed, String quote) {

Review comment:
       Same comment as for the `ImmutableColumn` constructor change, by adding a parameter to this constructor, the code will cause compilation errors in other project using this constructor. Can you please add an extra constructor with the signature of the original version of this constructor, which invokes this constructor with a default value for `decimalDigits`?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org