[jira] [Commented] (DRILL-6421) Refactor DecimalUtility and CoreDecimalUtility classes
[
https://issues.apache.org/jira/browse/DRILL-6421?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16481027#comment-16481027
]
ASF GitHub Bot commented on DRILL-6421:
---
asfgit closed pull request #1267: DRILL-6421: Refactor DecimalUtility and
CoreDecimalUtility classes
URL: https://github.com/apache/drill/pull/1267
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/common/src/main/java/org/apache/drill/common/types/Types.java
b/common/src/main/java/org/apache/drill/common/types/Types.java
index 5c6e7f3b97..e66a340f1f 100644
--- a/common/src/main/java/org/apache/drill/common/types/Types.java
+++ b/common/src/main/java/org/apache/drill/common/types/Types.java
@@ -27,12 +27,11 @@
import org.apache.drill.common.types.TypeProtos.DataMode;
import org.apache.drill.common.types.TypeProtos.MajorType;
import org.apache.drill.common.types.TypeProtos.MinorType;
-import org.apache.drill.common.util.CoreDecimalUtility;
import com.google.protobuf.TextFormat;
+@SuppressWarnings("WeakerAccess")
public class Types {
- static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(Types.class);
public static final int MAX_VARCHAR_LENGTH = 65535;
public static final int UNDEFINED = 0;
@@ -47,10 +46,6 @@ public static boolean isUnion(MajorType toType) {
return toType.getMinorType() == MinorType.UNION;
}
- public enum Comparability {
-UNKNOWN, NONE, EQUAL, ORDERED
- }
-
public static boolean isComplex(final MajorType type) {
switch(type.getMinorType()) {
case LIST:
@@ -90,6 +85,37 @@ public static boolean isNumericType(final MajorType type) {
case UINT4:
case UINT8:
return true;
+default:
+ return false;
+}
+ }
+
+ /**
+ * Returns true if specified type is decimal data type.
+ *
+ * @param type type to check
+ * @return true if specified type is decimal data type.
+ */
+ public static boolean isDecimalType(MajorType type) {
+return isDecimalType(type.getMinorType());
+ }
+
+ /**
+ * Returns true if specified type is decimal data type.
+ *
+ * @param minorType type to check
+ * @return true if specified type is decimal data type.
+ */
+ public static boolean isDecimalType(MinorType minorType) {
+switch(minorType) {
+ case VARDECIMAL:
+ case DECIMAL38SPARSE:
+ case DECIMAL38DENSE:
+ case DECIMAL28SPARSE:
+ case DECIMAL28DENSE:
+ case DECIMAL18:
+ case DECIMAL9:
+return true;
default:
return false;
}
@@ -182,11 +208,9 @@ public static String getBaseSqlTypeName(final MajorType
type) {
* Extend decimal type with precision and scale.
*
* @param type major type
- * @param typeName type converted to a string
* @return type name augmented with precision and scale,
* if type is a decimal
*/
-
public static String getExtendedSqlTypeName(MajorType type) {
String typeName = getSqlTypeName(type);
@@ -266,7 +290,7 @@ public static int getJdbcTypeCode(final String sqlTypeName)
{
/**
* Reports whether given RPC-level type is a signed type (per semantics of
- * {@link ResultSetMetaData#isSigned(int)}).
+ * {@link java.sql.ResultSetMetaData#isSigned(int)}).
*/
public static boolean isJdbcSignedType( final MajorType type ) {
final boolean isSigned;
@@ -477,41 +501,6 @@ public static boolean isScalarStringType(final MajorType
type) {
}
}
- public static boolean isBytesScalarType(final MajorType type) {
-if (type.getMode() == REPEATED) {
- return false;
-}
-switch(type.getMinorType()) {
-case FIXEDBINARY:
-case VARBINARY:
- return true;
-default:
- return false;
-}
- }
-
- public static Comparability getComparability(final MajorType type) {
-if (type.getMode() == REPEATED) {
- return Comparability.NONE;
-}
-if (type.getMinorType() == MinorType.LATE) {
- return Comparability.UNKNOWN;
-}
-
-switch(type.getMinorType()) {
-case LATE:
- return Comparability.UNKNOWN;
-case MAP:
- return Comparability.NONE;
-case BIT:
- return Comparability.EQUAL;
-default:
- return Comparability.ORDERED;
-}
-
- }
-
-
public static boolean softEquals(final MajorType a, final MajorType b, final
boolean allowNullSwap) {
if (a.getMinorType() != b.getMinorType()) {
return false;
@@ -532,10 +521,6 @@ public static boolean softEquals(final MajorType a, final
MajorType b, final boo
return a.getMode() == b.getMode();
}
- public static boolean isLateBind(final MajorType type) {
-return type.getMinorType() == MinorType.LATE;
- }
-
public static boolean isU
[jira] [Commented] (DRILL-6421) Refactor DecimalUtility and CoreDecimalUtility classes
[
https://issues.apache.org/jira/browse/DRILL-6421?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16479203#comment-16479203
]
ASF GitHub Bot commented on DRILL-6421:
---
arina-ielchiieva commented on issue #1267: DRILL-6421: Refactor DecimalUtility
and CoreDecimalUtility classes
URL: https://github.com/apache/drill/pull/1267#issuecomment-389907430
Looks good, thanks for making the changes. +1
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Refactor DecimalUtility and CoreDecimalUtility classes
> --
>
> Key: DRILL-6421
> URL: https://issues.apache.org/jira/browse/DRILL-6421
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Labels: ready-to-commit
> Fix For: 1.14.0
>
>
> After the changes, made in DRILL-6094, most of the methods in
> {{DecimalUtility}} class became unused, so they should be removed.
> Both {{DecimalUtility}} and {{CoreDecimalUtility}} are intended for the same
> goals, so they should be merged.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-6421) Refactor DecimalUtility and CoreDecimalUtility classes
[
https://issues.apache.org/jira/browse/DRILL-6421?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16479042#comment-16479042
]
ASF GitHub Bot commented on DRILL-6421:
---
vvysotskyi commented on a change in pull request #1267: DRILL-6421: Refactor
DecimalUtility and CoreDecimalUtility classes
URL: https://github.com/apache/drill/pull/1267#discussion_r188955295
##
File path:
exec/vector/src/main/java/org/apache/drill/exec/util/DecimalUtility.java
##
@@ -320,25 +203,25 @@ public static void getSparseFromBigDecimal(BigDecimal
input, ByteBuf data, int s
int destIndex = nDecimalDigits - roundUp(scale) - 1;
-// we use base 1 billion integer digits for out integernal representation
+// we use base 1 billion integer digits for out internal representation
BigDecimal base = new BigDecimal(DIGITS_BASE);
-while (integerPart.compareTo(BigDecimal.ZERO) == 1) {
-// store the modulo as the integer value
-data.setInt(startIndex + (destIndex * INTEGER_SIZE),
(integerPart.remainder(base)).intValue());
-destIndex--;
-// Divide by base 1 billion
-integerPart = (integerPart.divide(base)).setScale(0,
BigDecimal.ROUND_DOWN);
+while (integerPart.compareTo(BigDecimal.ZERO) > 0) {
Review comment:
Good catch! Agreed that for `BidDecimal` it returns 1, thanks for pointing
this, reverted the change.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Refactor DecimalUtility and CoreDecimalUtility classes
> --
>
> Key: DRILL-6421
> URL: https://issues.apache.org/jira/browse/DRILL-6421
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.14.0
>
>
> After the changes, made in DRILL-6094, most of the methods in
> {{DecimalUtility}} class became unused, so they should be removed.
> Both {{DecimalUtility}} and {{CoreDecimalUtility}} are intended for the same
> goals, so they should be merged.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-6421) Refactor DecimalUtility and CoreDecimalUtility classes
[
https://issues.apache.org/jira/browse/DRILL-6421?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16479028#comment-16479028
]
ASF GitHub Bot commented on DRILL-6421:
---
arina-ielchiieva commented on a change in pull request #1267: DRILL-6421:
Refactor DecimalUtility and CoreDecimalUtility classes
URL: https://github.com/apache/drill/pull/1267#discussion_r188950847
##
File path:
exec/vector/src/main/java/org/apache/drill/exec/util/DecimalUtility.java
##
@@ -320,25 +203,25 @@ public static void getSparseFromBigDecimal(BigDecimal
input, ByteBuf data, int s
int destIndex = nDecimalDigits - roundUp(scale) - 1;
-// we use base 1 billion integer digits for out integernal representation
+// we use base 1 billion integer digits for out internal representation
BigDecimal base = new BigDecimal(DIGITS_BASE);
-while (integerPart.compareTo(BigDecimal.ZERO) == 1) {
-// store the modulo as the integer value
-data.setInt(startIndex + (destIndex * INTEGER_SIZE),
(integerPart.remainder(base)).intValue());
-destIndex--;
-// Divide by base 1 billion
-integerPart = (integerPart.divide(base)).setScale(0,
BigDecimal.ROUND_DOWN);
+while (integerPart.compareTo(BigDecimal.ZERO) > 0) {
Review comment:
Not saying, this change is not correct but `compareTo` for BidDecimal
returns 1.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Refactor DecimalUtility and CoreDecimalUtility classes
> --
>
> Key: DRILL-6421
> URL: https://issues.apache.org/jira/browse/DRILL-6421
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.14.0
>
>
> After the changes, made in DRILL-6094, most of the methods in
> {{DecimalUtility}} class became unused, so they should be removed.
> Both {{DecimalUtility}} and {{CoreDecimalUtility}} are intended for the same
> goals, so they should be merged.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (DRILL-6421) Refactor DecimalUtility and CoreDecimalUtility classes
[
https://issues.apache.org/jira/browse/DRILL-6421?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16478767#comment-16478767
]
ASF GitHub Bot commented on DRILL-6421:
---
vvysotskyi opened a new pull request #1267: DRILL-6421: Refactor DecimalUtility
and CoreDecimalUtility classes
URL: https://github.com/apache/drill/pull/1267
- Removed unused methods
- Added javadocs for some methods
- Removed CoreDecimalUtility class
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Refactor DecimalUtility and CoreDecimalUtility classes
> --
>
> Key: DRILL-6421
> URL: https://issues.apache.org/jira/browse/DRILL-6421
> Project: Apache Drill
> Issue Type: Task
>Reporter: Volodymyr Vysotskyi
>Assignee: Volodymyr Vysotskyi
>Priority: Major
> Fix For: 1.14.0
>
>
> After the changes, made in DRILL-6094, most of the methods in
> {{DecimalUtility}} class became unused, so they should be removed.
> Both {{DecimalUtility}} and {{CoreDecimalUtility}} are intended for the same
> goals, so they should be merged.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
