HIVE-27157: AssertionError when inferring return type for unix_timestamp function #4135
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…amp function Calls to inferReturnType method for unix_timestamp operators always lead to AssertionError. Contrary to operand type checking and operand type inference that are not really relevant for Hive (the latter is not using the SqlValidator logic), the return type inference is important since it may kick in some calls to RelBuilder/RexBuilder APIs. Such calls exist in older versions of Hive and are widely used in Calcite's built-in rules. Change the implementation of unix_timestamp operators to avoid the AssertionError and infer the return type correctly; always BIGINT. Break the inheritance relation with SqlAbstractTimeFunction and change the SqlFunctionCategory from TIMEDATE to NUMERIC; unix_timestamp is not a time function since the result is never among DATE, TIME, or TIMESTAMP. Change the operant type checker to a more truthful implementation; the type checker is not really used at the moment but it is better to have something realistic there instead of null or something completely wrong. Change the function syntax from FUNCTION_ID to FUNCTION and update some out files. Not a must do but again there is no reason to omit parentheses from a regular function.
|
Kudos, SonarCloud Quality Gate passed!
|
kasakrisz
reviewed
Mar 22, 2023
...org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveToUnixTimestampSqlOperator.java
Show resolved
Hide resolved
henrib
pushed a commit
to henrib/hive
that referenced
this pull request
Apr 24, 2023
…amp function (Stamatis Zampetakis reviewed by Krisztian Kasa) Calls to inferReturnType method for unix_timestamp operators always lead to AssertionError. Contrary to operand type checking and operand type inference that are not really relevant for Hive (the latter is not using the SqlValidator logic), the return type inference is important since it may kick in some calls to RelBuilder/RexBuilder APIs. Such calls exist in older versions of Hive and are widely used in Calcite's built-in rules. Change the implementation of unix_timestamp operators to avoid the AssertionError and infer the return type correctly; always BIGINT. Break the inheritance relation with SqlAbstractTimeFunction and change the SqlFunctionCategory from TIMEDATE to NUMERIC; unix_timestamp is not a time function since the result is never among DATE, TIME, or TIMESTAMP. Change the operant type checker to a more truthful implementation; the type checker is not really used at the moment but it is better to have something realistic there instead of null or something completely wrong. Change the function syntax from FUNCTION_ID to FUNCTION and update some out files. Not a must do but again there is no reason to omit parentheses from a regular function. Closes apache#4135
yeahyung
pushed a commit
to yeahyung/hive
that referenced
this pull request
Jul 20, 2023
…amp function (Stamatis Zampetakis reviewed by Krisztian Kasa) Calls to inferReturnType method for unix_timestamp operators always lead to AssertionError. Contrary to operand type checking and operand type inference that are not really relevant for Hive (the latter is not using the SqlValidator logic), the return type inference is important since it may kick in some calls to RelBuilder/RexBuilder APIs. Such calls exist in older versions of Hive and are widely used in Calcite's built-in rules. Change the implementation of unix_timestamp operators to avoid the AssertionError and infer the return type correctly; always BIGINT. Break the inheritance relation with SqlAbstractTimeFunction and change the SqlFunctionCategory from TIMEDATE to NUMERIC; unix_timestamp is not a time function since the result is never among DATE, TIME, or TIMESTAMP. Change the operant type checker to a more truthful implementation; the type checker is not really used at the moment but it is better to have something realistic there instead of null or something completely wrong. Change the function syntax from FUNCTION_ID to FUNCTION and update some out files. Not a must do but again there is no reason to omit parentheses from a regular function. Closes apache#4135
tarak271
pushed a commit
to tarak271/hive-1
that referenced
this pull request
Dec 19, 2023
…amp function (Stamatis Zampetakis reviewed by Krisztian Kasa) Calls to inferReturnType method for unix_timestamp operators always lead to AssertionError. Contrary to operand type checking and operand type inference that are not really relevant for Hive (the latter is not using the SqlValidator logic), the return type inference is important since it may kick in some calls to RelBuilder/RexBuilder APIs. Such calls exist in older versions of Hive and are widely used in Calcite's built-in rules. Change the implementation of unix_timestamp operators to avoid the AssertionError and infer the return type correctly; always BIGINT. Break the inheritance relation with SqlAbstractTimeFunction and change the SqlFunctionCategory from TIMEDATE to NUMERIC; unix_timestamp is not a time function since the result is never among DATE, TIME, or TIMESTAMP. Change the operant type checker to a more truthful implementation; the type checker is not really used at the moment but it is better to have something realistic there instead of null or something completely wrong. Change the function syntax from FUNCTION_ID to FUNCTION and update some out files. Not a must do but again there is no reason to omit parentheses from a regular function. Closes apache#4135
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Change the implementation of unix_timestamp operators to avoid the AssertionError and infer the return type correctly; always BIGINT.
Break the inheritance relation with SqlAbstractTimeFunction and change the SqlFunctionCategory from TIMEDATE to NUMERIC; unix_timestamp is not a time function since the result is never among DATE, TIME, or TIMESTAMP.
Change the operand type checker to a more truthful implementation; the type checker is not really used at the moment but it is better to have something realistic there instead of null or something completely wrong.
Change the function syntax from FUNCTION_ID to FUNCTION and update some .q.out files. Not a must do but again there is no reason to omit parentheses from a regular function.
Why are the changes needed?
Calls to
inferReturnTypemethod forunix_timestampoperators always lead toAssertionError.Contrary to operand type checking and operand type inference that are not really relevant for Hive (the latter is not using the
SqlValidatorlogic), the return type inference is important since it may kick in some calls toRelBuilder/RexBuilderAPIs. Such calls exist in older versions of Hive and are widely used in Calcite's built-in rules.Does this PR introduce any user-facing change?
In this version no, but in older versions of Hive it can fix queries failing with
AssertionError.How was this patch tested?