-
Notifications
You must be signed in to change notification settings - Fork 187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add missing numeric precisions in Hive type system #501
Changes from all commits
05d3289
5e05b32
52de757
ad1f5b4
0218a7f
071b434
8bdf787
5b4cd4c
4445fea
5070841
93e873c
0dc5ce1
26304ea
729686d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,15 +11,13 @@ | |
import java.util.Set; | ||
|
||
import org.apache.calcite.rel.type.RelDataType; | ||
import org.apache.calcite.sql.SqlBasicTypeNameSpec; | ||
import org.apache.calcite.sql.SqlCall; | ||
import org.apache.calcite.sql.SqlDataTypeSpec; | ||
import org.apache.calcite.sql.SqlNode; | ||
import org.apache.calcite.sql.SqlOperator; | ||
import org.apache.calcite.sql.SqlTypeNameSpec; | ||
import org.apache.calcite.sql.fun.SqlStdOperatorTable; | ||
import org.apache.calcite.sql.type.BasicSqlType; | ||
import org.apache.calcite.sql.type.SqlTypeName; | ||
import org.apache.calcite.sql.type.SqlTypeUtil; | ||
|
||
import com.linkedin.coral.common.HiveTypeSystem; | ||
import com.linkedin.coral.common.calcite.CalciteUtil; | ||
|
@@ -122,12 +120,6 @@ protected SqlCall transform(SqlCall sqlCall) { | |
} | ||
|
||
private SqlCall castOperand(SqlNode operand, RelDataType relDataType) { | ||
return SqlStdOperatorTable.CAST.createCall(ZERO, operand, getSqlDataTypeSpecForCasting(relDataType)); | ||
} | ||
|
||
private SqlDataTypeSpec getSqlDataTypeSpecForCasting(RelDataType relDataType) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could you add an example of Coral translation for before & after this code change? Since there's no difference in unit tests, does that mean this is a bug or an unnecessary transformation? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
After adding in the precisions, the first case in |
||
final SqlTypeNameSpec typeNameSpec = new SqlBasicTypeNameSpec(relDataType.getSqlTypeName(), | ||
relDataType.getPrecision(), relDataType.getScale(), null, ZERO); | ||
return new SqlDataTypeSpec(typeNameSpec, ZERO); | ||
return SqlStdOperatorTable.CAST.createCall(ZERO, operand, SqlTypeUtil.convertTypeToSpec(relDataType)); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you link the reference for these values?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the javadoc to specify where the values are coming from, this entire class was copied from Hive source code but has now deviated. We have a backlog item mentioned above to eventually rename this to
CoralTypeSystem
.